NFD

Why did the priority for this issue get lowered? It remains important to the application team for trying to make field deployments of NDN more resilient to connectivity changes. Also, this appears to be targeted for v0.5 while its dependencies (#3353 and #3522) are targeted for v0.6. Please revise the target version for the dependencies to match.

I was suggesting this target v0.5, not the other way around.

When the configuration file is loaded or reloaded

As of NFD 0.5.0, NIC addition/removal triggers a partial configuration reload of face_system section. When we have NetworkMonitor (#3353), do we need to change anything in this part?

Junxiao Shi wrote:

As of NFD 0.5.0, NIC addition/removal triggers a partial configuration reload of face_system section. When we have NetworkMonitor (#3353), do we need to change anything in this part?

I believe we should stop doing that. It's rather non-standard behavior and could be unexpected/confusing to some. The config file should be reloaded only upon an explicit user action (e.g. SIGHUP or SIGUSR).

The new NetworkMonitor should be wired up to NFD, so that addition and removal of network interfaces result in the creation and destruction of the corresponding multicast and NIC-associated faces, when needed and allowed by the config file.

In NFD configuration file, add an option that specifies zero or more remote FaceUris to which NIC-associated permanent faces will be created
[...]
NFD creates and destroys NIC-associated permanent faces as necessary to maintain one NIC-associated permanent face per NIC per remote FaceUri.

Does this mean that if I have two NICs and configure two remote URIs, I get four NIC-associated faces? What's the rationale? I doubt that's the expected behavior in the vast majority of the cases.

What we want to be able to do, at least for our V2I use cases, is: specify one remote URI (udp4: or udp6:) for each network interface available on the system, or a subset of them, and have NIC-associated face created on it.
A network interface would be identified by its name, or optionally by its hardware address if you want. We don't currently need an IP/subnet match like in the multicast faces whitelist, but I can see a scenario where one wants to use different remote URIs depending on the IP address assigned by the network, in other words, a different remote URI for each upstream provider/ISP (although identifying that via the assigned IP would not be 100% reliable).
We also don't need more than one remote URI per NIC at the moment, but I would include multiple remotes in the design, at least to make sure the config file syntax is sufficiently extensible to eventually support that. In any case each network interface should have its own independent set of remote URIs, i.e. no cartesian product.

each network interface should have its own independent set of remote URIs

I agree. There should be multiple (whitelist,blacklist,remotes) tuples to support this use case. Cartesian product is used within each tuple.

The existing design (a single cartesian product) can also work, although it creates more faces than necessary. Those excessive faces simply don't get used by the strategy due to their bad performance. The overhead is the maintenance cost of those faces, such as keep-alives.

I agree. There should be multiple (whitelist,blacklist,remotes) tuples to support this use case. Cartesian product is used within each tuple.

Just to make sure I understand what you are saying. In this case, the config file could look like this, right?

nicfaces
{
  whitelist
  {
    ifname eth0
  }
  blacklist
  {
  }
  remotes
  {
    udp4://192.168.1.2:6363 
  }

  whitelist
  {
    ether 85:3b:4d:d3:5f:c2
  }
  blacklist
  {
  }
  remotes
  {
    udp4://192.168.3.4:6363 
  }
}

Or alternatively, can we just use (interface, remotes) pairs like the following? Since multiple whitelists and blacklists seem a little bit confusing to me.

nicfaces
{
  ifname eth0
  remotes
  {
    udp4://192.168.1.2:6363 
  }

  ether 85:3b:4d:d3:5f:c2
  remotes
  {
    udp4://192.168.3.4:6363 
  }
}

https://gerrit.named-data.net/#/c/3363/2
I added a field 'tuple' to explicitly group each (whitelist, blacklist, remotes) tuple. So the nicfaces section now has the following form.

nicfaces
{
  tuple
  {
    whitelist
    {
      ifname eth0
    }
    blacklist
    { 
    }
    remotes
    {
      udp4://192.168.1.2:6363 
    }
  }
}

Reply to note-19:

tuple doesn't reflect semantics. remotes key is unnecessary.

I suggest the following:

nicfaces
{
  nicface
  {
    remote udp4://192.0.2.1:6363
    remote udp4://192.0.2.2:6363
    whitelist
    {
      ifname eth0
    }
    blacklist
    {
    }
  }

  nicface
  {
    remote udp4://192.0.2.3:6363 
    whitelist
    {
      ether 85:3b:4d:d3:5f:c2
    }
    blacklist
    {
    }
  }
}

• nicfaces has zero or more nicface keys
• nicface has one or more remote keys
• nicface has zero or one whitelist key
• nicface has zero or one blacklist key
• If the same remote value appears under multiple nicface keys and a NIC matches multiple whitelist+blacklist combinations, only one face should be created between this NIC and this remote FaceUri.

Is the nicfaces "wrapper element" really needed? can we just have a bunch of nicface specifications?

Also, I would be ok with the simplified syntax of the 2nd proposal in note-18, i.e. without using whitelist and blacklist keys. So for instance:

nicface
{
  ifname eth0
  remote udp4://192.168.1.2:6363
}
nicface
{
  ether 85:3b:4d:d3:5f:c2
  ether 85:3b:4d:a4:5b:f8
  remote udp6://[...]:6363
}
nicface
{
  ; applies to all interfaces (or all interfaces not matched by a previous 'nicface'?)
  remote udp4://192.168.100.1:6363
}

Is the nicfaces "wrapper element" really needed? can we just have a bunch of nicface specifications?

Having a nicfaces wrapper keeps the section more organized.
But I have no objection eliminating it.

the simplified syntax without using whitelist and blacklist keys

No. You lose the capability of blacklist, and cannot easily reuse code for whitelist.

Junxiao Shi wrote:

the simplified syntax without using whitelist and blacklist keys

No. You lose the capability of blacklist, and cannot easily reuse code for whitelist.

I'm aware of that. It's just an alternative proposal that is slightly less powerful, but also much more succinct and intuitive.

As for code reuse, it's a very minor concern in my opinion. I'm sure we can easily do a small refactoring in NetworkInterfacePredicate in order to maximize code reuse.

Let's go with the proposal in note 20. Agree with Davide that it is a bit more cumbersome syntax, but it should be ok.

As suggested by Alex, I updated the code according to node 20.
https://gerrit.named-data.net/#/c/3363/3/

I just realized there's a (IMHO serious) flaw with the syntax in note-20. The nicface stanza seems to imply from its name that it defines one NIC-associated face. However, it can in fact create any number of faces, zero, one, or more than one. There isn't a 1-to-1 relationship between a nicface stanza and a NIC-associated face. But the syntax seems to imply that there is one, which is confusing.

So I'm now against the syntax in note-20, and I'd like to propose the alternative in note-21 again.

note-21 is no better than note-20 because each nicface key also corresponds to zero or more NIC-associated faces.

err sorry, completely forgot an important piece in my previous comment... I meant to say: note-21 syntax with nicface renamed to nicfaces.

I still disagree with note-21 and its amendment in note-28.
I acknowledge the issue pointed out in note-26, and my proposal is:

nicfaces
{
  rule
  {
    remote udp4://192.0.2.1:6363
    remote udp4://192.0.2.2:6363
    whitelist
    {
      ifname eth0
    }
    blacklist
    {
    }
  }

  rule
  {
    remote udp4://192.0.2.3:6363 
    whitelist
    {
      ether 85:3b:4d:d3:5f:c2
    }
    blacklist
    {
    }
  }
}

The whole section defines all NIC-associated permanent faces, while each rule causes the creation of zero or more faces.
I'd like to preserve whitelist+blacklist and be able to reuse existing whitelist+blacklist code with minimal changes.

I changed the design to note-29 since I didn't see any objections. https://gerrit.named-data.net/#/c/3363/4

Besides, for the next step, we need to actually create NIC-associated permanent UDP unicast face. I plan to create a new transport class "NicAssociatedUnicastUdpTransport" as a subclass of DatagramTransport instead of reusing UnicastUdpTransport.. Any suggestions?

Weiwei Liu wrote:

I plan to create a new transport class "NicAssociatedUnicastUdpTransport" as a subclass of DatagramTransport instead of reusing UnicastUdpTransport.. Any suggestions?

What are the advantages and disadvantages of doing so?

What are the advantages and disadvantages of doing so?

What I can see for now is just if reusing UnicastUdpTransport we'll need to add a new constructor and the beforeChangePersistency() also needs to be changed to handle the NIC-associated case (Just to make sure, in this case we wouldn't allow the persistency to be changed to anything other than FACE_PERSISTENCY_PERMANENT, right?). I just think putting them into an independent class may make the code clearer and easier to understand.

Before talking about what Transport type to use, we should first understand how to manage the lifetime of Face:

• When a NIC is added or removed, or becomes matching / non-matching the whitelist+blacklist filter due to addition or removal of a particular IP address, which component is responsible for creating and destroying the Face objects?
• When a NIC gains its first IP address or loses its last IP address but does not affect its matching status (such as a whitelist matching by MAC address), should we create/destroy the Face object, or should we simply set the transport up / down? Which component is responsible for do so?

I think the logical answer in current structure would be FaceManager, however I don't think it's the right answer: FaceManager is taking way too many responsibilities, and it's getting more and more complicated.

My proposal is to split FaceManager into three components:

• FaceSystem owns all ProtocolFactory objects; it belongs in face/ rather than mgmt/. It directly processes the configuration section, but delegates nicfaces and multicast sections to NicManager.
• NicManager, also a part of face/, listens to signals from NetworkMonitor and takes proper reactions. It controls the creation and destroying of NIC-associated permanent faces and multicast faces.
• FaceManager is an interface to the management system. Its responsibility is to serve FaceMgmt commands and datasets. It calls into FaceSystem for those functions.

I know these sound like drastic changes, but I believe they lead to a cleaner architecture.

Weiwei Liu wrote:

What I can see for now is just if reusing UnicastUdpTransport we'll need to add a new constructor and the beforeChangePersistency() also needs to be changed to handle the NIC-associated case (Just to make sure, in this case we wouldn't allow the persistency to be changed to anything other than FACE_PERSISTENCY_PERMANENT, right?). I just think putting them into an independent class may make the code clearer and easier to understand.

Sorry, I think I misunderstood note-30. I thought you were comparing subclassing DatagramTransport vs subclassing UnicastUdpTransport. Modifying UnicastUdpTransport itself to support NIC-associated faces is obviously out of the question. NicAssociatedUdpTransport is a separate transport type, hence it needs a separate class. Another question is whether you want to inherit from UnicastUdpTransport or DatagramTransport. I think you can start from the latter, and see how much duplication you end up having in the new class.

Junxiao Shi wrote:

Before talking about what Transport type to use, we should first understand how to manage the lifetime of Face:
[...]
I think the logical answer in current structure would be FaceManager, however I don't think it's the right answer: FaceManager is taking way too many responsibilities, and it's getting more and more complicated.

While I agree with the above sentiment, and had the same thoughts when I was helping Andrea with his quick 'n' dirty implementation of this feature, I don't think doing this refactoring now is a good idea. This is an important feature and several applications would benefit from it. It should be implemented asap. Requiring such a large refactoring to be completed before this feature can be merged could delay the latter for weeks/months.

Requiring such a large refactoring to be completed before this feature can be merged could delay the latter for weeks/months.

I see no difficulty in this refactoring. It's just moving code around, and can be done in one week if review turnaround is fast enough.
Without the refactoring, it would be difficult doing note-14 changes.

In the meanwhile, Weiwei should finish up #3353 which blocks this issue, and also work on #3817 which is necessary in Jeff's scenario.

In https://gerrit.named-data.net/#/c/3551/6/tests/daemon/face/ethernet-fixture.hpp@38 Davide pointed out:

I'm insisting on not using "NIC". The "C" in NIC stands for Card or Controller... a NIC represents a piece of hardware. The "interfaces" we normally use do not necessarily correspond to a real hardware controller. Think about tun/tap, virtual ethernet pairs, or virtual bridges, etc... even the loopback interface is not a real hw device.
So NIC is incorrect, it's better to use "network interface" or one of its abbreviations.

Does the same argument apply to "NIC-associated permanent faces"? They can be associated with hardware interfaces, but can also be associated with virtual interfaces.
If so, is there a better name?

Junxiao Shi wrote:

Does the same argument apply to "NIC-associated permanent faces"? They can be associated with hardware interfaces, but can also be associated with virtual interfaces.

Yep.

If so, is there a better name?

I tried to come up with a better (and possibly shorter) name but I failed.

Davide Pesavento wrote:

Junxiao Shi wrote:

Does the same argument apply to "NIC-associated permanent faces"? They can be associated with hardware interfaces, but can also be associated with virtual interfaces.

Yep.

If so, is there a better name?

I tried to come up with a better (and possibly shorter) name but I failed.

As a (better) alternative to "NIC", I suggest the terms "netdev" or "netdevice", which are used in the Linux kernel networking stack.

I also suggest "bound" rather than "associated", the former is shorter and already used in the Linux networking stack (e.g., SO_BINDTODEVICE socket option). However, I don't think we need to include "bound" (or "associated" or any other alternative) in the name of the feature or related classes that implement the feature; it can be used in documentation and code comments that talk about the feature, e.g., "...a face bound to a netdev/netdevice...".