Merge pull request #21277 from yuwata/permanent-hw-addr

network, udev: several improvement of permanent hardware address handling

src/libsystemd/sd-netlink/netlink-util.c

@@ -355,8 +355,16 @@ int rtnl_resolve_interface_or_warn(sd_netlink **rtnl, const char *name) {
         return r;
 }
 
-int rtnl_get_link_info(sd_netlink **rtnl, int ifindex, unsigned short *ret_iftype, unsigned *ret_flags) {
+int rtnl_get_link_info(
+                sd_netlink **rtnl,
+                int ifindex,
+                unsigned short *ret_iftype,
+                unsigned *ret_flags,
+                struct hw_addr_data *ret_hw_addr,
+                struct hw_addr_data *ret_permanent_hw_addr) {
+
         _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL;
+        struct hw_addr_data addr = HW_ADDR_NULL, perm_addr = HW_ADDR_NULL;
         unsigned short iftype;
         unsigned flags;
         int r;
@@ -395,10 +403,26 @@ int rtnl_get_link_info(sd_netlink **rtnl, int ifindex, unsigned short *ret_iftyp
                         return r;
         }
 
+        if (ret_hw_addr) {
+                r = netlink_message_read_hw_addr(reply, IFLA_ADDRESS, &addr);
+                if (r < 0 && r != -ENODATA)
+                        return r;
+        }
+
+        if (ret_permanent_hw_addr) {
+                r = netlink_message_read_hw_addr(reply, IFLA_PERM_ADDRESS, &perm_addr);
+                if (r < 0 && r != -ENODATA)
+                        return r;
+        }
+
         if (ret_iftype)
                 *ret_iftype = iftype;
         if (ret_flags)
                 *ret_flags = flags;
+        if (ret_hw_addr)
+                *ret_hw_addr = addr;
+        if (ret_permanent_hw_addr)
+                *ret_permanent_hw_addr = perm_addr;
         return 0;
 }
 

src/libsystemd/sd-netlink/netlink-util.h

@@ -91,7 +91,13 @@ int rtnl_resolve_link_alternative_name(sd_netlink **rtnl, const char *name, char
 int rtnl_resolve_ifname(sd_netlink **rtnl, const char *name);
 int rtnl_resolve_interface(sd_netlink **rtnl, const char *name);
 int rtnl_resolve_interface_or_warn(sd_netlink **rtnl, const char *name);
-int rtnl_get_link_info(sd_netlink **rtnl, int ifindex, unsigned short *ret_iftype, unsigned *ret_flags);
+int rtnl_get_link_info(
+                sd_netlink **rtnl,
+                int ifindex,
+                unsigned short *ret_iftype,
+                unsigned *ret_flags,
+                struct hw_addr_data *ret_hw_addr,
+                struct hw_addr_data *ret_permanent_hw_addr);
 
 int rtnl_log_parse_error(int r);
 int rtnl_log_create_error(int r);

src/network/networkctl.c

@@ -273,7 +273,7 @@ typedef struct LinkInfo {
         int ifindex;
         unsigned short iftype;
         struct hw_addr_data hw_address;
-        struct ether_addr permanent_mac_address;
+        struct hw_addr_data permanent_hw_address;
         uint32_t master;
         uint32_t mtu;
         uint32_t min_mtu;
@@ -347,8 +347,8 @@ typedef struct LinkInfo {
         char *ssid;
         struct ether_addr bssid;
 
-        bool has_mac_address:1;
-        bool has_permanent_mac_address:1;
+        bool has_hw_address:1;
+        bool has_permanent_hw_address:1;
         bool has_tx_queues:1;
         bool has_rx_queues:1;
         bool has_stats64:1;
@@ -552,15 +552,15 @@ static int decode_link(sd_netlink_message *m, LinkInfo *info, char **patterns, b
         info->ifindex = ifindex;
         info->alternative_names = TAKE_PTR(altnames);
 
-        info->has_mac_address =
+        info->has_hw_address =
                 netlink_message_read_hw_addr(m, IFLA_ADDRESS, &info->hw_address) >= 0 &&
                 info->hw_address.length > 0;
 
-        info->has_permanent_mac_address =
-                ethtool_get_permanent_macaddr(NULL, info->name, &info->permanent_mac_address) >= 0 &&
-                !ether_addr_is_null(&info->permanent_mac_address) &&
-                (info->hw_address.length != sizeof(struct ether_addr) ||
-                 memcmp(&info->permanent_mac_address, info->hw_address.bytes, sizeof(struct ether_addr)) != 0);
+        info->has_permanent_hw_address =
+                (netlink_message_read_hw_addr(m, IFLA_PERM_ADDRESS, &info->permanent_hw_address) >= 0 ||
+                 ethtool_get_permanent_hw_addr(NULL, info->name, &info->permanent_hw_address) >= 0) &&
+                !hw_addr_is_null(&info->permanent_hw_address) &&
+                !hw_addr_equal(&info->permanent_hw_address, &info->hw_address);
 
         (void) sd_netlink_message_read_u32(m, IFLA_MTU, &info->mtu);
         (void) sd_netlink_message_read_u32(m, IFLA_MIN_MTU, &info->min_mtu);
@@ -1457,6 +1457,34 @@ static int dump_statistics(Table *table, const LinkInfo *info) {
         return 0;
 }
 
+static int dump_hw_address(Table *table, sd_hwdb *hwdb, const char *field, const struct hw_addr_data *addr) {
+        _cleanup_free_ char *description = NULL;
+        int r;
+
+        assert(table);
+        assert(field);
+        assert(addr);
+
+        if (addr->length == ETH_ALEN)
+                (void) ieee_oui(hwdb, &addr->ether, &description);
+
+        r = table_add_many(table,
+                           TABLE_EMPTY,
+                           TABLE_STRING, field);
+        if (r < 0)
+                return table_log_add_error(r);
+
+        r = table_add_cell_stringf(table, NULL, "%s%s%s%s",
+                                   HW_ADDR_TO_STR(addr),
+                                   description ? " (" : "",
+                                   strempty(description),
+                                   description ? ")" : "");
+        if (r < 0)
+                return table_log_add_error(r);
+
+        return 0;
+}
+
 static OutputFlags get_output_flags(void) {
         return
                 arg_all * OUTPUT_SHOW_ALL |
@@ -1676,43 +1704,16 @@ static int link_status_one(
                         return table_log_add_error(r);
         }
 
-        if (info->has_mac_address) {
-                _cleanup_free_ char *description = NULL;
-
-                if (info->hw_address.length == ETH_ALEN)
-                        (void) ieee_oui(hwdb, &info->hw_address.ether, &description);
-
-                r = table_add_many(table,
-                                   TABLE_EMPTY,
-                                   TABLE_STRING, "HW Address:");
+        if (info->has_hw_address) {
+                r = dump_hw_address(table, hwdb, "Hardware Address:", &info->hw_address);
                 if (r < 0)
-                        return table_log_add_error(r);
-                r = table_add_cell_stringf(table, NULL, "%s%s%s%s",
-                                           HW_ADDR_TO_STR(&info->hw_address),
-                                           description ? " (" : "",
-                                           strempty(description),
-                                           description ? ")" : "");
-                if (r < 0)
-                        return table_log_add_error(r);
+                        return r;
         }
 
-        if (info->has_permanent_mac_address) {
-                _cleanup_free_ char *description = NULL;
-
-                (void) ieee_oui(hwdb, &info->permanent_mac_address, &description);
-
-                r = table_add_many(table,
-                                   TABLE_EMPTY,
-                                   TABLE_STRING, "HW Permanent Address:");
+        if (info->has_permanent_hw_address) {
+                r = dump_hw_address(table, hwdb, "Permanent Hardware Address:", &info->permanent_hw_address);
                 if (r < 0)
-                        return table_log_add_error(r);
-                r = table_add_cell_stringf(table, NULL, "%s%s%s%s",
-                                           ETHER_ADDR_TO_STR(&info->permanent_mac_address),
-                                           description ? " (" : "",
-                                           strempty(description),
-                                           description ? ")" : "");
-                if (r < 0)
-                        return table_log_add_error(r);
+                        return r;
         }
 
         if (info->mtu > 0) {

src/network/networkd-link.c

@@ -10,6 +10,7 @@
 #include <unistd.h>
 
 #include "alloc-util.h"
+#include "arphrd-util.h"
 #include "batadv.h"
 #include "bond.h"
 #include "bridge.h"
@@ -1196,8 +1197,8 @@ static int link_get_network(Link *link, Network **ret) {
                 r = net_match_config(
                                 &network->match,
                                 link->sd_device,
-                                &link->hw_addr.ether,
-                                &link->permanent_mac,
+                                link->hw_addr.length == ETH_ALEN ? &link->hw_addr.ether : NULL,
+                                link->permanent_hw_addr.length == ETH_ALEN ? &link->permanent_hw_addr.ether : NULL,
                                 link->driver,
                                 link->iftype,
                                 link->ifname,
@@ -2400,15 +2401,28 @@ static int link_new(Manager *manager, sd_netlink_message *message, Link **ret) {
         if (r < 0)
                 return log_link_debug_errno(link, r, "Failed to manage link by its interface name: %m");
 
-        r = ethtool_get_permanent_macaddr(&manager->ethtool_fd, link->ifname, &link->permanent_mac);
-        if (r < 0)
-                log_link_debug_errno(link, r, "Permanent MAC address not found for new device, continuing without: %m");
+        log_link_debug(link, "Saved new link: ifindex=%i, iftype=%s(%u), kind=%s",
+                       link->ifindex, strna(arphrd_to_name(link->iftype)), link->iftype, strna(link->kind));
+
+        r = netlink_message_read_hw_addr(message, IFLA_PERM_ADDRESS, &link->permanent_hw_addr);
+        if (r < 0) {
+                if (r != -ENODATA)
+                        log_link_debug_errno(link, r, "Failed to read IFLA_PERM_ADDRESS attribute, ignoring: %m");
+
+                if (netlink_message_read_hw_addr(message, IFLA_ADDRESS, NULL) >= 0) {
+                        /* Fallback to ethtool, if the link has a hardware address. */
+                        r = ethtool_get_permanent_hw_addr(&manager->ethtool_fd, link->ifname, &link->permanent_hw_addr);
+                        if (r < 0)
+                                log_link_debug_errno(link, r, "Permanent hardware address not found, continuing without: %m");
+                }
+        }
+        if (link->permanent_hw_addr.length > 0)
+                log_link_debug(link, "Saved permanent hardware address: %s", HW_ADDR_TO_STR(&link->permanent_hw_addr));
 
         r = ethtool_get_driver(&manager->ethtool_fd, link->ifname, &link->driver);
         if (r < 0)
                 log_link_debug_errno(link, r, "Failed to get driver, continuing without: %m");
 
-        log_link_debug(link, "Link %d added", link->ifindex);
         *ret = TAKE_PTR(link);
         return 0;
 }

src/network/networkd-link.h

@@ -55,7 +55,7 @@ typedef struct Link {
         char *state_file;
         struct hw_addr_data hw_addr;
         struct hw_addr_data bcast_addr;
-        struct ether_addr permanent_mac;
+        struct hw_addr_data permanent_hw_addr;
         struct in6_addr ipv6ll_address;
         uint32_t mtu;
         uint32_t min_mtu;

src/shared/ethtool-util.c

@@ -339,14 +339,14 @@ int ethtool_get_link_info(
         return 0;
 }
 
-int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct ether_addr *ret) {
+int ethtool_get_permanent_hw_addr(int *ethtool_fd, const char *ifname, struct hw_addr_data *ret) {
         _cleanup_close_ int fd = -1;
         struct {
                 struct ethtool_perm_addr addr;
-                uint8_t space[MAX_ADDR_LEN];
+                uint8_t space[HW_ADDR_MAX_SIZE];
         } epaddr = {
                 .addr.cmd = ETHTOOL_GPERMADDR,
-                .addr.size = MAX_ADDR_LEN,
+                .addr.size = HW_ADDR_MAX_SIZE,
         };
         struct ifreq ifr = {
                 .ifr_data = (caddr_t) &epaddr,
@@ -367,17 +367,14 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et
         if (ioctl(*ethtool_fd, SIOCETHTOOL, &ifr) < 0)
                 return -errno;
 
-        if (epaddr.addr.size != 6)
-                return -EOPNOTSUPP;
+        if (epaddr.addr.size == 0)
+                return -ENODATA;
 
-#pragma GCC diagnostic push
-#if HAVE_ZERO_LENGTH_BOUNDS
-#  pragma GCC diagnostic ignored "-Wzero-length-bounds"
-#endif
-        for (size_t i = 0; i < epaddr.addr.size; i++)
-                ret->ether_addr_octet[i] = epaddr.addr.data[i];
-#pragma GCC diagnostic pop
+        if (epaddr.addr.size > HW_ADDR_MAX_SIZE)
+                return -EINVAL;
 
+        ret->length = epaddr.addr.size;
+        memcpy(ret->bytes, epaddr.addr.data, epaddr.addr.size);
         return 0;
 }
 

src/shared/ethtool-util.h

@@ -6,6 +6,7 @@
 #include <linux/ethtool.h>
 
 #include "conf-parser.h"
+#include "ether-addr-util.h"
 
 #define N_ADVERTISE 3
 
@@ -163,7 +164,7 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret);
 int ethtool_get_link_info(int *ethtool_fd, const char *ifname,
                           int *ret_autonegotiation, uint64_t *ret_speed,
                           Duplex *ret_duplex, NetDevPort *ret_port);
-int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct ether_addr *ret);
+int ethtool_get_permanent_hw_addr(int *ethtool_fd, const char *ifname, struct hw_addr_data *ret);
 int ethtool_set_wol(int *ethtool_fd, const char *ifname, uint32_t wolopts, const uint8_t password[SOPASS_MAX]);
 int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netdev_ring_param *ring);
 int ethtool_set_features(int *ethtool_fd, const char *ifname, const int features[static _NET_DEV_FEAT_MAX]);

src/shared/net-condition.c

@@ -143,16 +143,11 @@ int net_match_config(
                 return r;
 
         if (device) {
-                const char *mac_str;
-
                 (void) sd_device_get_property_value(device, "ID_PATH", &path);
                 if (!driver)
                         (void) sd_device_get_property_value(device, "ID_NET_DRIVER", &driver);
                 if (!ifname)
                         (void) sd_device_get_sysname(device, &ifname);
-                if (!mac &&
-                    sd_device_get_sysattr_value(device, "address", &mac_str) >= 0)
-                        mac = ether_aton(mac_str);
         }
 
         if (match->mac && (!mac || !set_contains(match->mac, mac)))

src/udev/net/link-config.c

@@ -349,7 +349,7 @@ bool link_config_should_reload(LinkConfigContext *ctx) {
 
 int link_config_get(LinkConfigContext *ctx, sd_netlink **rtnl, sd_device *device, LinkConfig **ret) {
         unsigned name_assign_type = NET_NAME_UNKNOWN;
-        struct ether_addr permanent_mac = {};
+        struct hw_addr_data hw_addr, permanent_hw_addr;
         unsigned short iftype;
         LinkConfig *link;
         const char *name;
@@ -369,7 +369,7 @@ int link_config_get(LinkConfigContext *ctx, sd_netlink **rtnl, sd_device *device
         if (r < 0)
                 return r;
 
-        r = rtnl_get_link_info(rtnl, ifindex, &iftype, &flags);
+        r = rtnl_get_link_info(rtnl, ifindex, &iftype, &flags, &hw_addr, &permanent_hw_addr);
         if (r < 0)
                 return r;
 
@@ -377,14 +377,19 @@ int link_config_get(LinkConfigContext *ctx, sd_netlink **rtnl, sd_device *device
         if (flags & IFF_LOOPBACK)
                 return -ENOENT;
 
-        r = ethtool_get_permanent_macaddr(&ctx->ethtool_fd, name, &permanent_mac);
-        if (r < 0)
-                log_device_debug_errno(device, r, "Failed to get permanent MAC address, ignoring: %m");
+        if (hw_addr.length > 0 && permanent_hw_addr.length == 0) {
+                r = ethtool_get_permanent_hw_addr(&ctx->ethtool_fd, name, &permanent_hw_addr);
+                if (r < 0)
+                        log_device_debug_errno(device, r, "Failed to get permanent hardware address, ignoring: %m");
+        }
 
         (void) link_unsigned_attribute(device, "name_assign_type", &name_assign_type);
 
         LIST_FOREACH(links, link, ctx->links) {
-                r = net_match_config(&link->match, device, NULL, &permanent_mac, NULL, iftype, NULL, NULL, 0, NULL, NULL);
+                r = net_match_config(&link->match, device,
+                                     hw_addr.length == ETH_ALEN ? &hw_addr.ether : NULL,
+                                     permanent_hw_addr.length == ETH_ALEN ? &permanent_hw_addr.ether : NULL,
+                                     NULL, iftype, NULL, NULL, 0, NULL, NULL);
                 if (r < 0)
                         return r;
                 if (r == 0)

src/udev/udev-event.c

@@ -855,7 +855,7 @@ static int rename_netif(UdevEvent *event) {
                 return 0;
         }
 
-        r = rtnl_get_link_info(&event->rtnl, ifindex, NULL, &flags);
+        r = rtnl_get_link_info(&event->rtnl, ifindex, NULL, &flags, NULL, NULL);
         if (r < 0)
                 return log_device_warning_errno(dev, r, "Failed to get link flags: %m");