On 31 Oct 2025, at 2:09, Changliang Wu wrote:
> The 802.1 and 802.3 standards list ways for communicating important PoE, > Maximum Frame Size, and other important link-level features that peers > may want to look at. This commit provides a port of the LLDPD project's > protocol decode for 802.1 and 802.3 TLVs of interest, preserving the > implementation as closely as possible to the source materials. > > The supported decodes will allow decoding and displaying: > > * IEEE 802.1-2005 (Port VLAN and Protocol Configuration) > * IEEE 802.3at (Power over Ethernet plus) > * IEEE 802.3bt (4-pair PoE) > > The commit supports displaying this data in human readable and json > formats. See comments below, but also take a look at the comments in the previous patch, as they might apply here as well. //Eelco > Signed-off-by: Changliang Wu <[email protected]> > --- > NEWS | 2 + > lib/lldp/lldp-const.h | 5 + > lib/lldp/lldp.c | 169 ++++++++++++++++++++- > lib/lldp/lldpd-structs.c | 33 +++++ > lib/lldp/lldpd-structs.h | 69 +++++++++ > lib/lldp/lldpd.c | 5 + > lib/ovs-lldp.c | 312 +++++++++++++++++++++++++++++++++++++++ > 7 files changed, 593 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 8259fdd2e..6643d6efd 100644 > --- a/NEWS > +++ b/NEWS > @@ -11,6 +11,8 @@ Post-v3.6.0 > to modify the database data through this transaction. > - ovs-vsctl: > * Added 'lldp/neighbor' command that displays lldp neighbor information. > + - lldp: > + * Added support for decoding dot1 and dot3 TLVs. > > > v3.6.0 - 18 Aug 2025 > diff --git a/lib/lldp/lldp-const.h b/lib/lldp/lldp-const.h > index 8c5c0733e..e03c77f71 100644 > --- a/lib/lldp/lldp-const.h > +++ b/lib/lldp/lldp-const.h > @@ -98,6 +98,11 @@ > #define LLDP_DOT3_POWER_8023AT_TYPE1 1 > #define LLDP_DOT3_POWER_8023AT_TYPE2 2 > > +/* 802.3bt additions */ > +#define LLDP_DOT3_POWER_8023BT_OFF 0 > +#define LLDP_DOT3_POWER_8023BT_TYPE3 1 > +#define LLDP_DOT3_POWER_8023BT_TYPE4 2 > + > /* Dot3 power source */ > #define LLDP_DOT3_POWER_SOURCE_UNKNOWN 0 > #define LLDP_DOT3_POWER_SOURCE_PRIMARY 1 > diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c > index 86e0381d6..5ba18d750 100644 > --- a/lib/lldp/lldp.c > +++ b/lib/lldp/lldp.c > @@ -18,6 +18,8 @@ > */ > > #include <config.h> > +#include "lldp-const.h" > +#include "lldp-tlv.h" > #include "lldpd.h" > #include <errno.h> > #include <time.h> > @@ -373,6 +375,12 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, > int s, > void *b; > struct lldpd_aa_isid_vlan_maps_tlv *isid_vlan_map = NULL; > u_int8_t msg_auth_digest[LLDP_TLV_AA_ISID_VLAN_DIGEST_LENGTH]; > + > + struct lldpd_vlan *vlan = NULL; > + struct lldpd_pi *pi = NULL; > + struct lldpd_ppvid *ppvid; > + int vlan_len; > + > struct lldpd_mgmt *mgmt; > u_int8_t addr_str_length, addr_str_buffer[32]; > u_int8_t addr_family, addr_length, *addr_ptr, iface_subtype; > @@ -385,6 +393,9 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, > int s, > > port = xzalloc(sizeof *port); > ovs_list_init(&port->p_isid_vlan_maps); > + ovs_list_init(&port->p_vlans); > + ovs_list_init(&port->p_ppvids); > + ovs_list_init(&port->p_pids); Should we maybe introduce a structure init like functions, we already have cleanup ones. > > length = s; > pos = (u_int8_t*) frame; > @@ -569,9 +580,163 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, > int s, > PEEK_BYTES(orgid, sizeof orgid); > tlv_subtype = PEEK_UINT8; > if (memcmp(dot1, orgid, sizeof orgid) == 0) { > - hardware->h_rx_unrecognized_cnt++; > + switch (tlv_subtype) { > + case LLDP_TLV_DOT1_VLANNAME: > + CHECK_TLV_SIZE(7, "VLAN"); > + vlan = xzalloc(sizeof *vlan); > + vlan->v_vid = PEEK_UINT16; > + vlan_len = PEEK_UINT8; > + CHECK_TLV_SIZE(7 + vlan_len, "VLAN"); > + vlan->v_name = xzalloc(vlan_len + 1); > + PEEK_BYTES(vlan->v_name, vlan_len); > + ovs_list_push_back(&port->p_vlans, &vlan->v_entries); > + vlan = NULL; > + break; > + > + case LLDP_TLV_DOT1_PVID: > + CHECK_TLV_SIZE(6, "PVID"); > + port->p_pvid = PEEK_UINT16; > + break; > + > + case LLDP_TLV_DOT1_PPVID: > + CHECK_TLV_SIZE(7, "PPVID"); > + /* Validation needed. */ > + /* PPVID has to be unique if more than > + one PPVID TLVs are received - > + discard if duplicate. */ > + /* If support bit is not set and > + enabled bit is set - PPVID TLV is > + considered error and discarded. */ Are these comments done anywhere? We should add some test cases for this. > + ppvid = xzalloc(sizeof *ppvid); > + ppvid->p_cap_status = PEEK_UINT8; > + ppvid->p_ppvid = PEEK_UINT16; > + if (ppvid->p_ppvid > 4096) { Should this be >= 4096? > + VLOG_WARN("PPVID value %d invalid on %s", > + ppvid->p_ppvid, hardware->h_ifname); > + free(ppvid); > + } else { > + ovs_list_push_back(&port->p_ppvids, > &ppvid->p_entries); > + } > + break; > + > + case LLDP_TLV_DOT1_PI: > + /* Validation needed. */ > + /* PI has to be unique if more than > + one PI TLVs are received - discard > + if duplicate ?? */ Clean up comments and implement the checks. > + pi = xzalloc(sizeof *pi); > + pi->p_pi_len = PEEK_UINT8; > + CHECK_TLV_SIZE(5 + pi->p_pi_len, "PI"); > + pi->p_pi = xzalloc(pi->p_pi_len); > + PEEK_BYTES(pi->p_pi, pi->p_pi_len); > + ovs_list_push_back(&port->p_pids, &pi->p_entries); > + pi = NULL; > + break; > + > + default: > + VLOG_INFO("unknown org tlv [%02x:%02x:%02x] received " > + "on %s", > + orgid[0], orgid[1], orgid[2], > + hardware->h_ifname); I think we should also log the tlv_subtype. Message can be more specific as we already know it’s an dot1 type. > + hardware->h_rx_unrecognized_cnt++; > + } > } else if (memcmp(dot3, orgid, sizeof orgid) == 0) { > - hardware->h_rx_unrecognized_cnt++; > + switch (tlv_subtype) { > + case LLDP_TLV_DOT3_MAC: > + CHECK_TLV_SIZE(9, "MAC/PHY"); > + port->p_macphy.autoneg_support = PEEK_UINT8; > + port->p_macphy.autoneg_enabled = > + (port->p_macphy.autoneg_support & 0x2) >> 1; > + port->p_macphy.autoneg_support = > + port->p_macphy.autoneg_support & 0x1; > + port->p_macphy.autoneg_advertised = PEEK_UINT16; > + port->p_macphy.mau_type = PEEK_UINT16; > + break; > + > + case LLDP_TLV_DOT3_LA: > + CHECK_TLV_SIZE(9, "Link aggregation"); > + PEEK_DISCARD_UINT8; > + port->p_aggregid = PEEK_UINT32; > + break; > + > + case LLDP_TLV_DOT3_MFS: > + CHECK_TLV_SIZE(6, "MFS"); > + port->p_mfs = PEEK_UINT16; > + break; > + > + case LLDP_TLV_DOT3_POWER: > + CHECK_TLV_SIZE(7, "Power"); > + port->p_power.devicetype = PEEK_UINT8; > + port->p_power.supported = > + (port->p_power.devicetype & 0x2) >> 1; > + port->p_power.enabled = (port->p_power.devicetype & 0x4) > >> > + 2; ‘>>’ should be on the second line. > + port->p_power.paircontrol = > + (port->p_power.devicetype & 0x8) >> 3; > + port->p_power.devicetype = (port->p_power.devicetype & > 0x1) > + ? LLDP_DOT3_POWER_PSE > + : LLDP_DOT3_POWER_PD; > + port->p_power.pairs = PEEK_UINT8; > + port->p_power.class = PEEK_UINT8; > + /* 802.3at? */ > + if (tlv_size >= 12) { > + port->p_power.powertype = PEEK_UINT8; > + port->p_power.source = > + (port->p_power.powertype & (1 << 5 | 1 << 4)) >> > 4; > + port->p_power.priority = > + (port->p_power.powertype & (1 << 1 | 1 << 0)); > + port->p_power.powertype = > + (port->p_power.powertype & (1 << 7)) > + ? LLDP_DOT3_POWER_8023AT_TYPE1 > + : LLDP_DOT3_POWER_8023AT_TYPE2; > + port->p_power.requested = PEEK_UINT16; > + port->p_power.allocated = PEEK_UINT16; > + } else { > + port->p_power.powertype = LLDP_DOT3_POWER_8023AT_OFF; > + } > + /* 802.3bt? */ > + if (tlv_size >= 29) { > + port->p_power.requested_a = PEEK_UINT16; > + port->p_power.requested_b = PEEK_UINT16; > + port->p_power.allocated_a = PEEK_UINT16; > + port->p_power.allocated_b = PEEK_UINT16; > + port->p_power.pse_status = PEEK_UINT16; > + port->p_power.pd_status = > + (port->p_power.pse_status & (1 << 13 | 1 << 12)) > >> > + 12; > + port->p_power.pse_pairs_ext = > + (port->p_power.pse_status & (1 << 11 | 1 << 10)) > >> > + 10; > + port->p_power.class_a = (port->p_power.pse_status & > + (1 << 9 | 1 << 8 | 1 << 7)) > >> > + 7; > + port->p_power.class_b = (port->p_power.pse_status & > + (1 << 6 | 1 << 5 | 1 << 4)) > >> > + 4; > + port->p_power.class_ext = > + (port->p_power.pse_status & 0xf); > + port->p_power.pse_status = > + (port->p_power.pse_status & (1 << 15 | 1 << 14)) > >> > + 14; > + port->p_power.type_ext = PEEK_UINT8; > + port->p_power.pd_load = (port->p_power.type_ext & > 0x1); > + port->p_power.type_ext = > + ((port->p_power.type_ext & > + (1 << 3 | 1 << 2 | 1 << 1)) + > + 1); > + port->p_power.pse_max = PEEK_UINT16; > + } else { > + port->p_power.type_ext = LLDP_DOT3_POWER_8023BT_OFF; > + } > + break; > + > + default: > + VLOG_INFO("unknown org tlv [%02x:%02x:%02x] received " > + "on %s", > + orgid[0], orgid[1], orgid[2], > + hardware->h_ifname); See above. > + hardware->h_rx_unrecognized_cnt++; > + } > } else if (memcmp(med, orgid, sizeof orgid) == 0) { > /* LLDP-MED */ > hardware->h_rx_unrecognized_cnt++; > diff --git a/lib/lldp/lldpd-structs.c b/lib/lldp/lldpd-structs.c > index a8c7fad09..3a886e78d 100644 > --- a/lib/lldp/lldpd-structs.c > +++ b/lib/lldp/lldpd-structs.c > @@ -113,6 +113,37 @@ lldpd_aa_maps_cleanup(struct lldpd_port *port) > } > } > > +static void > +lldpd_dot13_lists_cleanup(struct lldpd_port *port) > +{ > + struct lldpd_ppvid *ppvid; > + struct lldpd_vlan *vlan; > + struct lldpd_pi *pid; New line > + if (!ovs_list_is_empty(&port->p_vlans)) { > + LIST_FOR_EACH_SAFE (vlan, v_entries, &port->p_vlans) { > + ovs_list_remove(&vlan->v_entries); > + free(vlan->v_name); > + free(vlan); > + } > + ovs_list_init(&port->p_vlans); I think you do not need to initialize the list, this should have been done before when the port was allocated (in lldp_decode()). Then you can also remove the ‘if (!ovs_list_is_empty(&port->p_vlans)) {‘ and only keep the LIST_FOR_EACH(). > + } New line > + if (!ovs_list_is_empty(&port->p_ppvids)) { > + LIST_FOR_EACH_SAFE (ppvid, p_entries, &port->p_ppvids) { > + ovs_list_remove(&ppvid->p_entries); > + free(ppvid); > + } > + ovs_list_init(&port->p_ppvids); > + } New line > + if (!ovs_list_is_empty(&port->p_pids)) { > + LIST_FOR_EACH_SAFE (pid, p_entries, &port->p_pids) { > + ovs_list_remove(&pid->p_entries); > + free(pid->p_pi); > + free(pid); > + } > + ovs_list_init(&port->p_pids); > + } > +} > + > /* If `all' is true, clear all information, including information that > are not refreshed periodically. Port should be freed manually. */ > void > @@ -127,6 +158,8 @@ lldpd_port_cleanup(struct lldpd_port *port, bool all) > /* Cleanup auto-attach mappings */ > lldpd_aa_maps_cleanup(port); > > + lldpd_dot13_lists_cleanup(port); > + > if (all) { > free(port->p_lastframe); > /* Chassis may not have been attributed, yet.*/ > diff --git a/lib/lldp/lldpd-structs.h b/lib/lldp/lldpd-structs.h > index 9a83d5499..308242dd2 100644 > --- a/lib/lldp/lldpd-structs.h > +++ b/lib/lldp/lldpd-structs.h > @@ -45,6 +45,63 @@ lldpd_af(int af) > } > } > > +struct lldpd_ppvid { > + struct ovs_list p_entries; > + u_int8_t p_cap_status; > + u_int16_t p_ppvid; > +}; > + > +struct lldpd_vlan { > + struct ovs_list v_entries; > + char *v_name; > + u_int16_t v_vid; > +}; > + > +struct lldpd_pi { > + struct ovs_list p_entries; > + char *p_pi; > + u_int32_t p_pi_len; > +}; > + > +struct lldpd_dot3_macphy { > + u_int8_t autoneg_support; > + u_int8_t autoneg_enabled; > + u_int16_t autoneg_advertised; > + u_int16_t mau_type; > +}; > + > +struct lldpd_dot3_power { > + u_int8_t devicetype; > + u_int8_t supported; > + u_int8_t enabled; > + u_int8_t paircontrol; > + u_int8_t pairs; > + u_int8_t class; > + u_int8_t powertype; /* If set to LLDP_DOT3_POWER_8023AT_OFF, > + * following fields have no meaning. */ ’the below’ > + u_int8_t source; > + u_int8_t priority; > + u_int16_t requested; > + u_int16_t allocated; > + > + /* For 802.3BT */ > + u_int8_t pd_4pid; > + u_int16_t requested_a; > + u_int16_t requested_b; > + u_int16_t allocated_a; > + u_int16_t allocated_b; > + u_int16_t pse_status; > + u_int8_t pd_status; > + u_int8_t pse_pairs_ext; > + u_int8_t class_a; > + u_int8_t class_b; > + u_int8_t class_ext; > + u_int8_t type_ext; > + u_int8_t pd_load; > + u_int16_t pse_max; > +}; > + > + > #define LLDPD_MGMT_MAXADDRSIZE 16 /* sizeof(struct in6_addr) */ > struct lldpd_mgmt { > struct ovs_list m_entries; > @@ -98,6 +155,18 @@ struct lldpd_port { > char *p_descr; > u_int16_t p_mfs; > struct lldpd_aa_element_tlv p_element; > + > + /* Dot3 stuff */ /* IEEE 802.3 (DOT3) TLV data. */ > + u_int32_t p_aggregid; > + struct lldpd_dot3_macphy p_macphy; > + struct lldpd_dot3_power p_power; > + > + /* Dot1 stuff */ /* IEEE 80213 (DOT1) TLV data. */ > + u_int16_t p_pvid; > + struct ovs_list p_vlans; /* Contains "struct lldpd_vlan"s. */ > + struct ovs_list p_ppvids; /* Contains "struct lldpd_ppvid"s. */ > + struct ovs_list p_pids; /* Contains "struct lldpd_pi"s. */ Maybe align comments? > + > struct ovs_list p_isid_vlan_maps; /* Contains "struct > lldpd_aa_isid_vlan_maps_tlv"s. */ > }; > > diff --git a/lib/lldp/lldpd.c b/lib/lldp/lldpd.c > index 4bff7b017..dd9ba9925 100644 > --- a/lib/lldp/lldpd.c > +++ b/lib/lldp/lldpd.c > @@ -90,6 +90,11 @@ lldpd_alloc_hardware(struct lldpd *cfg, const char *name, > int index) > hw->h_lport.p_chassis = CONTAINER_OF(ovs_list_front(&cfg->g_chassis), > struct lldpd_chassis, list); > hw->h_lport.p_chassis->c_refcount++; > + ovs_list_init(&hw->h_lport.p_vlans); > + ovs_list_init(&hw->h_lport.p_ppvids); > + ovs_list_init(&hw->h_lport.p_pids); > + ovs_list_init(&hw->h_lport.p_isid_vlan_maps); Should we maybe introduce a structure init like functions, we already have cleanup ones. > + > ovs_list_init(&hw->h_rports); > > return hw; > diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c > index d85845ba6..f755fb180 100644 > --- a/lib/ovs-lldp.c > +++ b/lib/ovs-lldp.c > @@ -364,6 +364,312 @@ lldp_get_network_addr_string(char *port_id) > return ipaddress; > } > > +static void > +lldp_print_neighbor_port_dot1(struct ds *ds, struct lldpd_port *port) > +{ > + struct lldpd_ppvid *ppvid; > + struct lldpd_vlan *vlan; > + bool foundpvid = false; > + struct lldpd_pi *pid; > + > + LIST_FOR_EACH (vlan, v_entries, &port->p_vlans) { > + ds_put_format(ds, " %-20s%d, %s", "VLAN:", vlan->v_vid, > + port->p_pvid == vlan->v_vid ? "pvid: yes" : "pvid: > no"); > + if (port->p_pvid == vlan->v_vid) { > + foundpvid = true; > + } > + if (vlan->v_name) { > + ds_put_format(ds, ", %s", vlan->v_name); > + } > + ds_put_format(ds, "\n"); > + } > + if (!foundpvid && port->p_pvid > 0) { > + ds_put_format(ds, " %-20s%d, %s\n", "VLAN:", port->p_pvid, > + "pvid: yes"); > + } > + VLAN: 2, pvid: no, VLAN2 Looks a bit messy, what about: VLAN: ID 2, name “VLAN2”, PVID yes Or dump them separately as they are different TLVs: PORT VLAN ID: 200 VLAN NAME: “VLAN1”, ID 100 VLAN NAME: “VLAN2”, ID 200 > + LIST_FOR_EACH (ppvid, p_entries, &port->p_ppvids) { > + ds_put_format(ds, " %-20s%d, ", "PPVID:", ppvid->p_ppvid); > + ds_put_format( > + ds, "supported: %s,enabled %s\n", Missing space after %s. However, what about a shorter format: PPVID: 0 supported, enabled PPVID: 1024 supported, disabled PPVID: 2020 unsupported > + (ppvid->p_cap_status & LLDP_PPVID_CAP_SUPPORTED) ? "yes" : "no", > + (ppvid->p_cap_status & LLDP_PPVID_CAP_ENABLED) ? "yes" : "no"); > + } > + > + LIST_FOR_EACH (pid, p_entries, &port->p_pids) { > + if (pid->p_pi && pid->p_pi_len > 0) { > + ds_put_format(ds, " %-20s", "PI:"); > + ds_put_buffer(ds, pid->p_pi, pid->p_pi_len); > + ds_put_format(ds, "\n"); I see no test case for this. Also, there needs to be proper decoding for this. > + } > + } > +} > + > +static void > +lldp_print_neighbor_port_dot1_json(struct json *neighbor_item_json, > + struct lldpd_port *port) > +{ > + struct json *ppvid_json_array = json_array_create_empty(); > + struct json *vlan_json_array = json_array_create_empty(); > + struct json *pi_json_array = json_array_create_empty(); > + struct json *ppvid_json = NULL; > + struct json *vlan_json = NULL; > + struct json *pi_json = NULL; > + bool foundpvid = false; > + struct lldpd_ppvid *ppvid; > + struct lldpd_vlan *vlan; > + struct lldpd_pi *pid; > + > + LIST_FOR_EACH (vlan, v_entries, &port->p_vlans) { > + vlan_json = json_object_create(); > + json_object_put(vlan_json, "vlan-id", > + json_integer_create(vlan->v_vid)); > + json_object_put(vlan_json, "pvid", > + json_boolean_create(port->p_pvid == vlan->v_vid)); > + if (port->p_pvid == vlan->v_vid) { > + foundpvid = true; > + } > + if (vlan->v_name) { > + json_object_put(vlan_json, "value", > + json_string_create(vlan->v_name)); > + } > + json_array_add(vlan_json_array, vlan_json); > + } > + if (!foundpvid && port->p_pvid > 0) { > + vlan_json = json_object_create(); > + json_object_put(vlan_json, "vlan-id", > + json_integer_create(port->p_pvid)); > + json_object_put(vlan_json, "pvid", json_boolean_create(true)); > + json_array_add(vlan_json_array, vlan_json); > + } > + if (json_array_size(vlan_json_array) > 0) { > + json_object_put(neighbor_item_json, "vlan", vlan_json_array); > + } > + > + LIST_FOR_EACH (ppvid, p_entries, &port->p_ppvids) { > + ppvid_json = json_object_create(); > + if (ppvid->p_ppvid) { > + json_object_put(ppvid_json, "ppvid", > + json_integer_create(ppvid->p_ppvid)); > + } > + json_object_put(ppvid_json, "supported", > + json_boolean_create(ppvid->p_cap_status & > + LLDP_PPVID_CAP_SUPPORTED)); > + json_object_put(ppvid_json, "enabled", > + json_boolean_create(ppvid->p_cap_status & > + LLDP_PPVID_CAP_ENABLED)); > + json_array_add(ppvid_json_array, ppvid_json); > + } > + if (json_array_size(ppvid_json_array) > 0) { > + json_object_put(neighbor_item_json, "ppvid", ppvid_json_array); > + } > + > + LIST_FOR_EACH (pid, p_entries, &port->p_pids) { > + if (pid->p_pi && pid->p_pi_len > 0) { > + pi_json = json_object_create(); > + struct ds pi_ds = DS_EMPTY_INITIALIZER; > + ds_put_buffer(&pi_ds, pid->p_pi, pid->p_pi_len); > + json_object_put(pi_json, "pi", > + > json_string_create_nocopy(ds_steal_cstr(&pi_ds))); > + json_array_add(pi_json_array, pi_json); > + } > + } > + if (json_array_size(pi_json_array) > 0) { > + json_object_put(neighbor_item_json, "pi", pi_json_array); > + } > +} > + > +static void > +lldp_dot3_autoneg_advertised(struct ds *ds, uint16_t pmd_auto_nego, int > bithd, > + int bitfd, const char *type) > +{ > + if (!((pmd_auto_nego & bithd) || (pmd_auto_nego & bitfd))) { > + return; > + } > + > + ds_put_format(ds, " %-18s%s", "Adv:", type); > + if (bithd != bitfd) { > + ds_put_format(ds, ", HD: %s, FD: %s\n", > + (pmd_auto_nego & bithd) ? "yes" : "no", > + (pmd_auto_nego & bitfd) ? "yes" : "no"); > + } > +} > + > +static void > +lldp_dot3_autoneg_advertised_json(struct json **advertised_json, > + uint16_t pmd_auto_nego, int bithd, int > bitfd, > + const char *type) > +{ > + if (!((pmd_auto_nego & bithd) || (pmd_auto_nego & bitfd))) { > + return; > + } > + > + if (!*advertised_json) { > + *advertised_json = json_array_create_empty(); > + } > + > + struct json *advertised_item_json = json_object_create(); > + json_object_put(advertised_item_json, "type", json_string_create(type)); > + if (bithd != bitfd) { > + json_object_put(advertised_item_json, "hd", > + json_boolean_create((pmd_auto_nego & bithd) ? true > + : > false)); > + json_object_put(advertised_item_json, "fd", > + json_boolean_create((pmd_auto_nego & bitfd) ? true > + : > false)); > + } > + json_array_add(*advertised_json, advertised_item_json); > +} > + > +static void > +lldp_print_neighbor_port_dot3(struct ds *ds, struct lldpd_port *port) > +{ > + if (port->p_mfs) { > + ds_put_format(ds, " %-20s%d\n", "MFS:", port->p_mfs); > + } > + > + uint16_t autoneg_advertised = port->p_macphy.autoneg_advertised; > + uint16_t autoneg_support = port->p_macphy.autoneg_support; > + uint16_t autoneg_enabled = port->p_macphy.autoneg_enabled; > + uint16_t mautype = port->p_macphy.mau_type; > + if (autoneg_support || autoneg_enabled || mautype) { > + ds_put_format(ds, " PMD autoneg: supported: %s, enabled: %s\n", > + autoneg_support ? "yes" : "no", > + autoneg_enabled ? "yes" : "no"); This does not follow the other format (%-20s)? Also al the extra yes/no, etc flags make it hard to read. Maybe do it as ethtool does it. What about: MFS: 10240 Auto-Negotiation: Supported, Enabled Capabilities: 10baseT/Half 10baseT/Full 100Base-T2/Full > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_10BASE_T, > + LLDP_DOT3_LINK_AUTONEG_10BASET_FD, > + "10Base-T"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T4, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T4, T4 is only half duplex. > + "100Base-T4"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_100BASE_TX, > + LLDP_DOT3_LINK_AUTONEG_100BASE_TXFD, > + "100Base-TX"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T2, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T2FD, > + "100Base-T2"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_1000BASE_X, > + LLDP_DOT3_LINK_AUTONEG_1000BASE_XFD, > + "1000Base-X"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_1000BASE_T, > + LLDP_DOT3_LINK_AUTONEG_1000BASE_TFD, > + "1000Base-T"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_PAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_PAUSE, > + "FDX_PAUSE"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_APAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_APAUSE, > + "FDX_APAUSE"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_SPAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_SPAUSE, > + "FDX_SPAUSE"); > + lldp_dot3_autoneg_advertised(ds, autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_BPAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_BPAUSE, > + "FDX_BPAUSE"); > + ds_put_format(ds, " %-18s%hu\n", "MAU oper type:", mautype); > + } > + > + if (port->p_power.devicetype) { > + ds_put_format(ds, Same format as usual; MDI Power: supported, disabled, … I also think you miss quite some info from this TLV… MDI Power: Device Type : Powered Device Power Class : 2 Power Available: 15.4 W Power Priority : High Pairs Used : Signal > + " MDI Power: supported: %s, enabled: %s, pair " > + "control: %s\n", > + port->p_power.supported ? "yes" : "no", > + port->p_power.enabled ? "yes" : "no", > + port->p_power.paircontrol ? "yes" : "no"); > + } > +} > + > +static void > +lldp_print_neighbor_port_dot3_json(struct json *port_json, > + struct lldpd_port *port) > +{ > + if (port->p_mfs) { > + json_object_put(port_json, "mfs", json_integer_create(port->p_mfs)); > + } > + > + uint16_t autoneg_advertised = port->p_macphy.autoneg_advertised; > + uint16_t autoneg_support = port->p_macphy.autoneg_support; > + uint16_t autoneg_enabled = port->p_macphy.autoneg_enabled; > + uint16_t mautype = port->p_macphy.mau_type; > + if (autoneg_support || autoneg_enabled || mautype) { > + struct json *auto_nego_json = json_object_create(); > + struct json *advertised_json = NULL; > + > + json_object_put(auto_nego_json, "supported", > + json_boolean_create(autoneg_support)); > + json_object_put(auto_nego_json, "enabled", > + json_boolean_create(autoneg_enabled)); > + > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_10BASE_T, > + LLDP_DOT3_LINK_AUTONEG_10BASET_FD, > + "10Base-T"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T4, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T4, > + "100Base-T4"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_100BASE_TX, > + > LLDP_DOT3_LINK_AUTONEG_100BASE_TXFD, > + "100Base-TX"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_100BASE_T2, > + > LLDP_DOT3_LINK_AUTONEG_100BASE_T2FD, > + "100Base-T2"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_1000BASE_X, > + > LLDP_DOT3_LINK_AUTONEG_1000BASE_XFD, > + "1000Base-X"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_1000BASE_T, > + > LLDP_DOT3_LINK_AUTONEG_1000BASE_TFD, > + "1000Base-T"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_PAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_PAUSE, > + "FDX_PAUSE"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_APAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_APAUSE, > + "FDX_APAUSE"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_SPAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_SPAUSE, > + "FDX_SPAUSE"); > + lldp_dot3_autoneg_advertised_json(&advertised_json, > autoneg_advertised, > + LLDP_DOT3_LINK_AUTONEG_FDX_BPAUSE, > + LLDP_DOT3_LINK_AUTONEG_FDX_BPAUSE, > + "FDX_BPAUSE"); > + json_object_put(auto_nego_json, "current", > + json_integer_create(mautype)); > + > + json_object_put(port_json, "auto-negotiation", auto_nego_json); > + } > + > + if (port->p_power.devicetype) { > + struct json *power_json = json_object_create(); > + > + json_object_put(power_json, "supported", > + json_boolean_create(port->p_power.supported)); > + json_object_put(power_json, "enabled", > + json_boolean_create(port->p_power.enabled)); > + json_object_put(power_json, "paircontrol", > + json_boolean_create(port->p_power.paircontrol)); > + json_object_put(port_json, "power", power_json); > + } > +} > + > static void > lldp_print_neighbor(struct ds *ds, struct lldp *lldp) OVS_REQUIRES(mutex) > { > @@ -557,6 +863,9 @@ lldp_print_neighbor(struct ds *ds, struct lldp *lldp) > OVS_REQUIRES(mutex) > ds_put_format(ds, " %-20s%d\n", > "MgmtIface:", mgmt->m_iface); > } > + > + lldp_print_neighbor_port_dot3(ds, port); > + lldp_print_neighbor_port_dot1(ds, port); I would reverse the order. > } > } > } > @@ -817,6 +1126,9 @@ lldp_print_neighbor_json(struct json *neighbor_json, > struct lldp *lldp) > json_object_put(chassis_sys_json, "mgmt-iface", > chassis_mgmt_iface_json); > > + lldp_print_neighbor_port_dot1_json(neighbor_item_json, port); > + lldp_print_neighbor_port_dot3_json(port_json, port); Why is dot1 going to neighbor_item_json and dot3 to port_json? > + > json_object_put(neighbor_item_json, "chassis", chassis_json); > json_object_put(neighbor_item_json, "port", port_json); > > -- > 2.43.5 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
