Adding vlan support to nm

[fdizazzo]

Hi, i'm trying to add vlan support to the new network manager and i'd like to do it well enough so it can be ported to future releases.
Right now, nm is able to see and change the network configuration on vlan connections but i have some doubts on how to handle the vlan-specific configurations required by the network manager: in my current version the vlan configuration (the "vlan" map in the Connection settings) are retrieved and reused in the enableInterface method of NMDbusConnector, could a cleaner way be populating the connection properties in the kura snapshot?
If so, some pointers on how ( /where) to write additional configurations to nm would be extremely useful.

Thanks,
Fabio

[mattdibi]

Ciao Fabio,
I suppose you're referring to these settings available from the NetworkManaged dbus API.

The current flux is the following:

• NMDbusConnector receives the network settings (the "desiderata" from the user, which corresponds to the content of the Kura snapshot) from the Configuration service in the form of a Map<String, Object> [1].
• Then, in the enableInterface (if the network interface is configured), Kura calls the NetworkManager DBus APIs to retrieve and update the current connection associated with the configured network interface [2]
• To update the connection it needs to convert the Kura settings into the NetworkManager settings. This is done by the NMSettingsConverter class.

Therefore, all the code responsible for writing additional configuration settings to the NetworkManager connection should be placed inside the NMSettingsConverter.

From the user point of view: to pass these settings from the UI to the Configuration service (and therefore, by consequence, update the Kura snapshot), one has to update the metatype and the UI accordingly. I suggest you take a look at what @sfiorani and @marcellorinaldo did in the last PRs for the IPv6 configuration support.

• NMSettingsConverter changes: feat(nm): implementation of IPv6 configuration #4786
• Metatype update changes: feat(net.configuration): Updated metatype with new IPv6 properties #4796
• UI changes: feat(web2): add IPv6 configuration tab #4800 feat(web.server.net2): add IPv6 configuration properties support #4804

Hope this helps.

[1] Reference:

https://github.com/eclipse/kura/blob/f0a99ec5380699f49245852a64241c22a51b79a5/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java#L308

[2] Reference:

https://github.com/eclipse/kura/blob/f0a99ec5380699f49245852a64241c22a51b79a5/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java#L432-L434

https://github.com/eclipse/kura/blob/f0a99ec5380699f49245852a64241c22a51b79a5/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java#L439-L447

https://github.com/eclipse/kura/blob/f0a99ec5380699f49245852a64241c22a51b79a5/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java#L450

[fdizazzo]

Ciao Mattia,
thanks for the quick response!
I'm actually at a loss with the "other half" of the flux, as in what to do with configuration parameters when a network is first detected and how to save them among the other network properties in the kura snapshot: a vlan has this set of parameters vlan => { egress-priority-map => [[]],flags => [1],id => [40],ingress-priority-map => [[]],parent => [ens33] } which i hardly see as user configurable but are needed for other configurations such as changing ip address and could be useful to show as read only fields on the gui.
Another possible way is to simply reuse them while changing configuration, just like it's done with the connection map in NMSettingsConverter.

The "reuse" logic is currently working, is suitable for "simpler" use cases like needing to access and change the ip configuration of an existing vlan (which would still need to be created via nmcli command line) but does not allow for creation and management of vlans directly through the Kura Ui

[mattdibi]

Ciao Fabio,
I see.

I'm actually at a loss with the "other half" of the flux, as in what to do with configuration parameters when a network is first detected and how to save them among the other network properties in the kura snapshot

This would be taken care of by the Configuration service I mentioned. The Configuration service is also responsible for writing the configuration onto the snapshot.

You can simulate the flux without changing the UI by manually changing the snapshot and re-uploading it. Proceed as follows:

• In the Kura UI go to: Settings-> Select a Snapshot Id -> Download -> XML
• Manually add the settings as you would using a UI in your preferred editor
• Go back to the Kura UI and: Settings -> Upload and Apply -> Select your modified snapshot on your PC

Given you have updated your code to leverage the Map<String, Object> the NMDbusConnector receives, you should be able to retrieve the settings from the snapshot and pass them to NetworkManager.

Another possible way is to simply reuse them while changing configuration, just like it's done with the connection map in NMSettingsConverter.

With regards to this I would actually avoid that and keep consistent to what the rest of the network settings expect, that is: expect that the UI will always provide all the necessary parameters (now that I think about it we should probably remove that part you were referring to since it's inconsistent with the rest and not really needed IIRC 🤔 )

In your case you should expect to always receive egress-priority-map, flags, id, ingress-priority-map, parent. The UI will keep them cached and the user won't need to change them.

[fdizazzo]

With regards to this I would actually avoid that and keep consistent to what the rest of the network settings expect, that is: expect that the UI will always provide all the necessary parameters (now that I think about it we should probably remove that part you were referring to since it's inconsistent with the rest and not really needed IIRC 🤔 )

I just assumed the "reuse" logic in this
https://github.com/eclipse/kura/blob/6be683f53247ce81aa2c4ed3b739fa878342825b/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMSettingsConverter.java#L391-L399
is to let kura prioritize the actual network interface settings rather than the snapshot ones to avoid any unexpected behaviour.

In my specific case, it becomes kinda like a chicken-and-egg problem, as the first retrieval of vlan parameters requires a preexisting active connection (and really don't want to start losing field devices to configuration mistakes :) ) so manually modifying a snapshot.xml for the first configuration so that properties can be retrieved by NMSettingsConverter seems like the only option.

[mattdibi]

🤔 I might have finally understood where's the problem. Let'see...

The only limitation (not gonna lie, is not small) I see for implementing complete support to vlan in Kura is given by the fact that, in our current implementation, we only manage/configure interfaces that are already present on the system. From what I understand, when you create a vlan connection with NetworkManager, it will create a new device (see example)

Could this be your chicken-and-egg problem?

Once you create this connection (and therefore the related device) you want to preserve the vlan related settings when updating the configuration of the vlan interface but everything else can be managed by Kura.

Did I get this right?

So I suppose that you're currently:

• Creating the vlan connection (and the related device) using the CLI
• Once the device and the connection spawn you manage them using the Kura UI treating them as "normal" devices
• You need to "preserve" the vlan settings that are associated with the device you're currently manipulating and therefore, in the NMSettingsConverter, you grab them from the old connection and essentially re-set them in the new settings.

[fdizazzo]

So I suppose that you're currently:

• Creating the vlan connection (and the related device) using the CLI
• Once the device and the connection spawn you manage them using the Kura UI treating them as "normal" devices
• You need to "preserve" the vlan settings that are associated with the device you're currently manipulating and therefore, in the NMSettingsConverter, you grab them from the old connection and essentially re-set them in the new settings.

That's correct, sorry if I didn't manage to be clearer before :)
To be more specific with my use case: I have several devices with a single (hardware) nic but the need to have different ip address, this is usually addressed by creating vlan which are largely unchanged during the life of the device. So my implementation aims at being able to use Kura to configure ips and use the other network related services rather than creating right now a full vlan creation and management interface ex-novo, I'm just trying to do it well enough so it can be built upon.

[mattdibi]

CC @pierantoniomerlino and @MMaiero

So my implementation aims at being able to use Kura to configure ips and use the other network related services rather than creating right now a full vlan creation and management interface ex-novo, I'm just trying to do it well enough so it can be built upon.

Thank you for the additional context.

Meanwhile I talked about this with the rest of the Eclipse Kura team and agree that the feature you're proposing is really interesting and we'd really like to see it in the Kura codebase.

However we agree that, to be a "complete" feature and be accepted into the codebase, Kura needs to be responsible for the creation of the connection and the vlan device even if we don't expose a dedicated UI for that (i.e. we don't like having to rely on the CLI for creating the initial connection).

Therefore, as we already discussed, the flow should be the following:

• Kura receives the new configuration through a RestAPI call (or via snapshot upload as we already described).
• There it'll find the required configuration to be able to create a vlan connection (i.e. egress-priority-map, flags, id, ingress-priority-map, parent in addition to the other network configuration options)
• Kura will create the connection by issuing the required commands to the NetworkManage dbus api.

This should work because, from what I understand, upon creating the connection the virtual device gets created and therefore Kura is only responsible for creating the connection (which already does). The already existing paradigm should work and I think Kura is already capable of performing what is needed to create the vlan device without any additional major change in the NMDbusConnector and the use of the nmcli command is not necessary. From what you describe you're already 90% there with your implementation.

What do you think? Does this looks feasible to you?

Additionally, if you have any doubt or just want to speak about this in real-time, we're available for a call on Microsoft Teams. Furthermore we also have the bi-monthly Kura Dev Meeting on the 23rd. Here's the invite, feel free to drop by if you want to :)

[fdizazzo]

However we agree that, to be a "complete" feature and be accepted into the codebase, Kura needs to be responsible for the creation of the connection and the vlan device even if we don't expose a dedicated UI for that (i.e. we don't like having to rely on the CLI for creating the initial connection).

Fair enough :)

Right now, a vlan can be created via the configuration map created from a snapshot by NMSettingsConverter with a minimal set of properties like

props.put("net.interface.ens33.41.config.vlan.parent", "ens33");
props.put("net.interface.ens33.41.config.vlan.id", 41);
props.put("net.interface.ens33.41.config.ip4.status", "netIPv4StatusEnabledLAN");
props.put("net.interface.ens33.41.type", "VLAN");
props.put("net.interface.ens33.41.config.ip4.address", "192.168.189.90");
props.put("net.interface.ens33.41.config.ip4.prefix", prefix);
props.put("net.interface.ens33.41.config.dhcpClient4.enabled", false); 

everything else being optional but supported.

The main issue about this is that the current network configuration relies on the assumption that every device is already existing when a connection needs to be created. A possible solution could be adding a new property in NetworkConfigurationService containing the names of all vlan to be managed (net.interface.vlans ? ) so that any missing device/connection can be created before handing over control to preexisting flow, adding such check somewhere like here before line 362:
https://github.com/eclipse/kura/blob/870b84b5e655bca9d36c96d4ac397a2427ab20be/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java#L360-L371

[mattdibi]

Right now, a vlan can be created via the configuration map created from a snapshot by NMSettingsConverter with a minimal set of properties like

That's exactly what I was thinking.

The main issue about this is that the current network configuration relies on the assumption that every device is already existing when a connection needs to be created. A possible solution could be adding a new property in NetworkConfigurationService containing the names of all vlan to be managed (net.interface.vlans ? ) so that any missing device/connection can be created before handing over control to preexisting flow, adding such check somewhere like here before line 362

