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.

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index dfdebd3e4d6219c9c659eaf677f37aa9edb29921..466ade6b510255a49b4e6e4b4ceebb0a9ad29fb5 100644 (file)
--- 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 f7605f7857f7daec48a5a6c5cdc71ba584a7fd22..5c89c7673318451ecb052467b22969e977610091 100644 (file)
--- 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