> > > > >>> 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.
I think both of those options should likely be on by default, I suspect that they're not is likely an oversight, Jerome can you provide more details here? > Would you all agree with enabling both? I don't think we have anything > that needs to sends fraemented packets via lwIP yet. Yes, I don't think we do by default, but networks, especially outside of the datacentre, are hard and you never know why a packet maybe fragmented. > Is there value in adding support for the 'tftpblocksize' env var > supported in legacy tftp? Maybe for consistency, but also it's a new feature and if they've not need it to now is it likely useful? Peter