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.

1 files changed, 26 insertions, 20 deletions

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index f7605f785..5c89c7673 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