Hey Ben,
Could you help review this? also,
Since, there can be some scenarios about netdev deletion we do not know,
Thanks,
Alex Wang,
On Thu, May 15, 2014 at 3:01 AM, Ryan Wilson <wr...@nicira.com> wrote:
> When the user changes port type (i.e. changing p0 from type 'internal' to
> 'gre'), the netdev must first be deleted, then re-created with the new
> type.
> Deleting the netdev requires there exist no more references to the netdev.
> However, the xlate cache holds references to netdevs and the cache is only
> invalidated by revalidator threads. Thus, if cache is not invalidated
> prior to
> the netdev being re-created, the netdev will not be able to be re-created
> and
> the configuration change will fail.
>
> This patch always removes the netdev from the global netdev shash when the
> user changes port type. This ensures that the new netdev can always be
> created
> while handler and revalidator threads can retain references to the old
> netdev
> until they are finished.
> ---
> lib/netdev.c | 26 +++++++++++++++++++++++++-
> lib/netdev.h | 1 +
> vswitchd/bridge.c | 2 +-
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 5bf2220..af42c9a 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -489,7 +489,9 @@ netdev_unref(struct netdev *dev)
>
> dev->netdev_class->destruct(dev);
>
> - shash_delete(&netdev_shash, dev->node);
> + if (dev->node) {
> + shash_delete(&netdev_shash, dev->node);
> + }
> free(dev->name);
> dev->netdev_class->dealloc(dev);
> ovs_mutex_unlock(&netdev_mutex);
> @@ -515,6 +517,28 @@ netdev_close(struct netdev *netdev)
> }
> }
>
> +/* Removes 'netdev' from global hash table and destroys 'netdev' if
> possible.
> + *
> + * This allows handler and revalidator threads to still retain references
> + * to this netdev while the main thread changes interface configuration.
> + *
> + * This function should only be called by the main thread when closing
> + * netdevs during user configuration changes. Otherwise, netdev_close
> should be
> + * used to close netdevs. */
> +void
> +netdev_remove(struct netdev *netdev)
> +{
> + if (netdev) {
> + ovs_mutex_lock(&netdev_mutex);
> + if (netdev->node) {
> + shash_delete(&netdev_shash, netdev->node);
> + netdev->node = NULL;
> + netdev_change_seq_changed(netdev);
> + }
> + netdev_unref(netdev);
> + }
> +}
> +
> /* Parses 'netdev_name_', which is of the form [type@]name into its
> component
> * pieces. 'name' and 'type' must be freed by the caller. */
> void
> diff --git a/lib/netdev.h b/lib/netdev.h
> index 7cb3c80..9b35972 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -141,6 +141,7 @@ bool netdev_is_pmd(const struct netdev *netdev);
> int netdev_open(const char *name, const char *type, struct netdev
> **netdevp);
>
> struct netdev *netdev_ref(const struct netdev *);
> +void netdev_remove(struct netdev *);
> void netdev_close(struct netdev *);
>
> void netdev_parse_name(const char *netdev_name, char **name, char **type);
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 43c109c..7189340 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -3471,7 +3471,7 @@ iface_destroy__(struct iface *iface)
> list_remove(&iface->port_elem);
> hmap_remove(&br->iface_by_name, &iface->name_node);
>
> - netdev_close(iface->netdev);
> + netdev_remove(iface->netdev);
>
> free(iface->name);
> free(iface);
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
