NFD

As discussed in the NFD call on July 31, 2019, the third design (with some changes) involves the minimum codebase update. More discussions are needed on next Monday's NFD call.

Here is the basic workflow of how self-learning works with face switching:

1. A consumer multicasts "discovery" Interests to all potential faces, if the consumer has no routes.
2. On receiving Data packets, self-learning checks their incoming Face.

• if the Face is unicast, self-learning add routes towards the Face (with the same behaviors as in #4279)
• if the Face is multicast, self-learning creates a unicast face towards the Data sender, and adds routes routes towards the unicast Face.

In this design, face creation is at consumers, and the first Interest-Data exchange is symmetric.

The main task is to create unicast face on receiving data from multicast face.

To allow fw strategy create a unicast UDP face from a multicast UDP face in NFD, here are the tasks:

1. in Nfd class, add a static method getFaceManager to access the FaceManager instance
2. in FaceManager class, add a method createUnicastFromMulticastFace, that accepts three parameters FaceId, onSuccessCallback, and onFailureCallback.

• In this method, the unicast face parameters are obtained from the multicast face, that is found by FaceId and Face::getTransport
• After getting all necessary information, try to create the unicast face, if success, invoke onSuccessCallback with the new unicast Face as the parameter, otherwise invoke onFailureCallback with the multicast Face as the parameter.

Update in self-learning fw strategy:

• the addRoute method needs to moved to the callback functions

Teng Liang wrote:

1. in Nfd class, add a static method getFaceManager to access the FaceManager instance

Don't like this. Static stuff is evil (in most cases) and makes dependency injection a lot harder if not impossible. Why can't you pass a FaceManager& to the objects that need it? (but also see below)

1. in FaceManager class, add a method createUnicastFromMulticastFace, that accepts three parameters FaceId, onSuccessCallback, and onFailureCallback.

You need to explain this point in a lot more detail... face creation triggers a number of other things, you need to convince us that this is all going to work out, and for that you need to be more specific.

Also, you haven't justified why you need to go through management to create the face.

In this method, the unicast face parameters are obtained from the multicast face, that is found by FaceId and Face::getTransport

What parameters exactly? How will you initialize those parameters that make sense only for unicast faces?

Davide Pesavento wrote:

Also, you haven't justified why you need to go through management to create the face.

Looking into the details of FaceManager::createFace, it is not necessary to go though FaceManager in this case. Fw only needs to accessFaceTable and FaceSystem for face creation.

Then, the question is how to pass the FaceManager object in Nfd class to the forwarder object. One option is to make m_faceSystem shared_ptr, and call forwarder.setFaceSystem after it is initialized. However, this way does not seem elegant enough.

As discussed in the NFD call on Oct. 2 and in another call with Junxiao, we nailed down the implementation design details:

STEP ONE
When packet arrives at a multicast face, the packet is tagged:

• which ProtocolFactory owns the multicast face?
• what's the remote endpoint? (format defined within ProtocolFactory)

STEP TWO: strategy requests unicast face creation

void
MyStrategy::processData(const PitEntry& pitEntry, const Data& data)
{
    // 1. prolong PIT entry lifetime, so that PIT entry still exists when face is created

    // 2. request face creation
    this->createUnicastFaceForPacketSender(pitEntry, data);
}

STEP THREE: forwarding requests face creation from Forwarder to FaceTable

void
Forwarder::createUnicastFaceForPacketSender(const PitEntry& pitEntry, const Packet& pkt)
{
    faceTable.createUnicastFaceForPacketSender(pkt, [] (const Face& face) {
        // STEP FIVE

        // 1. if PIT entry has been deleted, return
        // 2. find effective strategy for pitEntry.getName()
        // 3. strategy.afterUnicastFaceCreation(pitEntry, face)
    });
}

class FaceTable
{
public:
    signal::Signal<FaceTable, Packet, function<void(const Face& face)>> requestCreateUnicastFaceForPacketSender;

    void
    createUnicastFaceForPacketSender(const Packet& pkt, function<void(const Face& face)> callback)
    {
        requestCreateUnicastFaceForPacketSender(pkt, callback);
    }
};

STEP FOUR: FaceSystem asks ProtocolFactory to create face

• note : this is hooked to faceTable.requestCreateUnicastFaceForPacketSender signal

void
FaceSystem::createUnicastFaceForPacketSender(const Packet& pkt, function<void(const Face& face)> callback)
{
    // 1. extract packet tag and determine which ProtocolFactory owns the multicast face
    // 2. pass remote endpoint information to the ProtocolFactory, which then creates the face
    ProtocolFactory.createFace(remoteEndpoint, callback);
}

STEP FIVE is in the callback function in the step three.

STEP SIX: use the newly created face in the callback function

void
MyStrategy::afterUnicastFaceCreation(const PitEntry& pitEntry, const Face& face)
{
}

class Strategy {
public:
    void afterUnicastFaceCreation(const PitEntry& pitEntry, const Face& face);

protected:
    void createUnicastFaceForPacketSender(const PitEntry& pitEntry, const Packet& pkt);
};

An alternative design is to pass FaceSystem to Forwarder. This is simpler as Davide mentioned. The downside is that FaceSystem has to be created for Forwarder unit testing, which is acceptable to me

STEP ONE
When Data arrives at a multicast face, the Data is tagged:

• the ProtocolFactory that owns the multicast face
• the remote uri (format defined within ProtocolFactory)

STEP TWO: strategy requests unicast face creation

void
SelfLearningStrategy::afterReceiveData(const shared_ptr<pit::Entry>& pitEntry, const FaceEndpoint& ingress, const Data& data)
{
     // 1. check if the unicast face needs to be created
     // 2, prolong PIT entry lifetime, so that PIT entry still exists when face is created
     // 3. request face creation 
     this->createUnicastFaceOnData(pitEntry, data);
}

STEP THREE: Forwarder forwards face creation request to FaceSystem

void
Forwarder::createUnicastFaceOnData(const PitEntry& pitEntry, const Data& data)
{
    faceSystem.createUnicastFaceOnData(data, [] (const Face& face) {
        // STEP FIVE

        // 1. if PIT entry has been deleted, return
        // 2. find effective strategy for pitEntry.getName()
        // 3. strategy.afterUnicastFaceCreation(pitEntry, face)
    });
}

STEP FOUR: FaceSystem asks ProtocolFactory to create face

void
FaceSystem::createUnicastFaceOnData(const Data& data, function<void(const Face& face)> callback)
{
    // 1. extract packet tag and determine which ProtocolFactory owns the multicast face
    // 2. pass remote endpoint information to the ProtocolFactory, which then creates the face
    ProtocolFactory.createFace(remoteEndpoint, callback);
}

STEP FIVE is in the callback function in the step three.

STEP SIX: use the newly created face in the callback function

void
SelfLearningStrategy::afterUnicastFaceCreation(const PitEntry& pitEntry, const Face& face)
{
  // add route towards the face
}

class Strategy {
public:
    void afterUnicastFaceCreation(const PitEntry& pitEntry, const Face& face);

protected:
    void createUnicastFaceOnData(const PitEntry& pitEntry, const Data& data);
};

Signals are for notifications.

False. Face type has signals to start forwarding pipeline processing of incoming packets.

Moreover, it adds another unnecessary layer of indirection and additional complexity. Just use FaceSystem to create the face.

Using FaceSystem causes significant complexity in isolating forwarding for unit testing.
Unless, you define BaseFaceSystem as an abstract class with no implementation, and provide a complete mock alongside the real thing. Bottom line is, forwarding unit testing cannot rely on any of the protocol factories.

after face creation you need to register a route on that face (which is also async), maybe the two operations can be combined and the strategy be re-invoked only once at the end of the combined operation?

Yes.

Junxiao Shi wrote:

Signals are for notifications.

False. Face type has signals to start forwarding pipeline processing of incoming packets.

Nope. You're confusing the signal with its handlers. Face's signals merely notify of the arrival of a packet, what happens after is defined by the handler (or handlers), not by the signal.

Moreover, it adds another unnecessary layer of indirection and additional complexity. Just use FaceSystem to create the face.

Using FaceSystem causes significant complexity in isolating forwarding for unit testing.

Complexity in unit tests is preferable to complexity in the actual forwarder. Tests come second.

Unless, you define BaseFaceSystem as an abstract class with no implementation, and provide a complete mock alongside the real thing.

Which would be the proper testing procedure anyway. We hardly mock anything in our current unit tests out of laziness. Besides, in order to create a face, Forwarder will only need one or two functions from FaceSystem (depending on how Teng designs this part, but it's a minor detail), so you won't need to abstract the entire FaceSystem class and you won't need a "complete mock". It's really quite simple in fact.

Complexity in unit tests is preferable to complexity in the actual forwarder. Tests come second.

No. Test coverage is important for software quality.

Unless, you define BaseFaceSystem as an abstract class with no implementation, and provide a complete mock alongside the real thing.

Which would be the proper testing procedure anyway. We hardly mock anything in our current unit tests out of laziness. Besides, in order to create a face, Forwarder will only need one or two functions from FaceSystem (depending on how Teng designs this part, but it's a minor detail), so you won't need to abstract the entire FaceSystem class and you won't need a "complete mock". It's really quite simple in fact.

I'm talking more about non-self-learning tests. They should not require creation of the real FaceSystem along with all the protocol factories.

Junxiao Shi wrote:

Complexity in unit tests is preferable to complexity in the actual forwarder. Tests come second.

No. Test coverage is important for software quality.

https://yourlogicalfallacyis.com/strawman

None of what I suggested above has the effect of reducing test coverage.

STEP one in note-8 does not describe implementation details. The target is to get ProtocolFactory and RemoteUri when calling functions in FaceSystem, in order to create a unicast UDP/Ether face.

• To get ProtocolFactory, one way is to add ProtocolFactory Id to Transport, then FaceSystem is able to get ProtocolFactory from Face->Transport->ProtocolFactory Id
• The Remote Uri can be attached to Data by the sender. This should only happen to discovery Interest sent to multicast Face.

note-13 design makes sense.

Teng Liang wrote:

• The Remote Uri can be attached to Data by the sender. This should only happen to discovery Interest sent to multicast Face.

I strongly disagree with this for a number of reasons. But we already discussed another (better) approach at today's call so I don't need to elaborate.

In the current design, SlStrategy calls face->channel->connect() to create a unicast face on receiving data from a multicast face. However, the base Channel class does not have connect method, thus the the compiler cannot find the call. To solve the issue, a virtual method connect is added to the base class.