[ovs-dev] [PATCH 4/9] datapath-windows: Fix bugs in Event.c around subscribe and lock

[Sairam Venugopal]

Yes, I was also thinking of using the gOvsSwitchContext->dispatchLock to
keep it safe. 

That said, we would want to do the following:
1) Add this to OvsGetOpenInstance()
2) Update all unsafe usages of gOvsSwitchContext to either use
OvsGetOpenInstance() or acquire gOvsSwitchContext->dispatchLock.

I will file a bug for this and follow it up.

Thanks,
Sairam

On 7/22/16, 9:57 AM, "Alin Serdean" <aserdean at cloudbasesolutions.com>
wrote:

>The idea is dpNo could change in odd circumstances.
>
>
>
>We could use gOvsSwitchContext->dispatchLock and make OvsGetOpenInstance
>safe by itself.
>
>
>
>Thanks,
>
>Alin.
>
>
>
>> -----Mesaj original-----
>
>> De la: Sairam Venugopal [mailto:vsairam at vmware.com]
>
>> Trimis: Thursday, July 21, 2016 9:52 PM
>
>> Către: Alin Serdean <aserdean at cloudbasesolutions.com>; Yin Lin
>
>> <yinlin10 at gmail.com>
>
>> Cc: dev at openvswitch.org
>
>> Subiect: Re: [ovs-dev] [PATCH 4/9] datapath-windows: Fix bugs in Event.c
>
>> around subscribe and lock
>
>> 
>
>> Hi Alin,
>
>> 
>
>> gOvsSwitchContext->dpNo and gOvsSwitchContext in general is currently
>
>> gOvsSwitchContext->read
>
>> in a lot of places without any locks being taken out.
>
>> 
>
>> Eg: -
>
>> 
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswit
>>ch_ovs_blob_15850211ce88d540e57a6f2fc8096&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKI
>>iDJAXVeAw-YihVMNtXt-uEs&r=Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=3
>>u3GI2uiqwVsds-qnZQrKxEq0hIgf5QgkFs4Fgq_9a4&s=HXq_96fjf6h_3PgWBQMt342vsEcd
>>J4gwQL12akeRGao&e=
>
>> 3465b9
>
>> a5475/datapath-windows/ovsext/Datapath.c#L973
>
>> 
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswit
>>ch_ovs_blob_15850211ce88d540e57a6f2fc8096&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKI
>>iDJAXVeAw-YihVMNtXt-uEs&r=Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=3
>>u3GI2uiqwVsds-qnZQrKxEq0hIgf5QgkFs4Fgq_9a4&s=HXq_96fjf6h_3PgWBQMt342vsEcd
>>J4gwQL12akeRGao&e=
>
>> 3465b9
>
>> a5475/datapath-windows/ovsext/Datapath.c#L1420
>
>> 
>
>> The function OvsGetOpenInstance() is currently defined in Datapath.c and
>
>> only used by 2 methods in Event.c. If gOvsSwitchContext->dpNo was meant
>
>> to be read after acquiring a lock,  then we need to define a lock
>>specific to
>
>> gOvsSwitchContext inside OvsGetOpenInstance() and not rely on
>
>> EventQueue lock to enforce it.
>
>> 
>
>> Thanks,
>
>> Sairam
>
>> 
>
>> 
>
>> 
>
>> 
>
>> On 7/21/16, 6:58 AM, "Alin Serdean" <aserdean at cloudbasesolutions.com>
>
>> wrote:
>
>> 
>
>> >> -----Mesaj original-----
>
>> >
>
>> >> De la: dev [mailto:dev-bounces at openvswitch.org] În numele Sairam
>
>> >
>
>> >> Venugopal
>
>> >
>
>> >> Trimis: Monday, July 18, 2016 9:27 PM
>
>> >
>
>> >> Către: Yin Lin <yinlin10 at gmail.com>
>
>> >
>
>> >> Cc: dev at openvswitch.org
>
>> >
>
>> >> Subiect: Re: [ovs-dev] [PATCH 4/9] datapath-windows: Fix bugs in
>
>> >> Event.c
>
>> >
>
>> >> around subscribe and lock
>
>> >
>
>> >>
>
>> >
>
>> >> Hi Yin,
>
>> >
>
>> >>
>
>> >
>
>> >> Thanks for reviewing this. (1) has been addressed in a different
>>patch.
>
>> >
>
>> >> (2) We don¹t need the lock for OvsGetOpenInstance().
>
>> >
>
>> >[Alin Gabriel Serdean: ] we need a lock in OvsGetOpenInstance because:
>
>> >
>
>> >https://urldefense.proofpoint.com/v2/url?u=https-
>
>> 3A__github.com_openvsw
>
>> >itc
>
>> >h_ovs_blob_master_datapath-2Dwindows_ovsext_Datapath.c-
>
>> 23L527&d=CwIGaQ&
>
>> >c=S
>
>> >qcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-
>
>> uEs&r=Dcruz40PROJ40ROzSpxyQSLw6f
>
>> >crO
>
>> >WpJgEcEmNR3JEQ&m=TIfmnsdkLsgajJWRZp6fyWcW_1eXVWsfuqHqHzvVhi
>
>> M&s=Lb5uwUve
>
>> >kGK
>
>> >ck_ubpcUaZVZkJMFEEgaBwQtbZVsD8vw&e=
>
>> >
>
>> >
>
>> >
>
>> >gOvsSwitchContext->dpNo
>
>
>