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. Signed-off-by: Ryan Wilson <wr...@nicira.com> Acked-by: Ben Pfaff <b...@nicira.com> --- v2: Clarified comments for netdev_remove based on Alex Wang's and Ben Pfaff's recommendations --- AUTHORS | 1 + lib/netdev.c | 26 +++++++++++++++++++++++++- lib/netdev.h | 1 + vswitchd/bridge.c | 4 +++- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 7ca92e5..90d897a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -106,6 +106,7 @@ Remko Tronçon g...@el-tramo.be Rich Lane rl...@bigswitch.com Rob Hoes rob.h...@citrix.com Romain Lenglet romain.leng...@berabera.info +Ryan Wilson wr...@nicira.com Sajjad Lateef slat...@nicira.com Sanjay Sane ss...@nicira.com Saurabh Shah ssaur...@nicira.com diff --git a/lib/netdev.c b/lib/netdev.c index 5bf2220..f545a51 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 the global shash and unrefs 'netdev'. + * + * 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 1b06ec9..123c15e 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -3452,7 +3452,9 @@ iface_destroy__(struct iface *iface) list_remove(&iface->port_elem); hmap_remove(&br->iface_by_name, &iface->name_node); - netdev_close(iface->netdev); + /* The user is changing configuration here, so netdev_remove needs to be + * used as opposed to netdev_close */ + 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