[ovs-dev] [PATCH] dpif-netdev: Fix a race.

[Alex Wang]

Yeah,

If we take a ref count, then, there is possibility that threads like
'monitor'
thread can take a reference when main thread is deleting the 'port'
=> delay the netdev_close
=> if main thread want to immediately recreate the port, it will fail...

So, it is most safe to just acquire the lock here,

Does this make sense?

Thanks,
Alex Wang,

On Wed, Nov 26, 2014 at 1:43 PM, Jarno Rajahalme <jrajahalme at nicira.com>
wrote:

> Alex,
>
> Did you consider simply taking a reference instead of the mutex? pros/cons?
>
>   Jarno
>
> On Nov 25, 2014, at 4:01 PM, Alex Wang <alexw at nicira.com> wrote:
>
> > On current master, the 'struct dp_netdev_port' is destroyed
> > immediately when the ref count reaches 0.  However, non-pmd
> > threads calling the dpif_netdev_execute() for sending packets
> > could hold pointer to 'port' that is not ref-counted.  Thusly
> > those threads could possibly access freed memory when the port
> > is deleted.
> >
> > To fix this bug, this commit makes non-pmd threads acquiring
> > the 'port_mutex' before doing the actual execution in
> > dpif_netdev_execute().
> >
> > Signed-off-by: Alex Wang <alexw at nicira.com>
> > ---
> > lib/dpif-netdev.c |    2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index ea87023..5233130 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -2029,10 +2029,12 @@ dpif_netdev_execute(struct dpif *dpif, struct
> dpif_execute *execute)
> >      * the 'non_pmd_mutex'. */
> >     if (pmd->core_id == NON_PMD_CORE_ID) {
> >         ovs_mutex_lock(&dp->non_pmd_mutex);
> > +        ovs_mutex_lock(&dp->port_mutex);
> >     }
> >     dp_netdev_execute_actions(pmd, &pp, 1, false, execute->actions,
> >                               execute->actions_len);
> >     if (pmd->core_id == NON_PMD_CORE_ID) {
> > +        ovs_mutex_unlock(&dp->port_mutex);
> >         ovs_mutex_unlock(&dp->non_pmd_mutex);
> >     }
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
>