ofp-print: Make port "config" and "state" output easier to read.

This changes the output of "ovs-ofctl show" from printing ports like this:
     1(eth1): addr:50:54:00:00:00:02, config: 0x11, state:0x1
to this:
     1(eth1): addr:50:54:00:00:00:02
         config:     PORT_DOWN NO_FLOOD
         state:      LINK_DOWN
which seems much easier to read.

It also eliminates trailing white space from the output.

Reported-by: Ethan Jackson <ethan@nicira.com>

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index cb361a6..0265f30 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -432,47 +432,108 @@ compare_ports(const void *a_, const void *b_)
     return ap < bp ? -1 : ap > bp;
 }
 
-static void ofp_print_port_features(struct ds *string, uint32_t features)
+struct bit_name {
+    uint32_t bit;
+    const char *name;
+};
+
+static void
+ofp_print_bit_names(struct ds *string, uint32_t bits,
+                    const struct bit_name bit_names[])
 {
-    if (features == 0) {
-        ds_put_cstr(string, "Unsupported\n");
+    int n = 0;
+
+    if (!bits) {
+        ds_put_cstr(string, "0");
         return;
     }
-    if (features & OFPPF_10MB_HD) {
-        ds_put_cstr(string, "10MB-HD ");
-    }
-    if (features & OFPPF_10MB_FD) {
-        ds_put_cstr(string, "10MB-FD ");
-    }
-    if (features & OFPPF_100MB_HD) {
-        ds_put_cstr(string, "100MB-HD ");
-    }
-    if (features & OFPPF_100MB_FD) {
-        ds_put_cstr(string, "100MB-FD ");
-    }
-    if (features & OFPPF_1GB_HD) {
-        ds_put_cstr(string, "1GB-HD ");
-    }
-    if (features & OFPPF_1GB_FD) {
-        ds_put_cstr(string, "1GB-FD ");
-    }
-    if (features & OFPPF_10GB_FD) {
-        ds_put_cstr(string, "10GB-FD ");
-    }
-    if (features & OFPPF_COPPER) {
-        ds_put_cstr(string, "COPPER ");
-    }
-    if (features & OFPPF_FIBER) {
-        ds_put_cstr(string, "FIBER ");
-    }
-    if (features & OFPPF_AUTONEG) {
-        ds_put_cstr(string, "AUTO_NEG ");
+
+    for (; bits && bit_names->name; bit_names++) {
+        if (bits & bit_names->bit) {
+            if (n++) {
+                ds_put_char(string, ' ');
+            }
+            ds_put_cstr(string, bit_names->name);
+            bits &= ~bit_names->bit;
+        }
     }
-    if (features & OFPPF_PAUSE) {
-        ds_put_cstr(string, "AUTO_PAUSE ");
+
+    if (bits) {
+        if (n++) {
+            ds_put_char(string, ' ');
+        }
+        ds_put_format(string, "0x%"PRIx32, bits);
     }
-    if (features & OFPPF_PAUSE_ASYM) {
-        ds_put_cstr(string, "AUTO_PAUSE_ASYM ");
+}
+
+static void
+ofp_print_port_features(struct ds *string, uint32_t features)
+{
+    static const struct bit_name feature_bits[] = {
+        { OFPPF_10MB_HD,    "10MB-HD" },
+        { OFPPF_10MB_FD,    "10MB-FD" },
+        { OFPPF_100MB_HD,   "100MB-HD" },
+        { OFPPF_100MB_FD,   "100MB-FD" },
+        { OFPPF_1GB_HD,     "1GB-HD" },
+        { OFPPF_1GB_FD,     "1GB-FD" },
+        { OFPPF_10GB_FD,    "10GB-FD" },
+        { OFPPF_COPPER,     "COPPER" },
+        { OFPPF_FIBER,      "FIBER" },
+        { OFPPF_AUTONEG,    "AUTO_NEG" },
+        { OFPPF_PAUSE,      "AUTO_PAUSE" },
+        { OFPPF_PAUSE_ASYM, "AUTO_PAUSE_ASYM" },
+        { 0,                NULL },
+    };
+
+    ofp_print_bit_names(string, features, feature_bits);
+    ds_put_char(string, '\n');
+}
+
+static void
+ofp_print_port_config(struct ds *string, uint32_t config)
+{
+    static const struct bit_name config_bits[] = {
+        { OFPPC_PORT_DOWN,    "PORT_DOWN" },
+        { OFPPC_NO_STP,       "NO_STP" },
+        { OFPPC_NO_RECV,      "NO_RECV" },
+        { OFPPC_NO_RECV_STP,  "NO_RECV_STP" },
+        { OFPPC_NO_FLOOD,     "NO_FLOOD" },
+        { OFPPC_NO_FWD,       "NO_FWD" },
+        { OFPPC_NO_PACKET_IN, "NO_PACKET_IN" },
+        { 0,                  NULL },
+    };
+
+    ofp_print_bit_names(string, config, config_bits);
+    ds_put_char(string, '\n');
+}
+
+static void
+ofp_print_port_state(struct ds *string, uint32_t state)
+{
+    static const struct bit_name state_bits[] = {
+        { OFPPS_LINK_DOWN, "LINK_DOWN" },
+        { 0,               NULL },
+    };
+    uint32_t stp_state;
+
+    /* The STP state is a 2-bit field so it doesn't fit in with the bitmask
+     * pattern.  We have to special case it.
+     *
+     * OVS doesn't support STP, so this field will always be 0 if we are
+     * talking to OVS, so we'd always print STP_LISTEN in that case.
+     * Therefore, we don't print anything at all if the value is STP_LISTEN, to
+     * avoid confusing users. */
+    stp_state = state & OFPPS_STP_MASK;
+    if (stp_state) {
+        ds_put_cstr(string, (stp_state == OFPPS_STP_LEARN ? "STP_LEARN"
+                             : stp_state == OFPPS_STP_FORWARD ? "STP_FORWARD"
+                             : "STP_BLOCK"));
+        state &= ~OFPPS_STP_MASK;
+        if (state) {
+            ofp_print_bit_names(string, state, state_bits);
+        }
+    } else {
+        ofp_print_bit_names(string, state, state_bits);
     }
     ds_put_char(string, '\n');
 }
@@ -493,9 +554,15 @@ ofp_print_phy_port(struct ds *string, const struct ofp_phy_port *port)
 
     ds_put_char(string, ' ');
     ofp_print_port_name(string, ntohs(port->port_no));
-    ds_put_format(string, "(%s): addr:"ETH_ADDR_FMT", config: %#x, state:%#x\n",
-            name, ETH_ADDR_ARGS(port->hw_addr), ntohl(port->config),
-            ntohl(port->state));
+    ds_put_format(string, "(%s): addr:"ETH_ADDR_FMT"\n",
+                  name, ETH_ADDR_ARGS(port->hw_addr));
+
+    ds_put_cstr(string, "     config:     ");
+    ofp_print_port_config(string, ntohl(port->config));
+
+    ds_put_cstr(string, "     state:      ");
+    ofp_print_port_state(string, ntohl(port->state));
+
     if (port->curr) {
         ds_put_format(string, "     current:    ");
         ofp_print_port_features(string, ntohl(port->curr));

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 03509b6..c2018c7 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -138,19 +138,27 @@ ff fe 50 54 00 00 00 01 62 72 30 00 00 00 00 00 \
 OFPT_FEATURES_REPLY (xid=0x1): ver:0x1, dpid:0000505400000001
 n_tables:2, n_buffers:256
 features: capabilities:0x87, actions:0xfff
- 1(eth1): addr:50:54:00:00:00:02, config: 0, state:0
-     current:    100MB-FD AUTO_NEG @&t@
-     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
-     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
- 2(eth2): addr:50:54:00:00:00:03, config: 0, state:0
-     current:    100MB-FD AUTO_NEG @&t@
-     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
-     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
- 3(eth0): addr:50:54:00:00:00:01, config: 0, state:0
-     current:    100MB-FD AUTO_NEG @&t@
-     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
-     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
- LOCAL(br0): addr:50:54:00:00:00:01, config: 0x1, state:0x1
+ 1(eth1): addr:50:54:00:00:00:02
+     config:     0
+     state:      0
+     current:    100MB-FD AUTO_NEG
+     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ 2(eth2): addr:50:54:00:00:00:03
+     config:     0
+     state:      0
+     current:    100MB-FD AUTO_NEG
+     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ 3(eth0): addr:50:54:00:00:00:01
+     config:     0
+     state:      0
+     current:    100MB-FD AUTO_NEG
+     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+ LOCAL(br0): addr:50:54:00:00:00:01
+     config:     PORT_DOWN
+     state:      LINK_DOWN
 ])
 AT_CLEANUP
 
@@ -256,10 +264,12 @@ AT_CHECK([ovs-ofctl ofp-print "\
 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 01 \
 00 00 02 08 00 00 02 8f 00 00 02 8f 00 00 00 00 \
 "], [0], [dnl
-OFPT_PORT_STATUS (xid=0x0): MOD: 3(eth0): addr:50:54:00:00:00:01, config: 0x1, state:0x1
-     current:    100MB-FD AUTO_NEG @&t@
-     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
-     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG @&t@
+OFPT_PORT_STATUS (xid=0x0): MOD: 3(eth0): addr:50:54:00:00:00:01
+     config:     PORT_DOWN
+     state:      LINK_DOWN
+     current:    100MB-FD AUTO_NEG
+     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
 ])
 AT_CLEANUP
 

diff --git a/tests/ofproto.at b/tests/ofproto.at
index e6def3e..f299218 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -13,7 +13,9 @@ AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPT_FEATURES_REPLY: ver:0x1, dpid:fedcba9876543210
 n_tables:1, n_buffers:256
 features: capabilities:0x87, actions:0xfff
- LOCAL(br0): addr:aa:55:aa:55:00:00, config: 0x1, state:0x1
+ LOCAL(br0): addr:aa:55:aa:55:00:00
+     config:     PORT_DOWN
+     state:      LINK_DOWN
 OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
 ])
 OFPROTO_STOP
@@ -23,19 +25,21 @@ AT_SETUP([ofproto - mod-port])
 OFPROTO_START
 for command_config_state in \
     'up 0 0' \
-    'noflood 0x10 0' \
-    'down 0x11 0x1' \
-    'flood 0x1 0x1'
+    'noflood NO_FLOOD 0' \
+    'down PORT_DOWN,NO_FLOOD LINK_DOWN' \
+    'flood PORT_DOWN LINK_DOWN'
 do
     set $command_config_state
-    command=$[1] config=$[2] state=$[3]
+    command=$[1] config=`echo $[2] | sed 's/,/ /g'` state=$[3]
     AT_CHECK([ovs-ofctl -vANY:ANY:WARN mod-port br0 br0 $command])
     AT_CHECK([ovs-ofctl -vANY:ANY:WARN show br0], [0], [stdout])
     AT_CHECK_UNQUOTED([STRIP_XIDS stdout], [0], [dnl
 OFPT_FEATURES_REPLY: ver:0x1, dpid:fedcba9876543210
 n_tables:1, n_buffers:256
 features: capabilities:0x87, actions:0xfff
- LOCAL(br0): addr:aa:55:aa:55:00:00, config: $config, state:$state
+ LOCAL(br0): addr:aa:55:aa:55:00:00
+     config:     $config
+     state:      $state
 OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
 ])
 done