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

[Alex Wang]

Thx, applied to master,


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

>
> On Nov 26, 2014, at 2:25 PM, Alex Wang <alexw at nicira.com> wrote:
>
> 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?
>
>
> Yes, thanks Alex.
>
>   Jarno
>
> 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
>>
>>
>
>