Add additional bridge port settings

[chdxD1]

Description

This MR adds additional bridge settings hairpin and port-mac-learning on bridge ports. They are useful when using VXLAN tunnels with e.g. FRR.

This is my first MR to netplan, I tried my best to run the tests locally first.

Checklist

• Runs make check successfully. (except some nmcli local tests)
• Retains 100% code coverage (make check-coverage).
• New/changed keys in YAML format are documented. (but with version ???)
• (Optional) Adds example YAML for new feature.
• (Optional) Closes an open bug in Launchpad.

[chdxD1]

Also I am not sure regarding the ABI re-generate, it seems there are quite a lot of changes if done on this branch which are not related?

[daniloegea]

Hi, thank you for your PR it's much appreciated.

I started reviewing it from the first commit but then realized you addresses most of the problems I pointed out and removed the mcast-snooping part in the last commit.

May I ask you to squash your commits and update the PR? It'll be easier to review.

Thanks again!

[daniloegea]

Hi Christopher, sorry for the delay.

It's looking good. I created a couple of integration tests (and executed them with autopkgtest) using these two new options. Feel free to integrate them into your PR: daniloegea@e543ddc

As it's adding two new keys to Netplan, I'll ask @slyon what he thinks.

[chdxD1]

@daniloegea I finally found the time to add your commit, thank you for the test cases!

[slyon]

Thank you @chdxD1 for your contribution to Netplan, and sorry for taking so long to comment.

I like the overall shape of this PR, thanks @daniloegea for your initial review! Those comments have been addressed already. Tests and CI are looking good, too! Thanks for adding/integrating unit- and integration tests.

I left some small inline-comments, mostly about documentation and naming of the new YAML schema.

We're introducing two new YAML settings here (let me try to confirm the schema with our architect):

• hairpin => We should keep this as suggested, IMO. "hairpin" is descriptive and common wording.
  • called "HairPin" in networkd
  • called "hairpin-mode" in NetworkManager
  • called "hairpin_mode" in iproute2
  • called "reflective relay" in Cisco/Juniper
• learning => We should change this to "port-mac-learning", IMO. Just "learning" might be a bit too generic. Also, we should stay in-line with the "mac-learning" setting for VXLAN devices and the "port-{priority,range}" settings for Bridges.
  • called "Learning" in systemd-networkd
  • Not generically implemented in NetworkManager (only for VXLAN)
  • called "learning" in iproute2
  • called "MAC Address Source-based Learning" in Cisco

The "learning" => "mac-learning" suggestion brings up one issue: What happens if a VXLAN device is used as the bridge-port? That way the new "mac-learning" setting would probably trump the VXLAN "mac-learning" setting and lead to unexpected behavior. You mention VXLAN tunnels in combination with FRR in your PR description. Could you please provide some more context on that? Would it be useful to actually re-use the VXLAN "mac-learning" setting and refactor it to become a generic setting. We could then write the "MacLearning=" setting in .netdev file and "Learning=" in .network file (for systemd-networkd), skipping the .netdev part on non-VXLAN ports. NetworkManager would not be affected, as it only supports the vxlan.learning setting (not on bridge-ports).

PS: rebasing your branch onto origin/main should fix the "RPM build (fedora:latest)" CI. The other failures are known and unrelated issues.

[chdxD1]

Thank you @chdxD1 for your contribution to Netplan, and sorry for taking so long to comment.

I like the overall shape of this PR, thanks @daniloegea for your initial review! Those comments have been addressed already. Tests and CI are looking good, too! Thanks for adding/integrating unit- and integration tests.

I left some small inline-comments, mostly about documentation and naming of the new YAML schema.

Thanks! I'll look into them 👍

We're introducing two new YAML settings here (let me try to confirm the schema with our architect):

• hairpin => We should keep this as suggested, IMO. "hairpin" is descriptive and common wording.

  • called "HairPin" in networkd
  • called "hairpin-mode" in NetworkManager
  • called "hairpin_mode" in iproute2
  • called "reflective relay" in Cisco/Juniper

• learning => We should change this to "mac-learning", IMO. Just "learning" might be a bit too generic. Also, we should stay in-line with the "mac-learning" setting for VXLAN devices.

  • called "Learning" in systemd-networkd
  • Not generically implemented in NetworkManager (only for VXLAN)
  • called "learning" in iproute2
  • called "MAC Address Source-based Learning" in Cisco

The "learning" => "mac-learning" suggestion brings up one issue: What happens if a VXLAN device is used as the bridge-port? That way the new "mac-learning" setting would probably trump the VXLAN "mac-learning" setting and lead to unexpected behavior. You mention VXLAN tunnels in combination with FRR in your PR description. Could you please provide some more context on that? Would it be useful to actually re-use the VXLAN "mac-learning" setting and refactor it to become a generic setting. We could then write the "MacLearning=" setting in .netdev file and "Learning=" in .network file (for systemd-networkd), skipping the .netdev part on non-VXLAN ports. NetworkManager would not be affected, as it only supports the vxlan.learning setting (not on bridge-ports).

One important remark: learning on bridges is different compared to learning on VXLAN interfaces, see the Kernel doc: https://docs.kernel.org/networking/vxlan.html

Unlike most tunnels, a VXLAN is a 1 to N network, not just point to point. A VXLAN device can learn the IP address of the other endpoint either dynamically in a manner similar to a learning bridge, or make use of statically-configured forwarding entries.

Learning for a bridge is about learning MACs for the FDB on ports, for VXLAN devices it is different and about learning remote tunnel endpoints. With a controlplane like BGP (with FRR) running you usually don't want learning on the VXLAN interface itself and on the VXLAN bridge port (Typically when deploying L2/L3VPNs with FRR you end up with a bridge for steering the traffic to different destinations and the VXLAN device to do the tunneling). In L2VPN setups you might want to use bridge FDB learning on the "workload"/VM/pod bridge ports to learn about the MACs.

We might want to distinguish between learning on bridge and VXLAN, however I am not sure if there is a case where you actually want to enable one but not the other (I am not aware at least).

PS: rebasing your branch onto origin/main should fix the "RPM build (fedora:latest)" CI. The other failures are known and unrelated issues.

Yeah was wondering about that, will do the rebase.

[slyon]

Thanks for having a look into my comments. As you can see, the rebase already resolved the CI (mostly). The remaining 3 "tunnels/GRE" failures in "Autopkgtest CI" are known and unrelated: LP#2037667 So you can ignore those.

One important remark: learning on bridges is different compared to learning on VXLAN interfaces, see the Kernel doc: https://docs.kernel.org/networking/vxlan.html

Unlike most tunnels, a VXLAN is a 1 to N network, not just point to point. A VXLAN device can learn the IP address of the other endpoint either dynamically in a manner similar to a learning bridge, or make use of statically-configured forwarding entries.

Learning for a bridge is about learning MACs for the FDB on ports, for VXLAN devices it is different and about learning remote tunnel endpoints. With a controlplane like BGP (with FRR) running you usually don't want learning on the VXLAN interface itself and on the VXLAN bridge port (Typically when deploying L2/L3VPNs with FRR you end up with a bridge for steering the traffic to different destinations and the VXLAN device to do the tunneling). In L2VPN setups you might want to use bridge FDB learning on the "workload"/VM/pod bridge ports to learn about the MACs.

We might want to distinguish between learning on bridge and VXLAN, however I am not sure if there is a case where you actually want to enable one but not the other (I am not aware at least).

ACK. That sounds like we do not want to mix this new "learning" setting with VXLAN's "mac-learning". So I propose to use "port-mac-learning" instead and updated my comment above. Let me cycle that discussion back to our architect @vorlonofportland

[vorlonofportland]

These key names are straightforward and consistent with naming in upstream tooling, +1 for the schema changes.

[vorlonofportland]

To be clear I'm ok with either 'learning' or 'port-mac-learning' as a name.

[slyon]

Thanks for the schema approval. Let's go with port-mac-learning then for my reasons given above.
I've rebased and pushed the corresponding renaming changes.

I think this should be ready for merging, just in time for the next Netplan release :)
This should cover some more cases for https://github.com/telekom/netplanner