[PATCHv2 bluetooth-next 0/2] ieee802154: multiple lowpan interface support and fix mac setting

[PATCHv2 bluetooth-next 0/2] ieee802154: multiple lowpan interface support and fix mac setting

[Alexander Aring]

Hi,

this patch series contains two patches. At first we remove the multiple
lowpan interface support. I don't see any use case for that, if somebody
has an use case for it. Please report your use case.

Currently it's broken to update the wpan mac address while having a lowpan
interface on the top. The first patch will introduce some easy mechanism
to access the lowpan interface inside the lower mac802154 layer. When the
mac802154 changes the mac address then it will updated also on the lowpan
interface, if the wpan interface belongs to a lowpan interface.

Another idea would be maybe to react on the lowpan_device_event, when a
wpan interface updates his address, but this doesn't work because the
lowpan interface need to be down to update the mac setting, otherwise the
SLAAC address will not be updated.

- Alex

changes since v2:
 - fix handling in patch 2/2 that we will check at first that the lowpan
   interface is down. Otherwise we had return -EBUSY and wpan interface
   address was updated and not the lowpan interface address.
 - change subject at patch 2/2 from "fix wpan mac setting while lowpan" to
   "fix wpan mac setting while lowpan is there". I don't use "up", because
   it was broken before, doesn't matter if the interface was up or down.
 - add several tested-by and reviewed-by tags

Alexander Aring (2):
  ieee802154: 6lowpan: remove multiple lowpan support
  mac802154: fix wpan mac setting while lowpan is there

 include/net/cfg802154.h            |  3 ++
 net/ieee802154/6lowpan/6lowpan_i.h |  6 ----
 net/ieee802154/6lowpan/core.c      | 67 +++++++++++---------------------------
 net/ieee802154/6lowpan/rx.c        | 38 +++++----------------
 net/mac802154/iface.c              | 15 +++++++++
 5 files changed, 45 insertions(+), 84 deletions(-)

-- 
2.5.0

[PATCHv2 bluetooth-next 1/2] ieee802154: 6lowpan: remove multiple lowpan support

[Alexander Aring]

We currently supports multiple lowpan interfaces per wpan interface. I
never saw any use case into such functionality. We drop this feature now
because it's much easier do deal with address changes inside the under
laying wpan interface.

This patch removes the multiple lowpan interface and adds a lowpan_dev
netdev pointer into the wpan_dev, if this pointer isn't null the wpan
interface belongs to the assigned lowpan interface.

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
Tested-by: Stefan Schmidt <stefan@osg.samsung.com>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/cfg802154.h            |  3 ++
 net/ieee802154/6lowpan/6lowpan_i.h |  6 ----
 net/ieee802154/6lowpan/core.c      | 67 +++++++++++---------------------------
 net/ieee802154/6lowpan/rx.c        | 38 +++++----------------
 4 files changed, 30 insertions(+), 84 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 382f94b..e53b6bf 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -173,6 +173,9 @@ struct wpan_dev {
 	struct list_head list;
 	struct net_device *netdev;
 
+	/* lowpan interface, set when the wpan_dev belongs to one lowpan_dev */
+	struct net_device *lowpan_dev;
+
 	u32 identifier;
 
 	/* MAC PIB */
diff --git a/net/ieee802154/6lowpan/6lowpan_i.h b/net/ieee802154/6lowpan/6lowpan_i.h
index e50f69d..876f093 100644
--- a/net/ieee802154/6lowpan/6lowpan_i.h
+++ b/net/ieee802154/6lowpan/6lowpan_i.h
@@ -37,15 +37,9 @@ static inline u32 ieee802154_addr_hash(const struct ieee802154_addr *a)
 	}
 }
 
-struct lowpan_dev_record {
-	struct net_device *ldev;
-	struct list_head list;
-};
-
 /* private device info */
 struct lowpan_dev_info {
 	struct net_device	*real_dev; /* real WPAN device ptr */
-	struct mutex		dev_list_mtx; /* mutex for list ops */
 	u16			fragment_tag;
 };
 
diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index f20a387..a4edee8 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -52,9 +52,6 @@
 
 #include "6lowpan_i.h"
 
-LIST_HEAD(lowpan_devices);
-static int lowpan_open_count;
-
 static struct header_ops lowpan_header_ops = {
 	.create	= lowpan_header_create,
 };
@@ -114,7 +111,6 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 			  struct nlattr *tb[], struct nlattr *data[])
 {
 	struct net_device *real_dev;
-	struct lowpan_dev_record *entry;
 	int ret;
 
 	ASSERT_RTNL();
@@ -133,31 +129,19 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 		return -EINVAL;
 	}
 
