hold off on this version I can save a couple lines of code, so I'll send a V7
Darrell On Wed, Aug 8, 2018 at 1:05 PM, Darrell Ball <[email protected]> wrote: > By making opt_dpif_open() more general, it can be used effectively > by all potential callers and avoids trying to open potentially bogus > datapaths provided by the user. Also, the error handling is improved by > having more specific errors. > > Signed-off-by: Darrell Ball <[email protected]> > --- > lib/dpctl.c | 61 ++++++++++++++++++++++++++++++ > +++++++++++++++---- > tests/system-traffic.at | 10 ++++---- > 2 files changed, 61 insertions(+), 10 deletions(-) > > diff --git a/lib/dpctl.c b/lib/dpctl.c > index c600eeb..6f50d89 100644 > --- a/lib/dpctl.c > +++ b/lib/dpctl.c > @@ -187,18 +187,69 @@ parsed_dpif_open(const char *arg_, bool create, > struct dpif **dpifp) > return result; > } > > +static bool > +dp_exists(const char *queried_dp) > +{ > + struct sset dpif_names = SSET_INITIALIZER(&dpif_names), > + dpif_types = SSET_INITIALIZER(&dpif_types); > + bool found = false; > + char *queried_name, *queried_type; > + > + dp_parse_name(queried_dp, &queried_name, &queried_type); > + dp_enumerate_types(&dpif_types); > + > + if (!sset_contains(&dpif_types, queried_type)) { > + goto out; > + } > + > + int error = dp_enumerate_names(queried_type, &dpif_names); > + if (!error) { > + const char *name; > + SSET_FOR_EACH (name, &dpif_names) { > + if (!strcmp(name, queried_name)) { > + found = true; > + goto out; > + } > + } > + } > + > +out: > + sset_destroy(&dpif_names); > + sset_destroy(&dpif_types); > + return found; > +} > + > +static bool > +dp_arg_exists(int argc, const char *argv[]) > +{ > + if (argc > 1 && dp_exists(argv[1])) { > + return true; > + } > + return false; > +} > + > /* Open a dpif with an optional name argument. > * > - * The datapath name is not a mandatory parameter for this command. If > - * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from > - * the current setup, assuming only one exists. On success stores the > - * opened dpif in '*dpifp'. */ > + * The datapath name is not a mandatory parameter for this command. If > it is > + * not specified, we retrieve it from the current setup, assuming only one > + * exists. On success stores the opened dpif in '*dpifp'. */ > static int > opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p, > uint8_t max_args, struct dpif **dpifp) > { > + char *dpname; > + if (dp_arg_exists(argc, argv)) { > + dpname = xstrdup(argv[1]); > + } else if (argc != max_args) { > + dpname = get_one_dp(dpctl_p); > + } else { > + /* If the arguments are the maximum possible number and there is > no > + * valid datapath argument, then we fall into the case of dpname > is > + * NULL, since this is an error. */ > + dpname = NULL; > + } > + > int error = 0; > - char *dpname = argc >= max_args ? xstrdup(argv[1]) : > get_one_dp(dpctl_p); > if (!dpname) { > error = EINVAL; > dpctl_error(dpctl_p, error, "datapath not found"); > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index cbd9542..a7c8a24 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -1040,7 +1040,7 @@ AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=5 > $ICMP_TUPLE]) > AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep > "orig=.src=10\.1\.1\.2,"], [1], [dnl > ]) > > -OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath/d"]) > +OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > AT_SETUP([conntrack - IPv4 ping]) > @@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error > ]) > > AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl > -ovs-vswitchd: opening datapath (Address family not supported by protocol) > +ovs-vswitchd: datapath not found (Invalid argument) > ovs-appctl: ovs-vswitchd: server returned an error > ]) > > AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl > -ovs-vswitchd: opening datapath (Address family not supported by protocol) > +ovs-vswitchd: datapath not found (Invalid argument) > ovs-appctl: ovs-vswitchd: server returned an error > ]) > > AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl > -ovs-vswitchd: opening datapath (Address family not supported by protocol) > +ovs-vswitchd: datapath not found (Invalid argument) > ovs-appctl: ovs-vswitchd: server returned an error > ]) > > @@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl > 10 > ]) > > -OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of > unknown type system/d"]) > +OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > AT_SETUP([conntrack - IPv6 ping]) > -- > 1.9.1 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
