On Wed, May 28, 2025 at 10:23 AM Peter Robinson <pbrobin...@gmail.com> wrote:
>
> On Wed, 28 May 2025 at 17:59, Tim Harvey <thar...@gateworks.com> wrote:
> >
> > On Wed, May 28, 2025 at 5:24 AM Jerome Forissier
> > <jerome.foriss...@linaro.org> wrote:
> > >
> > >
> > >
> > > On 5/27/25 18:35, Tim Harvey wrote:
> > > > On Mon, May 26, 2025 at 5:17 AM Jerome Forissier
> > > > <jerome.foriss...@linaro.org> wrote:
> > > >>
> > > >> Hi Tim,
> > > >>
> > > >> On 5/23/25 20:31, Tim Harvey wrote:
> > > >>> Hi Jerome,
> > > >>>
> > > >>> I've enabled LWIP on imx8mm_venice to see how it's doing and while
> > > >>> dhcp/dns/ping/wget appear to work great, tftp is broken.
> > > >>>
> > > >>> Enabling LWIP_DEBUG shows the following:
> > > >>> ip4_input: packet accepted on interface et
> > > >>> IP packet dropped since it was fragmented (0x2000) (while 
> > > >>> IP_REASSEMBLY == 0).
> > > >>
> > > >> Hmmmmm IP_REASSEMBLY should probably default to 1.
> > > >>>
> > > >>> I tried enabling IP_REASSEMBLY in lwip but that seems to change the 
> > > >>> issue to:
> > > >>> ip4_input: packet accepted on interface et
> > > >>> ip4_input:
> > > >>> IP header:
> > > >>> +-------------------------------+
> > > >>> | 4 | 5 |  0xc0 |       108     | (v, hl, tos, len)
> > > >>> +-------------------------------+
> > > >>> |    10568      |000|       0   | (id, flags, offset)
> > > >>> +-------------------------------+
> > > >>> |   64  |    1  |    0xe25b     | (ttl, proto, chksum)
> > > >>> +-------------------------------+
> > > >>> |  172  |   24  |    0  |    2  | (src)
> > > >>> +-------------------------------+
> > > >>> |  172  |   24  |   21  |  251  | (dest)
> > > >>> +-------------------------------+
> > > >>> ip4_input: p->len 108 p->tot_len 108
> > > >>> Unsupported transport protocol 1
> > > >>>
> > > >>> Any ideas what the issue is here?
> > > >>
> > > >> Not really :-/ Does the transfer work with a smaller block size?
> > > >> (CONFIG_TFTP_BLOCKSIZE, default 1468, try 500 for instance). You could
> > > >> also enable CONFIG_LWIP_DEBUG_RXTX in order to get a full hex dump of
> > > >> the messages going into/out of the lwIP stack.
> > > >
> > > > Hi Jerome,
> > > >
> > > > My config was with 4096, setting it to 1468 works and anything above
> > > > that seems to cause this failure.
> > >
> > > Good to know.
> > >
> > > > Some sort of fragmentation issue?
> > >
> > > Quite possibly, yes. In any case it does not make much sense to use a
> > > clock size that causes the packet to be larger than the PMTU (it would
> > > be sub-optimal) but it should definitely work, especially since I don't
> > > believe PMTU discovery is implemented in lwIP. I add this to my to-do
> > > list but feel free to investigate further :)
> > >
> >
> > Hi Jerome,
> >
> > I'll take a look at it as larger than MTU packets are definitely not
> > suboptimal for TFTP. A 4096B block size will ack once per 4K so 4x
> > less ACK's and in my testing the 4K blocksize gives a 3x performance
> > boost on large files.
>
> Refreshing my memory on TFTP slightly, I think larger than MTU tftp
> transfers need support for RFC 2348 [1], maybe that's something we
> need to explicitly enable if the TFTP layer is part of LWIP (or maybe
> LWIP doesn't support it), or if the tftp application is ours on top of
> LWIP maybe we're missing some glue. I didn't look at the code, rather
> this was from some of my own notes from a long time ago.
>
> [1] https://www.rfc-editor.org/rfc/rfc2348

The issue was simply that IP_REASSEMBLY was not enabled for U-Boot
lwIP - I thought I tried that originally but must have done it wrong
(probably picked the wrong header vs lib/lwip/u-boot/lwipopts.h).

(+cc Joe and Ramon network maintainers)

I can submit a patch but it seems to me we should enable both
IP_REASSEMBLY (reassemble incoming fragmented IP packets) as well as
IP_FRAG (fragment outgoing IP packets if their size exceeds MTU) for
lwIP as both are currently disabled.

Would you all agree with enabling both? I don't think we have anything
that needs to sends fraemented packets via lwIP yet.

Is there value in adding support for the 'tftpblocksize' env var
supported in legacy tftp?

Best Regards,

Tim

Reply via email to