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

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

[Alexander Smirnov]

Dear David, Eric,

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

With best regards,
Alex

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

 drivers/ieee802154/at86rf230.c |   14 ++++++--------
 net/ieee802154/6lowpan.c       |   36 +++++++++++++++++++-----------------
 net/mac802154/mac802154.h      |    1 +
 net/mac802154/mac_cmd.c        |    4 +++-
 net/mac802154/mib.c            |   16 +++++++++++++++-
 5 files changed, 44 insertions(+), 27 deletions(-)

--
1.7.2.3

[PATCH net-next 1/6] 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 2/6] 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 |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/ieee802154/at86rf230.c b/drivers/ieee802154/at86rf230.c
index 902e38b..17b8320 100644
--- a/drivers/ieee802154/at86rf230.c
+++ b/drivers/ieee802154/at86rf230.c
@@ -652,22 +652,20 @@ static void at86rf230_irqwork(struct work_struct *work)
 		}
 	}
 
-	if (lp->irq_disabled) {
-		lp->irq_disabled = 0;
-		enable_irq(lp->spi->irq);
-	}
+	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 3/6] 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 4/6] 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 |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index b872515..e7de085 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 */
@@ -761,7 +760,7 @@ 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);
 
@@ -1196,19 +1195,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 +1253,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 4/6] 6lowpan: rework fragment-deleting routine

[Eric Dumazet]

On Mon, 2012-07-09 at 14:22 +0400, Alexander Smirnov wrote:
> 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 |   28 ++++++++++++++++------------
>  1 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> index b872515..e7de085 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 */
> @@ -761,7 +760,7 @@ 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);
>  
> @@ -1196,19 +1195,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 +1253,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);

Problem is lowpan_fragment_timer_expired() can race with this code.

del_timer_sync() might block here if lowpan_fragment_timer_expired() is
waiting/spinning for spin_lock(&flist_lock)

You cant call del_timer_sync() holding flist_lock, you should find
another way to solve the problem.

Its explained in kernel/timer.c :

#ifdef CONFIG_SMP
/**
 * del_timer_sync - deactivate a timer and wait for the handler to finish.
 * @timer: the timer to be deactivated
 *
 * This function only differs from del_timer() on SMP: besides deactivating
 * the timer it also makes sure the handler has finished executing on other
 * CPUs.
 *
 * Synchronization rules: Callers must prevent restarting of the timer,
 * otherwise this function is meaningless. It must not be called from
 * interrupt contexts. The caller must not hold locks which would prevent
 * completion of the timer's handler. The timer's handler must not call
 * add_timer_on(). Upon exit the timer is not queued and the handler is
 * not running on any CPU.
 *
 * Note: You must not hold locks that are held in interrupt context
 *   while calling this function. Even if the lock has nothing to do
 *   with the timer in question.  Here's why:
 *
 *    CPU0                             CPU1
 *    ----                             ----
 *                                   <SOFTIRQ>
 *                                   call_timer_fn();
 *                                     base->running_timer = mytimer;
 *  spin_lock_irq(somelock);
 *                                     <IRQ>
 *                                        spin_lock(somelock);
 *  del_timer_sync(mytimer);
 *   while (base->running_timer == mytimer);
 *
 * Now del_timer_sync() will never return and never release somelock.
 * The interrupt on the other CPU is waiting to grab somelock but
 * it has interrupted the softirq that CPU0 is waiting to finish.
 *
 * The function returns whether it has deactivated a pending timer or not.
 */

[PATCH net-next 5/6] 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 e7de085..90ca7ba 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -660,8 +660,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 6/6] 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