On Fri, Jul 24, 2015 at 8:48 AM, Ben Pfaff <[email protected]> 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 <[email protected]>
> > Signed-off-by: Alex Wang <[email protected]>
>
> 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.~
> -/* If 'netdev' has an assigned IPv6 address, sets '*in6' to that address
> (if
> - * 'in6' is non-null) and returns true. Otherwise, returns false. */
> +/* If 'netdev' has an assigned IPv6 address, sets '*in6' to that address.
> + * Otherwise, sets '*in6' to 'in6addr_any' and returns the corresponding
> + * error. */
> static int
> netdev_linux_get_in6(const struct netdev *netdev_, struct in6_addr *in6)
> {
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> + int error;
>
> ovs_mutex_lock(&netdev->mutex);
> if (!(netdev->cache_valid & VALID_IN6)) {
> @@ -2478,6 +2481,7 @@ netdev_linux_get_in6(const struct netdev *netdev_,
> struct in6_addr *in6)
> char line[128];
>
> netdev->in6 = in6addr_any;
> + netdev->in6_error = EADDRNOTAVAIL;
>
> file = fopen("/proc/net/if_inet6", "r");
> if (file != NULL) {
> @@ -2489,17 +2493,21 @@ netdev_linux_get_in6(const struct netdev *netdev_,
> struct in6_addr *in6)
> && !strcmp(name, ifname))
> {
> netdev->in6 = in6_tmp;
> + netdev->in6_error = 0;
> break;
> }
> }
> fclose(file);
> + } else {
> + netdev->in6_error = EOPNOTSUPP;
> }
> netdev->cache_valid |= VALID_IN6;
> }
> *in6 = netdev->in6;
> + error = netdev->in6_error;
> ovs_mutex_unlock(&netdev->mutex);
>
>
Could we just return netdev->in6_error and get rid of 'error'?
I'll post V2.~
> - return 0;
> + return error;
> }
>
> static void
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