Thing is (and here please, correct me if I'm wrong) when you ask NM to create a connection of the vlan type, it creates the associated device on its own. Therefore Kura shouldn't care about creating the device beforehand and only focus on creating the connection (which already does in the existing flow).

The problem (IMO) can be addressed by adding a check in the NMDbusConnector code such that: if the device for which it received a configuration doesn't exists but it's of the VLAN type, it should carry on with the existing flow (since it creates the connection which, in turn, creates the device).

This is the intuition that I have but, not having worked on this as you did, might be wrong.

[fdizazzo]

Thing is (and here please, correct me if I'm wrong) when you ask NM to create a connection of the vlan type, it creates the associated device on its own. Therefore Kura shouldn't care about creating the device beforehand and only focus on creating the connection (which already does in the existing flow).

While there no need to create Device and Connection in separate steps, as the creation of a vlan connection creates the related device, almost every method call (from NMConfigurationServiceImpl to NMDbusConnector ) leading to connection creation assumes the existence of a device.
So I tried a check almost like one you just proposed: if the device for which it received a configuration doesn't exists and it's of the VLAN type (as configured in snapshot file) a new connection is created then carries on with the existing flow. A downside is that a newly created VLAN is configured twice (as in: configured, then overwritten).

A possible solution could be adding a new property in NetworkConfigurationService containing the names of all vlan to be managed (net.interface.vlans ? )

Need to correct myself on this: the list of managed vlan can easily be retrieved from net.interfaces and net.interface.[ifName].config.type so there no need for an additional property

[mattdibi]

While there no need to create Device and Connection in separate steps, as the creation of a vlan connection creates the related device, almost every method call (from NMConfigurationServiceImpl to NMDbusConnector ) leading to connection creation assumes the existence of a device.

Dang it! I didn't take that into account.

A downside is that a newly created VLAN is configured twice (as in: configured, then overwritten).

Yeah, I would avoid that if we can.

Need to correct myself on this: the list of managed vlan can easily be retrieved from net.interfaces and net.interface.[ifName].config.type so there no need for an additional property

This is nice. Leveraging that property is the right choice.

[fdizazzo]

we also have the bi-monthly Kura Dev Meeting on the 23rd. Here's the invite, feel free to drop by if you want to :)

@mattdibi Forgot to ask at what time the meetup starts, I'd gladly come say hi :)

[mattdibi]

@fdizazzo The meetings starts at 17:00 CEST :)

[mattdibi]

Just a recap. We're currently considering the following solution:

• This feature will be not exposed on the UI. Only RestAPI and manual snapshot upload will allow to leverage this feature. This is to limit the scope of the changes.
• net.interfaces will contain the name of the VLAN (i.e. eth0.30) in addition to the other existing devices
• The snapshot will contain the additional properties required for configuring the vlan field documented here. This properties will have names like:

net.interface.eth0.30.config.vlan.id
net.interface.eth0.30.config.ip4.status
net.interface.eth0.30.type
net.interface.eth0.30.config.ip4.address
net.interface.eth0.30.config.ip4.prefix
net.interface.eth0.30.config.dhcpClient4.enabled

• When Kura finds an interface inside net.interfaces that doesn't exists but is of type vlan as per the net.interface.eth0.30.type property, it will follow the "special" flow that allows for the creation of the interface.
• Once the device is created we follow the "normal" flow of operation

@fdizazzo is this correct?

@MMaiero @pierantoniomerlino sounds good to you guys?

[pierantoniomerlino]

@mattdibi @fdizazzo It sounds good for me. My only concern is about the property names (or the vlan interface names :-)): the dot is used as a field separator in Kura. So, we have to introduce some exceptions to correctly manage the eth0.30 name...

[fdizazzo]

The vlan naming format parentInterface.vlanId is not actually required but I guess it's common enough for people to try and use it. Not sure if actually impeding its usage is worth it.
On the properties, the dot is actually used as a separator as there are several vlan related ones:

net.interface.eth0.30.config.vlan.id
net.interface.eth0.30.config.vlan.parent
net.interface.eth0.30.config.vlan.flags
net.interface.eth0.30.config.vlan.ingress
net.interface.eth0.30.config.vlan.egress

with only id and parent being required

[mattdibi]

Hello there @fdizazzo,

first and foremost: thank you for your contribution. The feature discussed in this Discussion was merged.

One final thing: while discussing the changes with our Project Lead @MMaiero we changed our mind on our initial plans. To consider the feature "complete" and ready to ship in the next release (which is due in the following weeks) we'd like to have a minimal working UI for creating the VLANs device.

A barebone solution would be to add a "+" button in the device list which will show a pop-up where we can fill the VLAN details. That would be enough for us.

Do you think you can contribute that too? If not, we'll take care of it.

[fdizazzo]

Hi, thank you again for your support

Do you think you can contribute that too? If not, we'll take care of it.

I'd start from scratch on the UI, so maybe it will be quicker and less painful it you handle it 😅
In the meantime I noticed i also need configurable MTU, so expect to hear from me soon with a new PR (this time with UI too)

[mattdibi]

Ok, no problem. Thanks for letting us know.

I'll now close this discussion as answered. See you on the next contribution! :)

[mattdibi]

Documentation addressed in #4919