NFD

I'd like to suggest an alternative solution. Implement a new config key mcast_interfaces for both udp and ether sections, that tells nfd on which NICs a multicast face must be created. NICs that are not listed are skipped. If mcast_interfaces is not specified, multicast faces are created on all available NICs.

The alternate solution in note-1 is valid but more complex than the original description:

• it's a 3-hour Task including verifying on both ONL and Emulab
• integration test scripts must enumerate NICs and generate NFD configuration file; that's another 3-hour Task

The original solution needs just 2 hours including verifying on both ONL and Emulab.

A variation of the solution in note-1 is to have a config key to blacklist certain interfaces. An interface can be specified by name, IP address, IP+netmask (all interfaces with an IP in that subnet are excluded), or MAC address.

The solution in note-3 is also valid but it's more complex than what's minimally needed.

I would vote to "complicate" this task by implementing both whitelist and blacklist options. In either case they should specify prefixes (or allow to specify prefixes), instead of just interface names (regex could be used, but it could be an overkill... just prefix should be enough).

Steve DiBenedetto wrote:

I think whitelist and blacklist need to be subsections (e.g. like tcp or udp currently are). I don't think the INFO format supports doing foo={...}. Given the whitelist={"*"} blacklist={} default, omitting a subsection will then just have the effect of wildcard and empty set, respectively.

I agree, a missing blacklist should be treated as the empty set, a missing whitelist should be equal to the set of all interfaces. This has the advantage of backwards compatibility too, i.e. the current config file doesn't require any changes to keep the status quo.

I also wondering about having something like "name" and "prefix" items to denote each entry to help with error checking. Not sure if it makes a difference though. It may have secondary benefits like making it easy to grep through a bunch of config files for whitelisted/blacklisted names/prefixes.

Can we simply have the blacklist and whitelist keywords followed by a list of names/prefixes/macaddrs/... ? i.e. similar to what Junxiao proposed but without equal sign and curly braces.

