Re: [edk2] [PATCH v2 5/5] NetworkPkg/UefiPxeBcDxe: Use the specified MTFTP windowsize.

2018-09-25 Thread Laszlo Ersek
On 09/25/18 03:11, Jiaxin Wu wrote:
> *v2: Since the new PCD (PcdPxeTftpWindowSize) was renamed/defined in
> NetworkPkg instead of MdeModulePkg, this new version is to update the
> consuming PXE driver.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=886
>
> This patch is to use the specified MTFTP windowsize to benefit the PXE
> download performance.
>
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Shao Ming 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin 
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c  |  10 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.c | 137 +--
>  NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.h |   6 +-
>  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf |   3 +
>  4 files changed, 121 insertions(+), 35 deletions(-)

I compared this variant against the v1 posting. The difference is very
small (as you explain in the blurb):

> diff --git a/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf 
> b/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> index 949596c029be..e2a0eb44b1fc 100644
> --- a/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +++ b/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> @@ -55,6 +55,7 @@ [Sources]
>  [Packages]
>MdePkg/MdePkg.dec
>MdeModulePkg/MdeModulePkg.dec
> +  NetworkPkg/NetworkPkg.dec
>
>
>  [LibraryClasses]
> @@ -107,7 +108,7 @@ [Guids]
>
>  [Pcd]
>gEfiMdeModulePkgTokenSpaceGuid.PcdTftpBlockSize  ## SOMETIMES_CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdTftpWindowSize ## SOMETIMES_CONSUMES
> +  gEfiNetworkPkgTokenSpaceGuid.PcdPxeTftpWindowSize## SOMETIMES_CONSUMES
>
>  [UserExtensions.TianoCore."ExtraFiles"]
>UefiPxeBcDxeExtra.uni
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c 
> b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> index db463d1b11fb..468b38d887d8 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> @@ -874,9 +874,9 @@ EfiPxeBcMtftp (
>Mode  = Private->PxeBc.Mode;
>
>//
> -  // Get PcdTftpWindowSize.
> +  // Get PcdPxeTftpWindowSize.
>//
> -  WindowSize   = (UINTN) PcdGet64 (PcdTftpWindowSize);
> +  WindowSize = (UINTN) PcdGet64 (PcdPxeTftpWindowSize);
>
>if (Mode->UsingIpv6) {
>  if (!NetIp6IsValidUnicast (>v6)) {

Also, the renaming of the PCD hasn't changed its default value (4), its
data type (UINT64), or its "flavor" ([PcdsFixedAtBuild,
PcdsPatchableInModule]). Therefore, in this specific case, I think we
should carry forward my T-b for this patch (patch #5) as well, from:

bbd4d8e9-6470-8cc8-ff7d-6b154adbd7ce@redhat.com">http://mid.mail-archive.com/bbd4d8e9-6470-8cc8-ff7d-6b154adbd7ce@redhat.com

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v2 5/5] NetworkPkg/UefiPxeBcDxe: Use the specified MTFTP windowsize.

2018-09-24 Thread Jiaxin Wu
*v2: Since the new PCD (PcdPxeTftpWindowSize) was renamed/defined in
NetworkPkg instead of MdeModulePkg, this new version is to update the
consuming PXE driver.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=886

This patch is to use the specified MTFTP windowsize to benefit the PXE
download performance.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Shao Ming 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c  |  10 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.c | 137 +--
 NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.h |   6 +-
 NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf |   3 +
 4 files changed, 121 insertions(+), 35 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
index 13396903f5..468b38d887 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
@@ -847,11 +847,11 @@ EfiPxeBcMtftp (
   EFI_MTFTP4_CONFIG_DATA  Mtftp4Config;
   EFI_MTFTP6_CONFIG_DATA  Mtftp6Config;
   VOID*Config;
   EFI_STATUS  Status;
   EFI_PXE_BASE_CODE_IP_FILTER IpFilter;
-
+  UINTN   WindowSize;
 
   if ((This == NULL) ||
   (Filename == NULL) ||
   (BufferSize == NULL) ||
   (ServerIp == NULL) ||
@@ -871,10 +871,15 @@ EfiPxeBcMtftp (
   Config= NULL;
   Status= EFI_DEVICE_ERROR;
   Private   = PXEBC_PRIVATE_DATA_FROM_PXEBC (This);
   Mode  = Private->PxeBc.Mode;
 
+  //
+  // Get PcdPxeTftpWindowSize.
+  //
+  WindowSize = (UINTN) PcdGet64 (PcdPxeTftpWindowSize);
+
   if (Mode->UsingIpv6) {
 if (!NetIp6IsValidUnicast (>v6)) {
   return EFI_INVALID_PARAMETER;
 }
   } else {
@@ -928,10 +933,11 @@ EfiPxeBcMtftp (
 Status = PxeBcTftpGetFileSize (
Private,
Config,
Filename,
BlockSize,
+   (WindowSize > 1) ?  : NULL,
BufferSize
);
 
 break;
 
@@ -942,10 +948,11 @@ EfiPxeBcMtftp (
 Status = PxeBcTftpReadFile (
Private,
Config,
Filename,
BlockSize,
+   (WindowSize > 1) ?  : NULL,
BufferPtr,
BufferSize,
DontUseBuffer
);
 
@@ -974,10 +981,11 @@ EfiPxeBcMtftp (
 Status = PxeBcTftpReadDirectory (
Private,
Config,
Filename,
BlockSize,
+   (WindowSize > 1) ?  : NULL,
BufferPtr,
BufferSize,
DontUseBuffer
);
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.c
index 270190d42e..9725fb40dd 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcMtftp.c
@@ -17,11 +17,12 @@
 
 CHAR8 *mMtftpOptions[PXE_MTFTP_OPTION_MAXIMUM_INDEX] = {
   "blksize",
   "timeout",
   "tsize",
-  "multicast"
+  "multicast",
+  "windowsize"
 };
 
 
 /**
   This is a callback function when packets are received or transmitted in 
Mtftp driver.
@@ -120,28 +121,31 @@ EFI_STATUS
 PxeBcMtftp6GetFileSize (
   IN PXEBC_PRIVATE_DATA   *Private,
   IN EFI_MTFTP6_CONFIG_DATA   *Config,
   IN UINT8*Filename,
   IN UINTN*BlockSize,
+  IN UINTN*WindowSize,
   IN OUT UINT64   *BufferSize
   )
 {
   EFI_MTFTP6_PROTOCOL *Mtftp6;
-  EFI_MTFTP6_OPTION   ReqOpt[2];
+  EFI_MTFTP6_OPTION   ReqOpt[3];
   EFI_MTFTP6_PACKET   *Packet;
   EFI_MTFTP6_OPTION   *Option;
   UINT32  PktLen;
-  UINT8   OptBuf[128];
+  UINT8   OptBuf[PXE_MTFTP_OPTBUF_MAXNUM_INDEX];
+  UINTN   OptBufSize;
   UINT32  OptCnt;
   EFI_STATUS  Status;
 
   *BufferSize   = 0;
   Status= EFI_DEVICE_ERROR;
   Mtftp6= Private->Mtftp6;
   Packet= NULL;
   Option= NULL;
   PktLen= 0;
+  OptBufSize= PXE_MTFTP_OPTBUF_MAXNUM_INDEX;
   OptCnt= 1;
   Config->InitialServerPort = PXEBC_BS_DOWNLOAD_PORT;
 
   Status = Mtftp6->Configure (Mtftp6, Config);
   if (EFI_ERROR (Status)) {
@@ -150,17 +154,26 @@ PxeBcMtftp6GetFileSize (
 
   //
   // Build the required options for get info.
   //
   ReqOpt[0].OptionStr = (UINT8 *) mMtftpOptions[PXE_MTFTP_OPTION_TSIZE_INDEX];
-  PxeBcUintnToAscDec (0, OptBuf, PXE_MTFTP_OPTBUF_MAXNUM_INDEX);
+  PxeBcUintnToAscDec (0, OptBuf, OptBufSize);
   ReqOpt[0].ValueStr  = OptBuf;
 
   if (BlockSize != NULL) {
-ReqOpt[1].OptionStr