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 <[email protected]>: > 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
