[PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Signed-off-by: Martin Townsend<martin.townsend@xsilon.com>
---
  include/net/6lowpan.h         |  2 +-
  net/6lowpan/iphc.c            | 29 ++++++++++++++++-------------
  net/bluetooth/6lowpan.c       | 12 ++++++++----
  net/ieee802154/6lowpan_rtnl.c | 12 ++++++------
  4 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index d7e9169..aa0381e 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -374,7 +374,7 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
  
  typedef int (*skb_delivery_cb)(struct sk_buff *skb);
  
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
+int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
  		const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
  		const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
  		u8 iphc0, u8 iphc1, skb_delivery_cb skb_deliver);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 92eed6d..18dac0a 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -179,11 +179,11 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
  
  	new = skb_copy_expand(skb, sizeof(struct ipv6hdr), skb_tailroom(skb),
  			      GFP_ATOMIC);
-	kfree_skb(skb);
-
  	if (!new)
  		return -ENOMEM;
  
+	kfree_skb(skb);
+
  	skb_push(new, sizeof(struct ipv6hdr));
  	skb_reset_network_header(new);
  	skb_copy_to_linear_data(new, hdr, sizeof(struct ipv6hdr));
@@ -196,6 +196,8 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
  		       new->data, new->len);
  
  	stat = deliver_skb(new);
+	if (stat < 0)
+		stat = NET_RX_DROP;
  
  	kfree_skb(new);
  
@@ -333,7 +335,7 @@ err:
  /* TTL uncompression values */
  static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
  
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
+int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
  			const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
  			const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
  			u8 iphc0, u8 iphc1, skb_delivery_cb deliver_skb)
@@ -341,6 +343,8 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  	struct ipv6hdr hdr = {};
  	u8 tmp, num_context = 0;
  	int err;
+	int err_ret = -EINVAL;
+	struct sk_buff *skb = *skb_inout;
  
  	raw_dump_table(__func__, "raw skb data dump uncompressed",
  		       skb->data, skb->len);
@@ -461,7 +465,6 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  	/* UDP data uncompression */
  	if (iphc0 & LOWPAN_IPHC_NH_C) {
  		struct udphdr uh;
-		struct sk_buff *new;
  
  		if (uncompress_udp_header(skb, &uh))
  			goto drop;
@@ -469,14 +472,15 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  		/* replace the compressed UDP head by the uncompressed UDP
  		 * header
  		 */
-		new = skb_copy_expand(skb, sizeof(struct udphdr),
+		skb = skb_copy_expand(skb, sizeof(struct udphdr),
  				      skb_tailroom(skb), GFP_ATOMIC);
-		kfree_skb(skb);
-
-		if (!new)
-			return -ENOMEM;
-
-		skb = new;
+		if (!skb) {
+			err_ret = -ENOMEM;
+			goto drop;
+		}
+		
+		kfree_skb(*skb_inout);
+		*skb_inout = skb;
  
  		skb_push(skb, sizeof(struct udphdr));
  		skb_reset_transport_header(skb);
@@ -503,8 +507,7 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  	return skb_deliver(skb, &hdr, dev, deliver_skb);
  
  drop:
-	kfree_skb(skb);
-	return -EINVAL;
+	return err_ret;
  }
  EXPORT_SYMBOL_GPL(lowpan_process_data);
  
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f5df93f..38a2987 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -215,7 +215,7 @@ static int give_skb_to_upper(struct sk_buff *skb)
  	return ret;
  }
  
-static int process_data(struct sk_buff *skb, struct net_device *netdev,
+static int process_data(struct sk_buff **skb_inout, struct net_device *netdev,
  			struct l2cap_chan *chan)
  {
  	const u8 *saddr, *daddr;
@@ -223,6 +223,7 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
  	struct lowpan_dev *dev;
  	struct lowpan_peer *peer;
  	unsigned long flags;
+	struct sk_buff *skb = *skb_inout;
  
  	dev = lowpan_dev(netdev);
  
@@ -245,13 +246,12 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
  	if (lowpan_fetch_skb_u8(skb, &iphc1))
  		goto drop;
  
-	return lowpan_process_data(skb, netdev,
+	return lowpan_process_data(skb_inout, netdev,
  				   saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
  				   daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
  				   iphc0, iphc1, give_skb_to_upper);
  
  drop:
-	kfree_skb(skb);
  	return -EINVAL;
  }
  
@@ -300,7 +300,11 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
  			if (!local_skb)
  				goto drop;
  
-			ret = process_data(local_skb, dev, chan);
+			ret = process_data(&local_skb, dev, chan);
+			if (ret < 0) {
+				kfree_skb(local_skb);
+				goto drop;
+			}
  			if (ret != NET_RX_SUCCESS)
  				goto drop;
  
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 3154775..3ae7646 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -166,11 +166,12 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb)
  	return stat;
  }
  
-static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
+static int process_data(struct sk_buff **skb_inout, const struct ieee802154_hdr *hdr)
  {
  	u8 iphc0, iphc1;
  	struct ieee802154_addr_sa sa, da;
  	void *sap, *dap;
+	struct sk_buff *skb = *skb_inout;
  
  	raw_dump_table(__func__, "raw skb data dump", skb->data, skb->len);
  	/* at least two bytes will be used for the encoding */
@@ -196,13 +197,12 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
  	else
  		dap = &da.hwaddr;
  
-	return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
+	return lowpan_process_data(skb_inout, skb->dev, sap, sa.addr_type,
  				   IEEE802154_ADDR_LEN, dap, da.addr_type,
  				   IEEE802154_ADDR_LEN, iphc0, iphc1,
  				   lowpan_give_skb_to_devices);
  
  drop:
-	kfree_skb(skb);
  	return -EINVAL;
  }
  
@@ -485,14 +485,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
  	} else {
  		switch (skb->data[0] & 0xe0) {
  		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
-			ret = process_data(skb, &hdr);
+			ret = process_data(&skb, &hdr);
  			if (ret == NET_RX_DROP)
  				goto drop;
  			break;
  		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
  			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
  			if (ret == 1) {
-				ret = process_data(skb, &hdr);
+				ret = process_data(&skb, &hdr);
  				if (ret == NET_RX_DROP)
  					goto drop;
  			}
@@ -500,7 +500,7 @@ 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 = process_data(skb, &hdr);
+				ret = process_data(&skb, &hdr);
  				if (ret == NET_RX_DROP)
  					goto drop;
  			}
-- 
1.8.1.2

[Linux-zigbee-devel] [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Signed-off-by: Martin Townsend<martin.townsend@xsilon.com>
---
  include/net/6lowpan.h         |  2 +-
  net/6lowpan/iphc.c            | 29 ++++++++++++++++-------------
  net/bluetooth/6lowpan.c       | 12 ++++++++----
  net/ieee802154/6lowpan_rtnl.c | 12 ++++++------
  4 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index d7e9169..aa0381e 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -374,7 +374,7 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
  
  typedef int (*skb_delivery_cb)(struct sk_buff *skb);
  
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
+int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
  		const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
  		const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
  		u8 iphc0, u8 iphc1, skb_delivery_cb skb_deliver);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 92eed6d..18dac0a 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -179,11 +179,11 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
  
  	new = skb_copy_expand(skb, sizeof(struct ipv6hdr), skb_tailroom(skb),
  			      GFP_ATOMIC);
-	kfree_skb(skb);
-
  	if (!new)
  		return -ENOMEM;
  
+	kfree_skb(skb);
+
  	skb_push(new, sizeof(struct ipv6hdr));
  	skb_reset_network_header(new);
  	skb_copy_to_linear_data(new, hdr, sizeof(struct ipv6hdr));
@@ -196,6 +196,8 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
  		       new->data, new->len);
  
  	stat = deliver_skb(new);
+	if (stat < 0)
+		stat = NET_RX_DROP;
  
  	kfree_skb(new);
  
@@ -333,7 +335,7 @@ err:
  /* TTL uncompression values */
  static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
  
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
+int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
  			const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
  			const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
  			u8 iphc0, u8 iphc1, skb_delivery_cb deliver_skb)
@@ -341,6 +343,8 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  	struct ipv6hdr hdr = {};
  	u8 tmp, num_context = 0;
  	int err;
+	int err_ret = -EINVAL;
+	struct sk_buff *skb = *skb_inout;
  
  	raw_dump_table(__func__, "raw skb data dump uncompressed",
  		       skb->data, skb->len);
@@ -461,7 +465,6 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  	/* UDP data uncompression */
  	if (iphc0 & LOWPAN_IPHC_NH_C) {
  		struct udphdr uh;
-		struct sk_buff *new;
  
  		if (uncompress_udp_header(skb, &uh))
  			goto drop;
@@ -469,14 +472,15 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  		/* replace the compressed UDP head by the uncompressed UDP
  		 * header
  		 */
-		new = skb_copy_expand(skb, sizeof(struct udphdr),
+		skb = skb_copy_expand(skb, sizeof(struct udphdr),
  				      skb_tailroom(skb), GFP_ATOMIC);
-		kfree_skb(skb);
-
-		if (!new)
-			return -ENOMEM;
-
-		skb = new;
+		if (!skb) {
+			err_ret = -ENOMEM;
+			goto drop;
+		}
+		
+		kfree_skb(*skb_inout);
+		*skb_inout = skb;
  
  		skb_push(skb, sizeof(struct udphdr));
  		skb_reset_transport_header(skb);
@@ -503,8 +507,7 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
  	return skb_deliver(skb, &hdr, dev, deliver_skb);
  
  drop:
-	kfree_skb(skb);
-	return -EINVAL;
+	return err_ret;
  }
  EXPORT_SYMBOL_GPL(lowpan_process_data);
  
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f5df93f..38a2987 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -215,7 +215,7 @@ static int give_skb_to_upper(struct sk_buff *skb)
  	return ret;
  }
  
-static int process_data(struct sk_buff *skb, struct net_device *netdev,
+static int process_data(struct sk_buff **skb_inout, struct net_device *netdev,
  			struct l2cap_chan *chan)
  {
  	const u8 *saddr, *daddr;
@@ -223,6 +223,7 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
  	struct lowpan_dev *dev;
  	struct lowpan_peer *peer;
  	unsigned long flags;
+	struct sk_buff *skb = *skb_inout;
  
  	dev = lowpan_dev(netdev);
  
@@ -245,13 +246,12 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
  	if (lowpan_fetch_skb_u8(skb, &iphc1))
  		goto drop;
  
-	return lowpan_process_data(skb, netdev,
+	return lowpan_process_data(skb_inout, netdev,
  				   saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
  				   daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
  				   iphc0, iphc1, give_skb_to_upper);
  
  drop:
-	kfree_skb(skb);
  	return -EINVAL;
  }
  
@@ -300,7 +300,11 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
  			if (!local_skb)
  				goto drop;
  
-			ret = process_data(local_skb, dev, chan);
+			ret = process_data(&local_skb, dev, chan);
+			if (ret < 0) {
+				kfree_skb(local_skb);
+				goto drop;
+			}
  			if (ret != NET_RX_SUCCESS)
  				goto drop;
  
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 3154775..3ae7646 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -166,11 +166,12 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb)
  	return stat;
  }
  
-static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
+static int process_data(struct sk_buff **skb_inout, const struct ieee802154_hdr *hdr)
  {
  	u8 iphc0, iphc1;
  	struct ieee802154_addr_sa sa, da;
  	void *sap, *dap;
+	struct sk_buff *skb = *skb_inout;
  
  	raw_dump_table(__func__, "raw skb data dump", skb->data, skb->len);
  	/* at least two bytes will be used for the encoding */
@@ -196,13 +197,12 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
  	else
  		dap = &da.hwaddr;
  
-	return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
+	return lowpan_process_data(skb_inout, skb->dev, sap, sa.addr_type,
  				   IEEE802154_ADDR_LEN, dap, da.addr_type,
  				   IEEE802154_ADDR_LEN, iphc0, iphc1,
  				   lowpan_give_skb_to_devices);
  
  drop:
-	kfree_skb(skb);
  	return -EINVAL;
  }
  
