Changliang Wu <changliang...@smartx.com> writes:
> New appctl 'lldp/neighbor' displays lldp neighbor information.
> Support json output with --format json --pretty
>
> Signed-off-by: Changliang Wu <changliang...@smartx.com>
> ---
> NEWS | 1 +
> lib/ovs-lldp.c | 356 +++++++++++++++++++++++++++++++++++++
> vswitchd/ovs-vswitchd.8.in | 4 +
> 3 files changed, 361 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index c7bb53f2d..931fe5c71 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -20,6 +20,7 @@ Post-v3.5.0
> core file size.
> - ovs-appctl:
> * Added JSON output support to the 'ovs/route/show' command.
> + * Added 'lldp/neighbor' command that displays lldp neighbor infomation.
> - SSL/TLS:
> * Support for deprecated TLSv1 and TLSv1.1 protocols on OpenFlow and
> database connections is now removed.
> diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
> index 152777248..330cb16ba 100644
> --- a/lib/ovs-lldp.c
> +++ b/lib/ovs-lldp.c
> @@ -36,7 +36,10 @@
> #include <stdlib.h>
> #include "openvswitch/dynamic-string.h"
> #include "flow.h"
> +#include "openvswitch/json.h"
> #include "openvswitch/list.h"
> +#include "lldp/lldp-const.h"
> +#include "lldp/lldp-tlv.h"
> #include "lldp/lldpd.h"
> #include "lldp/lldpd-structs.h"
> #include "netdev.h"
> @@ -310,6 +313,315 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp)
> OVS_REQUIRES(mutex)
> }
> }
>
> +static void
> +lldp_print_neighbor(struct ds *ds, struct lldp *lldp) OVS_REQUIRES(mutex)
> +{
> + struct lldpd_hardware *hw;
> + struct lldpd_port *port;
> + const char *none_str = "<None>";
We try to keep reverse xmas tree arguments where possible. It isn't a
hard rule in the coding style, but it can make the function easier to
read. Please reorder.
> + if (!lldp->lldpd) {
> + return;
> + }
> +
> + bool is_first_interface = true;
> + LIST_FOR_EACH (hw, h_entries, &lldp->lldpd->g_hardware) {
> + if (!hw->h_rports.next) {
> + continue;
> + }
> + LIST_FOR_EACH (port, p_entries, &hw->h_rports) {
> + if (!port->p_chassis) {
> + continue;
> + }
> + if (is_first_interface) {
> + is_first_interface = false;
> + } else {
> + ds_put_format(ds, "\n");
> + }
> + ds_put_format(ds, "Interface: %s\n", lldp->name);
> + ds_put_format(ds, " Chassis:\n");
> +
> + struct ds id = DS_EMPTY_INITIALIZER;
Please call this 'chassis_id'.
> + if (port->p_chassis->c_id_len > 0) {
> + ds_put_hex_with_delimiter(&id, port->p_chassis->c_id,
> + port->p_chassis->c_id_len, ":");
> + }
> + ds_put_format(ds, " %-20s%s\n", "Chassis ID:",
> + id.length ? ds_cstr_ro(&id) : none_str);
> + ds_put_format(ds, " %-20s%s\n", "SysName:",
> + strlen(port->p_chassis->c_name)
> + ? port->p_chassis->c_name
> + : none_str);
> + ds_put_format(ds, " %-20s%s\n", "SysDescr:",
> + strlen(port->p_chassis->c_descr)
> + ? port->p_chassis->c_descr
> + : none_str);
> + ds_destroy(&id);
> +
> + struct lldpd_mgmt *mgmt;
> + LIST_FOR_EACH (mgmt, m_entries, &port->p_chassis->c_mgmt) {
> + struct in6_addr ip;
> + switch (mgmt->m_family) {
> + case LLDPD_AF_IPV4:
> + in6_addr_set_mapped_ipv4(&ip, mgmt->m_addr.inet.s_addr);
> + break;
> +
> + case LLDPD_AF_IPV6: ip = mgmt->m_addr.inet6; break;
Please don't shrink this case onto one line.
> +
> + default: continue;
Please don't shrink this case onto one line.
> + }
> + ds_put_format(ds, " %-20s", "MgmtIP:");
> + ipv6_format_mapped(&ip, ds);
> + ds_put_format(ds, "\n");
> + ds_put_format(ds, " %-20s%d\n",
> + "MgmtIface:", mgmt->m_iface);
> + }
> +
> + if (port->p_chassis->c_cap_available & LLDP_CAP_BRIDGE) {
> + ds_put_format(ds, " %-20sBridge, %s\n","Capability:",
> + port->p_chassis->c_cap_enabled &
> LLDP_CAP_BRIDGE
> + ? "on"
> + : "off");
> + }
> + if (port->p_chassis->c_cap_available & LLDP_CAP_ROUTER) {
> + ds_put_format(ds, " %-20sRouter, %s\n","Capability:",
> + port->p_chassis->c_cap_enabled &
> LLDP_CAP_ROUTER
> + ? "on"
> + : "off");
> + }
> + if (port->p_chassis->c_cap_available & LLDP_CAP_WLAN) {
> + ds_put_format(ds, " %-20sWlan, %s\n","Capability:",
> + port->p_chassis->c_cap_enabled & LLDP_CAP_WLAN
> + ? "on"
> + : "off");
> + }
> + if (port->p_chassis->c_cap_available & LLDP_CAP_STATION) {
> + ds_put_format(ds, " %-20sStation, %s\n","Capability:",
> + port->p_chassis->c_cap_enabled &
> LLDP_CAP_STATION
> + ? "on"
> + : "off");
> + }
I understand that this is how it is printed from lldpd, but this output
really doesn't seem pleasant. LLDPD does print this way, however:
...
Capability: Bridge, off
Capability: Router, off
Capability: Wlan, off
...
So I guess it would be fine to keep as is.
> +
> + ds_put_format(ds, " Port:\n");
> +
> + if (port->p_id_subtype == LLDP_PORTID_SUBTYPE_LLADDR) {
> + ds_init(&id);
Please create a separate lladdr dynamic string.
> + if (port->p_id_len > 0) {
> + ds_put_hex_with_delimiter(&id, (uint8_t *) port->p_id,
> + port->p_id_len, ":");
> + }
> + ds_put_format(ds, " %-20s%s\n", "PortID:",
> + id.length ? ds_cstr_ro(&id) : none_str);
> + ds_destroy(&id);
> + } else {
> + ds_put_format(ds, " %-20s%.*s\n", "PortID:",
> + (int) (port->p_id ? port->p_id_len
> + : strlen(none_str)),
> + port->p_id ?: none_str);
> + }
> + 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);
> + }
> + }
> +}
> +
> +static void
> +lldp_print_neighbor_json(struct json *interface_json, struct lldp *lldp)
> + OVS_REQUIRES(mutex)
> +{
> + struct lldpd_hardware *hw;
> + struct lldpd_port *port;
> + const char *none_str = "<None>";
Same comment.
> + if (!lldp->lldpd) {
> + return;
> + }
> +
> + struct json *interface_item_warp_json_last = NULL;
> + struct json *interface_array_json = NULL;
> + bool has_multi_interfaces = false;
> +
> + LIST_FOR_EACH (hw, h_entries, &lldp->lldpd->g_hardware) {
> + if (!hw->h_rports.next) {
> + continue;
> + }
> + LIST_FOR_EACH (port, p_entries, &hw->h_rports) {
> + if (!port->p_chassis) {
> + continue;
> + }
> +
> + struct json *interface_item_warp_json = json_object_create();
> + struct json *interface_item_json = json_object_create();
> + struct json *chassis_json = json_object_create();
> + struct json *chassis_sys_json = json_object_create();
> + struct json *chassis_id_json = json_object_create();
> + struct json *chassis_mgmt_ip_json = json_array_create_empty();
> + struct json *chassis_mgmt_iface_json = json_array_create_empty();
> + struct json *chassis_capability_json = json_array_create_empty();
> + struct json *chassis_capability_item_json;
> + struct json *port_json = json_object_create();
> + struct json *port_id_json = json_object_create();
> +
> + struct ds id = DS_EMPTY_INITIALIZER;
This 'id' object's declaration could come before the 'if' above.
> + if (port->p_chassis->c_id_len > 0) {
> + ds_put_hex_with_delimiter(&id, port->p_chassis->c_id,
> + port->p_chassis->c_id_len, ":");
> + }
> +
> + json_object_put(chassis_id_json, "type",
> + json_string_create("mac"));
> + json_object_put(chassis_id_json, "value",
> + json_string_create(id.length ? ds_cstr_ro(&id)
> + : none_str));
> + json_object_put(chassis_sys_json, "id", chassis_id_json);
> +
> + json_object_put(chassis_json,
> + strlen(port->p_chassis->c_name)
> + ? port->p_chassis->c_name
> + : none_str,
> + chassis_sys_json);
> +
> + json_object_put(chassis_sys_json, "descr",
> +
> json_string_create(strlen(port->p_chassis->c_descr)
> + ? port->p_chassis->c_descr
> + : none_str));
> +
> + ds_destroy(&id);
> +
> + struct lldpd_mgmt *mgmt;
> + struct in6_addr ip;
> + char addr_str[INET6_ADDRSTRLEN];
'addr_str' and 'ip' can be reduced in scope here (inside the
LIST_FOR_EACH below). 'mgmt' can be declared at the upper
'LIST_FOR_EACH (port...)'. You did this in the previous block, I think.
> + LIST_FOR_EACH (mgmt, m_entries, &port->p_chassis->c_mgmt) {
> + switch (mgmt->m_family) {
> + case LLDPD_AF_IPV4:
> + in6_addr_set_mapped_ipv4(&ip, mgmt->m_addr.inet.s_addr);
> + break;
> +
> + case LLDPD_AF_IPV6: ip = mgmt->m_addr.inet6; break;
> + default: continue;
As with the other block, please don't shrink these onto one line.
> + }
> +
> + ipv6_string_mapped(addr_str, &ip);
> + json_array_add(chassis_mgmt_ip_json,
> + json_string_create(addr_str));
> + json_array_add(chassis_mgmt_iface_json,
> + json_integer_create(mgmt->m_iface));
> + }
> + json_object_put(chassis_sys_json, "mgmt-ip",
> chassis_mgmt_ip_json);
> + json_object_put(chassis_sys_json, "mgmt-iface",
> + chassis_mgmt_iface_json);
> +
> + if (port->p_chassis->c_cap_available & LLDP_CAP_BRIDGE) {
> + chassis_capability_item_json = json_object_create();
> + json_object_put(chassis_capability_item_json, "type",
> + json_string_create("Bridge"));
> + json_object_put(
> + chassis_capability_item_json, "enabled",
> + json_boolean_create(port->p_chassis->c_cap_enabled &
> + LLDP_CAP_BRIDGE));
> + json_array_add(chassis_capability_json,
> + chassis_capability_item_json);
> + }
> + if (port->p_chassis->c_cap_available & LLDP_CAP_ROUTER) {
> + chassis_capability_item_json = json_object_create();
> + json_object_put(chassis_capability_item_json, "type",
> + json_string_create("Router"));
> + json_object_put(
> + chassis_capability_item_json, "enabled",
> + json_boolean_create(port->p_chassis->c_cap_enabled &
> + LLDP_CAP_ROUTER));
> + json_array_add(chassis_capability_json,
> + chassis_capability_item_json);
> + }
> + if (port->p_chassis->c_cap_available & LLDP_CAP_WLAN) {
> + chassis_capability_item_json = json_object_create();
> + json_object_put(chassis_capability_item_json, "type",
> + json_string_create("Wlan"));
> + json_object_put(
> + chassis_capability_item_json, "enabled",
> + json_boolean_create(port->p_chassis->c_cap_enabled &
> + LLDP_CAP_WLAN));
> + json_array_add(chassis_capability_json,
> + chassis_capability_item_json);
> + }
> + if (port->p_chassis->c_cap_available & LLDP_CAP_STATION) {
> + chassis_capability_item_json = json_object_create();
> + json_object_put(chassis_capability_item_json, "type",
> + json_string_create("Station"));
> + json_object_put(
> + chassis_capability_item_json, "enabled",
> + json_boolean_create(port->p_chassis->c_cap_enabled &
> + LLDP_CAP_STATION));
> + json_array_add(chassis_capability_json,
> + chassis_capability_item_json);
> + }
> + json_object_put(chassis_sys_json, "capability",
> + chassis_capability_json);
> +
> + if (port->p_id_subtype == LLDP_PORTID_SUBTYPE_LLADDR) {
> + ds_init(&id);
Please create a separate lladdr dynamic string.
> + if (port->p_id_len > 0) {
> + ds_put_hex_with_delimiter(&id, port->p_id,
> port->p_id_len,
> + ":");
> + }
> + json_object_put(port_id_json, "type",
> + json_string_create("mac"));
> + json_object_put(port_id_json, "value",
> + json_string_create(id.length ?
> ds_cstr_ro(&id)
> + : none_str));
> +
> + ds_destroy(&id);
> + } else {
> + json_object_put(port_id_json, "type",
> + json_string_create("ifname"));
> + if (port->p_id) {
> + struct ds port_ds = DS_EMPTY_INITIALIZER;
> + ds_put_buffer(&port_ds, port->p_id, port->p_id_len);
> + json_object_put(port_id_json, "value",
> +
> json_string_create(ds_cstr_ro(&port_ds)));
> + ds_destroy(&port_ds);
> + } else {
> + json_object_put(port_id_json, "value",
> + json_string_create(none_str));
> + }
> + }
> + json_object_put(port_json, "id", port_id_json);
> +
> + json_object_put(port_json, "desc",
> + json_string_create(strlen(port->p_descr)
> + ? port->p_descr
> + : none_str));
> + json_object_put(port_json, "ttl",
> + json_integer_create(port->p_chassis->c_ttl));
> +
> + json_object_put(interface_item_json, "chassis", chassis_json);
> + json_object_put(interface_item_json, "port", port_json);
> +
> + json_object_put(interface_item_warp_json, lldp->name,
> + interface_item_json);
> +
> + if (has_multi_interfaces) {
> + if (!interface_array_json) {
> + interface_array_json = json_array_create_empty();
> + json_array_add(interface_array_json,
> + interface_item_warp_json_last);
> + }
> + json_array_add(interface_array_json,
> interface_item_warp_json);
> + } else {
> + has_multi_interfaces = true;
> + interface_item_warp_json_last = interface_item_warp_json;
> + }
> + }
> + }
> + if (interface_array_json || interface_item_warp_json_last) {
> + json_object_put(interface_json, "interface",
> + interface_array_json == NULL
> + ? interface_item_warp_json_last
> + : interface_array_json);
> + }
> +}
> +
> static void
> aa_unixctl_status(struct unixctl_conn *conn, int argc OVS_UNUSED,
> const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> @@ -368,6 +680,48 @@ aa_unixctl_statistics(struct unixctl_conn *conn, int
> argc OVS_UNUSED,
> unixctl_command_reply(conn, ds_cstr(&ds));
> }
>
> +static void
> +lldp_unixctl_show_neighbor(struct unixctl_conn *conn, int argc,
> + const char *argv[], void *aux OVS_UNUSED)
> + OVS_EXCLUDED(mutex)
> +{
> + struct lldp *lldp;
> +
> + if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> + struct json *lldp_json = json_object_create();
> + struct json *interface_json = json_object_create();
> +
> + ovs_mutex_lock(&mutex);
> + HMAP_FOR_EACH (lldp, hmap_node, all_lldps) {
> + if (argc > 1 && strcmp(argv[1], lldp->name)) {
> + continue;
> + }
> + lldp_print_neighbor_json(interface_json, lldp);
> + }
> + ovs_mutex_unlock(&mutex);
> +
> + json_object_put(lldp_json, "lldp", interface_json);
> +
> + unixctl_command_reply_json(conn, lldp_json);
> + } else {
> + struct ds ds = DS_EMPTY_INITIALIZER;
> +
> + ds_put_format(&ds, "LLDP neighbor:\n");
> +
> + ovs_mutex_lock(&mutex);
> + HMAP_FOR_EACH (lldp, hmap_node, all_lldps) {
> + if (argc > 1 && strcmp(argv[1], lldp->name)) {
> + continue;
> + }
> + lldp_print_neighbor(&ds, lldp);
> + }
> + ovs_mutex_unlock(&mutex);
> +
> + unixctl_command_reply(conn, ds_cstr(&ds));
> + ds_destroy(&ds);
> + }
> +}
> +
> /* An Auto Attach mapping was configured. Populate the corresponding
> * structures in the LLDP hardware.
> */
> @@ -635,6 +989,8 @@ lldp_init(void)
> aa_unixctl_show_isid, NULL);
> unixctl_command_register("autoattach/statistics", "[bridge]", 0, 1,
> aa_unixctl_statistics, NULL);
> + unixctl_command_register("lldp/neighbor", "[interface]", 0, 1,
> + lldp_unixctl_show_neighbor, NULL);
> }
>
> /* Returns true if 'lldp' should process packets from 'flow'. Sets
> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
> index 98e58951d..51b0a46b5 100644
> --- a/vswitchd/ovs-vswitchd.8.in
> +++ b/vswitchd/ovs-vswitchd.8.in
> @@ -149,6 +149,10 @@ enabled.
> Force the fault status of the CFM module on \fIinterface\fR (or all
> interfaces if none is given) to be \fIstatus\fR. \fIstatus\fR can be
> "true", "false", or "normal" which reverts to the standard behavior.
> +.IP "\fBlldp/neighbor\fR [\fIinterface\fR]"
> +Displays detailed information about LLDP neighbors on \fIinterface\fR.
> +If \fIinterface\fR is not specified, then it displays detailed information
> +about all interfaces with LLDP enabled.
> .IP "\fBstp/tcn\fR [\fIbridge\fR]"
> Forces a topology change event on \fIbridge\fR if it's running STP. This
> may cause it to send Topology Change Notifications to its peers and flush
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
