On Fri 24 Jul 2009 04:04, Alessandro Rubini pondered:
> This patch add a quick and dirty defrag step in IP reception. This
> allows to increase the TFTP block size and get more performance in
> slow links (but at that point it should be made configurable).
>
> The overhead is negligible, verified with an ARM9 CPU and 10MB data
> file, changing the server MTU from 1500 to 800 and then 550. However,
> on a LAN connection, I didn't see advantes with using a 4k block
> size with default MTU.
>
> Signed-off-by: Alessandro Rubini <rub...@gnudd.com>
> ---
>
> This patch is over mainline, without the (much appreciated) cleanup
> patch that reached the list these days.
>
> net/net.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index 641c37c..5034a2e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1117,6 +1117,46 @@ static void CDPStart(void)
> }
> #endif
Should have a CONFIG_ something - to make this conditional.
> +/* This only reassembles fragments that come in proper order */
> +static inline IP_t *NetDefragment(IP_t *ip, int *lenp)
> +{
> + static uchar pkt_buff[16384]; /*temporary arbitrary limit */
> + static int next_fragment;
> + static ushort pkt_id;
pkt_buff needs to be aligned - in case you want to start poking in it (which
you are going to want to do...) just add a:
__attribute__((aligned(PKTALIGN)));
> + #define IPSZ 20
I think what you want is "IP_HDR_SIZE_NO_UDP" - from include/net.h
> + uchar *pkt = (uchar *)ip;
> + ushort ip_off;
> + int offset, len = *lenp -2;
Some ethernet drivers are messed up, and provide bogus lengths (add a few
bytes that shouldn't be there).
It's better to get the length from the IP header. Adjusted to dump the header:
len = ntohs(ip->ip_len) - IP_HDR_SIZE_NO_UDP;
> + ip_off = ntohs(ip->ip_off);
> + if (!(ip_off & (IP_OFFS | IP_FLAGS_MFRAG)))
> + return ip;
Why not do this in NetReceive()?
JMP/RTS are much more expensive on most archs than a if()
> + offset = (ip_off & IP_OFFS) * 8;
> + if (!offset) { /* new packet begins, discard any we might have
> */
> + pkt_id = ip->ip_id;
> + memcpy(pkt_buff, ip, len);
> + next_fragment = len;
> + return NULL;
> + }
> +
> + /* further fragment: discard IP header */
> + offset += IPSZ; len -= IPSZ; pkt += IPSZ;
> +
> + if (ip->ip_id != pkt_id || offset != next_fragment)
> + return NULL; /* out of order */
We should check more than packet id - in case it is coming from a different
host...
if (ip->ip_id != (IP_t *)pkt_buff->ip_id || /* check the packet ID */
ip->ip_p != (IP_t *)pkt_buff->ip_p || /* check the protocol */
ip->ip_src != (IP_t *)pkt_buff->ip_src || /* check the source */
ip->ip_dst != (IP_t *)pkt_buff->ip_dst ) /* check the dest */
> + /* further fragment: skip ip header (we miss offset_of...) */
> + memcpy(pkt_buff + next_fragment, pkt, len);
> + next_fragment += len;
> +
> + if (ip_off & IP_FLAGS_MFRAG)
> + return NULL; /* more expected */
> +
> + *lenp = next_fragment;
> + return (IP_t *)pkt_buff;
> +}
>
> void
> NetReceive(volatile uchar * inpkt, int len)
> @@ -1360,6 +1400,8 @@ NetReceive(volatile uchar * inpkt, int len)
> break;
>
> case PROT_IP:
> + if (!(ip = NetDefragment(ip, &len)))
> + return;
> #ifdef ET_DEBUG
> puts ("Got IP\n");
> #endif
I guess this is includes some generic cleanup - but at least the reassembly
should happen _after_ the sanity checks are done. Something like:
case PROT_IP:
#ifdef ET_DEBUG
puts ("Got IP\n");
#endif
/* Before we start poking the header, make sure it is there */
if (len < IP_HDR_SIZE) {
debug ("len bad %d < %lu\n", len, (ulong)IP_HDR_SIZE);
return;
}
/* can't deal with anything except IPv4 */
if ((ip->ip_hl_v & 0xf0) != 0x40) {
debug("I only understand IPv4\n");
return;
}
/* can't deal with IP options (headers != 20 bytes) */
if ((ip->ip_hl_v & 0x0f) * 4 != IP_HDR_SIZE_NO_UDP) {
debug("Can't deal with IP options\n");
return;
}
/* Check the Checksum of the header */
if (!NetCksumOk((uchar *)ip, IP_HDR_SIZE_NO_UDP / 2)) {
puts ("IP header checksum bad\n");
return;
}
/* Check the packet length */
if (len < ntohs(ip->ip_len)) {
printf("len bad %d < %d\n", len, ntohs(ip->ip_len));
return;
}
/* If it is not for us, ignore it */
tmp = NetReadIP(&ip->ip_dst);
if (NetOurIP && tmp != NetOurIP && tmp != 0xFFFFFFFF) {
#ifdef CONFIG_MCAST_TFTP
if (Mcast_addr != tmp)
#endif
return;
}
#ifdef CONFIG_NET_FRAGMENT
/* If it is a IP fragment, try to combine it with it */
if (ntohs(ip->ip_off) & (IP_OFFS | IP_FLAGS_MFRAG)) {
debug("received fragmented packet\n");
ip = NetDefragment(ip);
if (!ip)
return;
}
#endif
len = ntohs(ip->ip_len);
#ifdef ET_DEBUG
printf("len=%d, v=%02x\n", len, ip->ip_hl_v & 0xff);
#endif
> @@ -1378,10 +1420,6 @@ NetReceive(volatile uchar * inpkt, int len)
> if ((ip->ip_hl_v & 0xf0) != 0x40) {
> return;
> }
> - /* Can't deal with fragments */
> - if (ip->ip_off & htons(IP_OFFS | IP_FLAGS_MFRAG)) {
> - return;
> - }
> /* can't deal with headers > 20 bytes */
> if ((ip->ip_hl_v & 0x0f) > 0x05) {
> return;
I also had (for easier testing)
@@ -478,8 +497,15 @@ TftpStart (void)
#ifdef CONFIG_TFTP_PORT
char *ep; /* Environment pointer */
#endif
TftpServerIP = NetServerIP;
+#ifdef CONFIG_NET_FRAGMENT
+ {
+ char *tmp;
+ tmp = getenv("tftpblocksize") ;
+ if (tmp)
+ TftpBlkSizeOption = simple_strtol(tmp, NULL, 10);
+ else
+ TftpBlkSizeOption = TFTP_MTU_BLOCKSIZE;
+ debug("using TftpBlkSizeOption = %d\n", TftpBlkSizeOption);
+ }
+#endif
if (BootFile[0] == '\0') {
sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
NetOurIP & 0xFF,
and I was doing md5 or sha1 on things to make sure that things came over
properly...
-Robin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
