network/netdev: do not update MAC address if netdev is already running

Follow-up for 17c5337f7b2993619d84acc2088b2ba1789e6477.

Older kernels (older than v6.5) refuse RTM_NEWLINK messages with IFLA_ADDRESS
attribute when the netdev already exists and is running, even if the MAC
address is unchanged.

So, let's not set IFLA_ADDRESS or IFLA_MTU if they are unchanged, and
set the attributes only when we can update them.

diff --git a/src/network/netdev/bridge.c b/src/network/netdev/bridge.c
index da5b33227700688a983605848ebb939342ed0493..d3ba4989d942f12f516fc8edce762bb3975d9e75 100644 (file)
--- a/src/network/netdev/bridge.c
+++ b/src/network/netdev/bridge.c
@@ -281,12 +281,17 @@ static void bridge_init(NetDev *netdev) {
         b->ageing_time = USEC_INFINITY;
 }
 
+static bool bridge_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        return true;
+}
+
 const NetDevVTable bridge_vtable = {
         .object_size = sizeof(Bridge),
         .init = bridge_init,
         .sections = NETDEV_COMMON_SECTIONS "Bridge\0",
         .post_create = netdev_bridge_post_create,
         .create_type = NETDEV_CREATE_INDEPENDENT,
+        .can_set_mac = bridge_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };

diff --git a/src/network/netdev/dummy.c b/src/network/netdev/dummy.c
index 00df1d2787371bc8fc29c17f2cc09abba2b97e66..8b2893d5b4db60dd58a76a94a034e553aded5c37 100644 (file)
--- a/src/network/netdev/dummy.c
+++ b/src/network/netdev/dummy.c
@@ -4,10 +4,15 @@
 
 #include "dummy.h"
 
+static bool dummy_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        return true;
+}
+
 const NetDevVTable dummy_vtable = {
         .object_size = sizeof(Dummy),
         .sections = NETDEV_COMMON_SECTIONS,
         .create_type = NETDEV_CREATE_INDEPENDENT,
+        .can_set_mac = dummy_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };

diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c
index 493edcc42a58edf429eb9de4c6a1d2b6f6c741a4..1d68be9bc88073891f68e787e2e18d388d14fbb8 100644 (file)
--- a/src/network/netdev/geneve.c
+++ b/src/network/netdev/geneve.c
@@ -254,6 +254,10 @@ static int netdev_geneve_verify(NetDev *netdev, const char *filename) {
         return 0;
 }
 
