Hi Yin,
Thanks for reviewing this. (1) has been addressed in a different patch.
(2) We don¹t need the lock for OvsGetOpenInstance().
Thanks,
Sairam
On 7/18/16, 11:07 AM, "Yin Lin" <yinli...@gmail.com> wrote:
>Thanks Sai for fixing the bugs. A few questions:
>
>1. There is also an unrelease lock in line 359, function
>OvsWaitEventIoctl.
>2. Do we need the lock before OvsGetOpenInstance? If so, please release
>the
>lock inside "if (instance == NULL)"; if not, you should do the same thing
>on line 359.
>
>
>
>
>On Wed, Jul 13, 2016 at 4:38 PM, Sairam Venugopal <vsai...@vmware.com>
>wrote:
>
>> When userspace tries to resubscribe to an existing queue, return
>> STATUS_INVALID_PARAMETER since it's not supported. The current bug
>> overwrites status to STATUS_SUCCESS.
>>
>> The second bug fix is around releasing the EventQueue lock if an open
>> instance couldn't be found. The current version returns back without
>> releasing the lock. Moving the OvsAcquireEventQueueLock() after the
>> instance is verified.
>>
>> Signed-off-by: Sairam Venugopal <vsai...@vmware.com>
>> ---
>> datapath-windows/ovsext/Event.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/datapath-windows/ovsext/Event.c
>> b/datapath-windows/ovsext/Event.c
>> index 8c7c3ec..8ff0322 100644
>> --- a/datapath-windows/ovsext/Event.c
>> +++ b/datapath-windows/ovsext/Event.c
>> @@ -217,9 +217,8 @@ OvsSubscribeEventIoctl(PFILE_OBJECT fileObject,
>> if (queue->mask != request->mask) {
>> status = STATUS_INVALID_PARAMETER;
>> OVS_LOG_WARN("Can not chnage mask when the queue is
>> subscribed");
>> + goto done_event_subscribe;
>> }
>> - status = STATUS_SUCCESS;
>> - goto done_event_subscribe;
>> } else if (!request->subscribe && queue == NULL) {
>> status = STATUS_SUCCESS;
>> goto done_event_subscribe;
>> @@ -356,8 +355,6 @@ OvsWaitEventIoctl(PIRP irp,
>> }
>> poll = (POVS_EVENT_POLL)inputBuffer;
>>
>> - OvsAcquireEventQueueLock();
>> -
>> instance = OvsGetOpenInstance(fileObject, poll->dpNo);
>> if (instance == NULL) {
>> OVS_LOG_TRACE("Exit: Can not find open instance, dpNo: %d",
>> @@ -365,6 +362,7 @@ OvsWaitEventIoctl(PIRP irp,
>> return STATUS_INVALID_PARAMETER;
>> }
>>
>> + OvsAcquireEventQueueLock();
>> queue = (POVS_EVENT_QUEUE)instance->eventQueue;
>> if (queue == NULL) {
>> OVS_LOG_TRACE("Exit: Event queue does not exist");
>> --
>> 2.9.0.windows.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>>
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm
>>an_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=
>>Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=_qUqkq3AbylbGTt0FRPDctPJYJN
>>u3r80SqmGYDgiNA4&s=HbFssXsaXxSrxCuEo36BQmPdk4BlTDTMpnC_OQkp_Ok&e=
>>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=_qUqkq3AbylbGTt0FRPDctPJYJNu3r
>80SqmGYDgiNA4&s=HbFssXsaXxSrxCuEo36BQmPdk4BlTDTMpnC_OQkp_Ok&e=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
