[ovs-dev] [PATCH v3] datapath-windows: Avoid BSOD when switch context is NULL

Sorin Vinturis svinturis at cloudbasesolutions.com
Mon Nov 17 17:32:40 UTC 2014


Thanks!

-----Original Message-----
From: Eitan Eliahu [mailto:eliahue at vmware.com] 
Sent: Monday, 17 November, 2014 19:24
To: Sorin Vinturis
Subject: RE: [ovs-dev] [PATCH v3] datapath-windows: Avoid BSOD when switch context is NULL

Acked-by: Eitan Eliahu <eliahue at vmware.com>

Thanks Sorin.
Eitan

-----Original Message-----
From: Sorin Vinturis [mailto:svinturis at cloudbasesolutions.com] 
Sent: Monday, November 17, 2014 2:49 AM
To: Eitan Eliahu
Subject: RE: [ovs-dev] [PATCH v3] datapath-windows: Avoid BSOD when switch context is NULL

Hi Eitan,



As we already discussed, this patch solves the BSOD that appears when sending a request to the OVS driver when the extension is disabled. I would appreciate your feedback on it. If you think it would help, I can provide a detailed analysis of this issue.



Thank you,

Sorin





-----Original Message-----

From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Sorin Vinturis

Sent: Friday, 14 November, 2014 15:24

To: dev at openvswitch.org

Subject: [ovs-dev] [PATCH v3] datapath-windows: Avoid BSOD when switch context is NULL



I came around a BSOD that happened when trying to access pidHashLock from the gOvsSwitchContext, which was NULL. The stop happened in OvsAcquirePidHashLock function.



To reproduce this BSOD, make sure the extension is enabled and running, disable it and, after that, execute 'ovs-dpctl.exe show'. The BSOD is triggered afterwards.



Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>

Reported-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>

Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_53&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=QFzs2rDv9anxLbASMeVWWf9QTXSZBnREVgUGfNxsXlM&s=fBeUX0Zf27x8xsz1g3SQ7IoP3k_pMjycxTRsVcSIHxM&e= 

---

 datapath-windows/ovsext/BufferMgmt.c |  8 ++++++--

 datapath-windows/ovsext/Datapath.c   |  5 ++++-

 datapath-windows/ovsext/Tunnel.c     |  3 ++-

 datapath-windows/ovsext/User.c       | 11 +++++++----

 4 files changed, 19 insertions(+), 8 deletions(-)



diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-windows/ovsext/BufferMgmt.c

index e0377c1..1a68575 100644

--- a/datapath-windows/ovsext/BufferMgmt.c

+++ b/datapath-windows/ovsext/BufferMgmt.c

@@ -474,15 +474,19 @@ OvsAllocateVariableSizeNBL(PVOID ovsContext,  {

     PNET_BUFFER_LIST nbl = NULL;

     POVS_SWITCH_CONTEXT context = (POVS_SWITCH_CONTEXT)ovsContext;

-    POVS_NBL_POOL ovsPool = &context->ovsPool;

+    POVS_NBL_POOL ovsPool = NULL;

     POVS_BUFFER_CONTEXT ctx;

     UINT32 realSize;

     PMDL mdl;

     NDIS_STATUS status;

     PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO info;

-    if (size == 0) {

+

+    if ((NULL == ovsContext) || (0 == size)) {

         return NULL;

     }

+

+    ovsPool = &context->ovsPool;

+

     realSize = MEM_ALIGN_SIZE(size + headRoom);

 

     mdl = OvsAllocateMDLAndData(ovsPool->ndisHandle, realSize); diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c

index 1b504a2..d8962bd 100644

--- a/datapath-windows/ovsext/Datapath.c

+++ b/datapath-windows/ovsext/Datapath.c

@@ -994,9 +994,10 @@ OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,  {

     BOOLEAN writeOk;

     OVS_MESSAGE msgOutTmp;

-    OVS_DATAPATH *datapath = &ovsSwitchContext->datapath;

+    OVS_DATAPATH *datapath = NULL;

     PNL_MSG_HDR nlMsg;

 

+    ASSERT(ovsSwitchContext);

     ASSERT(NlBufAt(nlBuf, 0, 0) != 0 && NlBufRemLen(nlBuf) >= sizeof *msgIn);

 

     msgOutTmp.nlMsg.nlmsgType = OVS_WIN_NL_DATAPATH_FAMILY_ID; @@ -1009,6 +1010,8 @@ OvsDpFillInfo(POVS_SWITCH_CONTEXT ovsSwitchContext,

     msgOutTmp.genlMsg.reserved = 0;

 

     msgOutTmp.ovsHdr.dp_ifindex = ovsSwitchContext->dpNo;

+    

+    datapath = &ovsSwitchContext->datapath;

 

     writeOk = NlMsgPutHead(nlBuf, (PCHAR)&msgOutTmp, sizeof msgOutTmp);

     if (writeOk) {

diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c

index b55a223..fed58f1 100644

--- a/datapath-windows/ovsext/Tunnel.c

+++ b/datapath-windows/ovsext/Tunnel.c

@@ -224,9 +224,10 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,

     OvsCompletionList completionList;

     KIRQL irql;

     ULONG SendFlags = NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;

-    OVS_DATAPATH *datapath = &gOvsSwitchContext->datapath;

+    OVS_DATAPATH *datapath = NULL;

 

     ASSERT(gOvsSwitchContext);

+    datapath = &gOvsSwitchContext->datapath;

 

     /* Fill the tunnel key */

     status = OvsSlowPathDecapVxlan(pNbl, &tunnelKey); diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index fc27f7d..ca60cd0 100644

--- a/datapath-windows/ovsext/User.c

+++ b/datapath-windows/ovsext/User.c

@@ -141,10 +141,13 @@ OvsCleanupPacketQueue(POVS_OPEN_INSTANCE instance)

         OvsFreeMemory(queue);

     }

 

-    /* Remove the instance from pidHashArray */

-    OvsAcquirePidHashLock();

-    OvsDelPidInstance(gOvsSwitchContext, instance->pid);

-    OvsReleasePidHashLock();

+    /* Verify if gOvsSwitchContext exists. */

+    if (gOvsSwitchContext) {

+        /* Remove the instance from pidHashArray */

+        OvsAcquirePidHashLock();

+        OvsDelPidInstance(gOvsSwitchContext, instance->pid);

+        OvsReleasePidHashLock();

+    }

 }

 

 NTSTATUS

--

1.9.0.msysgit.0

_______________________________________________

dev mailing list

dev at openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=QFzs2rDv9anxLbASMeVWWf9QTXSZBnREVgUGfNxsXlM&s=Uk21NpNdNAF5FdfZU8g5sp0ZKLTMGhjR3k3ldmGbBK4&e= 



More information about the dev mailing list