[ovs-dev,V2,1/2] dpif: Fix cleanup of netdev_ports map

Executing dpctl commands from userspace also calls to
dpif_open()/dpif_close() but not really creating another dpif
but using a clone.
As for netdev_ports map is global we avoid adding duplicate entries
but also need to make sure we are not removing needed entries.
With this commit we make sure only the last dpif close should clean
the netdev_ports map.

Fixes: 6595cb95a4a9 ("dpif: Clean up netdev_ports map on dpif_close().")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Paul Blakey <paulb@mellanox.com>
---
 lib/dpif.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

On 17/08/2017 08:38, Roi Dayan wrote:
> Executing dpctl commands from userspace also calls to
> dpif_open()/dpif_close() but not really creating another dpif
> but using a clone.
> As for netdev_ports map is global we avoid adding duplicate entries
> but also need to make sure we are not removing needed entries.
> With this commit we make sure only the last dpif close should clean
> the netdev_ports map.
>
> Fixes: 6595cb95a4a9 ("dpif: Clean up netdev_ports map on dpif_close().")
> Signed-off-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Paul Blakey <paulb@mellanox.com>
> ---
>  lib/dpif.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 4c5eac6..0c8b91b 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -428,6 +428,18 @@ dpif_create_and_open(const char *name, const char *type, struct dpif **dpifp)
>      return error;
>  }
>
> +static void
> +dpif_remove_netdev_ports(struct dpif *dpif) {
> +        struct dpif_port_dump port_dump;
> +        struct dpif_port dpif_port;
> +
> +        DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
> +            if (!dpif_is_internal_port(dpif_port.type)) {
> +                netdev_ports_remove(dpif_port.port_no, dpif->dpif_class);
> +            }
> +        }
> +}
> +
>  /* Closes and frees the connection to 'dpif'.  Does not destroy the datapath
>   * itself; call dpif_delete() first, instead, if that is desirable. */
>  void
> @@ -435,18 +447,14 @@ dpif_close(struct dpif *dpif)
>  {
>      if (dpif) {
>          struct registered_dpif_class *rc;
> -        struct dpif_port_dump port_dump;
> -        struct dpif_port dpif_port;
>
>          rc = shash_find_data(&dpif_classes, dpif->dpif_class->type);
>
> -        DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
> -            if (!dpif_is_internal_port(dpif_port.type)) {
> -                netdev_ports_remove(dpif_port.port_no, dpif->dpif_class);
> -            }
> -        }
>          dpif_uninit(dpif, true);
>          dp_class_unref(rc);
> +        if (rc->refcount == 0) {
> +            dpif_remove_netdev_ports(dpif);
> +        }

actually didn't notice but there is a bug here.
the iteration using DPIF_PORT_FOR_EACH using the
dpif dump operation which can't be done now that dpif is closed.
so i need to update this patch to clean the ports before uninit.


>      }
>  }
>
>