ovs-ofctl: Reject impossible protocols configurations at startup.

The -O and -F options interact, so that it's possible to select only
flow formats that are not supported on a given OpenFlow version.  It seems
best to report these problems up front rather than failing in a more
mysterious way later.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Simon Horman <horms@verge.net.au>

diff --git a/lib/ofp-version-opt.c b/lib/ofp-version-opt.c
index 0aa2930..35d79e6 100644 (file)
--- a/lib/ofp-version-opt.c
+++ b/lib/ofp-version-opt.c
@@ -20,6 +20,12 @@ set_allowed_ofp_versions(const char *string)
     allowed_versions = ofputil_versions_from_string(string);
 }
 
+void
+mask_allowed_ofp_versions(uint32_t bitmap)
+{
+    allowed_versions &= bitmap;
+}
+
 void
 ofp_version_usage(void)
 {

diff --git a/lib/ofp-version-opt.h b/lib/ofp-version-opt.h
index 78e3135..6bf5eed 100644 (file)
--- a/lib/ofp-version-opt.h
+++ b/lib/ofp-version-opt.h
@@ -20,6 +20,7 @@
 
 uint32_t get_allowed_ofp_versions(void);
 void set_allowed_ofp_versions(const char *string);
+void mask_allowed_ofp_versions(uint32_t);
 void ofp_version_usage(void);
 
 #endif

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index d94d75c..2ad165c 100644 (file)
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2068,3 +2068,24 @@ AT_CHECK([ovs-ofctl diff-flows flows.txt br0], [2], [dnl
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ovs-ofctl -F and -O interaction])
+AT_CHECK([ovs-ofctl -F oxm -O openflow10], [1], [],
+  [ovs-ofctl: None of the enabled OpenFlow versions (OpenFlow10) supports any of the enabled flow formats (OXM).  (Use -O to enable additional OpenFlow versions or -F to enable additional flow formats.)
+])
+AT_CHECK([ovs-ofctl -F oxm -O openflow11], [1], [],
+  [ovs-ofctl: None of the enabled OpenFlow versions (OpenFlow11) supports any of the enabled flow formats (OXM).  (Use -O to enable additional OpenFlow versions or -F to enable additional flow formats.)
+])
+AT_CHECK([ovs-ofctl -F oxm -O openflow10,openflow11], [1], [],
+  [ovs-ofctl: None of the enabled OpenFlow versions (OpenFlow10, OpenFlow11) supports any of the enabled flow formats (OXM).  (Use -O to enable additional OpenFlow versions or -F to enable additional flow formats.)
+])
+AT_CHECK([ovs-ofctl -F oxm -O openflow10,openflow12], [1], [],
+ [ovs-ofctl: missing command name; use --help for help
+])
+AT_CHECK([ovs-ofctl -F oxm -O openflow12], [1], [],
+  [ovs-ofctl: missing command name; use --help for help
+])
+AT_CHECK([ovs-ofctl -F oxm -O openflow13], [1], [],
+  [ovs-ofctl: missing command name; use --help for help
+])
+AT_CLEANUP

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 101ce80..80a202f 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -169,6 +169,8 @@ parse_options(int argc, char *argv[])
         {NULL, 0, NULL, 0},
     };
     char *short_options = long_options_to_short_options(long_options);
+    uint32_t versions;
+    enum ofputil_protocol version_protocols;
 
     for (;;) {
         unsigned long int timeout;
@@ -251,6 +253,22 @@ parse_options(int argc, char *argv[])
     }
 
     free(short_options);
+
+    versions = get_allowed_ofp_versions();
+    version_protocols = ofputil_protocols_from_version_bitmap(versions);
+    if (!(allowed_protocols & version_protocols)) {
+        char *protocols = ofputil_protocols_to_string(allowed_protocols);
+        struct ds version_s = DS_EMPTY_INITIALIZER;
+
+        ofputil_format_version_bitmap_names(&version_s, versions);
+        ovs_fatal(0, "None of the enabled OpenFlow versions (%s) supports "
+                  "any of the enabled flow formats (%s).  (Use -O to enable "
+                  "additional OpenFlow versions or -F to enable additional "
+                  "flow formats.)", ds_cstr(&version_s), protocols);
+    }
+    allowed_protocols &= version_protocols;
+    mask_allowed_ofp_versions(ofputil_protocols_to_version_bitmap(
+                                  allowed_protocols));
 }
 
 static void