request for guidance on implementing VLAN reapplication in NetworkManager

[Beniamino Galvani]

On Thu, Jun 06, 2024 at 01:41:20PM +0200, Oleksij Rempel wrote:
> Hello devs,
> 
> My name is Oleksij Rempel, and I am a Linux kernel developer primarily
> focused on network switch drivers. I have been evaluating various
> options for managing switch and bridge configurations, and
> NetworkManager appears to be the best fit for our requirements.

Hi,

good to hear that!

> However,
> there is still some work to be done. One of the missing features is the
> ability to reapply certain settings without interrupting the data flow.
> For instance, the ability to reapply `bridge-port.vlans` for slave ports
> is currently lacking.
> 
> As I am new to the NetworkManager code base and its architecture, I
> would like to ensure that I am on the right path. To better illustrate
> my use case, here is a script that demonstrates the desired
> functionality:
> 
> ```bash
> [...] 
> # Reapply VLAN settings on veth1-slave and check if changes are
> # reflected
> bridge v
> nmcli connection modify veth1-slave bridge-port.vlans "1002,1003 untagged"
> nmcli dev reapply veth1
> bridge v
> ```
> 
> If I understand the NetworkManager code correctly, the bridge master is
> handled by `nm-device-bridge.c`, while the slave interfaces are handled
> by `nm-device.c`.

A bridge device is always an instance of NMDeviceBridge. A device that
is a port of the bridge can be of different types and so it can be
e.g. a NMDeviceEthernet, a NMDeviceVlan, a NMDeviceBond, etc,; what
distinguishes a bridge port is that is has a NMDeviceBridge as
controller (master).

Your statement "slave interfaces are handled by `nm-device.c`" can be
correct or not depending on the interpretation. Generally, a bond port
is handled by the implementation of that specific device
(NMDeviceEthernet for ethernet devices, etc), with a fall back to the
super-class (NMDevice).

If we speak about setting the bridge VLANs on the port, that is
handled by NMDeviceBridge in attach_port(): when a port is attached to
the bridge, NM reads the "bridge-port.vlans" property of the port and
sets the needed VLANs on it via
nm_platform_link_set_bridge_vlans(on_master=TRUE).

> At this point, I am not entirely sure whether it is a
> good idea to reapply settings only on the slave interface separately, or
> if I need to do this from the master down to the slave interfaces.

I think it should be allowed (and it's useful) to update bridge VLANs
on the port independently of the bridge itself.

> Should I introduce a function to iterate and reapply all slaves on the
> master/controller side? Or should I not worry too much and handle
> differences only on the slave side if possible?
> 
> I would greatly appreciate any guidance or recommendations on the best
> approach to achieve this functionality.

I think what is missing is:

 - can_reapply_change()	in NMDevice needs to accept changes to the
   "vlans" property of NMSettingBridgePort;

 - check_and_reapply_connection() in NMDevice needs to detect when the
   bridge VLANs changed, via:

   nm_g_hash_table_contains_any(
      nm_g_hash_table_lookup(diffs, NM_SETTING_BRIDGE_PORT_SETTING_NAME),
      NM_SETTING_BRIDGE_PORT_VLANS)

   and if so, reapply the VLANs.

 - to reapply them, it needs to construct the list of platform VLANs
   similarly to setting_vlans_to_platform() and then set them via
   nm_platform_link_set_bridge_vlans(on_master=TRUE).  The best way to
   do that is probably to move the code that builds the VLANs to a
   common file (nm-device-utils.c) so that it can be called both by
   NMDevice and NMDeviceBridge.

Beniamino
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/networkmanager/attachments/20240606/39afbf60/attachment.sig>