@@ -485,14 +485,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
  	} else {
  		switch (skb->data[0] & 0xe0) {
  		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
-			ret = process_data(skb, &hdr);
+			ret = process_data(&skb, &hdr);
  			if (ret == NET_RX_DROP)
  				goto drop;
  			break;
  		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
  			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
  			if (ret == 1) {
-				ret = process_data(skb, &hdr);
+				ret = process_data(&skb, &hdr);
  				if (ret == NET_RX_DROP)
  					goto drop;
  			}
@@ -500,7 +500,7 @@ 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 = process_data(skb, &hdr);
+				ret = process_data(&skb, &hdr);
  				if (ret == NET_RX_DROP)
  					goto drop;
  			}
-- 
1.8.1.2


------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

this patch doesn't apply for me on current bluetooth-next. Can you
respin the patch? Thanks.

- Alex

Re: [Linux-zigbee-devel] [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

this patch doesn't apply for me on current bluetooth-next. Can you
respin the patch? Thanks.

- Alex

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Re: [Linux-zigbee-devel] [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Hi Marcel,

I was just wondering what the status of this patch is?

- Martin.

On 02/08/14 14:27, Martin Townsend wrote:
> Signed-off-by: Martin Townsend<martin.townsend@xsilon.com>
> ---
>   include/net/6lowpan.h         |  2 +-
>   net/6lowpan/iphc.c            | 29 ++++++++++++++++-------------
>   net/bluetooth/6lowpan.c       | 12 ++++++++----
>   net/ieee802154/6lowpan_rtnl.c | 12 ++++++------
>   4 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index d7e9169..aa0381e 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -374,7 +374,7 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
>   
>   typedef int (*skb_delivery_cb)(struct sk_buff *skb);
>   
> -int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
> +int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
>   		const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
>   		const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
>   		u8 iphc0, u8 iphc1, skb_delivery_cb skb_deliver);
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 92eed6d..18dac0a 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -179,11 +179,11 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
>   
>   	new = skb_copy_expand(skb, sizeof(struct ipv6hdr), skb_tailroom(skb),
>   			      GFP_ATOMIC);
> -	kfree_skb(skb);
> -
>   	if (!new)
>   		return -ENOMEM;
>   
> +	kfree_skb(skb);
> +
>   	skb_push(new, sizeof(struct ipv6hdr));
>   	skb_reset_network_header(new);
>   	skb_copy_to_linear_data(new, hdr, sizeof(struct ipv6hdr));
> @@ -196,6 +196,8 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
>   		       new->data, new->len);
>   
>   	stat = deliver_skb(new);
> +	if (stat < 0)
> +		stat = NET_RX_DROP;
>   
>   	kfree_skb(new);
>   
> @@ -333,7 +335,7 @@ err:
>   /* TTL uncompression values */
>   static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
>   
> -int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
> +int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
>   			const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
>   			const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
>   			u8 iphc0, u8 iphc1, skb_delivery_cb deliver_skb)
> @@ -341,6 +343,8 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>   	struct ipv6hdr hdr = {};
>   	u8 tmp, num_context = 0;
>   	int err;
> +	int err_ret = -EINVAL;
> +	struct sk_buff *skb = *skb_inout;
>   
>   	raw_dump_table(__func__, "raw skb data dump uncompressed",
>   		       skb->data, skb->len);
> @@ -461,7 +465,6 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>   	/* UDP data uncompression */
>   	if (iphc0 & LOWPAN_IPHC_NH_C) {
>   		struct udphdr uh;
> -		struct sk_buff *new;
>   
>   		if (uncompress_udp_header(skb, &uh))
>   			goto drop;
> @@ -469,14 +472,15 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>   		/* replace the compressed UDP head by the uncompressed UDP
>   		 * header
>   		 */
> -		new = skb_copy_expand(skb, sizeof(struct udphdr),
> +		skb = skb_copy_expand(skb, sizeof(struct udphdr),
>   				      skb_tailroom(skb), GFP_ATOMIC);
> -		kfree_skb(skb);
> -
> -		if (!new)
> -			return -ENOMEM;
> -
> -		skb = new;
> +		if (!skb) {
> +			err_ret = -ENOMEM;
> +			goto drop;
> +		}
> +		
> +		kfree_skb(*skb_inout);
> +		*skb_inout = skb;
>   
>   		skb_push(skb, sizeof(struct udphdr));
>   		skb_reset_transport_header(skb);
> @@ -503,8 +507,7 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>   	return skb_deliver(skb, &hdr, dev, deliver_skb);
>   
>   drop:
> -	kfree_skb(skb);
> -	return -EINVAL;
> +	return err_ret;
>   }
>   EXPORT_SYMBOL_GPL(lowpan_process_data);
>   
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index f5df93f..38a2987 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -215,7 +215,7 @@ static int give_skb_to_upper(struct sk_buff *skb)
>   	return ret;
>   }
>   
> -static int process_data(struct sk_buff *skb, struct net_device *netdev,
> +static int process_data(struct sk_buff **skb_inout, struct net_device *netdev,
>   			struct l2cap_chan *chan)
>   {
>   	const u8 *saddr, *daddr;
> @@ -223,6 +223,7 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
>   	struct lowpan_dev *dev;
>   	struct lowpan_peer *peer;
>   	unsigned long flags;
> +	struct sk_buff *skb = *skb_inout;
>   
>   	dev = lowpan_dev(netdev);
>   
> @@ -245,13 +246,12 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
>   	if (lowpan_fetch_skb_u8(skb, &iphc1))
>   		goto drop;
>   
> -	return lowpan_process_data(skb, netdev,
> +	return lowpan_process_data(skb_inout, netdev,
>   				   saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
>   				   daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
>   				   iphc0, iphc1, give_skb_to_upper);
>   
>   drop:
> -	kfree_skb(skb);
>   	return -EINVAL;
>   }
>   
> @@ -300,7 +300,11 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>   			if (!local_skb)
>   				goto drop;
>   
> -			ret = process_data(local_skb, dev, chan);
> +			ret = process_data(&local_skb, dev, chan);
> +			if (ret < 0) {
> +				kfree_skb(local_skb);
> +				goto drop;
> +			}
>   			if (ret != NET_RX_SUCCESS)
>   				goto drop;
>   
> diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
> index 3154775..3ae7646 100644
> --- a/net/ieee802154/6lowpan_rtnl.c
> +++ b/net/ieee802154/6lowpan_rtnl.c
> @@ -166,11 +166,12 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb)
>   	return stat;
>   }
>   
> -static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
> +static int process_data(struct sk_buff **skb_inout, const struct ieee802154_hdr *hdr)
>   {
>   	u8 iphc0, iphc1;
>   	struct ieee802154_addr_sa sa, da;
>   	void *sap, *dap;
> +	struct sk_buff *skb = *skb_inout;
>   
>   	raw_dump_table(__func__, "raw skb data dump", skb->data, skb->len);
>   	/* at least two bytes will be used for the encoding */
> @@ -196,13 +197,12 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
>   	else
>   		dap = &da.hwaddr;
>   
> -	return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
> +	return lowpan_process_data(skb_inout, skb->dev, sap, sa.addr_type,
>   				   IEEE802154_ADDR_LEN, dap, da.addr_type,
>   				   IEEE802154_ADDR_LEN, iphc0, iphc1,
>   				   lowpan_give_skb_to_devices);
>   
>   drop:
> -	kfree_skb(skb);
>   	return -EINVAL;
>   }
>   
> @@ -485,14 +485,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
>   	} else {
>   		switch (skb->data[0] & 0xe0) {
>   		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
> -			ret = process_data(skb, &hdr);
> +			ret = process_data(&skb, &hdr);
>   			if (ret == NET_RX_DROP)
>   				goto drop;
>   			break;
>   		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
>   			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
>   			if (ret == 1) {
> -				ret = process_data(skb, &hdr);
> +				ret = process_data(&skb, &hdr);
>   				if (ret == NET_RX_DROP)
>   					goto drop;
>   			}
> @@ -500,7 +500,7 @@ 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 = process_data(skb, &hdr);
> +				ret = process_data(&skb, &hdr);
>   				if (ret == NET_RX_DROP)
>   					goto drop;
>   			}

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

this patch wasn't able to apply and now it doesn't apply either (because
lot of other changes). I thought you will resend a v3 and then I will
care about reviewing. Then putting Acked-by, then Marcel will put it on
the right repositories.

If you can, describe the changes in the commit msg (please note the 80
line width there).

This patch is also something for bluetooth (not bluetooth-next), it's a
bug fix. But this depends how Marcel will dealing with this issue. It's
a bugfix with a huge change of lines. I would base it on bluetooth,
because it's a bug fix.

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Marcel Holtmann]

Hi Alex,

> this patch wasn't able to apply and now it doesn't apply either (because
> lot of other changes). I thought you will resend a v3 and then I will
> care about reviewing. Then putting Acked-by, then Marcel will put it on
> the right repositories.
> 
> If you can, describe the changes in the commit msg (please note the 80
> line width there).
> 
> This patch is also something for bluetooth (not bluetooth-next), it's a
> bug fix. But this depends how Marcel will dealing with this issue. It's
> a bugfix with a huge change of lines. I would base it on bluetooth,
> because it's a bug fix.

simplifying something is not for -stable or a bug fix. If there is a memory leak, then fix that memory leak first in a separate patch. The rules are pretty clear here. Only bug fixes after the merge window has closed.

Regards

Marcel

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

On 21/08/14 14:24, Marcel Holtmann wrote:
> Hi Alex,
>
>> this patch wasn't able to apply and now it doesn't apply either (because
>> lot of other changes). I thought you will resend a v3 and then I will
>> care about reviewing. Then putting Acked-by, then Marcel will put it on
>> the right repositories.
>>
>> If you can, describe the changes in the commit msg (please note the 80
>> line width there).
>>
>> This patch is also something for bluetooth (not bluetooth-next), it's a
>> bug fix. But this depends how Marcel will dealing with this issue. It's
>> a bugfix with a huge change of lines. I would base it on bluetooth,
>> because it's a bug fix.
> simplifying something is not for -stable or a bug fix. If there is a memory leak, then fix that memory leak first in a separate patch. The rules are pretty clear here. Only bug fixes after the merge window has closed.
>
> Regards
>
> Marcel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

I'll respin and include the memory leak fix and this patch and a couple 
of others I have and send as a series to bluetooth.  What bluetooth git 
repository should I base the series on?

- Martin.

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

On Wed, Aug 27, 2014 at 09:49:49PM +0100, Martin Townsend wrote:
> Hi,
> 
> I'll respin and include the memory leak fix and this patch and a couple of
> others I have and send as a series to bluetooth.  What bluetooth git
> repository should I base the series on?

Okay,

net-next is open. That means the merge window has closed. Fix the memory
leak in a separate patch based on bluetooth at first of your series. New
features based on bluetooth-next, if these depends on each other write
this after the "---" in your commit msg.


There is also a third solution to send bug fixes to -stable, for the
stable kernel release. Forget the -stable releases, there is too much
stuff which don't work and the code differs too much. In future we can
do that.


What bluetooth and bluetooth-next means is:

bluetooth	- current kernel release
bluetooth-next	- next kernel release

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

On Thu, Aug 28, 2014 at 06:47:33AM +0200, Alexander Aring wrote:
> Hi Martin,
> 
> On Wed, Aug 27, 2014 at 09:49:49PM +0100, Martin Townsend wrote:
> > Hi,
> > 
> > I'll respin and include the memory leak fix and this patch and a couple of
> > others I have and send as a series to bluetooth.  What bluetooth git
> > repository should I base the series on?
> 
> Okay,
> 
> net-next is open. That means the merge window has closed. Fix the memory
> leak in a separate patch based on bluetooth at first of your series. New
> features based on bluetooth-next, if these depends on each other write
> this after the "---" in your commit msg.
> 

here I mean if a bluetooth-next (new feature) depends on bluetooth (bug
fix), then write it in the new feature commit msg after the "---" lines.

The rest will do the maintainer, some or later it will be in the
repository of bluetooth-next.


Marcel, this is okay for you?

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

On Wed, Aug 27, 2014 at 09:49:49PM +0100, Martin Townsend wrote:
> On 21/08/14 14:24, Marcel Holtmann wrote:
> >Hi Alex,
> >
> >>this patch wasn't able to apply and now it doesn't apply either (because
> >>lot of other changes). I thought you will resend a v3 and then I will
> >>care about reviewing. Then putting Acked-by, then Marcel will put it on
> >>the right repositories.
> >>
> >>If you can, describe the changes in the commit msg (please note the 80
> >>line width there).
> >>
> >>This patch is also something for bluetooth (not bluetooth-next), it's a
> >>bug fix. But this depends how Marcel will dealing with this issue. It's
> >>a bugfix with a huge change of lines. I would base it on bluetooth,
> >>because it's a bug fix.
> >simplifying something is not for -stable or a bug fix. If there is a memory leak, then fix that memory leak first in a separate patch. The rules are pretty clear here. Only bug fixes after the merge window has closed.
> >
> >Regards
> >
> >Marcel
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Hi,
> 
> I'll respin and include the memory leak fix and this patch and a couple of
> others I have and send as a series to bluetooth.  What bluetooth git
> repository should I base the series on?
> 

What's the state about to fix this bad issue? :-)

I didn't saw any new patches because of this.

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

On 08/09/14 11:40, Alexander Aring wrote:
> Hi Martin,
>
> On Wed, Aug 27, 2014 at 09:49:49PM +0100, Martin Townsend wrote:
>> On 21/08/14 14:24, Marcel Holtmann wrote:
>>> Hi Alex,
>>>
>>>> this patch wasn't able to apply and now it doesn't apply either (because
>>>> lot of other changes). I thought you will resend a v3 and then I will
>>>> care about reviewing. Then putting Acked-by, then Marcel will put it on
>>>> the right repositories.
>>>>
>>>> If you can, describe the changes in the commit msg (please note the 80
>>>> line width there).
>>>>
>>>> This patch is also something for bluetooth (not bluetooth-next), it's a
>>>> bug fix. But this depends how Marcel will dealing with this issue. It's
>>>> a bugfix with a huge change of lines. I would base it on bluetooth,
>>>> because it's a bug fix.
>>> simplifying something is not for -stable or a bug fix. If there is a memory leak, then fix that memory leak first in a separate patch. The rules are pretty clear here. Only bug fixes after the merge window has closed.
>>>
>>> Regards
>>>
>>> Marcel
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Hi,
>>
>> I'll respin and include the memory leak fix and this patch and a couple of
>> others I have and send as a series to bluetooth.  What bluetooth git
>> repository should I base the series on?
>>
> What's the state about to fix this bad issue? :-)
>
> I didn't saw any new patches because of this.
>
> - Alex
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sorry for the delay, been on hols for a few days, I will create the 
patches tomorrow if I get time.

I have also implemented Generic Header Compression[0] which is still in 
draft at the moment but I can't imagine it will change much before being 
released.  I'm going to integrate it into our linux repository this 
week.  Would you be interested in putting it in linux-wpan or 
linux-wpan-next or would you prefer to wait until it gets it RFC status?

[0] http://tools.ietf.org/html/draft-ietf-6lo-ghc-03

- Martin.

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

On Mon, Sep 08, 2014 at 07:13:02PM +0100, Martin Townsend wrote:
...
> >>Hi,
> >>
> >>I'll respin and include the memory leak fix and this patch and a couple of
> >>others I have and send as a series to bluetooth.  What bluetooth git
> >>repository should I base the series on?
> >>
> >What's the state about to fix this bad issue? :-)
> >
> >I didn't saw any new patches because of this.
> >
> >- Alex
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Sorry for the delay, been on hols for a few days, I will create the patches
> tomorrow if I get time.
> 
> I have also implemented Generic Header Compression[0] which is still in
> draft at the moment but I can't imagine it will change much before being
> released.  I'm going to integrate it into our linux repository this week.
> Would you be interested in putting it in linux-wpan or linux-wpan-next or
> would you prefer to wait until it gets it RFC status?
> 

no, for me it's okay to have this mainline. I heard that a draft becomes
no RFC when nobody implements it.

But another point, I want to avoid that everything is putted into the
iphc file for next header compression. Did you saw the patches for the
next header compression layer?

This is a layer to separate next header implementation, like IPv6 it
does for next header [0]. [0] shows the registration of the next header
protocols.

We should have something similar and I post some months a RFC about
that. I got some review notes and need to rework it, working branch is
at [1].

Also I added that we drop packets for IPv6 Header Extensions, described
at [2]. We currently no support this kind of next header compression and
if we don't check on it, we send garbage to IPv6 layer. (That's of
course one more argument to implement draft next header compression
formats).


For now, we should make it like this:

- First you send patches for the fixes.
- Then I will send patches for the nhc layer.
- You send patches to adding the your next header compression formats.
  based on the nhc layer.

Is that okay for you? It should easily for you to add your header compression
formats. (Filling some struct with macro and implement some callbacks).


This is all related stuff to the GENERIC 6LOWPAN branch, all which
belongs to IPHC and IPHC contains next header compression. As
conclusion this should be go to bluetooth/bluetooth-next.

Bug fixes to bluetooth, new features to bluetooth-next.



btw:
I working on the 802.15.4 rework. I am sure you know that I want to make
a rework of the 802.15.4 stack. Very busy at the moment, but this should
not affect the net/6lowpan branch. Only the net/ieee802.15.4/6lowpan.

I will take some time, so after you send fixes for this I will rebase/rework
the nhc layer patches to try to bring it mainline. Then you can make your work
on it.

- Alex

[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/af_inet6.c?id=refs/tags/v3.17-rc4#n822
[1] https://github.com/linux-wpan/linux-wpan-next/tree/nhc_layer
[2] http://tools.ietf.org/html/rfc6282#section-4.2

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

On Mon, Sep 08, 2014 at 08:36:52PM +0200, Alexander Aring wrote:
> Hi Martin,
> 
> On Mon, Sep 08, 2014 at 07:13:02PM +0100, Martin Townsend wrote:
> ...
> > >>Hi,
> > >>
> > >>I'll respin and include the memory leak fix and this patch and a couple of
> > >>others I have and send as a series to bluetooth.  What bluetooth git
> > >>repository should I base the series on?
> > >>
> > >What's the state about to fix this bad issue? :-)
> > >
> > >I didn't saw any new patches because of this.
> > >
> > >- Alex
> > >--
> > >To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> > >the body of a message to majordomo@vger.kernel.org
> > >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > Sorry for the delay, been on hols for a few days, I will create the patches
> > tomorrow if I get time.
> > 
> > I have also implemented Generic Header Compression[0] which is still in
> > draft at the moment but I can't imagine it will change much before being
> > released.  I'm going to integrate it into our linux repository this week.
> > Would you be interested in putting it in linux-wpan or linux-wpan-next or
> > would you prefer to wait until it gets it RFC status?
> > 
> 
> no, for me it's okay to have this mainline. I heard that a draft becomes
> no RFC when nobody implements it.
> 

I thought more about that, you mean the receiving part only? So the
uncompression. The point is that we don't have no interface for an user
that can decide if he like to use UDP compression like RFC 6282 or UDP
compression like GHC. This is only relevant for the transmit part. So
compression is optionally. (We should have some interface to make this
configurable by user -> adding this to the nhc layer, later).

On the uncompression part, means the receiving part we can support both.
UDP RFC 6282 or UDP like GHC, the next header id value should be
different there. That means currently we can receive every packets but
transmit only RFC6282 compression formats.

So for receiving this, it's okay. But for compression, since we don't
have some interface to make this configurable we should use RFC 6282.


Same opinion here? We can talk about that point.

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Hi Alex,

On 08/09/14 19:36, Alexander Aring wrote:
> Hi Martin,
>
> On Mon, Sep 08, 2014 at 07:13:02PM +0100, Martin Townsend wrote:
> ...
>>>> Hi,
>>>>
>>>> I'll respin and include the memory leak fix and this patch and a couple of
>>>> others I have and send as a series to bluetooth.  What bluetooth git
>>>> repository should I base the series on?
>>>>
>>> What's the state about to fix this bad issue? :-)
>>>
>>> I didn't saw any new patches because of this.
>>>
>>> - Alex
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Sorry for the delay, been on hols for a few days, I will create the patches
>> tomorrow if I get time.
>>
>> I have also implemented Generic Header Compression[0] which is still in
>> draft at the moment but I can't imagine it will change much before being
>> released.  I'm going to integrate it into our linux repository this week.
>> Would you be interested in putting it in linux-wpan or linux-wpan-next or
>> would you prefer to wait until it gets it RFC status?
>>
> no, for me it's okay to have this mainline. I heard that a draft becomes
> no RFC when nobody implements it.
>
> But another point, I want to avoid that everything is putted into the
> iphc file for next header compression. Did you saw the patches for the
> next header compression layer?
yes I did and would like to add GHC to this new framework so will wait until it's included.
>
> This is a layer to separate next header implementation, like IPv6 it
> does for next header [0]. [0] shows the registration of the next header
> protocols.
>
> We should have something similar and I post some months a RFC about
> that. I got some review notes and need to rework it, working branch is
> at [1].
>
> Also I added that we drop packets for IPv6 Header Extensions, described
> at [2]. We currently no support this kind of next header compression and
> if we don't check on it, we send garbage to IPv6 layer. (That's of
> course one more argument to implement draft next header compression
> formats).
>
>
> For now, we should make it like this:
>
> - First you send patches for the fixes.
> - Then I will send patches for the nhc layer.
> - You send patches to adding the your next header compression formats.
>   based on the nhc layer.
>
> Is that okay for you? It should easily for you to add your header compression
> formats. (Filling some struct with macro and implement some callbacks).
ok for me.
>
> This is all related stuff to the GENERIC 6LOWPAN branch, all which
> belongs to IPHC and IPHC contains next header compression. As
> conclusion this should be go to bluetooth/bluetooth-next.
>
> Bug fixes to bluetooth, new features to bluetooth-next.
>
ok.
>
> btw:
> I working on the 802.15.4 rework. I am sure you know that I want to make
> a rework of the 802.15.4 stack. Very busy at the moment, but this should
> not affect the net/6lowpan branch. Only the net/ieee802.15.4/6lowpan.
>
> I will take some time, so after you send fixes for this I will rebase/rework
> the nhc layer patches to try to bring it mainline. Then you can make your work
> on it.
ok.
>
> - Alex
>
> [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/af_inet6.c?id=refs/tags/v3.17-rc4#n822
> [1] https://github.com/linux-wpan/linux-wpan-next/tree/nhc_layer
> [2] http://tools.ietf.org/html/rfc6282#section-4.2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Hi Alex,

On 08/09/14 19:55, Alexander Aring wrote:
> Hi Martin,
>
> On Mon, Sep 08, 2014 at 08:36:52PM +0200, Alexander Aring wrote:
>> Hi Martin,
>>
>> On Mon, Sep 08, 2014 at 07:13:02PM +0100, Martin Townsend wrote:
>> ...
>>>>> Hi,
>>>>>
>>>>> I'll respin and include the memory leak fix and this patch and a couple of
>>>>> others I have and send as a series to bluetooth.  What bluetooth git
>>>>> repository should I base the series on?
>>>>>
>>>> What's the state about to fix this bad issue? :-)
>>>>
>>>> I didn't saw any new patches because of this.
>>>>
>>>> - Alex
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Sorry for the delay, been on hols for a few days, I will create the patches
>>> tomorrow if I get time.
>>>
>>> I have also implemented Generic Header Compression[0] which is still in
>>> draft at the moment but I can't imagine it will change much before being
>>> released.  I'm going to integrate it into our linux repository this week.
>>> Would you be interested in putting it in linux-wpan or linux-wpan-next or
>>> would you prefer to wait until it gets it RFC status?
>>>
>> no, for me it's okay to have this mainline. I heard that a draft becomes
>> no RFC when nobody implements it.
>>
> I thought more about that, you mean the receiving part only? So the
> uncompression. The point is that we don't have no interface for an user
> that can decide if he like to use UDP compression like RFC 6282 or UDP
> compression like GHC. This is only relevant for the transmit part. So
> compression is optionally. (We should have some interface to make this
> configurable by user -> adding this to the nhc layer, later).
I've implemented compression and decompression. You are right in that we need a mechanism of configuring what gets compressed by what method.
> On the uncompression part, means the receiving part we can support both.
> UDP RFC 6282 or UDP like GHC, the next header id value should be
> different there. That means currently we can receive every packets but
> transmit only RFC6282 compression formats.
>
> So for receiving this, it's okay. But for compression, since we don't
> have some interface to make this configurable we should use RFC 6282.
So I will ensure UDP is compressed by 6282.  Then I was going to start out by just compressing ICMPv6 with GHC and monitor how much data is saved by using GHC.  Later on we will implement a mechanism of configuring what gets compressed and by which compression method.

The GHC spec states that a device indicates it's GHC capability using a 6LoWPAN Capability Indication Option (6CIO), this is an ND option.  As far as I can see there is no type assigned yet by IANA so I was wondering if we should have this as an experimental configuration item in the kernel?
>
> Same opinion here? We can talk about that point.
>
> - Alex

- Martin

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

On Tue, Sep 09, 2014 at 10:28:36AM +0100, Martin Townsend wrote:
...
> > I thought more about that, you mean the receiving part only? So the
> > uncompression. The point is that we don't have no interface for an user
> > that can decide if he like to use UDP compression like RFC 6282 or UDP
> > compression like GHC. This is only relevant for the transmit part. So
> > compression is optionally. (We should have some interface to make this
> > configurable by user -> adding this to the nhc layer, later).
> I've implemented compression and decompression. You are right in that we need a mechanism of configuring what gets compressed by what method.

ok. But how we deal with that currently with GHC UDP and UDP RFC6282
compression. We can't not support both compression methods. 

btw. how we should call it now? Uncompression or decompression, I can
also name the callbacks to decompression. I am not a native speaker so
I will ask you which is better now. :-)

> > On the uncompression part, means the receiving part we can support both.
> > UDP RFC 6282 or UDP like GHC, the next header id value should be
> > different there. That means currently we can receive every packets but
> > transmit only RFC6282 compression formats.
> >
> > So for receiving this, it's okay. But for compression, since we don't
> > have some interface to make this configurable we should use RFC 6282.
> So I will ensure UDP is compressed by 6282.  Then I was going to start out by just compressing ICMPv6 with GHC and monitor how much data is saved by using GHC.  Later on we will implement a mechanism of configuring what gets compressed and by which compression method.

Okay, you mean that you will leave UDP compression by 6282 but insert a
receive handling (decompression) for UDP GHC?

RFC6282 doesn't describe any compression/decompression(or uncompression)
format for ICMPv6, so we could handle there compression and
uncompression. I understand now you did it that way, or?

About the mechanism by user:

There are several ways about to do it from userspace. I know now sysfs
or netlink. Do you have already some idea how you want to make this
configurable by user?


btw.
This reminds me a little bit like setting led trigger type, (blink,
heartbeat, mmc, net, etc...). This is done by sysfs. 

> 
> The GHC spec states that a device indicates it's GHC capability using a 6LoWPAN Capability Indication Option (6CIO), this is an ND option.  As far as I can see there is no type assigned yet by IANA so I was wondering if we should have this as an experimental configuration item in the kernel?

Yes, please make a bool into net/6lowpan/Kconfig and add support for
drafts only if selected. 

In code you simple need to use "if (IS_ENABLED(CONFIG_FOO))" to
registration the nhc format into the nhc framework/layer or not.

Replace FOO with a propber 6LOWPAN_NHC_DRAFTS or something else. You can
write in the help what exactly this means.

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

On Tue, Sep 09, 2014 at 11:46:52AM +0200, Alexander Aring wrote:
> Hi Martin,
> 
> On Tue, Sep 09, 2014 at 10:28:36AM +0100, Martin Townsend wrote:
> ...
> > > I thought more about that, you mean the receiving part only? So the
> > > uncompression. The point is that we don't have no interface for an user
> > > that can decide if he like to use UDP compression like RFC 6282 or UDP
> > > compression like GHC. This is only relevant for the transmit part. So
> > > compression is optionally. (We should have some interface to make this
> > > configurable by user -> adding this to the nhc layer, later).
> > I've implemented compression and decompression. You are right in that we need a mechanism of configuring what gets compressed by what method.
> 
> ok. But how we deal with that currently with GHC UDP and UDP RFC6282
> compression. We can't not support both compression methods. 
> 
> btw. how we should call it now? Uncompression or decompression, I can
> also name the callbacks to decompression. I am not a native speaker so
> I will ask you which is better now. :-)
> 
> > > On the uncompression part, means the receiving part we can support both.
> > > UDP RFC 6282 or UDP like GHC, the next header id value should be
> > > different there. That means currently we can receive every packets but
> > > transmit only RFC6282 compression formats.
> > >
> > > So for receiving this, it's okay. But for compression, since we don't
> > > have some interface to make this configurable we should use RFC 6282.
> > So I will ensure UDP is compressed by 6282.  Then I was going to start out by just compressing ICMPv6 with GHC and monitor how much data is saved by using GHC.  Later on we will implement a mechanism of configuring what gets compressed and by which compression method.
> 
> Okay, you mean that you will leave UDP compression by 6282 but insert a
> receive handling (decompression) for UDP GHC?
> 
> RFC6282 doesn't describe any compression/decompression(or uncompression)
> format for ICMPv6, so we could handle there compression and
> uncompression. I understand now you did it that way, or?
> 
> About the mechanism by user:
> 
> There are several ways about to do it from userspace. I know now sysfs
> or netlink. Do you have already some idea how you want to make this
> configurable by user?
> 
> 
> btw.
> This reminds me a little bit like setting led trigger type, (blink,
> heartbeat, mmc, net, etc...). This is done by sysfs. 
> 
> > 
> > The GHC spec states that a device indicates it's GHC capability using a 6LoWPAN Capability Indication Option (6CIO), this is an ND option.  As far as I can see there is no type assigned yet by IANA so I was wondering if we should have this as an experimental configuration item in the kernel?
> 
> Yes, please make a bool into net/6lowpan/Kconfig and add support for
> drafts only if selected. 
> 
> In code you simple need to use "if (IS_ENABLED(CONFIG_FOO))" to
> registration the nhc format into the nhc framework/layer or not.

Forget this one, I failed here. We should complete get out the draft
implementations when it's not selected by buildsystem.

We need something like this here. "Adding dummy functions with ifdefs".

#idef CONFIG_FOO
/* real prototype */
int lowpan_nhc_add_fancy_format(foo);
...
#else
static inline int lowpan_nhc_add_fancy_format(foo) { }
...
#endif

also according to Makefile that we don't build the files which
implements "lowpan_nhc_add_fancy_format".

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Hi Alex,

On 09/09/14 10:46, Alexander Aring wrote:
> Hi Martin,
>
> On Tue, Sep 09, 2014 at 10:28:36AM +0100, Martin Townsend wrote:
> ...
>>> I thought more about that, you mean the receiving part only? So the
>>> uncompression. The point is that we don't have no interface for an user
>>> that can decide if he like to use UDP compression like RFC 6282 or UDP
>>> compression like GHC. This is only relevant for the transmit part. So
>>> compression is optionally. (We should have some interface to make this
>>> configurable by user -> adding this to the nhc layer, later).
>> I've implemented compression and decompression. You are right in that we need a mechanism of configuring what gets compressed by what method.
> ok. But how we deal with that currently with GHC UDP and UDP RFC6282
> compression. We can't not support both compression methods. 
>
> btw. how we should call it now? Uncompression or decompression, I can
> also name the callbacks to decompression. I am not a native speaker so
> I will ask you which is better now. :-)
As an English speaker I have to admit I don't know.  Here's one link I found on the subject
http://english.stackexchange.com/questions/56480/difference-between-uncompress-and-decompress
to confuse you even more :)

>
>>> On the uncompression part, means the receiving part we can support both.
>>> UDP RFC 6282 or UDP like GHC, the next header id value should be
>>> different there. That means currently we can receive every packets but
>>> transmit only RFC6282 compression formats.
>>>
>>> So for receiving this, it's okay. But for compression, since we don't
>>> have some interface to make this configurable we should use RFC 6282.
>> So I will ensure UDP is compressed by 6282.  Then I was going to start out by just compressing ICMPv6 with GHC and monitor how much data is saved by using GHC.  Later on we will implement a mechanism of configuring what gets compressed and by which compression method.
> Okay, you mean that you will leave UDP compression by 6282 but insert a
> receive handling (decompression) for UDP GHC?
>
> RFC6282 doesn't describe any compression/decompression(or uncompression)
> format for ICMPv6, so we could handle there compression and
> uncompression. I understand now you did it that way, or?
For the moment I will assume all ICMPv6 traffic is compressed and decompressed with GHC as this will be the only Next Header Compression format.  In future we need something better.  We also need a method of knowing what compression formats a device supports.  I can see a list of compression formats which could also be a list by protocol.  Then when sending to a device you would select the highest ranking supported compression format for that device.
>
> About the mechanism by user:
>
> There are several ways about to do it from userspace. I know now sysfs
> or netlink. Do you have already some idea how you want to make this
> configurable by user?
>
>
> btw.
> This reminds me a little bit like setting led trigger type, (blink,
> heartbeat, mmc, net, etc...). This is done by sysfs. 
>
>> The GHC spec states that a device indicates it's GHC capability using a 6LoWPAN Capability Indication Option (6CIO), this is an ND option.  As far as I can see there is no type assigned yet by IANA so I was wondering if we should have this as an experimental configuration item in the kernel?
> Yes, please make a bool into net/6lowpan/Kconfig and add support for
> drafts only if selected. 
>
> In code you simple need to use "if (IS_ENABLED(CONFIG_FOO))" to
> registration the nhc format into the nhc framework/layer or not.
>
> Replace FOO with a propber 6LOWPAN_NHC_DRAFTS or something else. You can
> write in the help what exactly this means.
>
> - Alex

- Martin

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Martin,

On Tue, Sep 09, 2014 at 11:17:15AM +0100, Martin Townsend wrote:
> Hi Alex,
> 
> On 09/09/14 10:46, Alexander Aring wrote:
> > Hi Martin,
> >
> > On Tue, Sep 09, 2014 at 10:28:36AM +0100, Martin Townsend wrote:
> > ...
> >>> I thought more about that, you mean the receiving part only? So the
> >>> uncompression. The point is that we don't have no interface for an user
> >>> that can decide if he like to use UDP compression like RFC 6282 or UDP
> >>> compression like GHC. This is only relevant for the transmit part. So
> >>> compression is optionally. (We should have some interface to make this
> >>> configurable by user -> adding this to the nhc layer, later).
> >> I've implemented compression and decompression. You are right in that we need a mechanism of configuring what gets compressed by what method.
> > ok. But how we deal with that currently with GHC UDP and UDP RFC6282
> > compression. We can't not support both compression methods. 
> >
> > btw. how we should call it now? Uncompression or decompression, I can
> > also name the callbacks to decompression. I am not a native speaker so
> > I will ask you which is better now. :-)
> As an English speaker I have to admit I don't know.  Here's one link I found on the subject
> http://english.stackexchange.com/questions/56480/difference-between-uncompress-and-decompress
> to confuse you even more :)
> 
> >
> >>> On the uncompression part, means the receiving part we can support both.
> >>> UDP RFC 6282 or UDP like GHC, the next header id value should be
> >>> different there. That means currently we can receive every packets but
> >>> transmit only RFC6282 compression formats.
> >>>
> >>> So for receiving this, it's okay. But for compression, since we don't
> >>> have some interface to make this configurable we should use RFC 6282.
> >> So I will ensure UDP is compressed by 6282.  Then I was going to start out by just compressing ICMPv6 with GHC and monitor how much data is saved by using GHC.  Later on we will implement a mechanism of configuring what gets compressed and by which compression method.
> > Okay, you mean that you will leave UDP compression by 6282 but insert a
> > receive handling (decompression) for UDP GHC?
> >
> > RFC6282 doesn't describe any compression/decompression(or uncompression)
> > format for ICMPv6, so we could handle there compression and
> > uncompression. I understand now you did it that way, or?
> For the moment I will assume all ICMPv6 traffic is compressed and decompressed with GHC as this will be the only Next Header Compression format.  In future we need something better.  We also need a method of knowing what compression formats a device supports.  I can see a list of compression formats which could also be a list by protocol.  Then when sending to a device you would select the highest ranking supported compression format for that device.

Is "what compression methods like a device to use" part of any RFC? Is there
something which I don't know? I mean, okay you can do that in any
application layer in userspace. But I don't see that we need something
like this in kernelspace. I know there is no suggestion that you want to
implement something like this in kernelspace, but I want to clarify this.

Application layer in userspace means, use some own coap(or whatever) based
protocol and setup the right compressions via userspace by some application.

- Alex

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Martin Townsend]

Hi Alex,

On 09/09/14 11:47, Alexander Aring wrote:
> Hi Martin,
>
> On Tue, Sep 09, 2014 at 11:17:15AM +0100, Martin Townsend wrote:
>> Hi Alex,
>>
>> On 09/09/14 10:46, Alexander Aring wrote:
>>> Hi Martin,
>>>
>>> On Tue, Sep 09, 2014 at 10:28:36AM +0100, Martin Townsend wrote:
>>> ...
>>>>> I thought more about that, you mean the receiving part only? So the
>>>>> uncompression. The point is that we don't have no interface for an user
>>>>> that can decide if he like to use UDP compression like RFC 6282 or UDP
>>>>> compression like GHC. This is only relevant for the transmit part. So
>>>>> compression is optionally. (We should have some interface to make this
>>>>> configurable by user -> adding this to the nhc layer, later).
>>>> I've implemented compression and decompression. You are right in that we need a mechanism of configuring what gets compressed by what method.
>>> ok. But how we deal with that currently with GHC UDP and UDP RFC6282
>>> compression. We can't not support both compression methods. 
>>>
>>> btw. how we should call it now? Uncompression or decompression, I can
>>> also name the callbacks to decompression. I am not a native speaker so
>>> I will ask you which is better now. :-)
>> As an English speaker I have to admit I don't know.  Here's one link I found on the subject
>> http://english.stackexchange.com/questions/56480/difference-between-uncompress-and-decompress
>> to confuse you even more :)
>>
>>>>> On the uncompression part, means the receiving part we can support both.
>>>>> UDP RFC 6282 or UDP like GHC, the next header id value should be
>>>>> different there. That means currently we can receive every packets but
>>>>> transmit only RFC6282 compression formats.
>>>>>
>>>>> So for receiving this, it's okay. But for compression, since we don't
>>>>> have some interface to make this configurable we should use RFC 6282.
>>>> So I will ensure UDP is compressed by 6282.  Then I was going to start out by just compressing ICMPv6 with GHC and monitor how much data is saved by using GHC.  Later on we will implement a mechanism of configuring what gets compressed and by which compression method.
>>> Okay, you mean that you will leave UDP compression by 6282 but insert a
>>> receive handling (decompression) for UDP GHC?
>>>
>>> RFC6282 doesn't describe any compression/decompression(or uncompression)
>>> format for ICMPv6, so we could handle there compression and
>>> uncompression. I understand now you did it that way, or?
>> For the moment I will assume all ICMPv6 traffic is compressed and decompressed with GHC as this will be the only Next Header Compression format.  In future we need something better.  We also need a method of knowing what compression formats a device supports.  I can see a list of compression formats which could also be a list by protocol.  Then when sending to a device you would select the highest ranking supported compression format for that device.
> Is "what compression methods like a device to use" part of any RFC? Is there
> something which I don't know? I mean, okay you can do that in any
> application layer in userspace. But I don't see that we need something
> like this in kernelspace. I know there is no suggestion that you want to
> implement something like this in kernelspace, but I want to clarify this.
>
> Application layer in userspace means, use some own coap(or whatever) based
> protocol and setup the right compressions via userspace by some application.

I don't know what would be implemented in user or kernel space it's just a general observation that we need some mechanism in the future, of knowing what capabilities a device we want to send to supports so we can select compression accordingly.


> - Alex

- Martin.

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Marcel Holtmann]

Hi Alex,

>> 
>> The GHC spec states that a device indicates it's GHC capability using a 6LoWPAN Capability Indication Option (6CIO), this is an ND option.  As far as I can see there is no type assigned yet by IANA so I was wondering if we should have this as an experimental configuration item in the kernel?
> 
> Yes, please make a bool into net/6lowpan/Kconfig and add support for
> drafts only if selected. 
> 
> In code you simple need to use "if (IS_ENABLED(CONFIG_FOO))" to
> registration the nhc format into the nhc framework/layer or not.
> 
> Replace FOO with a propber 6LOWPAN_NHC_DRAFTS or something else. You can
> write in the help what exactly this means.

or you just create /sys/kernel/debug/6lowpan/foo and use debugfs to toggle experimental options on/off at runtime. Especially if you are dealing with not yet assigned types, you can also have an entry that allows you to define the type.

Regards

Marcel

Re: [PATCH v2 bluetooth-next] Simplify lowpan receive path so skb is freed in lowpan_rcv when dropped.

[Alexander Aring]

Hi Marcel,

On Tue, Sep 09, 2014 at 06:44:56AM -0700, Marcel Holtmann wrote:
> Hi Alex,
> 
> >> 
> >> The GHC spec states that a device indicates it's GHC capability using a 6LoWPAN Capability Indication Option (6CIO), this is an ND option.  As far as I can see there is no type assigned yet by IANA so I was wondering if we should have this as an experimental configuration item in the kernel?
> > 
> > Yes, please make a bool into net/6lowpan/Kconfig and add support for
> > drafts only if selected. 
> > 
> > In code you simple need to use "if (IS_ENABLED(CONFIG_FOO))" to
> > registration the nhc format into the nhc framework/layer or not.
> > 
> > Replace FOO with a propber 6LOWPAN_NHC_DRAFTS or something else. You can
> > write in the help what exactly this means.
> 
> or you just create /sys/kernel/debug/6lowpan/foo and use debugfs to toggle experimental options on/off at runtime. Especially if you are dealing with not yet assigned types, you can also have an entry that allows you to define the type.
> 

yea, runtime changeable stuff is nice. I will try to add some debugfs
entry registration with the nhc framework/layer. Then Martin can add
an entry to enable/disable his experimental stuff.

Thanks.

- Alex