Not sure. I suspect the INFO parser will treat adjacent listed items as key-value pairs. Looking at the example configurations (http://www.boost.org/doc/libs/1_55_0/doc/html/boost_propertytree/parsers.html), this may also be true for items listed within a block, even if they're listed on separate lines (they use "" to denote empty keys and values). If this is the case, then we'll need named entries (name, prefix, macaddr, etc.).

I see. And does ptree support multiple entries with the same key? e.g. to do something like:

blacklist
{
  name foo
  name bar
}

If items are on separate lines, then they are not treated as key value. We are actually using this already as part of privileges block.

And there are no problems including multiple items with the same name... But I haven't actually checked that.

I completely forgot about privileges, so I guess we can just list them.

In my understanding, duplicate keys are allowed via add_child, so I imagine the parser wouldn't have any problems.

My proposal uses whitelist={"*"} format to illustrate the concept of unordered set.
It's not the actual syntax in INFO format.

I agree with the syntax in note-7 and note-10.

How do we handle pseudo interfaces? They all have the same mac addresses and it's not possible to know the faceid beforehand.

faceid=259 remote=ether://[01:00:5e:00:17:aa] local=dev://enp68s0f1.803 counters={in={0i 0d 0B} out={0i 0d 0B}} non-local persistent multi-access
faceid=260 remote=ether://[01:00:5e:00:17:aa] local=dev://enp68s0f1 counters={in={0i 0d 0B} out={0i 0d 0B}} non-local persistent multi-access
faceid=261 remote=ether://[01:00:5e:00:17:aa] local=dev://enp68s0f1.802 counters={in={0i 0d 0B} out={0i 0d 0B}} non-local persistent multi-access
faceid=262 remote=ether://[01:00:5e:00:17:aa] local=dev://enp68s0f1.500 counters={in={0i 0d 0B} out={0i 0d 0B}} non-local persistent multi-access

Answer to note-16:

Pseudo interfaces can be whitelisted or blacklisted with its exact interface name, such as: whitelist={"enp68s0f1.803"}.

From http://gerrit.named-data.net/2642

Actually, let me withdraw the request/suggestion of implementing the use of mac addresses, and Junxiao should do the same with his request for IP addresses. If interface names are enough for Susmit's use case, I think we should accept his patch (once the other comments are addressed), instead of requesting more features that he might not be interested in, and thus has little or no motivation to develop. If others need IP or MAC address matching, they can submit a separate patch later.

The code design in 2642,2 is highly restricted.
It looks like a patchwork instead of a carefully designed feature.

A good design is:

• Use a std::set<std::string> to represent the whitelist and the blacklist.
• Have a bool shouldCreateFace(const NetworkInterfaceInfo& nic, const std::set<std::string>& whitelist, const std::set<std::string>& blacklist) function to evaluate the whitelist and blacklist.

I could accept "interface name only" and "Ethernet only" limitations, if the design above or a better design is adopted.

Shall we use regexp as part of white/black lists?

Alex Afanasyev wrote:
Shall we use regexp as part of white/black lists?

I have assumed exact match for name, mac address and "control". I can change it though. What's the use case you have in mind?

Junxiao Shi wrote:
From http://gerrit.named-data.net/2642

Actually, let me withdraw the request/suggestion of implementing the use of mac addresses, and Junxiao should do the same with his request for IP addresses. If interface names are enough for Susmit's use case, I think we should accept his patch (once the other comments are addressed), instead of requesting more features that he might not be interested in, and thus has little or no motivation to develop. If others need IP or MAC address matching, they can submit a separate patch later.

The code design in 2642,2 is highly restricted.
It looks like a patchwork instead of a carefully designed feature.

Correct me if I am wrong here.
Should the default be to create multicast face on a given interface by default or the other way (see 2642,3)?

In reply to note 18:

Absolutely, I agree. I specified "once the comments are addressed", which means the patch set was not acceptable as-is.

Alex Afanasyev wrote:

Shall we use regexp as part of white/black lists?

I'm not sure, regexps sound a bit overkill for the task. But a limited form of wildcard matching could be useful (e.g. just having "*" as special char). Or we could simply use fnmatch() without extended patterns.

@Alex, why do we need both whitelist and blacklist? Doesn't having one by default (blacklist/whitelist) do the same thing?

Reply to note-7:

While I agree with the general idea of using key-value pairs, it's necessary to define all possible keys.

• key=*, value=(ignored): match every NIC
• key=name, value=NIC name such as eth0: exact match NIC name
• key=ether, value=MAC address in hex-digits-and-colons notation such as 08:00:27:01:01:01: exact match MAC address
• key=subnet, value=IPv4 subnet in CIDR notation such as 192.0.2.0/28: match if any IPv4 address on the NIC falls under this subnet

I disagree with representing IPv4 subnet with key=prefix because it can be confused with NDN name prefix.

Junxiao Shi wrote:
Reply to note-7:

While I agree with the general idea of using key-value pairs, it's necessary to define all possible keys.

• key=*, value=(ignored): match every NIC
• key=name, value=NIC name such as eth0: exact match NIC name
• key=ether, value=MAC address in hex-digits-and-colons notation such as 08:00:27:01:01:01: exact match MAC address
• key=subnet, value=IPv4 subnet in CIDR notation such as 192.0.2.0/28: match if any IPv4 address on the NIC falls under this subnet

I disagree with representing IPv4 subnet with key=prefix because it can be confused with NDN name prefix.

That looks OK to me.

Junxiao Shi wrote:

key=subnet, value=IPv4 subnet in CIDR notation such as 192.0.2.0/28: match if any IPv4 address on the NIC falls under this subnet

No ipv6?

I disagree with representing IPv4 subnet with key=prefix because it can be confused with NDN name prefix.

key=name can also be confused with an NDN name.

The rest looks good to me.

Reply to note-28:

IPv6 is intentionally not supported due to lack of use case.

How about key=ifname for NIC name?

do we wait for this issue closed before release? or release without?

This issue does not block 0.4.1 release.

Work on this issue is suspended until July 13th (Susmit has an upcoming examination)

How do I get the actual netmask for an interface?

boost::asio::ip::address_v4 provides the default netmask.

nfd::NetworkInterfaceInfo doesn't provide the actual netmask defined in the system configuration.

Junxiao Shi wrote:

nfd::NetworkInterfaceInfo is meant to provide the actual netmask. If not, report a bug.

No, it doesn't currently provide the netmask, there was never any use case for it.

However, I would be lightly against adding support for it now, if there's still no need. Also consider that we will probably have to implement a substantially enhanced version of NetworkInterfaceInfo in ndn-cxx as part of #3353. At that point we should probably remove nfd::NetworkInterfaceInfo from NFD and just use ndn-cxx facilities.

https://gerrit.named-data.net/#/c/3357/

Junxiao Shi wrote:

One concern is: Creating MulticastUdpTransport on linux requires ifname argument if there are more than one multicast face on the same group. When the predicate is being changed, it's possible that the old predicate has created exactly one multicast face on netif0, and the new predicate would also create exactly one multicast face on a different netif1. The current logic creates new faces before closing old faces, and would not pass ifname argument because each time there is only one multicast face being created. I guess there would be a problem, but ChangePredicate test case does not have an error.

No, that shouldn't be a problem. IIRC, the reason we need the interface name is to set the SO_BINDTODEVICE socket option, otherwise one face will receive all packets addressed to that multicast group, even those received on other network interfaces. So omitting the ifname will not trigger any errors/exceptions per se, but some packets will be received on the wrong face. But since the other face is being closed immediately after, that shouldn't be a problem in practice.

Anything to update in the dev guide? If not, I believe this issue can be closed.