From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mx.groups.io with SMTP id smtpd.web11.87782.1674250563118269739 for ; Fri, 20 Jan 2023 13:36:03 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@bsdio.com header.s=fm1 header.b=idxJ4wrF; spf=pass (domain: bsdio.com, ip: 66.111.4.29, mailfrom: rebecca@bsdio.com) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 56B9D5C00D6; Fri, 20 Jan 2023 16:36:02 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Fri, 20 Jan 2023 16:36:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdio.com; h=cc :content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1674250562; x= 1674336962; bh=QFSZEZmMYCGi/SZV1r4LwCcNpxLItsuGnpCMqrxUyec=; b=i dxJ4wrF+LdDCDOCAjle33o7Ajk4imdoAsUY03xQaZr75/gYNYayXcIH3CdIbTxRe abn8hHqj1imytQhl3srUdV6LSnawCZJpex7Y3IWys25sooq+oLoQSSztLQemgJbH keYmpmqQ4aGxy8AlunEyfJXkh3jRe6pqNkDWL/Zikc2WxLacJbRy+N5c3jOF6SjQ hrALnbea7CQMgkStkh0AXYPTpwMqLqtGd409mS+AJJkGvnM4nrGlGTQXrY0edbOW XkNOOPZZmLAoE/O6xN8KblADRRyMFCPXZ0FSoLzzeIurtIWPwMncKZZERkstwGW/ dT6H4dzHyR8tDIwYnPahQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:feedback-id:feedback-id:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to:x-me-proxy:x-me-proxy:x-me-sender :x-me-sender:x-sasl-enc; s=fm3; t=1674250562; x=1674336962; bh=Q FSZEZmMYCGi/SZV1r4LwCcNpxLItsuGnpCMqrxUyec=; b=l1uvOoiXQfORY3qM1 FfMtODnaQGpixcQ+K/PdWow4+cOIN6k6qkrizS718rsKjWGuTMRjzKkzsE9HoXYu 2HewoKwZ7En5JAUK56wVeqhumwQHRsgAmJ3LojUIKgeOf4we+3qCuCwJCaX0DJnM FmM95kkbGEAmLlQlUMUmJoyYThkOyc9wZywYnKm1mG5saqP38d8AYfitadAzsoJO eeHCeHLCJnwEpKLHdidMXVqwVetZa9CbsoXl9ER6kRLbrJutHe9Et29Mdc1bIPg6 CnlSFf84Xu6u3I6Dmdpo+H54jxTw5f5iNGGPuvOLHthhyX3zsgAjTjkUVdRb6YP6 459Dg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrudduvddgudehgecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepkfffgggfuffvfhfhjggtgfesth ekredttdefjeenucfhrhhomheptfgvsggvtggtrgcuvehrrghnuceorhgvsggvtggtrges sghsughiohdrtghomheqnecuggftrfgrthhtvghrnhepkeeuudeggeetieefleeghfeihe dttdfgledtgeegkeeiledugfehveekleeijefgnecuffhomhgrihhnpehgihhthhhusgdr tghomhdpghhrohhuphhsrdhiohenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehrvggsvggttggrsegsshguihhordgtohhm X-ME-Proxy: Feedback-ID: i5b994698:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 20 Jan 2023 16:36:01 -0500 (EST) Message-ID: <33d701d7-813c-66df-2a38-91e759da3509@bsdio.com> Date: Fri, 20 Jan 2023 14:36:00 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [edk2-devel] [PATCH 1/3] UsbNetworkPkg/UsbRndis: Add USB RNDIS devices support To: devel@edk2.groups.io, richardho@ami.com, Michael Brown , "tinhnguyen@os.amperecomputing.com" References: <1271.1673174497155150644@groups.io> <0102018598f12261-59c6c0fd-55a7-4b2d-9517-0470d71bb36a-000000@eu-west-1.amazonses.com> From: "Rebecca Cran" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 > Sent: 2023年1月10日 7:50 AM > To: devel@edk2.groups.io; tinhnguyen@os.amperecomputing.com; Richard Ho (何明忠) > 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. > > > > >