[ovs-dev] [patch v5] dpctl: Make opt_dpif_open() more general.
Ben Pfaff
blp at ovn.org
Wed Aug 8 18:13:43 UTC 2018
On Tue, Aug 07, 2018 at 09:49:38PM -0700, Darrell Ball wrote:
> On Tue, Aug 7, 2018 at 4:20 PM, Ben Pfaff <blp at ovn.org> 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 <dlu998 at gmail.com>
> > > ---
> > > 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.
More information about the dev
mailing list