[U-Boot] [PATCH 0/2] net: Additional IP fragmentation check

[U-Boot] [PATCH 0/2] net: Additional IP fragmentation check

[Peter Tyser]

It looks like U-Boot ignores fragmented IP packets with
non-zero "fragment offset" fields, but doesn't ignore the
initial fragmented IP packet which has a "fragment offset"
field value of 0.

An additional check was added to catch the initial
fragmented packet which should have the "more fragments"
bit set in its flags field.

The bug initially resulted in TFTP transfers which
appeared to work, but in reality failed as some
fragmented packets were received, others were not.
With these patches applied a TFTP download from a
server with a low MTU results in a timeout as U-Boot
drops all incominig fragmented TFTP data packets.


I quickly tried the "Add simple IP/UDP fragmentation
support" patch in the net/testing repository, but
it did not work out of the box.  It looks like U-Boot
was assembling the fragmented packets correctly based on
a memory display of &NetFragBuf, but the assembled
packet was not making its way up the stack FWIW.

Peter Tyser (2):
  net: Define IP flag field values
  net: Add additional IP fragmentation check

 include/net.h |    6 ++++++
 net/net.c     |    8 +++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

[U-Boot] [PATCH 1/2] net: Define IP flag field values

[Peter Tyser]

These defines were pulled from the "Add simple
IP/UDP fragmentation support" patch from Frank
Haverkamp <haver@vnet.ibm.com>.
---
 include/net.h |    6 ++++++
 net/net.c     |    6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/net.h b/include/net.h
index a5a256b..199872e 100644
--- a/include/net.h
+++ b/include/net.h
@@ -200,6 +200,12 @@ typedef struct {
 	ushort		udp_xsum;	/* Checksum			*/
 } IP_t;
 
+#define IP_OFFS		0x1fff /* ip offset *= 8 */
+#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))
 
diff --git a/net/net.c b/net/net.c
index 77e83b5..cf1f4fa 100644
--- a/net/net.c
+++ b/net/net.c
@@ -735,7 +735,7 @@ int PingSend(void)
 	ip->ip_tos   = 0;
 	ip->ip_len   = htons(IP_HDR_SIZE_NO_UDP + 8);
 	ip->ip_id    = htons(NetIPID++);
-	ip->ip_off   = htons(0x4000);	/* No fragmentation */
+	ip->ip_off   = htons(IP_FLAGS_DFRAG);	/* Don't fragment */
 	ip->ip_ttl   = 255;
 	ip->ip_p     = 0x01;		/* ICMP */
 	ip->ip_sum   = 0;
@@ -1399,7 +1399,7 @@ NetReceive(volatile uchar * inpkt, int len)
 		if ((ip->ip_hl_v & 0xf0) != 0x40) {
 			return;
 		}
-		if (ip->ip_off & htons(0x1fff)) { /* Can't deal w/ fragments */
+		if (ip->ip_off & htons(IP_OFFS)) { /* Can't deal w/ fragments */
 			return;
 		}
 		/* can't deal with headers > 20 bytes */
@@ -1698,7 +1698,7 @@ NetSetIP(volatile uchar * xip, IPaddr_t dest, int dport, int sport, int len)
 	ip->ip_tos   = 0;
 	ip->ip_len   = htons(IP_HDR_SIZE + len);
 	ip->ip_id    = htons(NetIPID++);
-	ip->ip_off   = htons(0x4000);	/* No fragmentation */
+	ip->ip_off   = htons(IP_FLAGS_DFRAG);	/* Don't fragment */
 	ip->ip_ttl   = 255;
 	ip->ip_p     = 17;		/* UDP */
 	ip->ip_sum   = 0;
-- 
1.6.0.2.GIT

[U-Boot] [PATCH 2/2] net: Add additional IP fragmentation check

[Peter Tyser]

Ignore IP packets which have the "more fragments" flag bit
set.  This flag indicates the IP packet is fragmented and
must be ignored by U-Boot.
---
 net/net.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/net.c b/net/net.c
index cf1f4fa..b8e09e8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1399,7 +1399,8 @@ NetReceive(volatile uchar * inpkt, int len)
 		if ((ip->ip_hl_v & 0xf0) != 0x40) {
 			return;
 		}
-		if (ip->ip_off & htons(IP_OFFS)) { /* Can't deal w/ fragments */
+		/* Can't deal with fragments */
+		if (ip->ip_off & htons(IP_OFFS | IP_FLAGS_MFRAG)) {
 			return;
 		}
 		/* can't deal with headers > 20 bytes */
-- 
1.6.0.2.GIT

[U-Boot] [PATCH v2 0/2] net: Additional IP fragmentation check

[Ben Warren]

Hi Peter,

Peter Tyser wrote:
> v2: Forgot SOBs...
>
> It looks like U-Boot ignores fragmented IP packets with
> non-zero "fragment offset" fields, but doesn't ignore the
> initial fragmented IP packet which has a "fragment offset"
> field value of 0.
>
> An additional check was added to catch the initial
> fragmented packet which should have the "more fragments"
> bit set in its flags field.
>
> The bug initially resulted in TFTP transfers which
> appeared to work, but in reality failed as some
> fragmented packets were received, others were not.
> With these patches applied a TFTP download from a
> server with a low MTU results in a timeout as U-Boot
> drops all incominig fragmented TFTP data packets.
>
>
> I quickly tried the "Add simple IP/UDP fragmentation
> support" patch in the net/testing repository, but
> it did not work out of the box.  It looks like U-Boot
> was assembling the fragmented packets correctly based on
> a memory display of &NetFragBuf, but the assembled
> packet was not making its way up the stack FWIW.
>
> Peter Tyser (2):
>   net: Define IP flag field values
>   net: Add additional IP fragmentation check
>
>  include/net.h |    6 ++++++
>  net/net.c     |    8 +++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
>   
Patches 1 and 2 applied to net repo.

thanks,
Ben