[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