diff mbox series

[ovs-dev,v2] ofp-monitor: Extend Flow Monitoring support for OF 1.0-1.3 with Nicira Extensions

Message ID 20210514201525.83338-1-vdasari@gmail.com
State Changes Requested
Headers show
Series [ovs-dev,v2] ofp-monitor: Extend Flow Monitoring support for OF 1.0-1.3 with Nicira Extensions | expand

Commit Message

Vasu Dasari May 14, 2021, 8:15 p.m. UTC 
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.3 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+ 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 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@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
---
 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                  | 73 +++++++++++++++++++------------
 utilities/ovs-ofctl.c             |  5 ++-
 10 files changed, 93 insertions(+), 57 deletions(-)

Comments

Ben Pfaff June 11, 2021, 12:25 a.m. UTC | #1 
On Fri, May 14, 2021 at 04:15:25PM -0400, Vasu Dasari wrote:
> 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.3 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

Thanks for the patch!

This change here isn't quite accurate since only OF1.0-OF1.3 are
currently supported:

    @@ -515,13 +517,10 @@ 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;
     }

     /* Convert 'str_' (as described in the documentation for the "monitor" command

Actually, it occurs to me that this shouldn't add support for OF1.3
either, only for OF1.1 and OF1.2, because OF1.3 has a standardized
extension, which you can find here:
https://opennetworking.org/wp-content/uploads/2014/10/openflow-extensions-1.3.x-pack2.zip
in the pdf inside for extension 187.  It would be better to implement
that for 1.3 rather than to invent our own.

Thanks,

Ben.
Vasu Dasari June 14, 2021, 12:03 p.m. UTC | #2 
Hi Ben,

I see that the flow monitoring is added as an ONF extension. I am looking
at ofp-msgs.h, I do not see any ONF_ related extensions added in that file.
If we were to add ONF flow monitoring support to OVS this will be the first
one. Let me know if this is a desired one.

Or, we could continue with the Nicira extension till 1.3 and then follow
with a standardized one from 1.4.

Let me know what you think.

Thanks
-Vasu

*Vasu Dasari*


On Thu, Jun 10, 2021 at 8:25 PM Ben Pfaff <blp@ovn.org> wrote:

> On Fri, May 14, 2021 at 04:15:25PM -0400, Vasu Dasari wrote:
> > 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.3 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
>
> Thanks for the patch!
>
> This change here isn't quite accurate since only OF1.0-OF1.3 are
> currently supported:
>
>     @@ -515,13 +517,10 @@ 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;
>      }
>
>      /* Convert 'str_' (as described in the documentation for the
> "monitor" command
>
> Actually, it occurs to me that this shouldn't add support for OF1.3
> either, only for OF1.1 and OF1.2, because OF1.3 has a standardized
> extension, which you can find here:
>
> https://opennetworking.org/wp-content/uploads/2014/10/openflow-extensions-1.3.x-pack2.zip
> in the pdf inside for extension 187.  It would be better to implement
> that for 1.3 rather than to invent our own.
>
> Thanks,
>
> Ben.
>
Ben Pfaff June 14, 2021, 6:30 p.m. UTC | #3 
There are a number of ONF extensions already:

[bpfaff@sigfpe _build]$ git grep -h -E 'ONFS?T' ../include/openvswitch/ofp-msgs.h
 *          * ONFT: Open Networking Foundation extension message.
 *          * ONFST: Open Networking Foundation multipart message.
 *         For NXT or ONFT, the 'subtype' in struct ofp_vendor_header.
 *         For NXST or ONFST, the 'subtype' in an appropriate vendor stats
    /* ONFT 1.3 (1911): struct ofp14_role_status, uint8_t[8][]. */
    OFPRAW_ONFT13_ROLE_STATUS,
    /* ONFT 1.3 (2350): struct ofp14_requestforward, uint8_t[8][]. */
    OFPRAW_ONFT13_REQUESTFORWARD,
    /* ONFT 1.3 (2300): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
    OFPRAW_ONFT13_BUNDLE_CONTROL,
    /* ONFT 1.3 (2301): struct ofp14_bundle_ctrl_msg, uint8_t[]. */
    OFPRAW_ONFT13_BUNDLE_ADD_MESSAGE,
    OFPTYPE_ROLE_STATUS,          /* OFPRAW_ONFT13_ROLE_STATUS.
                                   * OFPRAW_ONFT13_REQUESTFORWARD.
                                   * OFPRAW_ONFT13_BUNDLE_CONTROL. */
                                   * OFPRAW_ONFT13_BUNDLE_ADD_MESSAGE. */

The ONF standardized extension is the one we should use.

On Mon, Jun 14, 2021 at 08:03:04AM -0400, Vasu Dasari wrote:
> Hi Ben,
> 
> I see that the flow monitoring is added as an ONF extension. I am looking
> at ofp-msgs.h, I do not see any ONF_ related extensions added in that file.
> If we were to add ONF flow monitoring support to OVS this will be the first
> one. Let me know if this is a desired one.
> 
> Or, we could continue with the Nicira extension till 1.3 and then follow
> with a standardized one from 1.4.
> 
> Let me know what you think.
> 
> Thanks
> -Vasu
> 
> *Vasu Dasari*
> 
> 
> On Thu, Jun 10, 2021 at 8:25 PM Ben Pfaff <blp@ovn.org> wrote:
> 
> > On Fri, May 14, 2021 at 04:15:25PM -0400, Vasu Dasari wrote:
> > > 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.3 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
> >
> > Thanks for the patch!
> >
> > This change here isn't quite accurate since only OF1.0-OF1.3 are
> > currently supported:
> >
> >     @@ -515,13 +517,10 @@ 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;
> >      }
> >
> >      /* Convert 'str_' (as described in the documentation for the
> > "monitor" command
> >
> > Actually, it occurs to me that this shouldn't add support for OF1.3
> > either, only for OF1.1 and OF1.2, because OF1.3 has a standardized
> > extension, which you can find here:
> >
> > https://opennetworking.org/wp-content/uploads/2014/10/openflow-extensions-1.3.x-pack2.zip
> > in the pdf inside for extension 187.  It would be better to implement
> > that for 1.3 rather than to invent our own.
> >
> > Thanks,
> >
> > Ben.
> >
Vasu Dasari June 14, 2021, 6:42 p.m. UTC | #4 
Ok Ben. Is it ok if I take this up as a separate submit.

So after this commit, I propose to add the following changes(listing the
tasks):
1. Support for 1.3 ONF extension for flow monitoring
2. Support for 1.4+ flow monitoring.

Thanks,
-Vasu

*Vasu Dasari*


On Mon, Jun 14, 2021 at 2:31 PM Ben Pfaff <blp@ovn.org> wrote:

> There are a number of ONF extensions already:
>
> [bpfaff@sigfpe _build]$ git grep -h -E 'ONFS?T'
> ../include/openvswitch/ofp-msgs.h
>  *          * ONFT: Open Networking Foundation extension message.
>  *          * ONFST: Open Networking Foundation multipart message.
>  *         For NXT or ONFT, the 'subtype' in struct ofp_vendor_header.
>  *         For NXST or ONFST, the 'subtype' in an appropriate vendor stats
>     /* ONFT 1.3 (1911): struct ofp14_role_status, uint8_t[8][]. */
>     OFPRAW_ONFT13_ROLE_STATUS,
>     /* ONFT 1.3 (2350): struct ofp14_requestforward, uint8_t[8][]. */
>     OFPRAW_ONFT13_REQUESTFORWARD,
>     /* ONFT 1.3 (2300): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
>     OFPRAW_ONFT13_BUNDLE_CONTROL,
>     /* ONFT 1.3 (2301): struct ofp14_bundle_ctrl_msg, uint8_t[]. */
>     OFPRAW_ONFT13_BUNDLE_ADD_MESSAGE,
>     OFPTYPE_ROLE_STATUS,          /* OFPRAW_ONFT13_ROLE_STATUS.
>                                    * OFPRAW_ONFT13_REQUESTFORWARD.
>                                    * OFPRAW_ONFT13_BUNDLE_CONTROL. */
>                                    * OFPRAW_ONFT13_BUNDLE_ADD_MESSAGE. */
>
> The ONF standardized extension is the one we should use.
>
> On Mon, Jun 14, 2021 at 08:03:04AM -0400, Vasu Dasari wrote:
> > Hi Ben,
> >
> > I see that the flow monitoring is added as an ONF extension. I am looking
> > at ofp-msgs.h, I do not see any ONF_ related extensions added in that
> file.
> > If we were to add ONF flow monitoring support to OVS this will be the
> first
> > one. Let me know if this is a desired one.
> >
> > Or, we could continue with the Nicira extension till 1.3 and then follow
> > with a standardized one from 1.4.
> >
> > Let me know what you think.
> >
> > Thanks
> > -Vasu
> >
> > *Vasu Dasari*
> >
> >
> > On Thu, Jun 10, 2021 at 8:25 PM Ben Pfaff <blp@ovn.org> wrote:
> >
> > > On Fri, May 14, 2021 at 04:15:25PM -0400, Vasu Dasari wrote:
> > > > 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.3 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
> > >
> > > Thanks for the patch!
> > >
> > > This change here isn't quite accurate since only OF1.0-OF1.3 are
> > > currently supported:
> > >
> > >     @@ -515,13 +517,10 @@ 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;
> > >      }
> > >
> > >      /* Convert 'str_' (as described in the documentation for the
> > > "monitor" command
> > >
> > > Actually, it occurs to me that this shouldn't add support for OF1.3
> > > either, only for OF1.1 and OF1.2, because OF1.3 has a standardized
> > > extension, which you can find here:
> > >
> > >
> https://opennetworking.org/wp-content/uploads/2014/10/openflow-extensions-1.3.x-pack2.zip
> > > in the pdf inside for extension 187.  It would be better to implement
> > > that for 1.3 rather than to invent our own.
> > >
> > > Thanks,
> > >
> > > Ben.
> > >
>
diff mbox series

Patch

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 60e4ffbe6..f212ce1c7 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -405,6 +405,7 @@  Tony van der Peet                  tony.vanderpeet@alliedtelesis.co.nz
 Tonghao Zhang                      xiangxia.m.yue@gmail.com
 Usman Ansari                       ua1422@gmail.com
 Valient Gough                      vgough@pobox.com
+Vasu Dasari                        vdasari@gmail.com
 Venkata Anil Kommaddi              vkommadi@redhat.com
 Vishal Deep Ajmera                 vishal.deep.ajmera@ericsson.com
 Vivien Bernet-Rollande             vbr@soprive.net
@@ -673,7 +674,6 @@  Tulio Ribeiro                   tribeiro@lasige.di.fc.ul.pt
 Tytus Kurek                     Tytus.Kurek@pega.com
 Valentin Bud                    valentin@hackaserver.com
 Vasiliy Tolstov                 v.tolstov@selfip.ru
-Vasu Dasari                     vdasari@gmail.com
 Vinllen Chen                    cvinllen@gmail.com
 Vishal Swarankar                vishal.swarnkar@gmail.com
 Vjekoslav Brajkovic             balkan@cs.washington.edu
diff --git a/NEWS b/NEWS
index 402ce5969..157807420 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,9 @@  Post-v2.15.0
 ---------------------
    - 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.3 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..501310648 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.3 (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.3 (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 b91517cd2..fdae5c462 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..755629ed4 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -4754,7 +4754,10 @@  OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([ofproto - flow monitoring with out_port])
+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 with out_port - (OpenFlow $1)])
 AT_KEYWORDS([monitor])
 OVS_VSWITCHD_START
 
@@ -4763,13 +4766,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 +4791,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 +4840,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 +4894,58 @@  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)])
+CHECK_FLOW_MONITORING([1.3], [OpenFlow13], [ (OF1.3)])
 
 AT_SETUP([ofproto - event filtering (OpenFlow 1.3)])
 AT_KEYWORDS([monitor])
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 62059e962..d551f6639 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2239,6 +2239,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 +2264,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 +2299,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")) {