From 0d3a4a36e1d63347e667c8b0e4771a89780ad206 Mon Sep 17 00:00:00 2001 From: Danilo Egea Gondolfo Date: Mon, 3 Apr 2023 19:01:58 -0300 Subject: [PATCH 1/3] parser: reimplement optional-addresses optional-addresses is generating invalid configuration for systemd-networkd. The option OptionalAddresses doesn't really exist. It doesn't emit any configuration for Network Manager. This changes reimplement optional-addresses using systemd's "RequiredFamilyForOnline" and NetworkManager's "may-fail". It keeps all the old options, for backwards compatibility, as noops, as they were before in practice, and introduce 3 new options: "ipv4", "ipv6" and "none". On systemd, RequiredFamilyForOnline defaults to "any", meaning that any interface that gets an IP will be enough to make the system online. Netplan will emit configuration for systemd only when the value of optional-addresses is "none", meaning that nothing is optional. In this case, RequiredFamilyForOnline will have the value "both". On Network Manager, it will emit "may-fail" as true or false according to the configuration. It will still allow the user to have the old values so we will not break systems out there. A warning message will emitted is this case. It resolves LP: #1880029 --- src/abi.h | 3 +++ src/netplan.c | 6 ++++++ src/networkd.c | 15 ++++++--------- src/nm.c | 9 +++++++++ src/parse.c | 12 ++++++++++++ 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/abi.h b/src/abi.h index 052e77f92..6105a70e8 100644 --- a/src/abi.h +++ b/src/abi.h @@ -30,6 +30,9 @@ typedef enum { NETPLAN_OPTIONAL_DHCP4 = 1<<2, NETPLAN_OPTIONAL_DHCP6 = 1<<3, NETPLAN_OPTIONAL_STATIC = 1<<4, + NETPLAN_OPTIONAL_IPV4 = 1<<5, + NETPLAN_OPTIONAL_IPV6 = 1<<6, + NETPLAN_OPTIONAL_NONE = 1<<7, } NetplanOptionalAddressFlag; /* Fields below are valid for dhcp4 and dhcp6 unless otherwise noted. */ diff --git a/src/netplan.c b/src/netplan.c index 35aa103ce..21c4a1b74 100644 --- a/src/netplan.c +++ b/src/netplan.c @@ -877,6 +877,12 @@ _serialize_yaml( YAML_SCALAR_PLAIN(event, emitter, "dhcp6") if (def->optional_addresses & NETPLAN_OPTIONAL_STATIC) YAML_SCALAR_PLAIN(event, emitter, "static") + if (def->optional_addresses & NETPLAN_OPTIONAL_IPV4) + YAML_SCALAR_PLAIN(event, emitter, "ipv4") + if (def->optional_addresses & NETPLAN_OPTIONAL_IPV6) + YAML_SCALAR_PLAIN(event, emitter, "ipv6") + if (def->optional_addresses & NETPLAN_OPTIONAL_NONE) + YAML_SCALAR_PLAIN(event, emitter, "none") YAML_SEQUENCE_CLOSE(event, emitter); } diff --git a/src/networkd.c b/src/networkd.c index 11997d850..bb4415705 100644 --- a/src/networkd.c +++ b/src/networkd.c @@ -739,15 +739,12 @@ netplan_netdef_write_network_file( is_optional = TRUE; } - if (is_optional || def->optional_addresses) { - if (is_optional) { - g_string_append(link, "RequiredForOnline=no\n"); - } - for (unsigned i = 0; NETPLAN_OPTIONAL_ADDRESS_TYPES[i].name != NULL; ++i) { - if (def->optional_addresses & NETPLAN_OPTIONAL_ADDRESS_TYPES[i].flag) { - g_string_append_printf(link, "OptionalAddresses=%s\n", NETPLAN_OPTIONAL_ADDRESS_TYPES[i].name); - } - } + if (is_optional) { + g_string_append(link, "RequiredForOnline=no\n"); + } + + if (def->optional_addresses & NETPLAN_OPTIONAL_NONE) { + g_string_append(link, "RequiredFamilyForOnline=both\n"); } if (def->mtubytes) diff --git a/src/nm.c b/src/nm.c index b8e5b0011..90d17019f 100644 --- a/src/nm.c +++ b/src/nm.c @@ -926,6 +926,15 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir, } } + if (def->optional_addresses & NETPLAN_OPTIONAL_NONE) { + g_key_file_set_string(kf, "ipv4", "may-fail", "false"); + g_key_file_set_string(kf, "ipv6", "may-fail", "false"); + } else { + if (def->optional_addresses & NETPLAN_OPTIONAL_IPV4) + g_key_file_set_string(kf, "ipv4", "may-fail", "true"); + if (def->optional_addresses & NETPLAN_OPTIONAL_IPV6) + g_key_file_set_string(kf, "ipv6", "may-fail", "true"); + } /* NM connection files might contain secrets, and NM insists on tight permissions */ full_path = g_strjoin(G_DIR_SEPARATOR_S, rootdir ?: "", conf_path, NULL); orig_umask = umask(077); diff --git a/src/parse.c b/src/parse.c index 7d75c7f16..57191b0fe 100644 --- a/src/parse.c +++ b/src/parse.c @@ -1530,6 +1530,10 @@ NETPLAN_OPTIONAL_ADDRESS_TYPES[] = { {"dhcp4", NETPLAN_OPTIONAL_DHCP4}, {"dhcp6", NETPLAN_OPTIONAL_DHCP6}, {"static", NETPLAN_OPTIONAL_STATIC}, + /* All above are deprecated */ + {"ipv4", NETPLAN_OPTIONAL_IPV4}, + {"ipv6", NETPLAN_OPTIONAL_IPV6}, + {"none", NETPLAN_OPTIONAL_NONE}, {NULL}, }; @@ -1543,6 +1547,14 @@ handle_optional_addresses(NetplanParser* npp, yaml_node_t* node, const void* _, for (unsigned i = 0; NETPLAN_OPTIONAL_ADDRESS_TYPES[i].name != NULL; ++i) { if (g_ascii_strcasecmp(scalar(entry), NETPLAN_OPTIONAL_ADDRESS_TYPES[i].name) == 0) { + + /* Values below NETPLAN_OPTIONAL_STATIC (including it) are not valid and + * considered deprecated. See LP: #1880029 + */ + if (NETPLAN_OPTIONAL_ADDRESS_TYPES[i].flag <= NETPLAN_OPTIONAL_STATIC) + g_warning("Flag \"%s\" in optional-addresses is deprecated. Valid values are: " + "\"ipv4\", \"ipv6\" and \"none\"\n", NETPLAN_OPTIONAL_ADDRESS_TYPES[i].name); + npp->current.netdef->optional_addresses |= NETPLAN_OPTIONAL_ADDRESS_TYPES[i].flag; found = TRUE; break; From 8da4f648b26c66ecee81d475ac0fcc0aa1e86732 Mon Sep 17 00:00:00 2001 From: Danilo Egea Gondolfo Date: Tue, 4 Apr 2023 08:27:04 -0300 Subject: [PATCH 2/3] docs: update optional-addresses entry --- doc/netplan-yaml.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/netplan-yaml.md b/doc/netplan-yaml.md index 1834ac6e9..103f49f8d 100644 --- a/doc/netplan-yaml.md +++ b/doc/netplan-yaml.md @@ -531,11 +531,12 @@ Match devices by MAC when setting options like: `wakeonlan` or `*-offload`. - **optional-addresses** (sequence of scalars) - > Specify types of addresses that are not required for a device to be + > Specify address families that are not required for a device to be > considered online. This changes the behavior of backends at boot time to > avoid waiting for addresses that are marked optional, and thus consider > the interface as "usable" sooner. This does not disable these addresses, - > which will be brought up anyway. + > which will be brought up anyway. Valid values are: ipv4, ipv6 and none. + > If "none" is used, ipv4 and ipv6 will be considered mandatory. Example: @@ -545,7 +546,7 @@ Match devices by MAC when setting options like: `wakeonlan` or `*-offload`. eth7: dhcp4: true dhcp6: true - optional-addresses: [ ipv4-ll, dhcp6 ] + optional-addresses: [ ipv6 ] ``` - **activation-mode** (scalar) – since **0.103** From 84fc04aebe9c78518df8f2feb9ed2e0fe2f8c3e1 Mon Sep 17 00:00:00 2001 From: Danilo Egea Gondolfo Date: Tue, 4 Apr 2023 12:57:29 -0300 Subject: [PATCH 3/3] tests: fix tests after optional-addresses changes --- tests/generator/base.py | 2 +- tests/generator/test_common.py | 71 ++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/tests/generator/base.py b/tests/generator/base.py index e9d5085f4..22fc73be1 100644 --- a/tests/generator/base.py +++ b/tests/generator/base.py @@ -405,7 +405,7 @@ def get_network_config_for_link(self, link_name): def get_optional_addresses(self, eth_name): config = self.get_network_config_for_link(eth_name) r = set() - prefix = "OptionalAddresses=" + prefix = "RequiredFamilyForOnline=" for line in config.splitlines(): if line.startswith(prefix): r.add(line[len(prefix):]) diff --git a/tests/generator/test_common.py b/tests/generator/test_common.py index ad96b787b..67f8158f3 100644 --- a/tests/generator/test_common.py +++ b/tests/generator/test_common.py @@ -58,15 +58,26 @@ def config_with_optional_addresses(self, eth_name, optional_addresses): def test_optional_addresses(self): eth_name = self.eth_name() - self.generate(self.config_with_optional_addresses(eth_name, '["dhcp4"]')) - self.assertEqual(self.get_optional_addresses(eth_name), set(["dhcp4"])) + self.generate(self.config_with_optional_addresses(eth_name, '["ipv4"]')) + self.assertEqual(self.get_optional_addresses(eth_name), set()) def test_optional_addresses_multiple(self): eth_name = self.eth_name() - self.generate(self.config_with_optional_addresses(eth_name, '[dhcp4, ipv4-ll, ipv6-ra, dhcp6, dhcp4, static]')) + self.generate(self.config_with_optional_addresses(eth_name, '["ipv4", "ipv6", "ipv4"]')) self.assertEqual( self.get_optional_addresses(eth_name), - set(["ipv4-ll", "ipv6-ra", "dhcp4", "dhcp6", "static"])) + set()) + + def test_optional_addresses_none(self): + eth_name = self.eth_name() + self.generate(self.config_with_optional_addresses(eth_name, '["none"]')) + self.assertEqual(self.get_optional_addresses(eth_name), set(["both"])) + + def test_optional_addresses_deprecated_flags(self): + flags = '["ipv4-ll", "ipv6-ra", "dhcp4", "dhcp6", "static"]' + eth_name = self.eth_name() + self.generate(self.config_with_optional_addresses(eth_name, flags)) + self.assertEqual(self.get_optional_addresses(eth_name), set()) def test_optional_addresses_invalid(self): eth_name = self.eth_name() @@ -1322,6 +1333,58 @@ def test_nameserver(self): method=ignore '''}) + def test_optional_addresses(self): + self.generate('''network: + version: 2 + renderer: NetworkManager + ethernets: + eth0: + optional-addresses: [ipv4, ipv6]''') + + self.assert_nm({'eth0': '''[connection] +id=netplan-eth0 +type=ethernet +interface-name=eth0 + +[ethernet] +wake-on-lan=0 + +[ipv4] +method=link-local +may-fail=true + +[ipv6] +method=ignore +may-fail=true +'''}) + self.assert_networkd({}) + + def test_optional_addresses_none(self): + self.generate('''network: + version: 2 + renderer: NetworkManager + ethernets: + eth0: + optional-addresses: [none]''') + + self.assert_nm({'eth0': '''[connection] +id=netplan-eth0 +type=ethernet +interface-name=eth0 + +[ethernet] +wake-on-lan=0 + +[ipv4] +method=link-local +may-fail=false + +[ipv6] +method=ignore +may-fail=false +'''}) + self.assert_networkd({}) + class TestForwardDeclaration(TestBase):