Changliang Wu <changliang...@smartx.com> writes:
> 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.
>
> Signed-off-by: Changliang Wu <changliang...@smartx.com>
> ---
> NEWS | 2 +
> lib/lldp/lldp-const.h | 5 +
> lib/lldp/lldp.c | 164 +++++++++++++++++++-
> lib/lldp/lldpd-structs.c | 31 ++++
> lib/lldp/lldpd-structs.h | 69 +++++++++
> lib/ovs-lldp.c | 316 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 585 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 931fe5c71..d46f78743 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,8 @@ Post-v3.5.0
> - ovs-appctl:
> * Added JSON output support to the 'ovs/route/show' command.
> * Added 'lldp/neighbor' command that displays lldp neighbor infomation.
> + - lldp:
> + * Added support for decoding dot1 and dot3 TLVs.
> - SSL/TLS:
> * Support for deprecated TLSv1 and TLSv1.1 protocols on OpenFlow and
> database connections is now removed.
> 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 6fdcfef56..c10bb27d9 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>
> @@ -372,6 +374,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;
> + int vlan_len;
> + struct lldpd_ppvid *ppvid;
> + struct lldpd_pi *pi = NULL;
> +
As before, please prefer reverse xmas tree for initialization. It isn't
a hard rule, but since this series will need a respin, please do it
in the next version.
> 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;
> @@ -384,6 +392,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);
>
> length = s;
> pos = (u_int8_t*) frame;
> @@ -568,9 +579,158 @@ 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 */
> + /* if PPVID > 4096 - bad and discard */
> + ppvid = xzalloc(sizeof *ppvid);
> + ppvid->p_cap_status = PEEK_UINT8;
> + ppvid->p_ppvid = PEEK_UINT16;
> + 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 ?? */
> + 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);
> + 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;
> + 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);
> + 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..9e1215293 100644
> --- a/lib/lldp/lldpd-structs.c
> +++ b/lib/lldp/lldpd-structs.c
> @@ -113,6 +113,35 @@ lldpd_aa_maps_cleanup(struct lldpd_port *port)
> }
> }
>
> +static void
> +lldpd_dot13_lists_cleanup(struct lldpd_port *port)
> +{
> + struct lldpd_vlan *vlan;
> + struct lldpd_ppvid *ppvid;
> + struct lldpd_pi *pid;
> + if (port->p_vlans.next && !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);
I think we need to release the v_name element here, right?
> + }
> + ovs_list_init(&port->p_vlans);
> + }
> + if (port->p_ppvids.next && !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);
> + }
> + if (port->p_pids.next && !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);
Likewise, we need to delete the p_pi element.
> + }
> + 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 +156,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 fe5d5f9f8..1ec7fe777 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;
NOT: please space the variables at a consistent whitespace.
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_6 v_vid;
};
etc. I also suggest adding a brief description of the field.
> +};
> +
> +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;
> + int 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 */
NIT: please make the comment spacing here consistent. For multiline
comments, we prefer:
/* Comment
* line 2 */
Also please end the comments with a '.'
> + 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 */
> + u_int32_t p_aggregid;
> + struct lldpd_dot3_macphy p_macphy;
> + struct lldpd_dot3_power p_power;
> +
> + /* Dot1 stuff */
> + 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. */
> +
> struct ovs_list p_isid_vlan_maps; /* Contains "struct
> lldpd_aa_isid_vlan_maps_tlv"s. */
> };
>
> diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
> index 2f3f6046a..ff153fc7e 100644
> --- a/lib/ovs-lldp.c
> +++ b/lib/ovs-lldp.c
> @@ -313,6 +313,316 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp)
> OVS_REQUIRES(mutex)
> }
> }
>
> +static void
> +lldp_print_neighbor_port_dot1(struct ds *ds, struct lldpd_port *port)
> +{
> + struct lldpd_vlan *vlan;
> + struct lldpd_ppvid *ppvid;
> + struct lldpd_pi *pid;
Same comment about reverse xmas tree
> +
> + 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 (vlan->v_name) {
> + ds_put_format(ds, ", %s", vlan->v_name);
> + }
> + ds_put_format(ds, "\n");
> + }
> +
> + LIST_FOR_EACH (ppvid, p_entries, &port->p_ppvids) {
> + ds_put_format(ds, " %-20s", "PPVID:");
> + if (ppvid->p_ppvid) {
> + ds_put_format(ds, ", %d ", ppvid->p_ppvid);
> + }
> + ds_put_format(
> + ds, "supported: %s,enabled %s\n",
> + (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");
> + }
> + }
> +}
> +
> +static void
> +lldp_print_neighbor_port_dot1_json(struct json *interface_item_json,
> + struct lldpd_port *port)
> +{
> + struct json *vlan_json_array = NULL;
> + struct json *vlan_json = NULL;
> + struct json *ppvid_json_array = NULL;
> + struct json *ppvid_json = NULL;
> + struct json *pi_json_array = NULL;
> + struct json *pi_json = NULL;
> +
> + struct lldpd_vlan *vlan;
> + struct lldpd_ppvid *ppvid;
> + 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 (vlan->v_name) {
> + json_object_put(vlan_json, "value",
> + json_string_create(vlan->v_name));
> + }
> + if (ovs_list_size(&port->p_vlans) > 1) {
> + if (vlan_json_array == NULL) {
> + vlan_json_array = json_array_create_empty();
> + }
> + json_array_add(vlan_json_array, vlan_json);
> + }
> + }
> + if (vlan_json_array || vlan_json) {
> + json_object_put(interface_item_json, "vlan",
> + vlan_json_array ? vlan_json_array : vlan_json);
> + }
> +
> + 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));
> + if (ovs_list_size(&port->p_ppvids) > 1) {
> + if (ppvid_json_array == NULL) {
> + ppvid_json_array = json_array_create_empty();
> + }
> + json_array_add(ppvid_json_array, ppvid_json);
> + }
> + }
> + if (ppvid_json_array || ppvid_json) {
> + json_object_put(interface_item_json, "ppvid",
> + ppvid_json_array ? ppvid_json_array : ppvid_json);
> + }
> +
> + 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;
pi_ds can be moved above the pi_json.
> + ds_put_buffer(&pi_ds, pid->p_pi, pid->p_pi_len);
> + json_object_put(ppvid_json, "ppvid",
> + json_string_create(ds_cstr_ro(&pi_ds)));
> + ds_destroy(&pi_ds);
> + if (ovs_list_size(&port->p_pids) > 1) {
> + if (pi_json_array == NULL) {
> + pi_json_array = json_array_create_empty();
> + }
> + json_array_add(pi_json_array, pi_json);
> + }
> + }
> + }
> + if (pi_json_array || pi_json) {
> + json_object_put(interface_item_json, "pi",
> + pi_json_array ? pi_json_array : pi_json);
> + }
> +}
> +
> +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);
> + }
> +
> + long int autoneg_support, autoneg_enabled, autoneg_advertised, mautype;
> + autoneg_support = port->p_macphy.autoneg_support;
> + autoneg_enabled = port->p_macphy.autoneg_enabled;
> + autoneg_advertised = port->p_macphy.autoneg_advertised;
> + mautype = port->p_macphy.mau_type;
I suggest something like:
uint16_t autoneg_advertised = port->p_macphy.autoneg_advertised,
autoneg_support = port->p_macphy.autoneg_support,
autoneg_enabled = port->p_macphy.autoneg_enabled,
mautype = port->p_macphytype.mau_type;
To keep the declaration and assignments together. This can be moved
above the 'if (port->p_mfs)' check.
> + if (autoneg_support > 0 || autoneg_enabled > 0 || mautype > 0) {
That also means we can just test:
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");
> + 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,
> + "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%ld\n", "MAU oper type:", mautype);
> + }
> +
> + if (port->p_power.devicetype) {
> + ds_put_format(ds,
> + " 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));
> + }
> +
> + long int autoneg_support, autoneg_enabled, autoneg_advertised, mautype;
> + autoneg_support = port->p_macphy.autoneg_support;
> + autoneg_enabled = port->p_macphy.autoneg_enabled;
> + autoneg_advertised = port->p_macphy.autoneg_advertised;
> + mautype = port->p_macphy.mau_type;
Same comments as before.
> + if (autoneg_support > 0 || autoneg_enabled > 0 || mautype > 0) {
> + struct json *auto_nego_json = json_object_create();
> + json_object_put(auto_nego_json, "supported",
> + json_boolean_create(autoneg_support));
> + json_object_put(auto_nego_json, "enabled",
> + json_boolean_create(autoneg_enabled));
> +
> + struct json *advertised_json = NULL;
Please move this with the auto_nego_json declaration and put whitespace
between the declarations and the rest of the control blocks.
> + 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)
> {
> @@ -426,6 +736,9 @@ lldp_print_neighbor(struct ds *ds, struct lldp *lldp)
> OVS_REQUIRES(mutex)
> ds_put_format(ds, " %-20s%s\n", "PortDescr:",
> strlen(port->p_descr) ? port->p_descr : none_str);
> ds_put_format(ds, " %-20s%d\n", "TTL:",
> port->p_chassis->c_ttl);
> +
> + lldp_print_neighbor_port_dot3(ds, port);
> + lldp_print_neighbor_port_dot1(ds, port);
> }
> }
> }
> @@ -601,6 +914,9 @@ lldp_print_neighbor_json(struct json *interface_json,
> struct lldp *lldp)
> json_object_put(port_json, "ttl",
> json_integer_create(port->p_chassis->c_ttl));
>
> + lldp_print_neighbor_port_dot1_json(interface_item_json, port);
> + lldp_print_neighbor_port_dot3_json(port_json, port);
> +
> json_object_put(interface_item_json, "chassis", chassis_json);
> json_object_put(interface_item_json, "port", port_json);
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
