modules: introduce LLDP support

[christophefontaine]

Interfaces must accept multicast frames:
grcli set interface port <port0> allmulti on
LLDP is configured globally

grcli set lldp [rx|tx|both|off]
grcli set lldp ttl [10..600]

# grcli show lldp config
lldp: rx: on tx: on ttl: 60

# grcli show lldp neighbors
LLDP Neighbors: 2
Age  TTL  ifname        Sys name
  4 120s   dpdk-p0      crs317
  6 120s   p1   compute04.mgmt.foobar.space

[david-marchand]

Interfaces must accept multicast frames: grcli set interface port <port0> allmulti on LLDP is configured globally

I would prefer a list of interfaces with LLDP enabled is set in the lldp module.

This way, the LLDP module request this allmulti stuff (better from a user pov).
And I would filter unwanted LLDP frames if received from an iface not in the list.

[christophefontaine]

Interfaces must accept multicast frames: grcli set interface port <port0> allmulti on LLDP is configured globally

I would prefer a list of interfaces with LLDP enabled is set in the lldp module.

This way, the LLDP module request this allmulti stuff (better from a user pov). And I would filter unwanted LLDP frames if received from an iface not in the list.

Shouldn't allmulti be enabled by default, as it is required for IPv6 ?
And then we'd need to have a rx/tx configuration knob for each interface.

[david-marchand]

Interfaces must accept multicast frames: grcli set interface port <port0> allmulti on LLDP is configured globally

I would prefer a list of interfaces with LLDP enabled is set in the lldp module.
This way, the LLDP module request this allmulti stuff (better from a user pov). And I would filter unwanted LLDP frames if received from an iface not in the list.

Shouldn't allmulti be enabled by default, as it is required for IPv6 ? And then we'd need to have a rx/tx configuration knob for each interface.

Ideally, no.

Each module requiring multicast traffic should register its set of mcast addresses.
If adding those addresses is not supported (or a limit, like a max number of mac is reached), then allmulti should be enabled.

For the latter case, if the user enables allmulti for debugging, or whatever else, allmulti should still be maintained when the user disables it.
In the Linux kernel, there is a counter of allmulti / promiscuous users to handle this case.

I think grout will need a similar infrastructure code, for adding unicast / mcast address filtering.

[david-marchand]

I created two issues https://github.com/rjarry/grout/issues/27 and https://github.com/rjarry/grout/issues/28.

[rjarry]

Maybe return EINVAL if ttl is 0?

[christophefontaine]

TTL value should be already checked when it is parsed (min 10s, max 600s ) in the CLI.
with_help("Interval in seconds, 10 < ttl < 600", ec_node_uint("TTL", 10, 600, 10)),
Here, we only check for existence of the parameter.

[rjarry]

see proper naming/sizing of enums in gr_lldp.h.

[rjarry]

minor nit pick:

mbuf, T_CHASSIS_ID, T_CHASSIS_MAC_ADDRESS, sizeof(src_addr), &src_addr

[rjarry]

Same:

mbuf, T_CHASSIS_ID, T_CHASSIS_MAC_ADDRESS, sizeof(src_addr), &src_addr

[vjardin]

Why should a complete LLDP stack be redesigned ? What's about all the LLDP options ?

This stack https://github.com/lldpd/lldpd would be a good fit, but I don't understand the datamodel that could be used for lldpd to leverage grout.

Moreover, it would be a proof point that the integration of a signaling stack is achievable.

cc @vincentbernat

[christophefontaine]

The initial idea wasn’t to implement all options, but to have rather a minimal implementation, to see what needs to be added to grout.

This stack https://github.com/lldpd/lldpd would be a good fit, but I don't understand the datamodel that could be used for lldpd to leverage grout. Moreover, it would be a proof point that the integration of a signaling stack is achievable.

Indeed, we may want to revisit it as the infra is here, to investigate how we can integrate external stacks to grout.

[christophefontaine]

Not enough time to work on that, closing to rework properly on it later.