[edk2-devel] [PATCH 1/3] UsbNetworkPkg/UsbRndis: Add USB RNDIS devices support
RichardHo [何明忠] via groups.io
richardho=ami.com at groups.io
Thu Jan 12 08:36:43 UTC 2023
Hi Michael,
We add this patch in my X86 platform and use the NCM device to test IPV4 PXE boot from 1330MB ISO file.
No this patch: 35 sec to download 1330M ISO file
Add this patch: 181 sec to download 1330M ISO file
The patch will increase boot time in my X86 platform.
Thanks,
Richard
-----Original Message-----
From: Michael Brown <mcb30 at ipxe.org>
Sent: 2023年1月10日 7:50 AM
To: devel at edk2.groups.io; tinhnguyen at os.amperecomputing.com; Richard Ho (何明忠) <RichardHo at ami.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH 1/3] UsbNetworkPkg/UsbRndis: Add USB RNDIS devices support
**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**
On 08/01/2023 10:41, tinhnguyen via groups.io wrote:
> The root cause is that we spend a long time waiting for USB
> UsbBulkTransfer to complete, but if there is no data to communicate
> -> it will always time out.
Coincidentally, I have just implemented a fix for this. It works by implementing a simple credit-based rate limiter. Calls to
UsbEthReceive() are limited to 10 per second while the receive datapath is idle. No limiting takes place while the receive datapath is active.
I have tried to match the existing code style (i.e. zero explanatory comments).
Richard Ho: please consider using this rate limiting approach (or something very similar).
Patch inline below:
diff --git a/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
b/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
index df6ebc64ef..d0e2048114 100644
--- a/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
+++ b/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
@@ -160,6 +160,8 @@ typedef struct {
UINT8 CurrentNodeAddress[PXE_MAC_LENGTH];
UINT8 BroadcastNodeAddress[PXE_MAC_LENGTH];
EFI_USB_DEVICE_REQUEST Request;
+ EFI_EVENT RateLimiter;
+ UINTN RateLimitCredit;
} NIC_DATA;
#define NIC_DATA_SIGNATURE SIGNATURE_32('n', 'i', 'c', 'd') diff --git a/UsbNetworkPkg/NetworkCommon/DriverBinding.h
b/UsbNetworkPkg/NetworkCommon/DriverBinding.h
index 946727ffc9..ae1d3c201e 100644
--- a/UsbNetworkPkg/NetworkCommon/DriverBinding.h
+++ b/UsbNetworkPkg/NetworkCommon/DriverBinding.h
@@ -25,6 +25,8 @@
#define RX_BUFFER_COUNT 32
#define TX_BUFFER_COUNT 32
#define MEMORY_REQUIRE 0
+#define RATE_LIMITER_INTERVAL 1000000 // 10Hz
+#define RATE_LIMITER_BURST 10
#define UNDI_DEV_SIGNATURE SIGNATURE_32('u','n','d','i')
#define UNDI_DEV_FROM_THIS(a) CR(a, NIC_DEVICE, NiiProtocol,
UNDI_DEV_SIGNATURE)
diff --git a/UsbNetworkPkg/NetworkCommon/PxeFunction.c
b/UsbNetworkPkg/NetworkCommon/PxeFunction.c
index fd53a215a4..2a9b4f6111 100644
--- a/UsbNetworkPkg/NetworkCommon/PxeFunction.c
+++ b/UsbNetworkPkg/NetworkCommon/PxeFunction.c
@@ -29,6 +29,21 @@ API_FUNC gUndiApiTable[] = {
UndiReceive
};
+STATIC
+VOID
+EFIAPI
+UndiRateLimiterCallback (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ NIC_DATA *Nic = Context;
+
+ if (Nic->RateLimitCredit < RATE_LIMITER_BURST) {
+ Nic->RateLimitCredit++;
+ }
+}
+
/**
This command is used to determine the operational state of the UNDI.
@@ -100,9 +115,6 @@ UndiStart (
Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
Cdb->StatCode = PXE_STATCODE_INVALID_CDB;
return;
- } else {
- Cdb->StatFlags = PXE_STATFLAGS_COMMAND_COMPLETE;
- Cdb->StatCode = PXE_STATCODE_SUCCESS;
}
if (Nic->State != PXE_STATFLAGS_GET_STATE_STOPPED) { @@ -120,14 +132,46 @@ UndiStart (
Nic->PxeStart.UnMap_Mem = 0;
Nic->PxeStart.Sync_Mem = Cpb->Sync_Mem;
Nic->PxeStart.Unique_ID = Cpb->Unique_ID;
- Nic->State = PXE_STATFLAGS_GET_STATE_STARTED;
+
+ Status = gBS->CreateEvent (
+ EVT_TIMER | EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ UndiRateLimiterCallback,
+ Nic,
+ &Nic->RateLimiter
+ );
+ if (EFI_ERROR (Status)) {
+ goto ErrorCreateEvent;
+ }
+
+ Status = gBS->SetTimer (
+ Nic->RateLimiter,
+ TimerPeriodic,
+ RATE_LIMITER_INTERVAL
+ );
+ if (EFI_ERROR (Status)) {
+ goto ErrorSetTimer;
+ }
if (Nic->UsbEth->UsbEthUndi.UsbEthUndiStart != NULL) {
Status = Nic->UsbEth->UsbEthUndi.UsbEthUndiStart (Cdb, Nic);
if (EFI_ERROR (Status)) {
- Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED;
+ goto ErrorUndiStart;
}
}
+
+ Nic->State = PXE_STATFLAGS_GET_STATE_STARTED;
+ Cdb->StatFlags = PXE_STATFLAGS_COMMAND_COMPLETE; Cdb->StatCode =
+ PXE_STATCODE_SUCCESS; return;
+
+ ErrorUndiStart:
+ gBS->SetTimer (&Nic->RateLimiter, TimerCancel, 0);
+ ErrorSetTimer:
+ gBS->CloseEvent (&Nic->RateLimiter);
+ ErrorCreateEvent:
+ Cdb->StatFlags = PXE_STATFLAGS_COMMAND_FAILED; Cdb->StatCode =
+ PXE_STATCODE_DEVICE_FAILURE;
}
/**
@@ -183,6 +227,10 @@ UndiStop (
Nic->PxeStart.Sync_Mem = 0;
Nic->State = PXE_STATFLAGS_GET_STATE_STOPPED;
+ gBS->SetTimer (&Nic->RateLimiter, TimerCancel, 0);
+
+ gBS->CloseEvent (&Nic->RateLimiter);
+
if (Nic->UsbEth->UsbEthUndi.UsbEthUndiStop != NULL) {
Status = Nic->UsbEth->UsbEthUndi.UsbEthUndiStop (Cdb, Nic);
if (EFI_ERROR (Status)) {
@@ -1506,8 +1554,13 @@ Receive (
}
while (1) {
+ if (Nic->RateLimitCredit == 0) {
+ break;
+ }
+
Status = Nic->UsbEth->UsbEthReceive (Cdb, Nic->UsbEth, (VOID *)BulkInData, &DataLength);
if (EFI_ERROR (Status)) {
+ Nic->RateLimitCredit--;
break;
}
Thanks,
Michael
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98338): https://edk2.groups.io/g/devel/message/98338
Mute This Topic: https://groups.io/mt/95531719/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list