Maybe we should have a PCD control the default value so that people who build it can set it as per their own best value. Adding a parameter seems like a great (but separate) solution.
-Jaben > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Marcin Wojtas > Sent: Friday, April 29, 2016 2:15 AM > To: Fu, Siyuan <siyuan...@intel.com> > Cc: ha...@marvell.com; edk2-devel@lists.01.org; Gao, Liming > <liming....@intel.com>; n...@marvell.com; leif.lindh...@linaro.org; > Kinney, Michael D <michael.d.kin...@intel.com> > Subject: Re: [edk2] [RFC] MdeModulePkg, ShellPkg: increase TFTP block size > Importance: High > > Hi Siyuan, > > Thanks a lot for thorough explanation. The default 512B won't be > changed then - PCD and extending tftp command seem a good alternative. > We will come back later with more proper solution. > > Best regards, > Marcin > > 2016-04-29 10:40 GMT+02:00 Fu, Siyuan <siyuan...@intel.com>: > > Hi, Marcin > > > > The RFC2348 has a detail discussion about the block size setting in tftp and > give the answer: using value of MTU is not the best choice. You may find you > still can gain more efficiency by increasing the block size over the path MTU, > until the IP fragmentation gives more overhead than the tftp package frame > and process. > > > > The default block size (512) of tftp is defined in the RFC1350, that's why > > the > Mtftp driver use 512 as the default macro value. The MTFTP4 prococol does > provide the interface to allow the caller to provide a "blksize" option in the > MTFTP request, so I prefer to keep the default value to 512 in the Mtftp4 > driver, to follow RFC's definition. Another driver in edk2 stack which is > using > the mtftp protocol, like the PXE driver, provides a PCD > gEfiMdeModulePkgTokenSpaceGuid.PcdTftpBlockSize which allow the users > to configure the default block size during PXE mtftp download. > > > > For the shell "tftp" command, I also suggest to add a new argument to let > user set the block size, not simply override it with 1468. > > > > Best Regards > > Siyuan > > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > >> Marcin Wojtas > >> Sent: Friday, April 29, 2016 9:09 AM > >> To: edk2-devel@lists.01.org > >> Cc: ha...@marvell.com; leif.lindh...@linaro.org; n...@marvell.com; > Gao, > >> Liming <liming....@intel.com>; Kinney, Michael D > >> <michael.d.kin...@intel.com> > >> Subject: [edk2] [RFC] MdeModulePkg, ShellPkg: increase TFTP block size > >> > >> From: Bartosz Szczepanek <b...@semihalf.com> > >> > >> Hi, > >> > >> When dealing with low network speed during tftp, the analysis showed > >> two issues - UEFI tftp and network stack spend a lot of time (~3.5ms, > >> comparing to U-Boot's ~0.6ms) on preparing ACK packet after receiving > >> data block from host. The network driver itself is using SNP and its > >> time spent on executing SnpTransmit routine is negligible, so it is > >> definitely not a problem. > >> > >> This patch however is only releated to above mentioned problem. It > >> occurred that despite our MTU is set to 1500, each block of data has > >> to be split to 512 chunks, so the very long receive/ack sequence was > >> executed three times more than needed. > >> > >> Below is an example of a solution of increasing TFTP block size > >> to maximum allowed by 1500 MTU. The default block size is changed, > >> as well as this option is negotiatied with host. For sure a situation, > >> when proposed, increased block size is rejected by host (a default > >> fall back to 512 should happen) should be handled properly. > >> > >> Also a dynamic dependency between port's MTU was considered, but > there > >> are two problems - how to pass it to Mtftp4 module and way of negotiating > >> size with host - option requires string format. Hence IMO it's better > >> to stick to arbitrary size. > >> > >> I would be very grateful for any comments and ideas, how to handle > >> this issue properly. > >> > >> Best reards, > >> Marcin > >> > >> --- > >> MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h | 2 +- > >> ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c | 5 +++++ > >> 2 files changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h > >> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h > >> index 527fd1d..9a2d409 100644 > >> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h > >> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h > >> @@ -55,7 +55,7 @@ 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_BLKSIZE 1468 > >> #define MTFTP4_TIME_TO_GETMAP 5 > >> > >> #define MTFTP4_STATE_UNCONFIGED 0 > >> diff --git a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c > >> b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c > >> index 831b9c3..3e06b67 100644 > >> --- a/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c > >> +++ b/ShellPkg/Library/UefiShellTftpCommandLib/Tftp.c > >> @@ -894,12 +894,17 @@ DownloadFile ( > >> TftpContext->DownloadedNbOfBytes = 0; > >> TftpContext->LastReportedNbOfBytes = 0; > >> > >> + UINT8 name[] = "blksize"; > >> + UINT8 val[] = "1468"; > >> + EFI_MTFTP4_OPTION Blksize = { name, val }; > >> ZeroMem (&Mtftp4Token, sizeof (EFI_MTFTP4_TOKEN)); > >> Mtftp4Token.Filename = (UINT8*)AsciiFilePath; > >> Mtftp4Token.BufferSize = FileSize; > >> Mtftp4Token.Buffer = Buffer; > >> Mtftp4Token.CheckPacket = CheckPacket; > >> Mtftp4Token.Context = (VOID*)TftpContext; > >> + Mtftp4Token.OptionCount = 1; > >> + Mtftp4Token.OptionList = &Blksize; > >> > >> ShellPrintHiiEx ( > >> -1, -1, NULL, STRING_TOKEN (STR_TFTP_DOWNLOADING), > >> -- > >> 1.8.3.1 > >> > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
