public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize
@ 2017-03-17  1:22 Jiaxin Wu
  2017-03-17 15:06 ` Carsey, Jaben
  2017-03-21  7:14 ` Ye, Ting
  0 siblings, 2 replies; 3+ messages in thread
From: Jiaxin Wu @ 2017-03-17  1:22 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Carsey Jaben, Wu Jiaxin

v2:
* Refine the constants definitions.

Define -w option for tftp shell command to specify the TFTP windowsize
option. That will benefit the big file download for tftp server.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Carsey Jaben <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c    | 67 ++++++++++++++++++----
 .../UefiShellTftpCommandLib.uni                    |  6 +-
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
index 5c50797..9b47839 100755
--- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
+++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
@@ -1,10 +1,10 @@
 /** @file
   The implementation for the 'tftp' Shell command.
 
   Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
-  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. <BR>
+  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. <BR>
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<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
@@ -180,10 +180,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
@@ -224,10 +225,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.
@@ -236,11 +238,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).
@@ -285,18 +297,20 @@ ShellCommandRunTftp (
   EFI_MTFTP4_PROTOCOL     *Mtftp4;
   UINTN                   FileSize;
   VOID                    *Data;
   SHELL_FILE_HANDLE       FileHandle;
   UINT16                  BlockSize;
+  UINT16                  WindowSize;
 
   ShellStatus         = SHELL_INVALID_PARAMETER;
   ProblemParam        = NULL;
   NicFound            = FALSE;
   AsciiRemoteFilePath = NULL;
   Handles             = NULL;
   FileSize            = 0;
   BlockSize           = MTFTP_DEFAULT_BLKSIZE;
+  WindowSize          = MTFTP_DEFAULT_WINDOWSIZE;
 
   //
   // Initialize the Shell library (we must be in non-auto-init...)
   //
   Status = ShellInitialize ();
@@ -432,10 +446,24 @@ ShellCommandRunTftp (
       );
       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),
+        gShellTftpHiiHandle, L"tftp", ValueStr
+      );
+      goto Error;
+    }
+  }
+
   //
   // Locate all MTFTP4 Service Binding protocols
   //
   ShellStatus = SHELL_NOT_FOUND;
   Status = gBS->LocateHandleBuffer (
@@ -506,11 +534,11 @@ ShellCommandRunTftp (
         gShellTftpHiiHandle, 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),
         gShellTftpHiiHandle, RemoteFilePath, NicName, Status
       );
@@ -890,20 +918,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
@@ -932,17 +961,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),
     gShellTftpHiiHandle, FilePath
@@ -954,14 +995,18 @@ DownloadFile (
     gShellTftpHiiHandle
     );
 
 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/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
index 4f4447d..ca6fa44 100644
--- a/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
+++ b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
@@ -1,9 +1,9 @@
 // /**
 //
 // (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
-// Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. <BR>
+// Copyright (c) 2010 - 2017, 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"
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize
  2017-03-17  1:22 [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
@ 2017-03-17 15:06 ` Carsey, Jaben
  2017-03-21  7:14 ` Ye, Ting
  1 sibling, 0 replies; 3+ messages in thread
From: Carsey, Jaben @ 2017-03-17 15:06 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org
  Cc: Ye, Ting, Fu, Siyuan, Wu, Jiaxin, Carsey, Jaben

Looks good.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

Note that I have no experience with "WindowSize" and FTP so I am talking about my understanding based on the code and not on the theory.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Thursday, March 16, 2017 6:22 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [PATCH v2] ShellPkg/tftp: Add one option for tftp command
> to specify windowsize
> Importance: High
> 
> v2:
> * Refine the constants definitions.
> 
> Define -w option for tftp shell command to specify the TFTP windowsize
> option. That will benefit the big file download for tftp server.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Carsey Jaben <jaben.carsey@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c    | 67
> ++++++++++++++++++----
>  .../UefiShellTftpCommandLib.uni                    |  6 +-
>  2 files changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> index 5c50797..9b47839 100755
> --- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> +++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> @@ -1,10 +1,10 @@
>  /** @file
>    The implementation for the 'tftp' Shell command.
> 
>    Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
> -  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. <BR>
> +  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. <BR>
>    (C) Copyright 2015 Hewlett Packard Enterprise Development LP<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
> @@ -180,10 +180,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
> @@ -224,10 +225,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.
> @@ -236,11 +238,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).
> @@ -285,18 +297,20 @@ ShellCommandRunTftp (
>    EFI_MTFTP4_PROTOCOL     *Mtftp4;
>    UINTN                   FileSize;
>    VOID                    *Data;
>    SHELL_FILE_HANDLE       FileHandle;
>    UINT16                  BlockSize;
> +  UINT16                  WindowSize;
> 
>    ShellStatus         = SHELL_INVALID_PARAMETER;
>    ProblemParam        = NULL;
>    NicFound            = FALSE;
>    AsciiRemoteFilePath = NULL;
>    Handles             = NULL;
>    FileSize            = 0;
>    BlockSize           = MTFTP_DEFAULT_BLKSIZE;
> +  WindowSize          = MTFTP_DEFAULT_WINDOWSIZE;
> 
>    //
>    // Initialize the Shell library (we must be in non-auto-init...)
>    //
>    Status = ShellInitialize ();
> @@ -432,10 +446,24 @@ ShellCommandRunTftp (
>        );
>        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),
> +        gShellTftpHiiHandle, L"tftp", ValueStr
> +      );
> +      goto Error;
> +    }
> +  }
> +
>    //
>    // Locate all MTFTP4 Service Binding protocols
>    //
>    ShellStatus = SHELL_NOT_FOUND;
>    Status = gBS->LocateHandleBuffer (
> @@ -506,11 +534,11 @@ ShellCommandRunTftp (
>          gShellTftpHiiHandle, 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),
>          gShellTftpHiiHandle, RemoteFilePath, NicName, Status
>        );
> @@ -890,20 +918,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
> @@ -932,17 +961,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),
>      gShellTftpHiiHandle, FilePath
> @@ -954,14 +995,18 @@ DownloadFile (
>      gShellTftpHiiHandle
>      );
> 
>  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/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
> b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
> index 4f4447d..ca6fa44 100644
> ---
> a/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
> +++
> b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
> @@ -1,9 +1,9 @@
>  // /**
>  //
>  // (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
> -// Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. <BR>
> +// Copyright (c) 2010 - 2017, 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"
> --
> 1.9.5.msysgit.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize
  2017-03-17  1:22 [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
  2017-03-17 15:06 ` Carsey, Jaben
@ 2017-03-21  7:14 ` Ye, Ting
  1 sibling, 0 replies; 3+ messages in thread
From: Ye, Ting @ 2017-03-21  7:14 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Carsey, Jaben

Reviewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: Wu, Jiaxin 
Sent: Friday, March 17, 2017 9:22 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize

v2:
* Refine the constants definitions.

Define -w option for tftp shell command to specify the TFTP windowsize option. That will benefit the big file download for tftp server.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Carsey Jaben <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c    | 67 ++++++++++++++++++----
 .../UefiShellTftpCommandLib.uni                    |  6 +-
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
index 5c50797..9b47839 100755
--- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
+++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
@@ -1,10 +1,10 @@
 /** @file
   The implementation for the 'tftp' Shell command.
 
   Copyright (c) 2015, ARM Ltd. All rights reserved.<BR>
-  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. <BR>
+  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. 
+ <BR>
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<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 @@ -180,10 +180,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 @@ -224,10 +225,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.
@@ -236,11 +238,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).
@@ -285,18 +297,20 @@ ShellCommandRunTftp (
   EFI_MTFTP4_PROTOCOL     *Mtftp4;
   UINTN                   FileSize;
   VOID                    *Data;
   SHELL_FILE_HANDLE       FileHandle;
   UINT16                  BlockSize;
+  UINT16                  WindowSize;
 
   ShellStatus         = SHELL_INVALID_PARAMETER;
   ProblemParam        = NULL;
   NicFound            = FALSE;
   AsciiRemoteFilePath = NULL;
   Handles             = NULL;
   FileSize            = 0;
   BlockSize           = MTFTP_DEFAULT_BLKSIZE;
+  WindowSize          = MTFTP_DEFAULT_WINDOWSIZE;
 
   //
   // Initialize the Shell library (we must be in non-auto-init...)
   //
   Status = ShellInitialize ();
@@ -432,10 +446,24 @@ ShellCommandRunTftp (
       );
       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),
+        gShellTftpHiiHandle, L"tftp", ValueStr
+      );
+      goto Error;
+    }
+  }
+
   //
   // Locate all MTFTP4 Service Binding protocols
   //
   ShellStatus = SHELL_NOT_FOUND;
   Status = gBS->LocateHandleBuffer (
@@ -506,11 +534,11 @@ ShellCommandRunTftp (
         gShellTftpHiiHandle, 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),
         gShellTftpHiiHandle, RemoteFilePath, NicName, Status
       );
@@ -890,20 +918,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 @@ -932,17 +961,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),
     gShellTftpHiiHandle, FilePath
@@ -954,14 +995,18 @@ DownloadFile (
     gShellTftpHiiHandle
     );
 
 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/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
index 4f4447d..ca6fa44 100644
--- a/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.uni
+++ b/ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.u
+++ ni
@@ -1,9 +1,9 @@
 // /**
 //
 // (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR> -// Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. <BR>
+// Copyright (c) 2010 - 2017, 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"
--
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-21  7:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-17  1:22 [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
2017-03-17 15:06 ` Carsey, Jaben
2017-03-21  7:14 ` Ye, Ting

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox