On Thu, Aug 1, 2013 at 2:29 PM, Ben Pfaff <b...@nicira.com> wrote:
> This API change is necessary for thread safety, to be added in an upcoming
> commit. Otherwise, the client would not be able to actually use any of
> the returned netdevs because they could already have been destroyed.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/netdev-bsd.c | 32 ++++++++++----------------------
> lib/netdev-linux.c | 4 ++++
> lib/netdev.c | 5 +++--
> 3 files changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 6ff6b3e..d6a8631 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -255,6 +255,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change *change,
> dev->cache_valid = 0;
> netdev_bsd_changed(dev);
> }
> + netdev_close(base_dev);
> }
> } else {
> /*
> @@ -271,6 +272,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change *change,
> dev = netdev_bsd_cast(netdev);
> dev->cache_valid = 0;
> netdev_bsd_changed(dev);
> + netdev_close(dev);
> }
> shash_destroy(&device_shash);
> }
> @@ -1200,9 +1202,10 @@ netdev_bsd_get_in6(const struct netdev *netdev_,
> struct in6_addr *in6)
> }
>
> #if defined(__NetBSD__)
> -static struct netdev *
> -find_netdev_by_kernel_name(const char *kernel_name)
> +static char *
> +netdev_bsd_kernel_name_to_ovs_name(const char *kernel_name)
> {
> + char *ovs_name = NULL;
> struct shash device_shash;
> struct shash_node *node;
>
> @@ -1213,24 +1216,14 @@ find_netdev_by_kernel_name(const char *kernel_name)
> struct netdev_bsd * const dev = netdev_bsd_cast(netdev);
>
> if (!strcmp(dev->kernel_name, kernel_name)) {
> - shash_destroy(&device_shash);
> - return &dev->up;
> + free(ovs_name);
> + ovs_name = xstrdup(netdev_get_name(&dev->up));
>
Would netdev_close to be called for &dev->up ?
> }
> + netdev_close(netdev);
> }
> shash_destroy(&device_shash);
> - return NULL;
> -}
> -
> -static const char *
> -netdev_bsd_convert_kernel_name_to_ovs_name(const char *kernel_name)
> -{
> - const struct netdev * const netdev =
> - find_netdev_by_kernel_name(kernel_name);
>
> - if (netdev == NULL) {
> - return NULL;
> - }
> - return netdev_get_name(netdev);
> + return ovs_name ? ovs_name : xstrdup(kernel_name);
> }
> #endif
>
> @@ -1315,12 +1308,7 @@ netdev_bsd_get_next_hop(const struct in_addr *host
> OVS_UNUSED,
> char *kernel_name;
>
> kernel_name = xmemdup0(sdl->sdl_data, sdl->sdl_nlen);
> - name =
> netdev_bsd_convert_kernel_name_to_ovs_name(kernel_name);
> - if (name == NULL) {
> - ifname = xstrdup(kernel_name);
> - } else {
> - ifname = xstrdup(name);
> - }
> + ifname = netdev_bsd_kernel_name_to_ovs_name(kernel_name);
> free(kernel_name);
> }
> RT_ADVANCE(cp, sa);
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index ba0d863..5bbaf63 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -555,6 +555,7 @@ netdev_linux_cache_cb(const struct
> rtnetlink_link_change *change,
>
> get_flags(&dev->up, &flags);
> netdev_linux_changed(dev, flags, 0);
> + netdev_close(netdev);
> }
> shash_destroy(&device_shash);
> }
> @@ -1180,6 +1181,7 @@ netdev_linux_miimon_run(void)
> bool miimon;
>
> if (dev->miimon_interval <= 0 ||
> !timer_expired(&dev->miimon_timer)) {
> + netdev_close(netdev);
> continue;
> }
>
> @@ -1190,6 +1192,7 @@ netdev_linux_miimon_run(void)
> }
>
> timer_set_duration(&dev->miimon_timer, dev->miimon_interval);
> + netdev_close(netdev);
> }
>
> shash_destroy(&device_shash);
> @@ -1210,6 +1213,7 @@ netdev_linux_miimon_wait(void)
> if (dev->miimon_interval > 0) {
> timer_wait(&dev->miimon_timer);
> }
> + netdev_close(netdev);
> }
> shash_destroy(&device_shash);
> }
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 0079dc2..2561538 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -1395,8 +1395,8 @@ netdev_from_name(const char *name)
>
> /* Fills 'device_list' with devices that match 'netdev_class'.
> *
> - * The caller is responsible for initializing and destroying 'device_list'
> - * but the contained netdevs must not be freed. */
> + * The caller is responsible for initializing and destroying
> 'device_list' and
> + * must close each device on the list. */
> void
> netdev_get_devices(const struct netdev_class *netdev_class,
> struct shash *device_list)
> @@ -1406,6 +1406,7 @@ netdev_get_devices(const struct netdev_class
> *netdev_class,
> struct netdev *dev = node->data;
>
> if (dev->netdev_class == netdev_class) {
> + dev->ref_cnt++;
> shash_add(device_list, node->name, node->data);
> }
> }
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
