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

Vasu Dasari vdasari at gmail.com
Fri Jul 30 00:01:11 UTC 2021


Hi Ben,

I am converting individual patch
<http://patchwork.ozlabs.org/project/openvswitch/patch/20210614034931.12245-1-vdasari@gmail.com/>,
which you briefly looked at earlier, to the first one in a series of
patches. Second patch 2/2 depends on this codebase.

Thanks
-Vasu

*Vasu Dasari*


On Thu, Jul 29, 2021 at 7:54 PM Vasu Dasari <vdasari at gmail.com> 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.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 092e92cdf..da16b537d 100644
> --- a/AUTHORS.rst
> +++ b/AUTHORS.rst
> @@ -413,6 +413,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
> @@ -683,7 +684,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 75045b67d..02884b774 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -24,6 +24,9 @@ v2.16.0 - xx xxx xxxx
>         processing database updates.
>     - 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 bd6103b1c..11aadbf20 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -6339,7 +6339,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;
> @@ -6362,14 +6363,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;
> @@ -6379,7 +6381,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);
>      }
>  }
>
> @@ -6543,7 +6545,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