From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7BB7780432 for ; Fri, 17 Mar 2017 08:06:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489763203; x=1521299203; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=qDBgqPQB35IxcW5LUvoY2fLtl4yNkP3V9gNXM7BeXUw=; b=W1OEvXbS/COMvA0XQDNjoABJPJBVpeI6HQ5MFt2iLxDgt4tkIgiCs1tS EubgMYzFl1h7QohQ7AmdsWBRLmx6zQ==; Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Mar 2017 08:06:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,177,1486454400"; d="scan'208";a="1109571541" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga001.jf.intel.com with ESMTP; 17 Mar 2017 08:06:42 -0700 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 17 Mar 2017 08:06:42 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.47]) by fmsmsx158.amr.corp.intel.com ([169.254.15.247]) with mapi id 14.03.0248.002; Fri, 17 Mar 2017 08:06:42 -0700 From: "Carsey, Jaben" To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" CC: "Ye, Ting" , "Fu, Siyuan" , "Wu, Jiaxin" , "Carsey, Jaben" Thread-Topic: [edk2] [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Thread-Index: AQHSnrzzy1NBMTGv/EKb8TAk4vNdIKGZIntg Date: Fri, 17 Mar 2017 15:06:41 +0000 Message-ID: References: <1489713743-16028-1-git-send-email-jiaxin.wu@intel.com> In-Reply-To: <1489713743-16028-1-git-send-email-jiaxin.wu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGRkMzU1MjctZDU4MC00YWMwLThhZTgtOWE2OTA0MGI2NGMxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImQ0SUQ0bEVqa21nZ25rQnZJR2dmRVdMWVRJV21HUzN2cjE4MHp2clBqZVk9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.1.200.108] MIME-Version: 1.0 Subject: Re: [PATCH v2] ShellPkg/tftp: Add one option for tftp command to specify windowsize X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Mar 2017 15:06:43 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Looks good. Reviewed-by: Jaben Carsey Note that I have no experience with "WindowSize" and FTP so I am talking ab= out 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 ; Carsey, Jaben ; > Fu, Siyuan ; Wu, Jiaxin > Subject: [edk2] [PATCH v2] ShellPkg/tftp: Add one option for tftp command > to specify windowsize > Importance: High >=20 > v2: > * Refine the constants definitions. >=20 > Define -w option for tftp shell command to specify the TFTP windowsize > option. That will benefit the big file download for tftp server. >=20 > Cc: Ye Ting > Cc: Fu Siyuan > Cc: Carsey Jaben > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wu Jiaxin > --- > ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c | 67 > ++++++++++++++++++---- > .../UefiShellTftpCommandLib.uni | 6 +- > 2 files changed, 60 insertions(+), 13 deletions(-) >=20 > 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. >=20 > Copyright (c) 2015, ARM Ltd. All rights reserved.
> - Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. > + Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. > (C) Copyright 2015 Hewlett Packard Enterprise Development LP
>=20 > 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 > ); >=20 > /** > Update the progress of a file download > @@ -224,10 +225,11 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] =3D { > {L"-l", TypeValue}, > {L"-r", TypeValue}, > {L"-c", TypeValue}, > {L"-t", TypeValue}, > {L"-s", TypeValue}, > + {L"-w", TypeValue}, > {NULL , TypeMax} > }; >=20 > /// > /// The default block size (512) of tftp is defined in the RFC1350. > @@ -236,11 +238,21 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] =3D { > /// > /// 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 >=20 > /** > Function for 'tftp' command. >=20 > @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; >=20 > ShellStatus =3D SHELL_INVALID_PARAMETER; > ProblemParam =3D NULL; > NicFound =3D FALSE; > AsciiRemoteFilePath =3D NULL; > Handles =3D NULL; > FileSize =3D 0; > BlockSize =3D MTFTP_DEFAULT_BLKSIZE; > + WindowSize =3D MTFTP_DEFAULT_WINDOWSIZE; >=20 > // > // Initialize the Shell library (we must be in non-auto-init...) > // > Status =3D ShellInitialize (); > @@ -432,10 +446,24 @@ ShellCommandRunTftp ( > ); > goto Error; > } > } >=20 > + ValueStr =3D ShellCommandLineGetValue (CheckPackage, L"-w"); > + if (ValueStr !=3D 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 =3D SHELL_NOT_FOUND; > Status =3D gBS->LocateHandleBuffer ( > @@ -506,11 +534,11 @@ ShellCommandRunTftp ( > gShellTftpHiiHandle, RemoteFilePath, NicName, Status > ); > goto NextHandle; > } >=20 > - Status =3D DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath= , > FileSize, BlockSize, &Data); > + Status =3D 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]; >=20 > // 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 =3D (UINT8*)AsciiFilePath; > Mtftp4Token.BufferSize =3D FileSize; > Mtftp4Token.Buffer =3D Buffer; > Mtftp4Token.CheckPacket =3D CheckPacket; > Mtftp4Token.Context =3D (VOID*)TftpContext; > + Mtftp4Token.OptionCount =3D 0; > + Mtftp4Token.OptionList =3D AllocatePool (sizeof (EFI_MTFTP4_OPTION) * > 2); > + if (Mtftp4Token.OptionList =3D=3D NULL) { > + Status =3D EFI_OUT_OF_RESOURCES; > + goto Error; > + } > + > if (BlockSize !=3D MTFTP_DEFAULT_BLKSIZE) { > - ReqOpt.OptionStr =3D (UINT8 *) "blksize"; > - AsciiSPrint ((CHAR8 *)OptBuf, sizeof (OptBuf), "%d", BlockSize); > - ReqOpt.ValueStr =3D OptBuf; > + Mtftp4Token.OptionList[Mtftp4Token.OptionCount].OptionStr =3D (UINT8 > *) "blksize"; > + AsciiSPrint ((CHAR8 *) BlksizeBuf, sizeof (BlksizeBuf), "%d", BlockS= ize); > + Mtftp4Token.OptionList[Mtftp4Token.OptionCount].ValueStr =3D > BlksizeBuf; > + Mtftp4Token.OptionCount ++; > + } >=20 > - Mtftp4Token.OptionCount =3D 1; > - Mtftp4Token.OptionList =3D &ReqOpt; > + if (WindowSize !=3D MTFTP_DEFAULT_WINDOWSIZE) { > + Mtftp4Token.OptionList[Mtftp4Token.OptionCount].OptionStr =3D (UINT8 > *) "windowsize"; > + AsciiSPrint ((CHAR8 *) WindowsizeBuf, sizeof (WindowsizeBuf), "%d", > WindowSize); > + Mtftp4Token.OptionList[Mtftp4Token.OptionCount].ValueStr =3D > WindowsizeBuf; > + Mtftp4Token.OptionCount ++; > } >=20 > ShellPrintHiiEx ( > -1, -1, NULL, STRING_TOKEN (STR_TFTP_DOWNLOADING), > gShellTftpHiiHandle, FilePath > @@ -954,14 +995,18 @@ DownloadFile ( > gShellTftpHiiHandle > ); >=20 > Error : >=20 > - if (TftpContext =3D=3D NULL) { > + if (TftpContext !=3D NULL) { > FreePool (TftpContext); > } >=20 > + if (Mtftp4Token.OptionList !=3D NULL) { > + FreePool (Mtftp4Token.OptionList); > + } > + > if (EFI_ERROR (Status)) { > gBS->FreePages (PagesAddress, EFI_SIZE_TO_PAGES (FileSize)); > return Status; > } >=20 > 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
> -// Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. > +// Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. > // 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 ] [-r ] [-c ] [-t > ]\r\n" > -" [-s ] host remotefilepath [localfilepath]\r\n" > +" [-s ] [-w ] 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 - The number of seconds to wait for a response after= \r\n" > " sending a request packet. Default value is 4s.\r\n= " > " -s - Specifies the TFTP blksize option as defined in RF= C > 2348.\r\n" > " Valid range is between 8 and 65464, default value = is 512.\r\n" > +" -w - 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 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel