RE: [PATCH] net: lwip: fix TFTP blocksize threshold check

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