We must always grab the lock in the same order. Thanks, Eitan -----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Nithin Raju Sent: Wednesday, October 22, 2014 5:21 PM To: Ankur Sharma Cc: <dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v2 5/6] datapath-windows: Refactor CreateQueue function to handle vport pid.
On Oct 21, 2014, at 5:24 PM, Ankur Sharma <ankursha...@vmware.com> wrote: > Refactored CreateQueue function so that packets are enqueued to correct > corresponding queue. > > Signed-off-by: Ankur Sharma <ankursha...@vmware.com> Looks good mostly. One comment I had was if it was the right thing to use gOvsControlLock. This can lead to a deadlock. Consider the following sequence: Context #1 executing OvsDeviceControl() code to do vport add: 1. Acquires gOvsControlLock 2. Acquires dispatch lock Context #2 executing packet processing code: 1. Acquires dispatch lock 2. Acquires gOvsControlLock for enqueueing packets We should add a separate lock for PID, and not use the gOvsControlLock. > @@ -932,6 +943,10 @@ OvsGetPid(POVS_VPORT_ENTRY vport, PNET_BUFFER nb, > UINT32 *pid) { > UNREFERENCED_PARAMETER(nb); > > + if (!vport) { > + return STATUS_INVALID_PARAMETER; > + } The existing code already checks for vport == NULL before calling OvsGetPid(). We can just ASSERT() here. thanks, -- Nithin _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=kQB9d9OV%2F1d0VQLP1uGwpcGjMNXh1OQMGiUOSMzDekg%3D%0A&s=458415d86f1f9aac09c2b999d37409a88c5c5d467ace09aa25aa31c0c629fc17 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev