[ovs-dev] [of1.2 6/9] ovs-ofctl: Reject impossible protocols configurations at startup.

[Simon Horman]

On Wed, Nov 28, 2012 at 11:06:53PM -0800, Ben Pfaff wrote:
> 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 at nicira.com>

Wow, that rather tedious.

Acked-by: Simon Horman <horms at verge.net.au>

> ---
>  lib/ofp-version-opt.c |    6 ++++++
>  lib/ofp-version-opt.h |    1 +
>  tests/ovs-ofctl.at    |   21 +++++++++++++++++++++
>  utilities/ovs-ofctl.c |   18 ++++++++++++++++++
>  4 files changed, 46 insertions(+)
> 
> diff --git a/lib/ofp-version-opt.c b/lib/ofp-version-opt.c
> index 0aa2930..35d79e6 100644
> --- a/lib/ofp-version-opt.c
> +++ b/lib/ofp-version-opt.c
> @@ -21,6 +21,12 @@ set_allowed_ofp_versions(const char *string)
>  }
>  
>  void
> +mask_allowed_ofp_versions(uint32_t bitmap)
> +{
> +    allowed_versions &= bitmap;
> +}
> +
> +void
>  ofp_version_usage(void)
>  {
>      struct ds msg = DS_EMPTY_INITIALIZER;
> diff --git a/lib/ofp-version-opt.h b/lib/ofp-version-opt.h
> index 78e3135..6bf5eed 100644
> --- 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
> --- 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
> --- 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
> -- 
> 1.7.10.4
>