From 10623654f9e096da1c0c9abdc5dece651bb671be Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 30 May 2019 07:54:11 +0200 Subject: [PATCH] platform: handle IFLA_BROADCAST in platform cache for links While at it, rename the "addr" field to "l_address". The term "addr" is used over and over. Instead we should use distinct names that make it easier to navigate the code. --- src/devices/nm-device.c | 12 ++-- src/devices/tests/test-lldp.c | 2 +- src/platform/nm-fake-platform.c | 12 ++-- src/platform/nm-linux-platform.c | 44 +++++++++---- src/platform/nm-platform.c | 108 ++++++++++++++++++++++--------- src/platform/nm-platform.h | 17 +++-- src/platform/tests/test-link.c | 8 +-- 7 files changed, 136 insertions(+), 67 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 84ed214e45..1000342824 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1913,7 +1913,7 @@ get_ip_iface_identifier (NMDevice *self, NMUtilsIPv6IfaceId *out_iid) const NMPlatformLink *pllink; const guint8 *hwaddr; guint8 pseudo_hwaddr[ETH_ALEN]; - guint hwaddr_len; + gsize hwaddr_len; int ifindex; gboolean success; @@ -1926,13 +1926,9 @@ get_ip_iface_identifier (NMDevice *self, NMUtilsIPv6IfaceId *out_iid) || NM_IN_SET (pllink->type, NM_LINK_TYPE_NONE, NM_LINK_TYPE_UNKNOWN)) return FALSE; - if (pllink->addr.len <= 0) + hwaddr = nmp_link_address_get (&pllink->l_address, &hwaddr_len); + if (hwaddr_len <= 0) return FALSE; - if (pllink->addr.len > NM_UTILS_HWADDR_LEN_MAX) - g_return_val_if_reached (FALSE); - - hwaddr = pllink->addr.data; - hwaddr_len = pllink->addr.len; if (pllink->type == NM_LINK_TYPE_6LOWPAN) { /* If the underlying IEEE 802.15.4 device has a short address we generate @@ -1964,7 +1960,7 @@ get_ip_iface_identifier (NMDevice *self, NMUtilsIPv6IfaceId *out_iid) out_iid); if (!success) { _LOGW (LOGD_PLATFORM, "failed to generate interface identifier " - "for link type %u hwaddr_len %u", pllink->type, (unsigned) pllink->addr.len); + "for link type %u hwaddr_len %zu", pllink->type, hwaddr_len); } return success; } diff --git a/src/devices/tests/test-lldp.c b/src/devices/tests/test-lldp.c index 7b135f5886..4cf0773462 100644 --- a/src/devices/tests/test-lldp.c +++ b/src/devices/tests/test-lldp.c @@ -457,7 +457,7 @@ _test_recv_fixture_setup (TestRecvFixture *fixture, gconstpointer user_data) fixture->ifindex = link->ifindex; fixture->fd = nm_steal_fd (&fd); - memcpy (fixture->mac, link->addr.data, ETH_ALEN); + memcpy (fixture->mac, link->l_address.data, ETH_ALEN); } typedef struct { diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 40a85390b6..ed538949e1 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -276,9 +276,9 @@ link_add_pre (NMPlatform *platform, o->_link.netlink.is_in_netlink = TRUE; if (address) { - g_assert (address_len > 0 && address_len <= sizeof (link->addr.data)); - memcpy (link->addr.data, address, address_len); - link->addr.len = address_len; + g_assert (address_len > 0 && address_len <= sizeof (link->l_address.data)); + memcpy (link->l_address.data, address, address_len); + link->l_address.len = address_len; } else g_assert (address_len == 0); @@ -584,9 +584,9 @@ link_set_address (NMPlatform *platform, int ifindex, gconstpointer addr, size_t return -NME_PL_EXISTS; obj_tmp = nmp_object_clone (device->obj, FALSE); - obj_tmp->link.addr.len = len; - memset (obj_tmp->link.addr.data, 0, sizeof (obj_tmp->link.addr.data)); - memcpy (obj_tmp->link.addr.data, addr, len); + obj_tmp->link.l_address.len = len; + memset (obj_tmp->link.l_address.data, 0, sizeof (obj_tmp->link.l_address.data)); + memcpy (obj_tmp->link.l_address.data, addr, len); link_set_obj (platform, device, obj_tmp); return 0; diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index ac8e869d8e..dbfc233f79 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -2587,6 +2587,25 @@ link_wireguard_change (NMPlatform *platform, /*****************************************************************************/ +static void +_nmp_link_address_set (NMPLinkAddress *dst, + const struct nlattr *nla) +{ + *dst = (NMPLinkAddress) { + .len = 0, + }; + if (nla) { + int l = nla_len (nla); + + if ( l > 0 + && l <= NM_UTILS_HWADDR_LEN_MAX) { + G_STATIC_ASSERT_EXPR (sizeof (dst->data) == NM_UTILS_HWADDR_LEN_MAX); + memcpy (dst->data, nla_data (nla), l); + dst->len = l; + } + } +} + /* Copied and heavily modified from libnl3's link_msg_parser(). */ static NMPObject * _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr *nlh, gboolean id_only) @@ -2630,6 +2649,7 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr const NMPObject *link_cached = NULL; const NMPObject *lnk_data = NULL; gboolean address_complete_from_cache = TRUE; + gboolean broadcast_complete_from_cache = TRUE; gboolean lnk_data_complete_from_cache = TRUE; gboolean need_ext_data = FALSE; gboolean af_inet6_token_valid = FALSE; @@ -2728,16 +2748,15 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr } if (tb[IFLA_ADDRESS]) { - int l = nla_len (tb[IFLA_ADDRESS]); - - if (l > 0 && l <= NM_UTILS_HWADDR_LEN_MAX) { - G_STATIC_ASSERT (NM_UTILS_HWADDR_LEN_MAX == sizeof (obj->link.addr.data)); - memcpy (obj->link.addr.data, nla_data (tb[IFLA_ADDRESS]), l); - obj->link.addr.len = l; - } + _nmp_link_address_set (&obj->link.l_address, tb[IFLA_ADDRESS]); address_complete_from_cache = FALSE; } + if (tb[IFLA_BROADCAST]) { + _nmp_link_address_set (&obj->link.l_broadcast, tb[IFLA_BROADCAST]); + broadcast_complete_from_cache = FALSE; + } + if (tb[IFLA_AF_SPEC]) { struct nlattr *af_attr; int remaining; @@ -2811,6 +2830,7 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr && ( lnk_data_complete_from_cache || need_ext_data || address_complete_from_cache + || broadcast_complete_from_cache || !af_inet6_token_valid || !af_inet6_addr_gen_mode_valid || !tb[IFLA_STATS64])) { @@ -2840,7 +2860,9 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr } if (address_complete_from_cache) - obj->link.addr = link_cached->link.addr; + obj->link.l_address = link_cached->link.l_address; + if (broadcast_complete_from_cache) + obj->link.l_broadcast = link_cached->link.l_broadcast; if (!af_inet6_token_valid) obj->link.inet6_token = link_cached->link.inet6_token; if (!af_inet6_addr_gen_mode_valid) @@ -5589,7 +5611,7 @@ cache_on_change (NMPlatform *platform, * Request it again. */ re_request_link = TRUE; } else if ( obj_new->link.type == NM_LINK_TYPE_ETHERNET - && obj_new->link.addr.len == 0) { + && obj_new->link.l_address.len == 0) { /* Due to a kernel bug, we sometimes receive spurious NEWLINK * messages after a wifi interface has disappeared. Since the * link is not present anymore we can't determine its type and @@ -6449,8 +6471,8 @@ retry: } else if ( NM_IN_SET (-((int) seq_result), ENFILE) && change_link_type == CHANGE_LINK_TYPE_SET_ADDRESS && (obj_cache = nmp_cache_lookup_link (nm_platform_get_cache (platform), ifindex)) - && obj_cache->link.addr.len == data->set_address.length - && memcmp (obj_cache->link.addr.data, data->set_address.address, data->set_address.length) == 0) { + && obj_cache->link.l_address.len == data->set_address.length + && memcmp (obj_cache->link.l_address.data, data->set_address.address, data->set_address.length) == 0) { /* workaround ENFILE which may be wrongly returned (bgo #770456). * If the MAC address is as expected, assume success? */ log_result = "success"; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 4a98df84e8..a4a0eb8d98 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -51,12 +51,58 @@ /*****************************************************************************/ -G_STATIC_ASSERT (sizeof ( ((NMPlatformLink *) NULL)->addr.data ) == NM_UTILS_HWADDR_LEN_MAX); G_STATIC_ASSERT (G_STRUCT_OFFSET (NMPlatformIPAddress, address_ptr) == G_STRUCT_OFFSET (NMPlatformIP4Address, address)); G_STATIC_ASSERT (G_STRUCT_OFFSET (NMPlatformIPAddress, address_ptr) == G_STRUCT_OFFSET (NMPlatformIP6Address, address)); G_STATIC_ASSERT (G_STRUCT_OFFSET (NMPlatformIPRoute, network_ptr) == G_STRUCT_OFFSET (NMPlatformIP4Route, network)); G_STATIC_ASSERT (G_STRUCT_OFFSET (NMPlatformIPRoute, network_ptr) == G_STRUCT_OFFSET (NMPlatformIP6Route, network)); +/*****************************************************************************/ + +G_STATIC_ASSERT (sizeof ( ((NMPLinkAddress *) NULL)->data ) == NM_UTILS_HWADDR_LEN_MAX); +G_STATIC_ASSERT (sizeof ( ((NMPlatformLink *) NULL)->l_address.data ) == NM_UTILS_HWADDR_LEN_MAX); +G_STATIC_ASSERT (sizeof ( ((NMPlatformLink *) NULL)->l_broadcast.data ) == NM_UTILS_HWADDR_LEN_MAX); + +static const char * +_nmp_link_address_to_string (const NMPLinkAddress *addr, + char buf[static (NM_UTILS_HWADDR_LEN_MAX * 3)]) +{ + nm_assert (addr); + + if (addr->len > 0) { + if (!nm_utils_hwaddr_ntoa_buf (addr->data, + addr->len, + TRUE, + buf, + NM_UTILS_HWADDR_LEN_MAX * 3)) { + buf[0] = '\0'; + g_return_val_if_reached (buf); + } + } else + buf[0] = '\0'; + + return buf; +} + +gconstpointer +nmp_link_address_get (const NMPLinkAddress *addr, size_t *length) +{ + if ( !addr + || addr->len <= 0) { + NM_SET_OUT (length, 0); + return NULL; + } + + if (addr->len > NM_UTILS_HWADDR_LEN_MAX) { + NM_SET_OUT (length, 0); + g_return_val_if_reached (NULL); + } + + NM_SET_OUT (length, addr->len); + return addr->data; +} + +/*****************************************************************************/ + #define _NMLOG_DOMAIN LOGD_PLATFORM #define _NMLOG_PREFIX_NAME "platform" @@ -941,14 +987,15 @@ nm_platform_link_get_by_ifname (NMPlatform *self, const char *ifname) } struct _nm_platform_link_get_by_address_data { - gconstpointer address; - guint8 length; + gconstpointer data; + guint8 len; }; static gboolean _nm_platform_link_get_by_address_match_link (const NMPObject *obj, struct _nm_platform_link_get_by_address_data *d) { - return obj->link.addr.len == d->length && !memcmp (obj->link.addr.data, d->address, d->length); + return obj->link.l_address.len == d->len + && !memcmp (obj->link.l_address.data, d->data, d->len); } /** @@ -968,8 +1015,8 @@ nm_platform_link_get_by_address (NMPlatform *self, { const NMPObject *obj; struct _nm_platform_link_get_by_address_data d = { - .address = address, - .length = length, + .data = address, + .len = length, }; _CHECK_SELF (self, klass, NULL); @@ -1507,19 +1554,7 @@ nm_platform_link_get_address (NMPlatform *self, int ifindex, size_t *length) const NMPlatformLink *pllink; pllink = nm_platform_link_get (self, ifindex); - if ( !pllink - || pllink->addr.len <= 0) { - NM_SET_OUT (length, 0); - return NULL; - } - - if (pllink->addr.len > NM_UTILS_HWADDR_LEN_MAX) { - NM_SET_OUT (length, 0); - g_return_val_if_reached (NULL); - } - - NM_SET_OUT (length, pllink->addr.len); - return pllink->addr.data; + return nmp_link_address_get (pllink ? &pllink->l_address : NULL, length); } /** @@ -2421,9 +2456,10 @@ nm_platform_link_6lowpan_get_properties (NMPlatform *self, int ifindex, int *out if (out_parent) { const NMPlatformLink *parent_plink; - parent_plink = nm_platform_link_get_by_address (self, NM_LINK_TYPE_WPAN, - plink->addr.data, - plink->addr.len); + parent_plink = nm_platform_link_get_by_address (self, + NM_LINK_TYPE_WPAN, + plink->l_address.data, + plink->l_address.len); NM_SET_OUT (out_parent, parent_plink ? parent_plink->ifindex : -1); } @@ -5356,7 +5392,8 @@ nm_platform_link_to_string (const NMPlatformLink *link, char *buf, gsize len) char parent[20]; GString *str_flags; char str_addrmode[30]; - gs_free char *str_addr = NULL; + char str_address[NM_UTILS_HWADDR_LEN_MAX * 3]; + char str_broadcast[NM_UTILS_HWADDR_LEN_MAX * 3]; char str_inet6_token[NM_UTILS_INET_ADDRSTRLEN]; const char *str_link_type; @@ -5392,8 +5429,8 @@ nm_platform_link_to_string (const NMPlatformLink *link, char *buf, gsize len) else parent[0] = 0; - if (link->addr.len) - str_addr = nm_utils_hwaddr_ntoa (link->addr.data, MIN (link->addr.len, sizeof (link->addr.data))); + _nmp_link_address_to_string (&link->l_address, str_address); + _nmp_link_address_to_string (&link->l_broadcast, str_broadcast); str_link_type = nm_link_type_to_string (link->type); @@ -5409,7 +5446,8 @@ nm_platform_link_to_string (const NMPlatformLink *link, char *buf, gsize len) "%s%s" /* kind */ "%s" /* is-in-udev */ "%s%s" /* addr-gen-mode */ - "%s%s" /* addr */ + "%s%s" /* l_address */ + "%s%s" /* l_broadcast */ "%s%s" /* inet6_token */ "%s%s" /* driver */ " rx:%"G_GUINT64_FORMAT",%"G_GUINT64_FORMAT @@ -5427,8 +5465,10 @@ nm_platform_link_to_string (const NMPlatformLink *link, char *buf, gsize len) link->initialized ? " init" : " not-init", link->inet6_addr_gen_mode_inv ? " addrgenmode " : "", link->inet6_addr_gen_mode_inv ? nm_platform_link_inet6_addrgenmode2str (_nm_platform_uint8_inv (link->inet6_addr_gen_mode_inv), str_addrmode, sizeof (str_addrmode)) : "", - str_addr ? " addr " : "", - str_addr ?: "", + str_address[0] ? " addr " : "", + str_address[0] ? str_address : "", + str_broadcast[0] ? " brd " : "", + str_broadcast[0] ? str_broadcast : "", link->inet6_token.id ? " inet6token " : "", link->inet6_token.id ? nm_utils_inet6_interface_identifier_to_token (link->inet6_token, str_inet6_token) : "", link->driver ? " driver " : "", @@ -6795,7 +6835,8 @@ nm_platform_link_hash_update (const NMPlatformLink *obj, NMHashState *h) nm_hash_update_str0 (h, obj->kind); nm_hash_update_str0 (h, obj->driver); /* nm_hash_update_mem() also hashes the length obj->addr.len */ - nm_hash_update_mem (h, obj->addr.data, obj->addr.len); + nm_hash_update_mem (h, obj->l_address.data, NM_MIN (obj->l_address.len, sizeof (obj->l_address.data))); + nm_hash_update_mem (h, obj->l_broadcast.data, NM_MIN (obj->l_broadcast.len, sizeof (obj->l_broadcast.data))); } int @@ -6812,12 +6853,15 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) NM_CMP_FIELD (a, b, mtu); NM_CMP_FIELD_BOOL (a, b, initialized); NM_CMP_FIELD (a, b, arptype); - NM_CMP_FIELD (a, b, addr.len); + NM_CMP_FIELD (a, b, l_address.len); + NM_CMP_FIELD (a, b, l_broadcast.len); NM_CMP_FIELD (a, b, inet6_addr_gen_mode_inv); NM_CMP_FIELD_STR_INTERNED (a, b, kind); NM_CMP_FIELD_STR_INTERNED (a, b, driver); - if (a->addr.len) - NM_CMP_FIELD_MEMCMP_LEN (a, b, addr.data, a->addr.len); + if (a->l_address.len) + NM_CMP_FIELD_MEMCMP_LEN (a, b, l_address.data, a->l_address.len); + if (a->l_broadcast.len) + NM_CMP_FIELD_MEMCMP_LEN (a, b, l_broadcast.data, a->l_broadcast.len); NM_CMP_FIELD_MEMCMP (a, b, inet6_token); NM_CMP_FIELD (a, b, rx_packets); NM_CMP_FIELD (a, b, rx_bytes); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 8c0a8764e2..972b009361 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -159,6 +159,13 @@ typedef enum { NM_PLATFORM_ROUTING_RULE_CMP_TYPE_FULL, } NMPlatformRoutingRuleCmpType; +typedef struct { + guint8 data[20 /* NM_UTILS_HWADDR_LEN_MAX */ ]; + guint8 len; +} NMPLinkAddress; + +gconstpointer nmp_link_address_get (const NMPLinkAddress *addr, size_t *length); + typedef enum { /* match-flags are strictly inclusive. That means, @@ -230,11 +237,11 @@ struct _NMPlatformLink { /* rtnl_link_get_arptype(), ifinfomsg.ifi_type. */ guint32 arptype; - /* rtnl_link_get_addr(), IFLA_ADDRESS */ - struct { - guint8 data[20]; /* NM_UTILS_HWADDR_LEN_MAX */ - guint8 len; - } addr; + /* IFLA_ADDRESS */ + NMPLinkAddress l_address; + + /* IFLA_BROADCAST */ + NMPLinkAddress l_broadcast; /* rtnl_link_inet6_get_token(), IFLA_INET6_TOKEN */ NMUtilsIPv6IfaceId inet6_token; diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index e4307c685d..3c322b9d16 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -515,8 +515,8 @@ test_bridge_addr (void) link = *plink; g_assert_cmpstr (link.name, ==, DEVICE_NAME); - g_assert_cmpint (link.addr.len, ==, sizeof (addr)); - g_assert (!memcmp (link.addr.data, addr, sizeof (addr))); + g_assert_cmpint (link.l_address.len, ==, sizeof (addr)); + g_assert (!memcmp (link.l_address.data, addr, sizeof (addr))); plink = nm_platform_link_get (NM_PLATFORM_GET, link.ifindex); g_assert (plink); @@ -538,8 +538,8 @@ test_bridge_addr (void) g_assert_cmpint (_nm_platform_uint8_inv (plink->inet6_addr_gen_mode_inv), ==, NM_IN6_ADDR_GEN_MODE_EUI64); } - g_assert_cmpint (plink->addr.len, ==, sizeof (addr)); - g_assert (!memcmp (plink->addr.data, addr, sizeof (addr))); + g_assert_cmpint (plink->l_address.len, ==, sizeof (addr)); + g_assert (!memcmp (plink->l_address.data, addr, sizeof (addr))); nmtstp_link_delete (NULL, -1, link.ifindex, link.name, TRUE); }