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:[email protected]] On Behalf Of > Marcin Wojtas > Sent: Friday, April 29, 2016 9:09 AM > To: [email protected] > Cc: [email protected]; [email protected]; [email protected]; Gao, > Liming <[email protected]>; Kinney, Michael D > <[email protected]> > Subject: [edk2] [RFC] MdeModulePkg, ShellPkg: increase TFTP block size > > From: Bartosz Szczepanek <[email protected]> > > 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 > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
