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

Robin Getz rgetz at blackfin.uclinux.org
Mon Jul 27 02:59:33 CEST 2009


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 <rubini at 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


More information about the U-Boot mailing list