Reviewed-by: Jaben Carsey <jaben.car...@intel.com>

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Sunday, September 16, 2018 10:44 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Carsey,
> Jaben <jaben.car...@intel.com>; Shao, Ming <ming.s...@intel.com>; Wu,
> Jiaxin <jiaxin...@intel.com>
> Subject: [Patch 3/5] ShellPkg/TftpDynamicCommand: Add one option for tftp
> command to specify windowsize.
> Importance: High
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=886
> 
> This patch is to define one new option for TFTP shell command to specify the
> windowsize option as defined in RFC 7440. Valid range is between 1 and 64,
> default value is 1.
> 
> Note that: RFC 7440 does not mention max window size value, but for the
> stability reason, the value is limited to 64.
> 
> Cc: Ye Ting <ting...@intel.com>
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Carsey Jaben <jaben.car...@intel.com>
> Cc: Shao Ming <ming.s...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin...@intel.com>
> ---
>  .../DynamicCommand/TftpDynamicCommand/Tftp.c  | 65
> ++++++++++++++++---
>  .../TftpDynamicCommand/Tftp.uni               |  6 +-
>  2 files changed, 59 insertions(+), 12 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> index 44be6d4e76..c66be6b9d9 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> @@ -181,10 +181,11 @@ DownloadFile (
>    IN   EFI_MTFTP4_PROTOCOL  *Mtftp4,
>    IN   CONST CHAR16         *FilePath,
>    IN   CONST CHAR8          *AsciiFilePath,
>    IN   UINTN                FileSize,
>    IN   UINT16               BlockSize,
> +  IN   UINT16               WindowSize,
>    OUT  VOID                 **Data
>    );
> 
>  /**
>    Update the progress of a file download
> @@ -225,10 +226,11 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>    {L"-l", TypeValue},
>    {L"-r", TypeValue},
>    {L"-c", TypeValue},
>    {L"-t", TypeValue},
>    {L"-s", TypeValue},
> +  {L"-w", TypeValue},
>    {NULL , TypeMax}
>    };
> 
>  ///
>  /// The default block size (512) of tftp is defined in the RFC1350.
> @@ -237,11 +239,21 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>  ///
>  /// The valid range of block size option is defined in the RFC2348.
>  ///
>  #define MTFTP_MIN_BLKSIZE          8
>  #define MTFTP_MAX_BLKSIZE          65464
> -
> +///
> +/// The default windowsize (1) of tftp.
> +///
> +#define MTFTP_DEFAULT_WINDOWSIZE   1
> +///
> +/// The valid range of window size option.
> +/// Note that: RFC 7440 does not mention max window size value, but for
> the
> +/// stability reason, the value is limited to 64.
> +///
> +#define MTFTP_MIN_WINDOWSIZE       1
> +#define MTFTP_MAX_WINDOWSIZE       64
> 
>  /**
>    Function for 'tftp' command.
> 
>    @param[in] ImageHandle  Handle to the Image (NULL if Internal).
> @@ -286,19 +298,21 @@ RunTftp (
>    UINTN                   FileSize;
>    UINTN                   DataSize;
>    VOID                    *Data;
>    SHELL_FILE_HANDLE       FileHandle;
>    UINT16                  BlockSize;
> +  UINT16                  WindowSize;
> 
>    ShellStatus         = SHELL_INVALID_PARAMETER;
>    ProblemParam        = NULL;
>    NicFound            = FALSE;
>    AsciiRemoteFilePath = NULL;
>    Handles             = NULL;
>    FileSize            = 0;
>    DataSize            = 0;
>    BlockSize           = MTFTP_DEFAULT_BLKSIZE;
> +  WindowSize          = MTFTP_DEFAULT_WINDOWSIZE;
> 
>    //
>    // Initialize the Shell library (we must be in non-auto-init...)
>    //
>    Status = ShellInitialize ();
> @@ -434,10 +448,24 @@ RunTftp (
>        );
>        goto Error;
>      }
>    }
> 
> +  ValueStr = ShellCommandLineGetValue (CheckPackage, L"-w");
> +  if (ValueStr != NULL) {
> +    if (!StringToUint16 (ValueStr, &WindowSize)) {
> +      goto Error;
> +    }
> +    if (WindowSize < MTFTP_MIN_WINDOWSIZE || WindowSize >
> MTFTP_MAX_WINDOWSIZE) {
> +      ShellPrintHiiEx (
> +        -1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> +        mTftpHiiHandle, L"tftp", ValueStr
> +      );
> +      goto Error;
> +    }
> +  }
> +
>    //
>    // Locate all MTFTP4 Service Binding protocols
>    //
>    ShellStatus = SHELL_NOT_FOUND;
>    Status = gBS->LocateHandleBuffer (
> @@ -508,11 +536,11 @@ RunTftp (
>          mTftpHiiHandle, RemoteFilePath, NicName, Status
>        );
>        goto NextHandle;
>      }
> 
> -    Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, &Data);
> +    Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, WindowSize, &Data);
>      if (EFI_ERROR (Status)) {
>        ShellPrintHiiEx (
>          -1, -1, NULL, STRING_TOKEN (STR_TFTP_ERR_DOWNLOAD),
>          mTftpHiiHandle, RemoteFilePath, NicName, Status
>        );
> @@ -894,20 +922,21 @@ DownloadFile (
>    IN   EFI_MTFTP4_PROTOCOL  *Mtftp4,
>    IN   CONST CHAR16         *FilePath,
>    IN   CONST CHAR8          *AsciiFilePath,
>    IN   UINTN                FileSize,
>    IN   UINT16               BlockSize,
> +  IN   UINT16               WindowSize,
>    OUT  VOID                 **Data
>    )
>  {
>    EFI_STATUS            Status;
>    EFI_PHYSICAL_ADDRESS  PagesAddress;
>    VOID                  *Buffer;
>    DOWNLOAD_CONTEXT      *TftpContext;
>    EFI_MTFTP4_TOKEN      Mtftp4Token;
> -  EFI_MTFTP4_OPTION     ReqOpt;
> -  UINT8                 OptBuf[10];
> +  UINT8                 BlksizeBuf[10];
> +  UINT8                 WindowsizeBuf[10];
> 
>    // Downloaded file can be large. BS.AllocatePages() is more faster
>    // than AllocatePool() and avoid fragmentation.
>    // The downloaded file could be an EFI application. Marking the
>    // allocated page as EfiBootServicesCode would allow to execute a
> @@ -936,17 +965,29 @@ DownloadFile (
>    Mtftp4Token.Filename    = (UINT8*)AsciiFilePath;
>    Mtftp4Token.BufferSize  = FileSize;
>    Mtftp4Token.Buffer      = Buffer;
>    Mtftp4Token.CheckPacket = CheckPacket;
>    Mtftp4Token.Context     = (VOID*)TftpContext;
> +  Mtftp4Token.OptionCount = 0;
> +  Mtftp4Token.OptionList  = AllocatePool (sizeof (EFI_MTFTP4_OPTION) *
> 2);
> +  if (Mtftp4Token.OptionList == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto Error;
> +  }
> +
>    if (BlockSize != MTFTP_DEFAULT_BLKSIZE) {
> -    ReqOpt.OptionStr = (UINT8 *) "blksize";
> -    AsciiSPrint ((CHAR8 *)OptBuf, sizeof (OptBuf), "%d", BlockSize);
> -    ReqOpt.ValueStr  = OptBuf;
> +    Mtftp4Token.OptionList[Mtftp4Token.OptionCount].OptionStr = (UINT8
> *) "blksize";
> +    AsciiSPrint ((CHAR8 *) BlksizeBuf, sizeof (BlksizeBuf), "%d", BlockSize);
> +    Mtftp4Token.OptionList[Mtftp4Token.OptionCount].ValueStr  =
> BlksizeBuf;
> +    Mtftp4Token.OptionCount ++;
> +  }
> 
> -    Mtftp4Token.OptionCount = 1;
> -    Mtftp4Token.OptionList  = &ReqOpt;
> +  if (WindowSize != MTFTP_DEFAULT_WINDOWSIZE) {
> +    Mtftp4Token.OptionList[Mtftp4Token.OptionCount].OptionStr = (UINT8
> *) "windowsize";
> +    AsciiSPrint ((CHAR8 *) WindowsizeBuf, sizeof (WindowsizeBuf), "%d",
> WindowSize);
> +    Mtftp4Token.OptionList[Mtftp4Token.OptionCount].ValueStr  =
> WindowsizeBuf;
> +    Mtftp4Token.OptionCount ++;
>    }
> 
>    ShellPrintHiiEx (
>      -1, -1, NULL, STRING_TOKEN (STR_TFTP_DOWNLOADING),
>      mTftpHiiHandle, FilePath
> @@ -958,14 +999,18 @@ DownloadFile (
>      mTftpHiiHandle
>      );
> 
>  Error :
> 
> -  if (TftpContext == NULL) {
> +  if (TftpContext != NULL) {
>      FreePool (TftpContext);
>    }
> 
> +  if (Mtftp4Token.OptionList != NULL) {
> +    FreePool (Mtftp4Token.OptionList);
> +  }
> +
>    if (EFI_ERROR (Status)) {
>      gBS->FreePages (PagesAddress, EFI_SIZE_TO_PAGES (FileSize));
>      return Status;
>    }
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> index 1393ba5679..654e42ad23 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> @@ -1,9 +1,9 @@
>  // /**
>  //
>  // (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
> -// Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. <BR>
> +// Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. <BR>
>  // This program and the accompanying materials
>  // are licensed and made available under the terms and conditions of the
> BSD License
>  // which accompanies this distribution. The full text of the license may be
> found at
>  // http://opensource.org/licenses/bsd-license.php
>  //
> @@ -48,11 +48,11 @@
>  ".SH NAME\r\n"
>  "Download a file from TFTP server.\r\n"
>  ".SH SYNOPSIS\r\n"
>  " \r\n"
>  "TFTP [-i interface] [-l <port>] [-r <port>] [-c <retry count>] [-t
> <timeout>]\r\n"
> -"     [-s <block size>] host remotefilepath [localfilepath]\r\n"
> +"     [-s <block size>] [-w <window size>] host remotefilepath
> [localfilepath]\r\n"
>  ".SH OPTIONS\r\n"
>  " \r\n"
>  "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
>  "  -l port          - Specifies the local port number. Default value is 
> 0\r\n"
>  "                     and the port number is automatically assigned.\r\n"
> @@ -61,10 +61,12 @@
>  "                     wait for a response. The default value is 6.\r\n"
>  "  -t <timeout>     - The number of seconds to wait for a response after\r\n"
>  "                     sending a request packet. Default value is 4s.\r\n"
>  "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC
> 2348.\r\n"
>  "                     Valid range is between 8 and 65464, default value is 
> 512.\r\n"
> +"  -w <window size> - Specifies the TFTP windowsize option as defined in
> RFC 7440.\r\n"
> +"                     Valid range is between 1 and 64, default value is 
> 1.\r\n"
>  "  host             - Specify TFTP Server IPv4 address.\r\n"
>  "  remotefilepath   - TFTP server file path to download the file.\r\n"
>  "  localfilepath    - Local destination file path.\r\n"
>  ".SH DESCRIPTION\r\n"
>  " \r\n"
> --
> 2.17.1.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to