[PATCH net-next v2 0/7] ieee802.15.4 general fixes

[PATCH net-next v2 0/7] ieee802.15.4 general fixes

[Alexander Smirnov]

Dear David, Eric,

this patch-set is mostly intended to fix sparse and LOCKDEP warnings.
It mostly contains some my previous patches reworked and extended according
to the hints from Eric Dumazet and Fengguang Wu. Many thanks to they!

Changes since v1:
1. A new patch from Tony Cheneau was added. The fragmentation stops working
after some amount of packets sent. This patch fixes this issue.
2. 6lowpan fragment deleting routine: I removed spinlocks from timer_expired
handler and use spin_lock_bh to disable concurrency races with timer interrupt.
3. at86rf230 irq handler was a little bit modified

With best regards,
Alex

Alexander Smirnov (7):
  6lowpan: revert: add missing spin_lock_init()
  drivers/ieee802154/at86rf230: rework irq handler
  mac802154: add get short address method
  6lowpan: get extra headroom in allocated frame
  mac802154: sparse warnings: make symbols static
  6lowpan: fix tag variable size
  6lowpan: rework fragment-deleting routine

 drivers/ieee802154/at86rf230.c |   32 ++++++++++++++-----------
 net/ieee802154/6lowpan.c       |   49 +++++++++++++++++++--------------------
 net/mac802154/mac802154.h      |    1 +
 net/mac802154/mac_cmd.c        |    4 ++-
 net/mac802154/mib.c            |   16 ++++++++++++-
 5 files changed, 61 insertions(+), 41 deletions(-)

--
1.7.2.3

[PATCH net-next v2 1/7] 6lowpan: revert: add missing spin_lock_init()

[Alexander Smirnov]

Revert the commit 768f7c7c121e80f458a9d013b2e8b169e5dfb1e5 to initialize
spinlock in the more preferable way and make it static to avoid sparse
warning.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 net/ieee802154/6lowpan.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index f4070e5..b872515 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -123,7 +123,7 @@ struct lowpan_fragment {
 
 static unsigned short fragment_tag;
 static LIST_HEAD(lowpan_fragments);
-spinlock_t flist_lock;
+static DEFINE_SPINLOCK(flist_lock);
 
 static inline struct
 lowpan_dev_info *lowpan_dev_info(const struct net_device *dev)
@@ -1186,8 +1186,6 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 	list_add_tail(&entry->list, &lowpan_devices);
 	mutex_unlock(&lowpan_dev_info(dev)->dev_list_mtx);
 
-	spin_lock_init(&flist_lock);
-
 	register_netdevice(dev);
 
 	return 0;
-- 
1.7.2.3

[PATCH net-next v2 2/7] drivers/ieee802154/at86rf230: rework irq handler

[Alexander Smirnov]

Fix LOCKDEP bug message for the irq handler spinlock.
Make the irq processing code more explicit and stable.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 drivers/ieee802154/at86rf230.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/ieee802154/at86rf230.c b/drivers/ieee802154/at86rf230.c
index 902e38b..5d30940 100644
--- a/drivers/ieee802154/at86rf230.c
+++ b/drivers/ieee802154/at86rf230.c
@@ -543,6 +543,13 @@ at86rf230_xmit(struct ieee802154_dev *dev, struct sk_buff *skb)
 	int rc;
 	unsigned long flags;
 
+	spin_lock(&lp->lock);
+	if  (lp->irq_disabled) {
+		spin_unlock(&lp->lock);
+		return -EBUSY;
+	}
+	spin_unlock(&lp->lock);
+
 	might_sleep();
 
 	rc = at86rf230_state(dev, STATE_FORCE_TX_ON);
@@ -592,12 +599,8 @@ static int at86rf230_rx(struct at86rf230_local *lp)
 	if (!skb)
 		return -ENOMEM;
 
-	if (at86rf230_write_subreg(lp, SR_RX_PDT_DIS, 1) ||
-	    at86rf230_read_fbuf(lp, skb_put(skb, len), &len, &lqi) ||
-	    at86rf230_write_subreg(lp, SR_RX_SAFE_MODE, 1) ||
-	    at86rf230_write_subreg(lp, SR_RX_PDT_DIS, 0)) {
+	if (at86rf230_read_fbuf(lp, skb_put(skb, len), &len, &lqi))
 		goto err;
-	}
 
 	if (len < 2)
 		goto err;
@@ -633,7 +636,6 @@ static void at86rf230_irqwork(struct work_struct *work)
 	int rc;
 	unsigned long flags;
 
-	spin_lock_irqsave(&lp->lock, flags);
 	rc = at86rf230_read_subreg(lp, RG_IRQ_STATUS, 0xff, 0, &val);
 	status |= val;
 
@@ -643,31 +645,33 @@ static void at86rf230_irqwork(struct work_struct *work)
 	status &= ~IRQ_TRX_UR; /* FIXME: possibly handle ???*/
 
 	if (status & IRQ_TRX_END) {
+		spin_lock_irqsave(&lp->lock, flags);
 		status &= ~IRQ_TRX_END;
 		if (lp->is_tx) {
 			lp->is_tx = 0;
+			spin_unlock_irqrestore(&lp->lock, flags);
 			complete(&lp->tx_complete);
 		} else {
+			spin_unlock_irqrestore(&lp->lock, flags);
 			at86rf230_rx(lp);
 		}
 	}
 
-	if (lp->irq_disabled) {
-		lp->irq_disabled = 0;
-		enable_irq(lp->spi->irq);
-	}
+	spin_lock_irqsave(&lp->lock, flags);
+	lp->irq_disabled = 0;
 	spin_unlock_irqrestore(&lp->lock, flags);
+
+	enable_irq(lp->spi->irq);
 }
 
 static irqreturn_t at86rf230_isr(int irq, void *data)
 {
 	struct at86rf230_local *lp = data;
 
+	disable_irq_nosync(irq);
+
 	spin_lock(&lp->lock);
-	if (!lp->irq_disabled) {
-		disable_irq_nosync(irq);
-		lp->irq_disabled = 1;
-	}
+	lp->irq_disabled = 1;
 	spin_unlock(&lp->lock);
 
 	schedule_work(&lp->irqwork);
-- 
1.7.2.3

[PATCH net-next v2 3/7] mac802154: add get short address method

[Alexander Smirnov]

Add method to get the device short 802.15.4 address. This call
needed by ieee802154 layer to satisfy 'iz list' request from
the user space.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 net/mac802154/mac802154.h |    1 +
 net/mac802154/mac_cmd.c   |    2 ++
 net/mac802154/mib.c       |   14 ++++++++++++++
 3 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/net/mac802154/mac802154.h b/net/mac802154/mac802154.h
index 6967864..a4dcaf1 100644
--- a/net/mac802154/mac802154.h
+++ b/net/mac802154/mac802154.h
@@ -109,6 +109,7 @@ netdev_tx_t mac802154_tx(struct mac802154_priv *priv, struct sk_buff *skb,
 
 /* MIB callbacks */
 void mac802154_dev_set_short_addr(struct net_device *dev, u16 val);
+u16 mac802154_dev_get_short_addr(const struct net_device *dev);
 void mac802154_dev_set_ieee_addr(struct net_device *dev);
 u16 mac802154_dev_get_pan_id(const struct net_device *dev);
 void mac802154_dev_set_pan_id(struct net_device *dev, u16 val);
diff --git a/net/mac802154/mac_cmd.c b/net/mac802154/mac_cmd.c
index 7f5403e..5d9a47b 100644
--- a/net/mac802154/mac_cmd.c
+++ b/net/mac802154/mac_cmd.c
@@ -71,4 +71,6 @@ struct ieee802154_reduced_mlme_ops mac802154_mlme_reduced = {
 struct ieee802154_mlme_ops mac802154_mlme_wpan = {
 	.get_phy = mac802154_get_phy,
 	.start_req = mac802154_mlme_start_req,
+	.get_pan_id = mac802154_dev_get_pan_id,
+	.get_short_addr = mac802154_dev_get_short_addr,
 };
diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index 380829d..5c66b8f 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -100,6 +100,20 @@ void mac802154_dev_set_short_addr(struct net_device *dev, u16 val)
 	}
 }
 
+u16 mac802154_dev_get_short_addr(const struct net_device *dev)
+{
+	struct mac802154_sub_if_data *priv = netdev_priv(dev);
+	u16 ret;
+
+	BUG_ON(dev->type != ARPHRD_IEEE802154);
+
+	spin_lock_bh(&priv->mib_lock);
+	ret = priv->short_addr;
+	spin_unlock_bh(&priv->mib_lock);
+
+	return ret;
+}
+
 void mac802154_dev_set_ieee_addr(struct net_device *dev)
 {
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
-- 
1.7.2.3

[PATCH net-next v2 4/7] 6lowpan: get extra headroom in allocated frame

[Alexander Smirnov]

Use netdev_alloc_skb_ip_align() instead of alloc_skb() to get some
extra headroom in case we need to forward this frame in a tunnel or
something else.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 net/ieee802154/6lowpan.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index b872515..ae3f4eb 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -661,8 +661,8 @@ lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u8 tag)
 	frame->tag = tag;
 
 	/* allocate buffer for frame assembling */
-	frame->skb = alloc_skb(frame->length +
-			       sizeof(struct ipv6hdr), GFP_ATOMIC);
+	frame->skb = netdev_alloc_skb_ip_align(skb->dev, frame->length +
+					       sizeof(struct ipv6hdr));
 
 	if (!frame->skb)
 		goto skb_err;
-- 
1.7.2.3

[PATCH net-next v2 5/7] mac802154: sparse warnings: make symbols static

[Alexander Smirnov]

Make symbols static to avoid the following warning shown up
by sparse:

    warning: symbol ... was not declared. Should it be static?

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 net/mac802154/mac_cmd.c |    2 +-
 net/mac802154/mib.c     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac802154/mac_cmd.c b/net/mac802154/mac_cmd.c
index 5d9a47b..d8d2770 100644
--- a/net/mac802154/mac_cmd.c
+++ b/net/mac802154/mac_cmd.c
@@ -55,7 +55,7 @@ static int mac802154_mlme_start_req(struct net_device *dev,
 	return 0;
 }
 
-struct wpan_phy *mac802154_get_phy(const struct net_device *dev)
+static struct wpan_phy *mac802154_get_phy(const struct net_device *dev)
 {
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
 
diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index 5c66b8f..f47781a 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -39,7 +39,7 @@ struct hw_addr_filt_notify_work {
 	unsigned long changed;
 };
 
-struct mac802154_priv *mac802154_slave_get_priv(struct net_device *dev)
+static struct mac802154_priv *mac802154_slave_get_priv(struct net_device *dev)
 {
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
 
-- 
1.7.2.3

[PATCH net-next v2 6/7] 6lowpan: fix tag variable size

[Alexander Smirnov]

Function lowpan_alloc_new_frame() takes u8 tag as an argument. However,
its only caller, lowpan_process_data() passes down a u16. Hence,
the tag value can get corrupted. This prevent 6lowpan fragment reassembly of a
message when the fragment tag value is over 256.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Cc: Tony Cheneau <tony.cheneau@amnesiak.org>
---
 net/ieee802154/6lowpan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index ae3f4eb..2e790fb 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -646,7 +646,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
 }
 
 static struct lowpan_fragment *
-lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u8 tag)
+lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u16 tag)
 {
 	struct lowpan_fragment *frame;
 
-- 
1.7.2.3

[PATCH net-next v2 7/7] 6lowpan: rework fragment-deleting routine

[Alexander Smirnov]

6lowpan module starts collecting incomming frames and fragments
right after lowpan_module_init() therefor it will be better to
clean unfinished fragments in lowpan_cleanup_module() function
instead of doing it when link goes down.

Changed spinlocks type to prevent deadlock with expired timer event
and removed unused one.

Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
 net/ieee802154/6lowpan.c |   39 ++++++++++++++++++++-------------------
 1 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 2e790fb..6871ec1 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -113,7 +113,6 @@ struct lowpan_dev_record {
 
 struct lowpan_fragment {
 	struct sk_buff		*skb;		/* skb to be assembled */
-	spinlock_t		lock;		/* concurency lock */
 	u16			length;		/* length to be assemled */
 	u32			bytes_rcv;	/* bytes received */
 	u16			tag;		/* current fragment tag */
@@ -637,10 +636,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
 
 	pr_debug("timer expired for frame with tag %d\n", entry->tag);
 
-	spin_lock(&flist_lock);
 	list_del(&entry->list);
-	spin_unlock(&flist_lock);
-
 	dev_kfree_skb(entry->skb);
 	kfree(entry);
 }
@@ -727,7 +723,7 @@ lowpan_process_data(struct sk_buff *skb)
 		 * check if frame assembling with the same tag is
 		 * already in progress
 		 */
-		spin_lock(&flist_lock);
+		spin_lock_bh(&flist_lock);
 
 		list_for_each_entry(frame, &lowpan_fragments, list)
 			if (frame->tag == tag) {
@@ -761,9 +757,9 @@ lowpan_process_data(struct sk_buff *skb)
 		if ((frame->bytes_rcv == frame->length) &&
 		     frame->timer.expires > jiffies) {
 			/* if timer haven't expired - first of all delete it */
-			del_timer(&frame->timer);
+			del_timer_sync(&frame->timer);
 			list_del(&frame->list);
-			spin_unlock(&flist_lock);
+			spin_unlock_bh(&flist_lock);
 
 			dev_kfree_skb(skb);
 			skb = frame->skb;
@@ -774,7 +770,7 @@ lowpan_process_data(struct sk_buff *skb)
 
 			break;
 		}
-		spin_unlock(&flist_lock);
+		spin_unlock_bh(&flist_lock);
 
 		return kfree_skb(skb), 0;
 	}
@@ -929,7 +925,7 @@ lowpan_process_data(struct sk_buff *skb)
 	return lowpan_skb_deliver(skb, &hdr);
 
 unlock_and_drop:
-	spin_unlock(&flist_lock);
+	spin_unlock_bh(&flist_lock);
 drop:
 	kfree_skb(skb);
 	return -EINVAL;
@@ -1196,19 +1192,9 @@ static void lowpan_dellink(struct net_device *dev, struct list_head *head)
 	struct lowpan_dev_info *lowpan_dev = lowpan_dev_info(dev);
 	struct net_device *real_dev = lowpan_dev->real_dev;
 	struct lowpan_dev_record *entry, *tmp;
-	struct lowpan_fragment *frame, *tframe;
 
 	ASSERT_RTNL();
 
-	spin_lock(&flist_lock);
-	list_for_each_entry_safe(frame, tframe, &lowpan_fragments, list) {
-		del_timer(&frame->timer);
-		list_del(&frame->list);
-		dev_kfree_skb(frame->skb);
-		kfree(frame);
-	}
-	spin_unlock(&flist_lock);
-
 	mutex_lock(&lowpan_dev_info(dev)->dev_list_mtx);
 	list_for_each_entry_safe(entry, tmp, &lowpan_devices, list) {
 		if (entry->ldev == dev) {
@@ -1264,9 +1250,24 @@ out:
 
 static void __exit lowpan_cleanup_module(void)
 {
+	struct lowpan_fragment *frame, *tframe;
+
 	lowpan_netlink_fini();
 
 	dev_remove_pack(&lowpan_packet_type);
+
+	/* Now 6lowpan packet_type is removed, so no new fragments are
+	 * expected on RX, therefore that's the time to clean incomplete
+	 * fragments.
+	 */
+	spin_lock_bh(&flist_lock);
+	list_for_each_entry_safe(frame, tframe, &lowpan_fragments, list) {
+		del_timer_sync(&frame->timer);
+		list_del(&frame->list);
+		dev_kfree_skb(frame->skb);
+		kfree(frame);
+	}
+	spin_unlock_bh(&flist_lock);
 }
 
 module_init(lowpan_init_module);
-- 
1.7.2.3

Re: [PATCH net-next v2 0/7] ieee802.15.4 general fixes

[David Miller]

From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Date: Wed, 11 Jul 2012 11:22:41 +0400

> Dear David, Eric,
> 
> this patch-set is mostly intended to fix sparse and LOCKDEP warnings.
> It mostly contains some my previous patches reworked and extended according
> to the hints from Eric Dumazet and Fengguang Wu. Many thanks to they!
> 
> Changes since v1:
> 1. A new patch from Tony Cheneau was added. The fragmentation stops working
> after some amount of packets sent. This patch fixes this issue.
> 2. 6lowpan fragment deleting routine: I removed spinlocks from timer_expired
> handler and use spin_lock_bh to disable concurrency races with timer interrupt.
> 3. at86rf230 irq handler was a little bit modified

Series applied, but you don't need to grab a spinlock to only
load one interger from some datastructure.  I mean:

	lock();
	ret = p->foo;
	unlock();

is completely pointless.