On 8/8/18, 11:14 AM, "[email protected] on behalf of Ben Pfaff"
<[email protected] on behalf of [email protected]> wrote:
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?
I was thinking that the number of datapaths is typically ‘1’, so, somehow, I
had little
practical concern about enumerating for this use case.
But sure, that is generally better; I made the adjustment.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=02%7C01%7Cdball%40vmware.com%7Cbd37aa3529b7424419c608d5fd5ab7d9%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636693488467972500&sdata=8c1NbVZOj%2BTBhqFmLBctytMzM7u7MmD42GRY%2Furz0Ww%3D&reserved=0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
