From 1d30fc5cb64ecba2f03fe42aa0d8c65c3decad82 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 4 Dec 2020 16:41:08 +0900 Subject: [PATCH] network: revert previous changes to address_compare_func() This partially reverts fe841414ef157f7f01d339c5d5730126e7b5fe0a and 2a236f9fc0ff8fb2152032551436fde74da7217a. For IPv4, kernel compares the local address, prefix, and prefixlen. For IPv6, kernel compares only the local address. Let's follow the kernel's comparison way. Fixes #17831. --- src/network/networkd-address.c | 79 ++++++++++++++++++---------------- src/network/test-network.c | 7 +-- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 9de4d82662..bc7e0a5b59 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -153,26 +153,40 @@ static bool address_may_have_broadcast(const Address *a) { return a->family == AF_INET && in4_addr_is_null(&a->in_addr_peer.in) && a->prefixlen <= 30; } +static uint32_t address_prefix(const Address *a) { + assert(a); + + /* make sure we don't try to shift by 32. + * See ISO/IEC 9899:TC3 ยง 6.5.7.3. */ + if (a->prefixlen == 0) + return 0; + + if (a->in_addr_peer.in.s_addr != 0) + return be32toh(a->in_addr_peer.in.s_addr) >> (32 - a->prefixlen); + else + return be32toh(a->in_addr.in.s_addr) >> (32 - a->prefixlen); +} + void address_hash_func(const Address *a, struct siphash *state) { assert(a); siphash24_compress(&a->family, sizeof(a->family), state); - if (!IN_SET(a->family, AF_INET, AF_INET6)) - /* treat non-IPv4 or IPv6 address family as AF_UNSPEC */ - return; + switch (a->family) { + case AF_INET: + siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state); - if (a->family == AF_INET) - siphash24_compress_string(a->label, state); + uint32_t prefix = address_prefix(a); + siphash24_compress(&prefix, sizeof(prefix), state); - siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state); - /* local address */ - siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state); - /* peer address */ - siphash24_compress(&a->in_addr_peer, FAMILY_ADDRESS_SIZE(a->family), state); - - if (address_may_have_broadcast(a)) - siphash24_compress(&a->broadcast, sizeof(a->broadcast), state); + _fallthrough_; + case AF_INET6: + siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state); + break; + default: + /* treat any other address family as AF_UNSPEC */ + break; + } } int address_compare_func(const Address *a1, const Address *a2) { @@ -182,32 +196,25 @@ int address_compare_func(const Address *a1, const Address *a2) { if (r != 0) return r; - if (!IN_SET(a1->family, AF_INET, AF_INET6)) - /* treat non-IPv4 or IPv6 address family as AF_UNSPEC */ - return 0; - - if (a1->family == AF_INET) { - r = strcmp_ptr(a1->label, a2->label); + switch (a1->family) { + case AF_INET: + /* See kernel's find_matching_ifa() in net/ipv4/devinet.c */ + r = CMP(a1->prefixlen, a2->prefixlen); if (r != 0) return r; + + r = CMP(address_prefix(a1), address_prefix(a2)); + if (r != 0) + return r; + + _fallthrough_; + case AF_INET6: + /* See kernel's ipv6_get_ifaddr() in net/ipv6/addrconf.c */ + return memcmp(&a1->in_addr, &a2->in_addr, FAMILY_ADDRESS_SIZE(a1->family)); + default: + /* treat any other address family as AF_UNSPEC */ + 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 (address_may_have_broadcast(a1)) - return CMP(a1->broadcast.s_addr, a2->broadcast.s_addr); - - return 0; } DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(address_hash_ops, Address, address_hash_func, address_compare_func, address_free); diff --git a/src/network/test-network.c b/src/network/test-network.c index bb67c74e9b..03c94409fa 100644 --- a/src/network/test-network.c +++ b/src/network/test-network.c @@ -159,10 +159,8 @@ static void test_address_equality(void) { 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(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)); - a2->in_addr_peer = a1->in_addr_peer; assert_se(address_equal(a1, a2)); a1->prefixlen = 10; assert_se(!address_equal(a1, a2)); @@ -173,13 +171,10 @@ static void test_address_equality(void) { assert_se(!address_equal(a1, a2)); a2->family = AF_INET6; - a1->in_addr_peer = a2->in_addr_peer = IN_ADDR_NULL; 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)); - a1->prefixlen = 8; - assert_se(!address_equal(a1, a2)); a2->prefixlen = 8; assert_se(address_equal(a1, a2));