+static bool geneve_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        return true;
+}
+
 static void geneve_init(NetDev *netdev) {
         Geneve *v = GENEVE(netdev);
 
@@ -272,6 +276,7 @@ const NetDevVTable geneve_vtable = {
         .fill_message_create = netdev_geneve_fill_message_create,
         .create_type = NETDEV_CREATE_INDEPENDENT,
         .config_verify = netdev_geneve_verify,
+        .can_set_mac = geneve_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };

diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c
index 6c8ad1b3e9d05179344716ea09fd2d9874f8f8d4..805ec095715a4ac5ca8bfb43c6a54c86de96c9d0 100644 (file)
--- a/src/network/netdev/netdev.c
+++ b/src/network/netdev/netdev.c
@@ -636,15 +636,32 @@ finalize:
 
 static bool netdev_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
         assert(netdev);
+        assert(netdev->manager);
         assert(hw_addr);
 
         if (hw_addr->length <= 0)
                 return false;
 
-        if (!NETDEV_VTABLE(netdev)->can_set_mac)
-                return true;
-
-        return NETDEV_VTABLE(netdev)->can_set_mac(netdev, hw_addr);
+        Link *link;
+        if (link_get_by_index(netdev->manager, netdev->ifindex, &link) < 0)
+                return true; /* The netdev does not exist yet. We can set MAC address. */
+
+        if (hw_addr_equal(&link->hw_addr, hw_addr))
+                return false; /* Unchanged, not necessary to set. */
+
+        /* Soem netdevs refuse to update MAC address even if the interface is not running, e.g. ipvlan.
+         * Some other netdevs have the IFF_LIVE_ADDR_CHANGE flag and can update update MAC address even if
+         * the interface is running, e.g. dummy. For those cases, use custom checkers. */
+        if (NETDEV_VTABLE(netdev)->can_set_mac)
+                return NETDEV_VTABLE(netdev)->can_set_mac(netdev, hw_addr);
+
+        /* Before ad72c4a06acc6762e84994ac2f722da7a07df34e and 0ec92a8f56ff07237dbe8af7c7a72aba7f957baf
+         * (both in v6.5), the kernel refuse to set MAC address for existing netdevs even if it is unchanged.
+         * So, by default, do not update MAC address if the it is running. See eth_prepare_mac_addr_change(),
+         * which is called by eth_mac_addr(). Note, the result of netif_running() is mapped to operstate
+         * and flags. See rtnl_fill_ifinfo() and dev_get_flags(). */
+        return link->kernel_operstate == IF_OPER_DOWN &&
+                (link->flags & (IFF_RUNNING | IFF_LOWER_UP | IFF_DORMANT)) == 0;
 }
 
 static bool netdev_can_set_mtu(NetDev *netdev, uint32_t mtu) {
@@ -653,10 +670,22 @@ static bool netdev_can_set_mtu(NetDev *netdev, uint32_t mtu) {
         if (mtu <= 0)
                 return false;
 
-        if (!NETDEV_VTABLE(netdev)->can_set_mtu)
-                return true;
+        Link *link;
+        if (link_get_by_index(netdev->manager, netdev->ifindex, &link) < 0)
+                return true; /* The netdev does not exist yet. We can set MTU. */
+
+        if (mtu < link->min_mtu || link->max_mtu < mtu)
+                return false; /* The MTU is out of range. */
+
+        if (link->mtu == mtu)
+                return false; /* Unchanged, not necessary to set. */
 
-        return NETDEV_VTABLE(netdev)->can_set_mtu(netdev, mtu);
+        /* Some netdevs cannot change MTU, e.g. vxlan. Let's use the custom checkers in such cases. */
+        if (NETDEV_VTABLE(netdev)->can_set_mtu)
+                return NETDEV_VTABLE(netdev)->can_set_mtu(netdev, mtu);
+
+        /* By default, allow to update the MTU. */
+        return true;
 }
 
 static int netdev_create_message(NetDev *netdev, Link *link, sd_netlink_message *m) {

diff --git a/src/network/netdev/netdevsim.c b/src/network/netdev/netdevsim.c
index 15d5c132f96719534a1da83a3390ea4b65a0cecd..59958c3bbef4b902570a6c29f90c1ec3a3b4e1cf 100644 (file)
--- a/src/network/netdev/netdevsim.c
+++ b/src/network/netdev/netdevsim.c
@@ -4,10 +4,15 @@
 
 #include "netdevsim.h"
 
+static bool netdevsim_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        return true;
+}
+
 const NetDevVTable netdevsim_vtable = {
         .object_size = sizeof(NetDevSim),
         .sections = NETDEV_COMMON_SECTIONS,
         .create_type = NETDEV_CREATE_INDEPENDENT,
+        .can_set_mac = netdevsim_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };

diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c
index 0339c49c8fac36daa993cc3dd90fd4756f011738..af05cfda81a2208f5bb20760e5dabc0017c9fac6 100644 (file)
--- a/src/network/netdev/tunnel.c
+++ b/src/network/netdev/tunnel.c
@@ -1119,6 +1119,11 @@ static void netdev_tunnel_init(NetDev *netdev) {
                 t->ttl = DEFAULT_IPV6_TTL;
 }
 
+static bool tunnel_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        assert(IN_SET(netdev->kind, NETDEV_KIND_GRETAP, NETDEV_KIND_IP6GRETAP, NETDEV_KIND_ERSPAN));
+        return true;
+}
+
 const NetDevVTable ipip_vtable = {
         .object_size = sizeof(Tunnel),
         .init = netdev_tunnel_init,
@@ -1188,6 +1193,7 @@ const NetDevVTable gretap_vtable = {
         .is_ready_to_create = netdev_tunnel_is_ready_to_create,
         .config_verify = netdev_tunnel_verify,
         .needs_reconfigure = tunnel_needs_reconfigure,
+        .can_set_mac = tunnel_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };
@@ -1213,6 +1219,7 @@ const NetDevVTable ip6gretap_vtable = {
         .is_ready_to_create = netdev_tunnel_is_ready_to_create,
         .config_verify = netdev_tunnel_verify,
         .needs_reconfigure = tunnel_needs_reconfigure,
+        .can_set_mac = tunnel_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };
@@ -1238,6 +1245,7 @@ const NetDevVTable erspan_vtable = {
         .is_ready_to_create = netdev_tunnel_is_ready_to_create,
         .config_verify = netdev_tunnel_verify,
         .needs_reconfigure = tunnel_needs_reconfigure,
+        .can_set_mac = tunnel_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };

diff --git a/src/network/netdev/vrf.c b/src/network/netdev/vrf.c
index 24079a7203c89063a9c2c4bd3a501d2b57c40873..9e20bd22b28ba2eafecac7170ce01da97df7dfdf 100644 (file)
--- a/src/network/netdev/vrf.c
+++ b/src/network/netdev/vrf.c
@@ -21,11 +21,16 @@ static int netdev_vrf_fill_message_create(NetDev *netdev, Link *link, sd_netlink
         return 0;
 }
 
+static bool vrf_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        return true;
+}
+
 const NetDevVTable vrf_vtable = {
         .object_size = sizeof(Vrf),
         .sections = NETDEV_COMMON_SECTIONS "VRF\0",
         .fill_message_create = netdev_vrf_fill_message_create,
         .create_type = NETDEV_CREATE_INDEPENDENT,
+        .can_set_mac = vrf_can_set_mac,
         .iftype = ARPHRD_ETHER,
         .generate_mac = true,
 };

diff --git a/src/network/netdev/vxlan.c b/src/network/netdev/vxlan.c
index 9f22794d34880077911203aa03ffe0542119d95a..d8a066370d7d10c777716d81b451d0a134505e0b 100644 (file)
--- a/src/network/netdev/vxlan.c
+++ b/src/network/netdev/vxlan.c
@@ -197,6 +197,10 @@ static int netdev_vxlan_fill_message_create(NetDev *netdev, Link *link, sd_netli
         return 0;
 }
 
+static bool vxlan_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) {
+        return true;
+}
+
 static bool vxlan_can_set_mtu(NetDev *netdev, uint32_t mtu) {
         assert(netdev);
 
@@ -452,6 +456,7 @@ const NetDevVTable vxlan_vtable = {
         .create_type = NETDEV_CREATE_STACKED,
         .is_ready_to_create = netdev_vxlan_is_ready_to_create,
         .config_verify = netdev_vxlan_verify,
+        .can_set_mac = vxlan_can_set_mac,
         .can_set_mtu = vxlan_can_set_mtu,
         .needs_reconfigure = vxlan_needs_reconfigure,
         .iftype = ARPHRD_ETHER,