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?