From e189d65ab64cef952802d00596dd1b4defc78bdc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 5 Jun 2020 19:54:05 +0200 Subject: [PATCH] lldp: expose raw LLDP message on D-Bus Also, track sd_lldp_neighbor instance directly. sd_lldp_neighbor is a perfectly reasonable container for keeping track of the LLDP neighbor information. Just keep a reference to it, and don't clone the data. Especially since the LLDP library keeps a reference to this instance as well. Also, to compare whether two neighbors are the same, it is sufficient to only consider the raw data. Everything else depends on these fields anyway. This is only possible and useful becuase sd_lldp_neighbor is of course immutable. It wouldn't make sense otherwise, but it also would be bad design to mutate the sd_lldp_neighbor instances. This couples our code slightly more to the systemd code, which we usually try to avoid. But when we move away in the future from systemd LLDP library, we anyway need to rework this heavily (and then too, we wouldn't want to clone the data, when we could just share the reference). --- libnm-core/nm-dbus-interface.h | 1 + src/devices/nm-lldp-listener.c | 97 ++++++++++++++++++---------------- src/devices/tests/test-lldp.c | 22 ++++++-- 3 files changed, 70 insertions(+), 50 deletions(-) diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h index 1e656333c..f902ee9ef 100644 --- a/libnm-core/nm-dbus-interface.h +++ b/libnm-core/nm-dbus-interface.h @@ -860,6 +860,7 @@ typedef enum /*< flags >*/ { #undef NM_AVAILABLE_IN_1_8 #endif +#define NM_LLDP_ATTR_RAW "raw" #define NM_LLDP_ATTR_DESTINATION "destination" #define NM_LLDP_ATTR_CHASSIS_ID_TYPE "chassis-id-type" #define NM_LLDP_ATTR_CHASSIS_ID "chassis-id" diff --git a/src/devices/nm-lldp-listener.c b/src/devices/nm-lldp-listener.c index 9f4fd8a69..362382682 100644 --- a/src/devices/nm-lldp-listener.c +++ b/src/devices/nm-lldp-listener.c @@ -108,6 +108,8 @@ typedef struct { char *chassis_id; char *port_id; + sd_lldp_neighbor *neighbor_sd; + LldpAttrData attrs[_LLDP_ATTR_ID_COUNT]; struct ether_addr destination_address; @@ -326,6 +328,27 @@ _lldp_attr_add_variant (LldpAttrData *pdata, LldpAttrId attr_id, GVariant *varia /*****************************************************************************/ +static void +lldp_neighbor_get_raw (LldpNeighbor *neigh, + const guint8 **out_raw_data, + gsize *out_raw_len) +{ + gconstpointer raw_data; + gsize raw_len; + int r; + + nm_assert (neigh); + + r = sd_lldp_neighbor_get_raw (neigh->neighbor_sd, &raw_data, &raw_len); + + nm_assert (r >= 0); + nm_assert (raw_data); + nm_assert (raw_len > 0); + + *out_raw_data = raw_data; + *out_raw_len = raw_len; +} + static guint lldp_neighbor_id_hash (gconstpointer ptr) { @@ -400,6 +423,7 @@ lldp_neighbor_free (LldpNeighbor *neighbor) } } nm_g_variant_unref (neighbor->variant); + sd_lldp_neighbor_unref (neighbor->neighbor_sd); nm_g_slice_free (neighbor); } @@ -412,56 +436,30 @@ lldp_neighbor_freep (LldpNeighbor **ptr) static gboolean lldp_neighbor_equal (LldpNeighbor *a, LldpNeighbor *b) { - LldpAttrId attr_id; - guint i; + const guint8 *raw_data_a; + const guint8 *raw_data_b; + gsize raw_len_a; + gsize raw_len_b; + gboolean equal; - nm_assert (a); - nm_assert (b); + if (a->neighbor_sd == b->neighbor_sd) + return TRUE; - if ( a->chassis_id_type != b->chassis_id_type - || a->port_id_type != b->port_id_type - || ether_addr_equal (&a->destination_address, &b->destination_address) - || !nm_streq0 (a->chassis_id, b->chassis_id) - || !nm_streq0 (a->port_id, b->port_id)) + 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; - for (attr_id = 0; attr_id < _LLDP_ATTR_ID_COUNT; attr_id++) { - if (a->attrs[attr_id].attr_type != b->attrs[attr_id].attr_type) - return FALSE; - switch (a->attrs[attr_id].attr_type) { - case LLDP_ATTR_TYPE_UINT32: - if (a->attrs[attr_id].v_uint32 != b->attrs[attr_id].v_uint32) - return FALSE; - break; - case LLDP_ATTR_TYPE_STRING: - if (!nm_streq (a->attrs[attr_id].v_string, b->attrs[attr_id].v_string)) - return FALSE; - break; - case LLDP_ATTR_TYPE_VARIANT: - if (!g_variant_equal (a->attrs[attr_id].v_variant, b->attrs[attr_id].v_variant)) - return FALSE; - break; - case LLDP_ATTR_TYPE_ARRAY_OF_VARIANTS: - if (a->attrs[attr_id].v_variant_list.len != b->attrs[attr_id].v_variant_list.len) - return FALSE; - if (a->attrs[attr_id].v_variant_list.alloc == 1) { - nm_assert (b->attrs[attr_id].v_variant_list.alloc == 1); - if (!g_variant_equal (a->attrs[attr_id].v_variant_list.arr1, b->attrs[attr_id].v_variant_list.arr1)) - return FALSE; - } else { - for (i = 0; i < a->attrs[attr_id].v_variant_list.len; i++) { - if (!g_variant_equal (a->attrs[attr_id].v_variant_list.arr[i], b->attrs[attr_id].v_variant_list.arr[i])) - return FALSE; - } - } - break; - default: - nm_assert (a->attrs[attr_id].attr_type == LLDP_ATTR_TYPE_NONE); - break; - } - } + equal = (memcmp (raw_data_a, raw_data_b, raw_len_a) == 0); - return TRUE; + nm_assert ( !equal + || ( a->chassis_id_type == b->chassis_id_type + && a->port_id_type == b->port_id_type + && ether_addr_equal (&a->destination_address, &b->destination_address) + && nm_streq0 (a->chassis_id, b->chassis_id) + && nm_streq0 (a->port_id, b->port_id))); + return equal; } static GVariant * @@ -575,6 +573,7 @@ lldp_neighbor_new (sd_lldp_neighbor *neighbor_sd, GError **error) *neigh = (LldpNeighbor) { .chassis_id_type = chassis_id_type, .port_id_type = port_id_type, + .neighbor_sd = sd_lldp_neighbor_ref (neighbor_sd), }; r = sd_lldp_neighbor_get_destination_address (neighbor_sd, &neigh->destination_address); @@ -807,12 +806,20 @@ lldp_neighbor_to_variant (LldpNeighbor *neigh) GVariantBuilder builder; const char *dest_str; LldpAttrId attr_id; + const guint8 *raw_data; + gsize raw_len; if (neigh->variant) return neigh->variant; + lldp_neighbor_get_raw (neigh, &raw_data, &raw_len); + g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); + nm_g_variant_builder_add_sv_bytearray (&builder, + NM_LLDP_ATTR_RAW, + raw_data, + raw_len); g_variant_builder_add (&builder, "{sv}", NM_LLDP_ATTR_CHASSIS_ID_TYPE, g_variant_new_uint32 (neigh->chassis_id_type)); diff --git a/src/devices/tests/test-lldp.c b/src/devices/tests/test-lldp.c index b394b0e50..4d563c604 100644 --- a/src/devices/tests/test-lldp.c +++ b/src/devices/tests/test-lldp.c @@ -104,7 +104,7 @@ typedef struct { #define TEST_IFNAME "nm-tap-test0" TEST_RECV_FRAME_DEFINE (_test_recv_data0_frame0, - "{'chassis-id-type': , 'chassis-id': <'00:01:02:03:04:05'>, 'port-id-type': , 'port-id': <'1/3'>, 'destination': <'nearest-non-tpmr-bridge'>, 'port-description': <'Port'>, 'system-name': <'SYS'>, 'system-description': <'foo'>}", + "{'raw': <[byte 0x01, 0x80, 0xc2, 0x00, 0x00, 0x03, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x88, 0xcc, 0x02, 0x07, 0x04, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x04, 0x04, 0x05, 0x31, 0x2f, 0x33, 0x06, 0x02, 0x00, 0x78, 0x08, 0x04, 0x50, 0x6f, 0x72, 0x74, 0x0a, 0x03, 0x53, 0x59, 0x53, 0x0c, 0x04, 0x66, 0x6f, 0x6f, 0x00, 0x00, 0x00]>, 'chassis-id-type': , 'chassis-id': <'00:01:02:03:04:05'>, 'port-id-type': , 'port-id': <'1/3'>, 'destination': <'nearest-non-tpmr-bridge'>, 'port-description': <'Port'>, 'system-name': <'SYS'>, 'system-description': <'foo'>}", /* Ethernet header */ 0x01, 0x80, 0xc2, 0x00, 0x00, 0x03, /* Destination MAC */ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, /* Source MAC */ @@ -135,7 +135,11 @@ _test_recv_data0_check_do (GMainLoop *loop, NMLldpListener *listener, const Test SD_LLDP_CHASSIS_SUBTYPE_MAC_ADDRESS, "00:01:02:03:04:05", SD_LLDP_PORT_SUBTYPE_INTERFACE_NAME, "1/3"); g_assert (neighbor); - g_assert_cmpint (g_variant_n_children (neighbor), ==, 4 + 4); + g_assert_cmpint (g_variant_n_children (neighbor), ==, 1 + 4 + 4); + + attr = g_variant_lookup_value (neighbor, NM_LLDP_ATTR_RAW, G_VARIANT_TYPE_BYTESTRING); + nmtst_assert_variant_bytestring (attr, frame->frame, frame->frame_len); + nm_clear_g_variant (&attr); attr = g_variant_lookup_value (neighbor, NM_LLDP_ATTR_PORT_DESCRIPTION, G_VARIANT_TYPE_STRING); nmtst_assert_variant_string (attr, "Port"); @@ -166,7 +170,7 @@ TEST_RECV_DATA_DEFINE (_test_recv_data0_twice, 1, _test_recv_data0_check, &_tes TEST_RECV_FRAME_DEFINE (_test_recv_data1_frame0, /* lldp.detailed.pcap from * https://wiki.wireshark.org/SampleCaptures#Link_Layer_Discovery_Protocol_.28LLDP.29 */ - "{'chassis-id-type': , 'chassis-id': <'00:01:30:F9:AD:A0'>, 'port-id-type': , 'port-id': <'1/1'>, 'destination': <'nearest-bridge'>, 'port-description': <'Summit300-48-Port 1001'>, 'system-name': <'Summit300-48'>, 'system-description': <'Summit300-48 - Version 7.4e.1 (Build 5) by Release_Master 05/27/05 04:53:11'>, 'system-capabilities': , 'management-addresses': <[{'address-subtype': , 'object-id': <@ay []>, 'interface-number': , 'address': <[byte 0x00, 0x01, 0x30, 0xf9, 0xad, 0xa0]>, 'interface-number-subtype': }]>, 'ieee-802-1-pvid': , 'ieee-802-1-ppvid': , 'ieee-802-1-ppvid-flags': , 'ieee-802-1-ppvids': <[{'flags': , 'ppvid': }]>, 'ieee-802-1-vid': , 'ieee-802-1-vlan-name': <'v2-0488-03-0505'>, 'ieee-802-1-vlans': <[{'vid': , 'name': <'v2-0488-03-0505'>}]>, 'ieee-802-3-mac-phy-conf': <{'operational-mau-type': , 'autoneg': , 'pmd-autoneg-cap': }>, 'ieee-802-3-power-via-mdi': <{'pse-power-pair': , 'mdi-power-support': , 'power-class': }>, 'ieee-802-3-max-frame-size': }", + "{'raw': <[byte 0x01, 0x80, 0xc2, 0x00, 0x00, 0x0e, 0x00, 0x01, 0x30, 0xf9, 0xad, 0xa0, 0x88, 0xcc, 0x02, 0x07, 0x04, 0x00, 0x01, 0x30, 0xf9, 0xad, 0xa0, 0x04, 0x04, 0x05, 0x31, 0x2f, 0x31, 0x06, 0x02, 0x00, 0x78, 0x08, 0x17, 0x53, 0x75, 0x6d, 0x6d, 0x69, 0x74, 0x33, 0x30, 0x30, 0x2d, 0x34, 0x38, 0x2d, 0x50, 0x6f, 0x72, 0x74, 0x20, 0x31, 0x30, 0x30, 0x31, 0x00, 0x0a, 0x0d, 0x53, 0x75, 0x6d, 0x6d, 0x69, 0x74, 0x33, 0x30, 0x30, 0x2d, 0x34, 0x38, 0x00, 0x0c, 0x4c, 0x53, 0x75, 0x6d, 0x6d, 0x69, 0x74, 0x33, 0x30, 0x30, 0x2d, 0x34, 0x38, 0x20, 0x2d, 0x20, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x20, 0x37, 0x2e, 0x34, 0x65, 0x2e, 0x31, 0x20, 0x28, 0x42, 0x75, 0x69, 0x6c, 0x64, 0x20, 0x35, 0x29, 0x20, 0x62, 0x79, 0x20, 0x52, 0x65, 0x6c, 0x65, 0x61, 0x73, 0x65, 0x5f, 0x4d, 0x61, 0x73, 0x74, 0x65, 0x72, 0x20, 0x30, 0x35, 0x2f, 0x32, 0x37, 0x2f, 0x30, 0x35, 0x20, 0x30, 0x34, 0x3a, 0x35, 0x33, 0x3a, 0x31, 0x31, 0x00, 0x0e, 0x04, 0x00, 0x14, 0x00, 0x14, 0x10, 0x0e, 0x07, 0x06, 0x00, 0x01, 0x30, 0xf9, 0xad, 0xa0, 0x02, 0x00, 0x00, 0x03, 0xe9, 0x00, 0xfe, 0x07, 0x00, 0x12, 0x0f, 0x02, 0x07, 0x01, 0x00, 0xfe, 0x09, 0x00, 0x12, 0x0f, 0x01, 0x03, 0x6c, 0x00, 0x00, 0x10, 0xfe, 0x09, 0x00, 0x12, 0x0f, 0x03, 0x01, 0x00, 0x00, 0x00, 0x00, 0xfe, 0x06, 0x00, 0x12, 0x0f, 0x04, 0x05, 0xf2, 0xfe, 0x06, 0x00, 0x80, 0xc2, 0x01, 0x01, 0xe8, 0xfe, 0x07, 0x00, 0x80, 0xc2, 0x02, 0x01, 0x00, 0x00, 0xfe, 0x16, 0x00, 0x80, 0xc2, 0x03, 0x01, 0xe8, 0x0f, 0x76, 0x32, 0x2d, 0x30, 0x34, 0x38, 0x38, 0x2d, 0x30, 0x33, 0x2d, 0x30, 0x35, 0x30, 0x35, 0xfe, 0x05, 0x00, 0x80, 0xc2, 0x04, 0x00, 0x00, 0x00]>, 'chassis-id-type': , 'chassis-id': <'00:01:30:F9:AD:A0'>, 'port-id-type': , 'port-id': <'1/1'>, 'destination': <'nearest-bridge'>, 'port-description': <'Summit300-48-Port 1001'>, 'system-name': <'Summit300-48'>, 'system-description': <'Summit300-48 - Version 7.4e.1 (Build 5) by Release_Master 05/27/05 04:53:11'>, 'system-capabilities': , 'management-addresses': <[{'address-subtype': , 'object-id': <@ay []>, 'interface-number': , 'address': <[byte 0x00, 0x01, 0x30, 0xf9, 0xad, 0xa0]>, 'interface-number-subtype': }]>, 'ieee-802-1-pvid': , 'ieee-802-1-ppvid': , 'ieee-802-1-ppvid-flags': , 'ieee-802-1-ppvids': <[{'flags': , 'ppvid': }]>, 'ieee-802-1-vid': , 'ieee-802-1-vlan-name': <'v2-0488-03-0505'>, 'ieee-802-1-vlans': <[{'vid': , 'name': <'v2-0488-03-0505'>}]>, 'ieee-802-3-mac-phy-conf': <{'operational-mau-type': , 'autoneg': , 'pmd-autoneg-cap': }>, 'ieee-802-3-power-via-mdi': <{'pse-power-pair': , 'mdi-power-support': , 'power-class': }>, 'ieee-802-3-max-frame-size': }", /* ethernet header */ 0x01, 0x80, 0xc2, 0x00, 0x00, 0x0e, /* destination mac */ 0x00, 0x01, 0x30, 0xf9, 0xad, 0xa0, /* source mac */ @@ -238,7 +242,11 @@ _test_recv_data1_check (GMainLoop *loop, NMLldpListener *listener) SD_LLDP_CHASSIS_SUBTYPE_MAC_ADDRESS, "00:01:30:F9:AD:A0", SD_LLDP_PORT_SUBTYPE_INTERFACE_NAME, "1/1"); g_assert (neighbor); - g_assert_cmpint (g_variant_n_children (neighbor), ==, 4 + 16); + g_assert_cmpint (g_variant_n_children (neighbor), ==, 1 + 4 + 16); + + attr = g_variant_lookup_value (neighbor, NM_LLDP_ATTR_RAW, G_VARIANT_TYPE_BYTESTRING); + nmtst_assert_variant_bytestring (attr, _test_recv_data1_frame0.frame, _test_recv_data1_frame0.frame_len); + nm_clear_g_variant (&attr); attr = g_variant_lookup_value (neighbor, NM_LLDP_ATTR_DESTINATION, G_VARIANT_TYPE_STRING); nmtst_assert_variant_string (attr, NM_LLDP_DEST_NEAREST_BRIDGE); @@ -361,7 +369,7 @@ _test_recv_data1_check (GMainLoop *loop, NMLldpListener *listener) TEST_RECV_DATA_DEFINE (_test_recv_data1, 1, _test_recv_data1_check, &_test_recv_data1_frame0); TEST_RECV_FRAME_DEFINE (_test_recv_data2_frame0_ttl1, - "{'chassis-id-type': , 'chassis-id': <'00:01:02:03:04:05'>, 'port-id-type': , 'port-id': <'1/3'>, 'destination': <'nearest-non-tpmr-bridge'>, 'port-description': <'Port'>, 'system-name': <'SYS'>, 'system-description': <'foo'>}", + "{'raw': <[byte 0x01, 0x80, 0xc2, 0x00, 0x00, 0x03, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x88, 0xcc, 0x02, 0x07, 0x04, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x04, 0x04, 0x05, 0x31, 0x2f, 0x33, 0x06, 0x02, 0x00, 0x01, 0x08, 0x04, 0x50, 0x6f, 0x72, 0x74, 0x0a, 0x03, 0x53, 0x59, 0x53, 0x0c, 0x04, 0x66, 0x6f, 0x6f, 0x00, 0x00, 0x00]>, 'chassis-id-type': , 'chassis-id': <'00:01:02:03:04:05'>, 'port-id-type': , 'port-id': <'1/3'>, 'destination': <'nearest-non-tpmr-bridge'>, 'port-description': <'Port'>, 'system-name': <'SYS'>, 'system-description': <'foo'>}", /* Ethernet header */ 0x01, 0x80, 0xc2, 0x00, 0x00, 0x03, /* Destination MAC */ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, /* Source MAC */ @@ -534,6 +542,10 @@ test_parse_frames (gconstpointer test_data) v_neighbor = nmtst_lldp_parse_from_raw (frame->frame, frame->frame_len); g_assert (v_neighbor); + attr = g_variant_lookup_value (v_neighbor, NM_LLDP_ATTR_RAW, G_VARIANT_TYPE_BYTESTRING); + nmtst_assert_variant_bytestring (attr, frame->frame, frame->frame_len); + nm_clear_g_variant (&attr); + as_variant = g_variant_print (v_neighbor, TRUE); g_assert (as_variant); g_assert_cmpstr (frame->as_variant, ==, as_variant);