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

[Laszlo Ersek <lersek@redhat.com>]

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 <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Shao Ming <ming.shao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  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 (&ServerIp->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:

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

Thanks!
Laszlo