On Tue, Aug 07, 2018 at 09:49:38PM -0700, Darrell Ball wrote: > On Tue, Aug 7, 2018 at 4:20 PM, Ben Pfaff <[email protected]> wrote: > > > On Mon, Aug 06, 2018 at 07:24:39PM -0700, Darrell Ball 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 | 8 +++---- > > > 2 files changed, 60 insertions(+), 9 deletions(-) > > > > > > diff --git a/lib/dpctl.c b/lib/dpctl.c > > > index c600eeb..b8a8dbf 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); > > > + > > > + const char *type; > > > + SSET_FOR_EACH (type, &dpif_types) { > > > + int error = dp_enumerate_names(type, &dpif_names); > > > + if (!error) { > > > + const char *name; > > > + SSET_FOR_EACH (name, &dpif_names) { > > > + if (!strcmp(type, queried_type) && > > > + !strcmp(name, queried_name)) { > > > + found = true; > > > + goto out; > > > + } > > > + } > > > + } > > > + } > > > + > > > +out: > > > + sset_destroy(&dpif_names); > > > + sset_destroy(&dpif_types); > > > + return found; > > > +} > > > > Thanks for working to make ovs-dpctl error messages better! > > > > I don't see why one would bother to enumerate the types here. There is > > only one type that could be of interest. I think you said that you > > don't want to just try to blindly open (type,name), but even if so, why > > not just try to enumerate the names within 'type'? > > > > > Because the type (i.e. queried_type) passed is often/usually bogus and > therefore > enumerating names with this bogus 'type' only serves to generate the > appropriate warning logs, > yielding a false positive.
OK, in that case, then use sset_contains() to check for type in dpif_types; there's no reason to enumerate all the names in every type, only in the one we care about. Right? Thanks, Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
