[PATCH net-next 00/15] 6lowpan: Some more bug fixes

[PATCH net-next 00/15] 6lowpan: Some more bug fixes

[Tony Cheneau]

Hello,

This patch serie fixes a few bugs within the 6lowpan modules and prepares the
integration of the serial driver that will be submitted in a subsequent patch serie.
It should apply cleanly on the latest net-next.

Regards,
	Tony Cheneau

Tony Cheneau (15):
  6lowpan: lowpan_is_iid_16_bit_compressable() does not detect
    compressable address correctly
  6lowpan: next header is not properly set upon decompression of a UDP
    header.
  6lowpan: always enable link-layer acknowledgments
  mac802154: turn on ACK when enabled by the upper layers
  6lowpan: use short IEEE 802.15.4 addresses for broadcast destination
  6lowpan: fix first fragment (FRAG1) handling
  6lowpan: store fragment tag values per device instead of net stack
    wide
  6lowpan: obtain IEEE802.15.4 sequence number from the MAC layer
  6lowpan: add a new parameter in sysfs to turn on/off ACK request at
    MAC layer
  6lowpan: use the PANID provided by the device instead of a static
    value
  6lowpan: modify udp compression/uncompression to match the standard
  6lowpan: make memory allocation atomic during 6lowpan header creation
  mac802154: make mem alloc ATOMIC to prevent "scheduling while atomic"
    crashes
  mac802154: remove unnecessary spinlocks
  mac802154: re-introduce MAC primitives required to send/receive
    packets

 net/ieee802154/6lowpan.c  |  144 ++++++++++++++++++++++++++++++++++++---------
 net/ieee802154/6lowpan.h  |   14 +++--
 net/mac802154/mac802154.h |    2 +
 net/mac802154/mac_cmd.c   |   22 +++++++-
 net/mac802154/mib.c       |   33 +++++++----
 net/mac802154/wpan.c      |    4 +-
 6 files changed, 172 insertions(+), 47 deletions(-)

-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 01/15] 6lowpan: lowpan_is_iid_16_bit_compressable() does not detect compressable address correctly

[Tony Cheneau]

The current test is not RFC6282 compliant. The same issue has been found
out and fixed in Contiki. This patch is basicaly a port of their fix.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.h |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/ieee802154/6lowpan.h b/net/ieee802154/6lowpan.h
index 8c2251f..efd1a57 100644
--- a/net/ieee802154/6lowpan.h
+++ b/net/ieee802154/6lowpan.h
@@ -87,14 +87,16 @@
 #define is_addr_link_local(a) (((a)->s6_addr16[0]) == 0x80FE)
 
 /*
- * check whether we can compress the IID to 16 bits,
- * it's possible for unicast adresses with first 49 bits are zero only.
- */
+* check whether we can compress the IID to 16 bits,
+* it's possible for unicast adresses with first 49 bits are zero only.
+*/
 #define lowpan_is_iid_16_bit_compressable(a)	\
 	((((a)->s6_addr16[4]) == 0) &&		\
-	 (((a)->s6_addr16[5]) == 0) &&		\
-	 (((a)->s6_addr16[6]) == 0) &&		\
-	 ((((a)->s6_addr[14]) & 0x80) == 0))
+	 (((a)->s6_addr[10]) == 0) &&		\
+	 (((a)->s6_addr[11]) == 0xff) &&	\
+	 (((a)->s6_addr[12]) == 0xfe) &&	\
+	 (((a)->s6_addr[13]) == 0))
+
 
 /* multicast address */
 #define is_addr_mcast(a) (((a)->s6_addr[0]) == 0xFF)
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

Re: [PATCH net-next 01/15] 6lowpan: lowpan_is_iid_16_bit_compressable() does not detect compressable address correctly

[David Miller]

From: Tony Cheneau <tony.cheneau@amnesiak.org>
Date: Tue, 23 Oct 2012 00:09:43 -0400

> The current test is not RFC6282 compliant. The same issue has been found
> out and fixed in Contiki. This patch is basicaly a port of their fix.
> 
> Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org>

It's really demoralizing to sit down to read a large 15 entry
patch series and this is the first thing I see:

>  /*
> - * check whether we can compress the IID to 16 bits,
> - * it's possible for unicast adresses with first 49 bits are zero only.
> - */
> +* check whether we can compress the IID to 16 bits,
> +* it's possible for unicast adresses with first 49 bits are zero only.
> +*/

Don't break the comment indentation, it was perfectly fine.

I'm not reviewing the rest of the series, you'll need to repost
the entire thing after you fix this.

And if you're smart you'll make sure there aren't similar coding
style issues in the rest of your patches, else it's going to take
a long time to merge this crap into the tree.

Re: [PATCH net-next 01/15] 6lowpan: lowpan_is_iid_16_bit_compressable() does not detect compressable address correctly

[Tony Cheneau]

Hello David,

Le 23.10.2012 08:49, David Miller a écrit :
[...]
>
> It's really demoralizing to sit down to read a large 15 entry
> patch series and this is the first thing I see:
>
>>  /*
>> - * check whether we can compress the IID to 16 bits,
>> - * it's possible for unicast adresses with first 49 bits are zero 
>> only.
>> - */
>> +* check whether we can compress the IID to 16 bits,
>> +* it's possible for unicast adresses with first 49 bits are zero 
>> only.
>> +*/
>
> Don't break the comment indentation, it was perfectly fine.
>
> I'm not reviewing the rest of the series, you'll need to repost
> the entire thing after you fix this.
>
> And if you're smart you'll make sure there aren't similar coding
> style issues in the rest of your patches, else it's going to take
> a long time to merge this crap into the tree.

Sorry about that, I failed to catch that mistake of mine (and several 
other ones that Jan spotted). I'll rework my patchset and resubmit. 
Thank you for pointing out this issue.

Regards,
Tony

Re: [PATCH net-next 01/15] 6lowpan: lowpan_is_iid_16_bit_compressable() does not detect compressable address correctly

[Jan Ceuleers]

On 10/23/2012 06:09 AM, Tony Cheneau wrote:

>  #define lowpan_is_iid_16_bit_compressable(a)	\
>  	((((a)->s6_addr16[4]) == 0) &&		\
> -	 (((a)->s6_addr16[5]) == 0) &&		\
> -	 (((a)->s6_addr16[6]) == 0) &&		\
> -	 ((((a)->s6_addr[14]) & 0x80) == 0))
> +	 (((a)->s6_addr[10]) == 0) &&		\
> +	 (((a)->s6_addr[11]) == 0xff) &&	\
> +	 (((a)->s6_addr[12]) == 0xfe) &&	\
> +	 (((a)->s6_addr[13]) == 0))
> +
>  
>  /* multicast address */
>  #define is_addr_mcast(a) (((a)->s6_addr[0]) == 0xFF)
> 

Is the additional newline needed at the end of the #define?

[PATCH net-next 02/15] 6lowpan: next header is not properly set upon decompression of a UDP header.

[Tony Cheneau]

This causes a drop of the UDP packet.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 6d42c17..b53a71a4 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -913,9 +913,12 @@ lowpan_process_data(struct sk_buff *skb)
 	}
 
 	/* UDP data uncompression */
-	if (iphc0 & LOWPAN_IPHC_NH_C)
+	if (iphc0 & LOWPAN_IPHC_NH_C) {
 		if (lowpan_uncompress_udp_header(skb))
 			goto drop;
+		hdr.nexthdr = UIP_PROTO_UDP;
+	}
+
 
 	/* Not fragmented package */
 	hdr.payload_len = htons(skb->len);
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

Re: [PATCH net-next 02/15] 6lowpan: next header is not properly set upon decompression of a UDP header.

[Jan Ceuleers]

On 10/23/2012 06:09 AM, Tony Cheneau wrote:
> This causes a drop of the UDP packet.
> 
> Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org>
> ---
>  net/ieee802154/6lowpan.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> index 6d42c17..b53a71a4 100644
> --- a/net/ieee802154/6lowpan.c
> +++ b/net/ieee802154/6lowpan.c
> @@ -913,9 +913,12 @@ lowpan_process_data(struct sk_buff *skb)
>  	}
>  
>  	/* UDP data uncompression */
> -	if (iphc0 & LOWPAN_IPHC_NH_C)
> +	if (iphc0 & LOWPAN_IPHC_NH_C) {
>  		if (lowpan_uncompress_udp_header(skb))
>  			goto drop;
> +		hdr.nexthdr = UIP_PROTO_UDP;
> +	}
> +

Superfluous newline.

>  
>  	/* Not fragmented package */
>  	hdr.payload_len = htons(skb->len);
> 

[PATCH net-next 03/15] 6lowpan: always enable link-layer acknowledgments

[Tony Cheneau]

This feature is especially important when using fragmentation, because
the reassembly mechanism can not recover from the loss of a fragment.

Note that some hardware ignore this flag and not will not transmit any
acknowledgments if this is set.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index b53a71a4..49d91df 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -589,6 +589,10 @@ static int lowpan_header_create(struct sk_buff *skb,
 
 		mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
 
+		/* request acknowledgment when possible */
+		if (!lowpan_is_addr_broadcast(daddr))
+			mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
+
 		return dev_hard_header(skb, lowpan_dev_info(dev)->real_dev,
 				type, (void *)&da, (void *)&sa, skb->len);
 	}
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 04/15] mac802154: turn on ACK when enabled by the upper layers

[Tony Cheneau]

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/mac802154/wpan.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index f30f6d4..d6aea7f 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -149,6 +149,8 @@ static int mac802154_header_create(struct sk_buff *skb,
 
 	head[pos++] = mac_cb(skb)->seq; /* DSN/BSN */
 	fc = mac_cb_type(skb);
+	if (mac_cb_is_ackreq(skb))
+		fc |= IEEE802154_FC_ACK_REQ;
 
 	if (!saddr) {
 		spin_lock_bh(&priv->mib_lock);
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 05/15] 6lowpan: use short IEEE 802.15.4 addresses for broadcast destination

[Tony Cheneau]

It is intended that the IEEE 802.15.4 standard uses the 0xFFFF short address (2
bytes) for message broadcasting.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 49d91df..8a2ee95 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -577,21 +577,26 @@ static int lowpan_header_create(struct sk_buff *skb,
 	 * this isn't implemented in mainline yet, so currently we assign 0xff
 	 */
 	{
+		mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
+
 		/* prepare wpan address data */
 		sa.addr_type = IEEE802154_ADDR_LONG;
 		sa.pan_id = 0xff;
-
-		da.addr_type = IEEE802154_ADDR_LONG;
-		da.pan_id = 0xff;
-
-		memcpy(&(da.hwaddr), daddr, 8);
 		memcpy(&(sa.hwaddr), saddr, 8);
 
-		mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
+		da.pan_id = 0xff;
+		/* if the destination address is the broadcast address,
+		   use short address */
+		if (lowpan_is_addr_broadcast(daddr)) {
+			da.addr_type = IEEE802154_ADDR_SHORT;
+			da.short_addr = IEEE802154_ADDR_BROADCAST;
+		} else {
+			da.addr_type = IEEE802154_ADDR_LONG;
+			memcpy(&(da.hwaddr), daddr, 8);
 
-		/* request acknowledgment when possible */
-		if (!lowpan_is_addr_broadcast(daddr))
+			/* request acknowledgment */
 			mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
+		}
 
 		return dev_hard_header(skb, lowpan_dev_info(dev)->real_dev,
 				type, (void *)&da, (void *)&sa, skb->len);
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

Re: [PATCH net-next 05/15] 6lowpan: use short IEEE 802.15.4 addresses for broadcast destination

[Jan Ceuleers]

On 10/23/2012 06:09 AM, Tony Cheneau wrote:
> It is intended that the IEEE 802.15.4 standard uses the 0xFFFF short address (2
> bytes) for message broadcasting.
> 
> Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org>
> ---
>  net/ieee802154/6lowpan.c |   21 +++++++++++++--------
>  1 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> index 49d91df..8a2ee95 100644
> --- a/net/ieee802154/6lowpan.c
> +++ b/net/ieee802154/6lowpan.c
> @@ -577,21 +577,26 @@ static int lowpan_header_create(struct sk_buff *skb,
>  	 * this isn't implemented in mainline yet, so currently we assign 0xff
>  	 */
>  	{
> +		mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
> +
>  		/* prepare wpan address data */
>  		sa.addr_type = IEEE802154_ADDR_LONG;
>  		sa.pan_id = 0xff;
> -
> -		da.addr_type = IEEE802154_ADDR_LONG;
> -		da.pan_id = 0xff;
> -
> -		memcpy(&(da.hwaddr), daddr, 8);
>  		memcpy(&(sa.hwaddr), saddr, 8);
>  
> -		mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
> +		da.pan_id = 0xff;
> +		/* if the destination address is the broadcast address,
> +		   use short address */

David likes comments in the networking code to look

		/* Like
		 * this
		 */

Note that the comment is closed on a line by itself. There's an example
at the top of this hunk.

[PATCH net-next 06/15] 6lowpan: fix first fragment (FRAG1) handling

[Tony Cheneau]

The first fragment, FRAG1, must contain some payload according to the
specs. However, as it is currently written, the first fragment will
remain empty and only contain the 6lowpan headers.

This patch also extract the transport layer information from the first
fragment. This information is later on use when uncompressing UDP
header.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |   54 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 8a2ee95..38cecaf 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -654,7 +654,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
 }
 
 static struct lowpan_fragment *
-lowpan_alloc_new_frame(struct sk_buff *skb, u8 len, u16 tag)
+lowpan_alloc_new_frame(struct sk_buff *skb, u16 len, u16 tag)
 {
 	struct lowpan_fragment *frame;
 
@@ -735,6 +735,18 @@ lowpan_process_data(struct sk_buff *skb)
 		/* adds the 3 MSB to the 8 LSB to retrieve the 11 bits length */
 		len = ((iphc0 & 7) << 8) | slen;
 
+		if ((iphc0 & LOWPAN_DISPATCH_MASK) == LOWPAN_DISPATCH_FRAG1) {
+			pr_debug("%s received a FRAG1 packet (tag: %d, "
+				 "size of the entire IP packet: %d)"
+				 , __func__, tag, len);
+		} else { /* FRAGN */
+			if (lowpan_fetch_skb_u8(skb, &offset))
+				goto unlock_and_drop;
+			pr_debug("%s received a FRAGN packet (tag: %d, "
+				 "size of the entire IP packet: %d, "
+				 "offset: %d)", __func__, tag, len, offset * 8);
+		}
+
 		/*
 		 * check if frame assembling with the same tag is
 		 * already in progress
@@ -749,17 +761,13 @@ lowpan_process_data(struct sk_buff *skb)
 
 		/* alloc new frame structure */
 		if (!found) {
+			pr_debug("%s first fragment received for tag %d, "
+				 "begin packet reassembly", __func__, tag);
 			frame = lowpan_alloc_new_frame(skb, len, tag);
 			if (!frame)
 				goto unlock_and_drop;
 		}
 
-		if ((iphc0 & LOWPAN_DISPATCH_MASK) == LOWPAN_DISPATCH_FRAG1)
-			goto unlock_and_drop;
-
-		if (lowpan_fetch_skb_u8(skb, &offset)) /* fetch offset */
-			goto unlock_and_drop;
-
 		/* if payload fits buffer, copy it */
 		if (likely((offset * 8 + skb->len) <= frame->length))
 			skb_copy_to_linear_data_offset(frame->skb, offset * 8,
@@ -777,6 +785,10 @@ lowpan_process_data(struct sk_buff *skb)
 			list_del(&frame->list);
 			spin_unlock_bh(&flist_lock);
 
+			pr_debug("%s successfully reassembled fragment "
+				 "(tag %d)", __func__, tag);
+
+
 			dev_kfree_skb(skb);
 			skb = frame->skb;
 			kfree(frame);
@@ -976,13 +988,13 @@ static int lowpan_get_mac_header_length(struct sk_buff *skb)
 
 static int
 lowpan_fragment_xmit(struct sk_buff *skb, u8 *head,
-			int mlen, int plen, int offset)
+			int mlen, int plen, int offset, int type)
 {
 	struct sk_buff *frag;
 	int hlen, ret;
 
-	/* if payload length is zero, therefore it's a first fragment */
-	hlen = (plen == 0 ? LOWPAN_FRAG1_HEAD_SIZE :  LOWPAN_FRAGN_HEAD_SIZE);
+	hlen = (type == LOWPAN_DISPATCH_FRAG1 ? LOWPAN_FRAG1_HEAD_SIZE :
+			LOWPAN_FRAGN_HEAD_SIZE);
 
 	lowpan_raw_dump_inline(__func__, "6lowpan fragment header", head, hlen);
 
@@ -1025,7 +1037,18 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
 	head[2] = tag >> 8;
 	head[3] = tag & 0xff;
 
-	err = lowpan_fragment_xmit(skb, head, header_length, 0, 0);
+	err = lowpan_fragment_xmit(skb, head, header_length, LOWPAN_FRAG_SIZE,
+				   0, LOWPAN_DISPATCH_FRAG1);
+
+	if (err) {
+#if DEBUG
+		pr_debug("%s unable to send FRAG1 packet (tag: %d)",
+			 __func__, tag);
+#endif /* DEBUG */
+		goto exit;
+	}
+
+	offset = LOWPAN_FRAG_SIZE;
 
 	/* next fragment header */
 	head[0] &= ~LOWPAN_DISPATCH_FRAG1;
@@ -1040,10 +1063,17 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
 			len = payload_length - offset;
 
 		err = lowpan_fragment_xmit(skb, head, header_length,
-							len, offset);
+					   len, offset, LOWPAN_DISPATCH_FRAGN);
+		if (err) {
+			pr_debug("%s unable to send a subsequent FRAGN packet "
+				 "(tag: %d, offset: %d", __func__, tag, offset);
+			goto exit;
+		}
+
 		offset += len;
 	}
 
+exit:
 	return err;
 }
 
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

Re: [PATCH net-next 06/15] 6lowpan: fix first fragment (FRAG1) handling

[Jan Ceuleers]

On 10/23/2012 06:09 AM, Tony Cheneau wrote:
> The first fragment, FRAG1, must contain some payload according to the
> specs. However, as it is currently written, the first fragment will
> remain empty and only contain the 6lowpan headers.
> 
> This patch also extract the transport layer information from the first
> fragment. This information is later on use when uncompressing UDP
> header.
> 
> Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org>
> ---
>  net/ieee802154/6lowpan.c |   54 +++++++++++++++++++++++++++++++++++----------
>  1 files changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> index 8a2ee95..38cecaf 100644
> --- a/net/ieee802154/6lowpan.c
> +++ b/net/ieee802154/6lowpan.c
> @@ -654,7 +654,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
>  }
>  
>  static struct lowpan_fragment *
> -lowpan_alloc_new_frame(struct sk_buff *skb, u8 len, u16 tag)
> +lowpan_alloc_new_frame(struct sk_buff *skb, u16 len, u16 tag)
>  {
>  	struct lowpan_fragment *frame;
>  
> @@ -735,6 +735,18 @@ lowpan_process_data(struct sk_buff *skb)
>  		/* adds the 3 MSB to the 8 LSB to retrieve the 11 bits length */
>  		len = ((iphc0 & 7) << 8) | slen;
>  
> +		if ((iphc0 & LOWPAN_DISPATCH_MASK) == LOWPAN_DISPATCH_FRAG1) {
> +			pr_debug("%s received a FRAG1 packet (tag: %d, "
> +				 "size of the entire IP packet: %d)"
> +				 , __func__, tag, len);

There are several schools of thought on the relative importance of
observing the 80-character line limit versus breaking up string
constants (in an attempt to maintain grepability). I think the above is
fine but others (whose opinion matters more than mine) may or may not
agree. Whatever you decide here, please apply consistently throughout.

However, the comma ahead of the __func__ should be at the end of the
previous line.

(...)

> -	/* if payload length is zero, therefore it's a first fragment */
> -	hlen = (plen == 0 ? LOWPAN_FRAG1_HEAD_SIZE :  LOWPAN_FRAGN_HEAD_SIZE);
> +	hlen = (type == LOWPAN_DISPATCH_FRAG1 ? LOWPAN_FRAG1_HEAD_SIZE :
> +			LOWPAN_FRAGN_HEAD_SIZE);

The second line of this statement should be aligned as follows:

+	hlen = (type == LOWPAN_DISPATCH_FRAG1 ? LOWPAN_FRAG1_HEAD_SIZE :
+		LOWPAN_FRAGN_HEAD_SIZE);

So the L for LOWPAN_FRAGN_HEAD_SIZE should be underneath the t for type.

Re: [PATCH net-next 06/15] 6lowpan: fix first fragment (FRAG1) handling

[Tony Cheneau]

Hello Jan,

Thank you for all your comments. See my answer inline.

Le 23.10.2012 09:19, Jan Ceuleers a écrit :
> On 10/23/2012 06:09 AM, Tony Cheneau wrote:
>> The first fragment, FRAG1, must contain some payload according to 
>> the
>> specs. However, as it is currently written, the first fragment will
>> remain empty and only contain the 6lowpan headers.
>>
>> This patch also extract the transport layer information from the 
>> first
>> fragment. This information is later on use when uncompressing UDP
>> header.
>>
>> Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org>
>> ---
>>  net/ieee802154/6lowpan.c |   54 
>> +++++++++++++++++++++++++++++++++++----------
>>  1 files changed, 42 insertions(+), 12 deletions(-)
>>
>> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
>> index 8a2ee95..38cecaf 100644
>> --- a/net/ieee802154/6lowpan.c
>> +++ b/net/ieee802154/6lowpan.c
>> @@ -654,7 +654,7 @@ static void 
>> lowpan_fragment_timer_expired(unsigned long entry_addr)
>>  }
>>
>>  static struct lowpan_fragment *
>> -lowpan_alloc_new_frame(struct sk_buff *skb, u8 len, u16 tag)
>> +lowpan_alloc_new_frame(struct sk_buff *skb, u16 len, u16 tag)
>>  {
>>  	struct lowpan_fragment *frame;
>>
>> @@ -735,6 +735,18 @@ lowpan_process_data(struct sk_buff *skb)
>>  		/* adds the 3 MSB to the 8 LSB to retrieve the 11 bits length */
>>  		len = ((iphc0 & 7) << 8) | slen;
>>
>> +		if ((iphc0 & LOWPAN_DISPATCH_MASK) == LOWPAN_DISPATCH_FRAG1) {
>> +			pr_debug("%s received a FRAG1 packet (tag: %d, "
>> +				 "size of the entire IP packet: %d)"
>> +				 , __func__, tag, len);
>
> There are several schools of thought on the relative importance of
> observing the 80-character line limit versus breaking up string
> constants (in an attempt to maintain grepability). I think the above 
> is
> fine but others (whose opinion matters more than mine) may or may not
> agree. Whatever you decide here, please apply consistently 
> throughout.
Yes, I've seen that particular issues when running checkpatch.pl. I 
decided to break down line, but I can easily be convinced to do things 
differently. Anyway, I'll make sure that all my patches are consistent 
in breaking up string after 80 characters the same way.

> However, the comma ahead of the __func__ should be at the end of the
> previous line.
Will do.

>
>> -	/* if payload length is zero, therefore it's a first fragment */
>> -	hlen = (plen == 0 ? LOWPAN_FRAG1_HEAD_SIZE :  
>> LOWPAN_FRAGN_HEAD_SIZE);
>> +	hlen = (type == LOWPAN_DISPATCH_FRAG1 ? LOWPAN_FRAG1_HEAD_SIZE :
>> +			LOWPAN_FRAGN_HEAD_SIZE);
>
> The second line of this statement should be aligned as follows:
>
> +	hlen = (type == LOWPAN_DISPATCH_FRAG1 ? LOWPAN_FRAG1_HEAD_SIZE :
> +		LOWPAN_FRAGN_HEAD_SIZE);
>
> So the L for LOWPAN_FRAGN_HEAD_SIZE should be underneath the t for 
> type.
Will do as well.

Again, thank you for all your detailed comments.

Regards,
Tony

Re: [PATCH net-next 06/15] 6lowpan: fix first fragment (FRAG1) handling

[Eric Dumazet]

On Tue, 2012-10-23 at 16:50 +0200, Tony Cheneau wrote:

> >
> > +	hlen = (type == LOWPAN_DISPATCH_FRAG1 ? LOWPAN_FRAG1_HEAD_SIZE :
> > +		LOWPAN_FRAGN_HEAD_SIZE);
> >
> > So the L for LOWPAN_FRAGN_HEAD_SIZE should be underneath the t for 
> > type.
> Will do as well.

By the way parens are not needed, or better like that :

	hlen = (type == LOWPAN_DISPATCH_FRAG1) ?
			LOWPAN_FRAG1_HEAD_SIZE : LOWPAN_FRAGN_HEAD_SIZE;

[PATCH net-next 07/15] 6lowpan: store fragment tag values per device instead of net stack wide

[Tony Cheneau]

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 38cecaf..eb8003b 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -104,6 +104,7 @@ static const u8 lowpan_llprefix[] = {0xfe, 0x80};
 struct lowpan_dev_info {
 	struct net_device	*real_dev; /* real WPAN device ptr */
 	struct mutex		dev_list_mtx; /* mutex for list ops */
+	unsigned short fragment_tag;
 };
 
 struct lowpan_dev_record {
@@ -120,7 +121,6 @@ struct lowpan_fragment {
 	struct list_head	list;		/* fragments list */
 };
 
-static unsigned short fragment_tag;
 static LIST_HEAD(lowpan_fragments);
 static DEFINE_SPINLOCK(flist_lock);
 
@@ -1022,14 +1022,14 @@ lowpan_fragment_xmit(struct sk_buff *skb, u8 *head,
 }
 
 static int
-lowpan_skb_fragmentation(struct sk_buff *skb)
+lowpan_skb_fragmentation(struct sk_buff *skb, struct net_device *dev)
 {
 	int  err, header_length, payload_length, tag, offset = 0;
 	u8 head[5];
 
 	header_length = lowpan_get_mac_header_length(skb);
 	payload_length = skb->len - header_length;
-	tag = fragment_tag++;
+	tag = lowpan_dev_info(dev)->fragment_tag++;
 
 	/* first fragment header */
 	head[0] = LOWPAN_DISPATCH_FRAG1 | ((payload_length >> 8) & 0x7);
@@ -1095,7 +1095,7 @@ static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	pr_debug("frame is too big, fragmentation is needed\n");
-	err = lowpan_skb_fragmentation(skb);
+	err = lowpan_skb_fragmentation(skb, dev);
 error:
 	dev_kfree_skb(skb);
 out:
@@ -1216,6 +1216,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 		return -ENODEV;
 
 	lowpan_dev_info(dev)->real_dev = real_dev;
+	lowpan_dev_info(dev)->fragment_tag = 0;
 	mutex_init(&lowpan_dev_info(dev)->dev_list_mtx);
 
 	entry = kzalloc(sizeof(struct lowpan_dev_record), GFP_KERNEL);
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 08/15] 6lowpan: obtain IEEE802.15.4 sequence number from the MAC layer

[Tony Cheneau]

This patch sets the sequence number in the frame format. Without this
fix, the sequence number is always set to 0. This makes trafic analysis
very hard.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index eb8003b..24b83fa 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -578,6 +578,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 	 */
 	{
 		mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
+		mac_cb(skb)->seq = ieee802154_mlme_ops(dev)->get_dsn(dev);
 
 		/* prepare wpan address data */
 		sa.addr_type = IEEE802154_ADDR_LONG;
@@ -1123,6 +1124,12 @@ static u16 lowpan_get_short_addr(const struct net_device *dev)
 	return ieee802154_mlme_ops(real_dev)->get_short_addr(real_dev);
 }
 
+static u8 lowpan_get_dsn(const struct net_device *dev)
+{
+	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
+	return ieee802154_mlme_ops(real_dev)->get_dsn(real_dev);
+}
+
 static struct header_ops lowpan_header_ops = {
 	.create	= lowpan_header_create,
 };
@@ -1136,6 +1143,7 @@ static struct ieee802154_mlme_ops lowpan_mlme = {
 	.get_pan_id = lowpan_get_pan_id,
 	.get_phy = lowpan_get_phy,
 	.get_short_addr = lowpan_get_short_addr,
+	.get_dsn = lowpan_get_dsn,
 };
 
 static void lowpan_setup(struct net_device *dev)
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 09/15] 6lowpan: add a new parameter in sysfs to turn on/off ACK request at MAC layer

[Tony Cheneau]

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 24b83fa..f8fcdae 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -62,6 +62,8 @@
 
 #include "6lowpan.h"
 
+static bool req_802154_ack;
+
 /* TTL uncompression values */
 static const u8 lowpan_ttl_values[] = {0, 1, 64, 255};
 
@@ -596,7 +598,8 @@ static int lowpan_header_create(struct sk_buff *skb,
 			memcpy(&(da.hwaddr), daddr, 8);
 
 			/* request acknowledgment */
-			mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
+			if (req_802154_ack)
+				mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
 		}
 
 		return dev_hard_header(skb, lowpan_dev_info(dev)->real_dev,
@@ -1366,6 +1369,10 @@ static void __exit lowpan_cleanup_module(void)
 }
 
 module_init(lowpan_init_module);
+
+module_param(req_802154_ack, bool, 0644);
+MODULE_PARM_DESC(req_802154_ack, "request link-layer (i.e. IEEE 802.15.4) acknowledgments");
+
 module_exit(lowpan_cleanup_module);
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_RTNL_LINK("lowpan");
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 10/15] 6lowpan: use the PANID provided by the device instead of a static value

[Tony Cheneau]

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index f8fcdae..9711038 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -584,10 +584,12 @@ static int lowpan_header_create(struct sk_buff *skb,
 
 		/* prepare wpan address data */
 		sa.addr_type = IEEE802154_ADDR_LONG;
-		sa.pan_id = 0xff;
+		sa.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
+
 		memcpy(&(sa.hwaddr), saddr, 8);
+		/* intra-PAN communications */
+		da.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
 
-		da.pan_id = 0xff;
 		/* if the destination address is the broadcast address,
 		   use short address */
 		if (lowpan_is_addr_broadcast(daddr)) {
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 11/15] 6lowpan: modify udp compression/uncompression to match the standard

[Tony Cheneau]

The previous code would just compress the UDP header and send the compressed
UDP header along with the uncompressed one.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |   36 +++++++++++++++++++++++++++++++++---
 1 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 9711038..9c7ac2e 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -286,6 +286,9 @@ lowpan_compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 	/* checksum is always inline */
 	memcpy(*hc06_ptr, &uh->check, 2);
 	*hc06_ptr += 2;
+
+	/* skip the UDP header */
+	skb_pull(skb, sizeof(struct udphdr));
 }
 
 static inline int lowpan_fetch_skb_u8(struct sk_buff *skb, u8 *val)
@@ -311,9 +314,8 @@ static inline int lowpan_fetch_skb_u16(struct sk_buff *skb, u16 *val)
 }
 
 static int
-lowpan_uncompress_udp_header(struct sk_buff *skb)
+lowpan_uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 {
-	struct udphdr *uh = udp_hdr(skb);
 	u8 tmp;
 
 	if (!uh)
@@ -354,12 +356,19 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 			break;
 		}
 
+
 		pr_debug("uncompressed UDP ports: src = %d, dst = %d\n",
 			 uh->source, uh->dest);
 
 		/* copy checksum */
 		memcpy(&uh->check, &skb->data[0], 2);
 		skb_pull(skb, 2);
+
+		/* UDP lenght needs to be infered from the lower layers
+		   here, we obtain the hint from the remaining size of the
+		   frame */
+		uh->len = htons(skb->len + sizeof(struct udphdr));
+		pr_debug("uncompressed UDP length: src = %d", uh->len);
 	} else {
 		pr_debug("ERROR: unsupported NH format\n");
 		goto err;
@@ -941,8 +950,29 @@ lowpan_process_data(struct sk_buff *skb)
 
 	/* UDP data uncompression */
 	if (iphc0 & LOWPAN_IPHC_NH_C) {
-		if (lowpan_uncompress_udp_header(skb))
+		struct udphdr uh;
+		struct sk_buff *new;
+		if (lowpan_uncompress_udp_header(skb, &uh))
 			goto drop;
+
+		/* place the real UDP header instead of the
+		   compressed UDP header */
+		new = skb_copy_expand(skb, sizeof(struct udphdr),
+				      skb_tailroom(skb), GFP_ATOMIC);
+		kfree_skb(skb);
+
+		if (!new)
+			return -ENOMEM;
+
+		skb = new ;
+
+		skb_push(skb, sizeof(struct udphdr));
+		skb_reset_transport_header(skb);
+		skb_copy_to_linear_data(skb, &uh, sizeof(struct udphdr));
+
+		lowpan_raw_dump_table(__func__, "raw UDP header dump",
+				      (u8 *)&uh, sizeof(uh));
+
 		hdr.nexthdr = UIP_PROTO_UDP;
 	}
 
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

Re: [PATCH net-next 11/15] 6lowpan: modify udp compression/uncompression to match the standard

[Jan Ceuleers]

On 10/23/2012 06:09 AM, Tony Cheneau wrote:
> The previous code would just compress the UDP header and send the compressed
> UDP header along with the uncompressed one.
> 
> Signed-off-by: Tony Cheneau <tony.cheneau@amnesiak.org>
> ---
>  net/ieee802154/6lowpan.c |   36 +++++++++++++++++++++++++++++++++---
>  1 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> index 9711038..9c7ac2e 100644
> --- a/net/ieee802154/6lowpan.c
> +++ b/net/ieee802154/6lowpan.c
(...)

>  		/* copy checksum */
>  		memcpy(&uh->check, &skb->data[0], 2);
>  		skb_pull(skb, 2);
> +
> +		/* UDP lenght needs to be infered from the lower layers
> +		   here, we obtain the hint from the remaining size of the
> +		   frame */
> +		uh->len = htons(skb->len + sizeof(struct udphdr));
> +		pr_debug("uncompressed UDP length: src = %d", uh->len);

Multi-line comment style as per earlier comment.

[PATCH net-next 12/15] 6lowpan: make memory allocation atomic during 6lowpan header creation

[Tony Cheneau]

This is prevent various crashes when using the serial driver (not yet in
the tree).

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/ieee802154/6lowpan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 9c7ac2e..70ff171 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -396,7 +396,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 		/* TODO:
 		 * if this package isn't ipv6 one, where should it be routed?
 		 */
-	head = kzalloc(100, GFP_KERNEL);
+	head = kzalloc(100, GFP_ATOMIC);
 	if (head == NULL)
 		return -ENOMEM;
 
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 13/15] mac802154: make mem alloc ATOMIC to prevent "scheduling while atomic" crashes

[Tony Cheneau]

These crashes occur mainly with the serial driver (not yet in the tree).

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/mac802154/wpan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index d6aea7f..49ba8df 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -143,7 +143,7 @@ static int mac802154_header_create(struct sk_buff *skb,
 	if (!daddr)
 		return -EINVAL;
 
-	head = kzalloc(MAC802154_FRAME_HARD_HEADER_LEN, GFP_KERNEL);
+	head = kzalloc(MAC802154_FRAME_HARD_HEADER_LEN, GFP_ATOMIC);
 	if (head == NULL)
 		return -ENOMEM;
 
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 14/15] mac802154: remove unnecessary spinlocks

[Tony Cheneau]

These spinlock protects an int variable, that is always accessible in an
atomic fashion.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/mac802154/mib.c |   14 ++------------
 1 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index f47781a..2339f8d 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -103,15 +103,10 @@ void mac802154_dev_set_short_addr(struct net_device *dev, u16 val)
 u16 mac802154_dev_get_short_addr(const struct net_device *dev)
 {
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
-	u16 ret;
 
 	BUG_ON(dev->type != ARPHRD_IEEE802154);
 
-	spin_lock_bh(&priv->mib_lock);
-	ret = priv->short_addr;
-	spin_unlock_bh(&priv->mib_lock);
-
-	return ret;
+	return priv->short_addr;
 }
 
 void mac802154_dev_set_ieee_addr(struct net_device *dev)
@@ -131,15 +126,10 @@ void mac802154_dev_set_ieee_addr(struct net_device *dev)
 u16 mac802154_dev_get_pan_id(const struct net_device *dev)
 {
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
-	u16 ret;
 
 	BUG_ON(dev->type != ARPHRD_IEEE802154);
 
-	spin_lock_bh(&priv->mib_lock);
-	ret = priv->pan_id;
-	spin_unlock_bh(&priv->mib_lock);
-
-	return ret;
+	return priv->pan_id;
 }
 
 void mac802154_dev_set_pan_id(struct net_device *dev, u16 val)
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

[PATCH net-next 15/15] mac802154: re-introduce MAC primitives required to send/receive packets

[Tony Cheneau]

- mlme_assoc_req() and mlme_assoc_resp() are just place holder for the
  moment (they prevent the two corresponding function pointers in the
  mac802154_mlme_wpan structure to be left uninitialized)
- mac802514_dev_get_bsn() and mac802514_dev_get_dsn() MAC primitives
  were present in the Linux-Zigbee kernel and are being re-introduced.

Signed-off-by: Tony Cheneau <tony.cheneau-jNfjcPZKvDhg9hUCZPvPmw@public.gmane.org>
---
 net/mac802154/mac802154.h |    2 ++
 net/mac802154/mac_cmd.c   |   22 +++++++++++++++++++++-
 net/mac802154/mib.c       |   19 +++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/net/mac802154/mac802154.h b/net/mac802154/mac802154.h
index a4dcaf1..18d4044 100644
--- a/net/mac802154/mac802154.h
+++ b/net/mac802154/mac802154.h
@@ -114,5 +114,7 @@ void mac802154_dev_set_ieee_addr(struct net_device *dev);
 u16 mac802154_dev_get_pan_id(const struct net_device *dev);
 void mac802154_dev_set_pan_id(struct net_device *dev, u16 val);
 void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan);
+u8 mac802154_dev_get_dsn(const struct net_device *dev);
+u8 mac802154_dev_get_bsn(const struct net_device *dev);
 
 #endif /* MAC802154_H */
diff --git a/net/mac802154/mac_cmd.c b/net/mac802154/mac_cmd.c
index d8d2770..8e46e7b 100644
--- a/net/mac802154/mac_cmd.c
+++ b/net/mac802154/mac_cmd.c
@@ -33,6 +33,22 @@
 
 #include "mac802154.h"
 
+static int mac802154_mlme_assoc_req(struct net_device *dev,
+			struct ieee802154_addr *addr,
+			u8 channel, u8 page, u8 cap)
+{
+	/* TBD */
+	return 0;
+}
+
+static int mac802154_mlme_assoc_resp(struct net_device *dev,
+			struct ieee802154_addr *addr,
+			u16 short_addr, u8 status)
+{
+	/* TBD */
+	return 0;
+}
+
 static int mac802154_mlme_start_req(struct net_device *dev,
 				    struct ieee802154_addr *addr,
 				    u8 channel, u8 page,
@@ -70,7 +86,11 @@ struct ieee802154_reduced_mlme_ops mac802154_mlme_reduced = {
 
 struct ieee802154_mlme_ops mac802154_mlme_wpan = {
 	.get_phy = mac802154_get_phy,
-	.start_req = mac802154_mlme_start_req,
 	.get_pan_id = mac802154_dev_get_pan_id,
 	.get_short_addr = mac802154_dev_get_short_addr,
+	.get_dsn = mac802154_dev_get_dsn,
+	.get_bsn = mac802154_dev_get_bsn,
+	.start_req = mac802154_mlme_start_req,
+	.assoc_req = mac802154_mlme_assoc_req,
+	.assoc_resp = mac802154_mlme_assoc_resp
 };
diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index 2339f8d..70ab6ca 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -149,6 +149,25 @@ void mac802154_dev_set_pan_id(struct net_device *dev, u16 val)
 	}
 }
 
+u8 mac802154_dev_get_dsn(const struct net_device *dev)
+{
+	struct mac802154_sub_if_data *priv = netdev_priv(dev);
+
+	BUG_ON(dev->type != ARPHRD_IEEE802154);
+
+	return priv->dsn++;
+}
+
+u8 mac802154_dev_get_bsn(const struct net_device *dev)
+{
+	struct mac802154_sub_if_data *priv = netdev_priv(dev);
+
+	BUG_ON(dev->type != ARPHRD_IEEE802154);
+
+	return priv->bsn++;
+}
+
+
 static void phy_chan_notify(struct work_struct *work)
 {
 	struct phy_chan_notify_work *nw = container_of(work,
-- 
1.7.8.6


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

Re: [PATCH net-next 00/15] 6lowpan: Some more bug fixes

[Alexander Smirnov]

Hi,

2012/10/23 Tony Cheneau <tony.cheneau@amnesiak.org>:
> Hello,
>
> This patch serie fixes a few bugs within the 6lowpan modules and prepares the
> integration of the serial driver that will be submitted in a subsequent patch serie.
> It should apply cleanly on the latest net-next.

1. The series is quite huge what makes it difficult for the review. It
would be better to split it into one-two and submit separately (not
simultaneously).
2. Could you also please provide some notes about how have you tested
these changes (logs, plain text)? Do I need to check your changes
locally on my desk? If so I need some instructions.
3. Please DO NOT submit patches like: this patch fixes blablabla which
isn't in the kernel yet (like patch 13,15). I have no clue what you
have locally on your laptop and what you will send in some time. I'd
like to see here the working code, not a references to TBD.
4. The reference to linux-zigbee project isn't an occasion for me to
apply this code to this tree. I have no goal to merge all this fun to
mainline due to several things in linux-zigbee kernel work NOT
according to the standard (mostly it's a timing problems) and global
refactoring needed.

Alex

>
> Regards,
>         Tony Cheneau
>
> Tony Cheneau (15):
>   6lowpan: lowpan_is_iid_16_bit_compressable() does not detect
>     compressable address correctly
>   6lowpan: next header is not properly set upon decompression of a UDP
>     header.
>   6lowpan: always enable link-layer acknowledgments
>   mac802154: turn on ACK when enabled by the upper layers
>   6lowpan: use short IEEE 802.15.4 addresses for broadcast destination
>   6lowpan: fix first fragment (FRAG1) handling
>   6lowpan: store fragment tag values per device instead of net stack
>     wide
>   6lowpan: obtain IEEE802.15.4 sequence number from the MAC layer
>   6lowpan: add a new parameter in sysfs to turn on/off ACK request at
>     MAC layer
>   6lowpan: use the PANID provided by the device instead of a static
>     value
>   6lowpan: modify udp compression/uncompression to match the standard
>   6lowpan: make memory allocation atomic during 6lowpan header creation
>   mac802154: make mem alloc ATOMIC to prevent "scheduling while atomic"
>     crashes
>   mac802154: remove unnecessary spinlocks
>   mac802154: re-introduce MAC primitives required to send/receive
>     packets
>
>  net/ieee802154/6lowpan.c  |  144 ++++++++++++++++++++++++++++++++++++---------
>  net/ieee802154/6lowpan.h  |   14 +++--
>  net/mac802154/mac802154.h |    2 +
>  net/mac802154/mac_cmd.c   |   22 +++++++-
>  net/mac802154/mib.c       |   33 +++++++----
>  net/mac802154/wpan.c      |    4 +-
>  6 files changed, 172 insertions(+), 47 deletions(-)
>
> --
> 1.7.8.6
>

Re: [PATCH net-next 00/15] 6lowpan: Some more bug fixes

[Tony Cheneau]

Hello Alexander,

Thank you for your comments. See my answer inline.

Le 25.10.2012 06:52, Alexander Smirnov a écrit :
> Hi,
>
[...]
> 1. The series is quite huge what makes it difficult for the review. 
> It
> would be better to split it into one-two and submit separately (not
> simultaneously).
OK. Will do.

> 2. Could you also please provide some notes about how have you tested
> these changes (logs, plain text)? Do I need to check your changes
> locally on my desk? If so I need some instructions.
I'm not sure what you are asking here. There was some more debugging 
printk, that I removed because they were too verbose. Mostly, I test my 
patch with few userspace program, like ping6, iperf (both for TCP and 
UDP) and ssh. I also wrote a TCP and a UDP echo server, so as to test 
fragmentation in more details. For most functional patches, I usually 
confirm through wireshark that the packets indeed look the way they 
should (this, I've done it for patch 11 for example). But I don't have 
any automated scripts to check for regression. I can provide you the 
script I use to configure nodes if needed (but it pretty straighward and 
should look like your own scripts).
I'm not sure that really answers your question.

> 3. Please DO NOT submit patches like: this patch fixes blablabla 
> which
> isn't in the kernel yet (like patch 13,15). I have no clue what you
> have locally on your laptop and what you will send in some time. I'd
> like to see here the working code, not a references to TBD.
I'm OK with removing patch 13 (I'll introduce it alongside the serial 
driver later on). I'm not so OK with removing patch 15, or at least, it 
would require some more testing in other parts of the code. Basically, 
the TBD are placeholder for when the Association Request/Response will 
be reimplemented. I introduced it because otherwise, .assoc_req and 
.assoc_resp entries of mac802154_mlme_wpan would go uninitialized and 
would cause kernel crash when called. I could have modified the calling 
code to deal with that, but I thought these primitives were meant to be 
re-implemented soon anyway.

> 4. The reference to linux-zigbee project isn't an occasion for me to
> apply this code to this tree. I have no goal to merge all this fun to
> mainline due to several things in linux-zigbee kernel work NOT
> according to the standard (mostly it's a timing problems) and global
> refactoring needed.
I don't understand what you are talking about here. I would guess that 
you are talking about patch 15. If so, my answer is the following, while 
I understand your effort to refactor linux-zigbee code and fix things 
along the way, some code went through net-next that cause crashes. My 
pragmatic approach is to try to fix it before more people use it, even 
through it might not always be the most elegant way to do it.

Regards,
Tony

Re: [PATCH net-next 00/15] 6lowpan: Some more bug fixes

[Alan Ott]

On 10/25/2012 10:49 AM, Tony Cheneau wrote:
>  I'm not so OK with removing patch 15, or at least, it would require
> some more testing in other parts of the code. Basically, the TBD are
> placeholder for when the Association Request/Response will be
> reimplemented. I introduced it because otherwise, .assoc_req and
> .assoc_resp entries of mac802154_mlme_wpan would go uninitialized and
> would cause kernel crash when called. I could have modified the
> calling code to deal with that, but I thought these primitives were
> meant to be re-implemented soon anyway.

I think just simply stating this in the commit message would fix the
issue here. Maybe something like "Fix crash where assoc_req() and
assoc_resp() were not defined."

This should probably get split into two patches, one for BSN and the
other for assoc.

Also, make the bsn functions static, and the convention is to put static
functions above where they are being used, so that you don't have to
forward declare them. It looks though, like this source file doesn't do
that for other functions, so maybe following the convention of this file
for now is ok.

Alan.


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct