Siyuan, Some comments for this patch: 1) Please update all copyright year for the updated files. 2) Please add function description for MnpAllocTxBuf/MnpFreeTxBuf/MnpAddFreeTxBuf/MnpRecycleTxBuf in corresponding files. 3) I am curious the reason you don't put MnpRecycleTxBuf together with MnpAllocTxBuf/MnpFreeTxBuf in MnpConfig.c. Might be better to put them together? 4) There are some commented lines in MnpAllocTxBuf(), please clean up.
Best Regards, Ting -----Original Message----- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan Sent: Tuesday, December 15, 2015 3:59 PM To: edk2-de...@ml01.01.org Cc: Ye, Ting; Tian, Feng; Wu, Jiaxin Subject: [edk2] [Patch 2/2] MdeModulePkg: Update MNP driver to recycle TX buffer asynchronously. This patch updates the MNP driver to recycle TX buffer asynchronously, instead of using a while loop wait after each transmit command. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Fu Siyuan <siyuan...@intel.com> --- MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c | 177 ++++++++++++++++++---- MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h | 5 +- MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h | 24 ++- MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c | 144 +++++++++--------- MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c | 5 +- 5 files changed, 248 insertions(+), 107 deletions(-) diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c index 046d4df..70be4ac 100644 --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpConfig.c @@ -209,6 +209,139 @@ MnpFreeNbuf ( gBS->RestoreTPL (OldTpl); } +EFI_STATUS +MnpAddFreeTxBuf ( + IN OUT MNP_DEVICE_DATA *MnpDeviceData, + IN UINTN Count + ) +{ + EFI_STATUS Status; + UINT32 Index; + MNP_TX_BUF_WRAP *TxBufWrap; + + NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE); + ASSERT ((Count > 0) && (MnpDeviceData->BufferLength > 0)); + + Status = EFI_SUCCESS; + for (Index = 0; Index < Count; Index++) { + TxBufWrap = (MNP_TX_BUF_WRAP*) AllocatePool (sizeof (MNP_TX_BUF_WRAP) + MnpDeviceData->BufferLength - 1); + if (TxBufWrap == NULL) { + DEBUG ((EFI_D_ERROR, "MnpAddFreeTxBuf: TxBuf Alloc failed.\n")); + + Status = EFI_OUT_OF_RESOURCES; + break; + } + DEBUG ((EFI_D_INFO, "MnpAddFreeTxBuf: Add TxBufWrap %p, TxBuf %p\n", TxBufWrap, TxBufWrap->TxBuf)); + TxBufWrap->Signature = MNP_TX_BUF_WRAP_SIGNATURE; + TxBufWrap->InUse = FALSE; + InsertTailList (&MnpDeviceData->FreeTxBufList, &TxBufWrap->WrapEntry); + InsertTailList (&MnpDeviceData->AllTxBufList, + &TxBufWrap->AllEntry); } + + MnpDeviceData->TxBufCount += Index; + return Status; +} + + +UINT8 * +MnpAllocTxBuf ( + IN OUT MNP_DEVICE_DATA *MnpDeviceData + ) +{ + EFI_TPL OldTpl; + UINT8 *TxBuf; + EFI_STATUS Status; + LIST_ENTRY *Entry; + MNP_TX_BUF_WRAP *TxBufWrap; + + NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE); + + OldTpl = gBS->RaiseTPL (TPL_CALLBACK); + + if (IsListEmpty (&MnpDeviceData->FreeTxBufList)) { + if ((MnpDeviceData->TxBufCount + MNP_TX_BUFFER_INCREASEMENT) > MNP_MAX_TX_BUFFER_NUM) { + DEBUG ( + (EFI_D_ERROR, + "MnpAllocTxBuf: The maximum TxBuf size is reached for MNP driver instance %p.\n", + MnpDeviceData) + ); + + TxBuf = NULL; + goto ON_EXIT; + } + + Status = MnpAddFreeTxBuf (MnpDeviceData, MNP_TX_BUFFER_INCREASEMENT); + if (IsListEmpty (&MnpDeviceData->FreeTxBufList)) { + DEBUG ( + (EFI_D_ERROR, + "MnpAllocNbuf: Failed to add NET_BUFs into the FreeNbufQue, %r.\n", + Status) + ); + + TxBuf = NULL; + goto ON_EXIT; + } + } + + ASSERT (!IsListEmpty (&MnpDeviceData->FreeTxBufList)); Entry = + MnpDeviceData->FreeTxBufList.ForwardLink; + RemoveEntryList (MnpDeviceData->FreeTxBufList.ForwardLink); + //TxBufWrap = NET_LIST_USER_STRUCT_S (Entry, MNP_TX_BUF_WRAP, + WrapEntry, MNP_TX_BUF_WRAP_SIGNATURE); TxBufWrap = + NET_LIST_USER_STRUCT (Entry, MNP_TX_BUF_WRAP, WrapEntry); DEBUG + ((EFI_D_INFO, "MnpAllocTxBuf: Allocate TxBufWrap %p, TxBuf %p\n", + TxBufWrap, TxBufWrap->TxBuf)); ASSERT (TxBufWrap->Signature == MNP_TX_BUF_WRAP_SIGNATURE); TxBufWrap->InUse = TRUE; + TxBuf = TxBufWrap->TxBuf; + +ON_EXIT: + gBS->RestoreTPL (OldTpl); + + //DEBUG ((EFI_D_INFO, "MnpAllocTxBuf: allocate TxBuf %p\n", TxBuf)); + + return TxBuf; +} + + +VOID +MnpFreeTxBuf ( + IN OUT MNP_DEVICE_DATA *MnpDeviceData, + IN OUT UINT8 *TxBuf + ) +{ + MNP_TX_BUF_WRAP *TxBufWrap; + EFI_TPL OldTpl; + + NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE); + + DEBUG ((EFI_D_INFO, "MnpFreeTxBuf: recycle TxBuf %p\n", TxBuf)); + + if (TxBuf == NULL) { + return; + } + + TxBufWrap = NET_LIST_USER_STRUCT (TxBuf, MNP_TX_BUF_WRAP, TxBuf); if + (TxBufWrap->Signature != MNP_TX_BUF_WRAP_SIGNATURE) { + DEBUG ( + (EFI_D_ERROR, + "MnpFreeTxBuf: Signature check failed in MnpFreeTxBuf.\n") + ); + return; + } + + if (!TxBufWrap->InUse) { + DEBUG ( + (EFI_D_WARN, + "MnpFreeTxBuf: Duplicated recycle report from SNP.\n") + ); + return; + } + + OldTpl = gBS->RaiseTPL (TPL_CALLBACK); + InsertTailList (&MnpDeviceData->FreeTxBufList, +&TxBufWrap->WrapEntry); + TxBufWrap->InUse = FALSE; + gBS->RestoreTPL (OldTpl); +} + /** Initialize the mnp device context data. @@ -314,13 +447,9 @@ MnpInitializeDeviceData ( // // Allocate buffer pool for tx. // - MnpDeviceData->TxBuf = AllocatePool (MnpDeviceData->BufferLength); - if (MnpDeviceData->TxBuf == NULL) { - DEBUG ((EFI_D_ERROR, "MnpInitializeDeviceData: AllocatePool failed.\n")); - - Status = EFI_OUT_OF_RESOURCES; - goto ERROR; - } + InitializeListHead (&MnpDeviceData->FreeTxBufList); + InitializeListHead (&MnpDeviceData->AllTxBufList); + MnpDeviceData->TxBufCount = 0; // // Create the system poll timer. @@ -370,20 +499,6 @@ MnpInitializeDeviceData ( goto ERROR; } - // - // Create the timer for tx timeout check. - // - Status = gBS->CreateEvent ( - EVT_TIMER, - TPL_CALLBACK, - NULL, - NULL, - &MnpDeviceData->TxTimeoutEvent - ); - if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "MnpInitializeDeviceData: CreateEvent for tx timeout event failed.\n")); - } - ERROR: if (EFI_ERROR (Status)) { // @@ -405,10 +520,6 @@ ERROR: gBS->CloseEvent (MnpDeviceData->PollTimer); } - if (MnpDeviceData->TxBuf != NULL) { - FreePool (MnpDeviceData->TxBuf); - } - if (MnpDeviceData->RxNbufCache != NULL) { MnpFreeNbuf (MnpDeviceData, MnpDeviceData->RxNbufCache); } @@ -445,6 +556,10 @@ MnpDestroyDeviceData ( IN EFI_HANDLE ImageHandle ) { + LIST_ENTRY *Entry; + LIST_ENTRY *NextEntry; + MNP_TX_BUF_WRAP *TxBufWrap; + NET_CHECK_SIGNATURE (MnpDeviceData, MNP_DEVICE_DATA_SIGNATURE); // @@ -462,15 +577,21 @@ MnpDestroyDeviceData ( // // Close the event. // - gBS->CloseEvent (MnpDeviceData->TxTimeoutEvent); gBS->CloseEvent (MnpDeviceData->TimeoutCheckTimer); gBS->CloseEvent (MnpDeviceData->MediaDetectTimer); gBS->CloseEvent (MnpDeviceData->PollTimer); // - // Free the tx buffer. + // Free the Tx buffer pool. // - FreePool (MnpDeviceData->TxBuf); + NET_LIST_FOR_EACH_SAFE(Entry, NextEntry, &MnpDeviceData->AllTxBufList) { + TxBufWrap = NET_LIST_USER_STRUCT (Entry, MNP_TX_BUF_WRAP, AllEntry); + RemoveEntryList (Entry); + FreePool (TxBufWrap); + MnpDeviceData->TxBufCount--; + } + ASSERT (IsListEmpty (&MnpDeviceData->AllTxBufList)); ASSERT + (MnpDeviceData->TxBufCount == 0); // // Free the RxNbufCache. diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h b/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h index 35a9b71..ac21e7b 100644 --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpDriver.h @@ -67,7 +67,9 @@ typedef struct { LIST_ENTRY GroupAddressList; UINT32 GroupAddressCount; - EFI_EVENT TxTimeoutEvent; + LIST_ENTRY FreeTxBufList; + LIST_ENTRY AllTxBufList; + UINT32 TxBufCount; NET_BUF_QUEUE FreeNbufQue; INTN NbufCnt; @@ -90,7 +92,6 @@ typedef struct { UINT32 BufferLength; UINT32 PaddingSize; NET_BUF *RxNbufCache; - UINT8 *TxBuf; } MNP_DEVICE_DATA; #define MNP_DEVICE_DATA_FROM_THIS(a) \ diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h b/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h index f94e208..50ac65d 100644 --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpImpl.h @@ -27,6 +27,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #define MNP_INIT_NET_BUFFER_NUM 512 #define MNP_NET_BUFFER_INCREASEMENT 64 #define MNP_MAX_NET_BUFFER_NUM 65536 +#define MNP_TX_BUFFER_INCREASEMENT 64 +#define MNP_MAX_TX_BUFFER_NUM 65536 #define MNP_MAX_RCVD_PACKET_QUE_SIZE 256 @@ -92,6 +94,15 @@ typedef struct { UINT64 TimeoutTick; } MNP_RXDATA_WRAP; +#define MNP_TX_BUF_WRAP_SIGNATURE SIGNATURE_32 ('M', 'T', 'B', 'W') + +typedef struct { + UINT32 Signature; + LIST_ENTRY WrapEntry; // Link to FreeTxBufList + LIST_ENTRY AllEntry; // Link to AllTxBufList + BOOLEAN InUse; + UINT8 TxBuf[1]; +} MNP_TX_BUF_WRAP; /** Initialize the mnp device context data. @@ -343,7 +354,7 @@ MnpIsValidTxToken ( length. **/ -VOID +EFI_STATUS MnpBuildTxPacket ( IN MNP_SERVICE_DATA *MnpServiceData, IN EFI_MANAGED_NETWORK_TRANSMIT_DATA *TxData, @@ -448,6 +459,17 @@ MnpFreeNbuf ( IN OUT NET_BUF *Nbuf ); +UINT8 * +MnpAllocTxBuf ( + IN OUT MNP_DEVICE_DATA *MnpDeviceData + ); + +VOID +MnpFreeTxBuf ( + IN OUT MNP_DEVICE_DATA *MnpDeviceData, + IN OUT UINT8 *TxBuf + ); + /** Remove the received packets if timeout occurs. diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c b/MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c index 7f03b84..1014f9c 100644 --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c @@ -102,6 +102,32 @@ MnpIsValidTxToken ( return TRUE; } +EFI_STATUS +MnpRecycleTxBuf ( + IN OUT MNP_DEVICE_DATA *MnpDeviceData + ) +{ + UINT8 *TxBuf; + EFI_SIMPLE_NETWORK_PROTOCOL *Snp; + EFI_STATUS Status; + + Snp = MnpDeviceData->Snp; + + do { + TxBuf = NULL; + Status = Snp->GetStatus (Snp, NULL, (VOID **) &TxBuf); + if (EFI_ERROR (Status)) { + return Status; + } + + if (TxBuf != NULL) { + MnpFreeTxBuf (MnpDeviceData, TxBuf); + } + } while (TxBuf != NULL); + + return EFI_SUCCESS; +} + /** Build the packet to transmit from the TxData passed in. @@ -114,7 +140,7 @@ MnpIsValidTxToken ( length. **/ -VOID +EFI_STATUS MnpBuildTxPacket ( IN MNP_SERVICE_DATA *MnpServiceData, IN EFI_MANAGED_NETWORK_TRANSMIT_DATA *TxData, @@ -125,14 +151,33 @@ MnpBuildTxPacket ( EFI_SIMPLE_NETWORK_MODE *SnpMode; UINT8 *DstPos; UINT16 Index; - MNP_DEVICE_DATA *MnpDerviceData; - - MnpDerviceData = MnpServiceData->MnpDeviceData; + MNP_DEVICE_DATA *MnpDeviceData; + UINT8 *TxBuf; + EFI_STATUS Status; + + MnpDeviceData = MnpServiceData->MnpDeviceData; // - // Reserve space for vlan tag. + // Try to recyle some TxBufs before get the TxBuf from the free buffer pool. + // + Status = MnpRecycleTxBuf (MnpDeviceData); if (EFI_ERROR (Status)) { + return Status; + } + + TxBuf = MnpAllocTxBuf (MnpDeviceData); if (TxBuf == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + // + // Reserve space for vlan tag if needed. // - *PktBuf = MnpDerviceData->TxBuf + NET_VLAN_TAG_LEN; + if (MnpServiceData->VlanId != 0) { + *PktBuf = TxBuf + NET_VLAN_TAG_LEN; } else { + *PktBuf = TxBuf; + } if ((TxData->DestinationAddress == NULL) && (TxData->FragmentCount == 1)) { CopyMem ( @@ -148,7 +193,7 @@ MnpBuildTxPacket ( // one fragment, copy the data into the packet buffer. Reserve the // media header space if necessary. // - SnpMode = MnpDerviceData->Snp->Mode; + SnpMode = MnpDeviceData->Snp->Mode; DstPos = *PktBuf; *PktLen = 0; if (TxData->DestinationAddress != NULL) { @@ -177,6 +222,8 @@ MnpBuildTxPacket ( // *PktLen += TxData->DataLength + TxData->HeaderLength; } + + return EFI_SUCCESS; } @@ -205,14 +252,13 @@ MnpSyncSendPacket ( EFI_SIMPLE_NETWORK_PROTOCOL *Snp; EFI_MANAGED_NETWORK_TRANSMIT_DATA *TxData; UINT32 HeaderSize; - UINT8 *TxBuf; MNP_DEVICE_DATA *MnpDeviceData; UINT16 ProtocolType; MnpDeviceData = MnpServiceData->MnpDeviceData; Snp = MnpDeviceData->Snp; TxData = Token->Packet.TxData; - + Token->Status = EFI_SUCCESS; HeaderSize = Snp->Mode->MediaHeaderSize - TxData->HeaderLength; // @@ -224,19 +270,7 @@ MnpSyncSendPacket ( // Media not present, skip packet transmit and report EFI_NO_MEDIA // DEBUG ((EFI_D_WARN, "MnpSyncSendPacket: No network cable detected.\n")); - Status = EFI_NO_MEDIA; - goto SIGNAL_TOKEN; - } - - // - // Start the timeout event. - // - Status = gBS->SetTimer ( - MnpDeviceData->TxTimeoutEvent, - TimerRelative, - MNP_TX_TIMEOUT_TIME - ); - if (EFI_ERROR (Status)) { + Token->Status = EFI_NO_MEDIA; goto SIGNAL_TOKEN; } @@ -250,64 +284,24 @@ MnpSyncSendPacket ( ProtocolType = TxData->ProtocolType; } - for (;;) { - // - // Transmit the packet through SNP. - // - Status = Snp->Transmit ( - Snp, - HeaderSize, - Length, - Packet, - TxData->SourceAddress, - TxData->DestinationAddress, - &ProtocolType - ); - if ((Status != EFI_SUCCESS) && (Status != EFI_NOT_READY)) { - Status = EFI_DEVICE_ERROR; - break; - } - - // - // If Status is EFI_SUCCESS, the packet is put in the transmit queue. - // if Status is EFI_NOT_READY, the transmit engine of the network interface is busy. - // Both need to sync SNP. - // - TxBuf = NULL; - do { - // - // Get the recycled transmit buffer status. - // - Snp->GetStatus (Snp, NULL, (VOID **) &TxBuf); - - if (!EFI_ERROR (gBS->CheckEvent (MnpDeviceData->TxTimeoutEvent))) { - Status = EFI_TIMEOUT; - break; - } - } while (TxBuf == NULL); - - if ((Status == EFI_SUCCESS) || (Status == EFI_TIMEOUT)) { - break; - } else { - // - // Status is EFI_NOT_READY. Restart the timer event and call Snp->Transmit again. - // - gBS->SetTimer ( - MnpDeviceData->TxTimeoutEvent, - TimerRelative, - MNP_TX_TIMEOUT_TIME - ); - } - } - // - // Cancel the timer event. + // Transmit the packet through SNP. // - gBS->SetTimer (MnpDeviceData->TxTimeoutEvent, TimerCancel, 0); + Status = Snp->Transmit ( + Snp, + HeaderSize, + Length, + Packet, + TxData->SourceAddress, + TxData->DestinationAddress, + &ProtocolType + ); + if (EFI_ERROR (Status)) { + Token->Status = EFI_DEVICE_ERROR; + } SIGNAL_TOKEN: - Token->Status = Status; gBS->SignalEvent (Token->Event); // diff --git a/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c b/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c index 4c0f3dd..195fcae 100644 --- a/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c +++ b/MdeModulePkg/Universal/Network/MnpDxe/MnpMain.c @@ -552,7 +552,10 @@ MnpTransmit ( // // Build the tx packet // - MnpBuildTxPacket (MnpServiceData, Token->Packet.TxData, &PktBuf, &PktLen); + Status = MnpBuildTxPacket (MnpServiceData, Token->Packet.TxData, + &PktBuf, &PktLen); if (EFI_ERROR (Status)) { + goto ON_EXIT; + } // // OK, send the packet synchronously. -- 2.5.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
