[U-Boot] [PATCH 1/3] net: defragment IP packets

[Robin Getz]

On Fri 31 Jul 2009 03:46, Alessandro Rubini pondered:
> Thanks for your comments.
> 
> >> +#ifndef CONFIG_TFTP_MAXBLOCK
> >> +#define CONFIG_TFTP_MAXBLOCK 16384
> > 
> > It is more than tftp - nfs could also use the same.
> 
> Yes, I know. But most users are tftp ones. And if you want an even
> number (like 16k) as a tftp packet you need to add the headers and the
> sequence count. And I prefer to have the useful number in the config.
> So I used "TFTP" in the name in order for NFS users to know they must
> make some calculation.
>  
> > How about CONFIG_NET_MAXDEFRAG instead?
> 
> We could have MAXPAYLOAD if we count in NFS overhead as well (I don't
> know how much it is, currently.  Hope you see my point.

Not really.

IMHO - The protocol max payload should be taken care of on the protocol side, 
not the common network side.

It then becomes:

#define TFTP_MTU_BLOCKSIZE (CONFIG_NET_MAXDEFRAG - TFTP_OVERHEAD)

#define NFS_READ_SIZE   (CONFIG_NET_MAXDEFRAG - NFS_OVERHEAD)

or something like that (since NFS likes to be power of two, 1024, 2048, etc - 
it would need to be tweaked a little), but you get the idea...



> >> +static IP_t *__NetDefragment(IP_t *ip, int *lenp)
> >> +{
> > 
> > I don't understand the purpose of the lenp.
> > 
> > The calling function doesn't use the len var, except for
> > ICMP_ECHO_REQUEST,  which are not allowed to be fragmented.
> > 
> > I eliminated it - and suffered no side effects.
> 
> Well, since the caller has this "len" variable, I didn't want to leave
> it corrupted. But if it's actually unused after this point, we can well
> discard it.

OK.


> >> +#else /* !CONFIG_IP_DEFRAG */
> >> +
> >> +static inline IP_t *NetDefragment(IP_t *ip, int *lenp)
> >> +{
> >> +	return ip;
> >> +}
> >> +#endif
> > 
> > This needs to have the same logic (ip_off & (IP_OFFS | IP_FLAGS_MFRAG)) as
> > the above function. See comment below.
> 
> Yes, correct. Thanks.

Were you going to send an update for Ben?