[PATCH bluetooth-next 0/4] mac802154: remove pib/mib locks and locking fixes

[PATCH bluetooth-next 0/4] mac802154: remove pib/mib locks and locking fixes

[Alexander Aring]

Hi,

this patch series contains patches for remove pib and mib lock. We have now
a other locking strategie inside this stack which requires at the moment the
rtnl lock only. The first patch will hold the rtnl lock which is required to
access the mib attributes while calling socket ioctl. The last patch will use
atomic operations for incremet the dsn value for dataframes, currently it could
be that this value is the same in two contiguous frames which could be occur a
wrong ack frame handling in ARET mode.

- Alex

Alexander Aring (4):
  mac802154: fix hold rtnl while ioctl
  mac802154: remove pib lock
  mac802154: remove mib lock
  mac802154: use atomic ops for sequence incrementation

 include/net/cfg802154.h         |  6 ++--
 include/net/ieee802154_netdev.h | 10 -------
 net/ieee802154/6lowpan/core.c   | 28 ------------------
 net/ieee802154/6lowpan/tx.c     |  2 +-
 net/ieee802154/core.c           |  2 --
 net/ieee802154/nl-mac.c         | 14 ++++++---
 net/ieee802154/nl-phy.c         |  6 ++--
 net/ieee802154/socket.c         | 12 ++++----
 net/mac802154/ieee802154_i.h    |  7 -----
 net/mac802154/iface.c           | 29 ++++++-------------
 net/mac802154/mac_cmd.c         |  7 ++---
 net/mac802154/mib.c             | 63 ++---------------------------------------
 net/mac802154/rx.c              |  5 ----
 13 files changed, 34 insertions(+), 157 deletions(-)

-- 
2.4.1

[PATCH bluetooth-next 1/4] mac802154: fix hold rtnl while ioctl

[Alexander Aring]

This patch fixes an issue to set address configuration with ioctl.
Accessing the mib requires rtnl lock and the ndo_do_ioctl doesn't hold
the rtnl lock while this callback is called. This patch do that
manually.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
Reported-by: Matteo Petracca <matteo.petracca@sssup.it>
---
 net/mac802154/iface.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 91b75ab..2a58788 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -62,8 +62,7 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		(struct sockaddr_ieee802154 *)&ifr->ifr_addr;
 	int err = -ENOIOCTLCMD;
 
-	ASSERT_RTNL();
-
+	rtnl_lock();
 	spin_lock_bh(&sdata->mib_lock);
 
 	switch (cmd) {
@@ -90,6 +89,7 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	case SIOCSIFADDR:
 		if (netif_running(dev)) {
 			spin_unlock_bh(&sdata->mib_lock);
+			rtnl_unlock();
 			return -EBUSY;
 		}
 
@@ -112,6 +112,7 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	}
 
 	spin_unlock_bh(&sdata->mib_lock);
+	rtnl_unlock();
 	return err;
 }
 
-- 
2.4.1

[PATCH bluetooth-next 2/4] mac802154: remove pib lock

[Alexander Aring]

This patch removes the pib lock which is now replaced by rtnl lock. The
new interface already use the rtnl lock only. Nevertheless this patch
will fix issues while using new and old interface at the same time.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/cfg802154.h | 2 --
 net/ieee802154/core.c   | 2 --
 net/ieee802154/nl-phy.c | 6 +++---
 net/mac802154/iface.c   | 7 -------
 net/mac802154/mib.c     | 4 ++--
 5 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 11bbf17..c6aa1d2 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -121,8 +121,6 @@ enum wpan_phy_flags {
 };
 
 struct wpan_phy {
-	struct mutex pib_lock;
-
 	/* If multiple wpan_phys are registered and you're handed e.g.
 	 * a regular netdev with assigned ieee802154_ptr, you won't
 	 * know whether it points to a wpan_phy your driver has registered
diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 2ee00e8..b0248e9 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -121,8 +121,6 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size)
 	/* atomic_inc_return makes it start at 1, make it start at 0 */
 	rdev->wpan_phy_idx--;
 
-	mutex_init(&rdev->wpan_phy.pib_lock);
-
 	INIT_LIST_HEAD(&rdev->wpan_dev_list);
 	device_initialize(&rdev->wpan_phy.dev);
 	dev_set_name(&rdev->wpan_phy.dev, PHY_NAME "%d", rdev->wpan_phy_idx);
diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
index cbc0d09..77d7301 100644
--- a/net/ieee802154/nl-phy.c
+++ b/net/ieee802154/nl-phy.c
@@ -50,7 +50,7 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
 	if (!hdr)
 		goto out;
 
-	mutex_lock(&phy->pib_lock);
+	rtnl_lock();
 	if (nla_put_string(msg, IEEE802154_ATTR_PHY_NAME, wpan_phy_name(phy)) ||
 	    nla_put_u8(msg, IEEE802154_ATTR_PAGE, phy->current_page) ||
 	    nla_put_u8(msg, IEEE802154_ATTR_CHANNEL, phy->current_channel))
@@ -63,13 +63,13 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
 	    nla_put(msg, IEEE802154_ATTR_CHANNEL_PAGE_LIST,
 		    pages * sizeof(uint32_t), buf))
 		goto nla_put_failure;
-	mutex_unlock(&phy->pib_lock);
+	rtnl_unlock();
 	kfree(buf);
 	genlmsg_end(msg, hdr);
 	return 0;
 
 nla_put_failure:
-	mutex_unlock(&phy->pib_lock);
+	rtnl_unlock();
 	genlmsg_cancel(msg, hdr);
 out:
 	kfree(buf);
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 2a58788..22f478b 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -242,7 +242,6 @@ static int mac802154_wpan_open(struct net_device *dev)
 	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
 	struct ieee802154_local *local = sdata->local;
 	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
-	struct wpan_phy *phy = sdata->local->phy;
 
 	rc = ieee802154_check_concurrent_iface(sdata, sdata->vif.type);
 	if (rc < 0)
@@ -252,8 +251,6 @@ static int mac802154_wpan_open(struct net_device *dev)
 	if (rc < 0)
 		return rc;
 
-	mutex_lock(&phy->pib_lock);
-
 	if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) {
 		rc = drv_set_promiscuous_mode(local,
 					      wpan_dev->promiscuous_mode);
@@ -295,11 +292,7 @@ static int mac802154_wpan_open(struct net_device *dev)
 			goto out;
 	}
 
-	mutex_unlock(&phy->pib_lock);
-	return 0;
-
 out:
-	mutex_unlock(&phy->pib_lock);
 	return rc;
 }
 
diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index 5cf019a..033dfc7 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -91,16 +91,16 @@ void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan)
 	struct ieee802154_local *local = sdata->local;
 	int res;
 
+	ASSERT_RTNL();
+
 	BUG_ON(dev->type != ARPHRD_IEEE802154);
 
 	res = drv_set_channel(local, page, chan);
 	if (res) {
 		pr_debug("set_channel failed\n");
 	} else {
-		mutex_lock(&local->phy->pib_lock);
 		local->phy->current_channel = chan;
 		local->phy->current_page = page;
-		mutex_unlock(&local->phy->pib_lock);
 	}
 }
 
-- 
2.4.1

[PATCH bluetooth-next 3/4] mac802154: remove mib lock

[Alexander Aring]

This patch removes the mib lock. The new locking mechanism is to protect
the mib values with the rtnl lock. Note that this isn't always necessary
if we have an interface up the most mib values are readonly (e.g.
address settings). With this behaviour we can remove locking in
hotpath like frame parsing completely. It depends on context if we need
to hold the rtnl lock or not, this makes the callbacks of
ieee802154_mlme_ops unnecessary because these callbacks hols always the
locks.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/ieee802154_netdev.h | 10 -------
 net/ieee802154/6lowpan/core.c   | 28 -------------------
 net/ieee802154/6lowpan/tx.c     |  2 +-
 net/ieee802154/nl-mac.c         | 14 +++++++---
 net/ieee802154/socket.c         | 12 ++++-----
 net/mac802154/ieee802154_i.h    |  7 -----
 net/mac802154/iface.c           | 13 +++------
 net/mac802154/mac_cmd.c         |  7 ++---
 net/mac802154/mib.c             | 59 -----------------------------------------
 net/mac802154/rx.c              |  5 ----
 10 files changed, 22 insertions(+), 135 deletions(-)

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index 94a2970..84a72a1 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -422,16 +422,6 @@ struct ieee802154_mlme_ops {
 			       struct ieee802154_mac_params *params);
 
 	struct ieee802154_llsec_ops *llsec;
-
-	/* The fields below are required. */
-
-	/*
-	 * FIXME: these should become the part of PIB/MIB interface.
-	 * However we still don't have IB interface of any kind
-	 */
-	__le16 (*get_pan_id)(const struct net_device *dev);
-	__le16 (*get_short_addr)(const struct net_device *dev);
-	u8 (*get_dsn)(const struct net_device *dev);
 };
 
 static inline struct ieee802154_mlme_ops *
diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 0ae5822..f20a387 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -55,27 +55,6 @@
 LIST_HEAD(lowpan_devices);
 static int lowpan_open_count;
 
-static __le16 lowpan_get_pan_id(const struct net_device *dev)
-{
-	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
-
-	return ieee802154_mlme_ops(real_dev)->get_pan_id(real_dev);
-}
-
-static __le16 lowpan_get_short_addr(const struct net_device *dev)
-{
-	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
-
-	return ieee802154_mlme_ops(real_dev)->get_short_addr(real_dev);
-}
-
-static u8 lowpan_get_dsn(const struct net_device *dev)
-{
-	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
-
-	return ieee802154_mlme_ops(real_dev)->get_dsn(real_dev);
-}
-
 static struct header_ops lowpan_header_ops = {
 	.create	= lowpan_header_create,
 };
@@ -103,12 +82,6 @@ static const struct net_device_ops lowpan_netdev_ops = {
 	.ndo_start_xmit		= lowpan_xmit,
 };
 
-static struct ieee802154_mlme_ops lowpan_mlme = {
-	.get_pan_id = lowpan_get_pan_id,
-	.get_short_addr = lowpan_get_short_addr,
-	.get_dsn = lowpan_get_dsn,
-};
-
 static void lowpan_setup(struct net_device *dev)
 {
 	dev->addr_len		= IEEE802154_ADDR_LEN;
@@ -124,7 +97,6 @@ static void lowpan_setup(struct net_device *dev)
 
 	dev->netdev_ops		= &lowpan_netdev_ops;
 	dev->header_ops		= &lowpan_header_ops;
-	dev->ml_priv		= &lowpan_mlme;
 	dev->destructor		= free_netdev;
 	dev->features		|= NETIF_F_NETNS_LOCAL;
 }
diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
index 2349070..98acf73 100644
--- a/net/ieee802154/6lowpan/tx.c
+++ b/net/ieee802154/6lowpan/tx.c
@@ -207,7 +207,7 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *dev)
 
 	/* prepare wpan address data */
 	sa.mode = IEEE802154_ADDR_LONG;
-	sa.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
+	sa.pan_id = lowpan_dev_info(dev)->real_dev->ieee802154_ptr->pan_id;
 	sa.extended_addr = ieee802154_devaddr_from_raw(saddr);
 
 	/* intra-PAN communications */
diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
index cdc1cc3..ada58a8 100644
--- a/net/ieee802154/nl-mac.c
+++ b/net/ieee802154/nl-mac.c
@@ -97,8 +97,10 @@ static int ieee802154_nl_fill_iface(struct sk_buff *msg, u32 portid,
 	BUG_ON(!phy);
 	get_device(&phy->dev);
 
-	short_addr = ops->get_short_addr(dev);
-	pan_id = ops->get_pan_id(dev);
+	rtnl_lock();
+	short_addr = dev->ieee802154_ptr->short_addr;
+	pan_id = dev->ieee802154_ptr->pan_id;
+	rtnl_unlock();
 
 	if (nla_put_string(msg, IEEE802154_ATTR_DEV_NAME, dev->name) ||
 	    nla_put_string(msg, IEEE802154_ATTR_PHY_NAME, wpan_phy_name(phy)) ||
@@ -244,7 +246,9 @@ int ieee802154_associate_resp(struct sk_buff *skb, struct genl_info *info)
 	addr.mode = IEEE802154_ADDR_LONG;
 	addr.extended_addr = nla_get_hwaddr(
 			info->attrs[IEEE802154_ATTR_DEST_HW_ADDR]);
-	addr.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
+	rtnl_lock();
+	addr.pan_id = dev->ieee802154_ptr->pan_id;
+	rtnl_unlock();
 
 	ret = ieee802154_mlme_ops(dev)->assoc_resp(dev, &addr,
 		nla_get_shortaddr(info->attrs[IEEE802154_ATTR_DEST_SHORT_ADDR]),
@@ -281,7 +285,9 @@ int ieee802154_disassociate_req(struct sk_buff *skb, struct genl_info *info)
 		addr.short_addr = nla_get_shortaddr(
 				info->attrs[IEEE802154_ATTR_DEST_SHORT_ADDR]);
 	}
-	addr.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
+	rtnl_lock();
+	addr.pan_id = dev->ieee802154_ptr->pan_id;
+	rtnl_unlock();
 
 	ret = ieee802154_mlme_ops(dev)->disassoc_req(dev, &addr,
 			nla_get_u8(info->attrs[IEEE802154_ATTR_REASON]));
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 7aaaf96..e5cc253 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -64,10 +64,8 @@ ieee802154_get_dev(struct net *net, const struct ieee802154_addr *addr)
 			if (tmp->type != ARPHRD_IEEE802154)
 				continue;
 
-			pan_id = ieee802154_mlme_ops(tmp)->get_pan_id(tmp);
-			short_addr =
-				ieee802154_mlme_ops(tmp)->get_short_addr(tmp);
-
+			pan_id = tmp->ieee802154_ptr->pan_id;
+			short_addr = tmp->ieee802154_ptr->short_addr;
 			if (pan_id == addr->pan_id &&
 			    short_addr == addr->short_addr) {
 				dev = tmp;
@@ -797,9 +795,9 @@ static int ieee802154_dgram_deliver(struct net_device *dev, struct sk_buff *skb)
 	/* Data frame processing */
 	BUG_ON(dev->type != ARPHRD_IEEE802154);
 
-	pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
-	short_addr = ieee802154_mlme_ops(dev)->get_short_addr(dev);
-	hw_addr = ieee802154_devaddr_from_raw(dev->dev_addr);
+	pan_id = dev->ieee802154_ptr->pan_id;
+	short_addr = dev->ieee802154_ptr->short_addr;
+	hw_addr = dev->ieee802154_ptr->extended_addr;
 
 	read_lock(&dgram_lock);
 	sk_for_each(sk, &dgram_head) {
diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 127ba18..eec668f 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -86,8 +86,6 @@ struct ieee802154_sub_if_data {
 	unsigned long state;
 	char name[IFNAMSIZ];
 
-	spinlock_t mib_lock;
-
 	/* protects sec from concurrent access by netlink. access by
 	 * encrypt/decrypt/header_create safe without additional protection.
 	 */
@@ -136,12 +134,7 @@ ieee802154_subif_start_xmit(struct sk_buff *skb, struct net_device *dev);
 enum hrtimer_restart ieee802154_xmit_ifs_timer(struct hrtimer *timer);
 
 /* MIB callbacks */
-void mac802154_dev_set_short_addr(struct net_device *dev, __le16 val);
-__le16 mac802154_dev_get_short_addr(const struct net_device *dev);
-__le16 mac802154_dev_get_pan_id(const struct net_device *dev);
-void mac802154_dev_set_pan_id(struct net_device *dev, __le16 val);
 void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan);
-u8 mac802154_dev_get_dsn(const struct net_device *dev);
 
 int mac802154_get_params(struct net_device *dev,
 			 struct ieee802154_llsec_params *params);
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 22f478b..866d27f 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -63,7 +63,6 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	int err = -ENOIOCTLCMD;
 
 	rtnl_lock();
-	spin_lock_bh(&sdata->mib_lock);
 
 	switch (cmd) {
 	case SIOCGIFADDR:
@@ -88,7 +87,6 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	}
 	case SIOCSIFADDR:
 		if (netif_running(dev)) {
-			spin_unlock_bh(&sdata->mib_lock);
 			rtnl_unlock();
 			return -EBUSY;
 		}
@@ -111,7 +109,6 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		break;
 	}
 
-	spin_unlock_bh(&sdata->mib_lock);
 	rtnl_unlock();
 	return err;
 }
@@ -368,14 +365,15 @@ static int mac802154_header_create(struct sk_buff *skb,
 	hdr.fc.type = cb->type;
 	hdr.fc.security_enabled = cb->secen;
 	hdr.fc.ack_request = cb->ackreq;
-	hdr.seq = ieee802154_mlme_ops(dev)->get_dsn(dev);
+	/* TODO: use atomic_t as dsn, dsn need to be locked when AF_IEEE802154
+	 * and IEEE802154 6LoWPAN call this at the same time.
+	 */
+	hdr.seq = dev->ieee802154_ptr->dsn++;
 
 	if (mac802154_set_header_security(sdata, &hdr, cb) < 0)
 		return -EINVAL;
 
 	if (!saddr) {
-		spin_lock_bh(&sdata->mib_lock);
-
 		if (wpan_dev->short_addr == cpu_to_le16(IEEE802154_ADDR_BROADCAST) ||
 		    wpan_dev->short_addr == cpu_to_le16(IEEE802154_ADDR_UNDEF) ||
 		    wpan_dev->pan_id == cpu_to_le16(IEEE802154_PANID_BROADCAST)) {
@@ -387,8 +385,6 @@ static int mac802154_header_create(struct sk_buff *skb,
 		}
 
 		hdr.source.pan_id = wpan_dev->pan_id;
-
-		spin_unlock_bh(&sdata->mib_lock);
 	} else {
 		hdr.source = *(const struct ieee802154_addr *)saddr;
 	}
@@ -497,7 +493,6 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 		sdata->dev->ml_priv = &mac802154_mlme_wpan;
 		wpan_dev->promiscuous_mode = false;
 
-		spin_lock_init(&sdata->mib_lock);
 		mutex_init(&sdata->sec_mtx);
 
 		mac802154_llsec_init(&sdata->sec);
diff --git a/net/mac802154/mac_cmd.c b/net/mac802154/mac_cmd.c
index 6dcbb3b..5220c2b2 100644
--- a/net/mac802154/mac_cmd.c
+++ b/net/mac802154/mac_cmd.c
@@ -43,8 +43,8 @@ static int mac802154_mlme_start_req(struct net_device *dev,
 
 	BUG_ON(addr->mode != IEEE802154_ADDR_SHORT);
 
-	mac802154_dev_set_pan_id(dev, addr->pan_id);
-	mac802154_dev_set_short_addr(dev, addr->short_addr);
+	dev->ieee802154_ptr->pan_id = addr->pan_id;
+	dev->ieee802154_ptr->short_addr = addr->short_addr;
 	mac802154_dev_set_page_channel(dev, page, channel);
 
 	if (ops->llsec) {
@@ -151,9 +151,6 @@ static struct ieee802154_llsec_ops mac802154_llsec_ops = {
 
 struct ieee802154_mlme_ops mac802154_mlme_wpan = {
 	.start_req = mac802154_mlme_start_req,
-	.get_pan_id = mac802154_dev_get_pan_id,
-	.get_short_addr = mac802154_dev_get_short_addr,
-	.get_dsn = mac802154_dev_get_dsn,
 
 	.llsec = &mac802154_llsec_ops,
 
diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index 033dfc7..73f94fb 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -26,65 +26,6 @@
 #include "ieee802154_i.h"
 #include "driver-ops.h"
 
-void mac802154_dev_set_short_addr(struct net_device *dev, __le16 val)
-{
-	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-
-	BUG_ON(dev->type != ARPHRD_IEEE802154);
-
-	spin_lock_bh(&sdata->mib_lock);
-	sdata->wpan_dev.short_addr = val;
-	spin_unlock_bh(&sdata->mib_lock);
-}
-
-__le16 mac802154_dev_get_short_addr(const struct net_device *dev)
-{
-	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-	__le16 ret;
-
-	BUG_ON(dev->type != ARPHRD_IEEE802154);
-
-	spin_lock_bh(&sdata->mib_lock);
-	ret = sdata->wpan_dev.short_addr;
-	spin_unlock_bh(&sdata->mib_lock);
-
-	return ret;
-}
-
-__le16 mac802154_dev_get_pan_id(const struct net_device *dev)
-{
-	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-	__le16 ret;
-
-	BUG_ON(dev->type != ARPHRD_IEEE802154);
-
-	spin_lock_bh(&sdata->mib_lock);
-	ret = sdata->wpan_dev.pan_id;
-	spin_unlock_bh(&sdata->mib_lock);
-
-	return ret;
-}
-
-void mac802154_dev_set_pan_id(struct net_device *dev, __le16 val)
-{
-	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-
-	BUG_ON(dev->type != ARPHRD_IEEE802154);
-
-	spin_lock_bh(&sdata->mib_lock);
-	sdata->wpan_dev.pan_id = val;
-	spin_unlock_bh(&sdata->mib_lock);
-}
-
-u8 mac802154_dev_get_dsn(const struct net_device *dev)
-{
-	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-
-	BUG_ON(dev->type != ARPHRD_IEEE802154);
-
-	return sdata->wpan_dev.dsn++;
-}
-
 void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan)
 {
 	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index c0d67b2..e0f1006 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -47,8 +47,6 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 
 	pr_debug("getting packet via slave interface %s\n", sdata->dev->name);
 
-	spin_lock_bh(&sdata->mib_lock);
-
 	span = wpan_dev->pan_id;
 	sshort = wpan_dev->short_addr;
 
@@ -83,13 +81,10 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 			skb->pkt_type = PACKET_OTHERHOST;
 		break;
 	default:
-		spin_unlock_bh(&sdata->mib_lock);
 		pr_debug("invalid dest mode\n");
 		goto fail;
 	}
 
-	spin_unlock_bh(&sdata->mib_lock);
-
 	skb->dev = sdata->dev;
 
 	rc = mac802154_llsec_decrypt(&sdata->sec, skb);
-- 
2.4.1

[PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Alexander Aring]

This patch will use atomic operations for sequence number incrementation
while MAC header generation. Upper layers like af_802154 or 6LoWPAN
could call this function in a parallel context while generating 802.15.4
MAC header before queuing into wpan interfaces transmit queue.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/cfg802154.h |  4 ++--
 net/mac802154/iface.c   | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index c6aa1d2..4de59aa 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -177,9 +177,9 @@ struct wpan_dev {
 	__le64 extended_addr;
 
 	/* MAC BSN field */
-	u8 bsn;
+	atomic_t bsn;
 	/* MAC DSN field */
-	u8 dsn;
+	atomic_t dsn;
 
 	u8 min_be;
 	u8 max_be;
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 866d27f..f30788d 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -365,10 +365,7 @@ static int mac802154_header_create(struct sk_buff *skb,
 	hdr.fc.type = cb->type;
 	hdr.fc.security_enabled = cb->secen;
 	hdr.fc.ack_request = cb->ackreq;
-	/* TODO: use atomic_t as dsn, dsn need to be locked when AF_IEEE802154
-	 * and IEEE802154 6LoWPAN call this at the same time.
-	 */
-	hdr.seq = dev->ieee802154_ptr->dsn++;
+	hdr.seq = atomic_inc_return(&dev->ieee802154_ptr->dsn) & 0xFF;
 
 	if (mac802154_set_header_security(sdata, &hdr, cb) < 0)
 		return -EINVAL;
@@ -464,13 +461,16 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
 		       enum nl802154_iftype type)
 {
 	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
+	u8 tmp;
 
 	/* set some type-dependent values */
 	sdata->vif.type = type;
 	sdata->wpan_dev.iftype = type;
 
-	get_random_bytes(&wpan_dev->bsn, 1);
-	get_random_bytes(&wpan_dev->dsn, 1);
+	get_random_bytes(&tmp, sizeof(tmp));
+	atomic_set(&wpan_dev->bsn, tmp);
+	get_random_bytes(&tmp, sizeof(tmp));
+	atomic_set(&wpan_dev->dsn, tmp);
 
 	/* defaults per 802.15.4-2011 */
 	wpan_dev->min_be = 3;
-- 
2.4.1

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

On 05/22/2015 10:57 AM, Alexander Aring wrote:
> This patch will use atomic operations for sequence number incrementation
> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
> could call this function in a parallel context while generating 802.15.4
> MAC header before queuing into wpan interfaces transmit queue.

what about swapping patch 3 and 4?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Stefan Schmidt]

Hello.

On 22/05/15 10:59, Marc Kleine-Budde wrote:
> On 05/22/2015 10:57 AM, Alexander Aring wrote:
>> This patch will use atomic operations for sequence number incrementation
>> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
>> could call this function in a parallel context while generating 802.15.4
>> MAC header before queuing into wpan interfaces transmit queue.
> what about swapping patch 3 and 4?

To avoid having problems during a git bisect later one? E.g. having the 
lock removed but no atomic in place?

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

On 05/22/2015 12:30 PM, Stefan Schmidt wrote:
> Hello.
> 
> On 22/05/15 10:59, Marc Kleine-Budde wrote:
>> On 05/22/2015 10:57 AM, Alexander Aring wrote:
>>> This patch will use atomic operations for sequence number incrementation
>>> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
>>> could call this function in a parallel context while generating 802.15.4
>>> MAC header before queuing into wpan interfaces transmit queue.
>> what about swapping patch 3 and 4?
> 
> To avoid having problems during a git bisect later one? E.g. having the 
> lock removed but no atomic in place?

Yes, that's what I was thinking about. I don't know the code to tell if
this is an issue here.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Stefan Schmidt]

Hello.

On 22/05/15 12:34, Marc Kleine-Budde wrote:
> On 05/22/2015 12:30 PM, Stefan Schmidt wrote:
>> Hello.
>>
>> On 22/05/15 10:59, Marc Kleine-Budde wrote:
>>> On 05/22/2015 10:57 AM, Alexander Aring wrote:
>>>> This patch will use atomic operations for sequence number incrementation
>>>> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
>>>> could call this function in a parallel context while generating 802.15.4
>>>> MAC header before queuing into wpan interfaces transmit queue.
>>> what about swapping patch 3 and 4?
>> To avoid having problems during a git bisect later one? E.g. having the
>> lock removed but no atomic in place?
> Yes, that's what I was thinking about. I don't know the code to tell if
> this is an issue here.

A good point. I'm not sure either. I would say we could play safe and 
swap them. Alex, guess its your final call as you know the code best 
from us. Could we have problems with the locks removed but the atomic 
patch not applied (as it could happen with git bisect right now)? If 
yes, swapping them makes sense to avoid this.

Btw, I'm running some light testing with 6lowpan pings with these four 
patches applied right now. Looks good so far. Will send another mail 
once its done.

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Alexander Aring]

On Fri, May 22, 2015 at 12:34:19PM +0200, Marc Kleine-Budde wrote:
> On 05/22/2015 12:30 PM, Stefan Schmidt wrote:
> > Hello.
> > 
> > On 22/05/15 10:59, Marc Kleine-Budde wrote:
> >> On 05/22/2015 10:57 AM, Alexander Aring wrote:
> >>> This patch will use atomic operations for sequence number incrementation
> >>> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
> >>> could call this function in a parallel context while generating 802.15.4
> >>> MAC header before queuing into wpan interfaces transmit queue.
> >> what about swapping patch 3 and 4?
> > 
> > To avoid having problems during a git bisect later one? E.g. having the 
> > lock removed but no atomic in place?
> 
> Yes, that's what I was thinking about. I don't know the code to tell if
> this is an issue here.
> 

The problem is more difficult because the dsn incrementation which I do
atomic now had never a locking mechanism. So this was always not working
correctly. Somebody need to scream now "hey fix that in net, not next".
I do at the moment only critical things fixed in net, like [0].


But we should swap it because I first wrote some "TODO we should use
atomic here" and then later I decide to implement this TODO. At the end
this results in some cherry-pick orgy and I did not change it.

- Alex

[0] ("mac802154: tx: fix sync xmit handling") can't find them in the
    archive

Re: [PATCH bluetooth-next 1/4] mac802154: fix hold rtnl while ioctl

[Stefan Schmidt]

Hello.

On 22/05/15 10:57, Alexander Aring wrote:
> This patch fixes an issue to set address configuration with ioctl.
> Accessing the mib requires rtnl lock and the ndo_do_ioctl doesn't hold
> the rtnl lock while this callback is called. This patch do that
> manually.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> Reported-by: Matteo Petracca <matteo.petracca@sssup.it>
> ---
>   net/mac802154/iface.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 91b75ab..2a58788 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -62,8 +62,7 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   		(struct sockaddr_ieee802154 *)&ifr->ifr_addr;
>   	int err = -ENOIOCTLCMD;
>   
> -	ASSERT_RTNL();
> -
> +	rtnl_lock();
>   	spin_lock_bh(&sdata->mib_lock);
>   
>   	switch (cmd) {
> @@ -90,6 +89,7 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   	case SIOCSIFADDR:
>   		if (netif_running(dev)) {
>   			spin_unlock_bh(&sdata->mib_lock);
> +			rtnl_unlock();
>   			return -EBUSY;
>   		}
>   
> @@ -112,6 +112,7 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   	}
>   
>   	spin_unlock_bh(&sdata->mib_lock);
> +	rtnl_unlock();
>   	return err;
>   }
>   
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

This includes some smoke testing between two nodes (atusb and 
at86rf233), Basically ping6 with various sizes and a count of 1000 
pings. No problem showed up with these patches during the testing.

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 2/4] mac802154: remove pib lock

[Stefan Schmidt]

Hello.

On 22/05/15 10:57, Alexander Aring wrote:
> This patch removes the pib lock which is now replaced by rtnl lock. The
> new interface already use the rtnl lock only. Nevertheless this patch
> will fix issues while using new and old interface at the same time.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   include/net/cfg802154.h | 2 --
>   net/ieee802154/core.c   | 2 --
>   net/ieee802154/nl-phy.c | 6 +++---
>   net/mac802154/iface.c   | 7 -------
>   net/mac802154/mib.c     | 4 ++--
>   5 files changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> index 11bbf17..c6aa1d2 100644
> --- a/include/net/cfg802154.h
> +++ b/include/net/cfg802154.h
> @@ -121,8 +121,6 @@ enum wpan_phy_flags {
>   };
>   
>   struct wpan_phy {
> -	struct mutex pib_lock;
> -
>   	/* If multiple wpan_phys are registered and you're handed e.g.
>   	 * a regular netdev with assigned ieee802154_ptr, you won't
>   	 * know whether it points to a wpan_phy your driver has registered
> diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
> index 2ee00e8..b0248e9 100644
> --- a/net/ieee802154/core.c
> +++ b/net/ieee802154/core.c
> @@ -121,8 +121,6 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size)
>   	/* atomic_inc_return makes it start at 1, make it start at 0 */
>   	rdev->wpan_phy_idx--;
>   
> -	mutex_init(&rdev->wpan_phy.pib_lock);
> -
>   	INIT_LIST_HEAD(&rdev->wpan_dev_list);
>   	device_initialize(&rdev->wpan_phy.dev);
>   	dev_set_name(&rdev->wpan_phy.dev, PHY_NAME "%d", rdev->wpan_phy_idx);
> diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
> index cbc0d09..77d7301 100644
> --- a/net/ieee802154/nl-phy.c
> +++ b/net/ieee802154/nl-phy.c
> @@ -50,7 +50,7 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
>   	if (!hdr)
>   		goto out;
>   
> -	mutex_lock(&phy->pib_lock);
> +	rtnl_lock();
>   	if (nla_put_string(msg, IEEE802154_ATTR_PHY_NAME, wpan_phy_name(phy)) ||
>   	    nla_put_u8(msg, IEEE802154_ATTR_PAGE, phy->current_page) ||
>   	    nla_put_u8(msg, IEEE802154_ATTR_CHANNEL, phy->current_channel))
> @@ -63,13 +63,13 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
>   	    nla_put(msg, IEEE802154_ATTR_CHANNEL_PAGE_LIST,
>   		    pages * sizeof(uint32_t), buf))
>   		goto nla_put_failure;
> -	mutex_unlock(&phy->pib_lock);
> +	rtnl_unlock();
>   	kfree(buf);
>   	genlmsg_end(msg, hdr);
>   	return 0;
>   
>   nla_put_failure:
> -	mutex_unlock(&phy->pib_lock);
> +	rtnl_unlock();
>   	genlmsg_cancel(msg, hdr);
>   out:
>   	kfree(buf);
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 2a58788..22f478b 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -242,7 +242,6 @@ static int mac802154_wpan_open(struct net_device *dev)
>   	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
>   	struct ieee802154_local *local = sdata->local;
>   	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
> -	struct wpan_phy *phy = sdata->local->phy;
>   
>   	rc = ieee802154_check_concurrent_iface(sdata, sdata->vif.type);
>   	if (rc < 0)
> @@ -252,8 +251,6 @@ static int mac802154_wpan_open(struct net_device *dev)
>   	if (rc < 0)
>   		return rc;
>   
> -	mutex_lock(&phy->pib_lock);
> -
>   	if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) {
>   		rc = drv_set_promiscuous_mode(local,
>   					      wpan_dev->promiscuous_mode);
> @@ -295,11 +292,7 @@ static int mac802154_wpan_open(struct net_device *dev)
>   			goto out;
>   	}
>   
> -	mutex_unlock(&phy->pib_lock);
> -	return 0;

Hmm, why did you remove the return 0; here? Is this supposed to fall 
through to out: and use the return rc now?

>   out:
> -	mutex_unlock(&phy->pib_lock);
>   	return rc;
>   }
>   
> diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
> index 5cf019a..033dfc7 100644
> --- a/net/mac802154/mib.c
> +++ b/net/mac802154/mib.c
> @@ -91,16 +91,16 @@ void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan)
>   	struct ieee802154_local *local = sdata->local;
>   	int res;
>   
> +	ASSERT_RTNL();
> +
>   	BUG_ON(dev->type != ARPHRD_IEEE802154);
>   
>   	res = drv_set_channel(local, page, chan);
>   	if (res) {
>   		pr_debug("set_channel failed\n");
>   	} else {
> -		mutex_lock(&local->phy->pib_lock);
>   		local->phy->current_channel = chan;
>   		local->phy->current_page = page;
> -		mutex_unlock(&local->phy->pib_lock);
>   	}
>   }
>   

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 3/4] mac802154: remove mib lock

