Layer 2: Metallb shouldn't reply on interface with NOARP flag

[kvaps]

Is this a bug report or a feature request?:

Feature request

What happened:

Speaker sends replies to any interface, even if arp disabled for it

What you expected to happen:

If interface have NOARP flag set, speaker should ignore it.

How to reproduce it (as minimally and precisely as possible):

ip link set dev <interface> arp off

Then try to arping

Anything else we need to know?:

Related #349

Environment:

• MetalLB version:
• Kubernetes version:
• BGP router type/version:
• OS (e.g. from /etc/os-release):
• Kernel (e.g. uname -a):

[kvaps]

@danderson , what do you thinking about using https://github.com/mickep76/netlink for identify flags on interfaces such as FlagNoArp and FlagMaster?

[danderson]

I'm not a huge fan of adding a dependency, especially one that uses syscall and unsafe. Those are hard to get right, even for experienced Go programmers.

The one netlink library I trust is the one by @mdlayher . It's very well tested, and Matt's got a bunch of experience with kernel interfacing. But AFAICT, he hasn't implemented interface reading, so we'd have to add that on top of the base netlink (or genetlink? Not sure) package.

I mean, with that said, if I get a PR with anything, I'll probably merge it. If it's not using mdlayher's netlink packages, I'll just file an extra bug to remind me to change it later ;)

[kvaps]

Ok, I'm trying to parse noarp from /sys/class/net/<interface>/flags using only standard libs, right now

[danderson]

Hmm, actually, it looks like that repository is mostly just copy/pasting the linux implementation of net.Interfaces, and adding support for more flags. If we're going to just do that anyway, I would prefer it if we just copy/paste our own implementation using https://golang.org/src/net/interface_linux.go as a reference. That way we can tailor it to exactly the data we need.

But like I said, if you're sending a PR, I'll merge it if it's not hugely unsafe. Worst case I'll just file a bug to think about improving it later, but that's not a big deal.

[kvaps]

HI, I have success with another netlink library: github.com/vishvananda/netlink. It's well tested too.
Kubernetes uses it in ipvs proxier and kubenet projects.

Can you agree to using it in MetalLB?

[mdlayher]

I would have assumed the stdlib would have the "No ARP" flag, but apparently not.

vishvananda/netlink is pretty widely used. I personally am not a big fan of some of the design choices it has made, but it seems reliable enough. The equivalent in my netlink ecosystem would be https://godoc.org/github.com/jsimonetti/rtnetlink, but this one is currently less mature and less battle tested.

That said, if you can read that flag from /sys, I'd just do that instead of mucking with (route) netlink for this one tiny thing.

[kvaps]

Unfortunately it can't be simple readed from /sys/class/net/<interface>/flags, because all flags are encoded as hex values:

sysIFF_UP          = 0x1
sysIFF_BROADCAST   = 0x2
sysIFF_DEBUG       = 0x4
sysIFF_LOOPBACK    = 0x8
sysIFF_POINTOPOINT = 0x10
sysIFF_NOTRAILERS  = 0x20
sysIFF_RUNNING     = 0x40
sysIFF_NOARP       = 0x80
sysIFF_PROMISC     = 0x100
sysIFF_ALLMULTI    = 0x200
sysIFF_INTELLIGENT = 0x400
sysIFF_MULTICAST   = 0x800
sysIFF_MULTI_BCAST = 0x1000
sysIFF_UNNUMBERED  = 0x2000
sysIFF_PRIVATE     = 0x8000

The result is combination of all of them is written in /sys/class/net/<interface>/flags file.

I have no better idea for parse this value for and apply bitwise OR for all of them.

In bash I would like to use some workaround like:

grep '[8-f].$' /sys/class/net/*/flags

[mdlayher]

But all you'd need to do is read that hex value and then check that flag, right? That sounds like an easy solution to the problem rather than bringing netlink into the picture.

[kvaps]

You're right, I have an idea, I'll try to implement it now

[kfox1111]

This just broke on me. Apparently new openvswitchs on el7 don't tag it as noarp and don't honour the ifcfg ARP="no" flag. :(

[kfox1111]

My issue above is not technically a metallb fault, but its repeatedly been a problem that you can't specify an interface to listen to. Can we get support for this feature now?