From 45aa0e841ba084ef259d5970e340702130cd548c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 7 Dec 2021 08:23:27 +0900 Subject: [PATCH] network,udev: do not adjust local assignment bit of specified MAC address People often assigns the MAC address of the enslaved interface to e.g. bridge interface. So, the local assignment bit should not be adjusted. Fixes #21649. --- src/network/netdev/netdev.c | 6 +++--- src/network/networkd-setlink.c | 2 +- src/shared/netif-util.c | 23 +++++++++------------ src/shared/netif-util.h | 2 +- src/udev/net/link-config.c | 6 +++--- test/test-network/systemd-networkd-tests.py | 6 ++---- 6 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index f6732925940..9f9519fa50c 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -402,7 +402,7 @@ int netdev_generate_hw_addr( struct hw_addr_data *ret) { struct hw_addr_data a = HW_ADDR_NULL; - bool warn_invalid = false; + bool is_static = false; int r; assert(netdev); @@ -465,10 +465,10 @@ int netdev_generate_hw_addr( } else { a = *hw_addr; - warn_invalid = true; + is_static = true; } - r = net_verify_hardware_address(name, warn_invalid, NETDEV_VTABLE(netdev)->iftype, + r = net_verify_hardware_address(name, is_static, NETDEV_VTABLE(netdev)->iftype, parent ? &parent->hw_addr : NULL, &a); if (r < 0) return r; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 177c054f463..ff4eecf9b3b 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -807,7 +807,7 @@ int link_request_to_set_mac(Link *link, bool allow_retry) { return 0; link->requested_hw_addr = link->network->hw_addr; - r = net_verify_hardware_address(link->ifname, /* warn_invalid = */ true, + r = net_verify_hardware_address(link->ifname, /* is_static = */ true, link->iftype, &link->hw_addr, &link->requested_hw_addr); if (r < 0) return r; diff --git a/src/shared/netif-util.c b/src/shared/netif-util.c index 6afb3c3b691..603d4de109f 100644 --- a/src/shared/netif-util.c +++ b/src/shared/netif-util.c @@ -110,7 +110,7 @@ typedef struct Link { int net_verify_hardware_address( const char *ifname, - bool warn_invalid, + bool is_static, uint16_t iftype, const struct hw_addr_data *ib_hw_addr, /* current or parent HW address */ struct hw_addr_data *new_hw_addr) { @@ -123,7 +123,7 @@ int net_verify_hardware_address( return 0; if (new_hw_addr->length != arphrd_to_hw_addr_len(iftype)) { - if (warn_invalid) + if (is_static) log_link_warning(&link, "Specified MAC address with invalid length (%zu, expected %zu), refusing.", new_hw_addr->length, arphrd_to_hw_addr_len(iftype)); @@ -135,30 +135,27 @@ int net_verify_hardware_address( /* see eth_random_addr() in the kernel */ if (ether_addr_is_null(&new_hw_addr->ether)) { - if (warn_invalid) + if (is_static) log_link_warning(&link, "Specified MAC address is null, refusing."); return -EINVAL; } if (ether_addr_is_broadcast(&new_hw_addr->ether)) { - if (warn_invalid) + if (is_static) log_link_warning(&link, "Specified MAC address is broadcast, refusing."); return -EINVAL; } if (ether_addr_is_multicast(&new_hw_addr->ether)) { - if (warn_invalid) + if (is_static) log_link_warning(&link, "Specified MAC address has the multicast bit set, clearing the bit."); new_hw_addr->bytes[0] &= 0xfe; } - if (!ether_addr_is_local(&new_hw_addr->ether)) { - if (warn_invalid) - log_link_warning(&link, "Specified MAC address does not have the local assignment bit set, setting the bit."); - + if (!is_static && !ether_addr_is_local(&new_hw_addr->ether)) + /* Adjust local assignment bit when the MAC address is generated randomly. */ new_hw_addr->bytes[0] |= 0x02; - } break; @@ -168,13 +165,13 @@ int net_verify_hardware_address( assert(ib_hw_addr); assert(ib_hw_addr->length == INFINIBAND_ALEN); - if (warn_invalid && + if (is_static && (!memeqzero(new_hw_addr->bytes, INFINIBAND_ALEN - 8) || memcmp(new_hw_addr->bytes, ib_hw_addr->bytes, INFINIBAND_ALEN - 8) != 0)) log_link_warning(&link, "Only the last 8 bytes of the InifniBand MAC address can be changed, ignoring the first 12 bytes."); if (memeqzero(new_hw_addr->bytes + INFINIBAND_ALEN - 8, 8)) { - if (warn_invalid) + if (is_static) log_link_warning(&link, "The last 8 bytes of the InfiniBand MAC address cannot be null, refusing."); return -EINVAL; } @@ -183,7 +180,7 @@ int net_verify_hardware_address( break; default: - if (warn_invalid) + if (is_static) log_link_warning(&link, "Unsupported interface type %s%u to set MAC address, refusing.", strna(arphrd_to_name(iftype)), iftype); return -EINVAL; diff --git a/src/shared/netif-util.h b/src/shared/netif-util.h index 09776811538..c26dab63cf3 100644 --- a/src/shared/netif-util.h +++ b/src/shared/netif-util.h @@ -15,7 +15,7 @@ int net_get_unique_predictable_data(sd_device *device, bool use_sysname, uint64_ int net_get_unique_predictable_data_from_name(const char *name, const sd_id128_t *key, uint64_t *ret); int net_verify_hardware_address( const char *ifname, - bool warn_invalid, + bool is_static, uint16_t iftype, const struct hw_addr_data *ib_hw_addr, struct hw_addr_data *new_hw_addr); diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index c248024ca4c..880de0baf59 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -542,7 +542,7 @@ static bool hw_addr_is_valid(Link *link, const struct hw_addr_data *hw_addr) { static int link_generate_new_hw_addr(Link *link, struct hw_addr_data *ret) { struct hw_addr_data hw_addr = HW_ADDR_NULL; - bool warn_invalid = false; + bool is_static = false; uint8_t *p; size_t len; int r; @@ -558,7 +558,7 @@ static int link_generate_new_hw_addr(Link *link, struct hw_addr_data *ret) { if (link->config->mac_address_policy == MAC_ADDRESS_POLICY_NONE) { log_link_debug(link, "Using static MAC address."); hw_addr = link->config->hw_addr; - warn_invalid = true; + is_static = true; goto finalize; } @@ -634,7 +634,7 @@ static int link_generate_new_hw_addr(Link *link, struct hw_addr_data *ret) { finalize: - r = net_verify_hardware_address(link->ifname, warn_invalid, link->iftype, &link->hw_addr, &hw_addr); + r = net_verify_hardware_address(link->ifname, is_static, link->iftype, &link->hw_addr, &hw_addr); if (r < 0) return r; diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 0e0e6987d74..99eb89e3ee0 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1082,8 +1082,7 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): output = check_output('ip link show dropin-test') print(output) - # 00:50:56:c0:00:28 was requested, and the local bit is set by networkd. - self.assertRegex(output, '02:50:56:c0:00:28') + self.assertRegex(output, '00:50:56:c0:00:28') def test_match_udev_property(self): copy_unit_to_networkd_unit_path('12-dummy.netdev', '13-not-match-udev-property.network', '14-match-udev-property.network') @@ -2771,8 +2770,7 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): output = check_output('ip link show dummy98') print(output) - # 00:01:02:aa:bb:cc was requested, and the local bit is set by networkd. - self.assertRegex(output, '02:01:02:aa:bb:cc') + self.assertRegex(output, '00:01:02:aa:bb:cc') def test_ip_link_unmanaged(self): copy_unit_to_networkd_unit_path('25-link-section-unmanaged.network', '12-dummy.netdev')