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

[PATCH 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

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

 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              | 14 ++++++++
 5 files changed, 44 insertions(+), 84 deletions(-)

-- 
2.4.6

[PATCH 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.

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.4.6

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

[Stefan Schmidt]

Hello.

On 24/07/15 15:01, Alexander Aring wrote:
> 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.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

I did not come up with a use case for it and nobody complained for a 
week so it should be ok to be removed. If this brings trouble and we get 
a good use case we can always bring this back.

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

I also tested them on my setup without seeing any trouble. Do we collect 
Tested tags here or are we happy with reviewed tags? In case we collect them

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

regards
Stefan Schmidt

> ---
>   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 {

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

[Alexander Aring]

On Thu, Jul 30, 2015 at 06:15:17PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 24/07/15 15:01, Alexander Aring wrote:
> >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.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> 
> I did not come up with a use case for it and nobody complained for a week so
> it should be ok to be removed. If this brings trouble and we get a good use
> case we can always bring this back.
> 
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> 
> I also tested them on my setup without seeing any trouble. Do we collect
> Tested tags here or are we happy with reviewed tags? In case we collect them
> 
> Tested-by: Stefan Schmidt <stefan@osg.samsung.com>
> 

I am happy with both. :-)

- Alex

[PATCH bluetooth-next 2/2] mac802154: fix wpan mac setting while lowpan

[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.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/mac802154/iface.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 416de90..346c160 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -132,6 +132,20 @@ 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) {
+		/* lowpan need to be down for update
+		 * SLAAC address after ifup
+		 */
+		if (netif_running(sdata->wpan_dev.lowpan_dev))
+			return -EBUSY;
+
+		memcpy(sdata->wpan_dev.lowpan_dev->dev_addr, dev->dev_addr,
+		       dev->addr_len);
+	}
+
 	return mac802154_wpan_update_llsec(dev);
 }
 
-- 
2.4.6

Re: [PATCH bluetooth-next 2/2] mac802154: fix wpan mac setting while lowpan

[Stefan Schmidt]

Hello.

On 24/07/15 15:01, Alexander Aring wrote:
> 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.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

I have a bit trouble testing it though. With iwpan we only allow to set 
the short address while the extended address is set during device 
registration. Removing this device also removes the connected lowpan 
device so I wonder where we would run into this problem. Patch is fine 
in any case but having this in my test script would allow me to check 
for regressions on this one.

regards
Stefan Schmidt

> ---
>   net/mac802154/iface.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 416de90..346c160 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -132,6 +132,20 @@ 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) {
> +		/* lowpan need to be down for update
> +		 * SLAAC address after ifup
> +		 */
> +		if (netif_running(sdata->wpan_dev.lowpan_dev))
> +			return -EBUSY;
> +
> +		memcpy(sdata->wpan_dev.lowpan_dev->dev_addr, dev->dev_addr,
> +		       dev->addr_len);
> +	}
> +
>   	return mac802154_wpan_update_llsec(dev);
>   }
>   

Re: [PATCH bluetooth-next 2/2] mac802154: fix wpan mac setting while lowpan

[Alexander Aring]

On Thu, Jul 30, 2015 at 06:17:44PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 24/07/15 15:01, Alexander Aring wrote:
> >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.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> 
> I have a bit trouble testing it though. With iwpan we only allow to set the
> short address while the extended address is set during device registration.
> Removing this device also removes the connected lowpan device so I wonder
> where we would run into this problem. Patch is fine in any case but having

1. have running lowpan setup
2. ifdown wpan only, let lowpan up
3. change mac (extended_addr) of wpan interface
4. do it up again

-> something is wrong because the mac of lowpan isn't the same like wpan
   anymore. We have several issues, maybe we could deal with that in
   802.15.4 6LoWPAN code, but also the SLAAC address isn't right
   anymore. IPv6 can deal with this address only when the interface is
   down, because on ifup IPv6 on able interface -> the IPv6 stack will
   generate the SLAAC address then according to the mac address.

   What I mean in this case with "SLAAC" address is the standard
   link-local address "fe:80::$EUI64_ADDRESS_WITH_UE_BIT/64"

- Alex

Re: [PATCH bluetooth-next 2/2] mac802154: fix wpan mac setting while lowpan

[Stefan Schmidt]

Hello.

On 31/07/15 00:52, Alexander Aring wrote:
> On Thu, Jul 30, 2015 at 06:17:44PM +0200, Stefan Schmidt wrote:
>> Hello.
>>
>> On 24/07/15 15:01, Alexander Aring wrote:
>>> 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.
>>>
>>> Signed-off-by: Alexander Aring<alex.aring@gmail.com>
>> Reviewed-by: Stefan Schmidt<stefan@osg.samsung.com>
>>
>> I have a bit trouble testing it though. With iwpan we only allow to set the
>> short address while the extended address is set during device registration.
>> Removing this device also removes the connected lowpan device so I wonder
>> where we would run into this problem. Patch is fine in any case but having
> 1. have running lowpan setup
> 2. ifdown wpan only, let lowpan up
> 3. change mac (extended_addr) of wpan interface
> 4. do it up again
What confused me was that I had to get wpan and lowpan down. After that 
the patches do as said. It does not work before but with the patches the 
address gets updated.

