From d6b64f7abbbfca1a93c97c334ebaf28332650ae7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Jul 2023 18:01:58 +0900 Subject: [PATCH 1/9] network/address: do not set configuring flag when a request is canceled Fixes copy-and-paste error. Follow-up for 0a0c2672dbd22dc85d660e5baa7e1bef701beb88. --- src/network/networkd-address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index e49d21f804..42d790887a 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1320,7 +1320,7 @@ static int address_process_request(Request *req, Link *link, Address *address) { address_cancel_requesting(address); if (address_get(link, address, &existing) >= 0) - address_enter_configuring(existing); + address_cancel_requesting(existing); return 1; } From c5874748a483aa30b453ebd144003255b4ecc294 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Jul 2023 18:03:40 +0900 Subject: [PATCH 2/9] network/address: add missing space in log message --- src/network/networkd-address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 42d790887a..09615a4ce4 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1825,7 +1825,7 @@ int config_parse_address( r = in_addr_prefix_from_string_auto(rvalue, &f, &buffer, &prefixlen); if (r >= 0) log_syntax(unit, LOG_WARNING, filename, line, r, - "Address '%s' is specified without prefix length. Assuming the prefix length is %u." + "Address '%s' is specified without prefix length. Assuming the prefix length is %u. " "Please specify the prefix length explicitly.", rvalue, prefixlen); } if (r < 0) { From b8e898a681a7d8a663b766f8e44469bc910d20cc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Jul 2023 18:04:14 +0900 Subject: [PATCH 3/9] network/address: make Label= accept an empty string We usually reset setting when an emptry string is specified. --- src/network/networkd-address.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 09615a4ce4..a7772a5895 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1895,6 +1895,12 @@ int config_parse_label( return 0; } + if (isempty(rvalue)) { + n->label = mfree(n->label); + TAKE_PTR(n); + return 0; + } + if (!address_label_valid(rvalue)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Interface label is too long or invalid, ignoring assignment: %s", rvalue); From f35aecc4168ba99dc4319954593b2adc31c17c10 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Jul 2023 18:07:46 +0900 Subject: [PATCH 4/9] network/address: always set IFA_ADDRESS attribute for IPv6 address on configure IPv6 address can update the peer address without removing the address. If an address have a peer, but now we want to drop the peer, we need to specify a null address in IFA_ADDRESS attribute. IPv4 address cannot update the peer address, so let's specify peer address only when necessary. --- src/network/networkd-address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index a7772a5895..2a46f051e0 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1256,7 +1256,7 @@ static int address_configure(const Address *address, const struct ifa_cacheinfo if (r < 0) return r; - if (in_addr_is_set(address->family, &address->in_addr_peer)) { + if (address->family == AF_INET6 || in_addr_is_set(address->family, &address->in_addr_peer)) { r = netlink_message_append_in_addr_union(m, IFA_ADDRESS, address->family, &address->in_addr_peer); if (r < 0) return r; From b5aefc90e6d8428aca20a1c05e258f6d01c154fd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Jul 2023 11:21:30 +0900 Subject: [PATCH 5/9] network/address: check if existing addresses can be updated in more detail Some properties of address can be updated, but some cannot. On reconfiguring an interface or restarting networkd, let's keep an assigned address only when it can be updated later with the requested setting, and otherwise drop it. --- src/network/networkd-address.c | 89 +++++++++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 2a46f051e0..d9dbe56590 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -457,6 +457,83 @@ int address_equal(const Address *a1, const Address *a2) { return address_compare_func(a1, a2) == 0; } +static bool address_can_update(const Address *la, const Address *na) { + assert(la); + assert(la->link); + assert(na); + assert(na->network); + + /* + * property | IPv4 | IPv6 + * ----------------------------------------- + * family | ✗ | ✗ + * prefixlen | ✗ | ✗ + * address | ✗ | ✗ + * scope | ✗ | - + * label | ✗ | - + * broadcast | ✗ | - + * peer | ✗ | ✓ + * flags | ✗ | ✓ + * lifetime | ✓ | ✓ + * route metric | ✓ | ✓ + * protocol | ✓ | ✓ + * + * ✗ : cannot be changed + * ✓ : can be changed + * - : unused + * + * IPv4 : See inet_rtm_newaddr() in net/ipv4/devinet.c. + * IPv6 : See inet6_addr_modify() in net/ipv6/addrconf.c. + */ + + if (la->family != na->family) + return false; + + if (la->prefixlen != na->prefixlen) + return false; + + /* When a null address is requested, the address to be assigned/updated will be determined later. */ + if (!address_is_static_null(na) && + in_addr_equal(la->family, &la->in_addr, &na->in_addr) <= 0) + return false; + + switch (la->family) { + case AF_INET: { + struct in_addr bcast; + + if (la->scope != na->scope) + return false; + if (((la->flags ^ na->flags) & KNOWN_FLAGS & ~IPV6ONLY_FLAGS & ~UNMANAGED_FLAGS) != 0) + return false; + if (!streq_ptr(la->label, na->label)) + return false; + if (!in4_addr_equal(&la->in_addr_peer.in, &na->in_addr_peer.in)) + return false; + if (address_get_broadcast(na, la->link, &bcast) >= 0) { + /* If the broadcast address can be determined now, check if they match. */ + if (!in4_addr_equal(&la->broadcast, &bcast)) + return false; + } else { + /* When a null address is requested, then the broadcast address will be + * automatically calculated from the acquired address, e.g. + * 192.168.0.10/24 -> 192.168.0.255 + * So, here let's only check if the broadcast is the last address in the range, e.g. + * 0.0.0.0/24 -> 0.0.0.255 */ + if (!FLAGS_SET(la->broadcast.s_addr, htobe32(UINT32_C(0xffffffff) >> la->prefixlen))) + return false; + } + break; + } + case AF_INET6: + break; + + default: + assert_not_reached(); + } + + return true; +} + int address_dup(const Address *src, Address **ret) { _cleanup_(address_freep) Address *dest = NULL; int r; @@ -1116,17 +1193,7 @@ int link_drop_foreign_addresses(Link *link) { if (address_get(link, address, &existing) < 0) continue; - /* On update, the kernel ignores the address label and broadcast address. Hence we need to - * distinguish addresses with different labels or broadcast addresses. Thus, we need to check - * the existing address with address_equal(). Otherwise, the label or broadcast address - * change will not be applied when we reconfigure the interface. */ - if (address_is_static_null(address)) { - /* If the static configuration has null address, then the broadcast address will be - * determined automatically. Hence, here we need to compare only address label. */ - if (address->family == AF_INET && !streq_ptr(address->label, existing->label)) - continue; - - } else if (!address_equal(address, existing)) + if (!address_can_update(existing, address)) continue; /* Found matching static configuration. Keep the existing address. */ From f2bcd3245a948871fa3eef86bba395ffd66f28d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Jul 2023 17:52:09 +0900 Subject: [PATCH 6/9] test-network: add more tests for address properties --- .../conf/25-address-static.network | 204 ++++++++----- .../10-override-route-metric.conf | 9 - .../10-override.conf | 139 +++++++++ test/test-network/systemd-networkd-tests.py | 267 +++++++++++++----- 4 files changed, 470 insertions(+), 149 deletions(-) delete mode 100644 test/test-network/conf/25-address-static.network.d/10-override-route-metric.conf create mode 100644 test/test-network/conf/25-address-static.network.d/10-override.conf diff --git a/test/test-network/conf/25-address-static.network b/test/test-network/conf/25-address-static.network index 2a96491b52..a406aab7e4 100644 --- a/test/test-network/conf/25-address-static.network +++ b/test/test-network/conf/25-address-static.network @@ -14,102 +14,166 @@ Address=2001:db8:0:f102::15/64 Address=hogehoge Address=foofoo +## With Label= [Address] -Address=10.3.2.3/16 -Label=32 +Address=10.3.1.1/24 +Label=label1 [Address] -Address=10.4.2.3/16 -Peer=10.4.2.4/16 -Label=33 +Address=10.3.2.1/24 +Label=label2 [Address] -Address=0.0.0.0/24 -Label=34 +Address=10.3.3.1/24 +Label= +## With Broadcast= [Address] -Address=0.0.0.0/16 -Label=35 - -[Address] -# this section must be ignored -Peer=hoge -Address=10.10.0.1/16 -Label=30 - -[Address] -# this section must be ignored -Label=30 -Peer=hoge -Address=10.10.0.2/16 - -[Address] -# address and peer must be in the same family -Address=10.10.0.3/16 -Peer=2001:db8:0:f103::10/128 - -[Address] -Address=2001:db8:0:f102::16/64 - -[Address] -Address=2001:db8:0:f103::20/128 -Peer=2001:db8:0:f103::10/128 - -[Address] -Address=2001:db8:0:f104::16/64 -RouteMetric=128 - -[Address] -Address=::/64 - -[Network] -# this will later deduped by the following section -Address=10.7.8.9/16 - -[Address] -Address=10.7.8.9/16 -PreferredLifetime=0 -Scope=link - -[Address] -# this will also deduped -Address=2001:0db8:1:f101::1/64 -PreferredLifetime=infinity - -[Address] -Address=2001:0db8:1:f101::1/64 -PreferredLifetime=0 - -[Address] -Address=10.8.8.1/16 -Broadcast=yes -Broadcast= +Address=10.4.1.1/24 Broadcast=no [Address] -Address=10.8.8.2/16 -Broadcast=10.8.8.128 +Address=10.4.2.1/24 +Broadcast=yes + +[Address] +Address=10.4.3.1/24 +Broadcast=10.4.3.63 [Address] # Invalid broadcast -Address=10.8.8.2/16 +Address=10.4.4.1/24 +Broadcast=yes +Broadcast= Broadcast=::1 [Address] # broadcast cannot set for IPv6 address -Address=2001:0db8:1:f101::2/64 +Address=2001:db8:0:f101::2/64 Broadcast=::1 +## With/without Peer= [Address] -Address=10.9.0.1/16 +Address=10.5.1.1/24 +Peer=10.5.1.101/24 + +[Address] +Address=10.5.2.1/24 +Peer=10.5.2.101/24 + +[Address] +Address=10.5.3.1/24 +#Peer= (no-peer) + +[Address] +Address=2001:db8:0:f103::1/128 +Peer=2001:db8:0:f103::101/128 + +[Address] +Address=2001:db8:0:f103::2/128 +Peer=2001:db8:0:f103::102/128 + +[Address] +Address=2001:db8:0:f103::3/128 +#Peer= (no-peer) + +[Address] +# address and peer must be in the same family +Address=10.5.4.1/24 +Peer=2001:db8:0:f103::10/128 + +[Address] +# address and peer must be in the same family +Address=2001:db8:0:f103::4/64 +Peer=192.168.0.1 + +[Address] +# invalid peer, this section will be ignored +Address=10.5.5.1/24 +Peer=hoge + +## With Scope= +[Address] +Address=10.6.1.1/24 +Scope=global + +[Address] +Address=10.6.2.1/24 +Scope=link + +## For lifetime +[Address] +Address=10.7.1.1/24 +PreferredLifetime=infinity + +[Address] +Address=10.7.2.1/24 +PreferredLifetime=0 + +[Address] +Address=2001:db8:0:f104::1/64 +PreferredLifetime=infinity + +[Address] +Address=2001:db8:0:f104::2/64 +PreferredLifetime=0 + +## For RouteMetric= +[Address] +Address=10.8.1.1/24 RouteMetric=128 [Address] # invalid metric -Address=10.9.0.1/16 +Address=10.8.2.1/24 RouteMetric=hoge [Address] -Address=10.4.3.2/24 +Address=2001:db8:0:f105::1/64 +RouteMetric=128 + +## For flags +[Address] +Address=10.9.1.1/24 +AddPrefixRoute=no + +[Address] +Address=10.9.2.1/24 +AddPrefixRoute=yes + +[Address] +# invalid flags for IPv4 address +Address=10.9.3.1/24 +HomeAddress=yes +ManageTemporaryAddress=yes + +[Address] +Address=2001:db8:0:f106::1/64 +HomeAddress=no +ManageTemporaryAddress=no +AddPrefixRoute=no + +[Address] +Address=2001:db8:0:f106::2/64 +HomeAddress=yes +ManageTemporaryAddress=yes +AddPrefixRoute=yes + +# TODO: add tests for AutoJoin= + +## null address +[Address] +Address=0.0.0.0/16 +Label=subnet16 + +[Address] +Address=0.0.0.0/24 +Label=subnet24 + +[Address] +Address=::/73 + +[Address] +Address=10.10.1.1/24 # just a random label which should exist NetLabel=system_u:object_r:root_t:s0 diff --git a/test/test-network/conf/25-address-static.network.d/10-override-route-metric.conf b/test/test-network/conf/25-address-static.network.d/10-override-route-metric.conf deleted file mode 100644 index da5bec2a34..0000000000 --- a/test/test-network/conf/25-address-static.network.d/10-override-route-metric.conf +++ /dev/null @@ -1,9 +0,0 @@ -# SPDX-License-Identifier: LGPL-2.1-or-later - -[Address] -Address=10.9.0.1/16 -RouteMetric=256 - -[Address] -Address=2001:db8:0:f104::16/64 -RouteMetric=256 diff --git a/test/test-network/conf/25-address-static.network.d/10-override.conf b/test/test-network/conf/25-address-static.network.d/10-override.conf new file mode 100644 index 0000000000..0c87809b4a --- /dev/null +++ b/test/test-network/conf/25-address-static.network.d/10-override.conf @@ -0,0 +1,139 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later + +# For checking if each address property is correctly updated. +# Comments are previous values + +## For Label= +[Address] +Address=10.3.1.1/24 +#Label=label1 +Label=new-label1 + +[Address] +Address=10.3.2.1/24 +#Label=label2 +Label= + +[Address] +Address=10.3.3.1/24 +#Label= +Label=new-label3 + +## For Broadcast= +[Address] +Address=10.4.1.1/24 +#Broadcast=no +Broadcast=yes + +[Address] +Address=10.4.2.1/24 +#Broadcast=yes +Broadcast=no + +[Address] +Address=10.4.3.1/24 +#Broadcast=10.4.3.63 +Broadcast=10.4.3.31 + +## For Peer= +[Address] +Address=10.5.1.1/24 +#Peer=10.5.1.101/24 +Peer=10.5.1.102/24 + +[Address] +Address=10.5.2.1/24 +#Peer=10.5.2.101/24 +#Peer= (no-peer) + +[Address] +Address=10.5.3.1/24 +#Peer= (no-peer) +Peer=10.5.3.102/24 + +[Address] +Address=2001:db8:0:f103::1/128 +#Peer=2001:db8:0:f103::101/128 +Peer=2001:db8:0:f103::201/128 + +[Address] +Address=2001:db8:0:f103::2/128 +#Peer=2001:db8:0:f103::102/128 +#Peer= (no-peer) + +[Address] +Address=2001:db8:0:f103::3/128 +#Peer= (no-peer) +Peer=2001:db8:0:f103::203/128 + +## For Scope= +[Address] +Address=10.6.1.1/24 +#Scope=global +Scope=link + +[Address] +Address=10.6.2.1/24 +#Scope=link +Scope=global + +## For lifetime +[Address] +Address=10.7.1.1/24 +#PreferredLifetime=infinity +PreferredLifetime=0 + +[Address] +Address=10.7.2.1/24 +#PreferredLifetime=0 +PreferredLifetime=infinity + +[Address] +Address=2001:db8:0:f104::1/64 +#PreferredLifetime=infinity +PreferredLifetime=0 + +[Address] +Address=2001:db8:0:f104::2/64 +#PreferredLifetime=0 +PreferredLifetime=infinity + +## For RouteMetric= +[Address] +Address=10.8.1.1/24 +# RouteMetric=128 +RouteMetric=256 + +[Address] +Address=2001:db8:0:f105::1/64 +# RouteMetric=128 +RouteMetric=256 + +## For flags +[Address] +Address=10.9.1.1/24 +#AddPrefixRoute=no +AddPrefixRoute=yes + +[Address] +Address=10.9.2.1/24 +#AddPrefixRoute=yes +AddPrefixRoute=no + +[Address] +Address=2001:db8:0:f106::1/64 +#HomeAddress=no +#ManageTemporaryAddress=no +#AddPrefixRoute=no +HomeAddress=yes +ManageTemporaryAddress=yes +AddPrefixRoute=yes + +[Address] +Address=2001:db8:0:f106::2/64 +#HomeAddress=yes +#ManageTemporaryAddress=yes +#AddPrefixRoute=yes +HomeAddress=no +ManageTemporaryAddress=no +AddPrefixRoute=no diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 7c13af58e8..7f922b46f6 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -2329,107 +2329,234 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): def tearDown(self): tear_down_common() - def verify_address_static(self, route_metric): - # IPv4 addresses - output = check_output('ip -4 address show dev dummy98') + def verify_address_static( + self, + label1: str, + label2: str, + label3: str, + broadcast1: str, + broadcast2: str, + broadcast3: str, + peer1: str, + peer2: str, + peer3: str, + peer4: str, + peer5: str, + peer6: str, + scope1: str, + scope2: str, + deprecated1: str, + deprecated2: str, + deprecated3: str, + deprecated4: str, + route_metric: int, + flag1: str, + flag2: str, + flag3: str, + flag4: str, + ): + output = check_output('ip address show dev dummy98') print(output) + + # simple settings self.assertIn('inet 10.1.2.3/16 brd 10.1.255.255 scope global dummy98', output) self.assertIn('inet 10.1.2.4/16 brd 10.1.255.255 scope global secondary dummy98', output) self.assertIn('inet 10.2.2.4/16 brd 10.2.255.255 scope global dummy98', output) - self.assertIn('inet 10.7.8.9/16 brd 10.7.255.255 scope link deprecated dummy98', output) - self.assertIn('inet 10.8.8.1/16 scope global dummy98', output) - self.assertIn('inet 10.8.8.2/16 brd 10.8.8.128 scope global secondary dummy98', output) - self.assertRegex(output, rf'inet 10.9.0.1/16 (metric {route_metric} |)brd 10.9.255.255 scope global dummy98') - - # invalid sections - self.assertNotIn('10.10.0.1/16', output) - self.assertNotIn('10.10.0.2/16', output) - - # address labels - output = check_output('ip -4 address show dev dummy98 label 32') - self.assertIn('inet 10.3.2.3/16 brd 10.3.255.255 scope global 32', output) - - output = check_output('ip -4 address show dev dummy98 label 33') - self.assertIn('inet 10.4.2.3 peer 10.4.2.4/16 scope global 33', output) - - output = check_output('ip -4 address show dev dummy98 label 34') - self.assertRegex(output, r'inet 192.168.[0-9]*.1/24 brd 192.168.[0-9]*.255 scope global 34') - - output = check_output('ip -4 address show dev dummy98 label 35') - self.assertRegex(output, r'inet 172.[0-9]*.0.1/16 brd 172.[0-9]*.255.255 scope global 35') - - # check metric of prefix route - output = check_output('ip -4 route show dev dummy98 10.9.0.0/16') - print(output) - self.assertIn(f'10.9.0.0/16 proto kernel scope link src 10.9.0.1 metric {route_metric}', output) - output = check_output('ip -6 route show dev dummy98 2001:db8:0:f104::/64') - print(output) - self.assertIn(f'2001:db8:0:f104::/64 proto kernel metric {route_metric}', output) - - # IPv6 addresses - output = check_output('ip -6 address show dev dummy98') - print(output) self.assertIn('inet6 2001:db8:0:f101::15/64 scope global', output) self.assertIn('inet6 2001:db8:0:f101::16/64 scope global', output) self.assertIn('inet6 2001:db8:0:f102::15/64 scope global', output) - self.assertIn('inet6 2001:db8:0:f102::16/64 scope global', output) - self.assertIn('inet6 2001:db8:0:f103::20 peer 2001:db8:0:f103::10/128 scope global', output) - self.assertRegex(output, rf'inet6 2001:db8:0:f104::16/64 (metric {route_metric} |)scope global') - self.assertIn('inet6 2001:db8:1:f101::1/64 scope global deprecated', output) - self.assertRegex(output, r'inet6 fd[0-9a-f:]*1/64 scope global') + + # label + self.assertIn(f'inet 10.3.1.1/24 brd 10.3.1.255 scope global {label1}', output) + self.assertIn(f'inet 10.3.2.1/24 brd 10.3.2.255 scope global {label2}', output) + self.assertIn(f'inet 10.3.3.1/24 brd 10.3.3.255 scope global {label3}', output) + + # broadcast + self.assertIn(f'inet 10.4.1.1/24{broadcast1} scope global dummy98', output) + self.assertIn(f'inet 10.4.2.1/24{broadcast2} scope global dummy98', output) + self.assertIn(f'inet 10.4.3.1/24{broadcast3} scope global dummy98', output) + + # peer + self.assertIn(f'inet 10.5.1.1{peer1} scope global dummy98', output) + self.assertIn(f'inet 10.5.2.1{peer2} scope global dummy98', output) + self.assertIn(f'inet 10.5.3.1{peer3} scope global dummy98', output) + self.assertIn(f'inet6 2001:db8:0:f103::1{peer4} scope global', output) + self.assertIn(f'inet6 2001:db8:0:f103::2{peer5} scope global', output) + self.assertIn(f'inet6 2001:db8:0:f103::3{peer6} scope global', output) + + # scope + self.assertIn(f'inet 10.6.1.1/24 brd 10.6.1.255 scope {scope1} dummy98', output) + self.assertIn(f'inet 10.6.2.1/24 brd 10.6.2.255 scope {scope2} dummy98', output) + + # lifetime + self.assertIn(f'inet 10.7.1.1/24 brd 10.7.1.255 scope global{deprecated1} dummy98', output) + self.assertIn(f'inet 10.7.2.1/24 brd 10.7.2.255 scope global{deprecated2} dummy98', output) + self.assertIn(f'inet6 2001:db8:0:f104::1/64 scope global{deprecated3}', output) + self.assertIn(f'inet6 2001:db8:0:f104::2/64 scope global{deprecated4}', output) + + # route metric + self.assertRegex(output, rf'inet 10.8.1.1/24 (metric {route_metric} |)brd 10.8.1.255 scope global dummy98') + self.assertRegex(output, rf'inet6 2001:db8:0:f105::1/64 (metric {route_metric} |)scope global') + + output_route = check_output('ip -4 route show dev dummy98 10.8.1.0/24') + print(output_route) + self.assertIn(f'10.8.1.0/24 proto kernel scope link src 10.8.1.1 metric {route_metric}', output_route) + + output_route = check_output('ip -6 route show dev dummy98 2001:db8:0:f105::/64') + print(output_route) + self.assertIn(f'2001:db8:0:f105::/64 proto kernel metric {route_metric}', output_route) + + # flags + self.assertIn(f'inet 10.9.1.1/24 brd 10.9.1.255 scope global{flag1} dummy98', output) + self.assertIn(f'inet 10.9.2.1/24 brd 10.9.2.255 scope global{flag2} dummy98', output) + self.assertIn(f'inet6 2001:db8:0:f106::1/64 scope global{flag3}', output) + self.assertIn(f'inet6 2001:db8:0:f106::2/64 scope global{flag4}', output) + + # null address + self.assertRegex(output, r'inet [0-9]*.[0-9]*.0.1/16 brd [0-9]*.[0-9]*.255.255 scope global subnet16') + self.assertRegex(output, r'inet [0-9]*.[0-9]*.[0-9]*.1/24 brd [0-9]*.[0-9]*.[0-9]*.255 scope global subnet24') + self.assertRegex(output, r'inet6 [0-9a-f:]*:1/73 scope global') + + # invalid sections + self.assertNotIn('10.4.4.1', output) + self.assertNotIn('10.5.4.1', output) + self.assertNotIn('10.5.5.1', output) + self.assertNotIn('10.8.2.1', output) + self.assertNotIn('10.9.3.1', output) + self.assertNotIn('2001:db8:0:f101::2', output) + self.assertNotIn('2001:db8:0:f103::4', output) # netlabel - self.check_netlabel('dummy98', r'10\.4\.3\.0/24') + self.check_netlabel('dummy98', r'10\.10\.1\.0/24') output = check_output(*networkctl_cmd, '--json=short', 'status', env=env) check_json(output) def test_address_static(self): - # test for #22515. The address will be removed and replaced with /64 prefix. - check_output('ip link add dummy98 type dummy') - check_output('ip link set dev dummy98 up') - check_output('ip -6 address add 2001:db8:0:f101::15/128 dev dummy98') - self.wait_address('dummy98', '2001:db8:0:f101::15/128', ipv='-6') - check_output('ip -4 address add 10.3.2.3/16 brd 10.3.255.250 scope global label dummy98:hoge dev dummy98') - self.wait_address('dummy98', '10.3.2.3/16 brd 10.3.255.250', ipv='-4') - copy_network_unit('25-address-static.network', '12-dummy.netdev', copy_dropins=False) start_networkd() self.wait_online(['dummy98:routable']) - self.verify_address_static(route_metric=128) + self.verify_address_static( + label1='label1', + label2='label2', + label3='dummy98', + broadcast1='', + broadcast2=' brd 10.4.2.255', + broadcast3=' brd 10.4.3.63', + peer1=' peer 10.5.1.101/24', + peer2=' peer 10.5.2.101/24', + peer3='/24 brd 10.5.3.255', + peer4=' peer 2001:db8:0:f103::101/128', + peer5=' peer 2001:db8:0:f103::102/128', + peer6='/128', + scope1='global', + scope2='link', + deprecated1='', + deprecated2=' deprecated', + deprecated3='', + deprecated4=' deprecated', + route_metric=128, + flag1=' noprefixroute', + flag2='', + flag3=' noprefixroute', + flag4=' home mngtmpaddr', + ) - copy_network_unit('25-address-static.network.d/10-override-route-metric.conf') + copy_network_unit('25-address-static.network.d/10-override.conf') networkctl_reload() self.wait_online(['dummy98:routable']) - self.verify_address_static(route_metric=256) + self.verify_address_static( + label1='new-label1', + label2='dummy98', + label3='new-label3', + broadcast1=' brd 10.4.1.255', + broadcast2='', + broadcast3=' brd 10.4.3.31', + peer1=' peer 10.5.1.102/24', + peer2='/24 brd 10.5.2.255', + peer3=' peer 10.5.3.102/24', + peer4=' peer 2001:db8:0:f103::201/128', + peer5='/128', + peer6=' peer 2001:db8:0:f103::203/128', + scope1='link', + scope2='global', + deprecated1=' deprecated', + deprecated2='', + deprecated3=' deprecated', + deprecated4='', + route_metric=256, + flag1='', + flag2=' noprefixroute', + flag3=' home mngtmpaddr', + flag4=' noprefixroute', + ) networkctl_reconfigure('dummy98') self.wait_online(['dummy98:routable']) - self.verify_address_static(route_metric=256) + self.verify_address_static( + label1='new-label1', + label2='dummy98', + label3='new-label3', + broadcast1=' brd 10.4.1.255', + broadcast2='', + broadcast3=' brd 10.4.3.31', + peer1=' peer 10.5.1.102/24', + peer2='/24 brd 10.5.2.255', + peer3=' peer 10.5.3.102/24', + peer4=' peer 2001:db8:0:f103::201/128', + peer5='/128', + peer6=' peer 2001:db8:0:f103::203/128', + scope1='link', + scope2='global', + deprecated1=' deprecated', + deprecated2='', + deprecated3=' deprecated', + deprecated4='', + route_metric=256, + flag1='', + flag2=' noprefixroute', + flag3=' home mngtmpaddr', + flag4=' noprefixroute', + ) # Tests for #20891. # 1. set preferred lifetime forever to drop the deprecated flag for testing #20891. - check_output('ip address change 10.7.8.9/16 dev dummy98 preferred_lft forever') - check_output('ip address change 2001:db8:1:f101::1/64 dev dummy98 preferred_lft forever') - output = check_output('ip -4 address show dev dummy98') + check_output('ip address change 10.7.1.1/24 dev dummy98 preferred_lft forever') + check_output('ip address change 2001:db8:0:f104::1/64 dev dummy98 preferred_lft forever') + output = check_output('ip address show dev dummy98') print(output) - self.assertNotIn('deprecated', output) - output = check_output('ip -6 address show dev dummy98') - print(output) - self.assertNotIn('deprecated', output) + self.assertNotRegex(output, '10.7.1.1/24 .* deprecated') + self.assertNotRegex(output, '2001:db8:0:f104::1/64 .* deprecated') - # 2. reconfigure the interface. + # 2. reconfigure the interface, and check the deprecated flag is set again networkctl_reconfigure('dummy98') self.wait_online(['dummy98:routable']) - - # 3. check the deprecated flag is set for the address configured with PreferredLifetime=0 - output = check_output('ip -4 address show dev dummy98') - print(output) - self.assertIn('inet 10.7.8.9/16 brd 10.7.255.255 scope link deprecated dummy98', output) - output = check_output('ip -6 address show dev dummy98') - print(output) - self.assertIn('inet6 2001:db8:1:f101::1/64 scope global deprecated', output) + self.verify_address_static( + label1='new-label1', + label2='dummy98', + label3='new-label3', + broadcast1=' brd 10.4.1.255', + broadcast2='', + broadcast3=' brd 10.4.3.31', + peer1=' peer 10.5.1.102/24', + peer2='/24 brd 10.5.2.255', + peer3=' peer 10.5.3.102/24', + peer4=' peer 2001:db8:0:f103::201/128', + peer5='/128', + peer6=' peer 2001:db8:0:f103::203/128', + scope1='link', + scope2='global', + deprecated1=' deprecated', + deprecated2='', + deprecated3=' deprecated', + deprecated4='', + route_metric=256, + flag1='', + flag2=' noprefixroute', + flag3=' home mngtmpaddr', + flag4=' noprefixroute', + ) # test for ENOBUFS issue #17012 (with reload) copy_network_unit('25-address-static.network.d/10-many-address.conf') From 35f0cd02d8857fa1f5d0b11a0aab58d871eaf6ea Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Jul 2023 11:22:35 +0900 Subject: [PATCH 7/9] network/address: drop unused functions --- src/network/networkd-address.c | 49 ---------------------------------- src/network/networkd-address.h | 3 --- src/network/test-network.c | 49 ---------------------------------- 3 files changed, 101 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index d9dbe56590..0bc8e0f020 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -408,55 +408,6 @@ DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( address_kernel_compare_func, address_free); -int address_compare_func(const Address *a1, const Address *a2) { - int r; - - r = CMP(a1->family, a2->family); - if (r != 0) - return r; - - if (!IN_SET(a1->family, AF_INET, AF_INET6)) - return 0; - - r = CMP(a1->prefixlen, a2->prefixlen); - if (r != 0) - return r; - - r = memcmp(&a1->in_addr, &a2->in_addr, FAMILY_ADDRESS_SIZE(a1->family)); - if (r != 0) - return r; - - r = memcmp(&a1->in_addr_peer, &a2->in_addr_peer, FAMILY_ADDRESS_SIZE(a1->family)); - if (r != 0) - return r; - - if (a1->family == AF_INET) { - /* On update, the kernel ignores the address label and broadcast address, hence we need - * to distinguish addresses with different labels or broadcast addresses. Otherwise, - * the label or broadcast address change will not be applied when we reconfigure the - * interface. */ - r = strcmp_ptr(a1->label, a2->label); - if (r != 0) - return r; - - r = CMP(a1->broadcast.s_addr, a2->broadcast.s_addr); - if (r != 0) - return r; - } - - return 0; -} - -int address_equal(const Address *a1, const Address *a2) { - if (a1 == a2) - return true; - - if (!a1 || !a2) - return false; - - return address_compare_func(a1, a2) == 0; -} - static bool address_can_update(const Address *la, const Address *na) { assert(la); assert(la->link); diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index bdb5b5b5e5..7a611529f7 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -119,9 +119,6 @@ int manager_rtnl_process_address(sd_netlink *nl, sd_netlink_message *message, Ma int network_drop_invalid_addresses(Network *network); -int address_compare_func(const Address *a1, const Address *a2); -int address_equal(const Address *a1, const Address *a2); - DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Address, address); void link_mark_addresses(Link *link, NetworkConfigSource source); diff --git a/src/network/test-network.c b/src/network/test-network.c index 35592e0733..5f3b4c0b31 100644 --- a/src/network/test-network.c +++ b/src/network/test-network.c @@ -180,54 +180,6 @@ static int test_load_config(Manager *manager) { return 0; } -static void test_address_equality(void) { - _cleanup_(address_freep) Address *a1 = NULL, *a2 = NULL; - - assert_se(address_new(&a1) >= 0); - assert_se(address_new(&a2) >= 0); - - assert_se(address_equal(NULL, NULL)); - assert_se(!address_equal(a1, NULL)); - assert_se(!address_equal(NULL, a2)); - assert_se(address_equal(a1, a2)); - - a1->family = AF_INET; - assert_se(!address_equal(a1, a2)); - - a2->family = AF_INET; - assert_se(address_equal(a1, a2)); - - assert_se(in_addr_from_string(AF_INET, "192.168.3.9", &a1->in_addr) >= 0); - assert_se(!address_equal(a1, a2)); - assert_se(in_addr_from_string(AF_INET, "192.168.3.9", &a2->in_addr) >= 0); - assert_se(address_equal(a1, a2)); - assert_se(in_addr_from_string(AF_INET, "192.168.3.10", &a1->in_addr_peer) >= 0); - assert_se(!address_equal(a1, a2)); - assert_se(in_addr_from_string(AF_INET, "192.168.3.11", &a2->in_addr_peer) >= 0); - assert_se(!address_equal(a1, a2)); - assert_se(in_addr_from_string(AF_INET, "192.168.3.10", &a2->in_addr_peer) >= 0); - assert_se(address_equal(a1, a2)); - a1->prefixlen = 10; - assert_se(!address_equal(a1, a2)); - a2->prefixlen = 10; - assert_se(address_equal(a1, a2)); - - a1->family = AF_INET6; - assert_se(!address_equal(a1, a2)); - - a2->family = AF_INET6; - assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::2", &a1->in_addr) >= 0); - assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::2", &a2->in_addr) >= 0); - assert_se(address_equal(a1, a2)); - - a2->prefixlen = 8; - assert_se(!address_equal(a1, a2)); - - a2->prefixlen = 10; - assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::1", &a2->in_addr) >= 0); - assert_se(!address_equal(a1, a2)); -} - static void test_dhcp_hostname_shorten_overlong(void) { int r; @@ -281,7 +233,6 @@ int main(void) { test_deserialize_in_addr(); test_deserialize_dhcp_routes(); - test_address_equality(); test_dhcp_hostname_shorten_overlong(); assert_se(manager_new(&manager, /* test_mode = */ true) >= 0); From 4da252c59a30d94f7d6a698c6460c45a37e13154 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Jul 2023 11:24:06 +0900 Subject: [PATCH 8/9] network/address: rename address_kernel_{compare,hash}_func() -> address_{compare,hash}_func() Previously, we had two compare/hash functions. But we do not have one of them anymore. Let's rename them. --- src/network/networkd-address.c | 36 +++++++++++++++++----------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 0bc8e0f020..8d6dec3668 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -345,7 +345,7 @@ static uint32_t address_prefix(const Address *a) { return be32toh(a->in_addr.in.s_addr) >> (32 - a->prefixlen); } -static void address_kernel_hash_func(const Address *a, struct siphash *state) { +static void address_hash_func(const Address *a, struct siphash *state) { assert(a); siphash24_compress(&a->family, sizeof(a->family), state); @@ -367,7 +367,7 @@ static void address_kernel_hash_func(const Address *a, struct siphash *state) { } } -static int address_kernel_compare_func(const Address *a1, const Address *a2) { +static int address_compare_func(const Address *a1, const Address *a2) { int r; r = CMP(a1->family, a2->family); @@ -396,16 +396,16 @@ static int address_kernel_compare_func(const Address *a1, const Address *a2) { } DEFINE_PRIVATE_HASH_OPS( - address_kernel_hash_ops, + address_hash_ops, Address, - address_kernel_hash_func, - address_kernel_compare_func); + address_hash_func, + address_compare_func); DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( - address_kernel_hash_ops_free, + address_hash_ops_free, Address, - address_kernel_hash_func, - address_kernel_compare_func, + address_hash_func, + address_compare_func, address_free); static bool address_can_update(const Address *la, const Address *na) { @@ -561,7 +561,7 @@ static int address_add(Link *link, Address *address) { assert(link); assert(address); - r = set_ensure_put(&link->addresses, &address_kernel_hash_ops_free, address); + r = set_ensure_put(&link->addresses, &address_hash_ops_free, address); if (r < 0) return r; if (r == 0) @@ -658,8 +658,8 @@ static int address_get_request(Link *link, const Address *address, Request **ret .link = link, .type = REQUEST_TYPE_ADDRESS, .userdata = (void*) address, - .hash_func = (hash_func_t) address_kernel_hash_func, - .compare_func = (compare_func_t) address_kernel_compare_func, + .hash_func = (hash_func_t) address_hash_func, + .compare_func = (compare_func_t) address_compare_func, }); if (req) { if (ret) @@ -1406,8 +1406,8 @@ int link_request_address( r = link_queue_request_safe(link, REQUEST_TYPE_ADDRESS, tmp, address_free, - address_kernel_hash_func, - address_kernel_compare_func, + address_hash_func, + address_compare_func, address_process_request, message_counter, netlink_handler, ret); if (r < 0) @@ -1497,8 +1497,8 @@ void address_cancel_request(Address *address) { .link = address->link, .type = REQUEST_TYPE_ADDRESS, .userdata = address, - .hash_func = (hash_func_t) address_kernel_hash_func, - .compare_func = (compare_func_t) address_kernel_compare_func, + .hash_func = (hash_func_t) address_hash_func, + .compare_func = (compare_func_t) address_compare_func, }; request_detach(address->link->manager, &req); @@ -2342,9 +2342,9 @@ int network_drop_invalid_addresses(Network *network) { address_free(dup); } - /* Use address_kernel_hash_ops, instead of address_kernel_hash_ops_free. Otherwise, the - * Address objects will be freed. */ - r = set_ensure_put(&addresses, &address_kernel_hash_ops, address); + /* Use address_hash_ops, instead of address_hash_ops_free. Otherwise, the Address objects + * will be freed. */ + r = set_ensure_put(&addresses, &address_hash_ops, address); if (r < 0) return log_oom(); assert(r > 0); From 6756b61626afc343f8b6824f41bef67bb442f1d7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 31 Jul 2023 22:11:51 +0900 Subject: [PATCH 9/9] resolve: initialize 'current' when SD_RESOLVED_NO_STALE is set Otherwise, the check below is always fail. ``` if (FLAGS_SET(query_flags, SD_RESOLVED_NO_STALE) && j->until_valid < current) ``` Follow-up for 5ed91481abea382dc486507556e5cdf0f36b796f. --- src/resolve/resolved-dns-cache.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 9d2a2fa1ac..3805150d98 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -1030,9 +1030,10 @@ int dns_cache_lookup( goto miss; } - if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) { + if ((query_flags & (SD_RESOLVED_CLAMP_TTL | SD_RESOLVED_NO_STALE)) != 0) { /* 'current' is always passed to answer_add_clamp_ttl(), but is only used conditionally. - * We'll do the same assert there to make sure that it was initialized properly. */ + * We'll do the same assert there to make sure that it was initialized properly. + * 'current' is also used below when SD_RESOLVED_NO_STALE is set. */ current = now(CLOCK_BOOTTIME); assert(current > 0); }