-	lowpan_dev_info(dev)->real_dev = real_dev;
-	mutex_init(&lowpan_dev_info(dev)->dev_list_mtx);
-
-	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
-	if (!entry) {
+	if (real_dev->ieee802154_ptr->lowpan_dev) {
 		dev_put(real_dev);
-		lowpan_dev_info(dev)->real_dev = NULL;
-		return -ENOMEM;
+		return -EBUSY;
 	}
 
-	entry->ldev = dev;
-
+	lowpan_dev_info(dev)->real_dev = real_dev;
 	/* Set the lowpan hardware address to the wpan hardware address. */
 	memcpy(dev->dev_addr, real_dev->dev_addr, IEEE802154_ADDR_LEN);
 
-	mutex_lock(&lowpan_dev_info(dev)->dev_list_mtx);
-	INIT_LIST_HEAD(&entry->list);
-	list_add_tail(&entry->list, &lowpan_devices);
-	mutex_unlock(&lowpan_dev_info(dev)->dev_list_mtx);
-
 	ret = register_netdevice(dev);
 	if (ret >= 0) {
-		if (!lowpan_open_count)
-			lowpan_rx_init();
-		lowpan_open_count++;
+		real_dev->ieee802154_ptr->lowpan_dev = dev;
+		lowpan_rx_init();
 	}
 
 	return ret;
@@ -167,27 +151,12 @@ 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;
 
 	ASSERT_RTNL();
 
-	lowpan_open_count--;
-	if (!lowpan_open_count)
-		lowpan_rx_exit();
-
-	mutex_lock(&lowpan_dev_info(dev)->dev_list_mtx);
-	list_for_each_entry_safe(entry, tmp, &lowpan_devices, list) {
-		if (entry->ldev == dev) {
-			list_del(&entry->list);
-			kfree(entry);
-		}
-	}
-	mutex_unlock(&lowpan_dev_info(dev)->dev_list_mtx);
-
-	mutex_destroy(&lowpan_dev_info(dev)->dev_list_mtx);
-
-	unregister_netdevice_queue(dev, head);
-
+	lowpan_rx_exit();
+	real_dev->ieee802154_ptr->lowpan_dev = NULL;
+	unregister_netdevice(dev);
 	dev_put(real_dev);
 }
 
