Skip to contentSkip to conversation

ieee802154: 6lowpan: remove full-size calculation workarounds

PATCH In review Alexander Aring 1 participant 9 messages Dec 23, 2014 – Dec 25, 2014

Subsystems (3)
lore
Share & tools
lore

Alexander Aring

author maintainer
#0 lore raw

cover ieee802154: 6lowpan: remove full-size calculation workarounds

Hi,

this patch series based on "ieee802154: 6lowpan: separate receive/transmit functionality".

In this patch series I mainly removes the lowpan_uncompress_size function workarounds. Why this is a workaround solution? So this function parse again the 6LoWPAN header for getting the 6LoWPAN header size. We do parsing twice and this is the first ugly thing there. The second thing is that when we add new header compression we need to touch the lowpan_uncompress_size again for adding next header size calculation. This series removes the workaround so we need to parse/create this only once. It was myself which introduce this workaround to have at first something which works. Since Martin Townsend work which put out the lowpan_give_upper_layer outside of the iphc decompression we can handle it now in that way.

This patch also includes some cleanup patches which I detect while programming this one. In futher the 6LoWPAN parsing should look like mac802154 parsing. mac802154 parsing look like mac80211. To rework the 6LoWPAN parsing will be the next steps.

Cc Jukka Rissanen <[email protected]>

Alexander Aring (7):
6lowpan: allow pull only at lowpan_fetch_skb
ieee802154: 6lowpan: skb_pull with error handling
ieee802154: 6lowpan: use skb_network_header
ieee802154: 6lowpan: remove rx full-size calc workaround
ieee802154: 6lowpan: handle fragmented IPv6
ieee802154: 6lowpan: cleanup frag dispatch cases
ieee802154: 6lowpan: remove tx full-size calc workaround

 include/net/6lowpan.h               | 117 +-----------------------------------
 net/ieee802154/6lowpan/6lowpan_i.h  |   1 +
 net/ieee802154/6lowpan/reassembly.c |  31 ++++++++--
 net/ieee802154/6lowpan/rx.c         |  39 +++++-------
 net/ieee802154/6lowpan/tx.c         |  18 +++---
 5 files changed, 56 insertions(+), 150 deletions(-)
Signature · 1 line
2.2.0
author maintainer
#1 lore raw

PATCH 1/7 6lowpan: allow pull only at lowpan_fetch_skb

This patch adds a check on data pointer. If this pointer is NULL we don't copy data from skb into data. This is useful if we only check if we can run skb_pull and doing it afterwards.

---
 include/net/6lowpan.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
include/net/6lowpan.h +3 -1
index dc03d77..c64d274 100644
@@ -246,7 +246,9 @@ static inline bool lowpan_fetch_skb(struct sk_buff *skb,
 	if (unlikely(!pskb_may_pull(skb, len)))
 		return true;
 
-	skb_copy_from_linear_data(skb, data, len);
+	if (likely(data))
+		skb_copy_from_linear_data(skb, data, len);
+
 	skb_pull(skb, len);
 
 	return false;
Signature · 1 line
2.2.0
author maintainer
#2 lore raw

PATCH 2/7 ieee802154: 6lowpan: skb_pull with error handling

In reply to Alexander Aring

This patch replaces a skb_pull with a lowpan_fetch_skb with has some error handling functionaltiy if the skb data isn't enough to run a skb_pull.

---
 net/ieee802154/6lowpan/rx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
net/ieee802154/6lowpan/rx.c +3 -1
index 4be1d28..22a1db4 100644
@@ -108,7 +108,9 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 	/* check that it's our buffer */
 	if (skb->data[0] == LOWPAN_DISPATCH_IPV6) {
 		/* Pull off the 1-byte of 6lowpan header. */
-		skb_pull(skb, 1);
+		if (lowpan_fetch_skb(skb, NULL, 1))
+			goto drop_skb;
+
 		return lowpan_give_skb_to_devices(skb, NULL);
 	} else {
 		switch (skb->data[0] & 0xe0) {
Signature · 1 line
2.2.0
author maintainer
#3 lore raw

PATCH 3/7 ieee802154: 6lowpan: use skb_network_header

In reply to Alexander Aring

This patch is just a little cleanup to use *skb_network_header(skb) instead of skb->data[0].

---
 net/ieee802154/6lowpan/rx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
net/ieee802154/6lowpan/rx.c +2 -2
index 22a1db4..a7b9d39 100644
@@ -106,14 +106,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 		goto drop_skb;
 
 	/* check that it's our buffer */
-	if (skb->data[0] == LOWPAN_DISPATCH_IPV6) {
+	if (*skb_network_header(skb) == LOWPAN_DISPATCH_IPV6) {
 		/* Pull off the 1-byte of 6lowpan header. */
 		if (lowpan_fetch_skb(skb, NULL, 1))
 			goto drop_skb;
 
 		return lowpan_give_skb_to_devices(skb, NULL);
 	} else {
-		switch (skb->data[0] & 0xe0) {
+		switch (*skb_network_header(skb) & 0xe0) {
 		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
 			ret = iphc_decompress(skb, &hdr);
 			if (ret < 0)
Signature · 1 line
2.2.0
author maintainer
#4 lore raw

PATCH 4/7 ieee802154: 6lowpan: remove rx full-size calc workaround

In reply to Alexander Aring

This patch removes a workaround to call lowpan_uncompress_size while receiving first fragment. The lowpan_uncompress_size function is very statically and should not be used. Instead we do a uncompression on-the-fly while receiving first fragment.

---
 net/ieee802154/6lowpan/6lowpan_i.h  |  1 +
 net/ieee802154/6lowpan/reassembly.c | 25 +++++++++++++++++++++----
 net/ieee802154/6lowpan/rx.c         | 21 ++++++++-------------
 3 files changed, 30 insertions(+), 17 deletions(-)
net/ieee802154/6lowpan/6lowpan_i.h +1 -0
index e50f69d..63a7ad7 100644
@@ -60,6 +60,7 @@ extern struct list_head lowpan_devices;
 int lowpan_frag_rcv(struct sk_buff *skb, const u8 frag_type);
 void lowpan_net_frag_exit(void);
 int lowpan_net_frag_init(void);
+int iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr);
 
 void lowpan_rx_init(void);
 void lowpan_rx_exit(void);
net/ieee802154/6lowpan/reassembly.c +21 -4
index f46e4d1..c76c0cb 100644
@@ -201,12 +201,29 @@ found:
 
 	fq->q.stamp = skb->tstamp;
 	if (frag_type == LOWPAN_DISPATCH_FRAG1) {
-		/* Calculate uncomp. 6lowpan header to estimate full size */
-		fq->q.meat += lowpan_uncompress_size(skb, NULL);
+		int ret;
+		struct ieee802154_hdr hdr;
+
+		ret = ieee802154_hdr_peek_addrs(skb, &hdr);
+		if (ret < 0)
+			goto err;
+
+		/* TODO use CALL_RXH when parsing rework is done. */
+		switch (*skb_network_header(skb) & 0xe0) {
+		case LOWPAN_DISPATCH_IPHC:
+			/* uncompress ipv6 header */
+			ret = iphc_decompress(skb, &hdr);
+			if (ret < 0)
+				goto err;
+			break;
+		default:
+			goto err;
+		}
+
 		fq->q.flags |= INET_FRAG_FIRST_IN;
-	} else {
-		fq->q.meat += skb->len;
 	}
+
+	fq->q.meat += skb->len;
 	add_frag_mem_limit(&fq->q, skb->truesize);
 
 	if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
net/ieee802154/6lowpan/rx.c +8 -13
index a7b9d39..158756f 100644
@@ -47,8 +47,7 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
 	return stat;
 }
 
-static int
-iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
+int iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
 {
 	u8 iphc0, iphc1;
 	struct ieee802154_addr_sa sa, da;
@@ -102,9 +101,6 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (dev->type != ARPHRD_IEEE802154)
 		goto drop_skb;
 
-	if (ieee802154_hdr_peek_addrs(skb, &hdr) < 0)
-		goto drop_skb;
-
 	/* check that it's our buffer */
 	if (*skb_network_header(skb) == LOWPAN_DISPATCH_IPV6) {
 		/* Pull off the 1-byte of 6lowpan header. */
@@ -115,6 +111,9 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 	} else {
 		switch (*skb_network_header(skb) & 0xe0) {
 		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
+			if (ieee802154_hdr_peek_addrs(skb, &hdr) < 0)
+				goto drop_skb;
+
 			ret = iphc_decompress(skb, &hdr);
 			if (ret < 0)
 				goto drop_skb;
@@ -123,10 +122,8 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
 			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
 			if (ret == 1) {
-				ret = iphc_decompress(skb, &hdr);
-				if (ret < 0)
-					goto drop_skb;
-
+				ipv6_hdr(skb)->payload_len = htons(skb->len -
+								   sizeof(struct ipv6hdr));
 				return lowpan_give_skb_to_devices(skb, NULL);
 			} else if (ret == -1) {
 				return NET_RX_DROP;
@@ -136,10 +133,8 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 		case LOWPAN_DISPATCH_FRAGN:	/* next fragments headers */
 			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
 			if (ret == 1) {
-				ret = iphc_decompress(skb, &hdr);
-				if (ret < 0)
-					goto drop_skb;
-
+				ipv6_hdr(skb)->payload_len = htons(skb->len -
+								   sizeof(struct ipv6hdr));
 				return lowpan_give_skb_to_devices(skb, NULL);
 			} else if (ret == -1) {
 				return NET_RX_DROP;
Signature · 1 line
2.2.0
author maintainer
#5 lore raw

PATCH 5/7 ieee802154: 6lowpan: handle fragmented IPv6

In reply to Alexander Aring

This patch adds handling for a fragmented uncompressed 6LoWPAN packet. In case that the IPHC compression is above MTU 127, we should not run IPHC compression. This is still a bug in transmit functionality that we don't support that, but also not a bug because in worst-case we didn't hit the case that IPHC compression doesn't fit into a single 802.15.4 frame. Nevertheless this patch adds this handling for receive functionality only, so other 6LoWPAN stacks can send uncompressed fragmented 6LoWPAN packets.

---
 net/ieee802154/6lowpan/reassembly.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)
net/ieee802154/6lowpan/reassembly.c +14 -8
index c76c0cb..d47b34e 100644
@@ -209,15 +209,21 @@ found:
 			goto err;
 
 		/* TODO use CALL_RXH when parsing rework is done. */
-		switch (*skb_network_header(skb) & 0xe0) {
-		case LOWPAN_DISPATCH_IPHC:
-			/* uncompress ipv6 header */
-			ret = iphc_decompress(skb, &hdr);
-			if (ret < 0)
+		if (*skb_network_header(skb) == LOWPAN_DISPATCH_IPV6) {
+			/* Pull off the 1-byte of 6lowpan header. */
+			if (lowpan_fetch_skb(skb, NULL, 1))
 				goto err;
-			break;
-		default:
-			goto err;
+		} else {
+			switch (*skb_network_header(skb) & 0xe0) {
+			case LOWPAN_DISPATCH_IPHC:
+				/* uncompress ipv6 header */
+				ret = iphc_decompress(skb, &hdr);
+				if (ret < 0)
+					goto err;
+				break;
+			default:
+				goto err;
+			}
 		}
 
 		fq->q.flags |= INET_FRAG_FIRST_IN;
Signature · 1 line
2.2.0
author maintainer
#6 lore raw

PATCH 6/7 ieee802154: 6lowpan: cleanup frag dispatch cases

In reply to Alexander Aring

This patch is a cleanup because case for LOWPAN_DISPATCH_FRAG1 and LOWPAN_DISPATCH_FRAGN looks almost the same except the dispatch value which is stored in a variables before now.

---
 net/ieee802154/6lowpan/rx.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
net/ieee802154/6lowpan/rx.c +4 -12
index 158756f..fc7228c 100644
@@ -109,7 +109,9 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 
 		return lowpan_give_skb_to_devices(skb, NULL);
 	} else {
-		switch (*skb_network_header(skb) & 0xe0) {
+		u8 dispatch = *skb_network_header(skb) & 0xe0;
+
+		switch (dispatch) {
 		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
 			if (ieee802154_hdr_peek_addrs(skb, &hdr) < 0)
 				goto drop_skb;
@@ -120,18 +122,8 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 
 			return lowpan_give_skb_to_devices(skb, NULL);
 		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
-			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
-			if (ret == 1) {
-				ipv6_hdr(skb)->payload_len = htons(skb->len -
-								   sizeof(struct ipv6hdr));
-				return lowpan_give_skb_to_devices(skb, NULL);
-			} else if (ret == -1) {
-				return NET_RX_DROP;
-			} else {
-				return NET_RX_SUCCESS;
-			}
 		case LOWPAN_DISPATCH_FRAGN:	/* next fragments headers */
-			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
+			ret = lowpan_frag_rcv(skb, dispatch);
 			if (ret == 1) {
 				ipv6_hdr(skb)->payload_len = htons(skb->len -
 								   sizeof(struct ipv6hdr));
Signature · 1 line
2.2.0
author maintainer
#7 lore raw

PATCH 7/7 ieee802154: 6lowpan: remove tx full-size calc workaround

In reply to Alexander Aring

This patch removes a workaround in transmit fragmentation implementation which use lowpan_uncompress_size to calculate the full size of fragmented packet. Instead we calculate the size over skb->len before and after iphc compression. Additional we remove the whole lowpan_uncompress_size function which is used by 802154 6LoWPAN only.

---
 include/net/6lowpan.h       | 113 --------------------------------------------
 net/ieee802154/6lowpan/tx.c |  18 ++++---
 2 files changed, 11 insertions(+), 120 deletions(-)
include/net/6lowpan.h +0 -113
index c64d274..fd56994 100644
@@ -261,119 +261,6 @@ static inline void lowpan_push_hc_data(u8 **hc_ptr, const void *data,
 	*hc_ptr += len;
 }
 
-static inline u8 lowpan_addr_mode_size(const u8 addr_mode)
-{
-	static const u8 addr_sizes[] = {
-		[LOWPAN_IPHC_ADDR_00] = 16,
-		[LOWPAN_IPHC_ADDR_01] = 8,
-		[LOWPAN_IPHC_ADDR_02] = 2,
-		[LOWPAN_IPHC_ADDR_03] = 0,
-	};
-	return addr_sizes[addr_mode];
-}
-
-static inline u8 lowpan_next_hdr_size(const u8 h_enc, u16 *uncomp_header)
-{
-	u8 ret = 1;
-
-	if ((h_enc & LOWPAN_NHC_UDP_MASK) == LOWPAN_NHC_UDP_ID) {
-		*uncomp_header += sizeof(struct udphdr);
-
-		switch (h_enc & LOWPAN_NHC_UDP_CS_P_11) {
-		case LOWPAN_NHC_UDP_CS_P_00:
-			ret += 4;
-			break;
-		case LOWPAN_NHC_UDP_CS_P_01:
-		case LOWPAN_NHC_UDP_CS_P_10:
-			ret += 3;
-			break;
-		case LOWPAN_NHC_UDP_CS_P_11:
-			ret++;
-			break;
-		default:
-			break;
-		}
-
-		if (!(h_enc & LOWPAN_NHC_UDP_CS_C))
-			ret += 2;
-	}
-
-	return ret;
-}
-
-/**
- *	lowpan_uncompress_size - returns skb->len size with uncompressed header
- *	@skb: sk_buff with 6lowpan header inside
- *	@datagram_offset: optional to get the datagram_offset value
- *
- *	Returns the skb->len with uncompressed header
- */
-static inline u16
-lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
-{
-	u16 ret = 2, uncomp_header = sizeof(struct ipv6hdr);
-	u8 iphc0, iphc1, h_enc;
-
-	iphc0 = skb_network_header(skb)[0];
-	iphc1 = skb_network_header(skb)[1];
-
-	switch ((iphc0 & LOWPAN_IPHC_TF) >> 3) {
-	case 0:
-		ret += 4;
-		break;
-	case 1:
-		ret += 3;
-		break;
-	case 2:
-		ret++;
-		break;
-	default:
-		break;
-	}
-
-	if (!(iphc0 & LOWPAN_IPHC_NH_C))
-		ret++;
-
-	if (!(iphc0 & 0x03))
-		ret++;
-
-	ret += lowpan_addr_mode_size((iphc1 & LOWPAN_IPHC_SAM) >>
-				     LOWPAN_IPHC_SAM_BIT);
-
-	if (iphc1 & LOWPAN_IPHC_M) {
-		switch ((iphc1 & LOWPAN_IPHC_DAM_11) >>
-			LOWPAN_IPHC_DAM_BIT) {
-		case LOWPAN_IPHC_DAM_00:
-			ret += 16;
-			break;
-		case LOWPAN_IPHC_DAM_01:
-			ret += 6;
-			break;
-		case LOWPAN_IPHC_DAM_10:
-			ret += 4;
-			break;
-		case LOWPAN_IPHC_DAM_11:
-			ret++;
-			break;
-		default:
-			break;
-		}
-	} else {
-		ret += lowpan_addr_mode_size((iphc1 & LOWPAN_IPHC_DAM_11) >>
-					     LOWPAN_IPHC_DAM_BIT);
-	}
-
-	if (iphc0 & LOWPAN_IPHC_NH_C) {
-		h_enc = skb_network_header(skb)[ret];
-		ret += lowpan_next_hdr_size(h_enc, &uncomp_header);
-	}
-
-	if (dgram_offset)
-		*dgram_offset = uncomp_header;
-
-	return skb->len + uncomp_header - ret;
-}
-
 int
 lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
 			 const u8 *saddr, const u8 saddr_type,
net/ieee802154/6lowpan/tx.c +11 -7
index 2349070..102d0ec 100644
@@ -124,16 +124,14 @@ lowpan_xmit_fragment(struct sk_buff *skb, const struct ieee802154_hdr *wpan_hdr,
 
 static int
 lowpan_xmit_fragmented(struct sk_buff *skb, struct net_device *dev,
-		       const struct ieee802154_hdr *wpan_hdr)
+		       const struct ieee802154_hdr *wpan_hdr, u16 dgram_size,
+		       u16 dgram_offset)
 {
-	u16 dgram_size, dgram_offset;
 	__be16 frag_tag;
 	u8 frag_hdr[5];
 	int frag_cap, frag_len, payload_cap, rc;
 	int skb_unprocessed, skb_offset;
 
-	dgram_size = lowpan_uncompress_size(skb, &dgram_offset) -
-		     skb->mac_len;
 	frag_tag = htons(lowpan_dev_info(dev)->fragment_tag);
 	lowpan_dev_info(dev)->fragment_tag++;
 
@@ -188,7 +186,8 @@ err:
 	return rc;
 }
 
-static int lowpan_header(struct sk_buff *skb, struct net_device *dev)
+static int lowpan_header(struct sk_buff *skb, struct net_device *dev,
+			 u16 *dgram_size, u16 *dgram_offset)
 {
 	struct ieee802154_addr sa, da;
 	struct ieee802154_mac_cb *cb = mac_cb_init(skb);
@@ -201,7 +200,10 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *dev)
 	daddr = &info.daddr.u.extended_addr;
 	saddr = &info.saddr.u.extended_addr;
 
+	*dgram_size = skb->len;
 	lowpan_header_compress(skb, dev, ETH_P_IPV6, daddr, saddr, skb->len);
+	/* dgram_size = (saved bytes after compression) + lowpan header size */
+	*dgram_offset = (*dgram_size - skb->len) + skb_network_header_len(skb);
 
 	cb->type = IEEE802154_FC_TYPE_DATA;
 
@@ -234,6 +236,7 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct ieee802154_hdr wpan_hdr;
 	int max_single, ret;
+	u16 dgram_size, dgram_offset;
 
 	pr_debug("package xmit\n");
 
@@ -244,7 +247,7 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (!skb)
 		return NET_XMIT_DROP;
 
-	ret = lowpan_header(skb, dev);
+	ret = lowpan_header(skb, dev, &dgram_size, &dgram_offset);
 	if (ret < 0) {
 		kfree_skb(skb);
 		return NET_XMIT_DROP;
@@ -264,7 +267,8 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 		netdev_tx_t rc;
 
 		pr_debug("frame is too big, fragmentation is needed\n");
-		rc = lowpan_xmit_fragmented(skb, dev, &wpan_hdr);
+		rc = lowpan_xmit_fragmented(skb, dev, &wpan_hdr, dgram_size,
+					    dgram_offset);
 
 		return rc < 0 ? NET_XMIT_DROP : rc;
 	}
Signature · 1 line
2.2.0
author maintainer
#8 lore raw
In reply to Alexander Aring

Marcel,

please drop this series. I reconsider my change and detect that IPHC uncompression use "uh->len = htons(skb->len + sizeof(struct udphdr))" for udp next header compression.

This patch introduce now a iphc uncompression after first fragment and skb->len isn't the full payload there. Need to look how we can deal with that now. I really like to remove these workarounds to run a 6LoWPAN header parse twice.

When the first fragment arrived we know the full payload but we can't set skb->len with this value.

Need to look for another solution for this which can be used by btle and 802154 6LoWPAN.

- Alex