[ovs-dev] [PATCH 2/2] dpif-netdev: Introduce port_try_ref() to prevent a race.

Pravin Shelar pshelar at nicira.com
Wed Sep 3 02:34:53 UTC 2014


On Tue, Sep 2, 2014 at 6:44 PM, Alex Wang <alexw at nicira.com> wrote:
> When pmd thread interates through all ports for queue loading,
> the main thread may unreference and 'rcu-free' a port before
> pmd thread take new reference of it.  This could cause pmd
> thread fail the reference and access freed memory later.
>
> This commit fixes this race by introducing port_try_ref()
> which uses ovs_refcount_try_ref_rcu().  And the pmd thread
> will only load the port's queue, if port_try_ref() returns
> true.
>
> Found by inspection.
>
Good catch.

> Signed-off-by: Alex Wang <alexw at nicira.com>
> ---
>  lib/dpif-netdev.c |   17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 2476435..4297db0 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -880,6 +880,16 @@ port_ref(struct dp_netdev_port *port)
>      }
>  }
>
> +static bool
> +port_try_ref(struct dp_netdev_port *port)
> +{
> +    if (port) {
> +        return ovs_refcount_try_ref_rcu(&port->ref_cnt);
> +    }
> +
> +    return false;
> +}
> +
>  static void
>  port_destroy__(struct dp_netdev_port *port)
>  {
> @@ -1864,7 +1874,10 @@ pmd_load_queues(struct pmd_thread *f,
>      index = 0;
>
>      CMAP_FOR_EACH (port, node, &f->dp->ports) {
> -        if (netdev_is_pmd(port->netdev)) {
> +        /* Calls port_try_ref() to prevent the main thread
> +         * from deleting the port. */
> +        if (netdev_is_pmd(port->netdev)
> +            && port_try_ref(port)) {
>              int i;
>
port_try_ref() check should be first in the condition.

Otherwise looks good.
Acked-by: Pravin B Shelar <pshelar at nicira.com>

>              for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
> @@ -1878,6 +1891,8 @@ pmd_load_queues(struct pmd_thread *f,
>                  }
>                  index++;
>              }
> +            /* Unrefs the port_try_ref(). */
> +            port_unref(port);
>          }
>      }
>
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list