Re: [Patch] Nt32Pkg/SnpNt32Dxe: Fix hang issue when multiple network interfaces existed

["Fu, Siyuan" <siyuan.fu@intel.com>]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiaxin Wu
Sent: 2017年5月5日 11:31
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [edk2] [Patch] Nt32Pkg/SnpNt32Dxe: Fix hang issue when multiple network interfaces existed

Currently all the network interfaces share the one recycled transmit buffer array, which is used to store the recycled buffer address. However, those recycled buffers are allocated by the different MNP interface if the multiple network interfaces existed. Then, SNP GetStatus may return one recycled transmit buffer address to the another MNP interface, which may result in the MNP driver hang after 'reconnect -r' operation.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 Nt32Pkg/SnpNt32Dxe/SnpNt32.c | 50 ++++++++++++++++++++++----------------------
 Nt32Pkg/SnpNt32Dxe/SnpNt32.h | 31 ++++++++++++++-------------
 2 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c index 9018800..c1ef324 100644
--- a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
+++ b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
 
@@ -42,13 +42,10 @@ SNPNT32_GLOBAL_DATA         gSnpNt32GlobalData = {
   {
     0,
     0,
     EfiLockUninitialized
   },                          //  Lock
-  NULL,                       //  RecycledTxBuf
-  0,                          //  RecycledTxBufCount
-  32,                         //  MaxRecycledTxBuf
   //
   //  Private functions
   //
   SnpNt32InitializeGlobalData,            //  InitializeGlobalData
   SnpNt32InitializeInstanceData,          //  InitializeInstanceData
@@ -394,10 +391,13 @@ SNPNT32_INSTANCE_DATA gSnpNt32InstanceTemplate = {
   SNP_NT32_INSTANCE_SIGNATURE,            //  Signature
   {
     NULL,
     NULL
   },                                      //  Entry
+  NULL,                                   //  RecycledTxBuf
+  0,                                      //  RecycledTxBufCount
+  32,                                     //  MaxRecycledTxBuf
   NULL,                                   //  GlobalData
   NULL,                                   //  DeviceHandle
   NULL,                                   //  DevicePath
   {                                       //  Snp
     EFI_SIMPLE_NETWORK_PROTOCOL_REVISION, //  Revision @@ -863,20 +863,17 @@ SnpNt32GetStatus (
   OUT UINT32                     *InterruptStatus,
   OUT VOID                       **TxBuffer
   )
 {
   SNPNT32_INSTANCE_DATA *Instance;
-  SNPNT32_GLOBAL_DATA   *GlobalData;
 
   Instance    = SNP_NT32_INSTANCE_DATA_FROM_SNP_THIS (This);
 
-  GlobalData  = Instance->GlobalData;
-
   if (TxBuffer != NULL) {
-    if (GlobalData->RecycledTxBufCount != 0) {
-      GlobalData->RecycledTxBufCount --;
-      *((UINT8 **) TxBuffer)    = (UINT8 *) (UINTN)GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount];
+    if (Instance->RecycledTxBufCount != 0) {
+      Instance->RecycledTxBufCount --;
+      *((UINT8 **) TxBuffer)    = (UINT8 *) (UINTN)Instance->RecycledTxBuf[Instance->RecycledTxBufCount];
     } else {
       *((UINT8 **) TxBuffer)    = NULL;
     }
   }
 
@@ -959,26 +956,26 @@ SnpNt32Transmit (
   EfiReleaseLock (&GlobalData->Lock);
 
   if (ReturnValue < 0) {
     return EFI_DEVICE_ERROR;
   } else {
-    if ((GlobalData->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= SNP_MAX_TX_BUFFER_NUM) {
+    if ((Instance->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= 
+ SNP_MAX_TX_BUFFER_NUM) {
       return EFI_NOT_READY;
     }
 
-    if (GlobalData->RecycledTxBufCount < GlobalData->MaxRecycledTxBuf) {
-      GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount] = (UINT64) Buffer;
-      GlobalData->RecycledTxBufCount ++;
+    if (Instance->RecycledTxBufCount < Instance->MaxRecycledTxBuf) {
+      Instance->RecycledTxBuf[Instance->RecycledTxBufCount] = (UINT64) Buffer;
+      Instance->RecycledTxBufCount ++;
     } else {
-      Tmp = AllocatePool (sizeof (UINT64) * (GlobalData->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT));
+      Tmp = AllocatePool (sizeof (UINT64) * (Instance->MaxRecycledTxBuf 
+ + SNP_TX_BUFFER_INCREASEMENT));
       if (Tmp == NULL) {
         return EFI_DEVICE_ERROR;
       }
-      CopyMem (Tmp, GlobalData->RecycledTxBuf, sizeof (UINT64) * GlobalData->RecycledTxBufCount);
-      FreePool (GlobalData->RecycledTxBuf);
-      GlobalData->RecycledTxBuf    =  Tmp;
-      GlobalData->MaxRecycledTxBuf += SNP_TX_BUFFER_INCREASEMENT;
+      CopyMem (Tmp, Instance->RecycledTxBuf, sizeof (UINT64) * Instance->RecycledTxBufCount);
+      FreePool (Instance->RecycledTxBuf);
+      Instance->RecycledTxBuf    =  Tmp;
+      Instance->MaxRecycledTxBuf += SNP_TX_BUFFER_INCREASEMENT;
     }
   }
 
   return EFI_SUCCESS;
 }
@@ -1114,15 +1111,10 @@ SnpNt32InitializeGlobalData (
   InterfaceCount        = MAX_INTERFACE_INFO_NUMBER;
 
   InitializeListHead (&This->InstanceList);
   EfiInitializeLock (&This->Lock, TPL_CALLBACK);
 
-  This->RecycledTxBuf = AllocatePool (sizeof (UINT64) * This->MaxRecycledTxBuf);
-  if (This->RecycledTxBuf == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
   //
   //  Get the WinNT thunk
   //
   Status = gBS->LocateProtocol (&gEfiWinNtThunkProtocolGuid, NULL, (VOID **)&This->WinNtThunk);
 
@@ -1211,11 +1203,11 @@ SnpNt32InitializeGlobalData (
   //
   //  Create fake SNP instances
   //
   for (Index = 0; Index < InterfaceCount; Index++) {
 
-    Instance = AllocatePool (sizeof (SNPNT32_INSTANCE_DATA));
+    Instance = AllocateZeroPool (sizeof (SNPNT32_INSTANCE_DATA));
 
     if (NULL == Instance) {
       Status = EFI_OUT_OF_RESOURCES;
       goto ErrorReturn;
     }
@@ -1223,10 +1215,18 @@ SnpNt32InitializeGlobalData (
     //  Copy the content from a template
     //
     CopyMem (Instance, &gSnpNt32InstanceTemplate, sizeof (SNPNT32_INSTANCE_DATA));
 
     //
+    // Allocate the RecycledTxBuf.
+    //
+    Instance->RecycledTxBuf = AllocatePool (sizeof (UINT64) * Instance->MaxRecycledTxBuf);
+    if (Instance->RecycledTxBuf == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    //
     //  Set the interface information.
     //
     CopyMem (&Instance->InterfaceInfo, &NetInterfaceInfoBuffer[Index], sizeof(Instance->InterfaceInfo));
     //
     //  Initialize this instance
diff --git a/Nt32Pkg/SnpNt32Dxe/SnpNt32.h b/Nt32Pkg/SnpNt32Dxe/SnpNt32.h index cb95c57..5625134 100644
--- a/Nt32Pkg/SnpNt32Dxe/SnpNt32.h
+++ b/Nt32Pkg/SnpNt32Dxe/SnpNt32.h
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
 
@@ -159,24 +159,10 @@ struct _SNPNT32_GLOBAL_DATA {
   NT_NET_UTILITY_TABLE              NtNetUtilityTable;
 
   EFI_LOCK                          Lock;
 
   //
-  // Array of the recycled transmit buffer address.
-  //
-  UINT64                            *RecycledTxBuf;
-
-  //
-  // Current number of recycled buffer pointers in RecycledTxBuf.
-  //
-  UINT32                             RecycledTxBufCount;
-
-  // The maximum number of recycled buffer pointers in RecycledTxBuf.
-  //
-  UINT32                             MaxRecycledTxBuf;
-
-  //
   //  Private functions
   //
   SNPNT32_INITIALIZE_GLOBAL_DATA    InitializeGlobalData;
   SNPNT32_INITIALIZE_INSTANCE_DATA  InitializeInstanceData;
   SNPNT32_CLOSE_INSTANCE            CloseInstance;
@@ -193,10 +179,25 @@ struct _SNPNT32_INSTANCE_DATA {
   //
   //  List entry use for linking with other instance
   //
   LIST_ENTRY                  Entry;
 
+  //
+  // Array of the recycled transmit buffer address.
+  //
+  UINT64                      *RecycledTxBuf;
+
+  //
+  // Current number of recycled buffer pointers in RecycledTxBuf.
+  //
+  UINT32                      RecycledTxBufCount;
+
+  //
+  // The maximum number of recycled buffer pointers in RecycledTxBuf.
+  //
+  UINT32                      MaxRecycledTxBuf;
+
   SNPNT32_GLOBAL_DATA         *GlobalData;
 
   EFI_HANDLE                  DeviceHandle;
   EFI_DEVICE_PATH_PROTOCOL    *DevicePath;
 
--
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel