[U-Boot] [PATCH] Add simple IP/UDP fragmentation support
Ben Warren
biggerbadderben at gmail.com
Wed Aug 27 09:39:44 CEST 2008
Hi Josh & Frank,
A few nits. Overall a nice patch.
One question: Will multicast TFTP still work when this is applied?
Josh Boyer wrote:
> From: Frank Haverkamp <haver at vnet.ibm.com>
>
> http://tools.ietf.org/html/rfc2348 describes the TFTP block size option
> which allows larger packtes than the 512 byte default. This reduces the
>
s/packtes/packets/
> number of TFTP ACKs significantly and improves performance.
>
> To get the most benefit out of the tftp block size option the support
> of defragementation of IP/UDP packet is helpful. The current implemenation
>
s/defragementation/defragmentation/
s/implemenation/implementation/
> should work even with packets received out of order. To enable the large
> packet size the user should set "tftp_block_size" so a value like 16352.
>
>
s/so/to/
> We experimented with different packet sizes and found that more than those
> 16KiB do not contribute much to the performance anymore. Therefor I limited
>
s/Therefor/Therefore/
> the defragmentation buffer to 16KiB no too waste memory.
>
>
so as to not waste memory
> Signed-off-by: Frank Haverkamp <haver at vnet.ibm.com>
> Signed-off-by: Josh Boyer <jwboyer at linux.vnet.ibm.com>
>
> ---
> include/net.h | 17 ++++++
> net/net.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
> net/tftp.c | 22 ++++++++
> net/tftp.h | 10 +++
> 4 files changed, 185 insertions(+), 20 deletions(-)
>
> --- u-boot.git.orig/include/net.h
> +++ u-boot.git/include/net.h
> @@ -200,6 +200,13 @@ typedef struct {
> ushort udp_xsum; /* Checksum */
> } IP_t;
>
> +#define IP_OFFS 0x1FFF /* ip offset *= 8 */
> +#define IP_OFFS_SHIFT 3 /* in 8 byte steps */
> +#define IP_FLAGS 0xE000 /* first 3 bits */
> +#define IP_FLAGS_RES 0x8000 /* reserved */
> +#define IP_FLAGS_DFRAG 0x4000 /* don't fragments */
> +#define IP_FLAGS_MFRAG 0x2000 /* more fragments */
> +
>
Please align these better. After applying, it's:
#define IP_OFFS 0x1FFF /* ip offset *= 8 */
#define IP_OFFS_SHIFT 3 /* in 8 byte steps */
#define IP_FLAGS 0xE000 /* first 3 bits */
#define IP_FLAGS_RES 0x8000 /* reserved */
#define IP_FLAGS_DFRAG 0x4000 /* don't fragments */
#define IP_FLAGS_MFRAG 0x2000 /* more fragments */
> #define IP_HDR_SIZE_NO_UDP (sizeof (IP_t) - 8)
> #define IP_HDR_SIZE (sizeof (IP_t))
>
> @@ -282,6 +289,16 @@ typedef struct icmphdr {
> #define PKTSIZE_ALIGN 1536
> /*#define PKTSIZE 608*/
>
> + /*
> + * IP/UDP Fragmentation support
> + * See: http://en.wikipedia.org/wiki/IPv4#Fragmentation_and_reassembly
> + * MAX possible UDP packet size is 64 KiB, if there is memory available.
> + */
> +#define NET_ETH_MTU 1500
> +#define NET_FRAG_BUF_SIZE (16 * 1024) /* MAX is 64 KiB */
> +#define NET_UDP_FRAG_SIZE (NET_ETH_MTU - IP_HDR_SIZE_NO_UDP) /* 1480 */
> +#define NET_FRAG_BUF_USED (NET_FRAG_BUF_SIZE / NET_UDP_FRAG_SIZE + 1)
> +
> /*
> * Maximum receive ring size; that is, the number of packets
> * we can buffer before overflow happens. Basically, this just
> --- u-boot.git.orig/net/net.c
> +++ u-boot.git/net/net.c
> @@ -192,6 +192,15 @@ volatile uchar PktBuf[(PKTBUFSRX+1) * PK
>
> volatile uchar *NetRxPackets[PKTBUFSRX]; /* Receive packets */
>
> +/* Packet fragmentation support */
> +static uint16_t ip_id = 0; /* sequence number */
> +static uint16_t udp_len = 0;
> +static uint16_t udp_src = 0;
> +static uint16_t udp_dst = 0;
> +static int max_idx = 0;
> +static uchar NetFragBuf[NET_FRAG_BUF_SIZE];
> +static char NetFragBufUsed[NET_FRAG_BUF_USED] = { 0, };
> +
> static rxhand_f *packetHandler; /* Current RX packet handler */
> static thand_f *timeHandler; /* Current timeout handler */
> static ulong timeStart; /* Time base value */
> @@ -288,6 +297,13 @@ NetLoop(proto_t protocol)
> {
> bd_t *bd = gd->bd;
>
> + /* Packet fragmentation support */
> + ip_id = udp_len = udp_src = udp_dst = max_idx = 0;
> + memset(NetFragBuf, 0xFF, sizeof(NetFragBuf));
> + memset(NetFragBufUsed, 0, sizeof(NetFragBufUsed));
> + printf("NetFragBuf @ %08x max tftp_block_size=%d udp_frag_size=%d\n",
> + NetFragBuf, TFTP_BLOCK_SIZE_MAX, NET_UDP_FRAG_SIZE);
> +
> #ifdef CONFIG_NET_MULTI
> NetRestarted = 0;
> NetDevExists = 0;
> @@ -1150,6 +1166,39 @@ static void CDPStart(void)
> }
> #endif
>
> +#ifdef CONFIG_UDP_CHECKSUM
> +/*
> + * @sumptr: Points to UDP data
> + * @sumlen: Size of UDP data
> + * @xsum: UDP checksum across IP source, destination address, protocol and size
> + *
> + * Returns 0 when checksum is correct and 1 if it is not
>
Can you return -1 on failure? Not a big deal, just more conventional.
> + */
> +static int udp_checksum(ushort *sumptr, ushort sumlen, ulong xsum)
> +{
> + while (sumlen > 1) {
> + ushort sumdata;
> +
> + sumdata = *sumptr++;
> + xsum += ntohs(sumdata);
> + sumlen -= 2;
> + }
> + if (sumlen > 0) {
> + ushort sumdata;
> +
> + sumdata = *(unsigned char *) sumptr;
> + sumdata = (sumdata << 8) & 0xff00;
> + xsum += sumdata;
> + }
> + while ((xsum >> 16) != 0) {
> + xsum = (xsum & 0x0000ffff) + ((xsum >> 16) & 0x0000ffff);
> + }
> + if ((xsum != 0x00000000) && (xsum != 0x0000ffff))
> + return 1;
> +
> + return 0;
> +}
> +#endif /* CONFIG_UDP_CHECKSUM */
>
> void
> NetReceive(volatile uchar * inpkt, int len)
> @@ -1164,6 +1213,7 @@ NetReceive(volatile uchar * inpkt, int l
> int iscdp;
> #endif
> ushort cti = 0, vlanid = VLAN_NONE, myvlanid, mynvlanid;
> + uint32_t off; /* ip_off for fragmentation */
>
Can you pick a better variable name than 'off'? 'offset' maybe?
>
> #ifdef ET_DEBUG
> printf("packet received\n");
> @@ -1404,9 +1454,11 @@ NetReceive(volatile uchar * inpkt, int l
> if ((ip->ip_hl_v & 0xf0) != 0x40) {
> return;
> }
> +#if 0 /* Obsolete after adding the fragmentation support */
> if (ip->ip_off & htons(0x1fff)) { /* Can't deal w/ fragments */
> return;
> }
> +#endif
>
Please delete this block if it's now dead code.
> /* can't deal with headers > 20 bytes */
> if ((ip->ip_hl_v & 0x0f) > 0x05) {
> return;
> @@ -1422,6 +1474,88 @@ NetReceive(volatile uchar * inpkt, int l
> #endif
> return;
> }
> +
> + /*
> + * Fragmentation support. We need to check the ip_id
> + * and if all fragments were received correctly.
> + */
> + off = (ntohs(ip->ip_off) & IP_OFFS) << IP_OFFS_SHIFT;
> + if ((off != 0) || (ip->ip_off & htons(IP_FLAGS_MFRAG))) {
> + int size, idx, complete;
> + char *start;
> +
> + /* New fragmented packet arrived, clear data. */
> + if (ntohs(ip->ip_id) != ip_id) {
> + ip_id = ntohs(ip->ip_id);
> + memset(NetFragBufUsed, 0, sizeof(NetFragBufUsed));
> + udp_len = udp_src = udp_dst = max_idx = 0;
> + }
> +
> + idx = off / NET_UDP_FRAG_SIZE;
> +
> + /* Packet does not fit into IP/UDP fragmentation buf */
> + if (idx >= NET_FRAG_BUF_USED) {
> + return;
> + }
> +
> + NetFragBufUsed[idx] = 1;
> +
> + /* Copy the UDP hdr with the data for 1st
> + fragment, else copy just payload */
> + if (off == 0) {
> + udp_len = ntohs(ip->udp_len);
> + udp_src = ntohs(ip->udp_src);
> + udp_dst = ntohs(ip->udp_dst);
> + }
> + size = ntohs(ip->ip_len) - IP_HDR_SIZE_NO_UDP;
> + start = (char *)ip + IP_HDR_SIZE_NO_UDP;
> + memcpy(NetFragBuf + off, start, size);
> +
> + /*
> + * When last fragement has been received we
>
s/fragement/fragment/
> + * know the number of fragments we expect. If
> + * all have arrived we process the packet.
> + */
> + if (((off != 0) && !(ip->ip_off & htons(IP_FLAGS_MFRAG))))
> + max_idx = idx;
> +
> + if (max_idx == 0)
> + return;
> +
> + complete = 1;
> + for (idx = 0; idx < max_idx; idx++) {
> + if (NetFragBufUsed[idx] == 0) {
> + complete = 0;
> + break;
> + }
> + }
> + if (!complete)
> + return;
> +#ifdef CONFIG_UDP_CHECKSUM
> + if (ip->udp_xsum != 0) {
> + ulong xsum = ip->ip_p;
> + uint16_t *sumptr;
> +
> + xsum += udp_len;
> + xsum += (ntohl(ip->ip_src) >> 16) & 0xffff;
> + xsum += (ntohl(ip->ip_src) >> 0) & 0xffff;
> + xsum += (ntohl(ip->ip_dst) >> 16) & 0xffff;
> + xsum += (ntohl(ip->ip_dst) >> 0) & 0xffff;
> + sumptr = (ushort *)NetFragBuf;
> +
> + if (udp_checksum(sumptr, udp_len, xsum)) {
> + putc('U');
> + return;
> + }
> + }
> +#endif /* CONFIG_UDP_CHECKSUM */
> + (*packetHandler)(NetFragBuf + 8,
> + udp_dst,
> + udp_src,
> + udp_len - 8);
>
Can some of these arguments share lines?
> + return;
> + }
> +
> /*
> * watch for ICMP host redirects
> *
> @@ -1502,26 +1636,8 @@ NetReceive(volatile uchar * inpkt, int l
> sumlen = ntohs(ip->udp_len);
> sumptr = (ushort *) &(ip->udp_src);
>
> - while (sumlen > 1) {
> - ushort sumdata;
> -
> - sumdata = *sumptr++;
> - xsum += ntohs(sumdata);
> - sumlen -= 2;
> - }
> - if (sumlen > 0) {
> - ushort sumdata;
> -
> - sumdata = *(unsigned char *) sumptr;
> - sumdata = (sumdata << 8) & 0xff00;
> - xsum += sumdata;
> - }
> - while ((xsum >> 16) != 0) {
> - xsum = (xsum & 0x0000ffff) + ((xsum >> 16) & 0x0000ffff);
> - }
> - if ((xsum != 0x00000000) && (xsum != 0x0000ffff)) {
> - printf(" UDP wrong checksum %08lx %08x\n",
> - xsum, ntohs(ip->udp_xsum));
> + if (udp_checksum(sumptr, sumlen, xsum)) {
> + putc('U');
> return;
> }
> }
> --- u-boot.git.orig/net/tftp.c
> +++ u-boot.git/net/tftp.c
> @@ -456,6 +456,7 @@ TftpTimeout (void)
> void
> TftpStart (void)
> {
> + char *s, *err;
> #ifdef CONFIG_TFTP_PORT
> char *ep; /* Environment pointer */
> #endif
> @@ -518,6 +519,27 @@ TftpStart (void)
>
> puts ("Loading: *\b");
>
> + /* Get alternate tftp_block_size */
> + if ((s = getenv("tftp_block_size")) != NULL) {
>
Kind of a long environment variable name. Maybe "tftp_bs"?
> + err = NULL;
> +
> + TftpBlkSizeOption = simple_strtoul(s, &err, 10);
> + if (*err) {
> + printf("ERR: \"tftp_block_size\" is not a number\n");
> + TftpBlkSizeOption = TFTP_BLOCK_SIZE;
> + }
> + /*
> + * Reject values which require extensive handling.
> + * block size of 1428 octets (Ethernet MTU, less
> + * the TFTP, UDP and IP header lengths).
> + */
> + if (TftpBlkSizeOption > TFTP_BLOCK_SIZE_MAX) {
> + printf("ERR: tftp_block_sizes larger than %d not "
> + "supported\n", TFTP_BLOCK_SIZE_MAX);
> + TftpBlkSizeOption = TFTP_BLOCK_SIZE;
> + }
> + }
> +
> NetSetTimeout (TIMEOUT * CFG_HZ, TftpTimeout);
> NetSetHandler (TftpHandler);
>
> --- u-boot.git.orig/net/tftp.h
> +++ u-boot.git/net/tftp.h
> @@ -8,11 +8,21 @@
> #ifndef __TFTP_H__
> #define __TFTP_H__
>
> +#include <net.h>
> +
> /**********************************************************************/
> /*
> * Global functions and variables.
> */
>
> +/*
> + * Maximum TFTP block size bound to max size of fragmented IP/UDP
> + * packets minus TFTP and UDP/IP overhead. TFTP overhead is 2 byte
> + * opcode and 2 byte block-number.
> + */
> +#define TFTP_BLOCK_SIZE_MAX (NET_FRAG_BUF_SIZE - sizeof(IP_t) - 4)
> +
> +
> /* tftp.c */
> extern void TftpStart (void); /* Begin TFTP get */
>
>
>
regards,
Ben
More information about the U-Boot
mailing list