All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
@ 2014-07-30 14:55 Martin Townsend
  2014-07-30 14:55 ` [PATCH 1/2] Remove dev parameter from skb_delivery_cb in 6lowpan Martin Townsend
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Martin Townsend @ 2014-07-30 14:55 UTC (permalink / raw)
  To: linux-zigbee-devel, linux-bluetooth; +Cc: Martin Townsend

Currently it is up to the functions below lowpan_rcv to free the skb on error
    conditions.  This patch now removes all the UAPI error codes and process data
    now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
    the skb and return NET_RX_DROP.  This also fixes the problem where
    NET_RX_SUCCESS is returned on error

Martin Townsend (2):
  Remove dev parameter from skb_delivery_cb in 6lowpan.
  Change lowpan_rcv so skb is freed within function and fix return
    values.

 include/net/6lowpan.h         |  4 ++--
 net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
 net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
 net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
 4 files changed, 52 insertions(+), 54 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] Remove dev parameter from skb_delivery_cb in 6lowpan.
  2014-07-30 14:55 [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values Martin Townsend
@ 2014-07-30 14:55 ` Martin Townsend
  2014-07-30 14:55 ` [PATCH 2/2] Change lowpan_rcv so skb is freed within function and fix return values Martin Townsend
  2014-07-30 17:42 ` [PATCH 0/2] linux-wpan-next: lowpan_rcv - " Marcel Holtmann
  2 siblings, 0 replies; 12+ messages in thread
From: Martin Townsend @ 2014-07-30 14:55 UTC (permalink / raw)
  To: linux-zigbee-devel, linux-bluetooth; +Cc: Martin Townsend

This parameter is never used by any functions that are passed to
lowpan_process_data which uses this callback.

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

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index 79b530f..995cce86 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -422,7 +422,7 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
 	return skb->len + uncomp_header - ret;
 }
 
-typedef int (*skb_delivery_cb)(struct sk_buff *skb, struct net_device *dev);
+typedef int (*skb_delivery_cb)(struct sk_buff *skb);
 
 int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
 		const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index e82c9cc..b4bb27c 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -195,7 +195,7 @@ static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
 	raw_dump_table(__func__, "raw skb data dump before receiving",
 		       new->data, new->len);
 
-	stat = deliver_skb(new, dev);
+	stat = deliver_skb(new);
 
 	kfree_skb(new);
 
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 5a7f81d..f5df93f 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -197,7 +197,7 @@ static struct lowpan_dev *lookup_dev(struct l2cap_conn *conn)
 	return dev;
 }
 
-static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
+static int give_skb_to_upper(struct sk_buff *skb)
 {
 	struct sk_buff *skb_cp;
 	int ret;
@@ -283,7 +283,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
 		skb_reset_network_header(local_skb);
 		skb_set_transport_header(local_skb, sizeof(struct ipv6hdr));
 
-		if (give_skb_to_upper(local_skb, dev) != NET_RX_SUCCESS) {
+		if (give_skb_to_upper(local_skb) != NET_RX_SUCCESS) {
 			kfree_skb(local_skb);
 			goto drop;
 		}
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 016b77e..637caa6 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -143,8 +143,7 @@ static int lowpan_header_create(struct sk_buff *skb, struct net_device *dev,
 			type, (void *)&da, (void *)&sa, 0);
 }
 
-static int lowpan_give_skb_to_devices(struct sk_buff *skb,
-				      struct net_device *dev)
+static int lowpan_give_skb_to_devices(struct sk_buff *skb)
 {
 	struct lowpan_dev_record *entry;
 	struct sk_buff *skb_cp;
@@ -484,7 +483,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 		/* Pull off the 1-byte of 6lowpan header. */
 		skb_pull(skb, 1);
 
-		ret = lowpan_give_skb_to_devices(skb, NULL);
+		ret = lowpan_give_skb_to_devices(skb);
 		if (ret == NET_RX_DROP)
 			goto drop;
 	} else {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] Change lowpan_rcv so skb is freed within function and fix return values.
  2014-07-30 14:55 [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values Martin Townsend
  2014-07-30 14:55 ` [PATCH 1/2] Remove dev parameter from skb_delivery_cb in 6lowpan Martin Townsend
@ 2014-07-30 14:55 ` Martin Townsend
  2014-07-30 17:42 ` [PATCH 0/2] linux-wpan-next: lowpan_rcv - " Marcel Holtmann
  2 siblings, 0 replies; 12+ messages in thread
From: Martin Townsend @ 2014-07-30 14:55 UTC (permalink / raw)
  To: linux-zigbee-devel, linux-bluetooth; +Cc: Martin Townsend

Currently it is up to the functions below lowpan_rcv to free the skb on error
conditions.  This patch now removes all the UAPI error codes and process data
now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
the skb and return NET_RX_DROP.  This also fixes the problem where
NET_RX_SUCCESS is returned on error

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

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index 995cce86..561defe 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -424,7 +424,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 b4bb27c..61b5206 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -119,17 +119,17 @@ static int uncompress_addr(struct sk_buff *skb,
 			break;
 		default:
 			pr_debug("Invalid addr_type set\n");
-			return -EINVAL;
+			return -1;
 		}
 		break;
 	default:
 		pr_debug("Invalid address mode value: 0x%x\n", address_mode);
-		return -EINVAL;
+		return -1;
 	}
 
 	if (fail) {
 		pr_debug("Failed to fetch skb data\n");
-		return -EIO;
+		return -1;
 	}
 
 	raw_dump_inline(NULL, "Reconstructed ipv6 addr is",
@@ -158,10 +158,10 @@ static int uncompress_context_based_src_addr(struct sk_buff *skb,
 	case LOWPAN_IPHC_ADDR_03:
 		/* TODO */
 		netdev_warn(skb->dev, "SAM value 0x%x not supported\n", sam);
-		return -EINVAL;
+		return -1;
 	default:
 		pr_debug("Invalid sam value: 0x%x\n", sam);
-		return -EINVAL;
+		return -1;
 	}
 
 	raw_dump_inline(NULL,
@@ -179,10 +179,10 @@ 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;
+		return -1;
+
+	kfree_skb(skb);
 
 	skb_push(new, sizeof(struct ipv6hdr));
 	skb_reset_network_header(new);
@@ -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 == -1)
+		stat = NET_RX_DROP;
 
 	kfree_skb(new);
 
@@ -245,12 +247,12 @@ lowpan_uncompress_multicast_daddr(struct sk_buff *skb,
 		break;
 	default:
 		pr_debug("DAM value has a wrong value: 0x%x\n", dam);
-		return -EINVAL;
+		return -1;
 	}
 
 	if (fail) {
 		pr_debug("Failed to fetch skb data\n");
-		return -EIO;
+		return -1;
 	}
 
 	raw_dump_inline(NULL, "Reconstructed ipv6 multicast addr is",
@@ -329,13 +331,13 @@ uncompress_udp_header(struct sk_buff *skb, struct udphdr *uh)
 
 	return 0;
 err:
-	return -EINVAL;
+	return -1;
 }
 
 /* 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,
@@ -344,6 +346,7 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
 	struct ipv6hdr hdr = {};
 	u8 tmp, num_context = 0;
 	int err;
+	struct sk_buff *skb = *skb_inout;
 
 	raw_dump_table(__func__, "raw skb data dump uncompressed",
 		       skb->data, skb->len);
@@ -474,11 +477,10 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
 		 */
 		new = skb_copy_expand(skb, sizeof(struct udphdr),
 				      skb_tailroom(skb), GFP_ATOMIC);
-		kfree_skb(skb);
-
 		if (!new)
-			return -ENOMEM;
+			goto drop;
 
+		kfree_skb(skb);
 		skb = new;
 
 		skb_push(skb, sizeof(struct udphdr));
@@ -507,8 +509,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 -1;
 }
 EXPORT_SYMBOL_GPL(lowpan_process_data);
 
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index f5df93f..cf8290f 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -204,7 +204,7 @@ static int give_skb_to_upper(struct sk_buff *skb)
 
 	skb_cp = skb_copy(skb, GFP_ATOMIC);
 	if (!skb_cp)
-		return -ENOMEM;
+		return -1;
 
 	ret = netif_rx(skb_cp);
 	if (ret < 0) {
@@ -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,7 +246,7 @@ 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);
@@ -259,7 +260,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
 		    struct l2cap_chan *chan)
 {
 	struct sk_buff *local_skb;
-	int ret;
+	int ret = NET_RX_SUCCESS;
 
 	if (!netif_running(dev))
 		goto drop;
@@ -301,20 +302,18 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
 				goto drop;
 
 			ret = process_data(local_skb, dev, chan);
-			if (ret != NET_RX_SUCCESS)
+			if (ret == -1)
 				goto drop;
 
 			dev->stats.rx_bytes += skb->len;
 			dev->stats.rx_packets++;
-
-			kfree_skb(skb);
 			break;
 		default:
-			break;
+			goto drop;
 		}
 	}
 
-	return NET_RX_SUCCESS;
+	return ret;
 
 drop:
 	dev->stats.rx_dropped++;
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 637caa6..897d0fd 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -154,7 +154,7 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb)
 		if (lowpan_dev_info(entry->ldev)->real_dev == skb->dev) {
 			skb_cp = skb_copy(skb, GFP_ATOMIC);
 			if (!skb_cp) {
-				stat = -ENOMEM;
+				stat = -1;
 				break;
 			}
 
@@ -166,11 +166,13 @@ 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,14 +198,13 @@ 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;
+	return -1;
 }
 
 static int lowpan_set_address(struct net_device *dev, void *p)
@@ -484,37 +485,35 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb_pull(skb, 1);
 
 		ret = lowpan_give_skb_to_devices(skb);
-		if (ret == NET_RX_DROP)
-			goto drop;
 	} else {
 		switch (skb->data[0] & 0xe0) {
 		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
-			ret = process_data(skb, &hdr);
-			if (ret == NET_RX_DROP)
-				goto drop;
+			ret = process_data(&skb, &hdr);
 			break;
 		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
 			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
 			if (ret == 1) {
-				ret = process_data(skb, &hdr);
-				if (ret == NET_RX_DROP)
-					goto drop;
-			}
+				ret = process_data(&skb, &hdr);
+			} else {
+				ret = NET_RX_SUCCESS;
+ 			}
 			break;
 		case LOWPAN_DISPATCH_FRAGN:	/* next fragments headers */
 			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
 			if (ret == 1) {
-				ret = process_data(skb, &hdr);
-				if (ret == NET_RX_DROP)
-					goto drop;
-			}
+				ret = process_data(&skb, &hdr);
+			} else {
+				ret = NET_RX_SUCCESS;
+ 			}
 			break;
 		default:
-			break;
+			goto drop_skb;
 		}
 	}
 
-	return NET_RX_SUCCESS;
+	if (ret != -1)
+		return ret;
+
 drop_skb:
 	kfree_skb(skb);
 drop:
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
  2014-07-30 14:55 [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values Martin Townsend
  2014-07-30 14:55 ` [PATCH 1/2] Remove dev parameter from skb_delivery_cb in 6lowpan Martin Townsend
  2014-07-30 14:55 ` [PATCH 2/2] Change lowpan_rcv so skb is freed within function and fix return values Martin Townsend
@ 2014-07-30 17:42 ` Marcel Holtmann
  2014-07-30 18:27     ` Alexander Aring
  2 siblings, 1 reply; 12+ messages in thread
From: Marcel Holtmann @ 2014-07-30 17:42 UTC (permalink / raw)
  To: Martin Townsend, John W. Linville, Alexander Aring
  Cc: linux-zigbee-devel, BlueZ development, Martin Townsend,
	Network Development, linux-wireless@vger.kernel.org Wireless

Hi Martin,

> Currently it is up to the functions below lowpan_rcv to free the skb on error
>    conditions.  This patch now removes all the UAPI error codes and process data
>    now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
>    the skb and return NET_RX_DROP.  This also fixes the problem where
>    NET_RX_SUCCESS is returned on error
> 
> Martin Townsend (2):
>  Remove dev parameter from skb_delivery_cb in 6lowpan.
>  Change lowpan_rcv so skb is freed within function and fix return
>    values.
> 
> include/net/6lowpan.h         |  4 ++--
> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
> 4 files changed, 52 insertions(+), 54 deletions(-)

I can not take these patches at this point.

checking file net/ieee802154/6lowpan_rtnl.c
Hunk #1 FAILED at 143.
Hunk #2 succeeded at 480 (offset -4 lines).
1 out of 2 hunks FAILED

We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.

John, how confident are you with pulling net-next into wireless-next at this point in time?

Regards

Marcel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
@ 2014-07-30 18:27     ` Alexander Aring
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2014-07-30 18:27 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Martin Townsend, John W. Linville, linux-zigbee-devel,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless@vger.kernel.org Wireless

Hi Marcel,

On Wed, Jul 30, 2014 at 10:42:09AM -0700, Marcel Holtmann wrote:
> Hi Martin,
> 
> > Currently it is up to the functions below lowpan_rcv to free the skb on error
> >    conditions.  This patch now removes all the UAPI error codes and process data
> >    now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
> >    the skb and return NET_RX_DROP.  This also fixes the problem where
> >    NET_RX_SUCCESS is returned on error
> > 
> > Martin Townsend (2):
> >  Remove dev parameter from skb_delivery_cb in 6lowpan.
> >  Change lowpan_rcv so skb is freed within function and fix return
> >    values.
> > 
> > include/net/6lowpan.h         |  4 ++--
> > net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
> > net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
> > net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
> > 4 files changed, 52 insertions(+), 54 deletions(-)
> 
> I can not take these patches at this point.
> 
> checking file net/ieee802154/6lowpan_rtnl.c
> Hunk #1 FAILED at 143.
> Hunk #2 succeeded at 480 (offset -4 lines).
> 1 out of 2 hunks FAILED
> 
> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
> 

I already told Martin that these patches should be based on bluetooth
(better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
are also some known other issues in the current implementation). I don't
think that the current implementation is used in a real environment.

I get this patch-series in a resend of this series, after Martin
subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
need some time for review and he want to resend them tomorrow or friday.
The resend should based on bluetooth-next then, if this is okay for you.

I will cc bluetooth mailinglist in my review notes.

- Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
@ 2014-07-30 18:27     ` Alexander Aring
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2014-07-30 18:27 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Martin Townsend, John W. Linville,
	linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Wireless

Hi Marcel,

On Wed, Jul 30, 2014 at 10:42:09AM -0700, Marcel Holtmann wrote:
> Hi Martin,
> 
> > Currently it is up to the functions below lowpan_rcv to free the skb on error
> >    conditions.  This patch now removes all the UAPI error codes and process data
> >    now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
> >    the skb and return NET_RX_DROP.  This also fixes the problem where
> >    NET_RX_SUCCESS is returned on error
> > 
> > Martin Townsend (2):
> >  Remove dev parameter from skb_delivery_cb in 6lowpan.
> >  Change lowpan_rcv so skb is freed within function and fix return
> >    values.
> > 
> > include/net/6lowpan.h         |  4 ++--
> > net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
> > net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
> > net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
> > 4 files changed, 52 insertions(+), 54 deletions(-)
> 
> I can not take these patches at this point.
> 
> checking file net/ieee802154/6lowpan_rtnl.c
> Hunk #1 FAILED at 143.
> Hunk #2 succeeded at 480 (offset -4 lines).
> 1 out of 2 hunks FAILED
> 
> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
> 

I already told Martin that these patches should be based on bluetooth
(better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
are also some known other issues in the current implementation). I don't
think that the current implementation is used in a real environment.

I get this patch-series in a resend of this series, after Martin
subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
need some time for review and he want to resend them tomorrow or friday.
The resend should based on bluetooth-next then, if this is okay for you.

I will cc bluetooth mailinglist in my review notes.

- Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
  2014-07-30 18:27     ` Alexander Aring
@ 2014-07-30 18:32       ` Marcel Holtmann
  -1 siblings, 0 replies; 12+ messages in thread
From: Marcel Holtmann @ 2014-07-30 18:32 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Martin Townsend, John W. Linville, linux-zigbee-devel,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless@vger.kernel.org Wireless

Hi Alex,

>>> Currently it is up to the functions below lowpan_rcv to free the skb on error
>>>   conditions.  This patch now removes all the UAPI error codes and process data
>>>   now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
>>>   the skb and return NET_RX_DROP.  This also fixes the problem where
>>>   NET_RX_SUCCESS is returned on error
>>> 
>>> Martin Townsend (2):
>>> Remove dev parameter from skb_delivery_cb in 6lowpan.
>>> Change lowpan_rcv so skb is freed within function and fix return
>>>   values.
>>> 
>>> include/net/6lowpan.h         |  4 ++--
>>> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
>>> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
>>> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
>>> 4 files changed, 52 insertions(+), 54 deletions(-)
>> 
>> I can not take these patches at this point.
>> 
>> checking file net/ieee802154/6lowpan_rtnl.c
>> Hunk #1 FAILED at 143.
>> Hunk #2 succeeded at 480 (offset -4 lines).
>> 1 out of 2 hunks FAILED
>> 
>> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
>> 
> 
> I already told Martin that these patches should be based on bluetooth
> (better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
> are also some known other issues in the current implementation). I don't
> think that the current implementation is used in a real environment.
> 
> I get this patch-series in a resend of this series, after Martin
> subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
> need some time for review and he want to resend them tomorrow or friday.
> The resend should based on bluetooth-next then, if this is okay for you.

if patches are based on bluetooth-next, then we have no problem here. I can easily merge them.

Regards

Marcel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
@ 2014-07-30 18:32       ` Marcel Holtmann
  0 siblings, 0 replies; 12+ messages in thread
From: Marcel Holtmann @ 2014-07-30 18:32 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Martin Townsend, John W. Linville,
	linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Wireless

Hi Alex,

>>> Currently it is up to the functions below lowpan_rcv to free the skb on error
>>>   conditions.  This patch now removes all the UAPI error codes and process data
>>>   now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
>>>   the skb and return NET_RX_DROP.  This also fixes the problem where
>>>   NET_RX_SUCCESS is returned on error
>>> 
>>> Martin Townsend (2):
>>> Remove dev parameter from skb_delivery_cb in 6lowpan.
>>> Change lowpan_rcv so skb is freed within function and fix return
>>>   values.
>>> 
>>> include/net/6lowpan.h         |  4 ++--
>>> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
>>> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
>>> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
>>> 4 files changed, 52 insertions(+), 54 deletions(-)
>> 
>> I can not take these patches at this point.
>> 
>> checking file net/ieee802154/6lowpan_rtnl.c
>> Hunk #1 FAILED at 143.
>> Hunk #2 succeeded at 480 (offset -4 lines).
>> 1 out of 2 hunks FAILED
>> 
>> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
>> 
> 
> I already told Martin that these patches should be based on bluetooth
> (better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
> are also some known other issues in the current implementation). I don't
> think that the current implementation is used in a real environment.
> 
> I get this patch-series in a resend of this series, after Martin
> subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
> need some time for review and he want to resend them tomorrow or friday.
> The resend should based on bluetooth-next then, if this is okay for you.

if patches are based on bluetooth-next, then we have no problem here. I can easily merge them.

Regards

Marcel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
  2014-07-30 18:32       ` Marcel Holtmann
  (?)
@ 2014-07-31 13:59       ` John W. Linville
  2014-07-31 14:07           ` Alexander Aring
  2014-07-31 15:14         ` Marcel Holtmann
  -1 siblings, 2 replies; 12+ messages in thread
From: John W. Linville @ 2014-07-31 13:59 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Alexander Aring, Martin Townsend, linux-zigbee-devel,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless@vger.kernel.org Wireless

On Wed, Jul 30, 2014 at 11:32:16AM -0700, Marcel Holtmann wrote:
> Hi Alex,
> 
> >>> Currently it is up to the functions below lowpan_rcv to free the skb on error
> >>>   conditions.  This patch now removes all the UAPI error codes and process data
> >>>   now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
> >>>   the skb and return NET_RX_DROP.  This also fixes the problem where
> >>>   NET_RX_SUCCESS is returned on error
> >>> 
> >>> Martin Townsend (2):
> >>> Remove dev parameter from skb_delivery_cb in 6lowpan.
> >>> Change lowpan_rcv so skb is freed within function and fix return
> >>>   values.
> >>> 
> >>> include/net/6lowpan.h         |  4 ++--
> >>> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
> >>> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
> >>> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
> >>> 4 files changed, 52 insertions(+), 54 deletions(-)
> >> 
> >> I can not take these patches at this point.
> >> 
> >> checking file net/ieee802154/6lowpan_rtnl.c
> >> Hunk #1 FAILED at 143.
> >> Hunk #2 succeeded at 480 (offset -4 lines).
> >> 1 out of 2 hunks FAILED
> >> 
> >> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
> >> 
> > 
> > I already told Martin that these patches should be based on bluetooth
> > (better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
> > are also some known other issues in the current implementation). I don't
> > think that the current implementation is used in a real environment.
> > 
> > I get this patch-series in a resend of this series, after Martin
> > subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
> > need some time for review and he want to resend them tomorrow or friday.
> > The resend should based on bluetooth-next then, if this is okay for you.
> 
> if patches are based on bluetooth-next, then we have no problem here. I can easily merge them.

Does that resolve it?  That probably won't make 3.17, fwiw...

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
@ 2014-07-31 14:07           ` Alexander Aring
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2014-07-31 14:07 UTC (permalink / raw)
  To: John W. Linville
  Cc: Marcel Holtmann, Martin Townsend, linux-zigbee-devel,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless@vger.kernel.org Wireless

Hi,

On Thu, Jul 31, 2014 at 09:59:46AM -0400, John W. Linville wrote:
> On Wed, Jul 30, 2014 at 11:32:16AM -0700, Marcel Holtmann wrote:
> > Hi Alex,
> > 
> > >>> Currently it is up to the functions below lowpan_rcv to free the skb on error
> > >>>   conditions.  This patch now removes all the UAPI error codes and process data
> > >>>   now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
> > >>>   the skb and return NET_RX_DROP.  This also fixes the problem where
> > >>>   NET_RX_SUCCESS is returned on error
> > >>> 
> > >>> Martin Townsend (2):
> > >>> Remove dev parameter from skb_delivery_cb in 6lowpan.
> > >>> Change lowpan_rcv so skb is freed within function and fix return
> > >>>   values.
> > >>> 
> > >>> include/net/6lowpan.h         |  4 ++--
> > >>> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
> > >>> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
> > >>> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
> > >>> 4 files changed, 52 insertions(+), 54 deletions(-)
> > >> 
> > >> I can not take these patches at this point.
> > >> 
> > >> checking file net/ieee802154/6lowpan_rtnl.c
> > >> Hunk #1 FAILED at 143.
> > >> Hunk #2 succeeded at 480 (offset -4 lines).
> > >> 1 out of 2 hunks FAILED
> > >> 
> > >> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
> > >> 
> > > 
> > > I already told Martin that these patches should be based on bluetooth
> > > (better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
> > > are also some known other issues in the current implementation). I don't
> > > think that the current implementation is used in a real environment.
> > > 
> > > I get this patch-series in a resend of this series, after Martin
> > > subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
> > > need some time for review and he want to resend them tomorrow or friday.
> > > The resend should based on bluetooth-next then, if this is okay for you.
> > 
> > if patches are based on bluetooth-next, then we have no problem here. I can easily merge them.
> 
> Does that resolve it?  That probably won't make 3.17, fwiw...
> 

there are some issues on this patch series. Martin need to work on and
resend them as v2.

I told about the issues on an another thread which should be the same series.
I don't get exactly this patch series, I think Marcel add me in CC later.

So it needs some time anyway. Thanks.

- Alex

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
@ 2014-07-31 14:07           ` Alexander Aring
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2014-07-31 14:07 UTC (permalink / raw)
  To: John W. Linville
  Cc: Network Development, Marcel Holtmann,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Wireless,
	linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	BlueZ development

Hi,

On Thu, Jul 31, 2014 at 09:59:46AM -0400, John W. Linville wrote:
> On Wed, Jul 30, 2014 at 11:32:16AM -0700, Marcel Holtmann wrote:
> > Hi Alex,
> > 
> > >>> Currently it is up to the functions below lowpan_rcv to free the skb on error
> > >>>   conditions.  This patch now removes all the UAPI error codes and process data
> > >>>   now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
> > >>>   the skb and return NET_RX_DROP.  This also fixes the problem where
> > >>>   NET_RX_SUCCESS is returned on error
> > >>> 
> > >>> Martin Townsend (2):
> > >>> Remove dev parameter from skb_delivery_cb in 6lowpan.
> > >>> Change lowpan_rcv so skb is freed within function and fix return
> > >>>   values.
> > >>> 
> > >>> include/net/6lowpan.h         |  4 ++--
> > >>> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
> > >>> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
> > >>> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
> > >>> 4 files changed, 52 insertions(+), 54 deletions(-)
> > >> 
> > >> I can not take these patches at this point.
> > >> 
> > >> checking file net/ieee802154/6lowpan_rtnl.c
> > >> Hunk #1 FAILED at 143.
> > >> Hunk #2 succeeded at 480 (offset -4 lines).
> > >> 1 out of 2 hunks FAILED
> > >> 
> > >> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
> > >> 
> > > 
> > > I already told Martin that these patches should be based on bluetooth
> > > (better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
> > > are also some known other issues in the current implementation). I don't
> > > think that the current implementation is used in a real environment.
> > > 
> > > I get this patch-series in a resend of this series, after Martin
> > > subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
> > > need some time for review and he want to resend them tomorrow or friday.
> > > The resend should based on bluetooth-next then, if this is okay for you.
> > 
> > if patches are based on bluetooth-next, then we have no problem here. I can easily merge them.
> 
> Does that resolve it?  That probably won't make 3.17, fwiw...
> 

there are some issues on this patch series. Martin need to work on and
resend them as v2.

I told about the issues on an another thread which should be the same series.
I don't get exactly this patch series, I think Marcel add me in CC later.

So it needs some time anyway. 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values.
  2014-07-31 13:59       ` John W. Linville
  2014-07-31 14:07           ` Alexander Aring
@ 2014-07-31 15:14         ` Marcel Holtmann
  1 sibling, 0 replies; 12+ messages in thread
From: Marcel Holtmann @ 2014-07-31 15:14 UTC (permalink / raw)
  To: John W. Linville
  Cc: Alexander Aring, Martin Townsend, linux-zigbee-devel,
	BlueZ development, Martin Townsend, Network Development,
	linux-wireless@vger.kernel.org Wireless

Hi John,

>>>>> Currently it is up to the functions below lowpan_rcv to free the skb on error
>>>>>  conditions.  This patch now removes all the UAPI error codes and process data
>>>>>  now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
>>>>>  the skb and return NET_RX_DROP.  This also fixes the problem where
>>>>>  NET_RX_SUCCESS is returned on error
>>>>> 
>>>>> Martin Townsend (2):
>>>>> Remove dev parameter from skb_delivery_cb in 6lowpan.
>>>>> Change lowpan_rcv so skb is freed within function and fix return
>>>>>  values.
>>>>> 
>>>>> include/net/6lowpan.h         |  4 ++--
>>>>> net/6lowpan/iphc.c            | 37 ++++++++++++++++++------------------
>>>>> net/bluetooth/6lowpan.c       | 21 ++++++++++-----------
>>>>> net/ieee802154/6lowpan_rtnl.c | 44 +++++++++++++++++++++----------------------
>>>>> 4 files changed, 52 insertions(+), 54 deletions(-)
>>>> 
>>>> I can not take these patches at this point.
>>>> 
>>>> checking file net/ieee802154/6lowpan_rtnl.c
>>>> Hunk #1 FAILED at 143.
>>>> Hunk #2 succeeded at 480 (offset -4 lines).
>>>> 1 out of 2 hunks FAILED
>>>> 
>>>> We need to get the IEEE 802.15.4 changes merged through John's wireless-next tree. IEEE 802.15.4 going straight into net-next and Bluetooth going into wireless-next will not work out smoothly when both technologies are now utilizing 6LoWPAN and we are heavily working on 6LoWPAN.
>>>> 
>>> 
>>> I already told Martin that these patches should be based on bluetooth
>>> (better bluetooth-next, 802.15.4 hasn't a real stable strategie and there
>>> are also some known other issues in the current implementation). I don't
>>> think that the current implementation is used in a real environment.
>>> 
>>> I get this patch-series in a resend of this series, after Martin
>>> subscribed the linux-zigbee-devel mailinglist, then I told Martin that I
>>> need some time for review and he want to resend them tomorrow or friday.
>>> The resend should based on bluetooth-next then, if this is okay for you.
>> 
>> if patches are based on bluetooth-next, then we have no problem here. I can easily merge them.
> 
> Does that resolve it?  That probably won't make 3.17, fwiw...

I agree that this is out of the question for 3.17 at this point in time. Johan send our last pull request which includes what we have for 6LoWPAN right now. Everything else needs to wait for 3.18.

Regards

Marcel


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-07-31 15:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-30 14:55 [PATCH 0/2] linux-wpan-next: lowpan_rcv - skb is freed within function and fix return values Martin Townsend
2014-07-30 14:55 ` [PATCH 1/2] Remove dev parameter from skb_delivery_cb in 6lowpan Martin Townsend
2014-07-30 14:55 ` [PATCH 2/2] Change lowpan_rcv so skb is freed within function and fix return values Martin Townsend
2014-07-30 17:42 ` [PATCH 0/2] linux-wpan-next: lowpan_rcv - " Marcel Holtmann
2014-07-30 18:27   ` Alexander Aring
2014-07-30 18:27     ` Alexander Aring
2014-07-30 18:32     ` Marcel Holtmann
2014-07-30 18:32       ` Marcel Holtmann
2014-07-31 13:59       ` John W. Linville
2014-07-31 14:07         ` Alexander Aring
2014-07-31 14:07           ` Alexander Aring
2014-07-31 15:14         ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.