[Stefan Schmidt]

Hello.

On 22/05/15 10:57, Alexander Aring wrote:
> This patch removes the mib lock. The new locking mechanism is to protect
> the mib values with the rtnl lock. Note that this isn't always necessary
> if we have an interface up the most mib values are readonly (e.g.
> address settings). With this behaviour we can remove locking in
> hotpath like frame parsing completely. It depends on context if we need
> to hold the rtnl lock or not, this makes the callbacks of
> ieee802154_mlme_ops unnecessary because these callbacks hols always the
> locks.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   include/net/ieee802154_netdev.h | 10 -------
>   net/ieee802154/6lowpan/core.c   | 28 -------------------
>   net/ieee802154/6lowpan/tx.c     |  2 +-
>   net/ieee802154/nl-mac.c         | 14 +++++++---
>   net/ieee802154/socket.c         | 12 ++++-----
>   net/mac802154/ieee802154_i.h    |  7 -----
>   net/mac802154/iface.c           | 13 +++------
>   net/mac802154/mac_cmd.c         |  7 ++---
>   net/mac802154/mib.c             | 59 -----------------------------------------
>   net/mac802154/rx.c              |  5 ----
>   10 files changed, 22 insertions(+), 135 deletions(-)
>
> diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
> index 94a2970..84a72a1 100644
> --- a/include/net/ieee802154_netdev.h
> +++ b/include/net/ieee802154_netdev.h
> @@ -422,16 +422,6 @@ struct ieee802154_mlme_ops {
>   			       struct ieee802154_mac_params *params);
>   
>   	struct ieee802154_llsec_ops *llsec;
> -
> -	/* The fields below are required. */
> -
> -	/*
> -	 * FIXME: these should become the part of PIB/MIB interface.
> -	 * However we still don't have IB interface of any kind
> -	 */
> -	__le16 (*get_pan_id)(const struct net_device *dev);
> -	__le16 (*get_short_addr)(const struct net_device *dev);
> -	u8 (*get_dsn)(const struct net_device *dev);
>   };
>   
>   static inline struct ieee802154_mlme_ops *
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 0ae5822..f20a387 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -55,27 +55,6 @@
>   LIST_HEAD(lowpan_devices);
>   static int lowpan_open_count;
>   
> -static __le16 lowpan_get_pan_id(const struct net_device *dev)
> -{
> -	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
> -
> -	return ieee802154_mlme_ops(real_dev)->get_pan_id(real_dev);
> -}
> -
> -static __le16 lowpan_get_short_addr(const struct net_device *dev)
> -{
> -	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
> -
> -	return ieee802154_mlme_ops(real_dev)->get_short_addr(real_dev);
> -}
> -
> -static u8 lowpan_get_dsn(const struct net_device *dev)
> -{
> -	struct net_device *real_dev = lowpan_dev_info(dev)->real_dev;
> -
> -	return ieee802154_mlme_ops(real_dev)->get_dsn(real_dev);
> -}
> -
>   static struct header_ops lowpan_header_ops = {
>   	.create	= lowpan_header_create,
>   };
> @@ -103,12 +82,6 @@ static const struct net_device_ops lowpan_netdev_ops = {
>   	.ndo_start_xmit		= lowpan_xmit,
>   };
>   
> -static struct ieee802154_mlme_ops lowpan_mlme = {
> -	.get_pan_id = lowpan_get_pan_id,
> -	.get_short_addr = lowpan_get_short_addr,
> -	.get_dsn = lowpan_get_dsn,
> -};
> -
>   static void lowpan_setup(struct net_device *dev)
>   {
>   	dev->addr_len		= IEEE802154_ADDR_LEN;
> @@ -124,7 +97,6 @@ static void lowpan_setup(struct net_device *dev)
>   
>   	dev->netdev_ops		= &lowpan_netdev_ops;
>   	dev->header_ops		= &lowpan_header_ops;
> -	dev->ml_priv		= &lowpan_mlme;
>   	dev->destructor		= free_netdev;
>   	dev->features		|= NETIF_F_NETNS_LOCAL;
>   }
> diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
> index 2349070..98acf73 100644
> --- a/net/ieee802154/6lowpan/tx.c
> +++ b/net/ieee802154/6lowpan/tx.c
> @@ -207,7 +207,7 @@ static int lowpan_header(struct sk_buff *skb, struct net_device *dev)
>   
>   	/* prepare wpan address data */
>   	sa.mode = IEEE802154_ADDR_LONG;
> -	sa.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
> +	sa.pan_id = lowpan_dev_info(dev)->real_dev->ieee802154_ptr->pan_id;
>   	sa.extended_addr = ieee802154_devaddr_from_raw(saddr);
>   
>   	/* intra-PAN communications */
> diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
> index cdc1cc3..ada58a8 100644
> --- a/net/ieee802154/nl-mac.c
> +++ b/net/ieee802154/nl-mac.c
> @@ -97,8 +97,10 @@ static int ieee802154_nl_fill_iface(struct sk_buff *msg, u32 portid,
>   	BUG_ON(!phy);
>   	get_device(&phy->dev);
>   
> -	short_addr = ops->get_short_addr(dev);
> -	pan_id = ops->get_pan_id(dev);
> +	rtnl_lock();
> +	short_addr = dev->ieee802154_ptr->short_addr;
> +	pan_id = dev->ieee802154_ptr->pan_id;
> +	rtnl_unlock();
>   
>   	if (nla_put_string(msg, IEEE802154_ATTR_DEV_NAME, dev->name) ||
>   	    nla_put_string(msg, IEEE802154_ATTR_PHY_NAME, wpan_phy_name(phy)) ||
> @@ -244,7 +246,9 @@ int ieee802154_associate_resp(struct sk_buff *skb, struct genl_info *info)
>   	addr.mode = IEEE802154_ADDR_LONG;
>   	addr.extended_addr = nla_get_hwaddr(
>   			info->attrs[IEEE802154_ATTR_DEST_HW_ADDR]);
> -	addr.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
> +	rtnl_lock();
> +	addr.pan_id = dev->ieee802154_ptr->pan_id;
> +	rtnl_unlock();
>   
>   	ret = ieee802154_mlme_ops(dev)->assoc_resp(dev, &addr,
>   		nla_get_shortaddr(info->attrs[IEEE802154_ATTR_DEST_SHORT_ADDR]),
> @@ -281,7 +285,9 @@ int ieee802154_disassociate_req(struct sk_buff *skb, struct genl_info *info)
>   		addr.short_addr = nla_get_shortaddr(
>   				info->attrs[IEEE802154_ATTR_DEST_SHORT_ADDR]);
>   	}
> -	addr.pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
> +	rtnl_lock();
> +	addr.pan_id = dev->ieee802154_ptr->pan_id;
> +	rtnl_unlock();
>   
>   	ret = ieee802154_mlme_ops(dev)->disassoc_req(dev, &addr,
>   			nla_get_u8(info->attrs[IEEE802154_ATTR_REASON]));
> diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
> index 7aaaf96..e5cc253 100644
> --- a/net/ieee802154/socket.c
> +++ b/net/ieee802154/socket.c
> @@ -64,10 +64,8 @@ ieee802154_get_dev(struct net *net, const struct ieee802154_addr *addr)
>   			if (tmp->type != ARPHRD_IEEE802154)
>   				continue;
>   
> -			pan_id = ieee802154_mlme_ops(tmp)->get_pan_id(tmp);
> -			short_addr =
> -				ieee802154_mlme_ops(tmp)->get_short_addr(tmp);
> -
> +			pan_id = tmp->ieee802154_ptr->pan_id;
> +			short_addr = tmp->ieee802154_ptr->short_addr;
>   			if (pan_id == addr->pan_id &&
>   			    short_addr == addr->short_addr) {
>   				dev = tmp;
> @@ -797,9 +795,9 @@ static int ieee802154_dgram_deliver(struct net_device *dev, struct sk_buff *skb)
>   	/* Data frame processing */
>   	BUG_ON(dev->type != ARPHRD_IEEE802154);
>   
> -	pan_id = ieee802154_mlme_ops(dev)->get_pan_id(dev);
> -	short_addr = ieee802154_mlme_ops(dev)->get_short_addr(dev);
> -	hw_addr = ieee802154_devaddr_from_raw(dev->dev_addr);
> +	pan_id = dev->ieee802154_ptr->pan_id;
> +	short_addr = dev->ieee802154_ptr->short_addr;
> +	hw_addr = dev->ieee802154_ptr->extended_addr;
>   
>   	read_lock(&dgram_lock);
>   	sk_for_each(sk, &dgram_head) {
> diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
> index 127ba18..eec668f 100644
> --- a/net/mac802154/ieee802154_i.h
> +++ b/net/mac802154/ieee802154_i.h
> @@ -86,8 +86,6 @@ struct ieee802154_sub_if_data {
>   	unsigned long state;
>   	char name[IFNAMSIZ];
>   
> -	spinlock_t mib_lock;
> -
>   	/* protects sec from concurrent access by netlink. access by
>   	 * encrypt/decrypt/header_create safe without additional protection.
>   	 */
> @@ -136,12 +134,7 @@ ieee802154_subif_start_xmit(struct sk_buff *skb, struct net_device *dev);
>   enum hrtimer_restart ieee802154_xmit_ifs_timer(struct hrtimer *timer);
>   
>   /* MIB callbacks */
> -void mac802154_dev_set_short_addr(struct net_device *dev, __le16 val);
> -__le16 mac802154_dev_get_short_addr(const struct net_device *dev);
> -__le16 mac802154_dev_get_pan_id(const struct net_device *dev);
> -void mac802154_dev_set_pan_id(struct net_device *dev, __le16 val);
>   void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan);
> -u8 mac802154_dev_get_dsn(const struct net_device *dev);
>   
>   int mac802154_get_params(struct net_device *dev,
>   			 struct ieee802154_llsec_params *params);
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 22f478b..866d27f 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -63,7 +63,6 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   	int err = -ENOIOCTLCMD;
>   
>   	rtnl_lock();
> -	spin_lock_bh(&sdata->mib_lock);
>   
>   	switch (cmd) {
>   	case SIOCGIFADDR:
> @@ -88,7 +87,6 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   	}
>   	case SIOCSIFADDR:
>   		if (netif_running(dev)) {
> -			spin_unlock_bh(&sdata->mib_lock);
>   			rtnl_unlock();
>   			return -EBUSY;
>   		}
> @@ -111,7 +109,6 @@ mac802154_wpan_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>   		break;
>   	}
>   
> -	spin_unlock_bh(&sdata->mib_lock);
>   	rtnl_unlock();
>   	return err;
>   }
> @@ -368,14 +365,15 @@ static int mac802154_header_create(struct sk_buff *skb,
>   	hdr.fc.type = cb->type;
>   	hdr.fc.security_enabled = cb->secen;
>   	hdr.fc.ack_request = cb->ackreq;
> -	hdr.seq = ieee802154_mlme_ops(dev)->get_dsn(dev);
> +	/* TODO: use atomic_t as dsn, dsn need to be locked when AF_IEEE802154
> +	 * and IEEE802154 6LoWPAN call this at the same time.
> +	 */
> +	hdr.seq = dev->ieee802154_ptr->dsn++;

OK, given this comment I think swapping 3 and 4 would really make sense.

>   
>   	if (mac802154_set_header_security(sdata, &hdr, cb) < 0)
>   		return -EINVAL;
>   
>   	if (!saddr) {
> -		spin_lock_bh(&sdata->mib_lock);
> -
>   		if (wpan_dev->short_addr == cpu_to_le16(IEEE802154_ADDR_BROADCAST) ||
>   		    wpan_dev->short_addr == cpu_to_le16(IEEE802154_ADDR_UNDEF) ||
>   		    wpan_dev->pan_id == cpu_to_le16(IEEE802154_PANID_BROADCAST)) {
> @@ -387,8 +385,6 @@ static int mac802154_header_create(struct sk_buff *skb,
>   		}
>   
>   		hdr.source.pan_id = wpan_dev->pan_id;
> -
> -		spin_unlock_bh(&sdata->mib_lock);
>   	} else {
>   		hdr.source = *(const struct ieee802154_addr *)saddr;
>   	}
> @@ -497,7 +493,6 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
>   		sdata->dev->ml_priv = &mac802154_mlme_wpan;
>   		wpan_dev->promiscuous_mode = false;
>   
> -		spin_lock_init(&sdata->mib_lock);
>   		mutex_init(&sdata->sec_mtx);
>   
>   		mac802154_llsec_init(&sdata->sec);
> diff --git a/net/mac802154/mac_cmd.c b/net/mac802154/mac_cmd.c
> index 6dcbb3b..5220c2b2 100644
> --- a/net/mac802154/mac_cmd.c
> +++ b/net/mac802154/mac_cmd.c
> @@ -43,8 +43,8 @@ static int mac802154_mlme_start_req(struct net_device *dev,
>   
>   	BUG_ON(addr->mode != IEEE802154_ADDR_SHORT);
>   
> -	mac802154_dev_set_pan_id(dev, addr->pan_id);
> -	mac802154_dev_set_short_addr(dev, addr->short_addr);
> +	dev->ieee802154_ptr->pan_id = addr->pan_id;
> +	dev->ieee802154_ptr->short_addr = addr->short_addr;
>   	mac802154_dev_set_page_channel(dev, page, channel);
>   
>   	if (ops->llsec) {
> @@ -151,9 +151,6 @@ static struct ieee802154_llsec_ops mac802154_llsec_ops = {
>   
>   struct ieee802154_mlme_ops mac802154_mlme_wpan = {
>   	.start_req = mac802154_mlme_start_req,
> -	.get_pan_id = mac802154_dev_get_pan_id,
> -	.get_short_addr = mac802154_dev_get_short_addr,
> -	.get_dsn = mac802154_dev_get_dsn,
>   
>   	.llsec = &mac802154_llsec_ops,
>   
> diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
> index 033dfc7..73f94fb 100644
> --- a/net/mac802154/mib.c
> +++ b/net/mac802154/mib.c
> @@ -26,65 +26,6 @@
>   #include "ieee802154_i.h"
>   #include "driver-ops.h"
>   
> -void mac802154_dev_set_short_addr(struct net_device *dev, __le16 val)
> -{
> -	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> -
> -	BUG_ON(dev->type != ARPHRD_IEEE802154);
> -
> -	spin_lock_bh(&sdata->mib_lock);
> -	sdata->wpan_dev.short_addr = val;
> -	spin_unlock_bh(&sdata->mib_lock);
> -}
> -
> -__le16 mac802154_dev_get_short_addr(const struct net_device *dev)
> -{
> -	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> -	__le16 ret;
> -
> -	BUG_ON(dev->type != ARPHRD_IEEE802154);
> -
> -	spin_lock_bh(&sdata->mib_lock);
> -	ret = sdata->wpan_dev.short_addr;
> -	spin_unlock_bh(&sdata->mib_lock);
> -
> -	return ret;
> -}
> -
> -__le16 mac802154_dev_get_pan_id(const struct net_device *dev)
> -{
> -	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> -	__le16 ret;
> -
> -	BUG_ON(dev->type != ARPHRD_IEEE802154);
> -
> -	spin_lock_bh(&sdata->mib_lock);
> -	ret = sdata->wpan_dev.pan_id;
> -	spin_unlock_bh(&sdata->mib_lock);
> -
> -	return ret;
> -}
> -
> -void mac802154_dev_set_pan_id(struct net_device *dev, __le16 val)
> -{
> -	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> -
> -	BUG_ON(dev->type != ARPHRD_IEEE802154);
> -
> -	spin_lock_bh(&sdata->mib_lock);
> -	sdata->wpan_dev.pan_id = val;
> -	spin_unlock_bh(&sdata->mib_lock);
> -}
> -
> -u8 mac802154_dev_get_dsn(const struct net_device *dev)
> -{
> -	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> -
> -	BUG_ON(dev->type != ARPHRD_IEEE802154);
> -
> -	return sdata->wpan_dev.dsn++;
> -}
> -
>   void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan)
>   {
>   	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
> index c0d67b2..e0f1006 100644
> --- a/net/mac802154/rx.c
> +++ b/net/mac802154/rx.c
> @@ -47,8 +47,6 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
>   
>   	pr_debug("getting packet via slave interface %s\n", sdata->dev->name);
>   
> -	spin_lock_bh(&sdata->mib_lock);
> -
>   	span = wpan_dev->pan_id;
>   	sshort = wpan_dev->short_addr;
>   
> @@ -83,13 +81,10 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
>   			skb->pkt_type = PACKET_OTHERHOST;
>   		break;
>   	default:
> -		spin_unlock_bh(&sdata->mib_lock);
>   		pr_debug("invalid dest mode\n");
>   		goto fail;
>   	}
>   
> -	spin_unlock_bh(&sdata->mib_lock);
> -
>   	skb->dev = sdata->dev;
>   
>   	rc = mac802154_llsec_decrypt(&sdata->sec, skb);

After swapping 3 and for you can add:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

This includes some smoke testing between two nodes (atusb and 
at86rf233), Basically ping6 with various sizes and a count of 1000 
pings. No problem showed up with these patches during the testing.

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Stefan Schmidt]

Hello.

On 22/05/15 10:57, Alexander Aring wrote:
> This patch will use atomic operations for sequence number incrementation
> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
> could call this function in a parallel context while generating 802.15.4
> MAC header before queuing into wpan interfaces transmit queue.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   include/net/cfg802154.h |  4 ++--
>   net/mac802154/iface.c   | 12 ++++++------
>   2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> index c6aa1d2..4de59aa 100644
> --- a/include/net/cfg802154.h
> +++ b/include/net/cfg802154.h
> @@ -177,9 +177,9 @@ struct wpan_dev {
>   	__le64 extended_addr;
>   
>   	/* MAC BSN field */
> -	u8 bsn;
> +	atomic_t bsn;
>   	/* MAC DSN field */
> -	u8 dsn;
> +	atomic_t dsn;
>   
>   	u8 min_be;
>   	u8 max_be;
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 866d27f..f30788d 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -365,10 +365,7 @@ static int mac802154_header_create(struct sk_buff *skb,
>   	hdr.fc.type = cb->type;
>   	hdr.fc.security_enabled = cb->secen;
>   	hdr.fc.ack_request = cb->ackreq;
> -	/* TODO: use atomic_t as dsn, dsn need to be locked when AF_IEEE802154
> -	 * and IEEE802154 6LoWPAN call this at the same time.
> -	 */
> -	hdr.seq = dev->ieee802154_ptr->dsn++;
> +	hdr.seq = atomic_inc_return(&dev->ieee802154_ptr->dsn) & 0xFF;

After reviewing this in its full glory I also think the order needs to 
change an patch number 4 should come before 3.

>   
>   	if (mac802154_set_header_security(sdata, &hdr, cb) < 0)
>   		return -EINVAL;
> @@ -464,13 +461,16 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
>   		       enum nl802154_iftype type)
>   {
>   	struct wpan_dev *wpan_dev = &sdata->wpan_dev;
> +	u8 tmp;
>   
>   	/* set some type-dependent values */
>   	sdata->vif.type = type;
>   	sdata->wpan_dev.iftype = type;
>   
> -	get_random_bytes(&wpan_dev->bsn, 1);
> -	get_random_bytes(&wpan_dev->dsn, 1);
> +	get_random_bytes(&tmp, sizeof(tmp));
> +	atomic_set(&wpan_dev->bsn, tmp);
> +	get_random_bytes(&tmp, sizeof(tmp));
> +	atomic_set(&wpan_dev->dsn, tmp);
>   
>   	/* defaults per 802.15.4-2011 */
>   	wpan_dev->min_be = 3;

After swapping you can add:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

This includes some smoke testing between two nodes (atusb and 
at86rf233), Basically ping6 with various sizes and a count of 1000 
pings. No problem showed up with these patches during the testing.

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 4/4] mac802154: use atomic ops for sequence incrementation

[Stefan Schmidt]

Hello.

On 22/05/15 14:12, Alexander Aring wrote:
> On Fri, May 22, 2015 at 12:34:19PM +0200, Marc Kleine-Budde wrote:
>> On 05/22/2015 12:30 PM, Stefan Schmidt wrote:
>>> Hello.
>>>
>>> On 22/05/15 10:59, Marc Kleine-Budde wrote:
>>>> On 05/22/2015 10:57 AM, Alexander Aring wrote:
>>>>> This patch will use atomic operations for sequence number incrementation
>>>>> while MAC header generation. Upper layers like af_802154 or 6LoWPAN
>>>>> could call this function in a parallel context while generating 802.15.4
>>>>> MAC header before queuing into wpan interfaces transmit queue.
>>>> what about swapping patch 3 and 4?
>>> To avoid having problems during a git bisect later one? E.g. having the
>>> lock removed but no atomic in place?
>> Yes, that's what I was thinking about. I don't know the code to tell if
>> this is an issue here.
>>
> The problem is more difficult because the dsn incrementation which I do
> atomic now had never a locking mechanism. So this was always not working
> correctly. Somebody need to scream now "hey fix that in net, not next".

Hey, fix that in net not next! :P

If we know it causes problems in current net we should indeed try to 
submit it there.
Given it is only a few lines it might be worth the effort even now. Some 
people might be using kernel releases for their work and thus they could 
get this fix one release (3 months) earlier.

> I do at the moment only critical things fixed in net, like [0].
>
Sure, these are the most important ones. I leave it to you to decide if 
you think this one is worth the extra effort.

> But we should swap it because I first wrote some "TODO we should use
> atomic here" and then later I decide to implement this TODO. At the end
> this results in some cherry-pick orgy and I did not change it.
>
ok, cool.

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 2/4] mac802154: remove pib lock

[Alexander Aring]

On Fri, May 22, 2015 at 02:19:05PM +0200, Stefan Schmidt wrote:

...

> >  	if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) {
> >  		rc = drv_set_promiscuous_mode(local,
> >  					      wpan_dev->promiscuous_mode);
> >@@ -295,11 +292,7 @@ static int mac802154_wpan_open(struct net_device *dev)
> >  			goto out;
> >  	}
> >-	mutex_unlock(&phy->pib_lock);
> >-	return 0;
> 
> Hmm, why did you remove the return 0; here? Is this supposed to fall through
> to out: and use the return rc now?
> 

This function is the netdev open call of an wpan interface. In this call
we do at the moment all MAC dependent settings which are done by phy.
e.g. address filtering, frame reties, etc...

When this call failed the interface is still down. This callback is
usually called on an ifup. If it's successful it's up, if not still down.

The last call is:

        if (local->hw.flags & IEEE802154_HW_FRAME_RETRIES) {
                rc = drv_set_max_frame_retries(local, wpan_dev->frame_retries);
                if (rc < 0)
                        goto out;
        }

Which is the driver callback which have an identically return value
indicator, means -ERRNO if failed, according to the netdev open call.


We can still use the return 0 on successful or the rc. It's a silent
cleanup which I did here (hope that was okay).


btw:
Also I don't know why there is a pib hold, when some of them are mib
attributes. This all makes no sense for me.

To hold the pib_lock will represent the documentation of driver_ops, but
this makes no sense when you hold the pib lock for mib attributes in
this case of callback and when accessing mib attributes over netlink or
such else to hold the mib lock and not the pib lock, something is wrong
there.

Now, we doing everything over rtnl, which can be indicated by [0] flag
to hold this lock while netlink command and the _most_ mib attributes are
readonly while interface up. This will occur that we need don't care
about locking if the interface is up. For attributes like address
filtering this is also "impossible" to set address filtering registers
while the interface is running. The best option here is to set address
filtering while ifup and then don't allo the change the addresses while
having the interface up. This is what we have now.


The dsn/bsn values are special here, this need to be writeable while ifup.
Also we need to think about handling setting of short_address while
assoc with coordinators, but I think this will take some time to support
such feature. I am sure we will find some solution.

- Alex

[0] http://lxr.free-electrons.com/source/net/ieee802154/nl802154.c#L799

Re: [PATCH bluetooth-next 2/4] mac802154: remove pib lock

[Stefan Schmidt]

Hello.

On 22/05/15 14:41, Alexander Aring wrote:
> On Fri, May 22, 2015 at 02:19:05PM +0200, Stefan Schmidt wrote:
>
> ...
>
>>>   	if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) {
>>>   		rc = drv_set_promiscuous_mode(local,
>>>   					      wpan_dev->promiscuous_mode);
>>> @@ -295,11 +292,7 @@ static int mac802154_wpan_open(struct net_device *dev)
>>>   			goto out;
>>>   	}
>>> -	mutex_unlock(&phy->pib_lock);
>>> -	return 0;
>> Hmm, why did you remove the return 0; here? Is this supposed to fall through
>> to out: and use the return rc now?
>>
> This function is the netdev open call of an wpan interface. In this call
> we do at the moment all MAC dependent settings which are done by phy.
> e.g. address filtering, frame reties, etc...
>
> When this call failed the interface is still down. This callback is
> usually called on an ifup. If it's successful it's up, if not still down.
>
> The last call is:
>
>          if (local->hw.flags & IEEE802154_HW_FRAME_RETRIES) {
>                  rc = drv_set_max_frame_retries(local, wpan_dev->frame_retries);
>                  if (rc < 0)
>                          goto out;
>          }
>
> Which is the driver callback which have an identically return value
> indicator, means -ERRNO if failed, according to the netdev open call.
>
>
> We can still use the return 0 on successful or the rc. It's a silent
> cleanup which I did here (hope that was okay).
OK, so this change does not change the return value in any condition. 
That was the part I was worried about.

A silent cleanup is fine but it made me suspicious during the review. :)

>
> btw:
> Also I don't know why there is a pib hold, when some of them are mib
> attributes. This all makes no sense for me.
>
> To hold the pib_lock will represent the documentation of driver_ops, but
> this makes no sense when you hold the pib lock for mib attributes in
> this case of callback and when accessing mib attributes over netlink or
> such else to hold the mib lock and not the pib lock, something is wrong
> there.
>
> Now, we doing everything over rtnl, which can be indicated by [0] flag
> to hold this lock while netlink command and the _most_ mib attributes are
> readonly while interface up. This will occur that we need don't care
> about locking if the interface is up. For attributes like address
> filtering this is also "impossible" to set address filtering registers
> while the interface is running. The best option here is to set address
> filtering while ifup and then don't allo the change the addresses while
> having the interface up. This is what we have now.
>
>
> The dsn/bsn values are special here, this need to be writeable while ifup.
> Also we need to think about handling setting of short_address while
> assoc with coordinators, but I think this will take some time to support
> such feature. I am sure we will find some solution.

Yeah, we need to revive the coordinator functionality at all for this to 
be a problem. :)

With the above explained you can add my:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt