From 58f738fa3bf99accc9a85cc7e8146c42b3c221db Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 8 Jun 2020 15:18:27 +0200 Subject: [PATCH] lldp: use full chassis-id/port-id as ID for LLDP neighbor For the ID of LLDP neighbors follow what systemd does (w.r.t. what it consideres equality of two neighbors). Note that previously we did almost the same thing. Except, we compared priv->chassis_id and priv->port_id, but these values are string representations of the original (binary value). Don't use the pretty strings as ID but the original binary value. --- src/devices/nm-lldp-listener.c | 191 ++++++++++++++++++++------------- 1 file changed, 114 insertions(+), 77 deletions(-) diff --git a/src/devices/nm-lldp-listener.c b/src/devices/nm-lldp-listener.c index 855585cb05..77cf44d120 100644 --- a/src/devices/nm-lldp-listener.c +++ b/src/devices/nm-lldp-listener.c @@ -58,15 +58,11 @@ G_DEFINE_TYPE (NMLldpListener, nm_lldp_listener, G_TYPE_OBJECT) typedef struct { GVariant *variant; + sd_lldp_neighbor *neighbor_sd; char *chassis_id; char *port_id; - - sd_lldp_neighbor *neighbor_sd; - guint8 chassis_id_type; guint8 port_id_type; - - bool valid:1; } LldpNeighbor; /*****************************************************************************/ @@ -121,29 +117,96 @@ lldp_neighbor_get_raw (LldpNeighbor *neigh, *out_raw_len = raw_len; } +static gboolean +lldp_neighbor_id_get (struct sd_lldp_neighbor *neighbor_sd, + guint8 *out_chassis_id_type, + const guint8 **out_chassis_id, + gsize *out_chassis_id_len, + guint8 *out_port_id_type, + const guint8 **out_port_id, + gsize *out_port_id_len) +{ + int r; + + r = sd_lldp_neighbor_get_chassis_id (neighbor_sd, + out_chassis_id_type, + (gconstpointer *) out_chassis_id, + out_chassis_id_len); + if (r < 0) + return FALSE; + + r = sd_lldp_neighbor_get_port_id (neighbor_sd, + out_port_id_type, + (gconstpointer *) out_port_id, + out_port_id_len); + if (r < 0) + return FALSE; + + return TRUE; +} + static guint lldp_neighbor_id_hash (gconstpointer ptr) { const LldpNeighbor *neigh = ptr; + guint8 chassis_id_type; + guint8 port_id_type; + const guint8 *chassis_id; + const guint8 *port_id; + gsize chassis_id_len; + gsize port_id_len; NMHashState h; + if (!lldp_neighbor_id_get (neigh->neighbor_sd, &chassis_id_type, &chassis_id, &chassis_id_len, &port_id_type, &port_id, &port_id_len)) { + nm_assert_not_reached (); + return 0; + } + nm_hash_init (&h, 23423423u); - nm_hash_update_str0 (&h, neigh->chassis_id); - nm_hash_update_str0 (&h, neigh->port_id); nm_hash_update_vals (&h, - neigh->chassis_id_type, - neigh->port_id_type); + chassis_id_len, + port_id_len, + chassis_id_type, + port_id_type); + nm_hash_update (&h, chassis_id, chassis_id_len); + nm_hash_update (&h, port_id, port_id_len); return nm_hash_complete (&h); } static int -lldp_neighbor_id_cmp (const LldpNeighbor *x, const LldpNeighbor *y) +lldp_neighbor_id_cmp (const LldpNeighbor *a, const LldpNeighbor *b) { - NM_CMP_SELF (x, y); - NM_CMP_FIELD (x, y, chassis_id_type); - NM_CMP_FIELD (x, y, port_id_type); - NM_CMP_FIELD_STR0 (x, y, chassis_id); - NM_CMP_FIELD_STR0 (x, y, port_id); + guint8 a_chassis_id_type; + guint8 b_chassis_id_type; + guint8 a_port_id_type; + guint8 b_port_id_type; + const guint8 *a_chassis_id; + const guint8 *b_chassis_id; + const guint8 *a_port_id; + const guint8 *b_port_id; + gsize a_chassis_id_len; + gsize b_chassis_id_len; + gsize a_port_id_len; + gsize b_port_id_len; + + NM_CMP_SELF (a, b); + + if (!lldp_neighbor_id_get (a->neighbor_sd, &a_chassis_id_type, &a_chassis_id, &a_chassis_id_len, &a_port_id_type, &a_port_id, &a_port_id_len)) { + nm_assert_not_reached (); + return FALSE; + } + + if (!lldp_neighbor_id_get (b->neighbor_sd, &b_chassis_id_type, &b_chassis_id, &b_chassis_id_len, &b_port_id_type, &b_port_id, &b_port_id_len)) { + nm_assert_not_reached (); + return FALSE; + } + + NM_CMP_DIRECT (a_chassis_id_type, b_chassis_id_type); + NM_CMP_DIRECT (a_port_id_type, b_port_id_type); + NM_CMP_DIRECT (a_chassis_id_len, b_chassis_id_len); + NM_CMP_DIRECT (a_port_id_len, b_port_id_len); + NM_CMP_DIRECT_MEMCMP (a_chassis_id, b_chassis_id, a_chassis_id_len); + NM_CMP_DIRECT_MEMCMP (a_port_id, b_port_id, a_port_id_len); return 0; } @@ -186,25 +249,14 @@ lldp_neighbor_equal (LldpNeighbor *a, LldpNeighbor *b) const guint8 *raw_data_b; gsize raw_len_a; gsize raw_len_b; - gboolean equal; if (a->neighbor_sd == b->neighbor_sd) return TRUE; lldp_neighbor_get_raw (a, &raw_data_a, &raw_len_a); lldp_neighbor_get_raw (b, &raw_data_b, &raw_len_b); - - if (raw_len_a != raw_len_b) - return FALSE; - - equal = (memcmp (raw_data_a, raw_data_b, raw_len_a) == 0); - - nm_assert ( !equal - || ( a->chassis_id_type == b->chassis_id_type - && a->port_id_type == b->port_id_type - && nm_streq0 (a->chassis_id, b->chassis_id) - && nm_streq0 (a->port_id, b->port_id))); - return equal; + return raw_len_a == raw_len_b + && (memcmp (raw_data_a, raw_data_b, raw_len_a) == 0); } static GVariant * @@ -284,60 +336,43 @@ parse_management_address_tlv (const uint8_t *data, gsize len) static LldpNeighbor * lldp_neighbor_new (sd_lldp_neighbor *neighbor_sd, GError **error) { - nm_auto (lldp_neighbor_freep) LldpNeighbor *neigh = NULL; - uint8_t chassis_id_type, port_id_type; - const void *chassis_id, *port_id; - gsize chassis_id_len, port_id_len; - int r; + LldpNeighbor *neigh; + guint8 chassis_id_type; + guint8 port_id_type; + const guint8 *chassis_id; + const guint8 *port_id; + gsize chassis_id_len; + gsize port_id_len; + gs_free char *s_chassis_id = NULL; + gs_free char *s_port_id = NULL; - r = sd_lldp_neighbor_get_chassis_id (neighbor_sd, &chassis_id_type, - &chassis_id, &chassis_id_len); - if (r < 0) { + if (!lldp_neighbor_id_get (neighbor_sd, + &chassis_id_type, + &chassis_id, + &chassis_id_len, + &port_id_type, + &port_id, + &port_id_len)) { g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "failed reading chassis-id: %s", nm_strerror_native (-r)); + "invalid LLDP neighbor"); return NULL; } - if (chassis_id_len < 1) { - g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "empty chassis-id"); - return NULL; - } - - r = sd_lldp_neighbor_get_port_id (neighbor_sd, &port_id_type, - &port_id, &port_id_len); - if (r < 0) { - g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "failed reading port-id: %s", nm_strerror_native (-r)); - return NULL; - } - if (port_id_len < 1) { - g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "empty port-id"); - return NULL; - } - - neigh = g_slice_new (LldpNeighbor); - *neigh = (LldpNeighbor) { - .chassis_id_type = chassis_id_type, - .port_id_type = port_id_type, - .neighbor_sd = sd_lldp_neighbor_ref (neighbor_sd), - }; switch (chassis_id_type) { case SD_LLDP_CHASSIS_SUBTYPE_INTERFACE_ALIAS: case SD_LLDP_CHASSIS_SUBTYPE_INTERFACE_NAME: case SD_LLDP_CHASSIS_SUBTYPE_LOCALLY_ASSIGNED: case SD_LLDP_CHASSIS_SUBTYPE_CHASSIS_COMPONENT: - neigh->chassis_id = nm_utils_buf_utf8safe_escape_cp (chassis_id, chassis_id_len, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) - ?: g_new0 (char, 1); + s_chassis_id = nm_utils_buf_utf8safe_escape_cp (chassis_id, chassis_id_len, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) + ?: g_new0 (char, 1); break; case SD_LLDP_CHASSIS_SUBTYPE_MAC_ADDRESS: - neigh->chassis_id = nm_utils_hwaddr_ntoa (chassis_id, chassis_id_len); + s_chassis_id = nm_utils_hwaddr_ntoa (chassis_id, chassis_id_len); break; default: g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, "unsupported chassis-id type %d", chassis_id_type); - goto out; + return NULL; } switch (port_id_type) { @@ -345,22 +380,27 @@ lldp_neighbor_new (sd_lldp_neighbor *neighbor_sd, GError **error) case SD_LLDP_PORT_SUBTYPE_INTERFACE_NAME: case SD_LLDP_PORT_SUBTYPE_LOCALLY_ASSIGNED: case SD_LLDP_PORT_SUBTYPE_PORT_COMPONENT: - neigh->port_id = nm_utils_buf_utf8safe_escape_cp (port_id, port_id_len, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) - ?: g_new0 (char, 1); + s_port_id = nm_utils_buf_utf8safe_escape_cp (port_id, port_id_len, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII) + ?: g_new0 (char, 1); break; case SD_LLDP_PORT_SUBTYPE_MAC_ADDRESS: - neigh->port_id = nm_utils_hwaddr_ntoa (port_id, port_id_len); + s_port_id = nm_utils_hwaddr_ntoa (port_id, port_id_len); break; default: g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, "unsupported port-id type %d", port_id_type); - goto out; + return NULL; } - neigh->valid = TRUE; - -out: - return g_steal_pointer (&neigh); + neigh = g_slice_new (LldpNeighbor); + *neigh = (LldpNeighbor) { + .neighbor_sd = sd_lldp_neighbor_ref (neighbor_sd), + .chassis_id_type = chassis_id_type, + .chassis_id = g_steal_pointer (&s_chassis_id), + .port_id_type = port_id_type, + .port_id = g_steal_pointer (&s_port_id), + }; + return neigh; } static GVariant * @@ -715,9 +755,6 @@ process_lldp_neighbor (NMLldpListener *self, sd_lldp_neighbor *neighbor_sd, gboo return; } - if (!neigh->valid) - neighbor_valid = FALSE; - neigh_old = g_hash_table_lookup (priv->lldp_neighbors, neigh); if (neigh_old) { if (!neighbor_valid) {