parser: accept the special MAC address options (LP: #2045096) #427
Conversation
daniloegea
force-pushed
the
macaddress_random
branch
from
666a3f0
to
ead9c84
Compare
There was a problem hiding this comment.
This is OKish, I guess.
Although, I don't like the fact of diverging possible values, depending on the chosen backend too much.
I wonder if similar behavior could be implemented on the "networkd" backend, using udev's [Link].MACAddressPolicy= setting? https://www.freedesktop.org/software/systemd/man/latest/systemd.link.html#MACAddressPolicy=
As I see from parse-nm.c those cases have already been handled for NetworkManager integration (using "passthrough"). I wonder how this bug was hit in the first place? Was it a manual modification of the NetworkManager genrated YAML by the user?
|
I assumed the used modified it manually, yes. Although I can't confirm that (I saw you just asked them). I'll look at the networkd options, supporting them sounds like a good idea. Thanks! |
daniloegea
force-pushed
the
macaddress_random
branch
from
ead9c84
to
392aecb
Compare
|
So, I added support for the options supported by networkd as well, "persistent" and "random". It was a bit harder because it involves multiple files and the option is different (MACAddress vs MACAddressPolicy). |
There was a problem hiding this comment.
Thank you for implementing it for the networkd backend, too!
Overall, this is looking good to me, but I'd like to see an additional integration test, to make sure the new MACAddressPolicy= don't interfere with /usr/lib/systemd/network/99-default.link.
Also let's try to combine the "persistent"+"permanent" and "random", instead of special casing all of them (see inline comment).
daniloegea
force-pushed
the
macaddress_random
branch
5 times, most recently
from
c34cd18
to
d9a07f9
Compare
daniloegea
changed the title
Besides the actual MAC address value, Network Manager and networkd also accept special options to control the assignment of a MAC address. The options supported are: permanent, random, stable and preserve. All these options are supported by Network Manager. Only 'permanent' (which will be translated to 'persistent') and 'random' are supported by networkd and will be added to the .link file.
daniloegea
force-pushed
the
macaddress_random
branch
from
d9a07f9
to
ec0d531
Compare
|
Thanks Lukas, I tried to address all of your comments. As for the integration test, I couldn't get the option |
|
Is the documentation checks failing because of this? ( index: line 70) broken https://askubuntu.com/questions/tagged/netplan - 403 Client Error: Forbidden for url: https://askubuntu.com/questions/tagged/netplan |
slyon
changed the title
There was a problem hiding this comment.
Thanks, LGTM!
Is the documentation checks failing because of this?
( index: line 70) broken https://askubuntu.com/questions/tagged/netplan - 403 Client Error: Forbidden for url: https://askubuntu.com/questions/tagged/netplan
Yes, we can ignore that for now.
Besides the actual MAC address value, Network Manager also accepts the following values: preserve, permanent, random and stable.
When set via NetworkManager, these special values would end up in the passthrough section. But if users try to set it manually, the parsing would fail.
This changes add support for these special values for the NetworkManager renderer and return an error if the user tries to use them with networkd.
Related to https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/2045096
Description
Checklist
make checksuccessfully.make check-coverage).