AMD General Hi Jerome,
> From: Jerome Forissier <[email protected]> > Sent: Wednesday, August 5, 2026 5:15 PM > To: Begari, Padmarao <[email protected]>; [email protected] > project.org; Simek, Michal <[email protected]> > Cc: git (AMD-Xilinx) <[email protected]>; Tom Rini <[email protected]>; Vinay > Tilak, > Pranav <[email protected]>; nd <[email protected]> > Subject: Re: [PATCH] net: lwip: fix TFTP blocksize threshold check > > Hi Padmarao, > > On 05/08/2026 12:33, Padmarao Begari wrote: > > PBUF_POOL_SIZE/IP_REASS_MAX_PBUFS are only scaled up for large TFTP > > block sizes when CONFIG_TFTP_BLOCKSIZE is strictly greater than > > TFTP_BLOCKSIZE_THRESHOLD (4096). Since the threshold itself is 4096, > > setting CONFIG_TFTP_BLOCKSIZE=4096 exactly falls through to the small > > fixed pool (PBUF_POOL_SIZE=8, IP_REASS_MAX_PBUFS=4) instead of the > > scaled one. > > What practical benefit do we get by using the scaled velues (for 4096 that's > PBUF_POOL_SIZE=9, IP_REASS_MAX_PBUFS=5 if I'm not mistaken)? > > > > At blocksize 4096, the full UDP datagram (4096 data + 4-byte TFTP > > header + 8-byte UDP header = 4108 bytes) exceeds the usable IP > > fragment size (1480 bytes) and is split into 3 IP fragments that lwIP > > must reassemble on receive. The scaled sizing accounts for this > > explicitly (giving PBUF_POOL_SIZE=9, IP_REASS_MAX_PBUFS=5), so the > > threshold should include the boundary value rather than exclude it. > > > > Change the comparison to '>=' so that CONFIG_TFTP_BLOCKSIZE=4096 also > > gets the scaled pool sizing. > > > > Fixes: 67586012490a ("net: lwip: scale buffer pool size with TFTP > > block size") > > Signed-off-by: Padmarao Begari <[email protected]> > > --- > > lib/lwip/u-boot/lwipopts.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/lwip/u-boot/lwipopts.h b/lib/lwip/u-boot/lwipopts.h > > index 8dae004f1a2..733478a0cda 100644 > > --- a/lib/lwip/u-boot/lwipopts.h > > +++ b/lib/lwip/u-boot/lwipopts.h > > @@ -72,7 +72,7 @@ > > #define PBUF_POOL_RESERVE 4 > > #define TFTP_BLOCKSIZE_THRESHOLD 4096 > > > > -#if defined(CONFIG_TFTP_BLOCKSIZE) && (CONFIG_TFTP_BLOCKSIZE > > > TFTP_BLOCKSIZE_THRESHOLD) > > +#if defined(CONFIG_TFTP_BLOCKSIZE) && (CONFIG_TFTP_BLOCKSIZE >= > > +TFTP_BLOCKSIZE_THRESHOLD) > > #define PBUF_POOL_SIZE (((CONFIG_TFTP_BLOCKSIZE > + (IP_FRAG_MTU_USABLE - 1)) / \ > > IP_FRAG_MTU_USABLE) + > PBUF_POOL_HEADROOM) > > I realize that this computation of PBUF_POOL_SIZE does not take into account > the > TFTP overhead. Shouldn't this be: > > #define TFTP_PACKET_OVERHEAD (4 + 8) > > #define PBUF_POOL_SIZE \ > (((CONFIG_TFTP_BLOCKSIZE + TFTP_PACKET_OVERHEAD + \ > IP_FRAG_MTU_USABLE - 1) / IP_FRAG_MTU_USABLE) + \ > PBUF_POOL_HEADROOM) > I encountered an issue with tftpboot(lwip) when tftpblocksize was set to 4096 using the existing Xilinx/AMD MRMAC driver, which is configured with only two Rx/Tx buffer descriptors (BDs). Reducing the tftpblocksize to 1468 bytes allowed tftpboot to work correctly, but the issue persisted with a block size of 4096 bytes. I then changed the threshold comparison to use >= instead of >, and tftpboot started working with a block size of 4096. Additionally, I updated the MRMAC driver to increase the number of Rx BDs to 32 and tested again with tftpblocksize=4096, without applying the threshold comparison change. With this update, tftpboot also worked correctly. I will continue debugging to identify the root cause, but the change you suggested is working as well. Regards Padmarao > ? > Thanks, > -- > Jerome
