[ovs-dev] [PATCH] ofp-monitor: Extend Flow Monitoring support for OF 1.0+ with Nicira Extensions
Vasu Dasari
vdasari at gmail.com
Sat May 8 11:12:27 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+ 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
include/openvswitch/vlog.h
lib/ofp-monitor.c
ofproto/connmgr.c
ofproto/connmgr.h
ofproto/ofproto.c
4. Testcase for verification
tests/ofproto.at
Signed-off-by: Vasu Dasari <vdasari at gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-December/050820.html
Signed-off-by: Vasu Dasari <vdasari at gmail.com>
---
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 | 21 +++++++++++++++------
utilities/ovs-ofctl.c | 5 +++--
8 files changed, 60 insertions(+), 33 deletions(-)
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..be1bc2745 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+ (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+ (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..99b01a00e 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -4911,6 +4911,7 @@ NXT_FLOW_MONITOR_RESUMED:
OVS_VSWITCHD_STOP
AT_CLEANUP
+# Test to show flow monitoring support on different OpenFlow protocols
AT_SETUP([ofproto - flow monitoring usable protocols])
AT_KEYWORDS([monitor])
@@ -4920,14 +4921,22 @@ 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
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 1.4
+OVS_WAIT_UNTIL([grep "NXST_FLOW_MONITOR reply (OF1.4)" monitor.log])
+ovs-appctl -t ovs-ofctl exit
+
+# Make sure protocol type in messages from vswitchd, matches that of requested protocol
+ovs-ofctl -OOpenFlow13 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
-# 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])
+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 (OF1.3):
+NXST_FLOW_MONITOR reply (OF1.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
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")) {
--
2.29.2
More information about the dev
mailing list