@@ -214,19 +183,21 @@ static int lowpan_device_event(struct notifier_block *unused,
 			       unsigned long event, void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-	LIST_HEAD(del_list);
-	struct lowpan_dev_record *entry, *tmp;
 
 	if (dev->type != ARPHRD_IEEE802154)
 		goto out;
 
-	if (event == NETDEV_UNREGISTER) {
-		list_for_each_entry_safe(entry, tmp, &lowpan_devices, list) {
-			if (lowpan_dev_info(entry->ldev)->real_dev == dev)
-				lowpan_dellink(entry->ldev, &del_list);
-		}
-
-		unregister_netdevice_many(&del_list);
+	switch (event) {
+	case NETDEV_UNREGISTER:
+		/* Check if wpan interface is unregistered that we
+		 * also delete possible lowpan interfaces which belongs
+		 * to the wpan interface.
+		 */
+		if (dev->ieee802154_ptr && dev->ieee802154_ptr->lowpan_dev)
+			lowpan_dellink(dev->ieee802154_ptr->lowpan_dev, NULL);
+		break;
+	default:
+		break;
 	}
 
 out:
diff --git a/net/ieee802154/6lowpan/rx.c b/net/ieee802154/6lowpan/rx.c
index 4be1d28..d6f5e8e 100644
--- a/net/ieee802154/6lowpan/rx.c
+++ b/net/ieee802154/6lowpan/rx.c
@@ -15,36 +15,14 @@
 
 #include "6lowpan_i.h"
 
-static int lowpan_give_skb_to_devices(struct sk_buff *skb,
-				      struct net_device *dev)
+static int lowpan_give_skb_to_device(struct sk_buff *skb,
+				     struct net_device *dev)
 {
-	struct lowpan_dev_record *entry;
-	struct sk_buff *skb_cp;
-	int stat = NET_RX_SUCCESS;
-
+	skb->dev = dev->ieee802154_ptr->lowpan_dev;
 	skb->protocol = htons(ETH_P_IPV6);
 	skb->pkt_type = PACKET_HOST;
 
-	rcu_read_lock();
-	list_for_each_entry_rcu(entry, &lowpan_devices, list)
-		if (lowpan_dev_info(entry->ldev)->real_dev == skb->dev) {
-			skb_cp = skb_copy(skb, GFP_ATOMIC);
-			if (!skb_cp) {
-				kfree_skb(skb);
-				rcu_read_unlock();
-				return NET_RX_DROP;
-			}
-
-			skb_cp->dev = entry->ldev;
-			stat = netif_rx(skb_cp);
-			if (stat == NET_RX_DROP)
-				break;
-		}
-	rcu_read_unlock();
-
-	consume_skb(skb);
-
-	return stat;
+	return netif_rx(skb);
 }
 
 static int
@@ -109,7 +87,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (skb->data[0] == LOWPAN_DISPATCH_IPV6) {
 		/* Pull off the 1-byte of 6lowpan header. */
 		skb_pull(skb, 1);
-		return lowpan_give_skb_to_devices(skb, NULL);
+		return lowpan_give_skb_to_device(skb, dev);
 	} else {
 		switch (skb->data[0] & 0xe0) {
 		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
@@ -117,7 +95,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 			if (ret < 0)
 				goto drop_skb;
 
-			return lowpan_give_skb_to_devices(skb, NULL);
+			return lowpan_give_skb_to_device(skb, dev);
 		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
 			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
 			if (ret == 1) {
@@ -125,7 +103,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 				if (ret < 0)
 					goto drop_skb;
 
-				return lowpan_give_skb_to_devices(skb, NULL);
+				return lowpan_give_skb_to_device(skb, dev);
 			} else if (ret == -1) {
 				return NET_RX_DROP;
 			} else {
@@ -138,7 +116,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 				if (ret < 0)
 					goto drop_skb;
 
-				return lowpan_give_skb_to_devices(skb, NULL);
+				return lowpan_give_skb_to_device(skb, dev);
 			} else if (ret == -1) {
 				return NET_RX_DROP;
 			} else {
-- 
2.5.0

Re: [PATCHv2 bluetooth-next 1/2] ieee802154: 6lowpan: remove multiple lowpan support

[Michael Richardson]

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


Alexander Aring <alex.aring@gmail.com> wrote:
    > We currently supports multiple lowpan interfaces per wpan interface. I
    > never saw any use case into such functionality. We drop this feature

I was confused at first, because I was sure that you didn't mean that we
would not have support for multiple radios.

What you are saying is that we won't have multiple lowpan adaption layers per
wpan interface?  That does not, I think mean that we can't respond to multiple
EUI-64 layer-2 addresses, but does it have any affect on ability to respond
to multiple 16-bit (short) addresses?




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 481 bytes --]

Re: [PATCHv2 bluetooth-next 1/2] ieee802154: 6lowpan: remove multiple lowpan support

[Alexander Aring]

On Fri, Jul 31, 2015 at 08:34:57PM -0400, Michael Richardson wrote:
> 
> Alexander Aring <alex.aring@gmail.com> wrote:
>     > We currently supports multiple lowpan interfaces per wpan interface. I
>     > never saw any use case into such functionality. We drop this feature
> 
> I was confused at first, because I was sure that you didn't mean that we
> would not have support for multiple radios.
> 

What we have is:

 - wpan_phy - which represents the transceiver, which contains phy
   settings (channel/cca/page, usually known as pib, also some phy
   depended mac settings). This is in linux a device class.

 - wpan_dev - which represents the mac layer of 802.15.4 and contains
   the mib. Now it comes a little bit complicated because phy's
   (real transceivers) supports some mac functionality like address
   filtering, time critical things (aret,csma,etc.)
   These settings do we set on interface up in phy registers and currently
   not changeable while interface is up.

 - lowpan_dev - the lowpan interface which do the 6LoWPAN adaption layer
   on a wpan_dev.

Currently we have such architecture about handling multiple interfaces:

 - ONE phy
 - on each phy we can setup multiple wpan interfaces, this depends on
   the mac settings which is supported by phy.
   e.g. All node interfaces need to have the same address when the phy
   supports address-filtering, because we can setup one address filter
   only.
 - on each wpan interface you can have multiple lowpan interfaces.

I tried to draw some graphic about this situation:

                phy0                          phy1
	       / | \                          ....
              /  |  \
         wpan0 wpan1 wpan#
        /  \        \     \_____
       /    \        \        \\\
   lowpan01 lowpan01 lowpan10 lowpan##


For wpan interfaces the use case "could be", that you can change the mib
fast by having one wpan interface up and other down. MIB context
switching is down one interface, then up another interface.

This is the one usecase which I can see for wpan interfaces, but this
mechanism can also be done by userspace which handles the MIB in some
context table.

We have such support because "historical" reasons, I can do some
assumptions only why we have it. In my opinion it's because many code is
grabbed (also before I started) from wireless subsystem and they have
the use case because some phys supports multiple access points handling
and such things.

Nevertheless we don't talking here about now for the multiple wpan
interface feature. We talking about the multiple lowpan interface which
makes no sense for me. In linux you would have then some multiple lowpan
interfaces (maybe with different _IPv6_ addresses) which belongs to one
wpan interface. I never used such feature. Also I think when two lowpan
interface (which uses the same L2 addresses) will occur that the ndisc
cache at receiving nodes will get confused, because two IPv6 stacks on
one wpan interface with the same L2 addresses will operate then. (Maybe
you will also get some duplicate address detection, because the SLAAC address
(I mean the fe80::EUI64_WITH_BITFLIP/64) is the same on both interfaces).

Again I think why we have such feature currently is because the code was
copy&pasted from ethernet bridge code and there they support (and makes
sense) multiple bridges per ethernet interface.

> What you are saying is that we won't have multiple lowpan adaption layers per
> wpan interface?

yes.

> That does not, I think mean that we can't respond to multiple
> EUI-64 layer-2 addresses, but does it have any affect on ability to respond
> to multiple 16-bit (short) addresses?

On receiving we get frames from extended/short destination which are setup also
at the address filtering on phy setting (inclusive short address broadcast).

Note:

Short addresses are currently not handled by the 802.15.4 6LoWPAN stack
and is another issue where a possible solution for this is _maybe_ there
(will come up in some RFC next days/months). Maybe I should send a draft
for a possible handling of this in the next days.

This is currently another issue for handling short addresses correctly,
because we need special runtime decisions inside of ndisc cache for
setting the ICMPv6 length field and decide if short/extended.

Anyway back to the extended/short in lowpan_dev:

You said "respond", we can respond to any address which we set inside
the mac header. On receiving we can accept only the addresses which are
also set inside the MIB of the wpan interface (inklusive broadcast,
etc.) This all goes into the "address filtering" functionality inside
the phy "mac functionality for address filtering setting".

At the end I would answer your question "but does it have any affect on
ability to respond to multiple 16-bit (short) addresses?" with "no".

Because I don't see no different L2 address handling when we drop the
multiple lowpan interfaces functionality.


Does this answers some questions for you?

- Alex

[PATCHv2 bluetooth-next 2/2] mac802154: fix wpan mac setting while lowpan is there

[Alexander Aring]

If we currently change the mac address inside the wpan interface while
we have a lowpan interface on top of the wpan interface, the mac address
setting doesn't reach the lowpan interface. The effect would be that the
IPv6 lowpan interface has the old SLAAC address and isn't working
anymore because the lowpan interface use in internal mechanism sometimes
dev->addr which is the old mac address of the wpan interface.

This patch checks if a wpan interface belongs to lowpan interface, if
yes then we need to check if the lowpan interface is down and change the
mac address also at the lowpan interface. When the lowpan interface will
be set up afterwards, it will use the correct SLAAC address which based
on the updated mac address setting.

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
Tested-by: Stefan Schmidt <stefan@osg.samsung.com>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/mac802154/iface.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 416de90..ff99055 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -125,6 +125,14 @@ static int mac802154_wpan_mac_addr(struct net_device *dev, void *p)
 	if (netif_running(dev))
 		return -EBUSY;
 
+	/* lowpan need to be down for update
+	 * SLAAC address after ifup
+	 */
+	if (sdata->wpan_dev.lowpan_dev) {
+		if (netif_running(sdata->wpan_dev.lowpan_dev))
+			return -EBUSY;
+	}
+
 	ieee802154_be64_to_le64(&extended_addr, addr->sa_data);
 	if (!ieee802154_is_valid_extended_unicast_addr(extended_addr))
 		return -EINVAL;
@@ -132,6 +140,13 @@ static int mac802154_wpan_mac_addr(struct net_device *dev, void *p)
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	sdata->wpan_dev.extended_addr = extended_addr;
 
+	/* update lowpan interface mac address when
+	 * wpan mac has been changed
+	 */
+	if (sdata->wpan_dev.lowpan_dev)
+		memcpy(sdata->wpan_dev.lowpan_dev->dev_addr, dev->dev_addr,
+		       dev->addr_len);
+
 	return mac802154_wpan_update_llsec(dev);
 }
 
-- 
2.5.0