* [Patch 0/2] windowsize option support
@ 2017-03-16 8:35 Jiaxin Wu
2017-03-16 8:35 ` [Patch 1/2] MdeModulePke/Mtftp4Dxe: Add " Jiaxin Wu
2017-03-16 8:35 ` [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
0 siblings, 2 replies; 5+ messages in thread
From: Jiaxin Wu @ 2017-03-16 8:35 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Carsey Jaben, Wu Jiaxin
Support the tftp windowsize option that allows the client and server to
negotiate a window size of consecutive blocks to send as an alternative
for replacing the single-block lockstep schema.
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>
Jiaxin Wu (2):
MdeModulePke/Mtftp4Dxe: Add windowsize option support
ShellPkg/tftp: Add one option for tftp command to specify windowsize
.../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 5 +-
.../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h | 11 +++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Option.c | 12 ++++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Option.h | 6 ++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c | 44 +++++++++++-----
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 14 ++---
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.h | 16 +-----
.../Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 41 ++++++++++++---
ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c | 60 ++++++++++++++++++----
.../UefiShellTftpCommandLib.uni | 6 ++-
10 files changed, 158 insertions(+), 57 deletions(-)
--
1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Patch 1/2] MdeModulePke/Mtftp4Dxe: Add windowsize option support
2017-03-16 8:35 [Patch 0/2] windowsize option support Jiaxin Wu
@ 2017-03-16 8:35 ` Jiaxin Wu
2017-03-16 8:35 ` [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
1 sibling, 0 replies; 5+ messages in thread
From: Jiaxin Wu @ 2017-03-16 8:35 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Wu Jiaxin
This patch is to support the tftp windowsize option that allows the
client and server to negotiate a window size of consecutive blocks to
send as an alternative for replacing the single-block lockstep schema.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
.../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 5 ++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h | 11 +++++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Option.c | 12 +++++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Option.h | 6 ++-
.../Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c | 44 ++++++++++++++++------
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 14 +++----
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.h | 16 +-------
.../Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 41 +++++++++++++++++---
8 files changed, 104 insertions(+), 45 deletions(-)
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
index a2583a4..9d4de42 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
@@ -1,10 +1,10 @@
/** @file
Interface routine for Mtftp4.
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -78,10 +78,12 @@ Mtftp4CleanOperation (
ZeroMem (&Instance->RequestOption, sizeof (MTFTP4_OPTION));
Instance->Operation = 0;
Instance->BlkSize = MTFTP4_DEFAULT_BLKSIZE;
+ Instance->SentAckNo = 0;
+ Instance->WindowSize = 1;
Instance->LastBlock = 0;
Instance->ServerIp = 0;
Instance->ListeningPort = 0;
Instance->ConnectedPort = 0;
Instance->Gateway = 0;
@@ -438,10 +440,11 @@ Mtftp4Start (
// Set the operation parameters from the configuration or override data.
//
Config = &Instance->Config;
Instance->Token = Token;
Instance->BlkSize = MTFTP4_DEFAULT_BLKSIZE;
+ Instance->WindowSize = MTFTP4_DEFAULT_WINDOWSIZE;
CopyMem (&Instance->ServerIp, &Config->ServerIp, sizeof (IP4_ADDR));
Instance->ServerIp = NTOHL (Instance->ServerIp);
Instance->ListeningPort = Config->InitialServerPort;
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
index 527fd1d..733bc5d 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
@@ -6,12 +6,13 @@
RFC1350 - THE TFTP PROTOCOL (REVISION 2)
RFC2090 - TFTP Multicast Option
RFC2347 - TFTP Option Extension
RFC2348 - TFTP Blocksize Option
RFC2349 - TFTP Timeout Interval and Transfer Size Options
+ RFC7440 - TFTP Windowsize Option
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -54,10 +55,11 @@ typedef struct _MTFTP4_PROTOCOL MTFTP4_PROTOCOL;
#define MTFTP4_DEFAULT_SERVER_PORT 69
#define MTFTP4_DEFAULT_TIMEOUT 3
#define MTFTP4_DEFAULT_RETRY 5
#define MTFTP4_DEFAULT_BLKSIZE 512
+#define MTFTP4_DEFAULT_WINDOWSIZE 1
#define MTFTP4_TIME_TO_GETMAP 5
#define MTFTP4_STATE_UNCONFIGED 0
#define MTFTP4_STATE_CONFIGED 1
#define MTFTP4_STATE_DESTROY 2
@@ -119,10 +121,17 @@ struct _MTFTP4_PROTOCOL {
UINT16 BlkSize;
UINT16 LastBlock;
LIST_ENTRY Blocks;
//
+ // The last sent ack number.
+ //
+ UINT16 SentAckNo;
+
+ UINT16 WindowSize;
+
+ //
// The server's communication end point: IP and two ports. one for
// initial request, one for its selected port.
//
IP4_ADDR ServerIp;
UINT16 ListeningPort;
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.c
index e40561a..da6aee0 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.c
@@ -1,9 +1,9 @@
/** @file
Routines to process MTFTP4 options.
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "Mtftp4Impl.h"
CHAR8 *mMtftp4SupportedOptions[MTFTP4_SUPPORTED_OPTIONS] = {
"blksize",
+ "windowsize",
"timeout",
"tsize",
"multicast"
};
@@ -479,10 +480,19 @@ Mtftp4ParseOption (
}
}
MtftpOption->Exist |= MTFTP4_MCAST_EXIST;
+ } else if (NetStringEqualNoCase (This->OptionStr, (UINT8 *) "windowsize")) {
+ Value = NetStringToU32 (This->ValueStr);
+
+ if (Value < 1) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ MtftpOption->WindowSize = (UINT16) Value;
+ MtftpOption->Exist |= MTFTP4_WINDOWSIZE_EXIST;
} else if (Request) {
//
// Ignore the unsupported option if it is a reply, and return
// EFI_UNSUPPORTED if it's a request according to the UEFI spec.
//
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.h
index b7fdbf2..b86d77d 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Option.h
@@ -1,9 +1,9 @@
/** @file
Routines to process MTFTP4 options.
-Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -14,23 +14,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#ifndef __EFI_MTFTP4_OPTION_H__
#define __EFI_MTFTP4_OPTION_H__
-#define MTFTP4_SUPPORTED_OPTIONS 4
+#define MTFTP4_SUPPORTED_OPTIONS 5
#define MTFTP4_OPCODE_LEN 2
#define MTFTP4_ERRCODE_LEN 2
#define MTFTP4_BLKNO_LEN 2
#define MTFTP4_DATA_HEAD_LEN 4
#define MTFTP4_BLKSIZE_EXIST 0x01
#define MTFTP4_TIMEOUT_EXIST 0x02
#define MTFTP4_TSIZE_EXIST 0x04
#define MTFTP4_MCAST_EXIST 0x08
+#define MTFTP4_WINDOWSIZE_EXIST 0x10
typedef struct {
UINT16 BlkSize;
+ UINT16 WindowSize;
UINT8 Timeout;
UINT32 Tsize;
IP4_ADDR McastIp;
UINT16 McastPort;
BOOLEAN Master;
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c
index e983d79..5454d6c 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c
@@ -1,10 +1,10 @@
/** @file
Routines to process Rrq (download).
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -97,10 +97,13 @@ Mtftp4RrqSendAck (
IN UINT16 BlkNo
)
{
EFI_MTFTP4_PACKET *Ack;
NET_BUF *Packet;
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
Packet = NetbufAlloc (sizeof (EFI_MTFTP4_ACK_HEADER));
if (Packet == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -113,11 +116,16 @@ Mtftp4RrqSendAck (
ASSERT (Ack != NULL);
Ack->Ack.OpCode = HTONS (EFI_MTFTP4_OPCODE_ACK);
Ack->Ack.Block[0] = HTONS (BlkNo);
- return Mtftp4SendPacket (Instance, Packet);
+ Status = Mtftp4SendPacket (Instance, Packet);
+ if (!EFI_ERROR (Status)) {
+ Instance->SentAckNo = BlkNo;
+ }
+
+ return Status;
}
/**
Deliver the received data block to the user, which can be saved
@@ -156,11 +164,11 @@ Mtftp4RrqSaveBlock (
//
// This is the last block, save the block no
//
if (DataLen < Instance->BlkSize) {
- Completed = TRUE;
+ Completed = TRUE;
Instance->LastBlock = Block;
Mtftp4SetLastBlockNum (&Instance->Blocks, Block);
}
//
@@ -255,23 +263,23 @@ Mtftp4RrqHandleData (
EFI_STATUS Status;
UINT16 BlockNum;
INTN Expected;
*Completed = FALSE;
+ Status = EFI_SUCCESS;
BlockNum = NTOHS (Packet->Data.Block);
Expected = Mtftp4GetNextBlockNum (&Instance->Blocks);
ASSERT (Expected >= 0);
//
- // If we are active and received an unexpected packet, retransmit
- // the last ACK then restart receiving. If we are passive, save
- // the block.
+ // If we are active and received an unexpected packet, transmit
+ // the ACK for the block we received, then restart receiving the
+ // expected one. If we are passive, save the block.
//
if (Instance->Master && (Expected != BlockNum)) {
- Mtftp4Retransmit (Instance);
- return EFI_SUCCESS;
+ return Mtftp4RrqSendAck (Instance, (UINT16) (Expected - 1));
}
Status = Mtftp4RrqSaveBlock (Instance, Packet, Len);
if (EFI_ERROR (Status)) {
@@ -307,14 +315,17 @@ Mtftp4RrqHandleData (
} else {
BlockNum = (UINT16) (Expected - 1);
}
- Mtftp4RrqSendAck (Instance, BlockNum);
+ if (Instance->WindowSize == (BlockNum - Instance->SentAckNo) || Expected < 0) {
+ Status = Mtftp4RrqSendAck (Instance, BlockNum);
+ }
+
}
- return EFI_SUCCESS;
+ return Status;
}
/**
Validate whether the options received in the server's OACK packet is valid.
@@ -347,14 +358,15 @@ Mtftp4RrqOackValid (
if ((Reply->Exist &~Request->Exist) != 0) {
return FALSE;
}
//
- // Server can only specify a smaller block size to be used and
+ // Server can only specify a smaller block size and window size to be used and
// return the timeout matches that requested.
//
if ((((Reply->Exist & MTFTP4_BLKSIZE_EXIST) != 0)&& (Reply->BlkSize > Request->BlkSize)) ||
+ (((Reply->Exist & MTFTP4_WINDOWSIZE_EXIST) != 0)&& (Reply->WindowSize > Request->WindowSize)) ||
(((Reply->Exist & MTFTP4_TIMEOUT_EXIST) != 0) && (Reply->Timeout != Request->Timeout))) {
return FALSE;
}
//
@@ -527,11 +539,11 @@ Mtftp4RrqHandleOack (
if ((Reply.Exist & MTFTP4_MCAST_EXIST) != 0) {
//
// Save the multicast info. Always update the Master, only update the
- // multicast IP address, block size, timeoute at the first time. If IP
+ // multicast IP address, block size, window size, timeoute at the first time. If IP
// address is updated, create a UDP child to receive the multicast.
//
Instance->Master = Reply.Master;
if (Instance->McastIp == 0) {
@@ -596,10 +608,14 @@ Mtftp4RrqHandleOack (
// Update the parameters used.
//
if (Reply.BlkSize != 0) {
Instance->BlkSize = Reply.BlkSize;
}
+
+ if (Reply.WindowSize != 0) {
+ Instance->WindowSize = Reply.WindowSize;
+ }
if (Reply.Timeout != 0) {
Instance->Timeout = Reply.Timeout;
}
}
@@ -609,10 +625,14 @@ Mtftp4RrqHandleOack (
if (Reply.BlkSize != 0) {
Instance->BlkSize = Reply.BlkSize;
}
+ if (Reply.WindowSize != 0) {
+ Instance->WindowSize = Reply.WindowSize;
+ }
+
if (Reply.Timeout != 0) {
Instance->Timeout = Reply.Timeout;
}
}
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index c625fda..83e6837 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -1,9 +1,9 @@
/** @file
Support routines for Mtftp.
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -218,19 +218,19 @@ Mtftp4RemoveBlockNum (
// transfers of unlimited size. There is no consensus, however, whether
// the counter should wrap around to zero or to one. Many implementations
// wrap to zero, because this is the simplest to implement. Here we choose
// this solution.
//
- *TotalBlock = Num;
-
+ *TotalBlock = Num;
+
if (Range->Round > 0) {
- *TotalBlock += Range->Bound + MultU64x32 ((UINTN) (Range->Round -1), (UINT32) (Range->Bound + 1)) + 1;
- }
+ *TotalBlock += Range->Bound + MultU64x32 ((UINTN) (Range->Round -1), (UINT32) (Range->Bound + 1)) + 1;
+ }
if (Range->Start > Range->Bound) {
- Range->Start = 0;
- Range->Round ++;
+ Range->Start = 0;
+ Range->Round ++;
}
if ((Range->Start > Range->End) || Completed) {
RemoveEntryList (&Range->Link);
FreePool (Range);
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
index 802e386..4b7e2b0 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
@@ -1,9 +1,9 @@
/** @file
Support routines for MTFTP.
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -172,24 +172,10 @@ Mtftp4SendError (
IN UINT16 ErrCode,
IN UINT8 *ErrInfo
);
/**
- Retransmit the last packet for the instance.
-
- @param Instance The Mtftp instance
-
- @retval EFI_SUCCESS The last packet is retransmitted.
- @retval Others Failed to retransmit.
-
-**/
-EFI_STATUS
-Mtftp4Retransmit (
- IN MTFTP4_PROTOCOL *Instance
- );
-
-/**
The timer ticking function for the Mtftp service instance.
@param Event The ticking event
@param Context The Mtftp service instance
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
index e825714..ea4de93 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
@@ -1,9 +1,9 @@
/** @file
Routines to process Wrq (upload).
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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<BR>
@@ -19,21 +19,23 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
/**
Build then send a MTFTP data packet for the MTFTP upload session.
@param Instance The MTFTP upload session.
@param BlockNum The block number to send.
+ @param Completed Return whether the upload has finished.
@retval EFI_OUT_OF_RESOURCES Failed to build the packet.
@retval EFI_ABORTED The consumer of this child directs to abort the
transmission by return an error through PacketNeeded.
@retval EFI_SUCCESS The data is sent.
**/
EFI_STATUS
Mtftp4WrqSendBlock (
IN OUT MTFTP4_PROTOCOL *Instance,
- IN UINT16 BlockNum
+ IN UINT16 BlockNum,
+ OUT BOOLEAN *Completed
)
{
EFI_MTFTP4_PACKET *Packet;
EFI_MTFTP4_TOKEN *Token;
NET_BUF *UdpPacket;
@@ -66,19 +68,23 @@ Mtftp4WrqSendBlock (
if (Token->Buffer != NULL) {
Start = MultU64x32 (BlockNum - 1, Instance->BlkSize);
if (Token->BufferSize < Start + Instance->BlkSize) {
DataLen = (UINT16) (Token->BufferSize - Start);
+ if (DataLen == 0) {
+ *Completed = TRUE;
+ return EFI_SUCCESS;
+ }
+
Instance->LastBlock = BlockNum;
Mtftp4SetLastBlockNum (&Instance->Blocks, BlockNum);
}
if (DataLen > 0) {
NetbufAllocSpace (UdpPacket, DataLen, FALSE);
CopyMem (Packet->Data.Data, (UINT8 *) Token->Buffer + Start, DataLen);
- }
-
+ }
} else {
//
// Get data from PacketNeeded
//
DataBuf = NULL;
@@ -101,10 +107,15 @@ Mtftp4WrqSendBlock (
);
return EFI_ABORTED;
}
+ if (DataLen == 0) {
+ *Completed = TRUE;
+ return EFI_SUCCESS;
+ }
+
if (DataLen < Instance->BlkSize) {
Instance->LastBlock = BlockNum;
Mtftp4SetLastBlockNum (&Instance->Blocks, BlockNum);
}
@@ -144,10 +155,15 @@ Mtftp4WrqHandleAck (
)
{
UINT16 AckNum;
INTN Expected;
UINT64 TotalBlock;
+
+ INTN Index;
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
*Completed = FALSE;
AckNum = NTOHS (Packet->Ack.Block[0]);
Expected = Mtftp4GetNextBlockNum (&Instance->Blocks);
@@ -191,11 +207,19 @@ Mtftp4WrqHandleAck (
return EFI_TFTP_ERROR;
}
}
- return Mtftp4WrqSendBlock (Instance, (UINT16) Expected);
+ for (Index = 0; Index < Instance->WindowSize; Index++) {
+ if (!EFI_ERROR (Status) && !(*Completed)) {
+ Status = Mtftp4WrqSendBlock (Instance, (UINT16) (Expected + Index), Completed);
+ } else {
+ break;
+ }
+ }
+
+ return Status;
}
/**
Check whether the received OACK is valid.
@@ -225,14 +249,15 @@ Mtftp4WrqOackValid (
if ((Reply->Exist & ~Request->Exist) != 0) {
return FALSE;
}
//
- // Server can only specify a smaller block size to be used and
+ // Server can only specify a smaller block size and window size to be used and
// return the timeout matches that requested.
//
if ((((Reply->Exist & MTFTP4_BLKSIZE_EXIST) != 0) && (Reply->BlkSize > Request->BlkSize)) ||
+ (((Reply->Exist & MTFTP4_WINDOWSIZE_EXIST) != 0)&& (Reply->WindowSize > Request->WindowSize)) ||
(((Reply->Exist & MTFTP4_TIMEOUT_EXIST) != 0) && (Reply->Timeout != Request->Timeout))) {
return FALSE;
}
return TRUE;
@@ -302,10 +327,14 @@ Mtftp4WrqHandleOack (
if (Reply.BlkSize != 0) {
Instance->BlkSize = Reply.BlkSize;
}
+ if (Reply.WindowSize != 0) {
+ Instance->WindowSize = Reply.WindowSize;
+ }
+
if (Reply.Timeout != 0) {
Instance->Timeout = Reply.Timeout;
}
//
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize
2017-03-16 8:35 [Patch 0/2] windowsize option support Jiaxin Wu
2017-03-16 8:35 ` [Patch 1/2] MdeModulePke/Mtftp4Dxe: Add " Jiaxin Wu
@ 2017-03-16 8:35 ` Jiaxin Wu
2017-03-16 16:12 ` Carsey, Jaben
1 sibling, 1 reply; 5+ messages in thread
From: Jiaxin Wu @ 2017-03-16 8:35 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Carsey Jaben, Wu Jiaxin
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 | 60 ++++++++++++++++++----
.../UefiShellTftpCommandLib.uni | 6 ++-
2 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
index 5c50797..26f73ea 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,16 @@ 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 The default windowsize.
+///
+#define MTFTP_DEFAULT_WINDOWSIZE 1
+#define MTFTP_MAX_WINDOWSIZE 64
/**
Function for 'tftp' command.
@param[in] ImageHandle Handle to the Image (NULL if Internal).
@@ -285,18 +292,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 = 1;
//
// Initialize the Shell library (we must be in non-auto-init...)
//
Status = ShellInitialize ();
@@ -432,10 +441,24 @@ ShellCommandRunTftp (
);
goto Error;
}
}
+ ValueStr = ShellCommandLineGetValue (CheckPackage, L"-w");
+ if (ValueStr != NULL) {
+ if (!StringToUint16 (ValueStr, &WindowSize)) {
+ goto Error;
+ }
+ if (WindowSize < MTFTP_DEFAULT_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 +529,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 +913,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 +956,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 +990,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] 5+ messages in thread
* Re: [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize
2017-03-16 8:35 ` [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
@ 2017-03-16 16:12 ` Carsey, Jaben
2017-03-17 1:23 ` Wu, Jiaxin
0 siblings, 1 reply; 5+ messages in thread
From: Carsey, Jaben @ 2017-03-16 16:12 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan, Carsey, Jaben
I'm a little confused by the constants you defined. 2 questions inline...
Maybe we need another #define so we have: the max, the min, and the default?
-Jaben
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Thursday, March 16, 2017 1:36 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 2/2] ShellPkg/tftp: Add one option for tftp command to
> specify windowsize
> Importance: High
>
> 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 | 60
> ++++++++++++++++++----
> .../UefiShellTftpCommandLib.uni | 6 ++-
> 2 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> index 5c50797..26f73ea 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,16 @@ 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 The default windowsize.
> +///
> +#define MTFTP_DEFAULT_WINDOWSIZE 1
>
> +#define MTFTP_MAX_WINDOWSIZE 64
>
> /**
> Function for 'tftp' command.
>
> @param[in] ImageHandle Handle to the Image (NULL if Internal).
> @@ -285,18 +292,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 = 1;
[Jaben] If the #define is the default, then why do we not initialize the variable with it?
>
> //
> // Initialize the Shell library (we must be in non-auto-init...)
> //
> Status = ShellInitialize ();
> @@ -432,10 +441,24 @@ ShellCommandRunTftp (
> );
> goto Error;
> }
> }
>
> + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-w");
> + if (ValueStr != NULL) {
> + if (!StringToUint16 (ValueStr, &WindowSize)) {
> + goto Error;
> + }
> + if (WindowSize < MTFTP_DEFAULT_WINDOWSIZE || WindowSize >
> MTFTP_MAX_WINDOWSIZE) {
[Jaben] If the value must not be less that it, it should be labeled minimum and not default.
> + 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 +529,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 +913,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 +956,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 +990,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 [flat|nested] 5+ messages in thread
* Re: [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize
2017-03-16 16:12 ` Carsey, Jaben
@ 2017-03-17 1:23 ` Wu, Jiaxin
0 siblings, 0 replies; 5+ messages in thread
From: Wu, Jiaxin @ 2017-03-17 1:23 UTC (permalink / raw)
To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Thanks Jaben, according your comments, I refined the constants definitions.
Best Regards!
Jiaxin
> -----Original Message-----
> From: Carsey, Jaben
> Sent: Friday, March 17, 2017 12:12 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Carsey,
> Jaben <jaben.carsey@intel.com>
> Subject: RE: [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to
> specify windowsize
>
> I'm a little confused by the constants you defined. 2 questions inline...
> Maybe we need another #define so we have: the max, the min, and the
> default?
>
> -Jaben
>
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Thursday, March 16, 2017 1:36 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 2/2] ShellPkg/tftp: Add one option for tftp command to
> > specify windowsize
> > Importance: High
> >
> > 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 | 60
> > ++++++++++++++++++----
> > .../UefiShellTftpCommandLib.uni | 6 ++-
> > 2 files changed, 54 insertions(+), 12 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> > b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c
> > index 5c50797..26f73ea 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,16 @@ 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 The default windowsize.
> > +///
> > +#define MTFTP_DEFAULT_WINDOWSIZE 1
> >
> > +#define MTFTP_MAX_WINDOWSIZE 64
> >
> > /**
> > Function for 'tftp' command.
> >
> > @param[in] ImageHandle Handle to the Image (NULL if Internal).
> > @@ -285,18 +292,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 = 1;
>
> [Jaben] If the #define is the default, then why do we not initialize the
> variable with it?
>
> >
> > //
> > // Initialize the Shell library (we must be in non-auto-init...)
> > //
> > Status = ShellInitialize ();
> > @@ -432,10 +441,24 @@ ShellCommandRunTftp (
> > );
> > goto Error;
> > }
> > }
> >
> > + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-w");
> > + if (ValueStr != NULL) {
> > + if (!StringToUint16 (ValueStr, &WindowSize)) {
> > + goto Error;
> > + }
> > + if (WindowSize < MTFTP_DEFAULT_WINDOWSIZE || WindowSize >
> > MTFTP_MAX_WINDOWSIZE) {
>
> [Jaben] If the value must not be less that it, it should be labeled minimum
> and not default.
>
> > + 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 +529,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 +913,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 +956,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 +990,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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-17 1:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-16 8:35 [Patch 0/2] windowsize option support Jiaxin Wu
2017-03-16 8:35 ` [Patch 1/2] MdeModulePke/Mtftp4Dxe: Add " Jiaxin Wu
2017-03-16 8:35 ` [Patch 2/2] ShellPkg/tftp: Add one option for tftp command to specify windowsize Jiaxin Wu
2017-03-16 16:12 ` Carsey, Jaben
2017-03-17 1:23 ` Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox