[ovs-dev] [PATCH v3] ofp-monitor: Extend Flow Monitoring support for OF 1.0-1.2 with Nicira Extensions

Vasu Dasari vdasari at gmail.com
Mon Jun 14 03:49:31 UTC 2021


Currently OVS supports flow-monitoring for OpenFlow 1.0 and Nicira Extenstions.
Any other OpenFlow versioned messages are not accepted. This checkin will allow
OpenFlow1.0-1.2 Flow Monitoring wth Nicira extensions be accepted. Also made
sure that flow-monitoring updates, flow monitoring pause messages, resume
messages are sent in the same OpenFlow version as that of flow-monitor request.

Description of changes:

1. Generate ofp-msgs.inc to be able to support 1.0-1.2 Flow Monitoring messages.
include/openvswitch/ofp-msgs.h

2. Support vconn to accept user specified version and use it for vconn
flow-monitoring session
ofproto/ofproto.c

3. Modify APIs to use protocol as an argument to encode and decode messages
include/openvswitch/ofp-monitor.h
lib/ofp-monitor.c
ofproto/connmgr.c
ofproto/connmgr.h
ofproto/ofproto.c

4. Modified following testcases to be verified across supported OF Versions
    ofproto - flow monitoring
    ofproto - flow monitoring with !own
    ofproto - flow monitoring with out_port
    ofproto - flow monitoring pause and resume
    ofproto - flow monitoring usable protocols
tests/ofproto.at

5. Updated NEWS with the support added with this commit

Signed-off-by: Vasu Dasari <vdasari at gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-December/050820.html
---

v1:
 - Addressed code review comments from Ben Pfaff:
  1. Reduced supported versions from OF 1.0+ to 1.0-1.3 as there is flow
     monitoring is supported in OF 1.4+. Need to make changes as part of future
     development to add support for OpenFlow 1.4+
  2. Added announcement of this support to NEWS
  3. Extended test cases identified in commit to be tested agains all supported
     OF versions.
v2:
 - Fix 0-day robot error in NEWS file
v3:
 - Addressed code review comments
 - Reduced OF versions supported to (1.0-1.2)
 - Made all flow monitoring tests to be tested against all openflow versions
 - Added an option to dump openflow packets in hex for add-flow/del-flow/mod-flow.
   The option to dump bytes is available for "dump" commands but not for the flow
   management commands. Using this option to be able to generate packets dynamically
   during the flow monitoring tests
---
 AUTHORS.rst                       |   2 +-
 NEWS                              |   3 +
 include/openvswitch/ofp-monitor.h |   9 +-
 include/openvswitch/ofp-msgs.h    |   4 +-
 lib/ofp-monitor.c                 |  20 ++--
 ofproto/connmgr.c                 |  18 +++-
 ofproto/connmgr.h                 |   3 +-
 ofproto/ofproto.c                 |  13 ++-
 tests/ofproto.at                  | 172 +++++++++++++++++-------------
 utilities/ovs-ofctl.c             |  22 +++-
 10 files changed, 160 insertions(+), 106 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index dbc3bde44..53c749da7 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -405,6 +405,7 @@ Tony van der Peet                  tony.vanderpeet at alliedtelesis.co.nz
 Tonghao Zhang                      xiangxia.m.yue at gmail.com
 Usman Ansari                       ua1422 at gmail.com
 Valient Gough                      vgough at pobox.com
+Vasu Dasari                        vdasari at gmail.com
 Venkata Anil Kommaddi              vkommadi at redhat.com
 Vishal Deep Ajmera                 vishal.deep.ajmera at ericsson.com
 Vivien Bernet-Rollande             vbr at soprive.net
@@ -674,7 +675,6 @@ Tulio Ribeiro                   tribeiro at lasige.di.fc.ul.pt
 Tytus Kurek                     Tytus.Kurek at pega.com
 Valentin Bud                    valentin at hackaserver.com
 Vasiliy Tolstov                 v.tolstov at selfip.ru
-Vasu Dasari                     vdasari at gmail.com
 Vinllen Chen                    cvinllen at gmail.com
 Vishal Swarankar                vishal.swarnkar at gmail.com
 Vjekoslav Brajkovic             balkan at cs.washington.edu
diff --git a/NEWS b/NEWS
index ebba17b22..09db14511 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,9 @@ Post-v2.15.0
        monitors, etc.  More datails in Documentation/topics/record-replay.rst.
    - In ovs-vsctl and vtep-ctl, the "find" command now accept new
      operators {in} and {not-in}.
+   - OpenFlow:
+     * Extend Flow Monitoring support for OpenFlow 1.0-1.2 with Nicira
+       Extensions
    - Userspace datapath:
      * Auto load balancing of PMDs now partially supports cross-NUMA polling
        cases, e.g if all PMD threads are running on the same NUMA node.
diff --git a/include/openvswitch/ofp-monitor.h b/include/openvswitch/ofp-monitor.h
index 237cef85e..835efd0f3 100644
--- a/include/openvswitch/ofp-monitor.h
+++ b/include/openvswitch/ofp-monitor.h
@@ -70,7 +70,8 @@ struct ofputil_flow_monitor_request {
 int ofputil_decode_flow_monitor_request(struct ofputil_flow_monitor_request *,
                                         struct ofpbuf *msg);
 void ofputil_append_flow_monitor_request(
-    const struct ofputil_flow_monitor_request *, struct ofpbuf *msg);
+    const struct ofputil_flow_monitor_request *, struct ofpbuf *msg,
+    enum ofputil_protocol protocol);
 void ofputil_flow_monitor_request_format(
     struct ds *, const struct ofputil_flow_monitor_request *,
     const struct ofputil_port_map *, const struct ofputil_table_map *);
@@ -103,7 +104,8 @@ struct ofputil_flow_update {
 
 int ofputil_decode_flow_update(struct ofputil_flow_update *,
                                struct ofpbuf *msg, struct ofpbuf *ofpacts);
-void ofputil_start_flow_update(struct ovs_list *replies);
+void ofputil_start_flow_update(struct ovs_list *replies,
+                               enum ofputil_protocol protocol);
 void ofputil_append_flow_update(const struct ofputil_flow_update *,
                                 struct ovs_list *replies,
                                 const struct tun_table *);
@@ -114,7 +116,8 @@ void ofputil_flow_update_format(struct ds *,
 
 /* Abstract nx_flow_monitor_cancel. */
 uint32_t ofputil_decode_flow_monitor_cancel(const struct ofp_header *);
-struct ofpbuf *ofputil_encode_flow_monitor_cancel(uint32_t id);
+struct ofpbuf *ofputil_encode_flow_monitor_cancel(
+    uint32_t id, enum ofputil_protocol protocol);
 
 struct ofputil_requestforward {
     ovs_be32 xid;
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 8457bc7d0..c5fde0270 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -453,12 +453,12 @@ enum ofpraw {
 
     /* OFPST 1.4+ (16): uint8_t[8][]. */
     OFPRAW_OFPST14_FLOW_MONITOR_REQUEST,
-    /* NXST 1.0 (2): uint8_t[8][]. */
+    /* NXST 1.0-1.2 (2): uint8_t[8][]. */
     OFPRAW_NXST_FLOW_MONITOR_REQUEST,
 
     /* OFPST 1.4+ (16): uint8_t[8][]. */
     OFPRAW_OFPST14_FLOW_MONITOR_REPLY,
-    /* NXST 1.0 (2): uint8_t[8][]. */
+    /* NXST 1.0-1.2 (2): uint8_t[8][]. */
     OFPRAW_NXST_FLOW_MONITOR_REPLY,
 
 /* Nicira extension messages.
diff --git a/lib/ofp-monitor.c b/lib/ofp-monitor.c
index e12fa6d2b..51f01b100 100644
--- a/lib/ofp-monitor.c
+++ b/lib/ofp-monitor.c
@@ -386,14 +386,16 @@ ofputil_decode_flow_monitor_request(struct ofputil_flow_monitor_request *rq,
 
 void
 ofputil_append_flow_monitor_request(
-    const struct ofputil_flow_monitor_request *rq, struct ofpbuf *msg)
+    const struct ofputil_flow_monitor_request *rq, struct ofpbuf *msg,
+    enum ofputil_protocol protocol)
 {
     struct nx_flow_monitor_request *nfmr;
     size_t start_ofs;
     int match_len;
+    enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
 
     if (!msg->size) {
-        ofpraw_put(OFPRAW_NXST_FLOW_MONITOR_REQUEST, OFP10_VERSION, msg);
+        ofpraw_put(OFPRAW_NXST_FLOW_MONITOR_REQUEST, version, msg);
     }
 
     start_ofs = msg->size;
@@ -517,9 +519,6 @@ parse_flow_monitor_request__(struct ofputil_flow_monitor_request *fmr,
         if (error) {
             return error;
         }
-        /* Flow Monitor is supported in OpenFlow 1.0 or can be further reduced
-         * to a few 1.0 flavors by a match field. */
-        *usable_protocols &= OFPUTIL_P_OF10_ANY;
     }
     return NULL;
 }
@@ -661,23 +660,26 @@ ofputil_decode_flow_monitor_cancel(const struct ofp_header *oh)
 }
 
 struct ofpbuf *
-ofputil_encode_flow_monitor_cancel(uint32_t id)
+ofputil_encode_flow_monitor_cancel(uint32_t id, enum ofputil_protocol protocol)
 {
     struct nx_flow_monitor_cancel *nfmc;
+    enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
     struct ofpbuf *msg;
 
-    msg = ofpraw_alloc(OFPRAW_NXT_FLOW_MONITOR_CANCEL, OFP10_VERSION, 0);
+    msg = ofpraw_alloc(OFPRAW_NXT_FLOW_MONITOR_CANCEL, version, 0);
     nfmc = ofpbuf_put_uninit(msg, sizeof *nfmc);
     nfmc->id = htonl(id);
     return msg;
 }
 
 void
-ofputil_start_flow_update(struct ovs_list *replies)
+ofputil_start_flow_update(struct ovs_list *replies,
+                          enum ofputil_protocol protocol)
 {
     struct ofpbuf *msg;
+    enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
 
-    msg = ofpraw_alloc_xid(OFPRAW_NXST_FLOW_MONITOR_REPLY, OFP10_VERSION,
+    msg = ofpraw_alloc_xid(OFPRAW_NXST_FLOW_MONITOR_REPLY, version,
                            htonl(0), 1024);
 
     ovs_list_init(replies);
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index fa8f6cd0e..c14834f84 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2193,7 +2193,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
 
         if (flags) {
             if (ovs_list_is_empty(&ofconn->updates)) {
-                ofputil_start_flow_update(&ofconn->updates);
+                ofputil_start_flow_update(&ofconn->updates,
+                                          ofconn_get_protocol(ofconn));
                 ofconn->sent_abbrev_update = false;
             }
 
@@ -2243,6 +2244,7 @@ ofmonitor_flush(struct connmgr *mgr)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct ofconn *ofconn;
+    enum ofputil_protocol protocol;
 
     if (!mgr) {
         return;
@@ -2260,8 +2262,10 @@ ofmonitor_flush(struct connmgr *mgr)
             && rconn_packet_counter_n_bytes(counter) > 128 * 1024) {
             COVERAGE_INC(ofmonitor_pause);
             ofconn->monitor_paused = monitor_seqno++;
+            protocol = ofconn_get_protocol(ofconn);
             struct ofpbuf *pause = ofpraw_alloc_xid(
-                OFPRAW_NXT_FLOW_MONITOR_PAUSED, OFP10_VERSION, htonl(0), 0);
+                OFPRAW_NXT_FLOW_MONITOR_PAUSED,
+                ofputil_protocol_to_ofp_version(protocol), htonl(0), 0);
             ofconn_send(ofconn, pause, counter);
         }
     }
@@ -2271,6 +2275,7 @@ static void
 ofmonitor_resume(struct ofconn *ofconn)
     OVS_REQUIRES(ofproto_mutex)
 {
+    enum ofputil_protocol protocol;
     struct rule_collection rules;
     rule_collection_init(&rules);
 
@@ -2280,10 +2285,13 @@ ofmonitor_resume(struct ofconn *ofconn)
     }
 
     struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
-    ofmonitor_compose_refresh_updates(&rules, &msgs);
+    ofmonitor_compose_refresh_updates(&rules, &msgs,
+                                      ofconn_get_protocol(ofconn));
 
-    struct ofpbuf *resumed = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_RESUMED,
-                                              OFP10_VERSION, htonl(0), 0);
+    protocol = ofconn_get_protocol(ofconn);
+    struct ofpbuf *resumed = ofpraw_alloc_xid(
+            OFPRAW_NXT_FLOW_MONITOR_RESUMED,
+            ofputil_protocol_to_ofp_version(protocol), htonl(0), 0);
     ovs_list_push_back(&msgs, &resumed->list_node);
     ofconn_send_replies(ofconn, &msgs);
 
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index e299386c7..56fdc3504 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -199,7 +199,8 @@ void ofmonitor_collect_resume_rules(struct ofmonitor *, uint64_t seqno,
                                     struct rule_collection *)
     OVS_REQUIRES(ofproto_mutex);
 void ofmonitor_compose_refresh_updates(struct rule_collection *rules,
-                                       struct ovs_list *msgs)
+                                       struct ovs_list *msgs,
+                                       enum ofputil_protocol protocol)
     OVS_REQUIRES(ofproto_mutex);
 
 void connmgr_send_table_status(struct connmgr *,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 80ec2d9ac..5ef3b26b6 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6351,7 +6351,8 @@ static void
 ofproto_compose_flow_refresh_update(const struct rule *rule,
                                     enum nx_flow_monitor_flags flags,
                                     struct ovs_list *msgs,
-                                    const struct tun_table *tun_table)
+                                    const struct tun_table *tun_table,
+                                    enum ofputil_protocol protocol)
     OVS_REQUIRES(ofproto_mutex)
 {
     const struct rule_actions *actions;
@@ -6374,14 +6375,15 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
     fu.ofpacts_len = actions ? actions->ofpacts_len : 0;
 
     if (ovs_list_is_empty(msgs)) {
-        ofputil_start_flow_update(msgs);
+        ofputil_start_flow_update(msgs, protocol);
     }
     ofputil_append_flow_update(&fu, msgs, tun_table);
 }
 
 void
 ofmonitor_compose_refresh_updates(struct rule_collection *rules,
-                                  struct ovs_list *msgs)
+                                  struct ovs_list *msgs,
+                                  enum ofputil_protocol protocol)
     OVS_REQUIRES(ofproto_mutex)
 {
     struct rule *rule;
@@ -6391,7 +6393,7 @@ ofmonitor_compose_refresh_updates(struct rule_collection *rules,
         rule->monitor_flags = 0;
 
         ofproto_compose_flow_refresh_update(rule, flags, msgs,
-                ofproto_get_tun_tab(rule->ofproto));
+                ofproto_get_tun_tab(rule->ofproto), protocol);
     }
 }
 
@@ -6555,7 +6557,8 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ovs_list *msgs)
 
     struct ovs_list replies;
     ofpmp_init(&replies, ofpbuf_from_list(ovs_list_back(msgs))->header);
-    ofmonitor_compose_refresh_updates(&rules, &replies);
+    ofmonitor_compose_refresh_updates(&rules, &replies,
+                                      ofconn_get_protocol(ofconn));
     ovs_mutex_unlock(&ofproto_mutex);
 
     rule_collection_destroy(&rules);
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 08c0a20b6..899c0be55 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -4576,20 +4576,26 @@ sys.stdout.write("".join(sorted(buffer)))
 ]
 m4_divert_pop([PREPARE_TESTS])
 
-AT_SETUP([ofproto - flow monitoring])
+dnl Flow monitoring tests verified across all supported protocols
+dnl CHECK_FLOW_MONITORING(label, option, format)
+m4_define([CHECK_FLOW_MONITORING], [
+AT_SETUP([ofproto - flow monitoring - (OpenFlow $1)])
 AT_KEYWORDS([monitor])
 OVS_VSWITCHD_START
 
+# Get packet data to used as part of ofctl/send operation before doing anything else
+send_buf=$(ovs-ofctl -O $2 del-flows br0 table=0 -mmmmmm |  sed 's/0x/0/' | sed -E 's/^(.{8})(.{8})/\112345678/' )
+
 ovs-ofctl add-flow br0 in_port=0,dl_vlan=123,actions=output:1
 
 # Start a monitor watching the flow table and check the initial reply.
-ovs-ofctl monitor br0 watch: --detach --no-chdir --pidfile >monitor.log 2>&1
+ovs-ofctl -O $2 monitor br0 watch: --detach --no-chdir --pidfile >monitor.log 2>&1
 AT_CAPTURE_FILE([monitor.log])
 ovs-appctl -t ovs-ofctl ofctl/barrier
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
-  [NXST_FLOW_MONITOR reply:
+  [NXST_FLOW_MONITOR reply$3:
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:1
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 # Add, delete, and modify some flows and check the updates.
@@ -4620,59 +4626,59 @@ ovs-ofctl del-flows br0 dl_vlan=123
 ovs-ofctl del-flows br0
 ovs-appctl -t ovs-ofctl ofctl/barrier
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log | multiline_sort], [0],
-[NXST_FLOW_MONITOR reply (xid=0x0):
+[NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=124 actions=output:2
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:5
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123,dl_vlan_pcp=0 actions=output:6
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123,dl_vlan_pcp=1 actions=output:7
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:8
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=0 actions=output:9
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=1 actions=output:10
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,vlan_tci=0x0000 actions=output:11
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=4095,dl_vlan_pcp=0 actions=output:12
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=4095,dl_vlan_pcp=1 actions=output:13
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=4095 actions=output:14
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=0 actions=output:15
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=1 actions=output:16
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0 actions=output:17
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=0 actions=output:18
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=1 actions=output:19
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=0 actions=output:20
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan_pcp=0 actions=output:21
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan_pcp=1 actions=output:22
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=0 actions=output:23
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:3
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123,dl_vlan_pcp=0 actions=output:3
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123,dl_vlan_pcp=1 actions=output:3
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=MODIFIED table=0 cookie=0x5 in_port=0,dl_vlan=123 actions=output:3
  event=MODIFIED table=0 cookie=0x5 in_port=0,dl_vlan=123,dl_vlan_pcp=0 actions=output:3
  event=MODIFIED table=0 cookie=0x5 in_port=0,dl_vlan=123,dl_vlan_pcp=1 actions=output:3
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=DELETED reason=delete table=0 cookie=0x5 in_port=0,dl_vlan=123 actions=output:3
  event=DELETED reason=delete table=0 cookie=0x5 in_port=0,dl_vlan=123,dl_vlan_pcp=0 actions=output:3
  event=DELETED reason=delete table=0 cookie=0x5 in_port=0,dl_vlan=123,dl_vlan_pcp=1 actions=output:3
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=DELETED reason=delete table=0 cookie=0 in_port=0 actions=output:23
  event=DELETED reason=delete table=0 cookie=0 in_port=0,dl_vlan=0 actions=output:20
  event=DELETED reason=delete table=0 cookie=0 in_port=0,dl_vlan=0,dl_vlan_pcp=0 actions=output:18
@@ -4684,7 +4690,7 @@ NXST_FLOW_MONITOR reply (xid=0x0):
  event=DELETED reason=delete table=0 cookie=0 in_port=0,dl_vlan_pcp=0 actions=output:21
  event=DELETED reason=delete table=0 cookie=0 in_port=0,dl_vlan_pcp=1 actions=output:22
  event=DELETED reason=delete table=0 cookie=0 in_port=0,vlan_tci=0x0000 actions=output:11
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 # Check that our own changes are reported as full updates.
@@ -4692,41 +4698,44 @@ ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
 ovs-ofctl add-flow br0 in_port=1,actions=output:2
 ovs-ofctl add-flow br0 in_port=2,actions=output:1
 ovs-appctl -t ovs-ofctl ofctl/barrier
-ovs-appctl -t ovs-ofctl ofctl/send 010e004812345678003fffff00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003000000000000ffffffffffff0000
+ovs-appctl -t ovs-ofctl ofctl/send $send_buf
 ovs-appctl -t ovs-ofctl ofctl/barrier
 AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip], [0], [NXST_FLOW reply:
 ])
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log | multiline_sort], [0],
-[NXST_FLOW_MONITOR reply (xid=0x0):
+[NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=1 actions=output:2
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=2 actions=output:1
-OFPT_BARRIER_REPLY:
-send: OFPT_FLOW_MOD: DEL priority=0 actions=drop
-NXST_FLOW_MONITOR reply (xid=0x0):
+OFPT_BARRIER_REPLY$3:
+send: OFPT_FLOW_MOD$3: DEL actions=drop
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=DELETED reason=delete table=0 cookie=0 in_port=1 actions=output:2
  event=DELETED reason=delete table=0 cookie=0 in_port=2 actions=output:1
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - flow monitoring with !own])
+AT_SETUP([ofproto - flow monitoring with !own - (OpenFlow $1)])
 AT_KEYWORDS([monitor])
 OVS_VSWITCHD_START
 
+# Get packet data to used as part of ofctl/send operation before doing anything else
+send_buf=$(ovs-ofctl -O $2 del-flows br0 table=0 -mmmmmm |  sed 's/0x/0/' | sed -E 's/^(.{8})(.{8})/\112345678/' )
+
 ovs-ofctl add-flow br0 in_port=0,dl_vlan=123,actions=output:1
 
 # Start a monitor watching the flow table and check the initial reply.
-ovs-ofctl monitor br0 watch:\!own --detach --no-chdir --pidfile >monitor.log 2>&1
+ovs-ofctl -O $2 monitor br0 watch:\!own --detach --no-chdir --pidfile >monitor.log 2>&1
 AT_CAPTURE_FILE([monitor.log])
 ovs-appctl -t ovs-ofctl ofctl/barrier
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
-  [NXST_FLOW_MONITOR reply:
+  [NXST_FLOW_MONITOR reply$3:
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:1
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 # Check that our own changes are reported as abbreviations.
@@ -4734,27 +4743,27 @@ ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
 ovs-ofctl add-flow br0 in_port=1,actions=output:2
 ovs-ofctl add-flow br0 in_port=2,actions=output:1
 ovs-appctl -t ovs-ofctl ofctl/barrier
-ovs-appctl -t ovs-ofctl ofctl/send 010e004812345678003fffff00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003000000000000ffffffffffff0000
+ovs-appctl -t ovs-ofctl ofctl/send $send_buf
 ovs-appctl -t ovs-ofctl ofctl/barrier
 AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip], [0], [NXST_FLOW reply:
 ])
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
-[NXST_FLOW_MONITOR reply (xid=0x0):
+[NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=1 actions=output:2
-NXST_FLOW_MONITOR reply (xid=0x0):
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ADDED table=0 cookie=0 in_port=2 actions=output:1
-OFPT_BARRIER_REPLY:
-send: OFPT_FLOW_MOD: DEL priority=0 actions=drop
-NXST_FLOW_MONITOR reply (xid=0x0):
+OFPT_BARRIER_REPLY$3:
+send: OFPT_FLOW_MOD$3: DEL actions=drop
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=ABBREV xid=0x12345678
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - flow monitoring with out_port])
+AT_SETUP([ofproto - flow monitoring with out_port - (OpenFlow $1)])
 AT_KEYWORDS([monitor])
 OVS_VSWITCHD_START
 
@@ -4763,13 +4772,13 @@ ovs-ofctl add-flow br0 in_port=0,dl_vlan=122,actions=output:1
 ovs-ofctl add-flow br0 in_port=0,dl_vlan=123,actions=output:2
 
 # Start a monitor watching the flow table and check the initial reply.
-ovs-ofctl monitor br0 watch:out_port=2 --detach --no-chdir --pidfile >monitor.log 2>&1
+ovs-ofctl -O $2 monitor br0 watch:out_port=2 --detach --no-chdir --pidfile >monitor.log 2>&1
 AT_CAPTURE_FILE([monitor.log])
 ovs-appctl -t ovs-ofctl ofctl/barrier
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
-  [NXST_FLOW_MONITOR reply:
+  [NXST_FLOW_MONITOR reply$3:
  event=ADDED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:2
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
@@ -4788,25 +4797,25 @@ ovs-ofctl mod-flows br0 dl_vlan=123,actions=output:2
 ovs-appctl -t ovs-ofctl ofctl/barrier
 
 AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
-[NXST_FLOW_MONITOR reply (xid=0x0):
+[NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=122 actions=output:1,output:2
-OFPT_BARRIER_REPLY:
-NXST_FLOW_MONITOR reply (xid=0x0):
+OFPT_BARRIER_REPLY$3:
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:1,output:2
-OFPT_BARRIER_REPLY:
-NXST_FLOW_MONITOR reply (xid=0x0):
+OFPT_BARRIER_REPLY$3:
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=122 actions=output:1
-OFPT_BARRIER_REPLY:
-NXST_FLOW_MONITOR reply (xid=0x0):
+OFPT_BARRIER_REPLY$3:
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
  event=MODIFIED table=0 cookie=0 in_port=0,dl_vlan=123 actions=output:2
-OFPT_BARRIER_REPLY:
+OFPT_BARRIER_REPLY$3:
 ])
 
 OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - flow monitoring pause and resume])
+AT_SETUP([ofproto - flow monitoring pause and resume - (OpenFlow $1)])
 AT_KEYWORDS([monitor])
 
 # The maximum socket receive buffer size is important for this test, which
@@ -4837,7 +4846,7 @@ OVS_VSWITCHD_START
 
 # Start a monitor watching the flow table, then make it block.
 on_exit 'kill `cat ovs-ofctl.pid`'
-ovs-ofctl monitor br0 watch: --detach --no-chdir --pidfile >monitor.log 2>&1
+ovs-ofctl -O $2 monitor br0 watch: --detach --no-chdir --pidfile >monitor.log 2>&1
 AT_CAPTURE_FILE([monitor.log])
 ovs-appctl -t ovs-ofctl ofctl/block
 
@@ -4891,46 +4900,57 @@ AT_CHECK([test $adds = $deletes])
 AT_CHECK([ofctl_strip < monitor.log | sed -n -e '
 /reg1=0x22$/p
 /cookie=0x[[23]]/p
-/NXT_FLOW_MONITOR_PAUSED:/p
-/NXT_FLOW_MONITOR_RESUMED:/p
+/NXT_FLOW_MONITOR_PAUSED$3:/p
+/NXT_FLOW_MONITOR_RESUMED$3:/p
 ' > monitor.log.subset])
 AT_CHECK([grep -v MODIFIED monitor.log.subset], [0], [dnl
  event=ADDED table=0 cookie=0x1 reg1=0x22
-NXT_FLOW_MONITOR_PAUSED:
+NXT_FLOW_MONITOR_PAUSED$3:
  event=DELETED reason=delete table=0 cookie=0x1 reg1=0x22
  event=ADDED table=0 cookie=0x3 in_port=1
-NXT_FLOW_MONITOR_RESUMED:
+NXT_FLOW_MONITOR_RESUMED$3:
 ])
 AT_CHECK([grep -v ADDED monitor.log.subset], [0], [dnl
-NXT_FLOW_MONITOR_PAUSED:
+NXT_FLOW_MONITOR_PAUSED$3:
  event=DELETED reason=delete table=0 cookie=0x1 reg1=0x22
  event=MODIFIED table=0 cookie=0x2 in_port=2 actions=output:2
-NXT_FLOW_MONITOR_RESUMED:
+NXT_FLOW_MONITOR_RESUMED$3:
 ])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - flow monitoring usable protocols])
+# Test to show flow monitoring support on different OpenFlow protocols
+AT_SETUP([ofproto - flow monitoring usable protocols (OpenFlow $1)])
 AT_KEYWORDS([monitor])
 
 OVS_VSWITCHD_START
 
 on_exit 'kill `cat ovs-ofctl.pid`'
-ovs-ofctl -OOpenFlow14 monitor br0 watch:udp,udp_dst=8 --detach --no-chdir --pidfile >monitor.log 2>&1
+ovs-ofctl -O $2 monitor br0 watch:udp,udp_dst=8 --detach --no-chdir --pidfile >monitor.log 2>&1
 AT_CAPTURE_FILE([monitor.log])
 
-# ovs-ofctl should exit because monitor is not supported in OpenFlow 1.4
-OVS_WAIT_UNTIL([grep "ovs-ofctl: none of the usable flow formats (OpenFlow10,NXM) is among the allowed flow formats (OXM-OpenFlow14)" monitor.log])
+# Wait till reply comes backs with OF Version
+OVS_WAIT_UNTIL([grep "NXST_FLOW_MONITOR reply$3" monitor.log])
+ovs-appctl -t ovs-ofctl exit
 
-# check that only NXM flag is returned as usable protocols for sctp_dst
-# and ovs-ofctl should exit since monitor is not supported in OpenFlow 1.4
-ovs-ofctl -OOpenFlow14 monitor br0 watch:sctp,sctp_dst=9 --detach --no-chdir --pidfile >monitor.log 2>&1
-OVS_WAIT_UNTIL([grep "ovs-ofctl: none of the usable flow formats (NXM) is among the allowed flow formats (OXM-OpenFlow14)" monitor.log])
+# Make sure protocol type in messages from vswitchd, matches that of requested protocol
+ovs-ofctl -O $2 monitor br0 watch:sctp,sctp_dst=9 --detach --no-chdir --pidfile >monitor.log 2>&1
+ovs-ofctl add-flow br0 sctp,sctp_dst=9,action=normal
 
+OVS_WAIT_UNTIL([grep "event=ADDED " monitor.log])
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
+NXST_FLOW_MONITOR reply$3:
+NXST_FLOW_MONITOR reply$3 (xid=0x0):
+ event=ADDED table=0 cookie=0 sctp,tp_dst=9 actions=NORMAL
+])
+OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
-
+])
+CHECK_FLOW_MONITORING([1.0], [OpenFlow10], [])
+CHECK_FLOW_MONITORING([1.1], [OpenFlow11], [ (OF1.1)])
+CHECK_FLOW_MONITORING([1.2], [OpenFlow12], [ (OF1.2)])
 
 AT_SETUP([ofproto - event filtering (OpenFlow 1.3)])
 AT_KEYWORDS([monitor])
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index ede7f1e61..614b73006 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1740,6 +1740,7 @@ ofctl_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
 {
     enum ofputil_protocol protocol;
     struct vconn *vconn;
+    struct ds ds = DS_EMPTY_INITIALIZER;
     size_t i;
 
     if (bundle) {
@@ -1751,11 +1752,23 @@ ofctl_flow_mod__(const char *remote, struct ofputil_flow_mod *fms,
 
     for (i = 0; i < n_fms; i++) {
         struct ofputil_flow_mod *fm = &fms[i];
-
-        transact_noreply(vconn, ofputil_encode_flow_mod(fm, protocol));
+        struct ofpbuf *buf = ofputil_encode_flow_mod(fm, protocol);
+
+        /* If user has optod for verbosity of 5 or more dump the
+         * constructed OpenFlow packet in hex format */
+        if (verbosity == 5) {
+            ds_put_hex_dump(&ds, buf->data, buf->size, 0, true);
+        } else if (verbosity > 5) {
+            ds_put_hex(&ds, buf->data, buf->size);
+            ds_put_char(&ds, '\n');
+        }
+        transact_noreply(vconn, buf);
         free(CONST_CAST(struct ofpact *, fm->ofpacts));
         minimatch_destroy(&fm->match);
     }
+    fputs(ds_cstr(&ds), stdout);
+    ds_destroy(&ds);
+
     vconn_close(vconn);
 }
 
@@ -2239,6 +2252,7 @@ ofctl_monitor(struct ovs_cmdl_context *ctx)
 {
     struct vconn *vconn;
     int i;
+    enum ofputil_protocol protocol;
     enum ofputil_protocol usable_protocols;
 
     /* If the user wants the invalid_ttl_to_controller feature, limit the
@@ -2263,7 +2277,7 @@ ofctl_monitor(struct ovs_cmdl_context *ctx)
         }
     }
 
-    open_vconn(ctx->argv[1], &vconn);
+    protocol = open_vconn(ctx->argv[1], &vconn);
     bool resume_continuations = false;
     for (i = 2; i < ctx->argc; i++) {
         const char *arg = ctx->argv[i];
@@ -2298,7 +2312,7 @@ ofctl_monitor(struct ovs_cmdl_context *ctx)
             }
 
             msg = ofpbuf_new(0);
-            ofputil_append_flow_monitor_request(&fmr, msg);
+            ofputil_append_flow_monitor_request(&fmr, msg, protocol);
             dump_transaction(vconn, msg);
             fflush(stdout);
         } else if (!strcmp(arg, "resume")) {
-- 
2.29.2



More information about the dev mailing list