[ovs-dev] [PATCHv2 2/2] dpif: Fix clean up of dpif_ports on dpif_close().

[Paul Blakey]

Seems good to me!

Acked-by: Paul Blakey <paulb at mellanox.com>

Thanks

On 08/08/2017 21:23, Joe Stringer wrote:
> Commit 32b77c316d9982("dpif: Save added ports in a port map.")
> introduced tracking of all dpif ports by taking a reference on each
> available netdev when the dpif is opened, but it failed to clear out and
> release references to these netdevs when the dpif is closed.
> 
> One of the problems introduced by this was that upon clean exit of
> ovs-vswitchd via "ovs-appctl exit --cleanup", the "ovs-netdev" device
> was not deleted. This which could cause problems in subsequent start up.
> Commit 5119e258da92 ("dpif: Fix cleanup of userspace datapath.") fixed
> this particular problem by not adding such devices to the netdev_ports
> map, but the referencing/unreferencing upon dpif_open()/dpif_close() is
> still not balanced.
> 
> Balance the referencing of netdevs by introducing netdev_ports_flush()
> and clearing these during dpif_close().
> 
> Fixes: 32b77c316d9982("dpif: Save added ports in a port map.")
> Signed-off-by: Joe Stringer <joe at ovn.org>
> ---
> v2: Update commit message.
>      Rebase.
> v1: Initial posting.
> ---
>   lib/dpif.c   |  1 +
>   lib/netdev.c | 15 +++++++++++++++
>   lib/netdev.h |  1 +
>   3 files changed, 17 insertions(+)
> 
> diff --git a/lib/dpif.c b/lib/dpif.c
> index e71f6a3d1475..53bdf39f6e20 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -437,6 +437,7 @@ dpif_close(struct dpif *dpif)
>           struct registered_dpif_class *rc;
>   
>           rc = shash_find_data(&dpif_classes, dpif->dpif_class->type);
> +        netdev_ports_flush(dpif->dpif_class);
>           dpif_uninit(dpif, true);
>           dp_class_unref(rc);
>       }
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 3e8b211857d7..94f9e486d8b1 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -2284,6 +2284,21 @@ netdev_ports_remove(odp_port_t port_no, const struct dpif_class *dpif_class)
>       return ret;
>   }
>   
> +void
> +netdev_ports_flush(const struct dpif_class *class)
> +{
> +    struct port_to_netdev_data *data, *next;
> +
> +    ovs_mutex_lock(&netdev_hmap_mutex);
> +    HMAP_FOR_EACH_SAFE (data, next, node, &port_to_netdev) {
> +        if (data->dpif_class == class) {
> +            netdev_port_data_destroy(data);
> +        }
> +    }
> +
> +    ovs_mutex_unlock(&netdev_hmap_mutex);
> +}
> +
>   odp_port_t
>   netdev_ifindex_to_odp_port(int ifindex)
>   {
> diff --git a/lib/netdev.h b/lib/netdev.h
> index f8482f787e39..2d02be5826f6 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -215,6 +215,7 @@ int netdev_ports_insert(struct netdev *, const struct dpif_class *,
>                           struct dpif_port *);
>   struct netdev *netdev_ports_get(odp_port_t port, const struct dpif_class *);
>   int netdev_ports_remove(odp_port_t port, const struct dpif_class *);
> +void netdev_ports_flush(const struct dpif_class *);
>   odp_port_t netdev_ifindex_to_odp_port(int ifindex);
>   struct netdev_flow_dump **netdev_ports_flow_dump_create(
>                                           const struct dpif_class *,
>