Hi Richard,
I've continued working on my USB EEM support, and just pushed my changes
to https://github.com/bcran/edk2/tree/usb-net .
I was wondering when you think you might have the next set of patches
ready for review?
Thanks.
Rebecca Cran
On 1/12/23 01:36, RichardHo [何明忠] via groups.io wrote:
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 <mc...@ipxe.org>
Sent: 2023年1月10日 7:50 AM
To: devel@edk2.groups.io; tinhngu...@os.amperecomputing.com; Richard Ho (何明忠)
<richar...@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 (#98943): https://edk2.groups.io/g/devel/message/98943
Mute This Topic: https://groups.io/mt/95531719/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
