[ovs-dev] [PATCH] netdev-linux: Make netdev_linux_get_in6() conform to API definition.

Alex Wang alexw at nicira.com
Fri Jul 24 16:24:31 UTC 2015


On Fri, Jul 24, 2015 at 9:23 AM, Ben Pfaff <blp at nicira.com> wrote:

> On Fri, Jul 24, 2015 at 09:14:33AM -0700, Alex Wang wrote:
> > On Fri, Jul 24, 2015 at 8:48 AM, Ben Pfaff <blp at nicira.com> wrote:
> >
> > > On Thu, Jul 23, 2015 at 05:32:55PM -0700, Alex Wang wrote:
> > > > The get_in6() API defined in netdev-provider.h requires the return
> > > > of error values when the 'netdev' has no assigned IPv6 address or
> > > > the 'netdev' does not support IPv6.  However, the
> netdev_linux_get_in6()
> > > > implementation does not follow this (always return 0).  And this
> > > > causes the a bug in deleting vlan interfaces created for vlan
> > > > splinter.
> > > >
> > > > This commit makes netdev_linux_get_in6() conform to the API
> definition
> > > > and returns the specified error value when failing to get IPv6
> address.
> > > >
> > > > VMware-BZ: #1485521
> > > > Reported-by: Ronald Lee <ronaldlee at vmware.com>
> > > > Signed-off-by: Alex Wang <alexw at nicira.com>
> > >
> > > It's better if we can cache error conditions as well as success.  How
> > > about this diff?
> > >
> > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > > index 0656f36..aff2f62 100644
> > > --- a/lib/netdev-linux.c
> > > +++ b/lib/netdev-linux.c
> > > @@ -458,6 +458,7 @@ struct netdev_linux {
> > >      int netdev_policing_error;  /* Cached error code from set
> policing. */
> > >      int get_features_error;     /* Cached error code from
> ETHTOOL_GSET. */
> > >      int get_ifindex_error;      /* Cached error code from
> SIOCGIFINDEX. */
> > > +    int in6_error;              /* Cached error code from reading in6
> > > addr. */
> > >
> > >      enum netdev_features current;    /* Cached from ETHTOOL_GSET. */
> > >      enum netdev_features advertised; /* Cached from ETHTOOL_GSET. */
> > > @@ -2465,12 +2466,14 @@ parse_if_inet6_line(const char *line,
> > >                      ifname);
> > >  }
> > >
> > >
> >
> > I did not notice that we cache other errors.  did not see we do so for
> in4.
> >  that
> > is why i did that.~
>
> It would probably be better if we did so for IPv4 also.
>
>
I see, will do that in separate patch,



> > Could we just return netdev->in6_error and get rid of 'error'?
>
> That would introduce a race against some other thread grabbing the lock
> and changing netdev->in6_error before we read back what we just wrote.
>

Oh, yeah, totally makes sense!



More information about the dev mailing list