I only found one little problem when wpan was down but lowpan up I would 
set the new mac address and ip would error out but still set it for 
wpan. Alex send me a small follow up patch which fixes that. Should come 
squeezed in with that one as an update. With all this:

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


> -> something is wrong because the mac of lowpan isn't the same like wpan
>     anymore. We have several issues, maybe we could deal with that in
>     802.15.4 6LoWPAN code, but also the SLAAC address isn't right
>     anymore. IPv6 can deal with this address only when the interface is
>     down, because on ifup IPv6 on able interface -> the IPv6 stack will
>     generate the SLAAC address then according to the mac address.
>
>     What I mean in this case with "SLAAC" address is the standard
>     link-local address "fe:80::$EUI64_ADDRESS_WITH_UE_BIT/64"


As sidenote I have just seen that your patch subject is missing 
something at the end. I think you meant:

mac802154: fix wpan mac setting while lowpan is up

or similar. Not sure if you want to re-spin for this.

regards
Stefan Schmidt

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

[Stefan Schmidt]

Hello.

On 24/07/15 15:01, Alexander Aring wrote:
> 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.

I had to read the commit message to find out that you mean multiple 
lowpan interfaces on one wpan interface here. And not multiple lowpan 
interface on one machine which I thought first. :)

The later has for sure enough use cases :)

I don't have a use case for the former (the one you remove here) either. 
Still we might want to consider this as an RFC here to give folks a 
moment to speak up with they use this.

>
> 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.

Just back from vacation and catching up with to many things. I can give 
this a test on my setup next week if you want.

regards
Stefan Schmidt

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

[Alexander Aring]

On Fri, Jul 24, 2015 at 05:01:22PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 24/07/15 15:01, Alexander Aring wrote:
> >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.
> 
> I had to read the commit message to find out that you mean multiple lowpan
> interfaces on one wpan interface here. And not multiple lowpan interface on
> one machine which I thought first. :)
> 

ok, I will remember that. I should say more detailed information, but
yes it's multiple lowpan interface per wpan interface.

I think the reason why we have it is that some parts of the code is
grabbed from the eth bridge rtnl code. And multiple bridges for one
ethernet interface makes sense.

In our case, we don't have any use case, I see no sense to have multiple
lowpan interface.

> The later has for sure enough use cases :)
> 
> I don't have a use case for the former (the one you remove here) either.
> Still we might want to consider this as an RFC here to give folks a moment
> to speak up with they use this.
> 

ok. Keeping this functionaltiy makes things much complicated to deal
with it and I don't know that somebody use this functionaltiy which
makes for me no sense. I thought already 2 years long about to remove
this functionaltiy. :-)


Then to Marcel:

Please don't apply this patch until we can figure out any use case for
such functionaltiy.

> >
> >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.
> 
> Just back from vacation and catching up with to many things. I can give this
> a test on my setup next week if you want.
> 

ok.

- Alex

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

[Stefan Schmidt]

Hello.

On 24/07/15 17:10, Alexander Aring wrote:
> On Fri, Jul 24, 2015 at 05:01:22PM +0200, Stefan Schmidt wrote:
>> Hello.
>>
>> On 24/07/15 15:01, Alexander Aring wrote:
>>> 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.
>> I had to read the commit message to find out that you mean multiple lowpan
>> interfaces on one wpan interface here. And not multiple lowpan interface on
>> one machine which I thought first. :)
>>
> ok, I will remember that. I should say more detailed information, but
> yes it's multiple lowpan interface per wpan interface.
>
> I think the reason why we have it is that some parts of the code is
> grabbed from the eth bridge rtnl code. And multiple bridges for one
> ethernet interface makes sense.
>
> In our case, we don't have any use case, I see no sense to have multiple
> lowpan interface.
>
>> The later has for sure enough use cases :)
>>
>> I don't have a use case for the former (the one you remove here) either.
>> Still we might want to consider this as an RFC here to give folks a moment
>> to speak up with they use this.
>>
> ok. Keeping this functionaltiy makes things much complicated to deal
> with it and I don't know that somebody use this functionaltiy which
> makes for me no sense. I thought already 2 years long about to remove
> this functionaltiy. :-)

Sure, fair enough. I'm not saying we have to keep it just that we might 
one a bit longer than the usual patch-on-list-to-apply cycle to give 
people time to speak up.
I don't have a use case for it either right now but maybe we just miss 
something.

regards
Stefan Schmidt