All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFCv2 bluetooth-next 0/3] 6lowpan: debugfs and stateful compression support
@ 2015-11-17 22:33 Alexander Aring
  2015-11-17 22:33 ` [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support Alexander Aring
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-17 22:33 UTC (permalink / raw)
  To: linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, Alexander Aring

Hi,

This patch based on the work of Lukasz Duda. It doesn't contain any interface
lookup methods since we have a generic private dataroom per 6lowpan interface.

Also I dropped the list implementation and do a simple array one. It also
contains support for multicast stateful compression.

To access the context we supporting a complete callback structure for
operations, the include/net/6lowpan.h header contains static inline functions
to call these operations. With such behaviour we don't get any module
dependencies if we need upcomming runtime decisions inside the IPv6 stack.
The alternative would be to do everything inside static inline functions, but
there are few different to handling unicast and multicast context id's, so
it's maybe better to do this as "ops callback structure".

Few things which I am not sure about if it's correct:
 - I implement the dci multicast context table and dci unicast as different
   table. I don't know if when I have a context id "6" which is multicast
   then, I don't can have a unicast context id which is "6". I think they are
   different and we can indicate which table we need to use by the IPHC M bit
   (receiving side)  or the multicast address scope (transmit side).
 - Unicast context prefix length can be shorter than 64 bits. Yes this can be,
   but if somebody wants to do that he usually means "expand the bits until 64
   with zeros". I forbid to add prefix below 64 bits for now.
   Each stateful compression has cases for the IID bits (last 8 byte, use it as
   inline data/do compression, etc.). When I have a prefix 2000:: with prefix
   length 16, then I we hit the case of "Any remaining bits are zero." of
   stateful compression, this means it can be converted by "2000::" prefix
   length 64, then we don't need to care about that when doing a lookup of
   context.
 - I tried to get wireshark know about the available context without success.
   What I successful get working is tell prefix length exact 64 bits long. I
   don't know if wireshark do a correct context handling here, but it's also
   possible to have a prefix length above 64 (In my opinion). E.g. unicast
   stateful compression in case of SAM/DAM 01:
   "Bits covered by context information are always used.  Any IID bits not
    covered by context information are taken directly from the corresponding
    bits carried in-line.

   This patch series do following handling for this case:
   1. generate ipv6 address with inline IID data.
   2. copy the prefix bits from context to the address, which _maybe_ overwrite
      the IID (last eight bytes) data which was carried as inline data.

   This means, if my context is a full address 2001::1/128, then it will be
   the case of SAM/DAM = 11, which means the address is fully elided. (Don't
   care about the IID bits, which will be reconstructed from encapsulating
   header.

   All others SAM/DAM cases has a similar handling.

   Nevertheless I don't know how I can tell that wireshark, I can tell prefix which
   are 64 bit's long only.

Any comments are welcome.

- Alex

changes since RFCv2:
 - fix cleanup for 6LoWPAN per interface debugfs
 - add ipv6_addr_prefix_cpy
 - replace lowpan_iphc_bitcpy with ipv6_addr_prefix_cpy (unicast)
   replace lowpan_iphc_bitcpy with ipv6_addr_prefix and memcpy (multicast)
 - add netdev mailinglist into cc

Alexander Aring (3):
  6lowpan: add debugfs support
  ipv6: add ipv6_addr_prefix_cpy
  6lowpan: iphc: add support for stateful compression

 include/net/6lowpan.h         |  68 +++++-
 include/net/ipv6.h            |  15 ++
 net/6lowpan/6lowpan_i.h       |  31 +++
 net/6lowpan/Kconfig           |   8 +
 net/6lowpan/Makefile          |   1 +
 net/6lowpan/core.c            |  42 +++-
 net/6lowpan/debugfs.c         | 163 ++++++++++++++
 net/6lowpan/iphc.c            | 500 +++++++++++++++++++++++++++++++++++++-----
 net/bluetooth/6lowpan.c       |  21 +-
 net/ieee802154/6lowpan/core.c |  18 +-
 10 files changed, 796 insertions(+), 71 deletions(-)
 create mode 100644 net/6lowpan/6lowpan_i.h
 create mode 100644 net/6lowpan/debugfs.c

-- 
2.6.1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support
  2015-11-17 22:33 [RFCv2 bluetooth-next 0/3] 6lowpan: debugfs and stateful compression support Alexander Aring
@ 2015-11-17 22:33 ` Alexander Aring
  2015-11-25 16:42   ` Stefan Schmidt
       [not found] ` <1447799594-6050-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2015-11-17 22:33 ` [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression Alexander Aring
  2 siblings, 1 reply; 23+ messages in thread
From: Alexander Aring @ 2015-11-17 22:33 UTC (permalink / raw)
  To: linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, Alexander Aring

This patch will introduce a 6lowpan entry into the debugfs if enabled.
Inside this 6lowpan directory we create a subdirectories of all 6lowpan
interfaces to offer a per interface debugfs support.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/6lowpan.h         |  6 ++++-
 net/6lowpan/6lowpan_i.h       | 28 ++++++++++++++++++++++
 net/6lowpan/Kconfig           |  7 ++++++
 net/6lowpan/Makefile          |  1 +
 net/6lowpan/core.c            | 25 +++++++++++++++++++-
 net/6lowpan/debugfs.c         | 54 +++++++++++++++++++++++++++++++++++++++++++
 net/bluetooth/6lowpan.c       | 21 +++++++++++------
 net/ieee802154/6lowpan/core.c | 18 +++++++++++----
 8 files changed, 146 insertions(+), 14 deletions(-)
 create mode 100644 net/6lowpan/6lowpan_i.h
 create mode 100644 net/6lowpan/debugfs.c

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index cf3bc56..e484898 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -53,6 +53,8 @@
 #ifndef __6LOWPAN_H__
 #define __6LOWPAN_H__
 
+#include <linux/debugfs.h>
+
 #include <net/ipv6.h>
 #include <net/net_namespace.h>
 
@@ -98,6 +100,7 @@ enum lowpan_lltypes {
 
 struct lowpan_priv {
 	enum lowpan_lltypes lltype;
+	struct dentry *iface_debugfs;
 
 	/* must be last */
 	u8 priv[0] __aligned(sizeof(void *));
@@ -185,7 +188,8 @@ static inline void lowpan_push_hc_data(u8 **hc_ptr, const void *data,
 	*hc_ptr += len;
 }
 
-void lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype);
+int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype);
+void lowpan_netdev_unsetup(struct net_device *dev);
 
 /**
  * lowpan_header_decompress - replace 6LoWPAN header with IPv6 header
diff --git a/net/6lowpan/6lowpan_i.h b/net/6lowpan/6lowpan_i.h
new file mode 100644
index 0000000..33b2605
--- /dev/null
+++ b/net/6lowpan/6lowpan_i.h
@@ -0,0 +1,28 @@
+#ifndef __6LOWPAN_I_H
+#define __6LOWPAN_I_H
+
+#include <linux/netdevice.h>
+
+#ifdef CONFIG_6LOWPAN_DEBUGFS
+int lowpan_dev_debugfs_init(struct net_device *dev);
+void lowpan_dev_debugfs_uninit(struct net_device *dev);
+
+int __init lowpan_debugfs_init(void);
+void lowpan_debugfs_exit(void);
+#else
+static inline int lowpan_dev_debugfs_init(struct net_device *dev)
+{
+	return 0;
+}
+
+void lowpan_dev_debugfs_uninit(struct net_device *dev) { }
+
+static inline int __init lowpan_debugfs_init(void)
+{
+	return 0;
+}
+
+static inline void lowpan_debugfs_exit(void) { }
+#endif /* CONFIG_6LOWPAN_DEBUGFS */
+
+#endif /* __6LOWPAN_I_H */
diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
index 7fa0f38..01c901c 100644
--- a/net/6lowpan/Kconfig
+++ b/net/6lowpan/Kconfig
@@ -5,6 +5,13 @@ menuconfig 6LOWPAN
 	  This enables IPv6 over Low power Wireless Personal Area Network -
 	  "6LoWPAN" which is supported by IEEE 802.15.4 or Bluetooth stacks.
 
+config 6LOWPAN_DEBUGFS
+	bool "6LoWPAN debugfs support"
+	depends on 6LOWPAN
+	---help---
+	  This enables 6LoWPAN debugfs support. For example to manipulate
+	  IPHC context information at runtime.
+
 menuconfig 6LOWPAN_NHC
 	tristate "Next Header Compression Support"
 	depends on 6LOWPAN
diff --git a/net/6lowpan/Makefile b/net/6lowpan/Makefile
index c6ffc55..54cad8d 100644
--- a/net/6lowpan/Makefile
+++ b/net/6lowpan/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_6LOWPAN) += 6lowpan.o
 
 6lowpan-y := core.o iphc.o nhc.o
+6lowpan-$(CONFIG_6LOWPAN_DEBUGFS) += debugfs.o
 
 #rfc6282 nhcs
 obj-$(CONFIG_6LOWPAN_NHC_DEST) += nhc_dest.o
diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
index 83b19e0..fe58509 100644
--- a/net/6lowpan/core.c
+++ b/net/6lowpan/core.c
@@ -15,7 +15,9 @@
 
 #include <net/6lowpan.h>
 
-void lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
+#include "6lowpan_i.h"
+
+int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
 {
 	dev->addr_len = EUI64_ADDR_LEN;
 	dev->type = ARPHRD_6LOWPAN;
@@ -23,11 +25,25 @@ void lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
 	dev->priv_flags |= IFF_NO_QUEUE;
 
 	lowpan_priv(dev)->lltype = lltype;
+
+	return lowpan_dev_debugfs_init(dev);
 }
 EXPORT_SYMBOL(lowpan_netdev_setup);
 
+void lowpan_netdev_unsetup(struct net_device *dev)
+{
+	lowpan_dev_debugfs_uninit(dev);
+}
+EXPORT_SYMBOL(lowpan_netdev_unsetup);
+
 static int __init lowpan_module_init(void)
 {
+	int ret;
+
+	ret = lowpan_debugfs_init();
+	if (ret < 0)
+		return ret;
+
 	request_module_nowait("ipv6");
 
 	request_module_nowait("nhc_dest");
@@ -40,6 +56,13 @@ static int __init lowpan_module_init(void)
 
 	return 0;
 }
+
+static void __exit lowpan_module_exit(void)
+{
+	lowpan_debugfs_exit();
+}
+
 module_init(lowpan_module_init);
+module_exit(lowpan_module_exit);
 
 MODULE_LICENSE("GPL");
diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
new file mode 100644
index 0000000..b0a26fd
--- /dev/null
+++ b/net/6lowpan/debugfs.c
@@ -0,0 +1,54 @@
+/* This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Authors:
+ * (C) 2015 Pengutronix, Alexander Aring <aar@pengutronix.de>
+ * Copyright (c)  2015 Nordic Semiconductor. All Rights Reserved.
+ */
+
+#include <net/6lowpan.h>
+
+#include "6lowpan_i.h"
+
+static struct dentry *lowpan_debugfs;
+
+int lowpan_dev_debugfs_init(struct net_device *dev)
+{
+	struct lowpan_priv *lpriv = lowpan_priv(dev);
+
+	/* creating the root */
+	lpriv->iface_debugfs = debugfs_create_dir(dev->name, lowpan_debugfs);
+	if (!lpriv->iface_debugfs)
+		goto fail;
+
+	return 0;
+
+fail:
+	lowpan_dev_debugfs_uninit(dev);
+	return -EINVAL;
+}
+
+void lowpan_dev_debugfs_uninit(struct net_device *dev)
+{
+	debugfs_remove_recursive(lowpan_priv(dev)->iface_debugfs);
+}
+
+int __init lowpan_debugfs_init(void)
+{
+	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
+	if (!lowpan_debugfs)
+		return -EINVAL;
+
+	return 0;
+}
+
+void lowpan_debugfs_exit(void)
+{
+	debugfs_remove_recursive(lowpan_debugfs);
+}
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 9e9cca3..d10ce57 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -825,16 +825,14 @@ static int setup_netdev(struct l2cap_chan *chan, struct lowpan_dev **dev)
 	list_add_rcu(&(*dev)->list, &bt_6lowpan_devices);
 	spin_unlock(&devices_lock);
 
-	lowpan_netdev_setup(netdev, LOWPAN_LLTYPE_BTLE);
+	err = lowpan_netdev_setup(netdev, LOWPAN_LLTYPE_BTLE);
+	if (err < 0)
+		goto free_netdev;
 
 	err = register_netdev(netdev);
 	if (err < 0) {
 		BT_INFO("register_netdev failed %d", err);
-		spin_lock(&devices_lock);
-		list_del_rcu(&(*dev)->list);
-		spin_unlock(&devices_lock);
-		free_netdev(netdev);
-		goto out;
+		goto unsetup_lowpan;
 	}
 
 	BT_DBG("ifindex %d peer bdaddr %pMR type %d my addr %pMR type %d",
@@ -844,7 +842,15 @@ static int setup_netdev(struct l2cap_chan *chan, struct lowpan_dev **dev)
 
 	return 0;
 
-out:
+unsetup_lowpan:
+	lowpan_netdev_unsetup(netdev);
+
+free_netdev:
+	spin_lock(&devices_lock);
+	list_del_rcu(&(*dev)->list);
+	spin_unlock(&devices_lock);
+	free_netdev(netdev);
+
 	return err;
 }
 
@@ -1348,6 +1354,7 @@ static void disconnect_devices(void)
 		ifdown(entry->netdev);
 		BT_DBG("Unregistering netdev %s %p",
 		       entry->netdev->name, entry->netdev);
+		lowpan_netdev_unsetup(entry->netdev);
 		unregister_netdev(entry->netdev);
 		kfree(entry);
 	}
diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 20c49c7..3b42a75 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -161,16 +161,23 @@ static int lowpan_newlink(struct net *src_net, struct net_device *ldev,
 				wdev->needed_headroom;
 	ldev->needed_tailroom = wdev->needed_tailroom;
 
-	lowpan_netdev_setup(ldev, LOWPAN_LLTYPE_IEEE802154);
+	ret = lowpan_netdev_setup(ldev, LOWPAN_LLTYPE_IEEE802154);
+	if (ret < 0)
+		goto dev_put;
 
 	ret = register_netdevice(ldev);
-	if (ret < 0) {
-		dev_put(wdev);
-		return ret;
-	}
+	if (ret < 0)
+		goto unsetup_lowpan;
 
 	wdev->ieee802154_ptr->lowpan_dev = ldev;
 	return 0;
+
+unsetup_lowpan:
+	lowpan_netdev_unsetup(ldev);
+
+dev_put:
+	dev_put(wdev);
+	return ret;
 }
 
 static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
@@ -180,6 +187,7 @@ static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
 	ASSERT_RTNL();
 
 	wdev->ieee802154_ptr->lowpan_dev = NULL;
+	lowpan_netdev_unsetup(ldev);
 	unregister_netdevice(ldev);
 	dev_put(wdev);
 }
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
  2015-11-17 22:33 [RFCv2 bluetooth-next 0/3] 6lowpan: debugfs and stateful compression support Alexander Aring
@ 2015-11-17 22:33     ` Alexander Aring
       [not found] ` <1447799594-6050-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2015-11-17 22:33 ` [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression Alexander Aring
  2 siblings, 0 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-17 22:33 UTC (permalink / raw)
  To: linux-wpan-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc, Alexander Aring, David S . Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy

This patch adds a static inline function ipv6_addr_prefix_cpy which
copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
The prefix len is given by plen as bits. This function mainly based on
ipv6_addr_prefix which copies one address prefix from address into a new
ipv6 address destination and zero all other address bits.

The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
ipv6 address, it sets a prefix to an ipv6 address with keeping other
address bits. The use case is for context based address compression
inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
table to lookup address-bits without sending them.

Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 include/net/ipv6.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index e1a10b0..9d38fc2 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
 		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
 }
 
+static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
+					const struct in6_addr *pfx,
+					int plen)
+{
+	/* caller must guarantee 0 <= plen <= 128 */
+	int o = plen >> 3,
+	    b = plen & 0x7;
+
+	memcpy(addr->s6_addr, pfx, o);
+	if (b != 0) {
+		addr->s6_addr[o] &= ~(0xff00 >> b);
+		addr->s6_addr[o] |= (pfx->s6_addr[o] & (0xff00 >> b));
+	}
+}
+
 static inline void __ipv6_addr_set_half(__be32 *addr,
 					__be32 wh, __be32 wl)
 {
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
@ 2015-11-17 22:33     ` Alexander Aring
  0 siblings, 0 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-17 22:33 UTC (permalink / raw)
  To: linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, Alexander Aring, David S . Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy

This patch adds a static inline function ipv6_addr_prefix_cpy which
copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
The prefix len is given by plen as bits. This function mainly based on
ipv6_addr_prefix which copies one address prefix from address into a new
ipv6 address destination and zero all other address bits.

The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
ipv6 address, it sets a prefix to an ipv6 address with keeping other
address bits. The use case is for context based address compression
inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
table to lookup address-bits without sending them.

Cc: David S. Miller <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/ipv6.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index e1a10b0..9d38fc2 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
 		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
 }
 
+static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
+					const struct in6_addr *pfx,
+					int plen)
+{
+	/* caller must guarantee 0 <= plen <= 128 */
+	int o = plen >> 3,
+	    b = plen & 0x7;
+
+	memcpy(addr->s6_addr, pfx, o);
+	if (b != 0) {
+		addr->s6_addr[o] &= ~(0xff00 >> b);
+		addr->s6_addr[o] |= (pfx->s6_addr[o] & (0xff00 >> b));
+	}
+}
+
 static inline void __ipv6_addr_set_half(__be32 *addr,
 					__be32 wh, __be32 wl)
 {
-- 
2.6.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
  2015-11-17 22:33 [RFCv2 bluetooth-next 0/3] 6lowpan: debugfs and stateful compression support Alexander Aring
  2015-11-17 22:33 ` [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support Alexander Aring
       [not found] ` <1447799594-6050-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2015-11-17 22:33 ` Alexander Aring
       [not found]   ` <1447799594-6050-4-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2 siblings, 1 reply; 23+ messages in thread
From: Alexander Aring @ 2015-11-17 22:33 UTC (permalink / raw)
  To: linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, Alexander Aring

This patch introduce support for IPHC stateful address compression. It
will offer three debugfs per interface entries, which are:

 - dci_table: destination context indentifier table
 - sci_table: source context indentifier table
 - mcast_sci_table: multicast context identifier table

Example to setup a context id:

A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
contexts which are available. Example:

ID ipv6-address/prefix-length                  state
0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
15 0000:0000:0000:0000:0000:0000:0000:0000/000 0

For setting a context e.g. context id 0, context 2001::, prefix-length
64.

Hint: Simple copy one line and then maniuplate it.

echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
/sys/kernel/debug/6lowpan/lowpan0/dci_table

The state will display currently if the context is disabled(0) or
enabled(1).

On transmit side:

The IPHC code will automatically search for a context which would be
match for the address. Then it will be use the context which the
best compression, means the longest prefix which match will be used.

Example:

2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
last bit of the address which has the prefix 2001::/127 is the same like
the IID from the Encapsulating Header. A context ID can also be a
2001::1/128, which is then a full ipv6 address.

On Receive side:

If there is a context defined (when CID not available then it's the
default context 0) then it will be used, if the header doesn't set
SAC or DAC bit thens, it will be dropped.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/net/6lowpan.h   |  62 ++++++
 net/6lowpan/6lowpan_i.h |   3 +
 net/6lowpan/Kconfig     |   1 +
 net/6lowpan/core.c      |  17 ++
 net/6lowpan/debugfs.c   | 109 +++++++++++
 net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
 6 files changed, 635 insertions(+), 57 deletions(-)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index e484898..6e8d30e 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -75,6 +75,8 @@
 #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
 					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
 					 LOWPAN_NHC_MAX_HDR_LEN)
+/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
+#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
 
 #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
 #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
@@ -98,9 +100,45 @@ enum lowpan_lltypes {
 	LOWPAN_LLTYPE_IEEE802154,
 };
 
+enum lowpan_iphc_ctx_states {
+	LOWPAN_IPHC_CTX_STATE_DISABLED,
+	LOWPAN_IPHC_CTX_STATE_ENABLED,
+
+	__LOWPAN_IPHC_CTX_STATE_INVALID,
+	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
+};
+
+struct lowpan_iphc_ctx {
+	enum lowpan_iphc_ctx_states state;
+	u8 id;
+	struct in6_addr addr;
+	u8 prefix_len;
+};
+
+struct lowpan_iphc_ctx_ops {
+	bool (*valid_prefix)(const struct lowpan_iphc_ctx *ctx);
+	int (*update)(struct lowpan_iphc_ctx *table,
+		      const struct lowpan_iphc_ctx *ctx);
+	struct lowpan_iphc_ctx *(*get_by_id)(const struct net_device *dev,
+					     struct lowpan_iphc_ctx *table,
+					     u8 id);
+	struct lowpan_iphc_ctx *(*get_by_addr)(const struct net_device *dev,
+					       struct lowpan_iphc_ctx *table,
+					       const struct in6_addr *addr);
+};
+
+struct lowpan_iphc_ctx_table {
+	spinlock_t lock;
+	const struct lowpan_iphc_ctx_ops *ops;
+	struct lowpan_iphc_ctx table[LOWPAN_IPHC_CI_TABLE_SIZE];
+};
+
 struct lowpan_priv {
 	enum lowpan_lltypes lltype;
 	struct dentry *iface_debugfs;
+	struct lowpan_iphc_ctx_table iphc_dci;
+	struct lowpan_iphc_ctx_table iphc_sci;
+	struct lowpan_iphc_ctx_table iphc_mcast_dci;
 
 	/* must be last */
 	u8 priv[0] __aligned(sizeof(void *));
@@ -125,6 +163,30 @@ struct lowpan_802154_cb *lowpan_802154_cb(const struct sk_buff *skb)
 	return (struct lowpan_802154_cb *)skb->cb;
 }
 
+static inline int lowpan_ctx_update(struct lowpan_iphc_ctx_table *t,
+				    const struct lowpan_iphc_ctx *ctx)
+{
+	if (!t->ops->valid_prefix(ctx))
+		return -EINVAL;
+
+	return t->ops->update(t->table, ctx);
+}
+
+static inline struct lowpan_iphc_ctx *
+lowpan_ctx_by_id(const struct net_device *dev, struct lowpan_iphc_ctx_table *t,
+		 u8 id)
+{
+	return t->ops->get_by_id(dev, t->table, id);
+}
+
+static inline struct lowpan_iphc_ctx *
+lowpan_ctx_by_addr(const struct net_device *dev,
+		   struct lowpan_iphc_ctx_table *t,
+		   const struct in6_addr *addr)
+{
+	return t->ops->get_by_addr(dev, t->table, addr);
+}
+
 #ifdef DEBUG
 /* print data in line */
 static inline void raw_dump_inline(const char *caller, char *msg,
diff --git a/net/6lowpan/6lowpan_i.h b/net/6lowpan/6lowpan_i.h
index 33b2605..1312340 100644
--- a/net/6lowpan/6lowpan_i.h
+++ b/net/6lowpan/6lowpan_i.h
@@ -3,6 +3,9 @@
 
 #include <linux/netdevice.h>
 
+extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
+extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
+
 #ifdef CONFIG_6LOWPAN_DEBUGFS
 int lowpan_dev_debugfs_init(struct net_device *dev);
 void lowpan_dev_debugfs_uninit(struct net_device *dev);
diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
index 01c901c..7ecedd7 100644
--- a/net/6lowpan/Kconfig
+++ b/net/6lowpan/Kconfig
@@ -8,6 +8,7 @@ menuconfig 6LOWPAN
 config 6LOWPAN_DEBUGFS
 	bool "6LoWPAN debugfs support"
 	depends on 6LOWPAN
+	depends on DEBUG_FS
 	---help---
 	  This enables 6LoWPAN debugfs support. For example to manipulate
 	  IPHC context information at runtime.
diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
index fe58509..d354c5b 100644
--- a/net/6lowpan/core.c
+++ b/net/6lowpan/core.c
@@ -19,6 +19,8 @@
 
 int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
 {
+	int i;
+
 	dev->addr_len = EUI64_ADDR_LEN;
 	dev->type = ARPHRD_6LOWPAN;
 	dev->mtu = IPV6_MIN_MTU;
@@ -26,6 +28,21 @@ int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
 
 	lowpan_priv(dev)->lltype = lltype;
 
+	spin_lock_init(&lowpan_priv(dev)->iphc_dci.lock);
+	lowpan_priv(dev)->iphc_dci.ops = &iphc_ctx_unicast_ops;
+
+	spin_lock_init(&lowpan_priv(dev)->iphc_sci.lock);
+	lowpan_priv(dev)->iphc_sci.ops = &iphc_ctx_unicast_ops;
+
+	spin_lock_init(&lowpan_priv(dev)->iphc_mcast_dci.lock);
+	lowpan_priv(dev)->iphc_mcast_dci.ops = &iphc_ctx_mcast_ops;
+
+	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
+		lowpan_priv(dev)->iphc_dci.table[i].id = i;
+		lowpan_priv(dev)->iphc_sci.table[i].id = i;
+		lowpan_priv(dev)->iphc_mcast_dci.table[i].id = i;
+	}
+
 	return lowpan_dev_debugfs_init(dev);
 }
 EXPORT_SYMBOL(lowpan_netdev_setup);
diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
index b0a26fd..d6481eb 100644
--- a/net/6lowpan/debugfs.c
+++ b/net/6lowpan/debugfs.c
@@ -18,15 +18,124 @@
 
 static struct dentry *lowpan_debugfs;
 
+static int lowpan_context_show(struct seq_file *file, void *offset)
+{
+	struct lowpan_iphc_ctx_table *t = file->private;
+	int i;
+
+	seq_printf(file, "%-2s %-43s %s\n", "ID", "ipv6-address/prefix-length",
+		   "state");
+
+	spin_lock_bh(&t->lock);
+	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
+		seq_printf(file,
+			   "%-2d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%03d %d\n",
+			   t->table[i].id,
+			   be16_to_cpu(t->table[i].addr.s6_addr16[0]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[1]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[2]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[3]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[4]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[5]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[6]),
+			   be16_to_cpu(t->table[i].addr.s6_addr16[7]),
+			   t->table[i].prefix_len, t->table[i].state);
+	}
+	spin_unlock_bh(&t->lock);
+
+	return 0;
+}
+
+static int lowpan_context_dbgfs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, lowpan_context_show, inode->i_private);
+}
+
+static ssize_t lowpan_context_dbgfs_write(struct file *fp,
+					  const char __user *user_buf,
+					  size_t count, loff_t *ppos)
+{
+	char buf[128] = { };
+	struct seq_file *file = fp->private_data;
+	struct lowpan_iphc_ctx_table *t = file->private;
+	struct lowpan_iphc_ctx ctx;
+	int status = count, n, id, state, i, prefix_len, ret;
+	unsigned int addr[8];
+
+	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
+						 count))) {
+		status = -EFAULT;
+		goto out;
+	}
+
+	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
+		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
+		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
+	if (n != 11) {
+		status = -EIO;
+		goto out;
+	}
+
+	if (id > LOWPAN_IPHC_CI_TABLE_SIZE - 1 ||
+	    state > LOWPAN_IPHC_CTX_STATE_MAX || prefix_len > 128) {
+		status = -EINVAL;
+		goto out;
+	}
+
+	ctx.id = id;
+	ctx.state = state;
+	ctx.prefix_len = prefix_len;
+
+	for (i = 0; i < 8; i++)
+		ctx.addr.s6_addr16[i] = cpu_to_be16(addr[i] & 0xffff);
+
+	spin_lock_bh(&t->lock);
+	ret = lowpan_ctx_update(t, &ctx);
+	if (ret < 0)
+		status = ret;
+	spin_unlock_bh(&t->lock);
+
+out:
+	return status;
+}
+
+const struct file_operations lowpan_context_fops = {
+	.open		= lowpan_context_dbgfs_open,
+	.read		= seq_read,
+	.write		= lowpan_context_dbgfs_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 int lowpan_dev_debugfs_init(struct net_device *dev)
 {
 	struct lowpan_priv *lpriv = lowpan_priv(dev);
+	static struct dentry *dentry;
 
 	/* creating the root */
 	lpriv->iface_debugfs = debugfs_create_dir(dev->name, lowpan_debugfs);
 	if (!lpriv->iface_debugfs)
 		goto fail;
 
+	dentry = debugfs_create_file("dci_table", 0664, lpriv->iface_debugfs,
+				     &lowpan_priv(dev)->iphc_dci,
+				     &lowpan_context_fops);
+	if (!dentry)
+		goto fail;
+
+	dentry = debugfs_create_file("sci_table", 0664, lpriv->iface_debugfs,
+				     &lowpan_priv(dev)->iphc_sci,
+				     &lowpan_context_fops);
+	if (!dentry)
+		goto fail;
+
+	dentry = debugfs_create_file("mcast_dci_table", 0664,
+				     lpriv->iface_debugfs,
+				     &lowpan_priv(dev)->iphc_mcast_dci,
+				     &lowpan_context_fops);
+	if (!dentry)
+		goto fail;
+
 	return 0;
 
 fail:
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 346b5c1..5974ea5 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -56,6 +56,7 @@
 /* special link-layer handling */
 #include <net/mac802154.h>
 
+#include "6lowpan_i.h"
 #include "nhc.h"
 
 /* Values of fields within the IPHC encoding first byte */
@@ -147,6 +148,9 @@
 	 (((a)->s6_addr16[6]) == 0) &&		\
 	 (((a)->s6_addr[14]) == 0))
 
+#define LOWPAN_IPHC_CID_DCI(cid)	(cid & 0x0f)
+#define LOWPAN_IPHC_CID_SCI(cid)	((cid & 0xf0) >> 4)
+
 static inline void iphc_uncompress_eui64_lladdr(struct in6_addr *ipaddr,
 						const void *lladdr)
 {
@@ -259,30 +263,59 @@ static int uncompress_addr(struct sk_buff *skb, const struct net_device *dev,
 /* Uncompress address function for source context
  * based address(non-multicast).
  */
-static int uncompress_context_based_src_addr(struct sk_buff *skb,
-					     struct in6_addr *ipaddr,
-					     u8 address_mode)
+static int uncompress_ctx_addr(struct sk_buff *skb,
+			       const struct net_device *dev,
+			       const struct lowpan_iphc_ctx *ctx,
+			       struct in6_addr *ipaddr, u8 address_mode,
+			       const void *lladdr)
 {
+	bool fail;
+
 	switch (address_mode) {
-	case LOWPAN_IPHC_SAM_00:
-		/* unspec address ::
+	/* SAM and DAM are the same here */
+	case LOWPAN_IPHC_DAM_00:
+		fail = false;
+		/* SAM_00 -> unspec address ::
 		 * Do nothing, address is already ::
+		 *
+		 * DAM 00 -> reserved should never occur.
 		 */
 		break;
 	case LOWPAN_IPHC_SAM_01:
-		/* TODO */
+	case LOWPAN_IPHC_DAM_01:
+		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[8], 8);
+		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
+		break;
 	case LOWPAN_IPHC_SAM_10:
-		/* TODO */
+	case LOWPAN_IPHC_DAM_10:
+		ipaddr->s6_addr[11] = 0xFF;
+		ipaddr->s6_addr[12] = 0xFE;
+		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[14], 2);
+		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
+		break;
 	case LOWPAN_IPHC_SAM_11:
-		/* TODO */
-		netdev_warn(skb->dev, "SAM value 0x%x not supported\n",
-			    address_mode);
-		return -EINVAL;
+	case LOWPAN_IPHC_DAM_11:
+		fail = false;
+		switch (lowpan_priv(dev)->lltype) {
+		case LOWPAN_LLTYPE_IEEE802154:
+			iphc_uncompress_802154_lladdr(ipaddr, lladdr);
+			break;
+		default:
+			iphc_uncompress_eui64_lladdr(ipaddr, lladdr);
+			break;
+		}
+		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
+		break;
 	default:
 		pr_debug("Invalid sam value: 0x%x\n", address_mode);
 		return -EINVAL;
 	}
 
+	if (fail) {
+		pr_debug("Failed to fetch skb data\n");
+		return -EIO;
+	}
+
 	raw_dump_inline(NULL,
 			"Reconstructed context based ipv6 src addr is",
 			ipaddr->s6_addr, 16);
@@ -346,6 +379,33 @@ static int lowpan_uncompress_multicast_daddr(struct sk_buff *skb,
 	return 0;
 }
 
+static int lowpan_uncompress_multicast_ctx_daddr(struct sk_buff *skb,
+						 struct lowpan_iphc_ctx *ctx,
+						 struct in6_addr *ipaddr,
+						 u8 address_mode)
+{
+	struct in6_addr network_pfx = {};
+	bool fail;
+
+	if (address_mode != LOWPAN_IPHC_DAM_00)
+		return -EINVAL;
+
+	ipaddr->s6_addr[0] = 0xFF;
+	fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[1], 2);
+	fail |= lowpan_fetch_skb(skb, &ipaddr->s6_addr[12], 4);
+	/* take prefix_len and network prefix from the context */
+	ipaddr->s6_addr[3] = ctx->prefix_len;
+	/* get network prefix to copy into multicast address */
+	ipv6_addr_prefix(&network_pfx, &ctx->addr, ctx->prefix_len);
+	/* setting network prefix */
+	memcpy(&ipaddr->s6_addr[4], &network_pfx, 8);
+
+	if (fail < 0)
+		return -EIO;
+
+	return 0;
+}
+
 /* get the ecn values from iphc tf format and set it to ipv6hdr */
 static inline void lowpan_iphc_tf_set_ecn(struct ipv6hdr *hdr, const u8 *tf)
 {
@@ -459,7 +519,8 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 			     const void *daddr, const void *saddr)
 {
 	struct ipv6hdr hdr = {};
-	u8 iphc0, iphc1;
+	struct lowpan_iphc_ctx *ci;
+	u8 iphc0, iphc1, cid = 0;
 	int err;
 
 	raw_dump_table(__func__, "raw skb data dump uncompressed",
@@ -469,12 +530,14 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)))
 		return -EINVAL;
 
-	/* another if the CID flag is set */
-	if (iphc1 & LOWPAN_IPHC_CID)
-		return -ENOTSUPP;
-
 	hdr.version = 6;
 
+	/* default CID = 0, another if the CID flag is set */
+	if (iphc1 & LOWPAN_IPHC_CID) {
+		if (lowpan_fetch_skb(skb, &cid, sizeof(cid)))
+			return -EINVAL;
+	}
+
 	err = lowpan_iphc_tf_decompress(skb, &hdr,
 					iphc0 & LOWPAN_IPHC_TF_MASK);
 	if (err < 0)
@@ -500,10 +563,18 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 	}
 
 	if (iphc1 & LOWPAN_IPHC_SAC) {
-		/* Source address context based uncompression */
+		spin_lock_bh(&lowpan_priv(dev)->iphc_sci.lock);
+		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_sci,
+				      LOWPAN_IPHC_CID_SCI(cid));
+		if (!ci) {
+			spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
+			return -EINVAL;
+		}
+
 		pr_debug("SAC bit is set. Handle context based source address.\n");
-		err = uncompress_context_based_src_addr(skb, &hdr.saddr,
-							iphc1 & LOWPAN_IPHC_SAM_MASK);
+		err = uncompress_ctx_addr(skb, dev, ci, &hdr.saddr,
+					  iphc1 & LOWPAN_IPHC_SAM_MASK, saddr);
+		spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
 	} else {
 		/* Source address uncompression */
 		pr_debug("source address stateless compression\n");
@@ -515,27 +586,54 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 	if (err)
 		return -EINVAL;
 
-	/* check for Multicast Compression */
-	if (iphc1 & LOWPAN_IPHC_M) {
-		if (iphc1 & LOWPAN_IPHC_DAC) {
-			pr_debug("dest: context-based mcast compression\n");
-			/* TODO: implement this */
-		} else {
-			err = lowpan_uncompress_multicast_daddr(skb, &hdr.daddr,
-								iphc1 & LOWPAN_IPHC_DAM_MASK);
+	switch (iphc1 & (LOWPAN_IPHC_M | LOWPAN_IPHC_DAC)) {
+	case LOWPAN_IPHC_M | LOWPAN_IPHC_DAC:
+		spin_lock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
+		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_mcast_dci,
+				      LOWPAN_IPHC_CID_DCI(cid));
+		if (!ci) {
+			spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
+			return -EINVAL;
+		}
 
-			if (err)
-				return -EINVAL;
+		/* multicast with context */
+		pr_debug("dest: context-based mcast compression\n");
+		err = lowpan_uncompress_multicast_ctx_daddr(skb, ci,
+							    &hdr.daddr,
+							    iphc1 & LOWPAN_IPHC_DAM_MASK);
+		spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
+		break;
+	case LOWPAN_IPHC_M:
+		/* multicast */
+		err = lowpan_uncompress_multicast_daddr(skb, &hdr.daddr,
+							iphc1 & LOWPAN_IPHC_DAM_MASK);
+		break;
+	case LOWPAN_IPHC_DAC:
+		spin_lock_bh(&lowpan_priv(dev)->iphc_dci.lock);
+		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_dci,
+				      LOWPAN_IPHC_CID_DCI(cid));
+		if (!ci) {
+			spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
+			return -EINVAL;
 		}
-	} else {
+
+		/* Destination address context based uncompression */
+		pr_debug("DAC bit is set. Handle context based destination address.\n");
+		err = uncompress_ctx_addr(skb, dev, ci, &hdr.daddr,
+					  iphc1 & LOWPAN_IPHC_DAM_MASK, daddr);
+		spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
+		break;
+	default:
 		err = uncompress_addr(skb, dev, &hdr.daddr,
 				      iphc1 & LOWPAN_IPHC_DAM_MASK, daddr);
 		pr_debug("dest: stateless compression mode %d dest %pI6c\n",
 			 iphc1 & LOWPAN_IPHC_DAM_MASK, &hdr.daddr);
-		if (err)
-			return -EINVAL;
+		break;
 	}
 
+	if (err)
+		return -EINVAL;
+
 	/* Next header data uncompression */
 	if (iphc0 & LOWPAN_IPHC_NH) {
 		err = lowpan_nhc_do_uncompression(skb, dev, &hdr);
@@ -585,6 +683,60 @@ static const u8 lowpan_iphc_dam_to_sam_value[] = {
 	[LOWPAN_IPHC_DAM_11] = LOWPAN_IPHC_SAM_11,
 };
 
+static u8 lowpan_compress_ctx_addr(u8 **hc_ptr, const struct in6_addr *ipaddr,
+				   const struct lowpan_iphc_ctx *ctx,
+				   const unsigned char *lladdr, bool sam)
+{
+	struct in6_addr tmp = {};
+	u8 dam;
+
+	/* check for SAM/DAM = 11 */
+	memcpy(&tmp.s6_addr[8], lladdr, 8);
+	/* second bit-flip (Universe/Local)
+	 * is done according RFC2464
+	 */
+	tmp.s6_addr[8] ^= 0x02;
+	/* context information are always used */
+	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
+	if (ipv6_addr_equal(&tmp, ipaddr)) {
+		dam = LOWPAN_IPHC_DAM_11;
+		goto out;
+	}
+
+	memset(&tmp, 0, sizeof(tmp));
+	/* check for SAM/DAM = 01 */
+	tmp.s6_addr[11] = 0xFF;
+	tmp.s6_addr[12] = 0xFE;
+	memcpy(&tmp.s6_addr[14], &ipaddr->s6_addr[14], 2);
+	/* context information are always used */
+	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
+	if (ipv6_addr_equal(&tmp, ipaddr)) {
+		lowpan_push_hc_data(hc_ptr, &ipaddr->s6_addr[14], 2);
+		dam = LOWPAN_IPHC_DAM_10;
+		goto out;
+	}
+
+	memset(&tmp, 0, sizeof(tmp));
+	/* check for SAM/DAM = 10, should always match */
+	memcpy(&tmp.s6_addr[8], &ipaddr->s6_addr[8], 8);
+	/* context information are always used */
+	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
+	if (ipv6_addr_equal(&tmp, ipaddr)) {
+		lowpan_push_hc_data(hc_ptr, &ipaddr->s6_addr[8], 8);
+		dam = LOWPAN_IPHC_DAM_01;
+		goto out;
+	}
+
+	WARN_ON_ONCE("context found but no address mode matched\n");
+	return -EINVAL;
+out:
+
+	if (sam)
+		return lowpan_iphc_dam_to_sam_value[dam];
+	else
+		return dam;
+}
+
 static u8 lowpan_compress_addr_64(u8 **hc_ptr, const struct in6_addr *ipaddr,
 				  const unsigned char *lladdr, bool sam)
 {
@@ -708,6 +860,21 @@ static u8 lowpan_iphc_tf_compress(u8 **hc_ptr, const struct ipv6hdr *hdr)
 	return val;
 }
 
+static u8 lowpan_iphc_mcast_ctx_addr_compress(u8 **hc_ptr,
+					      const struct lowpan_iphc_ctx *ctx,
+					      const struct in6_addr *ipaddr)
+{
+	u8 data[6];
+
+	/* flags/scope, reserved (RIID) */
+	memcpy(data, &ipaddr->s6_addr[1], 2);
+	/* group ID */
+	memcpy(&data[1], &ipaddr->s6_addr[11], 4);
+	lowpan_push_hc_data(hc_ptr, data, 6);
+
+	return LOWPAN_IPHC_DAM_00;
+}
+
 static u8 lowpan_iphc_mcast_addr_compress(u8 **hc_ptr,
 					  const struct in6_addr *ipaddr)
 {
@@ -742,10 +909,11 @@ static u8 lowpan_iphc_mcast_addr_compress(u8 **hc_ptr,
 int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
 			   const void *daddr, const void *saddr)
 {
-	u8 iphc0, iphc1, *hc_ptr;
+	u8 iphc0, iphc1, *hc_ptr, cid = 0;
 	struct ipv6hdr *hdr;
 	u8 head[LOWPAN_IPHC_MAX_HC_BUF_LEN] = {};
-	int ret, addr_type;
+	struct lowpan_iphc_ctx *dci, *sci, dci_entry, sci_entry;
+	int ret, ipv6_daddr_type, ipv6_saddr_type;
 
 	if (skb->protocol != htons(ETH_P_IPV6))
 		return -EINVAL;
@@ -769,14 +937,47 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
 	iphc0 = LOWPAN_DISPATCH_IPHC;
 	iphc1 = 0;
 
-	/* TODO: context lookup */
-
 	raw_dump_inline(__func__, "saddr", saddr, EUI64_ADDR_LEN);
 	raw_dump_inline(__func__, "daddr", daddr, EUI64_ADDR_LEN);
 
 	raw_dump_table(__func__, "sending raw skb network uncompressed packet",
 		       skb->data, skb->len);
 
+	ipv6_daddr_type = ipv6_addr_type(&hdr->daddr);
+	if (ipv6_daddr_type & IPV6_ADDR_MULTICAST) {
+		spin_lock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
+		dci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_mcast_dci,
+					 &hdr->daddr);
+		if (dci) {
+			memcpy(&dci_entry, dci, sizeof(*dci));
+			cid |= dci->id;
+		}
+		spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
+	} else {
+		spin_lock_bh(&lowpan_priv(dev)->iphc_dci.lock);
+		dci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_dci,
+					 &hdr->daddr);
+		if (dci) {
+			memcpy(&dci_entry, dci, sizeof(*dci));
+			cid |= dci->id;
+		}
+		spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
+	}
+
+	spin_lock_bh(&lowpan_priv(dev)->iphc_sci.lock);
+	sci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_sci,
+				 &hdr->saddr);
+	if (sci) {
+		memcpy(&sci_entry, sci, sizeof(*sci));
+		cid |= (sci->id << 4);
+	}
+	spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
+
+	if (sci || dci) {
+		iphc1 |= LOWPAN_IPHC_CID;
+		lowpan_push_hc_data(&hc_ptr, &cid, sizeof(cid));
+	}
+
 	/* Traffic Class, Flow Label compression */
 	iphc0 |= lowpan_iphc_tf_compress(&hc_ptr, hdr);
 
@@ -813,39 +1014,63 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
 				    sizeof(hdr->hop_limit));
 	}
 
-	addr_type = ipv6_addr_type(&hdr->saddr);
+	ipv6_saddr_type = ipv6_addr_type(&hdr->saddr);
 	/* source address compression */
-	if (addr_type == IPV6_ADDR_ANY) {
+	if (ipv6_saddr_type == IPV6_ADDR_ANY) {
 		pr_debug("source address is unspecified, setting SAC\n");
 		iphc1 |= LOWPAN_IPHC_SAC;
 	} else {
-		if (addr_type & IPV6_ADDR_LINKLOCAL) {
-			iphc1 |= lowpan_compress_addr_64(&hc_ptr, &hdr->saddr,
-							 saddr, true);
-			pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
-				 &hdr->saddr, iphc1);
+		if (sci) {
+			iphc1 |= lowpan_compress_ctx_addr(&hc_ptr, &hdr->saddr,
+							  &sci_entry, saddr,
+							  true);
+			iphc1 |= LOWPAN_IPHC_SAC;
 		} else {
-			pr_debug("send the full source address\n");
-			lowpan_push_hc_data(&hc_ptr, hdr->saddr.s6_addr, 16);
+			if (ipv6_saddr_type & IPV6_ADDR_LINKLOCAL) {
+				iphc1 |= lowpan_compress_addr_64(&hc_ptr,
+								 &hdr->saddr,
+								 saddr, true);
+				pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
+					 &hdr->saddr, iphc1);
+			} else {
+				pr_debug("send the full source address\n");
+				lowpan_push_hc_data(&hc_ptr,
+						    hdr->saddr.s6_addr, 16);
+			}
 		}
 	}
 
-	addr_type = ipv6_addr_type(&hdr->daddr);
 	/* destination address compression */
-	if (addr_type & IPV6_ADDR_MULTICAST) {
+	if (ipv6_daddr_type & IPV6_ADDR_MULTICAST) {
 		pr_debug("destination address is multicast: ");
-		iphc1 |= LOWPAN_IPHC_M;
-		iphc1 |= lowpan_iphc_mcast_addr_compress(&hc_ptr, &hdr->daddr);
+		if (dci) {
+			iphc1 |= lowpan_iphc_mcast_ctx_addr_compress(&hc_ptr,
+								     &dci_entry,
+								     &hdr->daddr);
+		} else {
+			iphc1 |= LOWPAN_IPHC_M;
+			iphc1 |= lowpan_iphc_mcast_addr_compress(&hc_ptr,
+								 &hdr->daddr);
+		}
 	} else {
-		if (addr_type & IPV6_ADDR_LINKLOCAL) {
-			/* TODO: context lookup */
-			iphc1 |= lowpan_compress_addr_64(&hc_ptr, &hdr->daddr,
-							 daddr, false);
-			pr_debug("dest address unicast link-local %pI6c "
-				 "iphc1 0x%02x\n", &hdr->daddr, iphc1);
+		if (dci) {
+			iphc1 |= lowpan_compress_ctx_addr(&hc_ptr, &hdr->daddr,
+							  &dci_entry, daddr,
+							  false);
+			iphc1 |= LOWPAN_IPHC_DAC;
 		} else {
-			pr_debug("dest address unicast %pI6c\n", &hdr->daddr);
-			lowpan_push_hc_data(&hc_ptr, hdr->daddr.s6_addr, 16);
+			if (ipv6_daddr_type & IPV6_ADDR_LINKLOCAL) {
+				iphc1 |= lowpan_compress_addr_64(&hc_ptr,
+								 &hdr->daddr,
+								 daddr, false);
+				pr_debug("dest address unicast link-local %pI6c iphc1 0x%02x\n",
+					 &hdr->daddr, iphc1);
+			} else {
+				pr_debug("dest address unicast %pI6c\n",
+					 &hdr->daddr);
+				lowpan_push_hc_data(&hc_ptr,
+						    hdr->daddr.s6_addr, 16);
+			}
 		}
 	}
 
@@ -871,3 +1096,164 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(lowpan_header_compress);
+
+static bool lowpan_iphc_ctx_valid_prefix(const struct lowpan_iphc_ctx *ctx)
+{
+	/* prefix which are smaller than 64 bits are not valid, users
+	 * may mean than a prefix which is filled with zero until 64th bit.
+	 * Refere rfc6282 "Any remaining bits are zero." The remaining bits
+	 * in this case where are the prefix(< 64) ends until IID starts.
+	 */
+	if (ctx->prefix_len < 64)
+		return false;
+
+	return true;
+}
+
+static bool
+lowpan_iphc_mcast_ctx_valid_prefix(const struct lowpan_iphc_ctx *ctx)
+{
+	/* network prefix for multicast is at maximum 64 bits long */
+	if (ctx->prefix_len > 64)
+		return false;
+
+	return true;
+}
+
+static int lowpan_iphc_ctx_update(struct lowpan_iphc_ctx *table,
+				  const struct lowpan_iphc_ctx *ctx)
+{
+	int ret = 0, i;
+
+	switch (ctx->state) {
+	case LOWPAN_IPHC_CTX_STATE_DISABLED:
+		/* we don't care about if disabled */
+		break;
+	case LOWPAN_IPHC_CTX_STATE_ENABLED:
+		for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
+			if (ctx->prefix_len != table[i].prefix_len)
+				continue;
+
+			if (ipv6_prefix_equal(&ctx->addr, &table[i].addr,
+					      ctx->prefix_len)) {
+				switch (table[i].state) {
+				case LOWPAN_IPHC_CTX_STATE_ENABLED:
+					ret = -EEXIST;
+					goto out;
+				default:
+					break;
+				}
+			}
+		}
+		break;
+	default:
+		break;
+	}
+
+	memcpy(&table[ctx->id], ctx, sizeof(*ctx));
+
+out:
+	return ret;
+}
+
+static struct lowpan_iphc_ctx *
+lowpan_iphc_ctx_get_by_id(const struct net_device *dev,
+			  struct lowpan_iphc_ctx *table, u8 id)
+{
+	struct lowpan_iphc_ctx *ret = NULL;
+
+	WARN_ON_ONCE(id > LOWPAN_IPHC_CI_TABLE_SIZE);
+
+	switch (table[id].state) {
+	case LOWPAN_IPHC_CTX_STATE_ENABLED:
+		ret = &table[id];
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static struct lowpan_iphc_ctx *
+lowpan_iphc_ctx_get_by_addr(const struct net_device *dev,
+			    struct lowpan_iphc_ctx *table,
+			    const struct in6_addr *addr)
+{
+	struct lowpan_iphc_ctx *ret = NULL;
+	struct in6_addr addr_prefix;
+	int i;
+
+	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
+		ipv6_addr_prefix(&addr_prefix, addr, table[i].prefix_len);
+
+		if (ipv6_prefix_equal(&addr_prefix, &table[i].addr,
+				      table[i].prefix_len)) {
+			switch (table[i].state) {
+			case LOWPAN_IPHC_CTX_STATE_ENABLED:
+				/* remember first match */
+				if (!ret) {
+					ret = &table[i];
+					continue;
+				}
+
+				/* get the context with longest prefix_len */
+				if (table[i].prefix_len > ret->prefix_len)
+					ret = &table[i];
+				break;
+			default:
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
+static struct lowpan_iphc_ctx *
+lowpan_iphc_ctx_get_by_mcast_addr(const struct net_device *dev,
+				  struct lowpan_iphc_ctx *table,
+				  const struct in6_addr *addr)
+{
+	struct lowpan_iphc_ctx *ret = NULL;
+	struct in6_addr addr_mcast, network_pfx = {};
+	int i;
+
+	/* init mcast address with  */
+	memcpy(&addr_mcast, addr, sizeof(*addr));
+
+	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
+		/* setting plen */
+		addr_mcast.s6_addr[3] = table[i].prefix_len;
+		/*  get network prefix to copy into multicast address */
+		ipv6_addr_prefix(&network_pfx, &table[i].addr,
+				 table[i].prefix_len);
+		/* setting network prefix */
+		memcpy(&addr_mcast.s6_addr[4], &network_pfx, 8);
+
+		if (ipv6_addr_equal(addr, &addr_mcast)) {
+			switch (table[i].state) {
+			case LOWPAN_IPHC_CTX_STATE_ENABLED:
+				return &table[i];
+			default:
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
+const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops = {
+	.valid_prefix = lowpan_iphc_ctx_valid_prefix,
+	.update = lowpan_iphc_ctx_update,
+	.get_by_id = lowpan_iphc_ctx_get_by_id,
+	.get_by_addr = lowpan_iphc_ctx_get_by_addr,
+};
+
+const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
+	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
+	.update = lowpan_iphc_ctx_update,
+	.get_by_id = lowpan_iphc_ctx_get_by_id,
+	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
+};
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
  2015-11-17 22:33     ` Alexander Aring
@ 2015-11-18 13:55         ` Sergei Shtylyov
  -1 siblings, 0 replies; 23+ messages in thread
From: Sergei Shtylyov @ 2015-11-18 13:55 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 11/18/2015 1:33 AM, Alexander Aring wrote:

> This patch adds a static inline function ipv6_addr_prefix_cpy which

    I suggest not to reduce "copy".

> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
> The prefix len is given by plen as bits. This function mainly based on
> ipv6_addr_prefix which copies one address prefix from address into a new
> ipv6 address destination and zero all other address bits.
>
> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
> ipv6 address, it sets a prefix to an ipv6 address with keeping other
> address bits. The use case is for context based address compression
> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
> table to lookup address-bits without sending them.
>
> Cc: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   include/net/ipv6.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index e1a10b0..9d38fc2 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
>   		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>   }
>
> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
> +					const struct in6_addr *pfx,
> +					int plen)
> +{
> +	/* caller must guarantee 0 <= plen <= 128 */
> +	int o = plen >> 3,
> +	    b = plen & 0x7;

    Unusual declaration style, why not just have *int* on both lines?

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
@ 2015-11-18 13:55         ` Sergei Shtylyov
  0 siblings, 0 replies; 23+ messages in thread
From: Sergei Shtylyov @ 2015-11-18 13:55 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 11/18/2015 1:33 AM, Alexander Aring wrote:

> This patch adds a static inline function ipv6_addr_prefix_cpy which

    I suggest not to reduce "copy".

> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
> The prefix len is given by plen as bits. This function mainly based on
> ipv6_addr_prefix which copies one address prefix from address into a new
> ipv6 address destination and zero all other address bits.
>
> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
> ipv6 address, it sets a prefix to an ipv6 address with keeping other
> address bits. The use case is for context based address compression
> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
> table to lookup address-bits without sending them.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   include/net/ipv6.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index e1a10b0..9d38fc2 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
>   		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>   }
>
> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
> +					const struct in6_addr *pfx,
> +					int plen)
> +{
> +	/* caller must guarantee 0 <= plen <= 128 */
> +	int o = plen >> 3,
> +	    b = plen & 0x7;

    Unusual declaration style, why not just have *int* on both lines?

[...]

MBR, Sergei


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support
  2015-11-17 22:33 ` [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support Alexander Aring
@ 2015-11-25 16:42   ` Stefan Schmidt
  2015-11-25 18:00     ` Alexander Aring
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-25 16:42 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda, martin.gergeleit

Hello.

On 17/11/15 23:33, Alexander Aring wrote:
> This patch will introduce a 6lowpan entry into the debugfs if enabled.
> Inside this 6lowpan directory we create a subdirectories of all 6lowpan
> interfaces to offer a per interface debugfs support.

This will be useful for other things as well later on. Some small 
remarks below.

> Signed-off-by: Alexander Aring<alex.aring@gmail.com>
> ---
>   include/net/6lowpan.h         |  6 ++++-
>   net/6lowpan/6lowpan_i.h       | 28 ++++++++++++++++++++++
>   net/6lowpan/Kconfig           |  7 ++++++
>   net/6lowpan/Makefile          |  1 +
>   net/6lowpan/core.c            | 25 +++++++++++++++++++-
>   net/6lowpan/debugfs.c         | 54 +++++++++++++++++++++++++++++++++++++++++++
>   net/bluetooth/6lowpan.c       | 21 +++++++++++------
>   net/ieee802154/6lowpan/core.c | 18 +++++++++++----
>   8 files changed, 146 insertions(+), 14 deletions(-)
>   create mode 100644 net/6lowpan/6lowpan_i.h
>   create mode 100644 net/6lowpan/debugfs.c
>
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index cf3bc56..e484898 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -53,6 +53,8 @@
>   #ifndef __6LOWPAN_H__
>   #define __6LOWPAN_H__
>   
> +#include <linux/debugfs.h>
> +
>   #include <net/ipv6.h>
>   #include <net/net_namespace.h>
>   
> @@ -98,6 +100,7 @@ enum lowpan_lltypes {
>   
>   struct lowpan_priv {
>   	enum lowpan_lltypes lltype;
> +	struct dentry *iface_debugfs;
>   
>   	/* must be last */
>   	u8 priv[0] __aligned(sizeof(void *));
> @@ -185,7 +188,8 @@ static inline void lowpan_push_hc_data(u8 **hc_ptr, const void *data,
>   	*hc_ptr += len;
>   }
>   
> -void lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype);
> +int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype);
> +void lowpan_netdev_unsetup(struct net_device *dev);

Unsetup sounds really wrong. Can we call it teardown? Especially as we 
export this symbol.

>   
>   /**
>    * lowpan_header_decompress - replace 6LoWPAN header with IPv6 header
> diff --git a/net/6lowpan/6lowpan_i.h b/net/6lowpan/6lowpan_i.h
> new file mode 100644
> index 0000000..33b2605
> --- /dev/null
> +++ b/net/6lowpan/6lowpan_i.h
> @@ -0,0 +1,28 @@
> +#ifndef __6LOWPAN_I_H
> +#define __6LOWPAN_I_H
> +
> +#include <linux/netdevice.h>
> +
> +#ifdef CONFIG_6LOWPAN_DEBUGFS
> +int lowpan_dev_debugfs_init(struct net_device *dev);
> +void lowpan_dev_debugfs_uninit(struct net_device *dev);

Exit instead of uninit?
> +
> +int __init lowpan_debugfs_init(void);
> +void lowpan_debugfs_exit(void);
> +#else
> +static inline int lowpan_dev_debugfs_init(struct net_device *dev)
> +{
> +	return 0;
> +}
> +
> +void lowpan_dev_debugfs_uninit(struct net_device *dev) { }
> +
> +static inline int __init lowpan_debugfs_init(void)
> +{
> +	return 0;
> +}
> +
> +static inline void lowpan_debugfs_exit(void) { }
> +#endif /* CONFIG_6LOWPAN_DEBUGFS */
> +
> +#endif /* __6LOWPAN_I_H */
> diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
> index 7fa0f38..01c901c 100644
> --- a/net/6lowpan/Kconfig
> +++ b/net/6lowpan/Kconfig
> @@ -5,6 +5,13 @@ menuconfig 6LOWPAN
>   	  This enables IPv6 over Low power Wireless Personal Area Network -
>   	  "6LoWPAN" which is supported by IEEE 802.15.4 or Bluetooth stacks.
>   
> +config 6LOWPAN_DEBUGFS
> +	bool "6LoWPAN debugfs support"
> +	depends on 6LOWPAN
> +	---help---
> +	  This enables 6LoWPAN debugfs support. For example to manipulate
> +	  IPHC context information at runtime.
> +
>   menuconfig 6LOWPAN_NHC
>   	tristate "Next Header Compression Support"
>   	depends on 6LOWPAN
> diff --git a/net/6lowpan/Makefile b/net/6lowpan/Makefile
> index c6ffc55..54cad8d 100644
> --- a/net/6lowpan/Makefile
> +++ b/net/6lowpan/Makefile
> @@ -1,6 +1,7 @@
>   obj-$(CONFIG_6LOWPAN) += 6lowpan.o
>   
>   6lowpan-y := core.o iphc.o nhc.o
> +6lowpan-$(CONFIG_6LOWPAN_DEBUGFS) += debugfs.o
>   
>   #rfc6282 nhcs
>   obj-$(CONFIG_6LOWPAN_NHC_DEST) += nhc_dest.o
> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> index 83b19e0..fe58509 100644
> --- a/net/6lowpan/core.c
> +++ b/net/6lowpan/core.c
> @@ -15,7 +15,9 @@
>   
>   #include <net/6lowpan.h>
>   
> -void lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
> +#include "6lowpan_i.h"
> +
> +int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>   {
>   	dev->addr_len = EUI64_ADDR_LEN;
>   	dev->type = ARPHRD_6LOWPAN;
> @@ -23,11 +25,25 @@ void lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>   	dev->priv_flags |= IFF_NO_QUEUE;
>   
>   	lowpan_priv(dev)->lltype = lltype;
> +
> +	return lowpan_dev_debugfs_init(dev);
>   }
>   EXPORT_SYMBOL(lowpan_netdev_setup);
>   
> +void lowpan_netdev_unsetup(struct net_device *dev)
> +{
> +	lowpan_dev_debugfs_uninit(dev);
> +}
> +EXPORT_SYMBOL(lowpan_netdev_unsetup);
> +
>   static int __init lowpan_module_init(void)
>   {
> +	int ret;
> +
> +	ret = lowpan_debugfs_init();
> +	if (ret < 0)
> +		return ret;
> +
>   	request_module_nowait("ipv6");
>   
>   	request_module_nowait("nhc_dest");
> @@ -40,6 +56,13 @@ static int __init lowpan_module_init(void)
>   
>   	return 0;
>   }
> +
> +static void __exit lowpan_module_exit(void)
> +{
> +	lowpan_debugfs_exit();
> +}
> +
>   module_init(lowpan_module_init);
> +module_exit(lowpan_module_exit);
>   
>   MODULE_LICENSE("GPL");
> diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
> new file mode 100644
> index 0000000..b0a26fd
> --- /dev/null
> +++ b/net/6lowpan/debugfs.c
> @@ -0,0 +1,54 @@
> +/* This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Authors:
> + * (C) 2015 Pengutronix, Alexander Aring<aar@pengutronix.de>
> + * Copyright (c)  2015 Nordic Semiconductor. All Rights Reserved.
> + */
> +
> +#include <net/6lowpan.h>
> +
> +#include "6lowpan_i.h"
> +
> +static struct dentry *lowpan_debugfs;
> +
> +int lowpan_dev_debugfs_init(struct net_device *dev)
> +{
> +	struct lowpan_priv *lpriv = lowpan_priv(dev);
> +
> +	/* creating the root */
> +	lpriv->iface_debugfs = debugfs_create_dir(dev->name, lowpan_debugfs);
> +	if (!lpriv->iface_debugfs)
> +		goto fail;
> +
> +	return 0;
> +
> +fail:
> +	lowpan_dev_debugfs_uninit(dev);
> +	return -EINVAL;
> +}
> +
> +void lowpan_dev_debugfs_uninit(struct net_device *dev)
> +{
> +	debugfs_remove_recursive(lowpan_priv(dev)->iface_debugfs);
> +}
> +
> +int __init lowpan_debugfs_init(void)
> +{
> +	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
> +	if (!lowpan_debugfs)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +void lowpan_debugfs_exit(void)
> +{
> +	debugfs_remove_recursive(lowpan_debugfs);
> +}
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index 9e9cca3..d10ce57 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -825,16 +825,14 @@ static int setup_netdev(struct l2cap_chan *chan, struct lowpan_dev **dev)
>   	list_add_rcu(&(*dev)->list, &bt_6lowpan_devices);
>   	spin_unlock(&devices_lock);
>   
> -	lowpan_netdev_setup(netdev, LOWPAN_LLTYPE_BTLE);
> +	err = lowpan_netdev_setup(netdev, LOWPAN_LLTYPE_BTLE);
> +	if (err < 0)
> +		goto free_netdev;
>   
>   	err = register_netdev(netdev);
>   	if (err < 0) {
>   		BT_INFO("register_netdev failed %d", err);
> -		spin_lock(&devices_lock);
> -		list_del_rcu(&(*dev)->list);
> -		spin_unlock(&devices_lock);
> -		free_netdev(netdev);
> -		goto out;
> +		goto unsetup_lowpan;

Again teardown, please.

>   	}
>   
>   	BT_DBG("ifindex %d peer bdaddr %pMR type %d my addr %pMR type %d",
> @@ -844,7 +842,15 @@ static int setup_netdev(struct l2cap_chan *chan, struct lowpan_dev **dev)
>   
>   	return 0;
>   
> -out:
> +unsetup_lowpan:
> +	lowpan_netdev_unsetup(netdev);
> +
> +free_netdev:
> +	spin_lock(&devices_lock);
> +	list_del_rcu(&(*dev)->list);
> +	spin_unlock(&devices_lock);
> +	free_netdev(netdev);
> +
>   	return err;
>   }
>   
> @@ -1348,6 +1354,7 @@ static void disconnect_devices(void)
>   		ifdown(entry->netdev);
>   		BT_DBG("Unregistering netdev %s %p",
>   		       entry->netdev->name, entry->netdev);
> +		lowpan_netdev_unsetup(entry->netdev);
>   		unregister_netdev(entry->netdev);
>   		kfree(entry);
>   	}
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 20c49c7..3b42a75 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -161,16 +161,23 @@ static int lowpan_newlink(struct net *src_net, struct net_device *ldev,
>   				wdev->needed_headroom;
>   	ldev->needed_tailroom = wdev->needed_tailroom;
>   
> -	lowpan_netdev_setup(ldev, LOWPAN_LLTYPE_IEEE802154);
> +	ret = lowpan_netdev_setup(ldev, LOWPAN_LLTYPE_IEEE802154);
> +	if (ret < 0)
> +		goto dev_put;
>   
>   	ret = register_netdevice(ldev);
> -	if (ret < 0) {
> -		dev_put(wdev);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto unsetup_lowpan;
>   

Teardown
>   	wdev->ieee802154_ptr->lowpan_dev = ldev;
>   	return 0;
> +
> +unsetup_lowpan:
> +	lowpan_netdev_unsetup(ldev);
> +
> +dev_put:
> +	dev_put(wdev);
> +	return ret;
>   }
>   
>   static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
> @@ -180,6 +187,7 @@ static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
>   	ASSERT_RTNL();
>   
>   	wdev->ieee802154_ptr->lowpan_dev = NULL;
> +	lowpan_netdev_unsetup(ldev);
>   	unregister_netdevice(ldev);
>   	dev_put(wdev);
>   }

If you are going to change the th uninit to exit and the unsetup to 
teardown you can add my review.

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

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
  2015-11-18 13:55         ` Sergei Shtylyov
  (?)
@ 2015-11-25 16:42         ` Stefan Schmidt
  -1 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-25 16:42 UTC (permalink / raw)
  To: Sergei Shtylyov, Alexander Aring, linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 18/11/15 14:55, Sergei Shtylyov wrote:
> Hello.
>
> On 11/18/2015 1:33 AM, Alexander Aring wrote:
>
>> This patch adds a static inline function ipv6_addr_prefix_cpy which
>
>    I suggest not to reduce "copy".

Agreed. Not worth saving one character here.

>
>> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
>> The prefix len is given by plen as bits. This function mainly based on
>> ipv6_addr_prefix which copies one address prefix from address into a new
>> ipv6 address destination and zero all other address bits.
>>
>> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
>> ipv6 address, it sets a prefix to an ipv6 address with keeping other
>> address bits. The use case is for context based address compression
>> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
>> table to lookup address-bits without sending them.
>>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
>> Cc: James Morris <jmorris@namei.org>
>> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
>> Cc: Patrick McHardy <kaber@trash.net>
>> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
>> ---
>>   include/net/ipv6.h | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>> index e1a10b0..9d38fc2 100644
>> --- a/include/net/ipv6.h
>> +++ b/include/net/ipv6.h
>> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct 
>> in6_addr *pfx,
>>           pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>>   }
>>
>> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
>> +                    const struct in6_addr *pfx,
>> +                    int plen)
>> +{
>> +    /* caller must guarantee 0 <= plen <= 128 */
>> +    int o = plen >> 3,
>> +        b = plen & 0x7;
>
>    Unusual declaration style, why not just have *int* on both lines?
>

He took that from ipv6_addr_prefix() defined above it. I would also 
prefer a second line with int for the second declaration. But as he 
followed the coding style already around I think both way are fine.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
  2015-11-17 22:33     ` Alexander Aring
@ 2015-11-25 16:42         ` Stefan Schmidt
  -1 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-25 16:42 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 17/11/15 23:33, Alexander Aring wrote:
> This patch adds a static inline function ipv6_addr_prefix_cpy which
> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
> The prefix len is given by plen as bits. This function mainly based on
> ipv6_addr_prefix which copies one address prefix from address into a new
> ipv6 address destination and zero all other address bits.
>
> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
> ipv6 address, it sets a prefix to an ipv6 address with keeping other
> address bits. The use case is for context based address compression
> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
> table to lookup address-bits without sending them.
>
> Cc: David S. Miller<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Alexey Kuznetsov<kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> Cc: James Morris<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> Cc: Hideaki YOSHIFUJI<yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> Cc: Patrick McHardy<kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> Signed-off-by: Alexander Aring<alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   include/net/ipv6.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index e1a10b0..9d38fc2 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
>   		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>   }
>   
> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
> +					const struct in6_addr *pfx,
> +					int plen)
> +{

Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to 
read.
> +	/* caller must guarantee 0 <= plen <= 128 */
> +	int o = plen >> 3,
> +	    b = plen & 0x7;
> +

Any reason you are not doing the memset here before memcpy like it is 
done in ipv6_addr_prefi()?
> +	memcpy(addr->s6_addr, pfx, o);
> +	if (b != 0) {
> +		addr->s6_addr[o] &= ~(0xff00 >> b);
> +		addr->s6_addr[o] |= (pfx->s6_addr[o] & (0xff00 >> b));
> +	}
> +}
> +
>   static inline void __ipv6_addr_set_half(__be32 *addr,
>   					__be32 wh, __be32 wl)
>   {

regards
Stefan Schmidt

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
@ 2015-11-25 16:42         ` Stefan Schmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-25 16:42 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 17/11/15 23:33, Alexander Aring wrote:
> This patch adds a static inline function ipv6_addr_prefix_cpy which
> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
> The prefix len is given by plen as bits. This function mainly based on
> ipv6_addr_prefix which copies one address prefix from address into a new
> ipv6 address destination and zero all other address bits.
>
> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
> ipv6 address, it sets a prefix to an ipv6 address with keeping other
> address bits. The use case is for context based address compression
> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
> table to lookup address-bits without sending them.
>
> Cc: David S. Miller<davem@davemloft.net>
> Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru>
> Cc: James Morris<jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy<kaber@trash.net>
> Signed-off-by: Alexander Aring<alex.aring@gmail.com>
> ---
>   include/net/ipv6.h | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index e1a10b0..9d38fc2 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
>   		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>   }
>   
> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
> +					const struct in6_addr *pfx,
> +					int plen)
> +{

Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to 
read.
> +	/* caller must guarantee 0 <= plen <= 128 */
> +	int o = plen >> 3,
> +	    b = plen & 0x7;
> +

Any reason you are not doing the memset here before memcpy like it is 
done in ipv6_addr_prefi()?
> +	memcpy(addr->s6_addr, pfx, o);
> +	if (b != 0) {
> +		addr->s6_addr[o] &= ~(0xff00 >> b);
> +		addr->s6_addr[o] |= (pfx->s6_addr[o] & (0xff00 >> b));
> +	}
> +}
> +
>   static inline void __ipv6_addr_set_half(__be32 *addr,
>   					__be32 wh, __be32 wl)
>   {

regards
Stefan Schmidt

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
  2015-11-17 22:33 ` [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression Alexander Aring
@ 2015-11-25 17:12       ` Stefan Schmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-25 17:12 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc

Helllo.

On 17/11/15 23:33, Alexander Aring wrote:
> This patch introduce support for IPHC stateful address compression. It
> will offer three debugfs per interface entries, which are:
>
>   - dci_table: destination context indentifier table
>   - sci_table: source context indentifier table
>   - mcast_sci_table: multicast context identifier table
>
> Example to setup a context id:
>
> A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
> contexts which are available. Example:
>
> ID ipv6-address/prefix-length                  state
> 0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 15 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>
> For setting a context e.g. context id 0, context 2001::, prefix-length
> 64.
>
> Hint: Simple copy one line and then maniuplate it.
>
> echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
> /sys/kernel/debug/6lowpan/lowpan0/dci_table
>
> The state will display currently if the context is disabled(0) or
> enabled(1).
>
> On transmit side:
>
> The IPHC code will automatically search for a context which would be
> match for the address. Then it will be use the context which the
> best compression, means the longest prefix which match will be used.
>
> Example:
>
> 2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
> last bit of the address which has the prefix 2001::/127 is the same like
> the IID from the Encapsulating Header. A context ID can also be a
> 2001::1/128, which is then a full ipv6 address.
>
> On Receive side:
>
> If there is a context defined (when CID not available then it's the
> default context 0) then it will be used, if the header doesn't set
> SAC or DAC bit thens, it will be dropped.
>
> Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   include/net/6lowpan.h   |  62 ++++++
>   net/6lowpan/6lowpan_i.h |   3 +
>   net/6lowpan/Kconfig     |   1 +
>   net/6lowpan/core.c      |  17 ++
>   net/6lowpan/debugfs.c   | 109 +++++++++++
>   net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
>   6 files changed, 635 insertions(+), 57 deletions(-)
>
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index e484898..6e8d30e 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -75,6 +75,8 @@
>   #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
>   					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
>   					 LOWPAN_NHC_MAX_HDR_LEN)
> +/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
> +#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
>   
>   #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
>   #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
> @@ -98,9 +100,45 @@ enum lowpan_lltypes {
>   	LOWPAN_LLTYPE_IEEE802154,
>   };
>   
> +enum lowpan_iphc_ctx_states {
> +	LOWPAN_IPHC_CTX_STATE_DISABLED,
> +	LOWPAN_IPHC_CTX_STATE_ENABLED,
> +
> +	__LOWPAN_IPHC_CTX_STATE_INVALID,
> +	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
> +};
> +

You are expecting more states here besides enabled and disabled? Because 
normally a bool would be fine here and an enum overkill. If you have 
more states pending it makes sense to have the enum though.

> +struct lowpan_iphc_ctx {
> +	enum lowpan_iphc_ctx_states state;
> +	u8 id;
> +	struct in6_addr addr;
> +	u8 prefix_len;
> +};
> +
> +struct lowpan_iphc_ctx_ops {
> +	bool (*valid_prefix)(const struct lowpan_iphc_ctx *ctx);
> +	int (*update)(struct lowpan_iphc_ctx *table,
> +		      const struct lowpan_iphc_ctx *ctx);
> +	struct lowpan_iphc_ctx *(*get_by_id)(const struct net_device *dev,
> +					     struct lowpan_iphc_ctx *table,
> +					     u8 id);
> +	struct lowpan_iphc_ctx *(*get_by_addr)(const struct net_device *dev,
> +					       struct lowpan_iphc_ctx *table,
> +					       const struct in6_addr *addr);
> +};
> +
> +struct lowpan_iphc_ctx_table {
> +	spinlock_t lock;
> +	const struct lowpan_iphc_ctx_ops *ops;
> +	struct lowpan_iphc_ctx table[LOWPAN_IPHC_CI_TABLE_SIZE];
> +};
> +
>   struct lowpan_priv {
>   	enum lowpan_lltypes lltype;
>   	struct dentry *iface_debugfs;
> +	struct lowpan_iphc_ctx_table iphc_dci;
> +	struct lowpan_iphc_ctx_table iphc_sci;
> +	struct lowpan_iphc_ctx_table iphc_mcast_dci;
>   
>   	/* must be last */
>   	u8 priv[0] __aligned(sizeof(void *));
> @@ -125,6 +163,30 @@ struct lowpan_802154_cb *lowpan_802154_cb(const struct sk_buff *skb)
>   	return (struct lowpan_802154_cb *)skb->cb;
>   }
>   
> +static inline int lowpan_ctx_update(struct lowpan_iphc_ctx_table *t,
> +				    const struct lowpan_iphc_ctx *ctx)
> +{
> +	if (!t->ops->valid_prefix(ctx))
> +		return -EINVAL;
> +
> +	return t->ops->update(t->table, ctx);
> +}
> +
> +static inline struct lowpan_iphc_ctx *
> +lowpan_ctx_by_id(const struct net_device *dev, struct lowpan_iphc_ctx_table *t,
> +		 u8 id)
> +{
> +	return t->ops->get_by_id(dev, t->table, id);
> +}
> +
> +static inline struct lowpan_iphc_ctx *
> +lowpan_ctx_by_addr(const struct net_device *dev,
> +		   struct lowpan_iphc_ctx_table *t,
> +		   const struct in6_addr *addr)
> +{
> +	return t->ops->get_by_addr(dev, t->table, addr);
> +}
> +
>   #ifdef DEBUG
>   /* print data in line */
>   static inline void raw_dump_inline(const char *caller, char *msg,
> diff --git a/net/6lowpan/6lowpan_i.h b/net/6lowpan/6lowpan_i.h
> index 33b2605..1312340 100644
> --- a/net/6lowpan/6lowpan_i.h
> +++ b/net/6lowpan/6lowpan_i.h
> @@ -3,6 +3,9 @@
>   
>   #include <linux/netdevice.h>
>   
> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
> +
>   #ifdef CONFIG_6LOWPAN_DEBUGFS
>   int lowpan_dev_debugfs_init(struct net_device *dev);
>   void lowpan_dev_debugfs_uninit(struct net_device *dev);
> diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
> index 01c901c..7ecedd7 100644
> --- a/net/6lowpan/Kconfig
> +++ b/net/6lowpan/Kconfig
> @@ -8,6 +8,7 @@ menuconfig 6LOWPAN
>   config 6LOWPAN_DEBUGFS
>   	bool "6LoWPAN debugfs support"
>   	depends on 6LOWPAN
> +	depends on DEBUG_FS

This looks like a fix that should be merged into the first patch, no?
>   	---help---
>   	  This enables 6LoWPAN debugfs support. For example to manipulate
>   	  IPHC context information at runtime.
> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> index fe58509..d354c5b 100644
> --- a/net/6lowpan/core.c
> +++ b/net/6lowpan/core.c
> @@ -19,6 +19,8 @@
>   
>   int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>   {
> +	int i;
> +
>   	dev->addr_len = EUI64_ADDR_LEN;
>   	dev->type = ARPHRD_6LOWPAN;
>   	dev->mtu = IPV6_MIN_MTU;
> @@ -26,6 +28,21 @@ int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>   
>   	lowpan_priv(dev)->lltype = lltype;
>   
> +	spin_lock_init(&lowpan_priv(dev)->iphc_dci.lock);
> +	lowpan_priv(dev)->iphc_dci.ops = &iphc_ctx_unicast_ops;
> +
> +	spin_lock_init(&lowpan_priv(dev)->iphc_sci.lock);
> +	lowpan_priv(dev)->iphc_sci.ops = &iphc_ctx_unicast_ops;
> +
> +	spin_lock_init(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +	lowpan_priv(dev)->iphc_mcast_dci.ops = &iphc_ctx_mcast_ops;
> +
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		lowpan_priv(dev)->iphc_dci.table[i].id = i;
> +		lowpan_priv(dev)->iphc_sci.table[i].id = i;
> +		lowpan_priv(dev)->iphc_mcast_dci.table[i].id = i;
> +	}
> +
>   	return lowpan_dev_debugfs_init(dev);
>   }
>   EXPORT_SYMBOL(lowpan_netdev_setup);
> diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
> index b0a26fd..d6481eb 100644
> --- a/net/6lowpan/debugfs.c
> +++ b/net/6lowpan/debugfs.c
> @@ -18,15 +18,124 @@
>   
>   static struct dentry *lowpan_debugfs;
>   
> +static int lowpan_context_show(struct seq_file *file, void *offset)
> +{
> +	struct lowpan_iphc_ctx_table *t = file->private;
> +	int i;
> +
> +	seq_printf(file, "%-2s %-43s %s\n", "ID", "ipv6-address/prefix-length",
> +		   "state");
> +
> +	spin_lock_bh(&t->lock);
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		seq_printf(file,
> +			   "%-2d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%03d %d\n",
> +			   t->table[i].id,
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[0]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[1]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[2]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[3]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[4]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[5]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[6]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[7]),
> +			   t->table[i].prefix_len, t->table[i].state);
> +	}
> +	spin_unlock_bh(&t->lock);
> +
> +	return 0;
> +}
> +
> +static int lowpan_context_dbgfs_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, lowpan_context_show, inode->i_private);
> +}
> +
> +static ssize_t lowpan_context_dbgfs_write(struct file *fp,
> +					  const char __user *user_buf,
> +					  size_t count, loff_t *ppos)
> +{
> +	char buf[128] = { };
> +	struct seq_file *file = fp->private_data;
> +	struct lowpan_iphc_ctx_table *t = file->private;
> +	struct lowpan_iphc_ctx ctx;
> +	int status = count, n, id, state, i, prefix_len, ret;
> +	unsigned int addr[8];
> +
> +	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
> +						 count))) {
> +		status = -EFAULT;
> +		goto out;
> +	}
> +
> +	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
> +		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
> +		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
> +	if (n != 11) {

11 more like a magic number here. Not to bad, but a define could be nice.
> +		status = -EIO;
> +		goto out;
> +	}
> +
> +	if (id > LOWPAN_IPHC_CI_TABLE_SIZE - 1 ||
> +	    state > LOWPAN_IPHC_CTX_STATE_MAX || prefix_len > 128) {
> +		status = -EINVAL;
> +		goto out;
> +	}
> +
> +	ctx.id = id;
> +	ctx.state = state;
> +	ctx.prefix_len = prefix_len;
> +
> +	for (i = 0; i < 8; i++)
> +		ctx.addr.s6_addr16[i] = cpu_to_be16(addr[i] & 0xffff);
> +
> +	spin_lock_bh(&t->lock);
> +	ret = lowpan_ctx_update(t, &ctx);
> +	if (ret < 0)
> +		status = ret;
> +	spin_unlock_bh(&t->lock);
> +
> +out:
> +	return status;
> +}
> +
> +const struct file_operations lowpan_context_fops = {
> +	.open		= lowpan_context_dbgfs_open,
> +	.read		= seq_read,
> +	.write		= lowpan_context_dbgfs_write,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
>   int lowpan_dev_debugfs_init(struct net_device *dev)
>   {
>   	struct lowpan_priv *lpriv = lowpan_priv(dev);
> +	static struct dentry *dentry;
>   
>   	/* creating the root */
>   	lpriv->iface_debugfs = debugfs_create_dir(dev->name, lowpan_debugfs);
>   	if (!lpriv->iface_debugfs)
>   		goto fail;
>   
> +	dentry = debugfs_create_file("dci_table", 0664, lpriv->iface_debugfs,
> +				     &lowpan_priv(dev)->iphc_dci,
> +				     &lowpan_context_fops);
> +	if (!dentry)
> +		goto fail;
> +
> +	dentry = debugfs_create_file("sci_table", 0664, lpriv->iface_debugfs,
> +				     &lowpan_priv(dev)->iphc_sci,
> +				     &lowpan_context_fops);
> +	if (!dentry)
> +		goto fail;
> +
> +	dentry = debugfs_create_file("mcast_dci_table", 0664,
> +				     lpriv->iface_debugfs,
> +				     &lowpan_priv(dev)->iphc_mcast_dci,
> +				     &lowpan_context_fops);
> +	if (!dentry)
> +		goto fail;
> +
>   	return 0;
>   
>   fail:
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 346b5c1..5974ea5 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -56,6 +56,7 @@
>   /* special link-layer handling */
>   #include <net/mac802154.h>
>   
> +#include "6lowpan_i.h"
>   #include "nhc.h"
>   
>   /* Values of fields within the IPHC encoding first byte */
> @@ -147,6 +148,9 @@
>   	 (((a)->s6_addr16[6]) == 0) &&		\
>   	 (((a)->s6_addr[14]) == 0))
>   
> +#define LOWPAN_IPHC_CID_DCI(cid)	(cid & 0x0f)
> +#define LOWPAN_IPHC_CID_SCI(cid)	((cid & 0xf0) >> 4)
> +
>   static inline void iphc_uncompress_eui64_lladdr(struct in6_addr *ipaddr,
>   						const void *lladdr)
>   {
> @@ -259,30 +263,59 @@ static int uncompress_addr(struct sk_buff *skb, const struct net_device *dev,
>   /* Uncompress address function for source context
>    * based address(non-multicast).
>    */
> -static int uncompress_context_based_src_addr(struct sk_buff *skb,
> -					     struct in6_addr *ipaddr,
> -					     u8 address_mode)
> +static int uncompress_ctx_addr(struct sk_buff *skb,
> +			       const struct net_device *dev,
> +			       const struct lowpan_iphc_ctx *ctx,
> +			       struct in6_addr *ipaddr, u8 address_mode,
> +			       const void *lladdr)
>   {
> +	bool fail;
> +
>   	switch (address_mode) {
> -	case LOWPAN_IPHC_SAM_00:
> -		/* unspec address ::
> +	/* SAM and DAM are the same here */
> +	case LOWPAN_IPHC_DAM_00:
> +		fail = false;
> +		/* SAM_00 -> unspec address ::
>   		 * Do nothing, address is already ::
> +		 *
> +		 * DAM 00 -> reserved should never occur.
>   		 */
>   		break;
>   	case LOWPAN_IPHC_SAM_01:
> -		/* TODO */
> +	case LOWPAN_IPHC_DAM_01:
> +		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[8], 8);
> +		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
> +		break;
>   	case LOWPAN_IPHC_SAM_10:
> -		/* TODO */
> +	case LOWPAN_IPHC_DAM_10:
> +		ipaddr->s6_addr[11] = 0xFF;
> +		ipaddr->s6_addr[12] = 0xFE;
> +		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[14], 2);
> +		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
> +		break;
>   	case LOWPAN_IPHC_SAM_11:
> -		/* TODO */
> -		netdev_warn(skb->dev, "SAM value 0x%x not supported\n",
> -			    address_mode);
> -		return -EINVAL;
> +	case LOWPAN_IPHC_DAM_11:
> +		fail = false;
> +		switch (lowpan_priv(dev)->lltype) {
> +		case LOWPAN_LLTYPE_IEEE802154:
> +			iphc_uncompress_802154_lladdr(ipaddr, lladdr);
> +			break;
> +		default:
> +			iphc_uncompress_eui64_lladdr(ipaddr, lladdr);
> +			break;
> +		}
> +		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
> +		break;
>   	default:
>   		pr_debug("Invalid sam value: 0x%x\n", address_mode);
>   		return -EINVAL;
>   	}
>   
> +	if (fail) {
> +		pr_debug("Failed to fetch skb data\n");
> +		return -EIO;
> +	}
> +
>   	raw_dump_inline(NULL,
>   			"Reconstructed context based ipv6 src addr is",
>   			ipaddr->s6_addr, 16);
> @@ -346,6 +379,33 @@ static int lowpan_uncompress_multicast_daddr(struct sk_buff *skb,
>   	return 0;
>   }
>   
> +static int lowpan_uncompress_multicast_ctx_daddr(struct sk_buff *skb,
> +						 struct lowpan_iphc_ctx *ctx,
> +						 struct in6_addr *ipaddr,
> +						 u8 address_mode)
> +{
> +	struct in6_addr network_pfx = {};
> +	bool fail;
> +
> +	if (address_mode != LOWPAN_IPHC_DAM_00)
> +		return -EINVAL;
> +
> +	ipaddr->s6_addr[0] = 0xFF;
> +	fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[1], 2);
> +	fail |= lowpan_fetch_skb(skb, &ipaddr->s6_addr[12], 4);
> +	/* take prefix_len and network prefix from the context */
> +	ipaddr->s6_addr[3] = ctx->prefix_len;
> +	/* get network prefix to copy into multicast address */
> +	ipv6_addr_prefix(&network_pfx, &ctx->addr, ctx->prefix_len);
> +	/* setting network prefix */
> +	memcpy(&ipaddr->s6_addr[4], &network_pfx, 8);
> +
> +	if (fail < 0)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
>   /* get the ecn values from iphc tf format and set it to ipv6hdr */
>   static inline void lowpan_iphc_tf_set_ecn(struct ipv6hdr *hdr, const u8 *tf)
>   {
> @@ -459,7 +519,8 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   			     const void *daddr, const void *saddr)
>   {
>   	struct ipv6hdr hdr = {};
> -	u8 iphc0, iphc1;
> +	struct lowpan_iphc_ctx *ci;
> +	u8 iphc0, iphc1, cid = 0;
>   	int err;
>   
>   	raw_dump_table(__func__, "raw skb data dump uncompressed",
> @@ -469,12 +530,14 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)))
>   		return -EINVAL;
>   
> -	/* another if the CID flag is set */
> -	if (iphc1 & LOWPAN_IPHC_CID)
> -		return -ENOTSUPP;
> -
>   	hdr.version = 6;
>   
> +	/* default CID = 0, another if the CID flag is set */
> +	if (iphc1 & LOWPAN_IPHC_CID) {
> +		if (lowpan_fetch_skb(skb, &cid, sizeof(cid)))
> +			return -EINVAL;
> +	}
> +
>   	err = lowpan_iphc_tf_decompress(skb, &hdr,
>   					iphc0 & LOWPAN_IPHC_TF_MASK);
>   	if (err < 0)
> @@ -500,10 +563,18 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   	}
>   
>   	if (iphc1 & LOWPAN_IPHC_SAC) {
> -		/* Source address context based uncompression */
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_sci,
> +				      LOWPAN_IPHC_CID_SCI(cid));
> +		if (!ci) {
> +			spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +			return -EINVAL;
> +		}
> +
>   		pr_debug("SAC bit is set. Handle context based source address.\n");
> -		err = uncompress_context_based_src_addr(skb, &hdr.saddr,
> -							iphc1 & LOWPAN_IPHC_SAM_MASK);
> +		err = uncompress_ctx_addr(skb, dev, ci, &hdr.saddr,
> +					  iphc1 & LOWPAN_IPHC_SAM_MASK, saddr);
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
>   	} else {
>   		/* Source address uncompression */
>   		pr_debug("source address stateless compression\n");
> @@ -515,27 +586,54 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   	if (err)
>   		return -EINVAL;
>   
> -	/* check for Multicast Compression */
> -	if (iphc1 & LOWPAN_IPHC_M) {
> -		if (iphc1 & LOWPAN_IPHC_DAC) {
> -			pr_debug("dest: context-based mcast compression\n");
> -			/* TODO: implement this */
> -		} else {
> -			err = lowpan_uncompress_multicast_daddr(skb, &hdr.daddr,
> -								iphc1 & LOWPAN_IPHC_DAM_MASK);
> +	switch (iphc1 & (LOWPAN_IPHC_M | LOWPAN_IPHC_DAC)) {
> +	case LOWPAN_IPHC_M | LOWPAN_IPHC_DAC:
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_mcast_dci,
> +				      LOWPAN_IPHC_CID_DCI(cid));
> +		if (!ci) {
> +			spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +			return -EINVAL;
> +		}
>   
> -			if (err)
> -				return -EINVAL;
> +		/* multicast with context */
> +		pr_debug("dest: context-based mcast compression\n");
> +		err = lowpan_uncompress_multicast_ctx_daddr(skb, ci,
> +							    &hdr.daddr,
> +							    iphc1 & LOWPAN_IPHC_DAM_MASK);
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +		break;
> +	case LOWPAN_IPHC_M:
> +		/* multicast */
> +		err = lowpan_uncompress_multicast_daddr(skb, &hdr.daddr,
> +							iphc1 & LOWPAN_IPHC_DAM_MASK);
> +		break;
> +	case LOWPAN_IPHC_DAC:
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_dci,
> +				      LOWPAN_IPHC_CID_DCI(cid));
> +		if (!ci) {
> +			spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +			return -EINVAL;
>   		}
> -	} else {
> +
> +		/* Destination address context based uncompression */
> +		pr_debug("DAC bit is set. Handle context based destination address.\n");
> +		err = uncompress_ctx_addr(skb, dev, ci, &hdr.daddr,
> +					  iphc1 & LOWPAN_IPHC_DAM_MASK, daddr);
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +		break;
> +	default:
>   		err = uncompress_addr(skb, dev, &hdr.daddr,
>   				      iphc1 & LOWPAN_IPHC_DAM_MASK, daddr);
>   		pr_debug("dest: stateless compression mode %d dest %pI6c\n",
>   			 iphc1 & LOWPAN_IPHC_DAM_MASK, &hdr.daddr);
> -		if (err)
> -			return -EINVAL;
> +		break;
>   	}
>   
> +	if (err)
> +		return -EINVAL;
> +
>   	/* Next header data uncompression */
>   	if (iphc0 & LOWPAN_IPHC_NH) {
>   		err = lowpan_nhc_do_uncompression(skb, dev, &hdr);
> @@ -585,6 +683,60 @@ static const u8 lowpan_iphc_dam_to_sam_value[] = {
>   	[LOWPAN_IPHC_DAM_11] = LOWPAN_IPHC_SAM_11,
>   };
>   
> +static u8 lowpan_compress_ctx_addr(u8 **hc_ptr, const struct in6_addr *ipaddr,
> +				   const struct lowpan_iphc_ctx *ctx,
> +				   const unsigned char *lladdr, bool sam)
> +{
> +	struct in6_addr tmp = {};
> +	u8 dam;
> +
> +	/* check for SAM/DAM = 11 */
> +	memcpy(&tmp.s6_addr[8], lladdr, 8);
> +	/* second bit-flip (Universe/Local)
> +	 * is done according RFC2464
> +	 */
> +	tmp.s6_addr[8] ^= 0x02;
> +	/* context information are always used */
> +	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
> +	if (ipv6_addr_equal(&tmp, ipaddr)) {
> +		dam = LOWPAN_IPHC_DAM_11;
> +		goto out;
> +	}
> +
> +	memset(&tmp, 0, sizeof(tmp));
> +	/* check for SAM/DAM = 01 */
> +	tmp.s6_addr[11] = 0xFF;
> +	tmp.s6_addr[12] = 0xFE;
> +	memcpy(&tmp.s6_addr[14], &ipaddr->s6_addr[14], 2);
> +	/* context information are always used */
> +	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
> +	if (ipv6_addr_equal(&tmp, ipaddr)) {
> +		lowpan_push_hc_data(hc_ptr, &ipaddr->s6_addr[14], 2);
> +		dam = LOWPAN_IPHC_DAM_10;
> +		goto out;
> +	}
> +
> +	memset(&tmp, 0, sizeof(tmp));
> +	/* check for SAM/DAM = 10, should always match */
> +	memcpy(&tmp.s6_addr[8], &ipaddr->s6_addr[8], 8);
> +	/* context information are always used */
> +	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
> +	if (ipv6_addr_equal(&tmp, ipaddr)) {
> +		lowpan_push_hc_data(hc_ptr, &ipaddr->s6_addr[8], 8);
> +		dam = LOWPAN_IPHC_DAM_01;
> +		goto out;
> +	}
> +
> +	WARN_ON_ONCE("context found but no address mode matched\n");
> +	return -EINVAL;
> +out:
> +
> +	if (sam)
> +		return lowpan_iphc_dam_to_sam_value[dam];
> +	else
> +		return dam;
> +}
> +
>   static u8 lowpan_compress_addr_64(u8 **hc_ptr, const struct in6_addr *ipaddr,
>   				  const unsigned char *lladdr, bool sam)
>   {
> @@ -708,6 +860,21 @@ static u8 lowpan_iphc_tf_compress(u8 **hc_ptr, const struct ipv6hdr *hdr)
>   	return val;
>   }
>   
> +static u8 lowpan_iphc_mcast_ctx_addr_compress(u8 **hc_ptr,
> +					      const struct lowpan_iphc_ctx *ctx,
> +					      const struct in6_addr *ipaddr)
> +{
> +	u8 data[6];
> +
> +	/* flags/scope, reserved (RIID) */
> +	memcpy(data, &ipaddr->s6_addr[1], 2);
> +	/* group ID */
> +	memcpy(&data[1], &ipaddr->s6_addr[11], 4);
> +	lowpan_push_hc_data(hc_ptr, data, 6);
> +
> +	return LOWPAN_IPHC_DAM_00;
> +}
> +
>   static u8 lowpan_iphc_mcast_addr_compress(u8 **hc_ptr,
>   					  const struct in6_addr *ipaddr)
>   {
> @@ -742,10 +909,11 @@ static u8 lowpan_iphc_mcast_addr_compress(u8 **hc_ptr,
>   int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   			   const void *daddr, const void *saddr)
>   {
> -	u8 iphc0, iphc1, *hc_ptr;
> +	u8 iphc0, iphc1, *hc_ptr, cid = 0;
>   	struct ipv6hdr *hdr;
>   	u8 head[LOWPAN_IPHC_MAX_HC_BUF_LEN] = {};
> -	int ret, addr_type;
> +	struct lowpan_iphc_ctx *dci, *sci, dci_entry, sci_entry;
> +	int ret, ipv6_daddr_type, ipv6_saddr_type;
>   
>   	if (skb->protocol != htons(ETH_P_IPV6))
>   		return -EINVAL;
> @@ -769,14 +937,47 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   	iphc0 = LOWPAN_DISPATCH_IPHC;
>   	iphc1 = 0;
>   
> -	/* TODO: context lookup */
> -
>   	raw_dump_inline(__func__, "saddr", saddr, EUI64_ADDR_LEN);
>   	raw_dump_inline(__func__, "daddr", daddr, EUI64_ADDR_LEN);
>   
>   	raw_dump_table(__func__, "sending raw skb network uncompressed packet",
>   		       skb->data, skb->len);
>   
> +	ipv6_daddr_type = ipv6_addr_type(&hdr->daddr);
> +	if (ipv6_daddr_type & IPV6_ADDR_MULTICAST) {
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +		dci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_mcast_dci,
> +					 &hdr->daddr);
> +		if (dci) {
> +			memcpy(&dci_entry, dci, sizeof(*dci));
> +			cid |= dci->id;
> +		}
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +	} else {
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +		dci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_dci,
> +					 &hdr->daddr);
> +		if (dci) {
> +			memcpy(&dci_entry, dci, sizeof(*dci));
> +			cid |= dci->id;
> +		}
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +	}
> +
> +	spin_lock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +	sci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_sci,
> +				 &hdr->saddr);
> +	if (sci) {
> +		memcpy(&sci_entry, sci, sizeof(*sci));
> +		cid |= (sci->id << 4);
> +	}
> +	spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +
> +	if (sci || dci) {
> +		iphc1 |= LOWPAN_IPHC_CID;
> +		lowpan_push_hc_data(&hc_ptr, &cid, sizeof(cid));
> +	}
> +
>   	/* Traffic Class, Flow Label compression */
>   	iphc0 |= lowpan_iphc_tf_compress(&hc_ptr, hdr);
>   
> @@ -813,39 +1014,63 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   				    sizeof(hdr->hop_limit));
>   	}
>   
> -	addr_type = ipv6_addr_type(&hdr->saddr);
> +	ipv6_saddr_type = ipv6_addr_type(&hdr->saddr);
>   	/* source address compression */
> -	if (addr_type == IPV6_ADDR_ANY) {
> +	if (ipv6_saddr_type == IPV6_ADDR_ANY) {
>   		pr_debug("source address is unspecified, setting SAC\n");
>   		iphc1 |= LOWPAN_IPHC_SAC;
>   	} else {
> -		if (addr_type & IPV6_ADDR_LINKLOCAL) {
> -			iphc1 |= lowpan_compress_addr_64(&hc_ptr, &hdr->saddr,
> -							 saddr, true);
> -			pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
> -				 &hdr->saddr, iphc1);
> +		if (sci) {
> +			iphc1 |= lowpan_compress_ctx_addr(&hc_ptr, &hdr->saddr,
> +							  &sci_entry, saddr,
> +							  true);
> +			iphc1 |= LOWPAN_IPHC_SAC;
>   		} else {
> -			pr_debug("send the full source address\n");
> -			lowpan_push_hc_data(&hc_ptr, hdr->saddr.s6_addr, 16);
> +			if (ipv6_saddr_type & IPV6_ADDR_LINKLOCAL) {
> +				iphc1 |= lowpan_compress_addr_64(&hc_ptr,
> +								 &hdr->saddr,
> +								 saddr, true);
> +				pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
> +					 &hdr->saddr, iphc1);
> +			} else {
> +				pr_debug("send the full source address\n");
> +				lowpan_push_hc_data(&hc_ptr,
> +						    hdr->saddr.s6_addr, 16);
> +			}
>   		}
>   	}
>   
> -	addr_type = ipv6_addr_type(&hdr->daddr);
>   	/* destination address compression */
> -	if (addr_type & IPV6_ADDR_MULTICAST) {
> +	if (ipv6_daddr_type & IPV6_ADDR_MULTICAST) {
>   		pr_debug("destination address is multicast: ");
> -		iphc1 |= LOWPAN_IPHC_M;
> -		iphc1 |= lowpan_iphc_mcast_addr_compress(&hc_ptr, &hdr->daddr);
> +		if (dci) {
> +			iphc1 |= lowpan_iphc_mcast_ctx_addr_compress(&hc_ptr,
> +								     &dci_entry,
> +								     &hdr->daddr);
> +		} else {
> +			iphc1 |= LOWPAN_IPHC_M;
> +			iphc1 |= lowpan_iphc_mcast_addr_compress(&hc_ptr,
> +								 &hdr->daddr);
> +		}
>   	} else {
> -		if (addr_type & IPV6_ADDR_LINKLOCAL) {
> -			/* TODO: context lookup */
> -			iphc1 |= lowpan_compress_addr_64(&hc_ptr, &hdr->daddr,
> -							 daddr, false);
> -			pr_debug("dest address unicast link-local %pI6c "
> -				 "iphc1 0x%02x\n", &hdr->daddr, iphc1);
> +		if (dci) {
> +			iphc1 |= lowpan_compress_ctx_addr(&hc_ptr, &hdr->daddr,
> +							  &dci_entry, daddr,
> +							  false);
> +			iphc1 |= LOWPAN_IPHC_DAC;
>   		} else {
> -			pr_debug("dest address unicast %pI6c\n", &hdr->daddr);
> -			lowpan_push_hc_data(&hc_ptr, hdr->daddr.s6_addr, 16);
> +			if (ipv6_daddr_type & IPV6_ADDR_LINKLOCAL) {
> +				iphc1 |= lowpan_compress_addr_64(&hc_ptr,
> +								 &hdr->daddr,
> +								 daddr, false);
> +				pr_debug("dest address unicast link-local %pI6c iphc1 0x%02x\n",
> +					 &hdr->daddr, iphc1);
> +			} else {
> +				pr_debug("dest address unicast %pI6c\n",
> +					 &hdr->daddr);
> +				lowpan_push_hc_data(&hc_ptr,
> +						    hdr->daddr.s6_addr, 16);
> +			}
>   		}
>   	}
>   
> @@ -871,3 +1096,164 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(lowpan_header_compress);
> +
> +static bool lowpan_iphc_ctx_valid_prefix(const struct lowpan_iphc_ctx *ctx)
> +{
> +	/* prefix which are smaller than 64 bits are not valid, users
> +	 * may mean than a prefix which is filled with zero until 64th bit.
> +	 * Refere rfc6282 "Any remaining bits are zero." The remaining bits
> +	 * in this case where are the prefix(< 64) ends until IID starts.
> +	 */
> +	if (ctx->prefix_len < 64)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool
> +lowpan_iphc_mcast_ctx_valid_prefix(const struct lowpan_iphc_ctx *ctx)
> +{
> +	/* network prefix for multicast is at maximum 64 bits long */
> +	if (ctx->prefix_len > 64)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int lowpan_iphc_ctx_update(struct lowpan_iphc_ctx *table,
> +				  const struct lowpan_iphc_ctx *ctx)
> +{
> +	int ret = 0, i;
> +
> +	switch (ctx->state) {
> +	case LOWPAN_IPHC_CTX_STATE_DISABLED:
> +		/* we don't care about if disabled */
> +		break;
> +	case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +		for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +			if (ctx->prefix_len != table[i].prefix_len)
> +				continue;
> +
> +			if (ipv6_prefix_equal(&ctx->addr, &table[i].addr,
> +					      ctx->prefix_len)) {
> +				switch (table[i].state) {
> +				case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +					ret = -EEXIST;
> +					goto out;
> +				default:
> +					break;
> +				}
> +			}
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	memcpy(&table[ctx->id], ctx, sizeof(*ctx));
> +
> +out:
> +	return ret;
> +}
> +
> +static struct lowpan_iphc_ctx *
> +lowpan_iphc_ctx_get_by_id(const struct net_device *dev,
> +			  struct lowpan_iphc_ctx *table, u8 id)
> +{
> +	struct lowpan_iphc_ctx *ret = NULL;
> +
> +	WARN_ON_ONCE(id > LOWPAN_IPHC_CI_TABLE_SIZE);
> +
> +	switch (table[id].state) {
> +	case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +		ret = &table[id];
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static struct lowpan_iphc_ctx *
> +lowpan_iphc_ctx_get_by_addr(const struct net_device *dev,
> +			    struct lowpan_iphc_ctx *table,
> +			    const struct in6_addr *addr)
> +{
> +	struct lowpan_iphc_ctx *ret = NULL;
> +	struct in6_addr addr_prefix;
> +	int i;
> +
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		ipv6_addr_prefix(&addr_prefix, addr, table[i].prefix_len);
> +
> +		if (ipv6_prefix_equal(&addr_prefix, &table[i].addr,
> +				      table[i].prefix_len)) {
> +			switch (table[i].state) {
> +			case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +				/* remember first match */
> +				if (!ret) {
> +					ret = &table[i];
> +					continue;
> +				}
> +
> +				/* get the context with longest prefix_len */
> +				if (table[i].prefix_len > ret->prefix_len)
> +					ret = &table[i];
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static struct lowpan_iphc_ctx *
> +lowpan_iphc_ctx_get_by_mcast_addr(const struct net_device *dev,
> +				  struct lowpan_iphc_ctx *table,
> +				  const struct in6_addr *addr)
> +{
> +	struct lowpan_iphc_ctx *ret = NULL;
> +	struct in6_addr addr_mcast, network_pfx = {};
> +	int i;
> +
> +	/* init mcast address with  */
> +	memcpy(&addr_mcast, addr, sizeof(*addr));
> +
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		/* setting plen */
> +		addr_mcast.s6_addr[3] = table[i].prefix_len;
> +		/*  get network prefix to copy into multicast address */
> +		ipv6_addr_prefix(&network_pfx, &table[i].addr,
> +				 table[i].prefix_len);
> +		/* setting network prefix */
> +		memcpy(&addr_mcast.s6_addr[4], &network_pfx, 8);
> +
> +		if (ipv6_addr_equal(addr, &addr_mcast)) {
> +			switch (table[i].state) {
> +			case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +				return &table[i];
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops = {
> +	.valid_prefix = lowpan_iphc_ctx_valid_prefix,
> +	.update = lowpan_iphc_ctx_update,
> +	.get_by_id = lowpan_iphc_ctx_get_by_id,
> +	.get_by_addr = lowpan_iphc_ctx_get_by_addr,
> +};
> +
> +const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
> +	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
> +	.update = lowpan_iphc_ctx_update,
> +	.get_by_id = lowpan_iphc_ctx_get_by_id,
> +	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
> +};

These are the comments from a first look over it. I will have a more 
detailed review and actual testing once you are happy enough with it to 
move it from RFC to PATCH.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
@ 2015-11-25 17:12       ` Stefan Schmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-25 17:12 UTC (permalink / raw)
  To: Alexander Aring, linux-wpan
  Cc: linux-bluetooth, netdev, kernel, mcr, lukasz.duda, martin.gergeleit

Helllo.

On 17/11/15 23:33, Alexander Aring wrote:
> This patch introduce support for IPHC stateful address compression. It
> will offer three debugfs per interface entries, which are:
>
>   - dci_table: destination context indentifier table
>   - sci_table: source context indentifier table
>   - mcast_sci_table: multicast context identifier table
>
> Example to setup a context id:
>
> A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
> contexts which are available. Example:
>
> ID ipv6-address/prefix-length                  state
> 0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> 15 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>
> For setting a context e.g. context id 0, context 2001::, prefix-length
> 64.
>
> Hint: Simple copy one line and then maniuplate it.
>
> echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
> /sys/kernel/debug/6lowpan/lowpan0/dci_table
>
> The state will display currently if the context is disabled(0) or
> enabled(1).
>
> On transmit side:
>
> The IPHC code will automatically search for a context which would be
> match for the address. Then it will be use the context which the
> best compression, means the longest prefix which match will be used.
>
> Example:
>
> 2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
> last bit of the address which has the prefix 2001::/127 is the same like
> the IID from the Encapsulating Header. A context ID can also be a
> 2001::1/128, which is then a full ipv6 address.
>
> On Receive side:
>
> If there is a context defined (when CID not available then it's the
> default context 0) then it will be used, if the header doesn't set
> SAC or DAC bit thens, it will be dropped.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   include/net/6lowpan.h   |  62 ++++++
>   net/6lowpan/6lowpan_i.h |   3 +
>   net/6lowpan/Kconfig     |   1 +
>   net/6lowpan/core.c      |  17 ++
>   net/6lowpan/debugfs.c   | 109 +++++++++++
>   net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
>   6 files changed, 635 insertions(+), 57 deletions(-)
>
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index e484898..6e8d30e 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -75,6 +75,8 @@
>   #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
>   					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
>   					 LOWPAN_NHC_MAX_HDR_LEN)
> +/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
> +#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
>   
>   #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
>   #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
> @@ -98,9 +100,45 @@ enum lowpan_lltypes {
>   	LOWPAN_LLTYPE_IEEE802154,
>   };
>   
> +enum lowpan_iphc_ctx_states {
> +	LOWPAN_IPHC_CTX_STATE_DISABLED,
> +	LOWPAN_IPHC_CTX_STATE_ENABLED,
> +
> +	__LOWPAN_IPHC_CTX_STATE_INVALID,
> +	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
> +};
> +

You are expecting more states here besides enabled and disabled? Because 
normally a bool would be fine here and an enum overkill. If you have 
more states pending it makes sense to have the enum though.

> +struct lowpan_iphc_ctx {
> +	enum lowpan_iphc_ctx_states state;
> +	u8 id;
> +	struct in6_addr addr;
> +	u8 prefix_len;
> +};
> +
> +struct lowpan_iphc_ctx_ops {
> +	bool (*valid_prefix)(const struct lowpan_iphc_ctx *ctx);
> +	int (*update)(struct lowpan_iphc_ctx *table,
> +		      const struct lowpan_iphc_ctx *ctx);
> +	struct lowpan_iphc_ctx *(*get_by_id)(const struct net_device *dev,
> +					     struct lowpan_iphc_ctx *table,
> +					     u8 id);
> +	struct lowpan_iphc_ctx *(*get_by_addr)(const struct net_device *dev,
> +					       struct lowpan_iphc_ctx *table,
> +					       const struct in6_addr *addr);
> +};
> +
> +struct lowpan_iphc_ctx_table {
> +	spinlock_t lock;
> +	const struct lowpan_iphc_ctx_ops *ops;
> +	struct lowpan_iphc_ctx table[LOWPAN_IPHC_CI_TABLE_SIZE];
> +};
> +
>   struct lowpan_priv {
>   	enum lowpan_lltypes lltype;
>   	struct dentry *iface_debugfs;
> +	struct lowpan_iphc_ctx_table iphc_dci;
> +	struct lowpan_iphc_ctx_table iphc_sci;
> +	struct lowpan_iphc_ctx_table iphc_mcast_dci;
>   
>   	/* must be last */
>   	u8 priv[0] __aligned(sizeof(void *));
> @@ -125,6 +163,30 @@ struct lowpan_802154_cb *lowpan_802154_cb(const struct sk_buff *skb)
>   	return (struct lowpan_802154_cb *)skb->cb;
>   }
>   
> +static inline int lowpan_ctx_update(struct lowpan_iphc_ctx_table *t,
> +				    const struct lowpan_iphc_ctx *ctx)
> +{
> +	if (!t->ops->valid_prefix(ctx))
> +		return -EINVAL;
> +
> +	return t->ops->update(t->table, ctx);
> +}
> +
> +static inline struct lowpan_iphc_ctx *
> +lowpan_ctx_by_id(const struct net_device *dev, struct lowpan_iphc_ctx_table *t,
> +		 u8 id)
> +{
> +	return t->ops->get_by_id(dev, t->table, id);
> +}
> +
> +static inline struct lowpan_iphc_ctx *
> +lowpan_ctx_by_addr(const struct net_device *dev,
> +		   struct lowpan_iphc_ctx_table *t,
> +		   const struct in6_addr *addr)
> +{
> +	return t->ops->get_by_addr(dev, t->table, addr);
> +}
> +
>   #ifdef DEBUG
>   /* print data in line */
>   static inline void raw_dump_inline(const char *caller, char *msg,
> diff --git a/net/6lowpan/6lowpan_i.h b/net/6lowpan/6lowpan_i.h
> index 33b2605..1312340 100644
> --- a/net/6lowpan/6lowpan_i.h
> +++ b/net/6lowpan/6lowpan_i.h
> @@ -3,6 +3,9 @@
>   
>   #include <linux/netdevice.h>
>   
> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
> +
>   #ifdef CONFIG_6LOWPAN_DEBUGFS
>   int lowpan_dev_debugfs_init(struct net_device *dev);
>   void lowpan_dev_debugfs_uninit(struct net_device *dev);
> diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
> index 01c901c..7ecedd7 100644
> --- a/net/6lowpan/Kconfig
> +++ b/net/6lowpan/Kconfig
> @@ -8,6 +8,7 @@ menuconfig 6LOWPAN
>   config 6LOWPAN_DEBUGFS
>   	bool "6LoWPAN debugfs support"
>   	depends on 6LOWPAN
> +	depends on DEBUG_FS

This looks like a fix that should be merged into the first patch, no?
>   	---help---
>   	  This enables 6LoWPAN debugfs support. For example to manipulate
>   	  IPHC context information at runtime.
> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> index fe58509..d354c5b 100644
> --- a/net/6lowpan/core.c
> +++ b/net/6lowpan/core.c
> @@ -19,6 +19,8 @@
>   
>   int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>   {
> +	int i;
> +
>   	dev->addr_len = EUI64_ADDR_LEN;
>   	dev->type = ARPHRD_6LOWPAN;
>   	dev->mtu = IPV6_MIN_MTU;
> @@ -26,6 +28,21 @@ int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>   
>   	lowpan_priv(dev)->lltype = lltype;
>   
> +	spin_lock_init(&lowpan_priv(dev)->iphc_dci.lock);
> +	lowpan_priv(dev)->iphc_dci.ops = &iphc_ctx_unicast_ops;
> +
> +	spin_lock_init(&lowpan_priv(dev)->iphc_sci.lock);
> +	lowpan_priv(dev)->iphc_sci.ops = &iphc_ctx_unicast_ops;
> +
> +	spin_lock_init(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +	lowpan_priv(dev)->iphc_mcast_dci.ops = &iphc_ctx_mcast_ops;
> +
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		lowpan_priv(dev)->iphc_dci.table[i].id = i;
> +		lowpan_priv(dev)->iphc_sci.table[i].id = i;
> +		lowpan_priv(dev)->iphc_mcast_dci.table[i].id = i;
> +	}
> +
>   	return lowpan_dev_debugfs_init(dev);
>   }
>   EXPORT_SYMBOL(lowpan_netdev_setup);
> diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
> index b0a26fd..d6481eb 100644
> --- a/net/6lowpan/debugfs.c
> +++ b/net/6lowpan/debugfs.c
> @@ -18,15 +18,124 @@
>   
>   static struct dentry *lowpan_debugfs;
>   
> +static int lowpan_context_show(struct seq_file *file, void *offset)
> +{
> +	struct lowpan_iphc_ctx_table *t = file->private;
> +	int i;
> +
> +	seq_printf(file, "%-2s %-43s %s\n", "ID", "ipv6-address/prefix-length",
> +		   "state");
> +
> +	spin_lock_bh(&t->lock);
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		seq_printf(file,
> +			   "%-2d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%03d %d\n",
> +			   t->table[i].id,
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[0]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[1]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[2]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[3]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[4]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[5]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[6]),
> +			   be16_to_cpu(t->table[i].addr.s6_addr16[7]),
> +			   t->table[i].prefix_len, t->table[i].state);
> +	}
> +	spin_unlock_bh(&t->lock);
> +
> +	return 0;
> +}
> +
> +static int lowpan_context_dbgfs_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, lowpan_context_show, inode->i_private);
> +}
> +
> +static ssize_t lowpan_context_dbgfs_write(struct file *fp,
> +					  const char __user *user_buf,
> +					  size_t count, loff_t *ppos)
> +{
> +	char buf[128] = { };
> +	struct seq_file *file = fp->private_data;
> +	struct lowpan_iphc_ctx_table *t = file->private;
> +	struct lowpan_iphc_ctx ctx;
> +	int status = count, n, id, state, i, prefix_len, ret;
> +	unsigned int addr[8];
> +
> +	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
> +						 count))) {
> +		status = -EFAULT;
> +		goto out;
> +	}
> +
> +	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
> +		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
> +		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
> +	if (n != 11) {

11 more like a magic number here. Not to bad, but a define could be nice.
> +		status = -EIO;
> +		goto out;
> +	}
> +
> +	if (id > LOWPAN_IPHC_CI_TABLE_SIZE - 1 ||
> +	    state > LOWPAN_IPHC_CTX_STATE_MAX || prefix_len > 128) {
> +		status = -EINVAL;
> +		goto out;
> +	}
> +
> +	ctx.id = id;
> +	ctx.state = state;
> +	ctx.prefix_len = prefix_len;
> +
> +	for (i = 0; i < 8; i++)
> +		ctx.addr.s6_addr16[i] = cpu_to_be16(addr[i] & 0xffff);
> +
> +	spin_lock_bh(&t->lock);
> +	ret = lowpan_ctx_update(t, &ctx);
> +	if (ret < 0)
> +		status = ret;
> +	spin_unlock_bh(&t->lock);
> +
> +out:
> +	return status;
> +}
> +
> +const struct file_operations lowpan_context_fops = {
> +	.open		= lowpan_context_dbgfs_open,
> +	.read		= seq_read,
> +	.write		= lowpan_context_dbgfs_write,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
>   int lowpan_dev_debugfs_init(struct net_device *dev)
>   {
>   	struct lowpan_priv *lpriv = lowpan_priv(dev);
> +	static struct dentry *dentry;
>   
>   	/* creating the root */
>   	lpriv->iface_debugfs = debugfs_create_dir(dev->name, lowpan_debugfs);
>   	if (!lpriv->iface_debugfs)
>   		goto fail;
>   
> +	dentry = debugfs_create_file("dci_table", 0664, lpriv->iface_debugfs,
> +				     &lowpan_priv(dev)->iphc_dci,
> +				     &lowpan_context_fops);
> +	if (!dentry)
> +		goto fail;
> +
> +	dentry = debugfs_create_file("sci_table", 0664, lpriv->iface_debugfs,
> +				     &lowpan_priv(dev)->iphc_sci,
> +				     &lowpan_context_fops);
> +	if (!dentry)
> +		goto fail;
> +
> +	dentry = debugfs_create_file("mcast_dci_table", 0664,
> +				     lpriv->iface_debugfs,
> +				     &lowpan_priv(dev)->iphc_mcast_dci,
> +				     &lowpan_context_fops);
> +	if (!dentry)
> +		goto fail;
> +
>   	return 0;
>   
>   fail:
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 346b5c1..5974ea5 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -56,6 +56,7 @@
>   /* special link-layer handling */
>   #include <net/mac802154.h>
>   
> +#include "6lowpan_i.h"
>   #include "nhc.h"
>   
>   /* Values of fields within the IPHC encoding first byte */
> @@ -147,6 +148,9 @@
>   	 (((a)->s6_addr16[6]) == 0) &&		\
>   	 (((a)->s6_addr[14]) == 0))
>   
> +#define LOWPAN_IPHC_CID_DCI(cid)	(cid & 0x0f)
> +#define LOWPAN_IPHC_CID_SCI(cid)	((cid & 0xf0) >> 4)
> +
>   static inline void iphc_uncompress_eui64_lladdr(struct in6_addr *ipaddr,
>   						const void *lladdr)
>   {
> @@ -259,30 +263,59 @@ static int uncompress_addr(struct sk_buff *skb, const struct net_device *dev,
>   /* Uncompress address function for source context
>    * based address(non-multicast).
>    */
> -static int uncompress_context_based_src_addr(struct sk_buff *skb,
> -					     struct in6_addr *ipaddr,
> -					     u8 address_mode)
> +static int uncompress_ctx_addr(struct sk_buff *skb,
> +			       const struct net_device *dev,
> +			       const struct lowpan_iphc_ctx *ctx,
> +			       struct in6_addr *ipaddr, u8 address_mode,
> +			       const void *lladdr)
>   {
> +	bool fail;
> +
>   	switch (address_mode) {
> -	case LOWPAN_IPHC_SAM_00:
> -		/* unspec address ::
> +	/* SAM and DAM are the same here */
> +	case LOWPAN_IPHC_DAM_00:
> +		fail = false;
> +		/* SAM_00 -> unspec address ::
>   		 * Do nothing, address is already ::
> +		 *
> +		 * DAM 00 -> reserved should never occur.
>   		 */
>   		break;
>   	case LOWPAN_IPHC_SAM_01:
> -		/* TODO */
> +	case LOWPAN_IPHC_DAM_01:
> +		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[8], 8);
> +		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
> +		break;
>   	case LOWPAN_IPHC_SAM_10:
> -		/* TODO */
> +	case LOWPAN_IPHC_DAM_10:
> +		ipaddr->s6_addr[11] = 0xFF;
> +		ipaddr->s6_addr[12] = 0xFE;
> +		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[14], 2);
> +		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
> +		break;
>   	case LOWPAN_IPHC_SAM_11:
> -		/* TODO */
> -		netdev_warn(skb->dev, "SAM value 0x%x not supported\n",
> -			    address_mode);
> -		return -EINVAL;
> +	case LOWPAN_IPHC_DAM_11:
> +		fail = false;
> +		switch (lowpan_priv(dev)->lltype) {
> +		case LOWPAN_LLTYPE_IEEE802154:
> +			iphc_uncompress_802154_lladdr(ipaddr, lladdr);
> +			break;
> +		default:
> +			iphc_uncompress_eui64_lladdr(ipaddr, lladdr);
> +			break;
> +		}
> +		ipv6_addr_prefix_cpy(ipaddr, &ctx->addr, ctx->prefix_len);
> +		break;
>   	default:
>   		pr_debug("Invalid sam value: 0x%x\n", address_mode);
>   		return -EINVAL;
>   	}
>   
> +	if (fail) {
> +		pr_debug("Failed to fetch skb data\n");
> +		return -EIO;
> +	}
> +
>   	raw_dump_inline(NULL,
>   			"Reconstructed context based ipv6 src addr is",
>   			ipaddr->s6_addr, 16);
> @@ -346,6 +379,33 @@ static int lowpan_uncompress_multicast_daddr(struct sk_buff *skb,
>   	return 0;
>   }
>   
> +static int lowpan_uncompress_multicast_ctx_daddr(struct sk_buff *skb,
> +						 struct lowpan_iphc_ctx *ctx,
> +						 struct in6_addr *ipaddr,
> +						 u8 address_mode)
> +{
> +	struct in6_addr network_pfx = {};
> +	bool fail;
> +
> +	if (address_mode != LOWPAN_IPHC_DAM_00)
> +		return -EINVAL;
> +
> +	ipaddr->s6_addr[0] = 0xFF;
> +	fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[1], 2);
> +	fail |= lowpan_fetch_skb(skb, &ipaddr->s6_addr[12], 4);
> +	/* take prefix_len and network prefix from the context */
> +	ipaddr->s6_addr[3] = ctx->prefix_len;
> +	/* get network prefix to copy into multicast address */
> +	ipv6_addr_prefix(&network_pfx, &ctx->addr, ctx->prefix_len);
> +	/* setting network prefix */
> +	memcpy(&ipaddr->s6_addr[4], &network_pfx, 8);
> +
> +	if (fail < 0)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
>   /* get the ecn values from iphc tf format and set it to ipv6hdr */
>   static inline void lowpan_iphc_tf_set_ecn(struct ipv6hdr *hdr, const u8 *tf)
>   {
> @@ -459,7 +519,8 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   			     const void *daddr, const void *saddr)
>   {
>   	struct ipv6hdr hdr = {};
> -	u8 iphc0, iphc1;
> +	struct lowpan_iphc_ctx *ci;
> +	u8 iphc0, iphc1, cid = 0;
>   	int err;
>   
>   	raw_dump_table(__func__, "raw skb data dump uncompressed",
> @@ -469,12 +530,14 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)))
>   		return -EINVAL;
>   
> -	/* another if the CID flag is set */
> -	if (iphc1 & LOWPAN_IPHC_CID)
> -		return -ENOTSUPP;
> -
>   	hdr.version = 6;
>   
> +	/* default CID = 0, another if the CID flag is set */
> +	if (iphc1 & LOWPAN_IPHC_CID) {
> +		if (lowpan_fetch_skb(skb, &cid, sizeof(cid)))
> +			return -EINVAL;
> +	}
> +
>   	err = lowpan_iphc_tf_decompress(skb, &hdr,
>   					iphc0 & LOWPAN_IPHC_TF_MASK);
>   	if (err < 0)
> @@ -500,10 +563,18 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   	}
>   
>   	if (iphc1 & LOWPAN_IPHC_SAC) {
> -		/* Source address context based uncompression */
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_sci,
> +				      LOWPAN_IPHC_CID_SCI(cid));
> +		if (!ci) {
> +			spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +			return -EINVAL;
> +		}
> +
>   		pr_debug("SAC bit is set. Handle context based source address.\n");
> -		err = uncompress_context_based_src_addr(skb, &hdr.saddr,
> -							iphc1 & LOWPAN_IPHC_SAM_MASK);
> +		err = uncompress_ctx_addr(skb, dev, ci, &hdr.saddr,
> +					  iphc1 & LOWPAN_IPHC_SAM_MASK, saddr);
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
>   	} else {
>   		/* Source address uncompression */
>   		pr_debug("source address stateless compression\n");
> @@ -515,27 +586,54 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   	if (err)
>   		return -EINVAL;
>   
> -	/* check for Multicast Compression */
> -	if (iphc1 & LOWPAN_IPHC_M) {
> -		if (iphc1 & LOWPAN_IPHC_DAC) {
> -			pr_debug("dest: context-based mcast compression\n");
> -			/* TODO: implement this */
> -		} else {
> -			err = lowpan_uncompress_multicast_daddr(skb, &hdr.daddr,
> -								iphc1 & LOWPAN_IPHC_DAM_MASK);
> +	switch (iphc1 & (LOWPAN_IPHC_M | LOWPAN_IPHC_DAC)) {
> +	case LOWPAN_IPHC_M | LOWPAN_IPHC_DAC:
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_mcast_dci,
> +				      LOWPAN_IPHC_CID_DCI(cid));
> +		if (!ci) {
> +			spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +			return -EINVAL;
> +		}
>   
> -			if (err)
> -				return -EINVAL;
> +		/* multicast with context */
> +		pr_debug("dest: context-based mcast compression\n");
> +		err = lowpan_uncompress_multicast_ctx_daddr(skb, ci,
> +							    &hdr.daddr,
> +							    iphc1 & LOWPAN_IPHC_DAM_MASK);
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +		break;
> +	case LOWPAN_IPHC_M:
> +		/* multicast */
> +		err = lowpan_uncompress_multicast_daddr(skb, &hdr.daddr,
> +							iphc1 & LOWPAN_IPHC_DAM_MASK);
> +		break;
> +	case LOWPAN_IPHC_DAC:
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +		ci = lowpan_ctx_by_id(dev, &lowpan_priv(dev)->iphc_dci,
> +				      LOWPAN_IPHC_CID_DCI(cid));
> +		if (!ci) {
> +			spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +			return -EINVAL;
>   		}
> -	} else {
> +
> +		/* Destination address context based uncompression */
> +		pr_debug("DAC bit is set. Handle context based destination address.\n");
> +		err = uncompress_ctx_addr(skb, dev, ci, &hdr.daddr,
> +					  iphc1 & LOWPAN_IPHC_DAM_MASK, daddr);
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +		break;
> +	default:
>   		err = uncompress_addr(skb, dev, &hdr.daddr,
>   				      iphc1 & LOWPAN_IPHC_DAM_MASK, daddr);
>   		pr_debug("dest: stateless compression mode %d dest %pI6c\n",
>   			 iphc1 & LOWPAN_IPHC_DAM_MASK, &hdr.daddr);
> -		if (err)
> -			return -EINVAL;
> +		break;
>   	}
>   
> +	if (err)
> +		return -EINVAL;
> +
>   	/* Next header data uncompression */
>   	if (iphc0 & LOWPAN_IPHC_NH) {
>   		err = lowpan_nhc_do_uncompression(skb, dev, &hdr);
> @@ -585,6 +683,60 @@ static const u8 lowpan_iphc_dam_to_sam_value[] = {
>   	[LOWPAN_IPHC_DAM_11] = LOWPAN_IPHC_SAM_11,
>   };
>   
> +static u8 lowpan_compress_ctx_addr(u8 **hc_ptr, const struct in6_addr *ipaddr,
> +				   const struct lowpan_iphc_ctx *ctx,
> +				   const unsigned char *lladdr, bool sam)
> +{
> +	struct in6_addr tmp = {};
> +	u8 dam;
> +
> +	/* check for SAM/DAM = 11 */
> +	memcpy(&tmp.s6_addr[8], lladdr, 8);
> +	/* second bit-flip (Universe/Local)
> +	 * is done according RFC2464
> +	 */
> +	tmp.s6_addr[8] ^= 0x02;
> +	/* context information are always used */
> +	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
> +	if (ipv6_addr_equal(&tmp, ipaddr)) {
> +		dam = LOWPAN_IPHC_DAM_11;
> +		goto out;
> +	}
> +
> +	memset(&tmp, 0, sizeof(tmp));
> +	/* check for SAM/DAM = 01 */
> +	tmp.s6_addr[11] = 0xFF;
> +	tmp.s6_addr[12] = 0xFE;
> +	memcpy(&tmp.s6_addr[14], &ipaddr->s6_addr[14], 2);
> +	/* context information are always used */
> +	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
> +	if (ipv6_addr_equal(&tmp, ipaddr)) {
> +		lowpan_push_hc_data(hc_ptr, &ipaddr->s6_addr[14], 2);
> +		dam = LOWPAN_IPHC_DAM_10;
> +		goto out;
> +	}
> +
> +	memset(&tmp, 0, sizeof(tmp));
> +	/* check for SAM/DAM = 10, should always match */
> +	memcpy(&tmp.s6_addr[8], &ipaddr->s6_addr[8], 8);
> +	/* context information are always used */
> +	ipv6_addr_prefix_cpy(&tmp, &ctx->addr, ctx->prefix_len);
> +	if (ipv6_addr_equal(&tmp, ipaddr)) {
> +		lowpan_push_hc_data(hc_ptr, &ipaddr->s6_addr[8], 8);
> +		dam = LOWPAN_IPHC_DAM_01;
> +		goto out;
> +	}
> +
> +	WARN_ON_ONCE("context found but no address mode matched\n");
> +	return -EINVAL;
> +out:
> +
> +	if (sam)
> +		return lowpan_iphc_dam_to_sam_value[dam];
> +	else
> +		return dam;
> +}
> +
>   static u8 lowpan_compress_addr_64(u8 **hc_ptr, const struct in6_addr *ipaddr,
>   				  const unsigned char *lladdr, bool sam)
>   {
> @@ -708,6 +860,21 @@ static u8 lowpan_iphc_tf_compress(u8 **hc_ptr, const struct ipv6hdr *hdr)
>   	return val;
>   }
>   
> +static u8 lowpan_iphc_mcast_ctx_addr_compress(u8 **hc_ptr,
> +					      const struct lowpan_iphc_ctx *ctx,
> +					      const struct in6_addr *ipaddr)
> +{
> +	u8 data[6];
> +
> +	/* flags/scope, reserved (RIID) */
> +	memcpy(data, &ipaddr->s6_addr[1], 2);
> +	/* group ID */
> +	memcpy(&data[1], &ipaddr->s6_addr[11], 4);
> +	lowpan_push_hc_data(hc_ptr, data, 6);
> +
> +	return LOWPAN_IPHC_DAM_00;
> +}
> +
>   static u8 lowpan_iphc_mcast_addr_compress(u8 **hc_ptr,
>   					  const struct in6_addr *ipaddr)
>   {
> @@ -742,10 +909,11 @@ static u8 lowpan_iphc_mcast_addr_compress(u8 **hc_ptr,
>   int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   			   const void *daddr, const void *saddr)
>   {
> -	u8 iphc0, iphc1, *hc_ptr;
> +	u8 iphc0, iphc1, *hc_ptr, cid = 0;
>   	struct ipv6hdr *hdr;
>   	u8 head[LOWPAN_IPHC_MAX_HC_BUF_LEN] = {};
> -	int ret, addr_type;
> +	struct lowpan_iphc_ctx *dci, *sci, dci_entry, sci_entry;
> +	int ret, ipv6_daddr_type, ipv6_saddr_type;
>   
>   	if (skb->protocol != htons(ETH_P_IPV6))
>   		return -EINVAL;
> @@ -769,14 +937,47 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   	iphc0 = LOWPAN_DISPATCH_IPHC;
>   	iphc1 = 0;
>   
> -	/* TODO: context lookup */
> -
>   	raw_dump_inline(__func__, "saddr", saddr, EUI64_ADDR_LEN);
>   	raw_dump_inline(__func__, "daddr", daddr, EUI64_ADDR_LEN);
>   
>   	raw_dump_table(__func__, "sending raw skb network uncompressed packet",
>   		       skb->data, skb->len);
>   
> +	ipv6_daddr_type = ipv6_addr_type(&hdr->daddr);
> +	if (ipv6_daddr_type & IPV6_ADDR_MULTICAST) {
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +		dci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_mcast_dci,
> +					 &hdr->daddr);
> +		if (dci) {
> +			memcpy(&dci_entry, dci, sizeof(*dci));
> +			cid |= dci->id;
> +		}
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_mcast_dci.lock);
> +	} else {
> +		spin_lock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +		dci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_dci,
> +					 &hdr->daddr);
> +		if (dci) {
> +			memcpy(&dci_entry, dci, sizeof(*dci));
> +			cid |= dci->id;
> +		}
> +		spin_unlock_bh(&lowpan_priv(dev)->iphc_dci.lock);
> +	}
> +
> +	spin_lock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +	sci = lowpan_ctx_by_addr(dev, &lowpan_priv(dev)->iphc_sci,
> +				 &hdr->saddr);
> +	if (sci) {
> +		memcpy(&sci_entry, sci, sizeof(*sci));
> +		cid |= (sci->id << 4);
> +	}
> +	spin_unlock_bh(&lowpan_priv(dev)->iphc_sci.lock);
> +
> +	if (sci || dci) {
> +		iphc1 |= LOWPAN_IPHC_CID;
> +		lowpan_push_hc_data(&hc_ptr, &cid, sizeof(cid));
> +	}
> +
>   	/* Traffic Class, Flow Label compression */
>   	iphc0 |= lowpan_iphc_tf_compress(&hc_ptr, hdr);
>   
> @@ -813,39 +1014,63 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   				    sizeof(hdr->hop_limit));
>   	}
>   
> -	addr_type = ipv6_addr_type(&hdr->saddr);
> +	ipv6_saddr_type = ipv6_addr_type(&hdr->saddr);
>   	/* source address compression */
> -	if (addr_type == IPV6_ADDR_ANY) {
> +	if (ipv6_saddr_type == IPV6_ADDR_ANY) {
>   		pr_debug("source address is unspecified, setting SAC\n");
>   		iphc1 |= LOWPAN_IPHC_SAC;
>   	} else {
> -		if (addr_type & IPV6_ADDR_LINKLOCAL) {
> -			iphc1 |= lowpan_compress_addr_64(&hc_ptr, &hdr->saddr,
> -							 saddr, true);
> -			pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
> -				 &hdr->saddr, iphc1);
> +		if (sci) {
> +			iphc1 |= lowpan_compress_ctx_addr(&hc_ptr, &hdr->saddr,
> +							  &sci_entry, saddr,
> +							  true);
> +			iphc1 |= LOWPAN_IPHC_SAC;
>   		} else {
> -			pr_debug("send the full source address\n");
> -			lowpan_push_hc_data(&hc_ptr, hdr->saddr.s6_addr, 16);
> +			if (ipv6_saddr_type & IPV6_ADDR_LINKLOCAL) {
> +				iphc1 |= lowpan_compress_addr_64(&hc_ptr,
> +								 &hdr->saddr,
> +								 saddr, true);
> +				pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
> +					 &hdr->saddr, iphc1);
> +			} else {
> +				pr_debug("send the full source address\n");
> +				lowpan_push_hc_data(&hc_ptr,
> +						    hdr->saddr.s6_addr, 16);
> +			}
>   		}
>   	}
>   
> -	addr_type = ipv6_addr_type(&hdr->daddr);
>   	/* destination address compression */
> -	if (addr_type & IPV6_ADDR_MULTICAST) {
> +	if (ipv6_daddr_type & IPV6_ADDR_MULTICAST) {
>   		pr_debug("destination address is multicast: ");
> -		iphc1 |= LOWPAN_IPHC_M;
> -		iphc1 |= lowpan_iphc_mcast_addr_compress(&hc_ptr, &hdr->daddr);
> +		if (dci) {
> +			iphc1 |= lowpan_iphc_mcast_ctx_addr_compress(&hc_ptr,
> +								     &dci_entry,
> +								     &hdr->daddr);
> +		} else {
> +			iphc1 |= LOWPAN_IPHC_M;
> +			iphc1 |= lowpan_iphc_mcast_addr_compress(&hc_ptr,
> +								 &hdr->daddr);
> +		}
>   	} else {
> -		if (addr_type & IPV6_ADDR_LINKLOCAL) {
> -			/* TODO: context lookup */
> -			iphc1 |= lowpan_compress_addr_64(&hc_ptr, &hdr->daddr,
> -							 daddr, false);
> -			pr_debug("dest address unicast link-local %pI6c "
> -				 "iphc1 0x%02x\n", &hdr->daddr, iphc1);
> +		if (dci) {
> +			iphc1 |= lowpan_compress_ctx_addr(&hc_ptr, &hdr->daddr,
> +							  &dci_entry, daddr,
> +							  false);
> +			iphc1 |= LOWPAN_IPHC_DAC;
>   		} else {
> -			pr_debug("dest address unicast %pI6c\n", &hdr->daddr);
> -			lowpan_push_hc_data(&hc_ptr, hdr->daddr.s6_addr, 16);
> +			if (ipv6_daddr_type & IPV6_ADDR_LINKLOCAL) {
> +				iphc1 |= lowpan_compress_addr_64(&hc_ptr,
> +								 &hdr->daddr,
> +								 daddr, false);
> +				pr_debug("dest address unicast link-local %pI6c iphc1 0x%02x\n",
> +					 &hdr->daddr, iphc1);
> +			} else {
> +				pr_debug("dest address unicast %pI6c\n",
> +					 &hdr->daddr);
> +				lowpan_push_hc_data(&hc_ptr,
> +						    hdr->daddr.s6_addr, 16);
> +			}
>   		}
>   	}
>   
> @@ -871,3 +1096,164 @@ int lowpan_header_compress(struct sk_buff *skb, const struct net_device *dev,
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(lowpan_header_compress);
> +
> +static bool lowpan_iphc_ctx_valid_prefix(const struct lowpan_iphc_ctx *ctx)
> +{
> +	/* prefix which are smaller than 64 bits are not valid, users
> +	 * may mean than a prefix which is filled with zero until 64th bit.
> +	 * Refere rfc6282 "Any remaining bits are zero." The remaining bits
> +	 * in this case where are the prefix(< 64) ends until IID starts.
> +	 */
> +	if (ctx->prefix_len < 64)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool
> +lowpan_iphc_mcast_ctx_valid_prefix(const struct lowpan_iphc_ctx *ctx)
> +{
> +	/* network prefix for multicast is at maximum 64 bits long */
> +	if (ctx->prefix_len > 64)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int lowpan_iphc_ctx_update(struct lowpan_iphc_ctx *table,
> +				  const struct lowpan_iphc_ctx *ctx)
> +{
> +	int ret = 0, i;
> +
> +	switch (ctx->state) {
> +	case LOWPAN_IPHC_CTX_STATE_DISABLED:
> +		/* we don't care about if disabled */
> +		break;
> +	case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +		for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +			if (ctx->prefix_len != table[i].prefix_len)
> +				continue;
> +
> +			if (ipv6_prefix_equal(&ctx->addr, &table[i].addr,
> +					      ctx->prefix_len)) {
> +				switch (table[i].state) {
> +				case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +					ret = -EEXIST;
> +					goto out;
> +				default:
> +					break;
> +				}
> +			}
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	memcpy(&table[ctx->id], ctx, sizeof(*ctx));
> +
> +out:
> +	return ret;
> +}
> +
> +static struct lowpan_iphc_ctx *
> +lowpan_iphc_ctx_get_by_id(const struct net_device *dev,
> +			  struct lowpan_iphc_ctx *table, u8 id)
> +{
> +	struct lowpan_iphc_ctx *ret = NULL;
> +
> +	WARN_ON_ONCE(id > LOWPAN_IPHC_CI_TABLE_SIZE);
> +
> +	switch (table[id].state) {
> +	case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +		ret = &table[id];
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static struct lowpan_iphc_ctx *
> +lowpan_iphc_ctx_get_by_addr(const struct net_device *dev,
> +			    struct lowpan_iphc_ctx *table,
> +			    const struct in6_addr *addr)
> +{
> +	struct lowpan_iphc_ctx *ret = NULL;
> +	struct in6_addr addr_prefix;
> +	int i;
> +
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		ipv6_addr_prefix(&addr_prefix, addr, table[i].prefix_len);
> +
> +		if (ipv6_prefix_equal(&addr_prefix, &table[i].addr,
> +				      table[i].prefix_len)) {
> +			switch (table[i].state) {
> +			case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +				/* remember first match */
> +				if (!ret) {
> +					ret = &table[i];
> +					continue;
> +				}
> +
> +				/* get the context with longest prefix_len */
> +				if (table[i].prefix_len > ret->prefix_len)
> +					ret = &table[i];
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static struct lowpan_iphc_ctx *
> +lowpan_iphc_ctx_get_by_mcast_addr(const struct net_device *dev,
> +				  struct lowpan_iphc_ctx *table,
> +				  const struct in6_addr *addr)
> +{
> +	struct lowpan_iphc_ctx *ret = NULL;
> +	struct in6_addr addr_mcast, network_pfx = {};
> +	int i;
> +
> +	/* init mcast address with  */
> +	memcpy(&addr_mcast, addr, sizeof(*addr));
> +
> +	for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++) {
> +		/* setting plen */
> +		addr_mcast.s6_addr[3] = table[i].prefix_len;
> +		/*  get network prefix to copy into multicast address */
> +		ipv6_addr_prefix(&network_pfx, &table[i].addr,
> +				 table[i].prefix_len);
> +		/* setting network prefix */
> +		memcpy(&addr_mcast.s6_addr[4], &network_pfx, 8);
> +
> +		if (ipv6_addr_equal(addr, &addr_mcast)) {
> +			switch (table[i].state) {
> +			case LOWPAN_IPHC_CTX_STATE_ENABLED:
> +				return &table[i];
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops = {
> +	.valid_prefix = lowpan_iphc_ctx_valid_prefix,
> +	.update = lowpan_iphc_ctx_update,
> +	.get_by_id = lowpan_iphc_ctx_get_by_id,
> +	.get_by_addr = lowpan_iphc_ctx_get_by_addr,
> +};
> +
> +const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
> +	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
> +	.update = lowpan_iphc_ctx_update,
> +	.get_by_id = lowpan_iphc_ctx_get_by_id,
> +	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
> +};

These are the comments from a first look over it. I will have a more 
detailed review and actual testing once you are happy enough with it to 
move it from RFC to PATCH.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support
  2015-11-25 16:42   ` Stefan Schmidt
@ 2015-11-25 18:00     ` Alexander Aring
  2015-11-26 11:07       ` Stefan Schmidt
  0 siblings, 1 reply; 23+ messages in thread
From: Alexander Aring @ 2015-11-25 18:00 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: linux-wpan, linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit

Hi,

On Wed, Nov 25, 2015 at 05:42:32PM +0100, Stefan Schmidt wrote:
> Hello.
> 
> On 17/11/15 23:33, Alexander Aring wrote:
> >This patch will introduce a 6lowpan entry into the debugfs if enabled.
> >Inside this 6lowpan directory we create a subdirectories of all 6lowpan
> >interfaces to offer a per interface debugfs support.
> 

...

> >  static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
> >@@ -180,6 +187,7 @@ static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
> >  	ASSERT_RTNL();
> >  	wdev->ieee802154_ptr->lowpan_dev = NULL;
> >+	lowpan_netdev_unsetup(ldev);
> >  	unregister_netdevice(ldev);
> >  	dev_put(wdev);
> >  }
> 
> If you are going to change the th uninit to exit and the unsetup to teardown
> you can add my review.
> 
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> 

I will add a patch which introduce the two functions:

lowpan_register_netdevice - register_netdevice (without rtnl lock)
lowpan_register_netdev - register_netdev (with rtnl lock)

These two functions will call register_netdevice/register_netdev and 6lowpan
debugfs init.


I will remove the lowpan_netdev_unsetup function and introduce a:

lowpan_unregister_netdevice - unregister_netdevice

which calls unregister_netdevice and lowpan_dev_debugfs_exit. I will
change the uninit to exit.

Then we keep the same naming stuff like netdev and these function should
always called when doing register_netdev/unregister_netdev for all
lowpan interface.

Then we can also move lowpan_netdev_setup before register_netdev. This
will move more functionality which should be the same on all lowpan
interfaces into the net/6lowpan branch.

- Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
  2015-11-25 17:12       ` Stefan Schmidt
@ 2015-11-25 18:07           ` Alexander Aring
  -1 siblings, 0 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-25 18:07 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: linux-wpan-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc

Hi,

On Wed, Nov 25, 2015 at 06:12:25PM +0100, Stefan Schmidt wrote:
> Helllo.
> 
> On 17/11/15 23:33, Alexander Aring wrote:
> >This patch introduce support for IPHC stateful address compression. It
> >will offer three debugfs per interface entries, which are:
> >
> >  - dci_table: destination context indentifier table
> >  - sci_table: source context indentifier table
> >  - mcast_sci_table: multicast context identifier table
> >
> >Example to setup a context id:
> >
> >A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
> >contexts which are available. Example:
> >
> >ID ipv6-address/prefix-length                  state
> >0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >15 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >
> >For setting a context e.g. context id 0, context 2001::, prefix-length
> >64.
> >
> >Hint: Simple copy one line and then maniuplate it.
> >
> >echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
> >/sys/kernel/debug/6lowpan/lowpan0/dci_table
> >
> >The state will display currently if the context is disabled(0) or
> >enabled(1).
> >
> >On transmit side:
> >
> >The IPHC code will automatically search for a context which would be
> >match for the address. Then it will be use the context which the
> >best compression, means the longest prefix which match will be used.
> >
> >Example:
> >
> >2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
> >last bit of the address which has the prefix 2001::/127 is the same like
> >the IID from the Encapsulating Header. A context ID can also be a
> >2001::1/128, which is then a full ipv6 address.
> >
> >On Receive side:
> >
> >If there is a context defined (when CID not available then it's the
> >default context 0) then it will be used, if the header doesn't set
> >SAC or DAC bit thens, it will be dropped.
> >
> >Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >---
> >  include/net/6lowpan.h   |  62 ++++++
> >  net/6lowpan/6lowpan_i.h |   3 +
> >  net/6lowpan/Kconfig     |   1 +
> >  net/6lowpan/core.c      |  17 ++
> >  net/6lowpan/debugfs.c   | 109 +++++++++++
> >  net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
> >  6 files changed, 635 insertions(+), 57 deletions(-)
> >
> >diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> >index e484898..6e8d30e 100644
> >--- a/include/net/6lowpan.h
> >+++ b/include/net/6lowpan.h
> >@@ -75,6 +75,8 @@
> >  #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
> >  					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
> >  					 LOWPAN_NHC_MAX_HDR_LEN)
> >+/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
> >+#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
> >  #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
> >  #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
> >@@ -98,9 +100,45 @@ enum lowpan_lltypes {
> >  	LOWPAN_LLTYPE_IEEE802154,
> >  };
> >+enum lowpan_iphc_ctx_states {
> >+	LOWPAN_IPHC_CTX_STATE_DISABLED,
> >+	LOWPAN_IPHC_CTX_STATE_ENABLED,
> >+
> >+	__LOWPAN_IPHC_CTX_STATE_INVALID,
> >+	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
> >+};
> >+
> 
> You are expecting more states here besides enabled and disabled? Because
> normally a bool would be fine here and an enum overkill. If you have more
> states pending it makes sense to have the enum though.
> 

Yea, I thought about more states than just these ones. E.g. from which
sources came the context which was set, from ICMPv6 option field, manually
added from debugfs/netlink? Such things...

Nevertheless I will change it to bool here, we can still change it if we
want that. But then we need to talk about that again if doing userspace
API for that.

> >+struct lowpan_iphc_ctx {
> >+	enum lowpan_iphc_ctx_states state;
> >+	u8 id;
> >+	struct in6_addr addr;
> >+	u8 prefix_len;
> >+};
> >+
...
> >  #include <linux/netdevice.h>
> >+extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
> >+extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
> >+
> >  #ifdef CONFIG_6LOWPAN_DEBUGFS
> >  int lowpan_dev_debugfs_init(struct net_device *dev);
> >  void lowpan_dev_debugfs_uninit(struct net_device *dev);
> >diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
> >index 01c901c..7ecedd7 100644
> >--- a/net/6lowpan/Kconfig
> >+++ b/net/6lowpan/Kconfig
> >@@ -8,6 +8,7 @@ menuconfig 6LOWPAN
> >  config 6LOWPAN_DEBUGFS
> >  	bool "6LoWPAN debugfs support"
> >  	depends on 6LOWPAN
> >+	depends on DEBUG_FS
> 
> This looks like a fix that should be merged into the first patch, no?

yes. I will fix that.

> >  	---help---
> >  	  This enables 6LoWPAN debugfs support. For example to manipulate
> >  	  IPHC context information at runtime.
> >diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> >index fe58509..d354c5b 100644
> >--- a/net/6lowpan/core.c
> >+++ b/net/6lowpan/core.c
> >@@ -19,6 +19,8 @@
> >  int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
> >  {
> >+	int i;
> >+
> >  	dev->addr_len = EUI64_ADDR_LEN;
> >  	dev->type = ARPHRD_6LOWPAN;
> >  	dev->mtu = IPV6_MIN_MTU;
...
> >+static ssize_t lowpan_context_dbgfs_write(struct file *fp,
> >+					  const char __user *user_buf,
> >+					  size_t count, loff_t *ppos)
> >+{
> >+	char buf[128] = { };
> >+	struct seq_file *file = fp->private_data;
> >+	struct lowpan_iphc_ctx_table *t = file->private;
> >+	struct lowpan_iphc_ctx ctx;
> >+	int status = count, n, id, state, i, prefix_len, ret;
> >+	unsigned int addr[8];
> >+
> >+	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
> >+						 count))) {
> >+		status = -EFAULT;
> >+		goto out;
> >+	}
> >+
> >+	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
> >+		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
> >+		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
> >+	if (n != 11) {
> 
> 11 more like a magic number here. Not to bad, but a define could be nice.

ok.

...
> >+
> >+const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
> >+	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
> >+	.update = lowpan_iphc_ctx_update,
> >+	.get_by_id = lowpan_iphc_ctx_get_by_id,
> >+	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
> >+};
> 
> These are the comments from a first look over it. I will have a more
> detailed review and actual testing once you are happy enough with it to move
> it from RFC to PATCH.
> 

ok.

- Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
@ 2015-11-25 18:07           ` Alexander Aring
  0 siblings, 0 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-25 18:07 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: linux-wpan, linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit

Hi,

On Wed, Nov 25, 2015 at 06:12:25PM +0100, Stefan Schmidt wrote:
> Helllo.
> 
> On 17/11/15 23:33, Alexander Aring wrote:
> >This patch introduce support for IPHC stateful address compression. It
> >will offer three debugfs per interface entries, which are:
> >
> >  - dci_table: destination context indentifier table
> >  - sci_table: source context indentifier table
> >  - mcast_sci_table: multicast context identifier table
> >
> >Example to setup a context id:
> >
> >A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
> >contexts which are available. Example:
> >
> >ID ipv6-address/prefix-length                  state
> >0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >15 0000:0000:0000:0000:0000:0000:0000:0000/000 0
> >
> >For setting a context e.g. context id 0, context 2001::, prefix-length
> >64.
> >
> >Hint: Simple copy one line and then maniuplate it.
> >
> >echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
> >/sys/kernel/debug/6lowpan/lowpan0/dci_table
> >
> >The state will display currently if the context is disabled(0) or
> >enabled(1).
> >
> >On transmit side:
> >
> >The IPHC code will automatically search for a context which would be
> >match for the address. Then it will be use the context which the
> >best compression, means the longest prefix which match will be used.
> >
> >Example:
> >
> >2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
> >last bit of the address which has the prefix 2001::/127 is the same like
> >the IID from the Encapsulating Header. A context ID can also be a
> >2001::1/128, which is then a full ipv6 address.
> >
> >On Receive side:
> >
> >If there is a context defined (when CID not available then it's the
> >default context 0) then it will be used, if the header doesn't set
> >SAC or DAC bit thens, it will be dropped.
> >
> >Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> >---
> >  include/net/6lowpan.h   |  62 ++++++
> >  net/6lowpan/6lowpan_i.h |   3 +
> >  net/6lowpan/Kconfig     |   1 +
> >  net/6lowpan/core.c      |  17 ++
> >  net/6lowpan/debugfs.c   | 109 +++++++++++
> >  net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
> >  6 files changed, 635 insertions(+), 57 deletions(-)
> >
> >diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> >index e484898..6e8d30e 100644
> >--- a/include/net/6lowpan.h
> >+++ b/include/net/6lowpan.h
> >@@ -75,6 +75,8 @@
> >  #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
> >  					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
> >  					 LOWPAN_NHC_MAX_HDR_LEN)
> >+/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
> >+#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
> >  #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
> >  #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
> >@@ -98,9 +100,45 @@ enum lowpan_lltypes {
> >  	LOWPAN_LLTYPE_IEEE802154,
> >  };
> >+enum lowpan_iphc_ctx_states {
> >+	LOWPAN_IPHC_CTX_STATE_DISABLED,
> >+	LOWPAN_IPHC_CTX_STATE_ENABLED,
> >+
> >+	__LOWPAN_IPHC_CTX_STATE_INVALID,
> >+	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
> >+};
> >+
> 
> You are expecting more states here besides enabled and disabled? Because
> normally a bool would be fine here and an enum overkill. If you have more
> states pending it makes sense to have the enum though.
> 

Yea, I thought about more states than just these ones. E.g. from which
sources came the context which was set, from ICMPv6 option field, manually
added from debugfs/netlink? Such things...

Nevertheless I will change it to bool here, we can still change it if we
want that. But then we need to talk about that again if doing userspace
API for that.

> >+struct lowpan_iphc_ctx {
> >+	enum lowpan_iphc_ctx_states state;
> >+	u8 id;
> >+	struct in6_addr addr;
> >+	u8 prefix_len;
> >+};
> >+
...
> >  #include <linux/netdevice.h>
> >+extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
> >+extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
> >+
> >  #ifdef CONFIG_6LOWPAN_DEBUGFS
> >  int lowpan_dev_debugfs_init(struct net_device *dev);
> >  void lowpan_dev_debugfs_uninit(struct net_device *dev);
> >diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
> >index 01c901c..7ecedd7 100644
> >--- a/net/6lowpan/Kconfig
> >+++ b/net/6lowpan/Kconfig
> >@@ -8,6 +8,7 @@ menuconfig 6LOWPAN
> >  config 6LOWPAN_DEBUGFS
> >  	bool "6LoWPAN debugfs support"
> >  	depends on 6LOWPAN
> >+	depends on DEBUG_FS
> 
> This looks like a fix that should be merged into the first patch, no?

yes. I will fix that.

> >  	---help---
> >  	  This enables 6LoWPAN debugfs support. For example to manipulate
> >  	  IPHC context information at runtime.
> >diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> >index fe58509..d354c5b 100644
> >--- a/net/6lowpan/core.c
> >+++ b/net/6lowpan/core.c
> >@@ -19,6 +19,8 @@
> >  int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
> >  {
> >+	int i;
> >+
> >  	dev->addr_len = EUI64_ADDR_LEN;
> >  	dev->type = ARPHRD_6LOWPAN;
> >  	dev->mtu = IPV6_MIN_MTU;
...
> >+static ssize_t lowpan_context_dbgfs_write(struct file *fp,
> >+					  const char __user *user_buf,
> >+					  size_t count, loff_t *ppos)
> >+{
> >+	char buf[128] = { };
> >+	struct seq_file *file = fp->private_data;
> >+	struct lowpan_iphc_ctx_table *t = file->private;
> >+	struct lowpan_iphc_ctx ctx;
> >+	int status = count, n, id, state, i, prefix_len, ret;
> >+	unsigned int addr[8];
> >+
> >+	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
> >+						 count))) {
> >+		status = -EFAULT;
> >+		goto out;
> >+	}
> >+
> >+	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
> >+		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
> >+		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
> >+	if (n != 11) {
> 
> 11 more like a magic number here. Not to bad, but a define could be nice.

ok.

...
> >+
> >+const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
> >+	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
> >+	.update = lowpan_iphc_ctx_update,
> >+	.get_by_id = lowpan_iphc_ctx_get_by_id,
> >+	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
> >+};
> 
> These are the comments from a first look over it. I will have a more
> detailed review and actual testing once you are happy enough with it to move
> it from RFC to PATCH.
> 

ok.

- Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
  2015-11-25 16:42         ` Stefan Schmidt
@ 2015-11-25 18:17             ` Alexander Aring
  -1 siblings, 0 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-25 18:17 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: linux-wpan-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

On Wed, Nov 25, 2015 at 05:42:52PM +0100, Stefan Schmidt wrote:
> Hello.
> 
> On 17/11/15 23:33, Alexander Aring wrote:
> >This patch adds a static inline function ipv6_addr_prefix_cpy which
> >copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
> >The prefix len is given by plen as bits. This function mainly based on
> >ipv6_addr_prefix which copies one address prefix from address into a new
> >ipv6 address destination and zero all other address bits.
> >
> >The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
> >ipv6 address, it sets a prefix to an ipv6 address with keeping other
> >address bits. The use case is for context based address compression
> >inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
> >table to lookup address-bits without sending them.
> >
> >Cc: David S. Miller<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> >Cc: Alexey Kuznetsov<kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> >Cc: James Morris<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> >Cc: Hideaki YOSHIFUJI<yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> >Cc: Patrick McHardy<kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> >Signed-off-by: Alexander Aring<alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >---
> >  include/net/ipv6.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> >diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> >index e1a10b0..9d38fc2 100644
> >--- a/include/net/ipv6.h
> >+++ b/include/net/ipv6.h
> >@@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
> >  		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
> >  }
> >+static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
> >+					const struct in6_addr *pfx,
> >+					int plen)
> >+{
> 
> Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to
> read.

ok.

> >+	/* caller must guarantee 0 <= plen <= 128 */
> >+	int o = plen >> 3,
> >+	    b = plen & 0x7;
> >+
> 
> Any reason you are not doing the memset here before memcpy like it is done
> in ipv6_addr_prefi()?

ipv6_addr_prefix is more a "getter" for a prefix from an address. The
destination address (which should be the result) will memset the whole
address before copy the prefix from address to it.

THe ipv6_addr_prefix_copy will set a prefix given by second parameter
pfx and set the prefix to the address (given by first parameter).

Maybe the ipv6_addr_prefix could call:

1. memset(pfx, sizeof...)
2. ipv6_addr_prefix_copy(pfx, addr, plen);

but ipv6_addr_prefix_copy need to care about other bits which are set at
destination address, if (plen/8 != 0). I don't want to touch
ipv6_addr_prefix function and slow-down some mechanism.

- Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
@ 2015-11-25 18:17             ` Alexander Aring
  0 siblings, 0 replies; 23+ messages in thread
From: Alexander Aring @ 2015-11-25 18:17 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: linux-wpan, linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

On Wed, Nov 25, 2015 at 05:42:52PM +0100, Stefan Schmidt wrote:
> Hello.
> 
> On 17/11/15 23:33, Alexander Aring wrote:
> >This patch adds a static inline function ipv6_addr_prefix_cpy which
> >copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
> >The prefix len is given by plen as bits. This function mainly based on
> >ipv6_addr_prefix which copies one address prefix from address into a new
> >ipv6 address destination and zero all other address bits.
> >
> >The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
> >ipv6 address, it sets a prefix to an ipv6 address with keeping other
> >address bits. The use case is for context based address compression
> >inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
> >table to lookup address-bits without sending them.
> >
> >Cc: David S. Miller<davem@davemloft.net>
> >Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru>
> >Cc: James Morris<jmorris@namei.org>
> >Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org>
> >Cc: Patrick McHardy<kaber@trash.net>
> >Signed-off-by: Alexander Aring<alex.aring@gmail.com>
> >---
> >  include/net/ipv6.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> >diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> >index e1a10b0..9d38fc2 100644
> >--- a/include/net/ipv6.h
> >+++ b/include/net/ipv6.h
> >@@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
> >  		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
> >  }
> >+static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
> >+					const struct in6_addr *pfx,
> >+					int plen)
> >+{
> 
> Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to
> read.

ok.

> >+	/* caller must guarantee 0 <= plen <= 128 */
> >+	int o = plen >> 3,
> >+	    b = plen & 0x7;
> >+
> 
> Any reason you are not doing the memset here before memcpy like it is done
> in ipv6_addr_prefi()?

ipv6_addr_prefix is more a "getter" for a prefix from an address. The
destination address (which should be the result) will memset the whole
address before copy the prefix from address to it.

THe ipv6_addr_prefix_copy will set a prefix given by second parameter
pfx and set the prefix to the address (given by first parameter).

Maybe the ipv6_addr_prefix could call:

1. memset(pfx, sizeof...)
2. ipv6_addr_prefix_copy(pfx, addr, plen);

but ipv6_addr_prefix_copy need to care about other bits which are set at
destination address, if (plen/8 != 0). I don't want to touch
ipv6_addr_prefix function and slow-down some mechanism.

- Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support
  2015-11-25 18:00     ` Alexander Aring
@ 2015-11-26 11:07       ` Stefan Schmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-26 11:07 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan, linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit

Hello.

On 25/11/15 19:00, Alexander Aring wrote:
> Hi,
>
> On Wed, Nov 25, 2015 at 05:42:32PM +0100, Stefan Schmidt wrote:
>> Hello.
>>
>> On 17/11/15 23:33, Alexander Aring wrote:
>>> This patch will introduce a 6lowpan entry into the debugfs if enabled.
>>> Inside this 6lowpan directory we create a subdirectories of all 6lowpan
>>> interfaces to offer a per interface debugfs support.
> ...
>
>>>   static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
>>> @@ -180,6 +187,7 @@ static void lowpan_dellink(struct net_device *ldev, struct list_head *head)
>>>   	ASSERT_RTNL();
>>>   	wdev->ieee802154_ptr->lowpan_dev = NULL;
>>> +	lowpan_netdev_unsetup(ldev);
>>>   	unregister_netdevice(ldev);
>>>   	dev_put(wdev);
>>>   }
>> If you are going to change the th uninit to exit and the unsetup to teardown
>> you can add my review.
>>
>> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
>>
> I will add a patch which introduce the two functions:
>
> lowpan_register_netdevice - register_netdevice (without rtnl lock)
> lowpan_register_netdev - register_netdev (with rtnl lock)
>
> These two functions will call register_netdevice/register_netdev and 6lowpan
> debugfs init.
>
>
> I will remove the lowpan_netdev_unsetup function and introduce a:
>
> lowpan_unregister_netdevice - unregister_netdevice
>
> which calls unregister_netdevice and lowpan_dev_debugfs_exit. I will
> change the uninit to exit.
>
> Then we keep the same naming stuff like netdev and these function should
> always called when doing register_netdev/unregister_netdev for all
> lowpan interface.
>
> Then we can also move lowpan_netdev_setup before register_netdev. This
> will move more functionality which should be the same on all lowpan
> interfaces into the net/6lowpan branch.

Sounds good to me.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
  2015-11-25 18:17             ` Alexander Aring
@ 2015-11-26 11:11               ` Stefan Schmidt
  -1 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-26 11:11 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 25/11/15 19:17, Alexander Aring wrote:
> On Wed, Nov 25, 2015 at 05:42:52PM +0100, Stefan Schmidt wrote:
>> Hello.
>>
>> On 17/11/15 23:33, Alexander Aring wrote:
>>> This patch adds a static inline function ipv6_addr_prefix_cpy which
>>> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
>>> The prefix len is given by plen as bits. This function mainly based on
>>> ipv6_addr_prefix which copies one address prefix from address into a new
>>> ipv6 address destination and zero all other address bits.
>>>
>>> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
>>> ipv6 address, it sets a prefix to an ipv6 address with keeping other
>>> address bits. The use case is for context based address compression
>>> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
>>> table to lookup address-bits without sending them.
>>>
>>> Cc: David S. Miller<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>>> Cc: Alexey Kuznetsov<kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
>>> Cc: James Morris<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
>>> Cc: Hideaki YOSHIFUJI<yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
>>> Cc: Patrick McHardy<kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
>>> Signed-off-by: Alexander Aring<alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>   include/net/ipv6.h | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>>> index e1a10b0..9d38fc2 100644
>>> --- a/include/net/ipv6.h
>>> +++ b/include/net/ipv6.h
>>> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
>>>   		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>>>   }
>>> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
>>> +					const struct in6_addr *pfx,
>>> +					int plen)
>>> +{
>> Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to
>> read.
> ok.
>
>>> +	/* caller must guarantee 0 <= plen <= 128 */
>>> +	int o = plen >> 3,
>>> +	    b = plen & 0x7;
>>> +
>> Any reason you are not doing the memset here before memcpy like it is done
>> in ipv6_addr_prefi()?
> ipv6_addr_prefix is more a "getter" for a prefix from an address. The
> destination address (which should be the result) will memset the whole
> address before copy the prefix from address to it.

Ah, right. Missed that part.

> THe ipv6_addr_prefix_copy will set a prefix given by second parameter
> pfx and set the prefix to the address (given by first parameter).
>
> Maybe the ipv6_addr_prefix could call:
>
> 1. memset(pfx, sizeof...)
> 2. ipv6_addr_prefix_copy(pfx, addr, plen);
>
> but ipv6_addr_prefix_copy need to care about other bits which are set at
> destination address, if (plen/8 != 0). I don't want to touch
> ipv6_addr_prefix function and slow-down some mechanism.

No need to change this from my side. I just missed the memset being 
handled in the destination address.

With the name change to _copy you can add my

Reviewed-by: Stefan Schmidt <stefan-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy
@ 2015-11-26 11:11               ` Stefan Schmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-26 11:11 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan, linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit, David S . Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy

Hello.

On 25/11/15 19:17, Alexander Aring wrote:
> On Wed, Nov 25, 2015 at 05:42:52PM +0100, Stefan Schmidt wrote:
>> Hello.
>>
>> On 17/11/15 23:33, Alexander Aring wrote:
>>> This patch adds a static inline function ipv6_addr_prefix_cpy which
>>> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix.
>>> The prefix len is given by plen as bits. This function mainly based on
>>> ipv6_addr_prefix which copies one address prefix from address into a new
>>> ipv6 address destination and zero all other address bits.
>>>
>>> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an
>>> ipv6 address, it sets a prefix to an ipv6 address with keeping other
>>> address bits. The use case is for context based address compression
>>> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context
>>> table to lookup address-bits without sending them.
>>>
>>> Cc: David S. Miller<davem@davemloft.net>
>>> Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru>
>>> Cc: James Morris<jmorris@namei.org>
>>> Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org>
>>> Cc: Patrick McHardy<kaber@trash.net>
>>> Signed-off-by: Alexander Aring<alex.aring@gmail.com>
>>> ---
>>>   include/net/ipv6.h | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>>> index e1a10b0..9d38fc2 100644
>>> --- a/include/net/ipv6.h
>>> +++ b/include/net/ipv6.h
>>> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx,
>>>   		pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b);
>>>   }
>>> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr,
>>> +					const struct in6_addr *pfx,
>>> +					int plen)
>>> +{
>> Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to
>> read.
> ok.
>
>>> +	/* caller must guarantee 0 <= plen <= 128 */
>>> +	int o = plen >> 3,
>>> +	    b = plen & 0x7;
>>> +
>> Any reason you are not doing the memset here before memcpy like it is done
>> in ipv6_addr_prefi()?
> ipv6_addr_prefix is more a "getter" for a prefix from an address. The
> destination address (which should be the result) will memset the whole
> address before copy the prefix from address to it.

Ah, right. Missed that part.

> THe ipv6_addr_prefix_copy will set a prefix given by second parameter
> pfx and set the prefix to the address (given by first parameter).
>
> Maybe the ipv6_addr_prefix could call:
>
> 1. memset(pfx, sizeof...)
> 2. ipv6_addr_prefix_copy(pfx, addr, plen);
>
> but ipv6_addr_prefix_copy need to care about other bits which are set at
> destination address, if (plen/8 != 0). I don't want to touch
> ipv6_addr_prefix function and slow-down some mechanism.

No need to change this from my side. I just missed the memset being 
handled in the destination address.

With the name change to _copy you can add my

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

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
  2015-11-25 18:07           ` Alexander Aring
@ 2015-11-26 11:19             ` Stefan Schmidt
  -1 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-26 11:19 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	mcr-SWp7JaYWvAQV+D8aMU/kSg, lukasz.duda-hR+23Fw+YnFSHonuZl5R5Q,
	martin.gergeleit-6wGqcYweBVc

Hello.

On 25/11/15 19:07, Alexander Aring wrote:
> Hi,
>
> On Wed, Nov 25, 2015 at 06:12:25PM +0100, Stefan Schmidt wrote:
>> Helllo.
>>
>> On 17/11/15 23:33, Alexander Aring wrote:
>>> This patch introduce support for IPHC stateful address compression. It
>>> will offer three debugfs per interface entries, which are:
>>>
>>>   - dci_table: destination context indentifier table
>>>   - sci_table: source context indentifier table
>>>   - mcast_sci_table: multicast context identifier table
>>>
>>> Example to setup a context id:
>>>
>>> A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
>>> contexts which are available. Example:
>>>
>>> ID ipv6-address/prefix-length                  state
>>> 0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 15 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>>
>>> For setting a context e.g. context id 0, context 2001::, prefix-length
>>> 64.
>>>
>>> Hint: Simple copy one line and then maniuplate it.
>>>
>>> echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
>>> /sys/kernel/debug/6lowpan/lowpan0/dci_table
>>>
>>> The state will display currently if the context is disabled(0) or
>>> enabled(1).
>>>
>>> On transmit side:
>>>
>>> The IPHC code will automatically search for a context which would be
>>> match for the address. Then it will be use the context which the
>>> best compression, means the longest prefix which match will be used.
>>>
>>> Example:
>>>
>>> 2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
>>> last bit of the address which has the prefix 2001::/127 is the same like
>>> the IID from the Encapsulating Header. A context ID can also be a
>>> 2001::1/128, which is then a full ipv6 address.
>>>
>>> On Receive side:
>>>
>>> If there is a context defined (when CID not available then it's the
>>> default context 0) then it will be used, if the header doesn't set
>>> SAC or DAC bit thens, it will be dropped.
>>>
>>> Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>   include/net/6lowpan.h   |  62 ++++++
>>>   net/6lowpan/6lowpan_i.h |   3 +
>>>   net/6lowpan/Kconfig     |   1 +
>>>   net/6lowpan/core.c      |  17 ++
>>>   net/6lowpan/debugfs.c   | 109 +++++++++++
>>>   net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
>>>   6 files changed, 635 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
>>> index e484898..6e8d30e 100644
>>> --- a/include/net/6lowpan.h
>>> +++ b/include/net/6lowpan.h
>>> @@ -75,6 +75,8 @@
>>>   #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
>>>   					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
>>>   					 LOWPAN_NHC_MAX_HDR_LEN)
>>> +/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
>>> +#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
>>>   #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
>>>   #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
>>> @@ -98,9 +100,45 @@ enum lowpan_lltypes {
>>>   	LOWPAN_LLTYPE_IEEE802154,
>>>   };
>>> +enum lowpan_iphc_ctx_states {
>>> +	LOWPAN_IPHC_CTX_STATE_DISABLED,
>>> +	LOWPAN_IPHC_CTX_STATE_ENABLED,
>>> +
>>> +	__LOWPAN_IPHC_CTX_STATE_INVALID,
>>> +	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
>>> +};
>>> +
>> You are expecting more states here besides enabled and disabled? Because
>> normally a bool would be fine here and an enum overkill. If you have more
>> states pending it makes sense to have the enum though.
>>
> Yea, I thought about more states than just these ones. E.g. from which
> sources came the context which was set, from ICMPv6 option field, manually
> added from debugfs/netlink? Such things...

Hmm, the states would still need to show if it is enabled or disabled. 
Your plan was to interpret this state as a bitfield with bits for 
enabled/disabled, set my ICMP, set manually?

To me this looks a bit overloaded for a state field. Though I agree that 
the information where the context came from is interesting for the 
debugfs entry.

> Nevertheless I will change it to bool here, we can still change it if we
> want that. But then we need to talk about that again if doing userspace
> API for that.

Yeah, stay with bool for now. We can always have another column in the 
debugfs table.
Not sure the information where the context was coming from is of 
interest for normal userspace. The debugging case would be covered with 
the debugfs entry once we come to it.

>>> +struct lowpan_iphc_ctx {
>>> +	enum lowpan_iphc_ctx_states state;
>>> +	u8 id;
>>> +	struct in6_addr addr;
>>> +	u8 prefix_len;
>>> +};
>>> +
> ...
>>>   #include <linux/netdevice.h>
>>> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
>>> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
>>> +
>>>   #ifdef CONFIG_6LOWPAN_DEBUGFS
>>>   int lowpan_dev_debugfs_init(struct net_device *dev);
>>>   void lowpan_dev_debugfs_uninit(struct net_device *dev);
>>> diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
>>> index 01c901c..7ecedd7 100644
>>> --- a/net/6lowpan/Kconfig
>>> +++ b/net/6lowpan/Kconfig
>>> @@ -8,6 +8,7 @@ menuconfig 6LOWPAN
>>>   config 6LOWPAN_DEBUGFS
>>>   	bool "6LoWPAN debugfs support"
>>>   	depends on 6LOWPAN
>>> +	depends on DEBUG_FS
>> This looks like a fix that should be merged into the first patch, no?
> yes. I will fix that.

Thanks.

>>>   	---help---
>>>   	  This enables 6LoWPAN debugfs support. For example to manipulate
>>>   	  IPHC context information at runtime.
>>> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
>>> index fe58509..d354c5b 100644
>>> --- a/net/6lowpan/core.c
>>> +++ b/net/6lowpan/core.c
>>> @@ -19,6 +19,8 @@
>>>   int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>>>   {
>>> +	int i;
>>> +
>>>   	dev->addr_len = EUI64_ADDR_LEN;
>>>   	dev->type = ARPHRD_6LOWPAN;
>>>   	dev->mtu = IPV6_MIN_MTU;
> ...
>>> +static ssize_t lowpan_context_dbgfs_write(struct file *fp,
>>> +					  const char __user *user_buf,
>>> +					  size_t count, loff_t *ppos)
>>> +{
>>> +	char buf[128] = { };
>>> +	struct seq_file *file = fp->private_data;
>>> +	struct lowpan_iphc_ctx_table *t = file->private;
>>> +	struct lowpan_iphc_ctx ctx;
>>> +	int status = count, n, id, state, i, prefix_len, ret;
>>> +	unsigned int addr[8];
>>> +
>>> +	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
>>> +						 count))) {
>>> +		status = -EFAULT;
>>> +		goto out;
>>> +	}
>>> +
>>> +	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
>>> +		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
>>> +		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
>>> +	if (n != 11) {
>> 11 more like a magic number here. Not to bad, but a define could be nice.
> ok.
>
> ...
>>> +
>>> +const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
>>> +	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
>>> +	.update = lowpan_iphc_ctx_update,
>>> +	.get_by_id = lowpan_iphc_ctx_get_by_id,
>>> +	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
>>> +};
>> These are the comments from a first look over it. I will have a more
>> detailed review and actual testing once you are happy enough with it to move
>> it from RFC to PATCH.
>>
> ok.
>
> - Alex

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression
@ 2015-11-26 11:19             ` Stefan Schmidt
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Schmidt @ 2015-11-26 11:19 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan, linux-bluetooth, netdev, kernel, mcr, lukasz.duda,
	martin.gergeleit

Hello.

On 25/11/15 19:07, Alexander Aring wrote:
> Hi,
>
> On Wed, Nov 25, 2015 at 06:12:25PM +0100, Stefan Schmidt wrote:
>> Helllo.
>>
>> On 17/11/15 23:33, Alexander Aring wrote:
>>> This patch introduce support for IPHC stateful address compression. It
>>> will offer three debugfs per interface entries, which are:
>>>
>>>   - dci_table: destination context indentifier table
>>>   - sci_table: source context indentifier table
>>>   - mcast_sci_table: multicast context identifier table
>>>
>>> Example to setup a context id:
>>>
>>> A "cat /sys/kernel/debug/6lowpan/lowpan0/dci_table" will display all
>>> contexts which are available. Example:
>>>
>>> ID ipv6-address/prefix-length                  state
>>> 0  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 1  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 2  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 3  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 4  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 5  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 6  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 7  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 8  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 9  0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 10 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 11 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 12 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 13 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 14 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>> 15 0000:0000:0000:0000:0000:0000:0000:0000/000 0
>>>
>>> For setting a context e.g. context id 0, context 2001::, prefix-length
>>> 64.
>>>
>>> Hint: Simple copy one line and then maniuplate it.
>>>
>>> echo "0 2001:0000:0000:0000:0000:0000:0000:0000/64 1" >
>>> /sys/kernel/debug/6lowpan/lowpan0/dci_table
>>>
>>> The state will display currently if the context is disabled(0) or
>>> enabled(1).
>>>
>>> On transmit side:
>>>
>>> The IPHC code will automatically search for a context which would be
>>> match for the address. Then it will be use the context which the
>>> best compression, means the longest prefix which match will be used.
>>>
>>> Example:
>>>
>>> 2001::/126 vs 2001::/127 - the 2001::/127 can be full compressed if the
>>> last bit of the address which has the prefix 2001::/127 is the same like
>>> the IID from the Encapsulating Header. A context ID can also be a
>>> 2001::1/128, which is then a full ipv6 address.
>>>
>>> On Receive side:
>>>
>>> If there is a context defined (when CID not available then it's the
>>> default context 0) then it will be used, if the header doesn't set
>>> SAC or DAC bit thens, it will be dropped.
>>>
>>> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
>>> ---
>>>   include/net/6lowpan.h   |  62 ++++++
>>>   net/6lowpan/6lowpan_i.h |   3 +
>>>   net/6lowpan/Kconfig     |   1 +
>>>   net/6lowpan/core.c      |  17 ++
>>>   net/6lowpan/debugfs.c   | 109 +++++++++++
>>>   net/6lowpan/iphc.c      | 500 ++++++++++++++++++++++++++++++++++++++++++------
>>>   6 files changed, 635 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
>>> index e484898..6e8d30e 100644
>>> --- a/include/net/6lowpan.h
>>> +++ b/include/net/6lowpan.h
>>> @@ -75,6 +75,8 @@
>>>   #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
>>>   					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
>>>   					 LOWPAN_NHC_MAX_HDR_LEN)
>>> +/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
>>> +#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
>>>   #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
>>>   #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
>>> @@ -98,9 +100,45 @@ enum lowpan_lltypes {
>>>   	LOWPAN_LLTYPE_IEEE802154,
>>>   };
>>> +enum lowpan_iphc_ctx_states {
>>> +	LOWPAN_IPHC_CTX_STATE_DISABLED,
>>> +	LOWPAN_IPHC_CTX_STATE_ENABLED,
>>> +
>>> +	__LOWPAN_IPHC_CTX_STATE_INVALID,
>>> +	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
>>> +};
>>> +
>> You are expecting more states here besides enabled and disabled? Because
>> normally a bool would be fine here and an enum overkill. If you have more
>> states pending it makes sense to have the enum though.
>>
> Yea, I thought about more states than just these ones. E.g. from which
> sources came the context which was set, from ICMPv6 option field, manually
> added from debugfs/netlink? Such things...

Hmm, the states would still need to show if it is enabled or disabled. 
Your plan was to interpret this state as a bitfield with bits for 
enabled/disabled, set my ICMP, set manually?

To me this looks a bit overloaded for a state field. Though I agree that 
the information where the context came from is interesting for the 
debugfs entry.

> Nevertheless I will change it to bool here, we can still change it if we
> want that. But then we need to talk about that again if doing userspace
> API for that.

Yeah, stay with bool for now. We can always have another column in the 
debugfs table.
Not sure the information where the context was coming from is of 
interest for normal userspace. The debugging case would be covered with 
the debugfs entry once we come to it.

>>> +struct lowpan_iphc_ctx {
>>> +	enum lowpan_iphc_ctx_states state;
>>> +	u8 id;
>>> +	struct in6_addr addr;
>>> +	u8 prefix_len;
>>> +};
>>> +
> ...
>>>   #include <linux/netdevice.h>
>>> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
>>> +extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
>>> +
>>>   #ifdef CONFIG_6LOWPAN_DEBUGFS
>>>   int lowpan_dev_debugfs_init(struct net_device *dev);
>>>   void lowpan_dev_debugfs_uninit(struct net_device *dev);
>>> diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
>>> index 01c901c..7ecedd7 100644
>>> --- a/net/6lowpan/Kconfig
>>> +++ b/net/6lowpan/Kconfig
>>> @@ -8,6 +8,7 @@ menuconfig 6LOWPAN
>>>   config 6LOWPAN_DEBUGFS
>>>   	bool "6LoWPAN debugfs support"
>>>   	depends on 6LOWPAN
>>> +	depends on DEBUG_FS
>> This looks like a fix that should be merged into the first patch, no?
> yes. I will fix that.

Thanks.

>>>   	---help---
>>>   	  This enables 6LoWPAN debugfs support. For example to manipulate
>>>   	  IPHC context information at runtime.
>>> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
>>> index fe58509..d354c5b 100644
>>> --- a/net/6lowpan/core.c
>>> +++ b/net/6lowpan/core.c
>>> @@ -19,6 +19,8 @@
>>>   int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
>>>   {
>>> +	int i;
>>> +
>>>   	dev->addr_len = EUI64_ADDR_LEN;
>>>   	dev->type = ARPHRD_6LOWPAN;
>>>   	dev->mtu = IPV6_MIN_MTU;
> ...
>>> +static ssize_t lowpan_context_dbgfs_write(struct file *fp,
>>> +					  const char __user *user_buf,
>>> +					  size_t count, loff_t *ppos)
>>> +{
>>> +	char buf[128] = { };
>>> +	struct seq_file *file = fp->private_data;
>>> +	struct lowpan_iphc_ctx_table *t = file->private;
>>> +	struct lowpan_iphc_ctx ctx;
>>> +	int status = count, n, id, state, i, prefix_len, ret;
>>> +	unsigned int addr[8];
>>> +
>>> +	if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
>>> +						 count))) {
>>> +		status = -EFAULT;
>>> +		goto out;
>>> +	}
>>> +
>>> +	n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
>>> +		   &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
>>> +		   &addr[5], &addr[6], &addr[7], &prefix_len, &state);
>>> +	if (n != 11) {
>> 11 more like a magic number here. Not to bad, but a define could be nice.
> ok.
>
> ...
>>> +
>>> +const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
>>> +	.valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
>>> +	.update = lowpan_iphc_ctx_update,
>>> +	.get_by_id = lowpan_iphc_ctx_get_by_id,
>>> +	.get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
>>> +};
>> These are the comments from a first look over it. I will have a more
>> detailed review and actual testing once you are happy enough with it to move
>> it from RFC to PATCH.
>>
> ok.
>
> - Alex

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2015-11-26 11:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-17 22:33 [RFCv2 bluetooth-next 0/3] 6lowpan: debugfs and stateful compression support Alexander Aring
2015-11-17 22:33 ` [RFCv2 bluetooth-next 1/3] 6lowpan: add debugfs support Alexander Aring
2015-11-25 16:42   ` Stefan Schmidt
2015-11-25 18:00     ` Alexander Aring
2015-11-26 11:07       ` Stefan Schmidt
     [not found] ` <1447799594-6050-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-17 22:33   ` [RFCv2 bluetooth-next 2/3] ipv6: add ipv6_addr_prefix_cpy Alexander Aring
2015-11-17 22:33     ` Alexander Aring
     [not found]     ` <1447799594-6050-3-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-18 13:55       ` Sergei Shtylyov
2015-11-18 13:55         ` Sergei Shtylyov
2015-11-25 16:42         ` Stefan Schmidt
2015-11-25 16:42       ` Stefan Schmidt
2015-11-25 16:42         ` Stefan Schmidt
     [not found]         ` <5655E50C.5090906-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
2015-11-25 18:17           ` Alexander Aring
2015-11-25 18:17             ` Alexander Aring
2015-11-26 11:11             ` Stefan Schmidt
2015-11-26 11:11               ` Stefan Schmidt
2015-11-17 22:33 ` [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression Alexander Aring
     [not found]   ` <1447799594-6050-4-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-25 17:12     ` Stefan Schmidt
2015-11-25 17:12       ` Stefan Schmidt
     [not found]       ` <5655EBF9.7060102-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
2015-11-25 18:07         ` Alexander Aring
2015-11-25 18:07           ` Alexander Aring
2015-11-26 11:19           ` Stefan Schmidt
2015-11-26 11:19             ` Stefan Schmidt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.