ofp-print: Decode and print all async config messages the same way.

We have a single function to decode all of these messages, so there's no
reason to do it two different ways for printing.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index f36335b..bedfc94 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2063,108 +2063,42 @@ ofp_async_config_reason_to_string(uint32_t reason,
 
 #define OFP_ASYNC_CONFIG_REASON_BUFSIZE (INT_STRLEN(int) + 1)
 static void
-ofp_print_nxt_set_async_config(struct ds *string,
-                               const struct ofp_header *oh)
+ofp_print_set_async_config(struct ds *string, const struct ofp_header *oh,
+                           enum ofptype type)
 {
-    int i, j;
-    enum ofpraw raw;
-
-    ofpraw_decode(&raw, oh);
-
-    if (raw == OFPRAW_OFPT13_SET_ASYNC ||
-        raw == OFPRAW_NXT_SET_ASYNC_CONFIG ||
-        raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) {
-        const struct nx_async_config *nac = ofpmsg_body(oh);
-
-        for (i = 0; i < 2; i++) {
+    struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
+    struct ofputil_async_cfg ac;
 
-            ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
-
-            ds_put_cstr(string, "       PACKET_IN:");
-            for (j = 0; j < 32; j++) {
-                if (nac->packet_in_mask[i] & htonl(1u << j)) {
-                    char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
-                    const char *reason;
-
-                    reason = ofputil_packet_in_reason_to_string(j, reasonbuf,
-                                                            sizeof reasonbuf);
-                    ds_put_format(string, " %s", reason);
-                }
-            }
-            if (!nac->packet_in_mask[i]) {
-                ds_put_cstr(string, " (off)");
-            }
-            ds_put_char(string, '\n');
-
-            ds_put_cstr(string, "     PORT_STATUS:");
-            for (j = 0; j < 32; j++) {
-                if (nac->port_status_mask[i] & htonl(1u << j)) {
-                    char reasonbuf[OFP_PORT_REASON_BUFSIZE];
-                    const char *reason;
+    bool is_reply = type == OFPTYPE_GET_ASYNC_REPLY;
+    enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
+                                                        &basis, &ac);
+    if (error) {
+        ofp_print_error(string, error);
+        return;
+    }
 
-                    reason = ofp_port_reason_to_string(j, reasonbuf,
-                                                       sizeof reasonbuf);
-                    ds_put_format(string, " %s", reason);
-                }
-            }
-            if (!nac->port_status_mask[i]) {
-                ds_put_cstr(string, " (off)");
-            }
-            ds_put_char(string, '\n');
+    for (int i = 0; i < 2; i++) {
+        ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
+        for (uint32_t type = 0; type < OAM_N_TYPES; type++) {
+            ds_put_format(string, "%16s:",
+                          ofputil_async_msg_type_to_string(type));
 
-            ds_put_cstr(string, "    FLOW_REMOVED:");
-            for (j = 0; j < 32; j++) {
-                if (nac->flow_removed_mask[i] & htonl(1u << j)) {
-                    char reasonbuf[OFP_FLOW_REMOVED_REASON_BUFSIZE];
+            uint32_t role = i == 0 ? ac.master[type] : ac.slave[type];
+            for (int j = 0; j < 32; j++) {
+                if (role & (1u << j)) {
+                    char reasonbuf[OFP_ASYNC_CONFIG_REASON_BUFSIZE];
                     const char *reason;
 
-                    reason = ofp_flow_removed_reason_to_string(j, reasonbuf,
-                                                           sizeof reasonbuf);
+                    reason = ofp_async_config_reason_to_string(
+                        j, type, reasonbuf, sizeof reasonbuf);
                     ds_put_format(string, " %s", reason);
                 }
             }
-            if (!nac->flow_removed_mask[i]) {
+            if (!role) {
                 ds_put_cstr(string, " (off)");
             }
             ds_put_char(string, '\n');
         }
-    } else if (raw == OFPRAW_OFPT14_SET_ASYNC ||
-               raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) {
-        struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
-        struct ofputil_async_cfg ac;
-
-        bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY;
-        enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
-                                                            &basis, &ac);
-        if (error) {
-            ofp_print_error(string, error);
-            return;
-        }
-
-        for (i = 0; i < 2; i++) {
-            ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave");
-            for (uint32_t type = 0; type < OAM_N_TYPES; type++) {
-                ds_put_format(string, "%16s:",
-                              ofputil_async_msg_type_to_string(type));
-
-                uint32_t role = i == 0 ? ac.master[type] : ac.slave[type];
-                for (j = 0; j < 32; j++) {
-                    if (role & (1u << j)) {
-                        char reasonbuf[OFP_ASYNC_CONFIG_REASON_BUFSIZE];
-                        const char *reason;
-
-                        reason = ofp_async_config_reason_to_string(j, type,
-                                                                   reasonbuf,
-                                                           sizeof reasonbuf);
-                        ds_put_format(string, " %s", reason);
-                    }
-                }
-                if (!role) {
-                    ds_put_cstr(string, " (off)");
-                }
-                ds_put_char(string, '\n');
-            }
-        }
     }
 }
 
@@ -3137,8 +3071,9 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
     const void *msg = oh;
 
     ofp_header_to_string__(oh, raw, string);
-    switch (ofptype_from_ofpraw(raw)) {
 
+    enum ofptype type = ofptype_from_ofpraw(raw);
+    switch (type) {
     case OFPTYPE_GROUP_STATS_REQUEST:
         ofp_print_stats(string, oh);
         ofp_print_ofpst_group_request(string, oh);
@@ -3373,7 +3308,7 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
 
     case OFPTYPE_GET_ASYNC_REPLY:
     case OFPTYPE_SET_ASYNC_CONFIG:
-        ofp_print_nxt_set_async_config(string, oh);
+        ofp_print_set_async_config(string, oh, type);
         break;
     case OFPTYPE_GET_ASYNC_REQUEST:
         break;

diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 6d508d3..c791cb2 100644 (file)
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -2637,20 +2637,30 @@ AT_CLEANUP
 
 AT_SETUP([OFPT_SET_ASYNC - OF1.3])
 AT_KEYWORDS([ofp-print])
+dnl This message has bit 12 set for the PACKET_IN messages (master and slave).
+dnl Those aren't supported bits so they get silently ignored on decoding.
+dnl That seems reasonable because OF1.3 doesn't define any error codes for
+dnl OFPT_SET_ASYNC.
 AT_CHECK([ovs-ofctl ofp-print "\
 04 1c 00 20 00 00 00 00 00 00 10 05 00 00 10 07 \
 00 00 00 03 00 00 00 07 00 00 00 00 00 00 00 03 \
 "], [0], [dnl
 OFPT_SET_ASYNC (OF1.3) (xid=0x0):
  master:
-       PACKET_IN: no_match invalid_ttl 12
+       PACKET_IN: no_match invalid_ttl
      PORT_STATUS: add delete
     FLOW_REMOVED: (off)
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 
  slave:
-       PACKET_IN: no_match action invalid_ttl 12
+       PACKET_IN: no_match action invalid_ttl
      PORT_STATUS: add delete modify
     FLOW_REMOVED: idle hard
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 ])
 AT_CLEANUP
 
@@ -2838,6 +2848,8 @@ AT_CLEANUP
 
 AT_SETUP([NXT_SET_ASYNC_CONFIG])
 AT_KEYWORDS([ofp-print])
+dnl This message has bit 12 set for the PACKET_IN messages (master and slave).
+dnl Those aren't supported bits so they get silently ignored on decoding.
 AT_CHECK([ovs-ofctl ofp-print "\
 01 04 00 28 00 00 00 00 00 00 23 20 00 00 00 13 \
 00 00 10 05 00 00 10 07 00 00 00 03 00 00 00 07 \
@@ -2845,14 +2857,20 @@ AT_CHECK([ovs-ofctl ofp-print "\
 "], [0], [dnl
 NXT_SET_ASYNC_CONFIG (xid=0x0):
  master:
-       PACKET_IN: no_match invalid_ttl 12
+       PACKET_IN: no_match invalid_ttl
      PORT_STATUS: add delete
     FLOW_REMOVED: (off)
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 
  slave:
-       PACKET_IN: no_match action invalid_ttl 12
+       PACKET_IN: no_match action invalid_ttl
      PORT_STATUS: add delete modify
     FLOW_REMOVED: idle hard
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 ])
 AT_CLEANUP
 

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 4c2a995..bc1af8f 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2810,11 +2810,17 @@ send: OFPT_SET_ASYNC (OF1.3) (xid=0x2):
        PACKET_IN: (off)
      PORT_STATUS: (off)
     FLOW_REMOVED: (off)
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 
  slave:
        PACKET_IN: no_match
      PORT_STATUS: (off)
     FLOW_REMOVED: (off)
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 dnl
 OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via no_match) data_len=60 (unbuffered)
 tcp,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0

diff --git a/tests/ofproto.at b/tests/ofproto.at
index ab4d254..61a6be5 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -3845,11 +3845,17 @@ OFPT_GET_ASYNC_REPLY (OF1.3):
        PACKET_IN: no_match action
      PORT_STATUS: add delete modify
     FLOW_REMOVED: idle hard delete
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 
  slave:
        PACKET_IN: (off)
      PORT_STATUS: add delete modify
     FLOW_REMOVED: (off)
+     ROLE_STATUS: (off)
+    TABLE_STATUS: (off)
+  REQUESTFORWARD: (off)
 OFPT_BARRIER_REPLY (OF1.3):
 ])