> So, I'm not a fan of hard-coded buffer sizes, or how this existing bit > of code duplicates functionality already available elsewhere in the > tree ... but I also don't care much about ArmBds. > > What I'm curious about is - why is your Mtftp4GetFileSize call failing > such that you need to use a hardcoded buffer size? > Does your TFTP server not support rfc2349? > Would changing that (for example by switching to tftpd-hpa over the > unmaintained netkit-tftpd) not make more sense?
Took a while to test with tftpd-hpa, with this it does work fine, i.e. Mtftp4GetFileSize does succeed and buffer size is allocated accordingly. I would still request you to merge the change, as it will mean people using the default tftpd that comes with ubuntu distribution will also be able to use the tftp boot for ARM platforms. -----Original Message----- From: Leif Lindholm [mailto:leif.lindh...@linaro.org] Sent: 05 November 2015 12:26 To: Ashutosh Singh Cc: edk2-de...@ml01.01.org; ard.biesheu...@linaro.org; ryan.har...@linaro.org; James King Subject: Re: [PATCH] Increase tftp buffer size On Thu, Nov 05, 2015 at 11:37:43AM +0000, Ashutosh Singh wrote: > Default tftp buffer size (8MB) is too small for standard > ARM kernel images, which results in multiple aborted tftp fetches. > This fix increase the buffer size to 16MB. So, I'm not a fan of hard-coded buffer sizes, or how this existing bit of code duplicates functionality already available elsewhere in the tree ... but I also don't care much about ArmBds. What I'm curious about is - why is your Mtftp4GetFileSize call failing such that you need to use a hardcoded buffer size? Does your TFTP server not support rfc2349? Would changing that (for example by switching to tftpd-hpa over the unmaintained netkit-tftpd) not make more sense? / Leif > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ashutosh Singh <ashutosh.si...@arm.com> > --- > ArmPkg/Library/BdsLib/BdsFilePath.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c > b/ArmPkg/Library/BdsLib/BdsFilePath.c > index ff42175..0410236 100644 > --- a/ArmPkg/Library/BdsLib/BdsFilePath.c > +++ b/ArmPkg/Library/BdsLib/BdsFilePath.c > @@ -1198,7 +1198,7 @@ BdsTftpLoadImage ( > if (Mtftp4GetFileSize (Mtftp4, AsciiFilePath, &FileSize) == EFI_SUCCESS) { > TftpBufferSize = FileSize; > } else { > - TftpBufferSize = SIZE_8MB; > + TftpBufferSize = SIZE_16MB; > } > > TftpContext = AllocatePool (sizeof (BDS_TFTP_CONTEXT)); > @@ -1209,7 +1209,7 @@ BdsTftpLoadImage ( > TftpContext->FileSize = FileSize; > > for (; TftpBufferSize <= FixedPcdGet32 (PcdMaxTftpFileSize); > - TftpBufferSize = (TftpBufferSize + SIZE_8MB) & (~(SIZE_8MB-1))) { > + TftpBufferSize = (TftpBufferSize + SIZE_16MB) & (~(SIZE_16MB-1))) { > // > // Allocate a buffer to hold the whole file. > // > -- > 1.7.9.5 > > > ________________________________ > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > ________________________________ -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
