public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jiaxin Wu <Jiaxin.wu@intel.com>
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>,
	Shao Ming <ming.shao@intel.com>, Wu Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch 3/5] ShellPkg/TftpDynamicCommand: Add one option for tftp command to specify windowsize.
Date: Mon, 17 Sep 2018 13:43:46 +0800	[thread overview]
Message-ID: <20180917054348.19228-4-Jiaxin.wu@intel.com> (raw)
In-Reply-To: <20180917054348.19228-1-Jiaxin.wu@intel.com>

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.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Carsey Jaben <jaben.carsey@intel.com>
Cc: Shao Ming <ming.shao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@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



  parent reply	other threads:[~2018-09-17  5:43 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-17  5:43 [Patch 0/5] Support windowsize to benefit tftp/pxe download performance Jiaxin Wu
2018-09-17  5:43 ` [Patch 1/5] MdeModulePke/Mtftp4Dxe: Support windowsize in read request operation Jiaxin Wu
2018-09-17  5:43 ` [Patch 2/5] NetworkPkg/Mtftp6Dxe: " Jiaxin Wu
2018-09-17  5:43 ` Jiaxin Wu [this message]
2018-09-17 22:18   ` [Patch 3/5] ShellPkg/TftpDynamicCommand: Add one option for tftp command to specify windowsize Carsey, Jaben
2018-09-17  5:43 ` [Patch 4/5] MdeModulePkg/MdeModulePkg.dec: Define one PCD for PXE to specify MTFTP windowsize Jiaxin Wu
2018-09-18 11:04   ` Laszlo Ersek
2018-09-19  1:55     ` Wu, Jiaxin
2018-09-19  0:38   ` Fu, Siyuan
2018-09-19  2:21     ` Wu, Jiaxin
2018-09-17  5:43 ` [Patch 5/5] NetworkPkg/UefiPxeBcDxe: Use the specified " Jiaxin Wu
2018-09-18 11:23 ` [Patch 0/5] Support windowsize to benefit tftp/pxe download performance Laszlo Ersek
2018-09-19  2:20   ` Wu, Jiaxin
2018-09-19 11:24     ` Laszlo Ersek
2018-09-20  5:54       ` Wu, Jiaxin
     [not found]         ` <845df55b-b9ba-20f9-25fd-22778253c198@redhat.com>
2018-09-21  6:33           ` Wu, Jiaxin
2018-09-21  9:37             ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180917054348.19228-4-Jiaxin.wu@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox