From f9cbfbe4f4740cedbaba32a931181a724c3afbca Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 10 Mar 2011 11:07:10 -0800 Subject: [PATCH] ovs-ofctl: Check min flow format support in negotiate_highest_flow_format(). When the -F option wasn't set, or if it was set to an invalid flow format for the match, this code would happily select a flow format that did not select the user's requested match if the switch didn't support an advanced-enough flow format. This fixes the problem. It also changes behavior in the case where the user specifies a flow format that cannot represent the match, changing this from a warning to a fatal error; this is consistent with -F behavior for flow_mod commands. --- tests/ovs-ofctl.at | 22 +++++++++++++++++++++ utilities/ovs-ofctl.c | 46 ++++++++++++++++++++++++------------------- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index dfdebd3e..466ade6b 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -486,3 +486,25 @@ NXST_FLOW reply: ]) OFPROTO_STOP AT_CLEANUP + +dnl Check that "-F openflow10" is really honored on dump-flows. +dnl (If it isn't, then dump-flows will show the register match.) +AT_SETUP([ovs-ofctl dump-flows honors -F option]) +OFPROTO_START +AT_CHECK([ovs-ofctl add-flow br0 reg0=0x12345,actions=drop]) +AT_CHECK([ovs-ofctl -F openflow10 dump-flows br0 | STRIP_XIDS | STRIP_DURATION], [0], [dnl +OFPST_FLOW reply: + cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, actions=drop +]) +OFPROTO_STOP +AT_CLEANUP + +dnl Check that "-F openflow10" fails on dump-flows if the requested match +dnl can't be represented in OpenFlow 1.0. +AT_SETUP([ovs-ofctl dump-flows rejects bad -F option]) +OFPROTO_START +AT_CHECK([ovs-ofctl -F openflow10 dump-flows unix:br0.mgmt reg0=0xabcdef], [1], [], + [ovs-ofctl: unix:br0.mgmt: cannot use requested flow format nxm for specified flow +]) +OFPROTO_STOP +AT_CLEANUP diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index f7605f78..5c89c767 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -540,33 +540,39 @@ static enum nx_flow_format negotiate_highest_flow_format(struct vconn *vconn, const struct cls_rule *rule, bool cookie_support, ovs_be64 cookie) { - int flow_format; + enum nx_flow_format min_format; + min_format = ofputil_min_flow_format(rule, cookie_support, cookie); if (preferred_flow_format != -1) { - enum nx_flow_format min_format; + if (preferred_flow_format < min_format) { + ovs_fatal(0, "%s: cannot use requested flow format %s for " + "specified flow", vconn_get_name(vconn), + ofputil_flow_format_to_string(min_format)); + } - min_format = ofputil_min_flow_format(rule, cookie_support, cookie); - if (preferred_flow_format >= min_format) { - set_flow_format(vconn, preferred_flow_format); - return preferred_flow_format; + set_flow_format(vconn, preferred_flow_format); + return preferred_flow_format; + } else { + enum nx_flow_format flow_format; + + if (try_set_flow_format(vconn, NXFF_NXM)) { + flow_format = NXFF_NXM; + } else if (try_set_flow_format(vconn, NXFF_TUN_ID_FROM_COOKIE)) { + flow_format = NXFF_TUN_ID_FROM_COOKIE; + } else { + flow_format = NXFF_OPENFLOW10; } - VLOG_WARN("%s: cannot use requested flow format %s for " - "specified flow", vconn_get_name(vconn), - ofputil_flow_format_to_string(min_format)); - } + if (flow_format < min_format) { + ovs_fatal(0, "%s: cannot use switch's most advanced flow format " + "%s for specified flow", vconn_get_name(vconn), + ofputil_flow_format_to_string(min_format)); + } - if (try_set_flow_format(vconn, NXFF_NXM)) { - flow_format = NXFF_NXM; - } else if (try_set_flow_format(vconn, NXFF_TUN_ID_FROM_COOKIE)) { - flow_format = NXFF_TUN_ID_FROM_COOKIE; - } else { - flow_format = NXFF_OPENFLOW10; + VLOG_DBG("%s: negotiated flow format %s", vconn_get_name(vconn), + ofputil_flow_format_to_string(flow_format)); + return flow_format; } - - VLOG_DBG("%s: negotiated flow format %s", vconn_get_name(vconn), - ofputil_flow_format_to_string(flow_format)); - return flow_format; } static void -- 2.30.2