Create individual [email protected] units (LP: #2060311)#455
Create individual [email protected] units (LP: #2060311)#455slyon wants to merge 7 commits intocanonical:mainfrom
Conversation
a995dbf to
125baa3
Compare
enr0n
left a comment
There was a problem hiding this comment.
This looks good so far to me.
One other thing is that we will actually need to mask systemd-networkd-wait-online.service by default I think, because otherwise e.g. a unit generated by systemd-sysv-generator could re-enable it, which is just annoying.
Since netplan would be the one enforcing this new [email protected] policy, do you think it makes sense to ship the mask (symlink to /dev/null) in netplan?
| _netplan_safe_mkdir_p_dir(enablement_dir); | ||
| } | ||
|
|
||
| // Create an enablement link for each interface in the <ifnames> vector |
There was a problem hiding this comment.
Not something that will likely be addressed in this PR (because I expect the same thing is done elsewhere in the netplan generator), but systemd generators really should not touch unit search paths outside of the directories passed as arguments to the generator (e.g. usually /run/systemd/generator{,.early,.late}).
| * netplan_netdef_match_interface() to see if they match the current NetDef | ||
| */ | ||
| STATIC void | ||
| _netplan_enumerate_interfaces(const NetplanNetDefinition* def, GStrvBuilder* builder, const char* rootdir) |
There was a problem hiding this comment.
Is this here to work around the fact that we could have a generated file like:
# my-interface.network
[Match]
MacAddress=AA:BB:CC:DD:EE:FF
[Network]
...
but we want to enable e.g. [email protected], if ethZ is the interface with the matching MAC?
There was a problem hiding this comment.
Yes! We need to expand any Netplan match.mac|driver|name setting to the actual matching network interfaces. Therefore, we need to check those values on all of them.
f9528b7 to
cf6c4c6
Compare
| @@ -0,0 +1,26 @@ | |||
| # SPDX-License-Identifier: LGPL-2.1-or-later | |||
There was a problem hiding this comment.
note: This should be compatible with our GPL-3 code, according to https://www.gnu.org/licenses/gpl-faq.html#AllCompatibility
|
I've built a test-package in a PPA. May I ask @waveform80 and @frank-heimes for testing? |
| * Otherwise, systemd might wait indefinitely for the interface to | ||
| * come online. | ||
| */ | ||
| if (!(def->optional || def->activation_mode)) { |
There was a problem hiding this comment.
question: Should we also check if IP addresses or dhcp4/dhcp6 are defined, e.g. to ignore definitions like this ethernets.encc000: {}?
We could define such scenarios as "user error", or try to be smart about it..
There was a problem hiding this comment.
I'd probably opt to "user error" for now (the easy workaround is adding optional: true instead of {}) and in the future we might want to wait for different conditions, e.g. link-layer configuration, which would still involve some waiting, but not necessarily on routable IPs.
There was a problem hiding this comment.
I'm not sure if I understood the question. Are you suggesting we shouldn't create the link for that type of definition (empty)?
There was a problem hiding this comment.
Empty NetDefs will not reach the "online" state, due to lacking an IP address. I was wondering if we should implicitly mark them "optional: true". But I now feel like we shouldn't do it, as logic might change in the future with the implementation of "optional-addresses".
This is in favor to RequiredForOnline=yes as the behavior of upstream (pure) systemd-networkd-wait-online.service is not mean to be used in this way. If "RequiredForOnline=no" sd-networkd-wait-online will fully ignore the corresponding interface and it will block/delay network-online.target if no interfaces are "RequiredForOnline=yes" at all. FR-7246
This is to enforce individual "[email protected]" wait-online policy.
This reverts commit 4bc987f.
a9e5024 to
b16ca32
Compare
Description
This is in favor to RequiredForOnline=yes as the behavior of upstream (pure) systemd-networkd-wait-online.service is not mean to be used in this way.
If "RequiredForOnline=no" sd-networkd-wait-online will fully ignore the corresponding interface and it will block/delay network-online.target if no interfaces are "RequiredForOnline=yes" at all.
This is as per discussions in systemd/systemd#32016
FR-7246
Checklist
make checksuccessfully.make check-coverage).