Re: [edk2-devel] [PATCH v2 1/3] UsbNetworkPkg/UsbRndis: Add USB RNDIS devices support

["Tinh Nguyen" <tinhnguyen@os.amperecomputing.com>]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On 2/7/2023 3:20 PM, Michael Brown wrote:
> On 07/02/2023 06:21, Tinh Nguyen via groups.io wrote:
>> From: Tinh Nguyen <tinhn@os.amperecomputing.com>
>> Date: Tue, 7 Feb 2023 12:43:17 +0700
>> Subject: [PATCH] UsbNetworkPkg: Support rate limitting
>>
>> Signed-off-by: Tinh Nguyen <tinhn@os.amperecomputing.com>
>
> Thank you for extending my patch to add the PCD support.  The overall 
> patch appears still to be substantially my code: could you please 
> credit it as such?
>
Oops, sorry, my alias command generates a patch with my signature which 
I don't review on a regular


See my update below in attached file

Anyway, it is a proposal for Richard to use for his patch. And need his 
approval to pick-up

- Tinh


[-- Attachment #2: 0001-UsbNetworkPkg-Support-rate-limitting_v2.patch --]
[-- Type: text/plain, Size: 7587 bytes --]

From 1dd7ad0e590204008505ff039e77f8964210e811 Mon Sep 17 00:00:00 2001
From: Tinh Nguyen <tinhn@os.amperecomputing.com>
Date: Tue, 7 Feb 2023 12:43:17 +0700
Subject: [PATCH] UsbNetworkPkg: Support rate limitting

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 .../Protocol/EdkIIUsbEthernetProtocol.h       |  4 ++
 UsbNetworkPkg/NetworkCommon/DriverBinding.c   |  5 ++
 UsbNetworkPkg/NetworkCommon/DriverBinding.h   |  1 +
 UsbNetworkPkg/NetworkCommon/NetworkCommon.inf |  5 ++
 UsbNetworkPkg/NetworkCommon/PxeFunction.c     | 67 +++++++++++++++++--
 UsbNetworkPkg/UsbNetworkPkg.dec               |  5 ++
 6 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h b/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
index f1df1c4cbd..0ad25ad8ea 100644
--- a/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
+++ b/UsbNetworkPkg/Include/Protocol/EdkIIUsbEthernetProtocol.h
@@ -159,6 +159,10 @@ typedef struct {
   UINT8                          CurrentNodeAddress[PXE_MAC_LENGTH];
   UINT8                          BroadcastNodeAddress[PXE_MAC_LENGTH];
   EFI_USB_DEVICE_REQUEST         Request;
+  EFI_EVENT                      RateLimiter;
+  UINT32                         RateLimitingCredit;
+  UINT32                         RateLimitingCreditCount;
+  BOOLEAN                        RateLimitingEnable;
 } NIC_DATA;
 
 #define NIC_DATA_SIGNATURE  SIGNATURE_32('n', 'i', 'c', 'd')
diff --git a/UsbNetworkPkg/NetworkCommon/DriverBinding.c b/UsbNetworkPkg/NetworkCommon/DriverBinding.c
index ded334cd0a..f59bc0d5ab 100644
--- a/UsbNetworkPkg/NetworkCommon/DriverBinding.c
+++ b/UsbNetworkPkg/NetworkCommon/DriverBinding.c
@@ -347,6 +347,11 @@ NetworkCommonDriverStart (
   NicDevice->NiiProtocol.StringId[3] = 'I';
   NicDevice->DeviceHandle            = NULL;
 
+  NicDevice->NicInfo.RateLimitingEnable      = PcdGetBool (EnableRateLimiting);
+  NicDevice->NicInfo.RateLimitingCreditCount = 0;
+  NicDevice->NicInfo.RateLimitingCredit      = PcdGet32 (RateLimitingCredit);
+  NicDevice->NicInfo.RateLimiter             = NULL;
+
   ZeroMem (&NicDevice->NicInfo.Request, sizeof (EFI_USB_DEVICE_REQUEST));
 
   Status = UsbEth->UsbEthInterrupt (UsbEth, TRUE, NETWORK_COMMON_POLLING_INTERVAL, &NicDevice->NicInfo.Request);
diff --git a/UsbNetworkPkg/NetworkCommon/DriverBinding.h b/UsbNetworkPkg/NetworkCommon/DriverBinding.h
index 1aa4c86ff9..077a64719b 100644
--- a/UsbNetworkPkg/NetworkCommon/DriverBinding.h
+++ b/UsbNetworkPkg/NetworkCommon/DriverBinding.h
@@ -14,6 +14,7 @@
 #include <Library/DevicePathLib.h>
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/UefiUsbLib.h>
 #include <Protocol/UsbIo.h>
diff --git a/UsbNetworkPkg/NetworkCommon/NetworkCommon.inf b/UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
index 2691e8e70d..74a24ff7d6 100644
--- a/UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
+++ b/UsbNetworkPkg/NetworkCommon/NetworkCommon.inf
@@ -40,5 +40,10 @@
   gEfiDriverBindingProtocolGuid
   gEdkIIUsbEthProtocolGuid
 
+[Pcd]
+  gUsbNetworkPkgTokenSpaceGuid.EnableRateLimiting
+  gUsbNetworkPkgTokenSpaceGuid.RateLimitingCredit
+  gUsbNetworkPkgTokenSpaceGuid.RateLimitingFactor
+
 [Depex]
   TRUE
diff --git a/UsbNetworkPkg/NetworkCommon/PxeFunction.c b/UsbNetworkPkg/NetworkCommon/PxeFunction.c
index aae7d787db..7c67c13a5c 100644
--- a/UsbNetworkPkg/NetworkCommon/PxeFunction.c
+++ b/UsbNetworkPkg/NetworkCommon/PxeFunction.c
@@ -29,6 +29,20 @@ API_FUNC  gUndiApiTable[] = {
   UndiReceive
 };
 
+VOID
+EFIAPI
+UndiRateLimiterCallback (
+  IN  EFI_EVENT  Event,
+  IN  VOID       *Context
+  )
+{
+  NIC_DATA  *Nic = Context;
+
+  if (Nic->RateLimitingCreditCount < Nic->RateLimitingCredit) {
+    Nic->RateLimitingCreditCount++;
+  }
+}
+
 /**
   This command is used to determine the operational state of the UNDI.
 
@@ -100,9 +114,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 +131,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,
+                  PcdGet32 (RateLimitingFactor) * 10000
+                  );
+  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 +226,9 @@ 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)) {
@@ -1476,6 +1522,7 @@ Receive (
   UINT8            *BulkInData;
   UINTN            DataLength;
   EthernetHeader   *Header;
+  EFI_TPL          OriginalTpl;
 
   FrameType  = PXE_FRAME_TYPE_NONE;
   NicDevice  = UNDI_DEV_FROM_NIC (Nic);
@@ -1488,9 +1535,19 @@ Receive (
     return PXE_STATCODE_INVALID_PARAMETER;
   }
 
+  if ((Nic->RateLimitingCreditCount == 0) && (Nic->RateLimitingEnable == TRUE)) {
+    return PXE_STATCODE_NO_DATA;
+  }
+
   Status = Nic->UsbEth->UsbEthReceive (Cdb, Nic->UsbEth, (VOID *)BulkInData, &DataLength);
   if (EFI_ERROR (Status)) {
     Nic->ReceiveStatus = 0;
+    OriginalTpl = gBS->RaiseTPL (TPL_NOTIFY);
+    if (Nic->RateLimitingCreditCount != 0) {
+      Nic->RateLimitingCreditCount--;
+    }
+
+    gBS->RestoreTPL (OriginalTpl);
     return PXE_STATCODE_NO_DATA;
   }
 
diff --git a/UsbNetworkPkg/UsbNetworkPkg.dec b/UsbNetworkPkg/UsbNetworkPkg.dec
index fa7c40b7ef..8dec8024de 100644
--- a/UsbNetworkPkg/UsbNetworkPkg.dec
+++ b/UsbNetworkPkg/UsbNetworkPkg.dec
@@ -34,3 +34,8 @@
   ## Set the PCD 'UsbRndisSupport' to 'TRUE' if 'Usb Rndis device' need to be enabled.
   gUsbNetworkPkgTokenSpaceGuid.UsbRndisSupport|TRUE|BOOLEAN|0x00000003
 
+[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
+  ## Support rate limitting
+  gUsbNetworkPkgTokenSpaceGuid.EnableRateLimiting|FALSE|BOOLEAN|0x00000004
+  gUsbNetworkPkgTokenSpaceGuid.RateLimitingCredit|10|UINT32|0x00000005
+  gUsbNetworkPkgTokenSpaceGuid.RateLimitingFactor|100|UINT32|0x00000006  # unit 1ms
-- 
2.25.1