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

Roi Dayan roid at mellanox.com
Thu Aug 17 05:56:03 UTC 2017



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 at mellanox.com>
> Reviewed-by: Paul Blakey <paulb at 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.


>      }
>  }
>
>


More information about the dev mailing list