linux-wpan.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 0/4] Adding stateful compression to IPHC
@ 2015-07-13 11:50 Lukasz Duda
  2015-07-13 11:50 ` [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module Lukasz Duda
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Lukasz Duda @ 2015-07-13 11:50 UTC (permalink / raw)
  To: alex.aring; +Cc: linux-wpan, linux-bluetooth

Hi all,

Resending the RFC patch set after some clean up.

The following patches introduce support for stateful compression in IPHC.

Patch #1: Introduce debugfs entry for 6lowpan module
Patch #2: Add stateful compression component of 6lowpan module
Patch #3: Add stateful compression support for iphc.c
Patch #4: Enable stateful compression in bluetooth_6lowpan

Usage of stateful compression is described in Patch #2.

Notes
=====

RFC v2:
    - Splitting patch into smaller incremental patches
    - Moving debugfs logic from iphc.c into context.c
    - Updating patch set to align with style and coding guideline
    
RFC v1:
    - Initial patch set: http://www.spinics.net/lists/linux-bluetooth/msg62930.html

Limitations
===========

- Temporarly use of debugfs to be able to add context table entries.
- Current module does not support context time-outs.
- No support for multicast addresses stateful compression.

Discussion: Idea on how to make the 6lowpan context tables out of debug mode
============================================================================

The patch provided here is just a temporary solution where the contexts are
manually added. The proper way of adding/removing contexts would be to make
ndisc.c parse 6CO options and act upon it. Generated Router Advertisement
packets with 6CO option (for example from RADVD) will be handled in the
ndisc_router_discovery function.

Also, other flags like ARO, ABRO etc. which are specified in RFC6775 need
to be handled in the ipv6 module in order to do it in a neat way.
Potentially the RFC6775 extensions could also be used by other protocols
than BLE and 802.15.4.

There is a need for parsing the 6CO option in ndisc.c which parses Router
Advertisement messages. Today the function calls to IPHC (6lowpan module) are
quite hard to implement in ndisc.c, as there is no guarantee that the 6lowpan
module will be loaded or not. It would make sense to build 6lowpan module
as in-build part of IPv6. The features could be compiled in by using #defines.

What do you think about moving 6lowpan as a component of IPv6 and modify ndisc.c
to handled 6LoWPAN specific options? Do you see any other solution to make sure
that 6CO is parsed and acted upon and still keep 6lowpan as a stand-alone module?

Best regards,
Lukasz Duda

Lukasz Duda (4):
  6lowpan: Introduce debugfs entry for 6lowpan module
  6lowpan: Add stateful compression component of 6lowpan module
  6lowpan: Add stateful compression support for iphc.c
  Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan

 include/net/6lowpan.h   |  37 ++++
 net/6lowpan/Makefile    |   2 +-
 net/6lowpan/context.c   | 551 ++++++++++++++++++++++++++++++++++++++++++++++++
 net/6lowpan/context.h   |   7 +
 net/6lowpan/iphc.c      | 245 +++++++++++++++++----
 net/bluetooth/6lowpan.c |   4 +
 6 files changed, 809 insertions(+), 37 deletions(-)
 create mode 100644 net/6lowpan/context.c
 create mode 100644 net/6lowpan/context.h

-- 
2.1.4


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

* [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
@ 2015-07-13 11:50 ` Lukasz Duda
  2015-07-23  6:07   ` Alexander Aring
  2015-07-13 11:50 ` [RFC v2 2/4] 6lowpan: Add stateful compression component of " Lukasz Duda
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Lukasz Duda @ 2015-07-13 11:50 UTC (permalink / raw)
  To: alex.aring; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

Creating "6lowpan" debugfs entry (/sys/kernel/debugfs/6lowpan) for usage
by 6lowpan module.

Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
---
 include/net/6lowpan.h |  2 ++
 net/6lowpan/iphc.c    | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index dc03d77..37ddbdf 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -197,6 +197,8 @@
 #define LOWPAN_NHC_UDP_CS_P_11	0xF3 /* source & dest = 0xF0B + 4bit inline */
 #define LOWPAN_NHC_UDP_CS_C	0x04 /* checksum elided */
 
+extern struct dentry *lowpan_debugfs;
+
 #ifdef DEBUG
 /* print data in line */
 static inline void raw_dump_inline(const char *caller, char *msg,
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 9055d7b..c845a8a 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -50,12 +50,16 @@
 #include <linux/if_arp.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/debugfs.h>
 #include <net/6lowpan.h>
 #include <net/ipv6.h>
 #include <net/af_ieee802154.h>
 
 #include "nhc.h"
 
+struct dentry *lowpan_debugfs;
+EXPORT_SYMBOL_GPL(lowpan_debugfs);
+
 /* Uncompress address function for source and
  * destination address(non-multicast).
  *
@@ -613,6 +617,8 @@ EXPORT_SYMBOL_GPL(lowpan_header_compress);
 
 static int __init lowpan_module_init(void)
 {
+	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
+
 	request_module_nowait("ipv6");
 
 	request_module_nowait("nhc_dest");
@@ -625,6 +631,13 @@ static int __init lowpan_module_init(void)
 
 	return 0;
 }
+
+static void __exit lowpan_module_exit(void)
+{
+	debugfs_remove(lowpan_debugfs);
+}
+
 module_init(lowpan_module_init);
+module_exit(lowpan_module_exit);
 
 MODULE_LICENSE("GPL");
-- 
2.1.4


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

* [RFC v2 2/4] 6lowpan: Add stateful compression component of 6lowpan module
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
  2015-07-13 11:50 ` [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module Lukasz Duda
@ 2015-07-13 11:50 ` Lukasz Duda
  2015-07-23  6:52   ` Alexander Aring
  2015-07-13 11:50 ` [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c Lukasz Duda
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Lukasz Duda @ 2015-07-13 11:50 UTC (permalink / raw)
  To: alex.aring; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

The patch adds stateful compression (context ids based compression) to
the 6lowpan module [RFC6282].

Stateful Compression users allocate context tables through the functions
in the context.c using public APIs declared in 6lowpan.h. The context.c
is extending the 6lowpan module and not created as a separate module.

A debugfs entry for maintaining the context tables is implemented
(/sys/kernel/debug/6lowpan/context).

Example usage of the 6lowpan debugfs interface:

echo "<CMD> <IFACE> <CID> <CID_PREFIX> <CID_PREFIX_LENGTH>
<COMPRESSION_ENABLE_FLAG>" > /sys/kernel/debug/6lowpan/context

Examples:

echo "add bt0 3 2001:db8:: 64 1" > /sys/kernel/debug/6lowpan/context

echo "remove bt0 3" > /sys/kernel/debug/6lowpan/context

cat /sys/kernel/debug/6lowpan/context

e.g.
Context table of interface bt0

CID Prefix      Len CF
3   2001:db8::  64  1
4   2001:db8::1 128 1

Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
---
 include/net/6lowpan.h |  35 ++++
 net/6lowpan/Makefile  |   2 +-
 net/6lowpan/context.c | 551 ++++++++++++++++++++++++++++++++++++++++++++++++++
 net/6lowpan/context.h |   7 +
 4 files changed, 594 insertions(+), 1 deletion(-)
 create mode 100644 net/6lowpan/context.c
 create mode 100644 net/6lowpan/context.h

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index 37ddbdf..ee3a455 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -199,6 +199,27 @@
 
 extern struct dentry *lowpan_debugfs;
 
+struct lowpan_context_table {
+	struct list_head list;
+
+	/* Context entries */
+	struct net_device *netdev;
+	struct list_head context_entries;
+	atomic_t context_count;
+};
+
+struct lowpan_context_entry {
+	struct list_head list;
+	struct rcu_head rcu;
+	struct lowpan_context_table *ctx_table;
+
+	/* Context parameters */
+	u8 cid;
+	struct in6_addr prefix;
+	unsigned int prefix_len;
+	bool compression_flag;
+};
+
 #ifdef DEBUG
 /* print data in line */
 static inline void raw_dump_inline(const char *caller, char *msg,
@@ -337,6 +358,9 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
 	if (!(iphc0 & 0x03))
 		ret++;
 
+	if (iphc1 & LOWPAN_IPHC_CID)
+		ret++;
+
 	ret += lowpan_addr_mode_size((iphc1 & LOWPAN_IPHC_SAM) >>
 				     LOWPAN_IPHC_SAM_BIT);
 
@@ -374,6 +398,17 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
 	return skb->len + uncomp_header - ret;
 }
 
+int lowpan_context_table_alloc(struct net_device *netdev);
+int lowpan_context_table_free(struct net_device *netdev);
+int lowpan_context_free_all(void);
+int lowpan_context_add(struct net_device *netdev,
+		       struct lowpan_context_entry *entry);
+int lowpan_context_remove(struct lowpan_context_entry *entry);
+struct lowpan_context_entry *
+lowpan_context_decompress(struct net_device *netdev, u8 cid);
+struct lowpan_context_entry *
+lowpan_context_compress(struct net_device *netdev, struct in6_addr *addr);
+
 int
 lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
 			 const u8 *saddr, const u8 saddr_type,
diff --git a/net/6lowpan/Makefile b/net/6lowpan/Makefile
index eb8baa7..bda5be0 100644
--- a/net/6lowpan/Makefile
+++ b/net/6lowpan/Makefile
@@ -1,6 +1,6 @@
 obj-$(CONFIG_6LOWPAN) += 6lowpan.o
 
-6lowpan-y := iphc.o nhc.o
+6lowpan-y := iphc.o nhc.o context.o
 
 #rfc6282 nhcs
 obj-$(CONFIG_6LOWPAN_NHC_DEST) += nhc_dest.o
diff --git a/net/6lowpan/context.c b/net/6lowpan/context.c
new file mode 100644
index 0000000..00cd5af
--- /dev/null
+++ b/net/6lowpan/context.c
@@ -0,0 +1,551 @@
+/*
+ * Copyright (c)  2015 Nordic Semiconductor. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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.
+ */
+
+#include <linux/if_arp.h>
+#include <linux/netdevice.h>
+#include <linux/module.h>
+#include <linux/debugfs.h>
+#include <linux/inet.h>
+#include <linux/atomic.h>
+
+#include <net/ipv6.h>
+#include <net/addrconf.h>
+#include <net/6lowpan.h>
+
+#define LOWPAN_DBG(fmt, ...)  pr_debug(fmt "\n", ##__VA_ARGS__)
+
+#define LOWPAN_CONTEXT_TABLE_SIZE     16
+#define LOWPAN_CONTEXT_COMMAND_ADD    "add"
+#define LOWPAN_CONTEXT_COMMAND_REMOVE "remove"
+
+#define CHECK_CID_RANGE(cid) ((cid) > 0 && (cid) < LOWPAN_CONTEXT_TABLE_SIZE)
+#define CHECK_PREFIX_RANGE(len) ((len) > 0 && (len) <= 128)
+
+static struct dentry *lowpan_context_debugfs;
+
+static LIST_HEAD(context_tables);
+static DEFINE_SPINLOCK(module_lock);
+
+static inline void entry_update(struct lowpan_context_entry *entry,
+				u8 cid,
+				struct in6_addr prefix,
+				unsigned int prefix_len,
+				bool compression_flag)
+{
+	/* Fill context entry. */
+	entry->cid = cid;
+	entry->compression_flag = compression_flag;
+	entry->prefix = prefix;
+	entry->prefix_len = prefix_len;
+}
+
+static inline void entry_free(struct lowpan_context_entry *entry)
+{
+	/* Increase number of available contexts. */
+	atomic_dec(&entry->ctx_table->context_count);
+
+	list_del_rcu(&entry->list);
+	kfree_rcu(entry, rcu);
+}
+
+static int entry_add(struct lowpan_context_table *ctx_table,
+		     u8 cid,
+		     struct in6_addr prefix,
+		     unsigned int prefix_len,
+		     bool compression_flag)
+{
+	struct lowpan_context_entry *new_entry = NULL;
+
+	new_entry = kzalloc(sizeof(*new_entry), GFP_ATOMIC);
+	if (!new_entry)
+		return -ENOMEM;
+
+	/* Fill context entry. */
+	new_entry->ctx_table = ctx_table;
+	entry_update(new_entry, cid, prefix, prefix_len, compression_flag);
+
+	INIT_LIST_HEAD(&new_entry->list);
+	list_add_rcu(&new_entry->list, &ctx_table->context_entries);
+
+	/* Increase number of available contexts. */
+	atomic_inc(&ctx_table->context_count);
+
+	return 0;
+}
+
+static void table_free(struct lowpan_context_table *ctx_table)
+{
+	struct lowpan_context_entry *entry = NULL;
+
+	rcu_read_lock();
+
+	/* Clear context table. */
+	if (atomic_read(&ctx_table->context_count)) {
+		list_for_each_entry_rcu(entry,
+					&ctx_table->context_entries,
+					list)
+			entry_free(entry);
+	}
+
+	rcu_read_unlock();
+
+	/* Remove context table. */
+	list_del(&ctx_table->list);
+	kfree(ctx_table);
+}
+
+static inline struct net_device *find_netdev(const char *name)
+{
+	struct net_device *netdev;
+
+	rcu_read_lock();
+	netdev = dev_get_by_name_rcu(&init_net, name);
+	rcu_read_unlock();
+
+	return netdev;
+}
+
+static struct lowpan_context_table *get_table(struct net_device *netdev)
+{
+	struct lowpan_context_table *entry;
+	struct lowpan_context_table *net_table = NULL;
+
+	spin_lock(&module_lock);
+
+	list_for_each_entry(entry, &context_tables, list) {
+		if (entry->netdev == netdev) {
+			/* Table has been found. */
+			net_table = entry;
+			break;
+		}
+	}
+
+	spin_unlock(&module_lock);
+
+	return net_table;
+}
+
+static struct lowpan_context_entry *
+get_ctx_by_cid(struct lowpan_context_table *table, u8 cid)
+{
+	struct lowpan_context_entry *entry;
+	struct lowpan_context_entry *context = NULL;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(entry, &table->context_entries, list) {
+		if (entry->cid == cid) {
+			/* Context entry has been found. */
+			context = entry;
+			break;
+		}
+	}
+
+	rcu_read_unlock();
+
+	return context;
+}
+
+static struct lowpan_context_entry *
+get_ctx_by_addr(struct lowpan_context_table *table, struct in6_addr *addr)
+{
+	struct lowpan_context_entry *entry;
+	struct lowpan_context_entry *best_match = NULL;
+	unsigned int compare_len;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(entry, &table->context_entries, list) {
+		if (!entry->compression_flag)
+			continue;
+
+		compare_len = entry->prefix_len >= 64 ? entry->prefix_len : 64;
+		if (ipv6_prefix_equal(addr, &entry->prefix, compare_len)) {
+			/* Valid context entry has been found. Check if prefix
+			 * can cover more bytes than previous one.
+			 */
+			if (!best_match ||
+			    (best_match->prefix_len < entry->prefix_len))
+				best_match = entry;
+		}
+	}
+
+	rcu_read_unlock();
+
+	return best_match;
+}
+
+ssize_t lowpan_context_dbgfs_write(struct file *fp,
+				   const char __user *user_buffer,
+				   size_t count,
+				   loff_t *position)
+{
+	char buf[128];
+	char str_operation[16];
+	char str_interface[16];
+	char str_ipv6addr[40];
+	size_t buf_size = min(count, sizeof(buf) - 1);
+	int n;
+	int ret;
+	u8 cid;
+	unsigned int prefix_length;
+	unsigned int c_flag;
+	struct in6_addr ctx_prefix;
+	struct net_device *netdev;
+	struct lowpan_context_table *ctx_table;
+	struct lowpan_context_entry *entry;
+
+	memset(&ctx_prefix, 0, sizeof(ctx_prefix));
+
+	if (copy_from_user(buf, user_buffer, buf_size))
+		return -EFAULT;
+
+	buf[buf_size] = '\0';
+
+	/* Parse input parameters. */
+	n = sscanf(buf, "%s %s %hhd %s %d %d",
+		   str_operation, str_interface, &cid, str_ipv6addr,
+		   &prefix_length, &c_flag);
+
+	netdev = find_netdev(str_interface);
+	/* Look up for net device. */
+	if (!netdev) {
+		LOWPAN_DBG("Cannot find given interface");
+		return -EINVAL;
+	}
+
+	if (!CHECK_CID_RANGE(cid)) {
+		LOWPAN_DBG("CID value is out of range");
+		return -EINVAL;
+	}
+
+	/* Get context table. */
+	ctx_table = get_table(netdev);
+
+	if (!ctx_table) {
+		LOWPAN_DBG("Cannot find context table for %s interface",
+			   str_interface);
+		return -EINVAL;
+	}
+
+	entry = get_ctx_by_cid(ctx_table, cid);
+
+	/* Execute command. */
+	if (!memcmp(str_operation, LOWPAN_CONTEXT_COMMAND_ADD,
+		    sizeof(LOWPAN_CONTEXT_COMMAND_ADD))) {
+		/* Parse IPv6 address. */
+		in6_pton(str_ipv6addr,
+			 sizeof(str_ipv6addr),
+			 ctx_prefix.s6_addr,
+			 '\0',
+			 NULL);
+
+		/* Check parameters. */
+		if (ipv6_addr_any(&ctx_prefix) ||
+		    !CHECK_PREFIX_RANGE(prefix_length)) {
+			LOWPAN_DBG("Given parameters are not valid");
+			return -EINVAL;
+		}
+
+		LOWPAN_DBG("Got request: I:%s CID:%d Prefix:%pI6c/%d C:%d",
+			   str_interface, cid, &ctx_prefix, prefix_length,
+			   !!c_flag);
+
+		/* Check if entry for given CID already exists. */
+		if (entry) {
+			/* Just update parameters. */
+			rcu_read_lock();
+			entry_update(entry, cid, ctx_prefix, prefix_length,
+				     !!c_flag);
+			rcu_read_unlock();
+		} else {
+			/* Create a new entry. */
+			ret = entry_add(ctx_table, cid, ctx_prefix,
+					prefix_length, !!c_flag);
+			if (ret < 0) {
+				LOWPAN_DBG("Cannot add context entry");
+				return ret;
+			}
+		}
+	} else if (memcmp(str_operation, LOWPAN_CONTEXT_COMMAND_REMOVE,
+			  sizeof(LOWPAN_CONTEXT_COMMAND_REMOVE)) == 0) {
+		LOWPAN_DBG("Got new remove request: I:%s CID:%d",
+			   str_interface, cid);
+
+		if (!entry) {
+			LOWPAN_DBG("Cannot find context entry");
+			return -EINVAL;
+		}
+
+		rcu_read_lock();
+		entry_free(entry);
+		rcu_read_unlock();
+	} else {
+		LOWPAN_DBG("Invalid command - Cannot process the request");
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static int lowpan_context_show(struct seq_file *f, void *ptr)
+{
+	struct lowpan_context_table *table;
+	struct lowpan_context_entry *entry;
+
+	list_for_each_entry(table, &context_tables, list) {
+		if (!atomic_read(&table->context_count))
+			break;
+
+		seq_printf(f, "Context table of interface %s\r\n\r\n",
+			   table->netdev->name);
+		seq_printf(f, "%-3s %-39s %-3s %-3s \r\n",
+			   "CID", "Prefix", "Len", "CF");
+
+		list_for_each_entry_rcu(entry, &table->context_entries, list)
+			seq_printf(f, "%-3d %-39pI6c %-3d %-3s \r\n",
+				   entry->cid,
+				   &entry->prefix,
+				   entry->prefix_len,
+				   entry->compression_flag ? "1" : "0");
+		seq_puts(f, "\r\n\r\n");
+	}
+
+	return 0;
+}
+
+int lowpan_context_dbgfs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, lowpan_context_show, inode->i_private);
+}
+
+int lowpan_context_table_alloc(struct net_device *netdev)
+{
+	struct lowpan_context_table *ctx_table = NULL;
+
+	if (!netdev) {
+		LOWPAN_DBG("Network device has to be specified");
+		return -EPERM;
+	}
+
+	/* Look for context table. */
+	ctx_table = get_table(netdev);
+
+	if (ctx_table) {
+		LOWPAN_DBG("Context table already exists for interface %s %p",
+			   netdev->name, ctx_table);
+		return -EEXIST;
+	}
+
+	ctx_table = kzalloc(sizeof(*ctx_table), GFP_ATOMIC);
+	if (!ctx_table)
+		return -ENOMEM;
+
+	/* Assign network device to context table. */
+	ctx_table->netdev = netdev;
+
+	/* Initialize contexts list. */
+	INIT_LIST_HEAD(&ctx_table->context_entries);
+
+	spin_lock(&module_lock);
+	INIT_LIST_HEAD(&ctx_table->list);
+	list_add_rcu(&ctx_table->list, &context_tables);
+	spin_unlock(&module_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lowpan_context_table_alloc);
+
+int lowpan_context_table_free(struct net_device *netdev)
+{
+	struct lowpan_context_table *ctx_table = NULL;
+
+	if (!netdev) {
+		LOWPAN_DBG("Network device has to be specified");
+		return -EPERM;
+	}
+
+	/* Look for context table. */
+	ctx_table = get_table(netdev);
+	if (!ctx_table) {
+		LOWPAN_DBG("Cannot find context table for interface %s",
+			   netdev->name);
+		return -ENOENT;
+	}
+
+	spin_lock(&module_lock);
+	table_free(ctx_table);
+	spin_unlock(&module_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lowpan_context_table_free);
+
+int lowpan_context_add(struct net_device *netdev,
+		       struct lowpan_context_entry *entry)
+{
+	int ret;
+	struct lowpan_context_table *ctx_table = NULL;
+	struct lowpan_context_entry *stored_entry = NULL;
+
+	if (!entry || !netdev) {
+		LOWPAN_DBG("Parameters are incorrect");
+		return -EPERM;
+	}
+
+	/* Get context table.  */
+	ctx_table = get_table(netdev);
+	if (!ctx_table) {
+		LOWPAN_DBG("Cannot find context table for %s interface",
+			   netdev->name);
+		return -EINVAL;
+	}
+
+	stored_entry = get_ctx_by_cid(ctx_table, entry->cid);
+
+	/* Check if entry for given CID already exists. */
+	if (entry) {
+		/* Just update parameters. */
+		rcu_read_lock();
+		entry_update(stored_entry, entry->cid, entry->prefix,
+			     entry->prefix_len, entry->compression_flag);
+		rcu_read_unlock();
+	} else {
+		/* Create a new entry. */
+		ret  = entry_add(ctx_table, entry->cid, entry->prefix,
+				 entry->prefix_len, entry->compression_flag);
+
+		if (ret < 0) {
+			LOWPAN_DBG("Cannot add context entry");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lowpan_context_add);
+
+int lowpan_context_remove(struct lowpan_context_entry *entry)
+{
+	if (!entry) {
+		LOWPAN_DBG("Given entry is NULL");
+		return -EPERM;
+	}
+
+	/* Free given entry. */
+	rcu_read_lock();
+	entry_free(entry);
+	rcu_read_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(lowpan_context_remove);
+
+struct lowpan_context_entry *
+lowpan_context_decompress(struct net_device *netdev, u8 cid)
+{
+	struct lowpan_context_table *ctx_table = NULL;
+	struct lowpan_context_entry *entry = NULL;
+
+	if (!netdev) {
+		LOWPAN_DBG("Network device has to be specified");
+		return NULL;
+	}
+
+	/* Get proper interface table. */
+	ctx_table = get_table(netdev);
+	if (!ctx_table) {
+		LOWPAN_DBG("Cannot find context table for interface %s",
+			   netdev->name);
+		return NULL;
+	}
+
+	/* Search for given CID. */
+	entry = get_ctx_by_cid(ctx_table, cid);
+	if (!entry) {
+		LOWPAN_DBG("There is no context of id:%d for interface %s",
+			   cid, netdev->name);
+		return NULL;
+	}
+
+	LOWPAN_DBG("Found context prefix for context of id:%d - %pI6c",
+		   entry->cid, &entry->prefix);
+
+	return entry;
+}
+EXPORT_SYMBOL_GPL(lowpan_context_decompress);
+
+struct lowpan_context_entry *lowpan_context_compress(struct net_device *netdev,
+						     struct in6_addr *addr)
+{
+	struct lowpan_context_table *ctx_table = NULL;
+	struct lowpan_context_entry *entry = NULL;
+
+	if (!netdev || !addr) {
+		LOWPAN_DBG("Network device and address has to be specified");
+		return NULL;
+	}
+
+	/* Get proper interface table. */
+	ctx_table = get_table(netdev);
+	if (!ctx_table) {
+		LOWPAN_DBG("Cannot find context table for interface %s",
+			   netdev->name);
+		return NULL;
+	}
+
+	/* Search for given address. */
+	entry = get_ctx_by_addr(ctx_table, addr);
+	if (!entry) {
+		LOWPAN_DBG("No context for address %pI6c for interface %s",
+			   addr, netdev->name);
+		return NULL;
+	}
+
+	LOWPAN_DBG("Found context of id:%d", entry->cid);
+
+	/* Return null in case no compression capability in found context */
+	return entry;
+}
+EXPORT_SYMBOL_GPL(lowpan_context_compress);
+
+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
+};
+
+void lowpan_context_init(void)
+{
+	lowpan_context_debugfs = debugfs_create_file("context", 0664,
+						     lowpan_debugfs, NULL,
+						     &lowpan_context_fops);
+}
+EXPORT_SYMBOL_GPL(lowpan_context_init);
+
+void lowpan_context_uninit(void)
+{
+	struct lowpan_context_table *entry = NULL;
+
+	spin_lock(&module_lock);
+
+	list_for_each_entry(entry, &context_tables, list)
+		table_free(entry);
+
+	spin_unlock(&module_lock);
+
+	debugfs_remove(lowpan_context_debugfs);
+}
+EXPORT_SYMBOL_GPL(lowpan_context_uninit);
diff --git a/net/6lowpan/context.h b/net/6lowpan/context.h
new file mode 100644
index 0000000..88303b5
--- /dev/null
+++ b/net/6lowpan/context.h
@@ -0,0 +1,7 @@
+#ifndef __6LOWPAN_CONTEXT_H
+#define __6LOWPAN_CONTEXT_H
+
+void lowpan_context_init(void);
+void lowpan_context_uninit(void);
+
+#endif /* __6LOWPAN_CONTEXT_H */
-- 
2.1.4


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

* [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
  2015-07-13 11:50 ` [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module Lukasz Duda
  2015-07-13 11:50 ` [RFC v2 2/4] 6lowpan: Add stateful compression component of " Lukasz Duda
@ 2015-07-13 11:50 ` Lukasz Duda
  2015-07-23  7:48   ` Alexander Aring
  2015-07-13 11:50 ` [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan Lukasz Duda
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Lukasz Duda @ 2015-07-13 11:50 UTC (permalink / raw)
  To: alex.aring; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

Make use of stateful compression implemented in context.c.
The code does not impact current stateless compression.

Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
---
 net/6lowpan/iphc.c | 232 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 196 insertions(+), 36 deletions(-)

diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index c845a8a..48267e9 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -56,10 +56,63 @@
 #include <net/af_ieee802154.h>
 
 #include "nhc.h"
+#include "context.h"
+
+#define CTX_SRC(context)  ((context & 0xf0) >> 4)
+#define CTX_DEST(context) (context & 0x0f)
+#define CID_NOT_USED      0xff
 
 struct dentry *lowpan_debugfs;
 EXPORT_SYMBOL_GPL(lowpan_debugfs);
 
+/* Add context prefix into given IPv6 address.
+ * Context prefix is always used, therefore any original bits can be
+ * overwritten.
+ */
+static inline void add_cid_prefix(struct in6_addr *ipaddr,
+				  struct lowpan_context_entry *ctx_entry)
+{
+	int o = ctx_entry->prefix_len >> 3;
+	int b = ctx_entry->prefix_len & 0x7;
+
+	memcpy(ipaddr->s6_addr, ctx_entry->prefix.s6_addr, o);
+
+	if (b != 0)
+		ipaddr->s6_addr[o] = ctx_entry->prefix.s6_addr[o] &
+				     (0xff00 >> b);
+}
+
+/* Check if address can be fully compressed with stateful compression. */
+static inline bool is_iid_compressable(struct in6_addr *ipaddr,
+				       unsigned int ctx_length,
+				       u8 *lladdr)
+{
+	unsigned int start_byte, offset;
+
+	/* Context covers IPv6 address by its size. */
+	if (ctx_length == 128)
+		return true;
+
+	/* Check if IID can be retrieved when prefix is longer than 64 bits. */
+	if (ctx_length > 64) {
+		/* Check only IID bytes that are not covered by context. */
+		start_byte = ctx_length >> 3;
+		offset = ctx_length % 8;
+
+		/* Check all bytes from the second one. */
+		if (start_byte == 15 ||
+		    !memcmp(&ipaddr->s6_addr[start_byte + 1],
+			    &lladdr[start_byte - 7],
+			    15 - start_byte)) {
+			/* Then check first byte. */
+			return (u8)(ipaddr->s6_addr[start_byte] << offset) ==
+			       (u8)(lladdr[start_byte - 8] << offset);
+		}
+	}
+
+	return false;
+}
+
 /* Uncompress address function for source and
  * destination address(non-multicast).
  *
@@ -141,34 +194,82 @@ static int uncompress_addr(struct sk_buff *skb,
 	return 0;
 }
 
-/* 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,
-					     const u8 sam)
+/* Uncompress address after stateful compression. */
+static int uncompress_context_based_addr(struct sk_buff *skb,
+					 struct net_device *dev,
+					 struct in6_addr *ipaddr,
+					 const u8 address_mode,
+					 const u8 *lladdr,
+					 const u8 addr_type,
+					 const u8 addr_len,
+					 const u8 context_id)
 {
-	switch (sam) {
-	case LOWPAN_IPHC_ADDR_00:
-		/* unspec address ::
+	bool fail = false;
+	struct lowpan_context_entry *ctx_entry;
+
+	if (address_mode == LOWPAN_IPHC_ADDR_00)
+		/* Unspecified address ::
 		 * Do nothing, address is already ::
 		 */
-		break;
+		return 0;
+
+	/* Look for prefix assigned to given context id. */
+	ctx_entry = lowpan_context_decompress(dev, context_id);
+	if (!ctx_entry) {
+		pr_debug("Unknown Context. Unable to decompress\n");
+		return -EINVAL;
+	}
+
+	switch (address_mode) {
 	case LOWPAN_IPHC_ADDR_01:
-		/* TODO */
+		/* CID Prefix + 64-bits in-line. */
+		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[8], 8);
+	break;
 	case LOWPAN_IPHC_ADDR_02:
-		/* TODO */
+		/* CID Prefix + 16-bits in-line. */
+		ipaddr->s6_addr[11] = 0xFF;
+		ipaddr->s6_addr[12] = 0xFE;
+		fail = lowpan_fetch_skb(skb, &ipaddr->s6_addr[14], 2);
+	break;
 	case LOWPAN_IPHC_ADDR_03:
-		/* TODO */
-		netdev_warn(skb->dev, "SAM value 0x%x not supported\n", sam);
-		return -EINVAL;
+		switch (addr_type) {
+		case IEEE802154_ADDR_LONG:
+			/* CID Prefix + 64-bits from LL Address. */
+			memcpy(&ipaddr->s6_addr[8],
+			       lladdr,
+			       addr_len);
+
+			/* Second bit-flip (Universe/Local)
+			* is done according RFC2464
+			*/
+			ipaddr->s6_addr[8] ^= 0x02;
+			break;
+		case IEEE802154_ADDR_SHORT:
+			/* CID Prefix + 16-bits from LL Address. */
+			ipaddr->s6_addr[11] = 0xFF;
+			ipaddr->s6_addr[12] = 0xFE;
+			ipaddr->s6_addr16[7] = htons(*((u16 *)lladdr));
+			break;
+		default:
+			pr_debug("Invalid addr_type set\n");
+			return -EINVAL;
+		}
+	break;
 	default:
-		pr_debug("Invalid sam value: 0x%x\n", sam);
+		pr_debug("Invalid address mode value: 0x%x\n",
+			 address_mode);
 		return -EINVAL;
 	}
 
-	raw_dump_inline(NULL,
-			"Reconstructed context based ipv6 src addr is",
+	if (fail) {
+		pr_debug("Failed to fetch skb data\n");
+		return -EIO;
+	}
+
+	/* Add context prefix to IPv6 address. Context bits are always used. */
+	add_cid_prefix(ipaddr, ctx_entry);
+
+	raw_dump_inline(NULL, "Reconstructed ipv6 addr is",
 			ipaddr->s6_addr, 16);
 
 	return 0;
@@ -323,11 +424,14 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
 
 	if (iphc1 & LOWPAN_IPHC_SAC) {
 		/* Source address context based uncompression */
-		pr_debug("SAC bit is set. Handle context based source address.\n");
-		err = uncompress_context_based_src_addr(skb, &hdr.saddr, tmp);
+		pr_debug("source: address stateful compression.\n");
+		err = uncompress_context_based_addr(skb, dev, &hdr.saddr,
+						    tmp, saddr, saddr_type,
+						    saddr_len,
+						    CTX_SRC(num_context));
 	} else {
 		/* Source address uncompression */
-		pr_debug("source address stateless compression\n");
+		pr_debug("source: address stateless compression\n");
 		err = uncompress_addr(skb, &hdr.saddr, tmp, saddr,
 				      saddr_type, saddr_len);
 	}
@@ -352,10 +456,19 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
 				return -EINVAL;
 		}
 	} else {
-		err = uncompress_addr(skb, &hdr.daddr, tmp, daddr,
-				      daddr_type, daddr_len);
-		pr_debug("dest: stateless compression mode %d dest %pI6c\n",
-			 tmp, &hdr.daddr);
+		if (iphc1 & LOWPAN_IPHC_DAC) {
+			/* Destination address context based uncompression */
+			pr_debug("dest: address steteful compression.\n");
+			err = uncompress_context_based_addr(
+				skb, dev, &hdr.daddr, tmp, daddr, daddr_type,
+				daddr_len, CTX_DEST(num_context));
+		} else {
+			/* Destination address uncompression */
+			pr_debug("dest: address stateless compression\n");
+			err = uncompress_addr(skb, &hdr.daddr, tmp, daddr,
+					      daddr_type, daddr_len);
+		}
+
 		if (err)
 			return -EINVAL;
 	}
@@ -393,11 +506,12 @@ EXPORT_SYMBOL_GPL(lowpan_header_decompress);
 
 static u8 lowpan_compress_addr_64(u8 **hc_ptr, u8 shift,
 				  const struct in6_addr *ipaddr,
-				  const unsigned char *lladdr)
+				  const unsigned char *lladdr,
+				  bool cover_by_context)
 {
 	u8 val = 0;
 
-	if (is_addr_mac_addr_based(ipaddr, lladdr)) {
+	if (is_addr_mac_addr_based(ipaddr, lladdr) || cover_by_context) {
 		val = 3; /* 0-bits */
 		pr_debug("address compression 0 bits\n");
 	} else if (lowpan_is_iid_16_bit_compressable(ipaddr)) {
@@ -422,9 +536,15 @@ int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
 			   const void *_saddr, unsigned int len)
 {
 	u8 tmp, iphc0, iphc1, *hc_ptr;
+	u8 cid = 0;
 	struct ipv6hdr *hdr;
 	u8 head[100] = {};
 	int ret, addr_type;
+	struct lowpan_context_entry *entry;
+	bool scid_used = false;
+	bool dcid_used = false;
+	bool scid_cover = false;
+	bool dcid_cover = false;
 
 	if (type != ETH_P_IPV6)
 		return -EINVAL;
@@ -448,7 +568,26 @@ int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
 	iphc0 = LOWPAN_DISPATCH_IPHC;
 	iphc1 = 0;
 
-	/* TODO: context lookup */
+	/* Look for source context. */
+	entry = lowpan_context_compress(dev, &hdr->saddr);
+	if (entry) {
+		cid = entry->cid << 4;
+		scid_used = true;
+	}
+
+	/* Look for destination context. */
+	entry = lowpan_context_compress(dev, &hdr->daddr);
+	if (entry) {
+		cid |= entry->cid;
+		dcid_used = true;
+	}
+
+	if (scid_used || dcid_used) {
+		/* Add CID flag. */
+		iphc1 |= LOWPAN_IPHC_CID;
+
+		lowpan_push_hc_data(&hc_ptr, &cid, 1);
+	}
 
 	raw_dump_inline(__func__, "saddr",
 			(unsigned char *)_saddr, IEEE802154_ADDR_LEN);
@@ -535,11 +674,19 @@ int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
 		pr_debug("source address is unspecified, setting SAC\n");
 		iphc1 |= LOWPAN_IPHC_SAC;
 	} else {
-		if (addr_type & IPV6_ADDR_LINKLOCAL) {
+		if ((addr_type & IPV6_ADDR_LINKLOCAL) || scid_used) {
+			if (scid_used) {
+				iphc1 |= LOWPAN_IPHC_SAC;
+				scid_cover = is_iid_compressable(
+						&hdr->saddr, entry->prefix_len,
+						(u8 *)_saddr);
+			}
+
 			iphc1 |= lowpan_compress_addr_64(&hc_ptr,
 							 LOWPAN_IPHC_SAM_BIT,
-							 &hdr->saddr, _saddr);
-			pr_debug("source address unicast link-local %pI6c iphc1 0x%02x\n",
+							 &hdr->saddr, _saddr,
+							 scid_cover);
+			pr_debug("source ipv6 address %pI6c iphc1 0x%02x\n",
 				 &hdr->saddr, iphc1);
 		} else {
 			pr_debug("send the full source address\n");
@@ -580,12 +727,20 @@ int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
 			lowpan_push_hc_data(&hc_ptr, hdr->daddr.s6_addr, 16);
 		}
 	} else {
-		if (addr_type & IPV6_ADDR_LINKLOCAL) {
-			/* TODO: context lookup */
+		if (addr_type & IPV6_ADDR_LINKLOCAL || dcid_used) {
+			if (dcid_used) {
+				iphc1 |= LOWPAN_IPHC_DAC;
+				dcid_cover = is_iid_compressable(
+						&hdr->daddr, entry->prefix_len,
+						(u8 *)_daddr);
+			}
+
 			iphc1 |= lowpan_compress_addr_64(&hc_ptr,
-				LOWPAN_IPHC_DAM_BIT, &hdr->daddr, _daddr);
-			pr_debug("dest address unicast link-local %pI6c "
-				 "iphc1 0x%02x\n", &hdr->daddr, iphc1);
+							 LOWPAN_IPHC_DAM_BIT,
+							 &hdr->daddr, _daddr,
+							 dcid_cover);
+			pr_debug("dest address %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);
@@ -619,6 +774,9 @@ static int __init lowpan_module_init(void)
 {
 	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
 
+	/* Initialize stateful compression. */
+	lowpan_context_init();
+
 	request_module_nowait("ipv6");
 
 	request_module_nowait("nhc_dest");
@@ -635,6 +793,8 @@ static int __init lowpan_module_init(void)
 static void __exit lowpan_module_exit(void)
 {
 	debugfs_remove(lowpan_debugfs);
+
+	lowpan_context_uninit();
 }
 
 module_init(lowpan_module_init);
-- 
2.1.4


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

* [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
                   ` (2 preceding siblings ...)
  2015-07-13 11:50 ` [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c Lukasz Duda
@ 2015-07-13 11:50 ` Lukasz Duda
  2015-07-23  7:55   ` Alexander Aring
  2015-07-13 13:09 ` [RFC v2 0/4] Adding stateful compression to IPHC Jukka Rissanen
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Lukasz Duda @ 2015-07-13 11:50 UTC (permalink / raw)
  To: alex.aring; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

Allocating context table for stateful compression when interface is set
up. Removing context table when interface is being unregistered.

Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
---
 net/bluetooth/6lowpan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 2fb7b30..60c7e80 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -871,6 +871,9 @@ static int setup_netdev(struct l2cap_chan *chan, struct lowpan_dev **dev)
 	       &chan->src, chan->src_type);
 	set_bit(__LINK_STATE_PRESENT, &netdev->state);
 
+	/* Allocate context table for stateful compression. */
+	lowpan_context_table_alloc(netdev);
+
 	*dev = netdev_priv(netdev);
 	(*dev)->netdev = netdev;
 	(*dev)->hdev = chan->conn->hcon->hdev;
@@ -1415,6 +1418,7 @@ static int device_event(struct notifier_block *unused,
 			if (entry->netdev == netdev) {
 				BT_DBG("Unregistered netdev %s %p",
 				       netdev->name, netdev);
+				lowpan_context_table_free(netdev);
 				list_del(&entry->list);
 				break;
 			}
-- 
2.1.4


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

* Re: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
                   ` (3 preceding siblings ...)
  2015-07-13 11:50 ` [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan Lukasz Duda
@ 2015-07-13 13:09 ` Jukka Rissanen
  2015-07-15 13:42   ` Duda, Lukasz
  2015-07-13 14:15 ` Michael Richardson
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Jukka Rissanen @ 2015-07-13 13:09 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: alex.aring, linux-wpan, linux-bluetooth

Hi Lukasz,

On ma, 2015-07-13 at 13:50 +0200, Lukasz Duda wrote:
> Hi all,
> 
> Resending the RFC patch set after some clean up.
> 
> The following patches introduce support for stateful compression in IPHC.
> 
> Patch #1: Introduce debugfs entry for 6lowpan module
> Patch #2: Add stateful compression component of 6lowpan module
> Patch #3: Add stateful compression support for iphc.c
> Patch #4: Enable stateful compression in bluetooth_6lowpan
> 
> Usage of stateful compression is described in Patch #2.
> 
> Notes
> =====
> 
> RFC v2:
>     - Splitting patch into smaller incremental patches
>     - Moving debugfs logic from iphc.c into context.c
>     - Updating patch set to align with style and coding guideline
>     
> RFC v1:
>     - Initial patch set: http://www.spinics.net/lists/linux-bluetooth/msg62930.html
> 
> Limitations
> ===========
> 
> - Temporarly use of debugfs to be able to add context table entries.
> - Current module does not support context time-outs.
> - No support for multicast addresses stateful compression.
> 
> Discussion: Idea on how to make the 6lowpan context tables out of debug mode
> ============================================================================
> 
> The patch provided here is just a temporary solution where the contexts are
> manually added. The proper way of adding/removing contexts would be to make
> ndisc.c parse 6CO options and act upon it. Generated Router Advertisement
> packets with 6CO option (for example from RADVD) will be handled in the
> ndisc_router_discovery function.
> 
> Also, other flags like ARO, ABRO etc. which are specified in RFC6775 need
> to be handled in the ipv6 module in order to do it in a neat way.
> Potentially the RFC6775 extensions could also be used by other protocols
> than BLE and 802.15.4.
> 
> There is a need for parsing the 6CO option in ndisc.c which parses Router
> Advertisement messages. Today the function calls to IPHC (6lowpan module) are
> quite hard to implement in ndisc.c, as there is no guarantee that the 6lowpan
> module will be loaded or not. It would make sense to build 6lowpan module
> as in-build part of IPv6. The features could be compiled in by using #defines.
> 
> What do you think about moving 6lowpan as a component of IPv6 and modify ndisc.c
> to handled 6LoWPAN specific options? Do you see any other solution to make sure
> that 6CO is parsed and acted upon and still keep 6lowpan as a stand-alone module?

Have you considered that 6lowpan module could register a callback in
ipv6 module. When needed, ipv6 could then call the 6lowpan callback if
needed. This way there would be no need to embed 6lowpan functionality
to ipv6 module.


Cheers,
Jukka



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

* Re: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
                   ` (4 preceding siblings ...)
  2015-07-13 13:09 ` [RFC v2 0/4] Adding stateful compression to IPHC Jukka Rissanen
@ 2015-07-13 14:15 ` Michael Richardson
  2015-07-15  9:55 ` Duda, Lukasz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Michael Richardson @ 2015-07-13 14:15 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: alex.aring, linux-wpan, linux-bluetooth


Lukasz Duda <lukasz.duda@nordicsemi.no> wrote:
    > There is a need for parsing the 6CO option in ndisc.c which parses Router
    > Advertisement messages. Today the function calls to IPHC (6lowpan module) are
    > quite hard to implement in ndisc.c, as there is no guarantee that the 6lowpan
    > module will be loaded or not. It would make sense to build 6lowpan module
    > as in-build part of IPv6. The features could be compiled in by using #defines.

    > What do you think about moving 6lowpan as a component of IPv6 and modify ndisc.c
    > to handled 6LoWPAN specific options? Do you see any other solution to make sure
    > that 6CO is parsed and acted upon and still keep 6lowpan as a stand-alone module?

What about if ndisc.c accepted non-exclusive registrations for ND type codes?
Then 6lowpan could register itself as handler for RAs, and could process the
6CO options, ABROs, etc.
(BTW: 6tisch contemplates extending ABROs as well: there is more to come)

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mcr@sandelman.ca  http://www.sandelman.ca/        |   ruby on rails    [



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

* RE: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
                   ` (5 preceding siblings ...)
  2015-07-13 14:15 ` Michael Richardson
@ 2015-07-15  9:55 ` Duda, Lukasz
  2015-07-23  8:22 ` Alexander Aring
  2015-10-12 16:15 ` Alexander Aring
  8 siblings, 0 replies; 20+ messages in thread
From: Duda, Lukasz @ 2015-07-15  9:55 UTC (permalink / raw)
  To: marcel@holtmann.org
  Cc: linux-wpan@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	alex.aring@gmail.com

Hi Marcel,

Marcel Holtmann <marcel@holtmann.org> wrote:
> I am not sure that 6LoWPAN long-term should be a compile time option of
> IPv6. However that is certainly a possibility. If we want to discuss that, then it
> needs to be a discussion on the netdev mailing list with the netdev folks.
> 
> Initially the direction was to go for net/6lowpan/ and make 6LoWPAN a
> subsystem of its own. Making it part of IPv6 would mean this has to move into
> net/core/ and that needs convincing of the netdev folks.
> 
> My viewpoint of 6LoWPAN subsystem was always that besides compression, it
> actually becomes a full net_device abstraction. So instead of BLE and 802.15.4
> allocating net_device on its own, we have something like lowpan_netdev that
> gets allocated by each subsystem. And then you just provide the framing
> functions that take the encapsulated frames and transport it over the bearer.
> Right now we are doing a lot of duplication in each subsystem to support
> 6LoWPAN. The less it takes to hook up 6LoWPAN to new bearers, the better.
> Moving the compression into a separate module was just the first step.
> 
> Regards
> 
> Marcel

Thanks a lot for your feedback and valuable comments. 

I hope it is okay that I am responding using the new mail thread.
As you might have noticed I have resubmitted the RFC patch after
addressing your comments in a new thread, and split the patch
into smaller parts.

The idea of making an abstraction of lowpan_dev sounds like a good idea,
but I'm not sure if I'm grasping the details involved. Could you please
elaborate a bit more on you proposal?

Is the structure like below something you had in mind for lowpan_dev?
struct lowpan_dev { // Struct build-in /net/core like wpan_dev
    net_device * net_dev;   // Pointer to net device
    FUNC_PTR tx_xmit;       // Transmit function in the subsystem
                               (assigned on the registration of a 
                               lowpan_dev)
    FUNC_PTR decompress;    // Is called after receive of new data from
                               the subsystem. But how to deliver such data,
                               for example by creating lowpan_dev_rx() API?
    FUNC_PTR compress;      // Is called when net_device sends packet to
                               lowpan_dev (xmit).
    FUNC_PTR handle_ra;     // Is called when ndisc.c finds lowpan options
                               e.g. 6CO.
}

struct net_device {
    ...
    lowpan_dev  * lowpan_dev_ptr;
    ...
}

For now 6CO is the only flag that I am currently interested in, but with
such structure, it could easily be extended to handle other message types.

Having the pointers to compress/decompress and to act upon lowpan options,
allow us to keep 6lowpan as a module in /net/6lowpan. Could this work? 
Or you are proposing to completely move IP Header compression mechanism to
lowpan_dev implementation (built-in - /net/core)?

Best regards,
Łukasz Duda

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

* RE: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-13 13:09 ` [RFC v2 0/4] Adding stateful compression to IPHC Jukka Rissanen
@ 2015-07-15 13:42   ` Duda, Lukasz
  2015-07-16  7:25     ` Jukka Rissanen
  0 siblings, 1 reply; 20+ messages in thread
From: Duda, Lukasz @ 2015-07-15 13:42 UTC (permalink / raw)
  To: Jukka Rissanen
  Cc: alex.aring@gmail.com, linux-wpan@vger.kernel.org,
	linux-bluetooth@vger.kernel.org

Hi Jukka,

> Hi Lukasz,
> 
> On ma, 2015-07-13 at 13:50 +0200, Lukasz Duda wrote:
> > Hi all,
> >
> > Resending the RFC patch set after some clean up.
> >
> > The following patches introduce support for stateful compression in IPHC.
> >
> > Patch #1: Introduce debugfs entry for 6lowpan module
> > Patch #2: Add stateful compression component of 6lowpan module
> > Patch #3: Add stateful compression support for iphc.c
> > Patch #4: Enable stateful compression in bluetooth_6lowpan
> >
> > Usage of stateful compression is described in Patch #2.
> >
> > Notes
> > =====
> >
> > RFC v2:
> >     - Splitting patch into smaller incremental patches
> >     - Moving debugfs logic from iphc.c into context.c
> >     - Updating patch set to align with style and coding guideline
> >
> > RFC v1:
> >     - Initial patch set: http://www.spinics.net/lists/linux-
> bluetooth/msg62930.html
> >
> > Limitations
> > ===========
> >
> > - Temporarly use of debugfs to be able to add context table entries.
> > - Current module does not support context time-outs.
> > - No support for multicast addresses stateful compression.
> >
> > Discussion: Idea on how to make the 6lowpan context tables out of debug
> mode
> >
> ============================================================
> ================
> >
> > The patch provided here is just a temporary solution where the contexts are
> > manually added. The proper way of adding/removing contexts would be to
> make
> > ndisc.c parse 6CO options and act upon it. Generated Router Advertisement
> > packets with 6CO option (for example from RADVD) will be handled in the
> > ndisc_router_discovery function.
> >
> > Also, other flags like ARO, ABRO etc. which are specified in RFC6775 need
> > to be handled in the ipv6 module in order to do it in a neat way.
> > Potentially the RFC6775 extensions could also be used by other protocols
> > than BLE and 802.15.4.
> >
> > There is a need for parsing the 6CO option in ndisc.c which parses Router
> > Advertisement messages. Today the function calls to IPHC (6lowpan module)
> are
> > quite hard to implement in ndisc.c, as there is no guarantee that the
> 6lowpan
> > module will be loaded or not. It would make sense to build 6lowpan module
> > as in-build part of IPv6. The features could be compiled in by using #defines.
> >
> > What do you think about moving 6lowpan as a component of IPv6 and
> modify ndisc.c
> > to handled 6LoWPAN specific options? Do you see any other solution to
> make sure
> > that 6CO is parsed and acted upon and still keep 6lowpan as a stand-alone
> module?
> 
> Have you considered that 6lowpan module could register a callback in
> ipv6 module. When needed, ipv6 could then call the 6lowpan callback if
> needed. This way there would be no need to embed 6lowpan functionality
> to ipv6 module.
> 
> 
> Cheers,
> Jukka
> 

I did consider that solution but I assume you think about the way of 
registration  used for NHC callbacks (for example nhc_udp.c) in the 6lowpan
module.

This could imply a new API for ndisc.c for example
ndisc_register_lowpan_callback(). This callback can be issued from the
6lowpan module in order to register a callback.
When an RA message comes, the registered callback will be issued.
Should this be called only when a specific option is detected, or for all RA
messages in general?

How do you see a way of handling all RFC6775 options like ARO,DAR,DAC etc.
Should this be extended in ndisc.c itself, or should it be handled within
the 6lowpan module (e.g. by using this registered lowpan callback). 
Because when receiving NS with ARO it is expected to answer with NA that
contains ARO option with reply status.

I believe that using callbacks like this could be used for parsing 6CO 
very well which does not need any interaction. However, would there be 
any chance of getting such dynamic registration of callbacks into ipv6 
module? Is there any example that implements such solution to see how
it works.

Best regards,
Łukasz Duda

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

* Re: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-15 13:42   ` Duda, Lukasz
@ 2015-07-16  7:25     ` Jukka Rissanen
  0 siblings, 0 replies; 20+ messages in thread
From: Jukka Rissanen @ 2015-07-16  7:25 UTC (permalink / raw)
  To: Duda, Lukasz
  Cc: alex.aring@gmail.com, linux-wpan@vger.kernel.org,
	linux-bluetooth@vger.kernel.org

Hi Lukasz,

On ke, 2015-07-15 at 13:42 +0000, Duda, Lukasz wrote:
> Hi Jukka,
> 
> > Hi Lukasz,
> > 
> > On ma, 2015-07-13 at 13:50 +0200, Lukasz Duda wrote:
> > > Hi all,
> > >
> > > Resending the RFC patch set after some clean up.
> > >
> > > The following patches introduce support for stateful compression in IPHC.
> > >
> > > Patch #1: Introduce debugfs entry for 6lowpan module
> > > Patch #2: Add stateful compression component of 6lowpan module
> > > Patch #3: Add stateful compression support for iphc.c
> > > Patch #4: Enable stateful compression in bluetooth_6lowpan
> > >
> > > Usage of stateful compression is described in Patch #2.
> > >
> > > Notes
> > > =====
> > >
> > > RFC v2:
> > >     - Splitting patch into smaller incremental patches
> > >     - Moving debugfs logic from iphc.c into context.c
> > >     - Updating patch set to align with style and coding guideline
> > >
> > > RFC v1:
> > >     - Initial patch set: http://www.spinics.net/lists/linux-
> > bluetooth/msg62930.html
> > >
> > > Limitations
> > > ===========
> > >
> > > - Temporarly use of debugfs to be able to add context table entries.
> > > - Current module does not support context time-outs.
> > > - No support for multicast addresses stateful compression.
> > >
> > > Discussion: Idea on how to make the 6lowpan context tables out of debug
> > mode
> > >
> > ============================================================
> > ================
> > >
> > > The patch provided here is just a temporary solution where the contexts are
> > > manually added. The proper way of adding/removing contexts would be to
> > make
> > > ndisc.c parse 6CO options and act upon it. Generated Router Advertisement
> > > packets with 6CO option (for example from RADVD) will be handled in the
> > > ndisc_router_discovery function.
> > >
> > > Also, other flags like ARO, ABRO etc. which are specified in RFC6775 need
> > > to be handled in the ipv6 module in order to do it in a neat way.
> > > Potentially the RFC6775 extensions could also be used by other protocols
> > > than BLE and 802.15.4.
> > >
> > > There is a need for parsing the 6CO option in ndisc.c which parses Router
> > > Advertisement messages. Today the function calls to IPHC (6lowpan module)
> > are
> > > quite hard to implement in ndisc.c, as there is no guarantee that the
> > 6lowpan
> > > module will be loaded or not. It would make sense to build 6lowpan module
> > > as in-build part of IPv6. The features could be compiled in by using #defines.
> > >
> > > What do you think about moving 6lowpan as a component of IPv6 and
> > modify ndisc.c
> > > to handled 6LoWPAN specific options? Do you see any other solution to
> > make sure
> > > that 6CO is parsed and acted upon and still keep 6lowpan as a stand-alone
> > module?
> > 
> > Have you considered that 6lowpan module could register a callback in
> > ipv6 module. When needed, ipv6 could then call the 6lowpan callback if
> > needed. This way there would be no need to embed 6lowpan functionality
> > to ipv6 module.
> > 
> > 
> > Cheers,
> > Jukka
> > 
> 
> I did consider that solution but I assume you think about the way of 
> registration  used for NHC callbacks (for example nhc_udp.c) in the 6lowpan
> module.

I was thinking more something like the notifier API. Perhaps it can be
utilized to send information about received ndisc events.

So in 6lowpan module you would just do

static struct notifier_block ndisc_notifier = {
	.notifier_call = ndisc_event,
};

and register the notifier handler, and then in ndisc.c:ndisc_rcv() could
then just send information about RAs etc.

> 
> This could imply a new API for ndisc.c for example
> ndisc_register_lowpan_callback(). This callback can be issued from the
> 6lowpan module in order to register a callback.
> When an RA message comes, the registered callback will be issued.
> Should this be called only when a specific option is detected, or for all RA
> messages in general?

Perhaps more general solution would be preferable instead of being
specific to 6lowpan needs.

> 
> How do you see a way of handling all RFC6775 options like ARO,DAR,DAC etc.
> Should this be extended in ndisc.c itself, or should it be handled within
> the 6lowpan module (e.g. by using this registered lowpan callback). 
> Because when receiving NS with ARO it is expected to answer with NA that
> contains ARO option with reply status.

If the options are specific to 6lowpan, then I would say the 6lowpan
module needs to deal with them.

> 
> I believe that using callbacks like this could be used for parsing 6CO 
> very well which does not need any interaction. However, would there be 
> any chance of getting such dynamic registration of callbacks into ipv6 
> module? Is there any example that implements such solution to see how
> it works.

I do not know and we are just pondering different ideas here. Could you
ask the gurus in netdev mailing list their opinion/guidance?


Cheers,
Jukka



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

* Re: [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module
  2015-07-13 11:50 ` [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module Lukasz Duda
@ 2015-07-23  6:07   ` Alexander Aring
  0 siblings, 0 replies; 20+ messages in thread
From: Alexander Aring @ 2015-07-23  6:07 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

Hi,

I will try add review notes to your patch series at first.

On Mon, Jul 13, 2015 at 01:50:30PM +0200, Lukasz Duda wrote:
> Creating "6lowpan" debugfs entry (/sys/kernel/debugfs/6lowpan) for usage
> by 6lowpan module.
> 
> Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
> Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
> ---
>  include/net/6lowpan.h |  2 ++
>  net/6lowpan/iphc.c    | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index dc03d77..37ddbdf 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -197,6 +197,8 @@
>  #define LOWPAN_NHC_UDP_CS_P_11	0xF3 /* source & dest = 0xF0B + 4bit inline */
>  #define LOWPAN_NHC_UDP_CS_C	0x04 /* checksum elided */
>  
> +extern struct dentry *lowpan_debugfs;
> +
>  #ifdef DEBUG
>  /* print data in line */
>  static inline void raw_dump_inline(const char *caller, char *msg,
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 9055d7b..c845a8a 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -50,12 +50,16 @@
>  #include <linux/if_arp.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/debugfs.h>
>  #include <net/6lowpan.h>
>  #include <net/ipv6.h>
>  #include <net/af_ieee802154.h>
>  
>  #include "nhc.h"
>  
> +struct dentry *lowpan_debugfs;
> +EXPORT_SYMBOL_GPL(lowpan_debugfs);
> +
>  /* Uncompress address function for source and
>   * destination address(non-multicast).
>   *
> @@ -613,6 +617,8 @@ EXPORT_SYMBOL_GPL(lowpan_header_compress);
>  
>  static int __init lowpan_module_init(void)
>  {
> +	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
> +

add error handling here please.

>  	request_module_nowait("ipv6");
>  
>  	request_module_nowait("nhc_dest");
> @@ -625,6 +631,13 @@ static int __init lowpan_module_init(void)
>  
>  	return 0;
>  }
> +
> +static void __exit lowpan_module_exit(void)
> +{
> +	debugfs_remove(lowpan_debugfs);
> +}
> +
>  module_init(lowpan_module_init);
> +module_exit(lowpan_module_exit);
>  
>  MODULE_LICENSE("GPL");
> -- 
> 2.1.4
> 

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

* Re: [RFC v2 2/4] 6lowpan: Add stateful compression component of 6lowpan module
  2015-07-13 11:50 ` [RFC v2 2/4] 6lowpan: Add stateful compression component of " Lukasz Duda
@ 2015-07-23  6:52   ` Alexander Aring
  2015-07-23  6:59     ` Alexander Aring
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Aring @ 2015-07-23  6:52 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

Hi,

On Mon, Jul 13, 2015 at 01:50:31PM +0200, Lukasz Duda wrote:
> The patch adds stateful compression (context ids based compression) to
> the 6lowpan module [RFC6282].
> 
> Stateful Compression users allocate context tables through the functions
> in the context.c using public APIs declared in 6lowpan.h. The context.c
> is extending the 6lowpan module and not created as a separate module.
> 
> A debugfs entry for maintaining the context tables is implemented
> (/sys/kernel/debug/6lowpan/context).
> 
> Example usage of the 6lowpan debugfs interface:
> 
> echo "<CMD> <IFACE> <CID> <CID_PREFIX> <CID_PREFIX_LENGTH>
> <COMPRESSION_ENABLE_FLAG>" > /sys/kernel/debug/6lowpan/context
> 
> Examples:
> 
> echo "add bt0 3 2001:db8:: 64 1" > /sys/kernel/debug/6lowpan/context
> 
> echo "remove bt0 3" > /sys/kernel/debug/6lowpan/context
> 
> cat /sys/kernel/debug/6lowpan/context
> 
> e.g.
> Context table of interface bt0
> 
> CID Prefix      Len CF
> 3   2001:db8::  64  1
> 4   2001:db8::1 128 1
> 

I tried to test it and I got a:

[  132.092412] =================================
[  132.096976] [ INFO: inconsistent lock state ]
[  132.101545] 4.1.0-268283-gca74796 #9 Tainted: G        W      
[  132.107650] ---------------------------------
[  132.112214] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[  132.118506] sh/1254 [HC0[0]:SC0[0]:HE1:SE1] takes:
[  132.123525]  (module_lock){+.?...}, at: [<bf07e6e4>] get_table+0x14/0x5c [6lowpan]
[  132.131536] {IN-SOFTIRQ-W} state was registered at:
[  132.136645]   [<c06c1d30>] _raw_spin_lock+0x30/0x40
[  132.141785]   [<bf07e6e4>] get_table+0x14/0x5c [6lowpan]
[  132.147372]   [<bf07f418>] lowpan_context_compress+0x24/0x1d8 [6lowpan]
[  132.154324]   [<bf07d36c>] lowpan_header_compress+0x68/0x6c0 [6lowpan]
[  132.161175]   [<bf0a1648>] lowpan_header+0x84/0x250 [ieee802154_6lowpan]
[  132.168220]   [<bf0a1a98>] lowpan_xmit+0x68/0x344 [ieee802154_6lowpan]
[  132.175070]   [<c04fdeb8>] dev_hard_start_xmit+0x2a8/0x498
[  132.180829]   [<c04fe84c>] __dev_queue_xmit+0x660/0x798
[  132.186315]   [<bf0024c0>] ip6_finish_output2+0x320/0xbb0 [ipv6]
[  132.192845]   [<bf00805c>] ip6_output+0x6c/0x2c8 [ipv6]
[  132.198412]   [<bf02e880>] mld_sendpack+0x32c/0x89c [ipv6]
[  132.204318]   [<bf02f678>] mld_ifc_timer_expire+0x1d4/0x304 [ipv6]
[  132.210929]   [<c00aa9e0>] call_timer_fn+0x7c/0x180
[  132.216055]   [<c00aac54>] run_timer_softirq+0x170/0x2b4
[  132.221626]   [<c00440c8>] __do_softirq+0x138/0x340
[  132.226755]   [<c00445f0>] irq_exit+0xbc/0x130
[  132.231417]   [<c009a618>] __handle_domain_irq+0x6c/0xe0
[  132.237004]   [<c00094ac>] omap_intc_handle_irq+0xa8/0xb8
[  132.242673]   [<c06c2824>] __irq_svc+0x44/0x5c
[  132.247340]   [<c06bffa8>] __mutex_unlock_slowpath+0x10c/0x1a4
[  132.253462]   [<c06bffa8>] __mutex_unlock_slowpath+0x10c/0x1a4
[  132.259586]   [<c01556a8>] unlock_rename+0x18/0x40
[  132.264630]   [<c015c128>] SyS_renameat2+0x258/0x4d0
[  132.269844]   [<c015c3e4>] SyS_rename+0x28/0x30
[  132.274601]   [<c000f5e0>] ret_fast_syscall+0x0/0x54
[  132.279821] irq event stamp: 76337
[  132.283380] hardirqs last  enabled at (76337): [<c000f60c>] ret_fast_syscall+0x2c/0x54
[  132.291686] hardirqs last disabled at (76336): [<c000f5ec>] ret_fast_syscall+0xc/0x54
[  132.299896] softirqs last  enabled at (75714): [<c004422c>] __do_softirq+0x29c/0x340
[  132.308017] softirqs last disabled at (75709): [<c00445f0>] irq_exit+0xbc/0x130
[  132.315683] 
[  132.315683] other info that might help us debug this:
[  132.322515]  Possible unsafe locking scenario:
[  132.322515] 
[  132.328712]        CPU0
[  132.331274]        ----
[  132.333833]   lock(module_lock);
[  132.337228]   <Interrupt>
[  132.339970]     lock(module_lock);
[  132.343547] 
[  132.343547]  *** DEADLOCK ***
[  132.343547] 
[  132.349752] 1 lock held by sh/1254:
[  132.353401]  #0:  (sb_writers#9){.+.+.+}, at: [<c014cff0>] vfs_write+0x13c/0x164
[  132.361210] 
[  132.361210] stack backtrace:
[  132.365788] CPU: 0 PID: 1254 Comm: sh Tainted: G        W       4.1.0-268283-gca74796 #9
[  132.374257] Hardware name: Generic AM33XX (Flattened Device Tree)
[  132.380663] [<c00168c4>] (unwind_backtrace) from [<c00133c0>] (show_stack+0x10/0x14)
[  132.388777] [<c00133c0>] (show_stack) from [<c06ba0b8>] (dump_stack+0x84/0x9c)
[  132.396363] [<c06ba0b8>] (dump_stack) from [<c008beb0>] (print_usage_bug+0x1d8/0x2cc)
[  132.404572] [<c008beb0>] (print_usage_bug) from [<c008c180>] (mark_lock+0x1dc/0x6c4)
[  132.412690] [<c008c180>] (mark_lock) from [<c008d48c>] (__lock_acquire+0x860/0x2048)
[  132.420809] [<c008d48c>] (__lock_acquire) from [<c008f4fc>] (lock_acquire+0xa8/0x124)
[  132.429016] [<c008f4fc>] (lock_acquire) from [<c06c1d30>] (_raw_spin_lock+0x30/0x40)
[  132.437147] [<c06c1d30>] (_raw_spin_lock) from [<bf07e6e4>] (get_table+0x14/0x5c [6lowpan])
[  132.445922] [<bf07e6e4>] (get_table [6lowpan]) from [<bf07ebc0>] (lowpan_context_dbgfs_write+0x128/0x4ec [6lowpan])
[  132.456860] [<bf07ebc0>] (lowpan_context_dbgfs_write [6lowpan]) from [<c014c6a8>] (__vfs_write+0x20/0xd8)
[  132.466884] [<c014c6a8>] (__vfs_write) from [<c014cf44>] (vfs_write+0x90/0x164)
[  132.474540] [<c014cf44>] (vfs_write) from [<c014d76c>] (SyS_write+0x44/0x9c)
[  132.481931] [<c014d76c>] (SyS_write) from [<c000f5e0>] (ret_fast_syscall+0x0/0x54)

You should see the same by enable "CONFIG_PROVE_LOCKING". I think it's
because "spin_lock(&module_lock);" inside of "get_table" function. This
is called by lowpan_xmit which has a softirq context (I think the
experts says here bh (?bottom half?) are disabled). Anyway, you need to
call spin_lock_bh/spin_unlock_bh.

Linux device drivers says something similar:

"Finally, spin_lock_bh disables software interrupts before taking the
lock, but leaves hardware interrupts enabled." Without bh software
interrupts are still enabled.

> Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
> Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
> ---
>  include/net/6lowpan.h |  35 ++++
>  net/6lowpan/Makefile  |   2 +-
>  net/6lowpan/context.c | 551 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/6lowpan/context.h |   7 +
>  4 files changed, 594 insertions(+), 1 deletion(-)
>  create mode 100644 net/6lowpan/context.c
>  create mode 100644 net/6lowpan/context.h
> 
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index 37ddbdf..ee3a455 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -199,6 +199,27 @@
>  
>  extern struct dentry *lowpan_debugfs;
>  
> +struct lowpan_context_table {
> +	struct list_head list;
> +
> +	/* Context entries */
> +	struct net_device *netdev;
> +	struct list_head context_entries;
> +	atomic_t context_count;
> +};
> +
> +struct lowpan_context_entry {
> +	struct list_head list;
> +	struct rcu_head rcu;
> +	struct lowpan_context_table *ctx_table;
> +
> +	/* Context parameters */
> +	u8 cid;
> +	struct in6_addr prefix;
> +	unsigned int prefix_len;
> +	bool compression_flag;
> +};
> +

Is a list really necessary here? I assume that a context-id (one byte
long) with a maximum value of 255. And then it's splitted into sci and
dci, so we have a maximum of 16 and you already introduced nicely an
LOWPAN_CONTEXT_TABLE_SIZE for that.

Can't we simple to some array to have a fast lookup for lowpan_context_table[cid]
and if it's not null then we have an entry for that?

To lookup the CID by an ipv6 address, then we need to iterate the array
and using ipv6_prefix_equal.

>  #ifdef DEBUG
>  /* print data in line */
>  static inline void raw_dump_inline(const char *caller, char *msg,
> @@ -337,6 +358,9 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
>  	if (!(iphc0 & 0x03))
>  		ret++;
>  
> +	if (iphc1 & LOWPAN_IPHC_CID)
> +		ret++;
> +
>  	ret += lowpan_addr_mode_size((iphc1 & LOWPAN_IPHC_SAM) >>
>  				     LOWPAN_IPHC_SAM_BIT);
>  
> @@ -374,6 +398,17 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
>  	return skb->len + uncomp_header - ret;
>  }
>  
> +int lowpan_context_table_alloc(struct net_device *netdev);
> +int lowpan_context_table_free(struct net_device *netdev);
> +int lowpan_context_free_all(void);
> +int lowpan_context_add(struct net_device *netdev,
> +		       struct lowpan_context_entry *entry);
> +int lowpan_context_remove(struct lowpan_context_entry *entry);
> +struct lowpan_context_entry *
> +lowpan_context_decompress(struct net_device *netdev, u8 cid);
> +struct lowpan_context_entry *
> +lowpan_context_compress(struct net_device *netdev, struct in6_addr *addr);
> +
>  int
>  lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
>  			 const u8 *saddr, const u8 saddr_type,
> diff --git a/net/6lowpan/Makefile b/net/6lowpan/Makefile
> index eb8baa7..bda5be0 100644
> --- a/net/6lowpan/Makefile
> +++ b/net/6lowpan/Makefile
> @@ -1,6 +1,6 @@
>  obj-$(CONFIG_6LOWPAN) += 6lowpan.o
>  
> -6lowpan-y := iphc.o nhc.o
> +6lowpan-y := iphc.o nhc.o context.o
>  
>  #rfc6282 nhcs
>  obj-$(CONFIG_6LOWPAN_NHC_DEST) += nhc_dest.o
> diff --git a/net/6lowpan/context.c b/net/6lowpan/context.c
> new file mode 100644
> index 0000000..00cd5af
> --- /dev/null
> +++ b/net/6lowpan/context.c
> @@ -0,0 +1,551 @@
> +/*
> + * Copyright (c)  2015 Nordic Semiconductor. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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.
> + */
> +
> +#include <linux/if_arp.h>
> +#include <linux/netdevice.h>
> +#include <linux/module.h>
> +#include <linux/debugfs.h>
> +#include <linux/inet.h>
> +#include <linux/atomic.h>
> +
> +#include <net/ipv6.h>
> +#include <net/addrconf.h>
> +#include <net/6lowpan.h>
> +
> +#define LOWPAN_DBG(fmt, ...)  pr_debug(fmt "\n", ##__VA_ARGS__)
> +
> +#define LOWPAN_CONTEXT_TABLE_SIZE     16
> +#define LOWPAN_CONTEXT_COMMAND_ADD    "add"
> +#define LOWPAN_CONTEXT_COMMAND_REMOVE "remove"
> +
> +#define CHECK_CID_RANGE(cid) ((cid) > 0 && (cid) < LOWPAN_CONTEXT_TABLE_SIZE)
> +#define CHECK_PREFIX_RANGE(len) ((len) > 0 && (len) <= 128)
> +
> +static struct dentry *lowpan_context_debugfs;
> +
> +static LIST_HEAD(context_tables);
> +static DEFINE_SPINLOCK(module_lock);
> +
> +static inline void entry_update(struct lowpan_context_entry *entry,

please add lowpan prefix here to this function.

> +				u8 cid,
> +				struct in6_addr prefix,
> +				unsigned int prefix_len,
> +				bool compression_flag)
> +{
> +	/* Fill context entry. */
> +	entry->cid = cid;
> +	entry->compression_flag = compression_flag;
> +	entry->prefix = prefix;
> +	entry->prefix_len = prefix_len;
> +}
> +
> +static inline void entry_free(struct lowpan_context_entry *entry)

lowpan prefix.

> +{
> +	/* Increase number of available contexts. */
> +	atomic_dec(&entry->ctx_table->context_count);
> +
> +	list_del_rcu(&entry->list);
> +	kfree_rcu(entry, rcu);
> +}
> +
> +static int entry_add(struct lowpan_context_table *ctx_table,

lowpan prefix.

> +		     u8 cid,
> +		     struct in6_addr prefix,
> +		     unsigned int prefix_len,
> +		     bool compression_flag)
> +{
> +	struct lowpan_context_entry *new_entry = NULL;
> +
> +	new_entry = kzalloc(sizeof(*new_entry), GFP_ATOMIC);
> +	if (!new_entry)
> +		return -ENOMEM;
> +
> +	/* Fill context entry. */
> +	new_entry->ctx_table = ctx_table;
> +	entry_update(new_entry, cid, prefix, prefix_len, compression_flag);
> +
> +	INIT_LIST_HEAD(&new_entry->list);
> +	list_add_rcu(&new_entry->list, &ctx_table->context_entries);
> +
> +	/* Increase number of available contexts. */
> +	atomic_inc(&ctx_table->context_count);
> +
> +	return 0;
> +}
> +
> +static void table_free(struct lowpan_context_table *ctx_table)

lowpan prefix.

> +{
> +	struct lowpan_context_entry *entry = NULL;
> +
> +	rcu_read_lock();
> +
> +	/* Clear context table. */
> +	if (atomic_read(&ctx_table->context_count)) {
> +		list_for_each_entry_rcu(entry,
> +					&ctx_table->context_entries,
> +					list)
> +			entry_free(entry);
> +	}
> +
> +	rcu_read_unlock();
> +
> +	/* Remove context table. */
> +	list_del(&ctx_table->list);
> +	kfree(ctx_table);
> +}
> +
> +static inline struct net_device *find_netdev(const char *name)

lowpan prefix.

> +{
> +	struct net_device *netdev;
> +
> +	rcu_read_lock();
> +	netdev = dev_get_by_name_rcu(&init_net, name);
> +	rcu_read_unlock();
> +
> +	return netdev;
> +}
> +
> +static struct lowpan_context_table *get_table(struct net_device *netdev)
> +{
> +	struct lowpan_context_table *entry;
> +	struct lowpan_context_table *net_table = NULL;
> +
> +	spin_lock(&module_lock);
> +
> +	list_for_each_entry(entry, &context_tables, list) {
> +		if (entry->netdev == netdev) {
> +			/* Table has been found. */
> +			net_table = entry;
> +			break;
> +		}
> +	}
> +
> +	spin_unlock(&module_lock);
> +

I know why you need such functionality. This is okay for now. But I
already thought about a nicer handling for that (which is also useful
for other mechanism). The idea is:

The netdev structure has some private pointer "netdev->priv". Currently
bluetooth 6lowpan/802.15.4 6lowpan do their own stuff inside this struct
which is okay. But we should provide some "upper class" struct for all
ARPHRD_6LOWPAN, which looks like:

-----------------------------
|   6LOWPAN_ARPHRD_STRUCT   | <-- All ARPHRD_6LOWPAN have this struct as first
-----------------------------
|        PRIVATE_DATA       | <-- 802.15.4/btle own stuff (private struct of subsystem) simple a
|                           |     "u8 priv __aligned(sizeof(void *));" inside ARPHRD_6LOWPAN which need to be at last
-----------------------------

Then you can put the net_table into the "6LOWPAN_ARPHRD_STRUCT" and cast
the "netdev->priv" pointer to "6LOWPAN_ARPHRD_STRUCT".

We really need such thing to avoid the above search&found functionality.
We should introduce such behaviour at first, then you can rebase your
stuff on it.

Also we need some functionality lowpan_alloc_netdev which ensure the
priv size is "sizeof(6LOWPAN_ARPHRD_STRUCT) + sizeof(PRIVATE_DATA)".

In upper layers like IPv6 you can do the following then:

if (dev->type == ARPHRD_6LOWPAN) {
	6LOWPAN_ARPHRD_STRUCT foobar = (struct 6LOWPAN_ARPHRD_STRUCT *)dev->priv;

	...do great cid handling here...
}

I hope this will also help you by implementing 6CO.

btw: (struct 6LOWPAN_ARPHRD_STRUCT *) should be implemented by a macro.


So please begin at first to add such functionality, please. Is that okay
for you?

> +	return net_table;
> +}
> +
> +static struct lowpan_context_entry *
> +get_ctx_by_cid(struct lowpan_context_table *table, u8 cid)
> +{
> +	struct lowpan_context_entry *entry;
> +	struct lowpan_context_entry *context = NULL;
> +
> +	rcu_read_lock();
> +
> +	list_for_each_entry_rcu(entry, &table->context_entries, list) {
> +		if (entry->cid == cid) {
> +			/* Context entry has been found. */
> +			context = entry;
> +			break;
> +		}
> +	}
> +
> +	rcu_read_unlock();
> +
> +	return context;
> +}
> +
> +static struct lowpan_context_entry *
> +get_ctx_by_addr(struct lowpan_context_table *table, struct in6_addr *addr)
> +{
> +	struct lowpan_context_entry *entry;
> +	struct lowpan_context_entry *best_match = NULL;
> +	unsigned int compare_len;
> +
> +	rcu_read_lock();
> +
> +	list_for_each_entry_rcu(entry, &table->context_entries, list) {
> +		if (!entry->compression_flag)
> +			continue;
> +
> +		compare_len = entry->prefix_len >= 64 ? entry->prefix_len : 64;
> +		if (ipv6_prefix_equal(addr, &entry->prefix, compare_len)) {
> +			/* Valid context entry has been found. Check if prefix
> +			 * can cover more bytes than previous one.
> +			 */
> +			if (!best_match ||
> +			    (best_match->prefix_len < entry->prefix_len))
> +				best_match = entry;
> +		}
> +	}
> +
> +	rcu_read_unlock();
> +
> +	return best_match;
> +}
> +
> +ssize_t lowpan_context_dbgfs_write(struct file *fp,
> +				   const char __user *user_buffer,
> +				   size_t count,
> +				   loff_t *position)
> +{
> +	char buf[128];
> +	char str_operation[16];
> +	char str_interface[16];
> +	char str_ipv6addr[40];
> +	size_t buf_size = min(count, sizeof(buf) - 1);
> +	int n;
> +	int ret;
> +	u8 cid;
> +	unsigned int prefix_length;
> +	unsigned int c_flag;
> +	struct in6_addr ctx_prefix;
> +	struct net_device *netdev;
> +	struct lowpan_context_table *ctx_table;
> +	struct lowpan_context_entry *entry;
> +
> +	memset(&ctx_prefix, 0, sizeof(ctx_prefix));
> +
> +	if (copy_from_user(buf, user_buffer, buf_size))
> +		return -EFAULT;
> +
> +	buf[buf_size] = '\0';
> +
> +	/* Parse input parameters. */
> +	n = sscanf(buf, "%s %s %hhd %s %d %d",
> +		   str_operation, str_interface, &cid, str_ipv6addr,
> +		   &prefix_length, &c_flag);
> +
> +	netdev = find_netdev(str_interface);
> +	/* Look up for net device. */
> +	if (!netdev) {
> +		LOWPAN_DBG("Cannot find given interface");
> +		return -EINVAL;
> +	}
> +
> +	if (!CHECK_CID_RANGE(cid)) {
> +		LOWPAN_DBG("CID value is out of range");
> +		return -EINVAL;
> +	}
> +
> +	/* Get context table. */
> +	ctx_table = get_table(netdev);
> +
> +	if (!ctx_table) {
> +		LOWPAN_DBG("Cannot find context table for %s interface",
> +			   str_interface);
> +		return -EINVAL;
> +	}
> +
> +	entry = get_ctx_by_cid(ctx_table, cid);
> +
> +	/* Execute command. */
> +	if (!memcmp(str_operation, LOWPAN_CONTEXT_COMMAND_ADD,
> +		    sizeof(LOWPAN_CONTEXT_COMMAND_ADD))) {
> +		/* Parse IPv6 address. */
> +		in6_pton(str_ipv6addr,
> +			 sizeof(str_ipv6addr),
> +			 ctx_prefix.s6_addr,
> +			 '\0',
> +			 NULL);
> +
> +		/* Check parameters. */
> +		if (ipv6_addr_any(&ctx_prefix) ||
> +		    !CHECK_PREFIX_RANGE(prefix_length)) {
> +			LOWPAN_DBG("Given parameters are not valid");
> +			return -EINVAL;
> +		}
> +
> +		LOWPAN_DBG("Got request: I:%s CID:%d Prefix:%pI6c/%d C:%d",
> +			   str_interface, cid, &ctx_prefix, prefix_length,
> +			   !!c_flag);
> +
> +		/* Check if entry for given CID already exists. */
> +		if (entry) {
> +			/* Just update parameters. */
> +			rcu_read_lock();
> +			entry_update(entry, cid, ctx_prefix, prefix_length,
> +				     !!c_flag);
> +			rcu_read_unlock();
> +		} else {
> +			/* Create a new entry. */
> +			ret = entry_add(ctx_table, cid, ctx_prefix,
> +					prefix_length, !!c_flag);
> +			if (ret < 0) {
> +				LOWPAN_DBG("Cannot add context entry");
> +				return ret;
> +			}
> +		}
> +	} else if (memcmp(str_operation, LOWPAN_CONTEXT_COMMAND_REMOVE,
> +			  sizeof(LOWPAN_CONTEXT_COMMAND_REMOVE)) == 0) {
> +		LOWPAN_DBG("Got new remove request: I:%s CID:%d",
> +			   str_interface, cid);
> +
> +		if (!entry) {
> +			LOWPAN_DBG("Cannot find context entry");
> +			return -EINVAL;
> +		}
> +
> +		rcu_read_lock();
> +		entry_free(entry);
> +		rcu_read_unlock();
> +	} else {
> +		LOWPAN_DBG("Invalid command - Cannot process the request");
> +		return -EINVAL;
> +	}
> +
> +	return count;
> +}
> +
> +static int lowpan_context_show(struct seq_file *f, void *ptr)
> +{
> +	struct lowpan_context_table *table;
> +	struct lowpan_context_entry *entry;
> +
> +	list_for_each_entry(table, &context_tables, list) {
> +		if (!atomic_read(&table->context_count))
> +			break;
> +
> +		seq_printf(f, "Context table of interface %s\r\n\r\n",
> +			   table->netdev->name);
> +		seq_printf(f, "%-3s %-39s %-3s %-3s \r\n",
> +			   "CID", "Prefix", "Len", "CF");
> +
> +		list_for_each_entry_rcu(entry, &table->context_entries, list)
> +			seq_printf(f, "%-3d %-39pI6c %-3d %-3s \r\n",
> +				   entry->cid,
> +				   &entry->prefix,
> +				   entry->prefix_len,
> +				   entry->compression_flag ? "1" : "0");
> +		seq_puts(f, "\r\n\r\n");
> +	}
> +
> +	return 0;
> +}
> +
> +int lowpan_context_dbgfs_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, lowpan_context_show, inode->i_private);
> +}
> +
> +int lowpan_context_table_alloc(struct net_device *netdev)
> +{
> +	struct lowpan_context_table *ctx_table = NULL;
> +
> +	if (!netdev) {
> +		LOWPAN_DBG("Network device has to be specified");
> +		return -EPERM;
> +	}
> +
> +	/* Look for context table. */
> +	ctx_table = get_table(netdev);
> +
> +	if (ctx_table) {
> +		LOWPAN_DBG("Context table already exists for interface %s %p",
> +			   netdev->name, ctx_table);
> +		return -EEXIST;
> +	}
> +
> +	ctx_table = kzalloc(sizeof(*ctx_table), GFP_ATOMIC);
> +	if (!ctx_table)
> +		return -ENOMEM;
> +
> +	/* Assign network device to context table. */
> +	ctx_table->netdev = netdev;
> +
> +	/* Initialize contexts list. */
> +	INIT_LIST_HEAD(&ctx_table->context_entries);
> +
> +	spin_lock(&module_lock);
> +	INIT_LIST_HEAD(&ctx_table->list);
> +	list_add_rcu(&ctx_table->list, &context_tables);
> +	spin_unlock(&module_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_table_alloc);
> +
> +int lowpan_context_table_free(struct net_device *netdev)
> +{
> +	struct lowpan_context_table *ctx_table = NULL;
> +
> +	if (!netdev) {
> +		LOWPAN_DBG("Network device has to be specified");
> +		return -EPERM;
> +	}
> +
> +	/* Look for context table. */
> +	ctx_table = get_table(netdev);
> +	if (!ctx_table) {
> +		LOWPAN_DBG("Cannot find context table for interface %s",
> +			   netdev->name);
> +		return -ENOENT;
> +	}
> +
> +	spin_lock(&module_lock);
> +	table_free(ctx_table);
> +	spin_unlock(&module_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_table_free);
> +
> +int lowpan_context_add(struct net_device *netdev,
> +		       struct lowpan_context_entry *entry)
> +{
> +	int ret;
> +	struct lowpan_context_table *ctx_table = NULL;
> +	struct lowpan_context_entry *stored_entry = NULL;
> +
> +	if (!entry || !netdev) {
> +		LOWPAN_DBG("Parameters are incorrect");
> +		return -EPERM;
> +	}
> +
> +	/* Get context table.  */
> +	ctx_table = get_table(netdev);
> +	if (!ctx_table) {
> +		LOWPAN_DBG("Cannot find context table for %s interface",
> +			   netdev->name);
> +		return -EINVAL;
> +	}
> +
> +	stored_entry = get_ctx_by_cid(ctx_table, entry->cid);
> +
> +	/* Check if entry for given CID already exists. */
> +	if (entry) {
> +		/* Just update parameters. */
> +		rcu_read_lock();
> +		entry_update(stored_entry, entry->cid, entry->prefix,
> +			     entry->prefix_len, entry->compression_flag);
> +		rcu_read_unlock();
> +	} else {
> +		/* Create a new entry. */
> +		ret  = entry_add(ctx_table, entry->cid, entry->prefix,
> +				 entry->prefix_len, entry->compression_flag);
> +
> +		if (ret < 0) {
> +			LOWPAN_DBG("Cannot add context entry");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_add);
> +

Don't export function which isn't used by other modules. Maybe you
prepared for using this function for the 6CO field in ipv6. If yes, we
can do that later.

> +int lowpan_context_remove(struct lowpan_context_entry *entry)
> +{
> +	if (!entry) {
> +		LOWPAN_DBG("Given entry is NULL");
> +		return -EPERM;
> +	}
> +
> +	/* Free given entry. */
> +	rcu_read_lock();
> +	entry_free(entry);
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_remove);
> +

same here.

> +struct lowpan_context_entry *
> +lowpan_context_decompress(struct net_device *netdev, u8 cid)
> +{
> +	struct lowpan_context_table *ctx_table = NULL;
> +	struct lowpan_context_entry *entry = NULL;
> +
> +	if (!netdev) {
> +		LOWPAN_DBG("Network device has to be specified");
> +		return NULL;
> +	}
> +
> +	/* Get proper interface table. */
> +	ctx_table = get_table(netdev);
> +	if (!ctx_table) {
> +		LOWPAN_DBG("Cannot find context table for interface %s",
> +			   netdev->name);
> +		return NULL;
> +	}
> +
> +	/* Search for given CID. */
> +	entry = get_ctx_by_cid(ctx_table, cid);
> +	if (!entry) {
> +		LOWPAN_DBG("There is no context of id:%d for interface %s",
> +			   cid, netdev->name);
> +		return NULL;
> +	}
> +
> +	LOWPAN_DBG("Found context prefix for context of id:%d - %pI6c",
> +		   entry->cid, &entry->prefix);
> +
> +	return entry;
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_decompress);
> +

same here.

> +struct lowpan_context_entry *lowpan_context_compress(struct net_device *netdev,
> +						     struct in6_addr *addr)
> +{
> +	struct lowpan_context_table *ctx_table = NULL;
> +	struct lowpan_context_entry *entry = NULL;
> +
> +	if (!netdev || !addr) {
> +		LOWPAN_DBG("Network device and address has to be specified");
> +		return NULL;
> +	}
> +
> +	/* Get proper interface table. */
> +	ctx_table = get_table(netdev);
> +	if (!ctx_table) {
> +		LOWPAN_DBG("Cannot find context table for interface %s",
> +			   netdev->name);
> +		return NULL;
> +	}
> +
> +	/* Search for given address. */
> +	entry = get_ctx_by_addr(ctx_table, addr);
> +	if (!entry) {
> +		LOWPAN_DBG("No context for address %pI6c for interface %s",
> +			   addr, netdev->name);
> +		return NULL;
> +	}
> +
> +	LOWPAN_DBG("Found context of id:%d", entry->cid);
> +
> +	/* Return null in case no compression capability in found context */
> +	return entry;
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_compress);
> +

same here.

> +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
> +};
> +
> +void lowpan_context_init(void)
> +{
> +	lowpan_context_debugfs = debugfs_create_file("context", 0664,
> +						     lowpan_debugfs, NULL,
> +						     &lowpan_context_fops);
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_init);
> +

same here.

> +void lowpan_context_uninit(void)
> +{
> +	struct lowpan_context_table *entry = NULL;
> +
> +	spin_lock(&module_lock);
> +
> +	list_for_each_entry(entry, &context_tables, list)
> +		table_free(entry);
> +
> +	spin_unlock(&module_lock);
> +
> +	debugfs_remove(lowpan_context_debugfs);
> +}
> +EXPORT_SYMBOL_GPL(lowpan_context_uninit);

same here.

> diff --git a/net/6lowpan/context.h b/net/6lowpan/context.h
> new file mode 100644
> index 0000000..88303b5
> --- /dev/null
> +++ b/net/6lowpan/context.h
> @@ -0,0 +1,7 @@
> +#ifndef __6LOWPAN_CONTEXT_H
> +#define __6LOWPAN_CONTEXT_H
> +
> +void lowpan_context_init(void);
> +void lowpan_context_uninit(void);
> +
> +#endif /* __6LOWPAN_CONTEXT_H */
> -- 
> 2.1.4
> 

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

* Re: [RFC v2 2/4] 6lowpan: Add stateful compression component of 6lowpan module
  2015-07-23  6:52   ` Alexander Aring
@ 2015-07-23  6:59     ` Alexander Aring
  0 siblings, 0 replies; 20+ messages in thread
From: Alexander Aring @ 2015-07-23  6:59 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

On Thu, Jul 23, 2015 at 08:52:09AM +0200, Alexander Aring wrote:
> 
> I know why you need such functionality. This is okay for now. But I
> already thought about a nicer handling for that (which is also useful
> for other mechanism). The idea is:
> 
> The netdev structure has some private pointer "netdev->priv". Currently
> bluetooth 6lowpan/802.15.4 6lowpan do their own stuff inside this struct
> which is okay. But we should provide some "upper class" struct for all
> ARPHRD_6LOWPAN, which looks like:
> 
> -----------------------------
> |   6LOWPAN_ARPHRD_STRUCT   | <-- All ARPHRD_6LOWPAN have this struct as first
> -----------------------------
> |        PRIVATE_DATA       | <-- 802.15.4/btle own stuff (private struct of subsystem) simple a
> |                           |     "u8 priv __aligned(sizeof(void *));" inside ARPHRD_6LOWPAN which need to be at last
> -----------------------------
> 
> Then you can put the net_table into the "6LOWPAN_ARPHRD_STRUCT" and cast
> the "netdev->priv" pointer to "6LOWPAN_ARPHRD_STRUCT".
> 
> We really need such thing to avoid the above search&found functionality.
> We should introduce such behaviour at first, then you can rebase your
> stuff on it.
> 
> Also we need some functionality lowpan_alloc_netdev which ensure the
> priv size is "sizeof(6LOWPAN_ARPHRD_STRUCT) + sizeof(PRIVATE_DATA)".
> 
> In upper layers like IPv6 you can do the following then:
> 
> if (dev->type == ARPHRD_6LOWPAN) {
> 	6LOWPAN_ARPHRD_STRUCT foobar = (struct 6LOWPAN_ARPHRD_STRUCT *)dev->priv;
> 
> 	...do great cid handling here...
> }
> 
> I hope this will also help you by implementing 6CO.
> 
> btw: (struct 6LOWPAN_ARPHRD_STRUCT *) should be implemented by a macro.
> 
> 
> So please begin at first to add such functionality, please. Is that okay
> for you?
> 

This is actually the same idea like introducing the lowpan_ptr inside
the netdev structure, just a little different. I am fine also for
introducing the lowpan_ptr when netdev acked such patch. Anyway we need
some core lowpan functionality to allocate/free the memory then. Or we
just point the lowpan_ptr to the right netdev private memory area, then
we don't need to cast every time.

- Alex

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

* Re: [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c
  2015-07-13 11:50 ` [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c Lukasz Duda
@ 2015-07-23  7:48   ` Alexander Aring
  2015-07-23  8:20     ` Duda, Lukasz
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Aring @ 2015-07-23  7:48 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

On Mon, Jul 13, 2015 at 01:50:32PM +0200, Lukasz Duda wrote:
...
>  		} else {
>  			pr_debug("dest address unicast %pI6c\n", &hdr->daddr);
>  			lowpan_push_hc_data(&hc_ptr, hdr->daddr.s6_addr, 16);
> @@ -619,6 +774,9 @@ static int __init lowpan_module_init(void)
>  {
>  	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
>  
> +	/* Initialize stateful compression. */
> +	lowpan_context_init();
> +

Error handling here, too. You need to change lowpan_context_init for
returning return value from debugfs_create_file then.

The rest will I review when I get it running. :-)

- Alex

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

* Re: [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan
  2015-07-13 11:50 ` [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan Lukasz Duda
@ 2015-07-23  7:55   ` Alexander Aring
  2015-07-23  8:09     ` Duda, Lukasz
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Aring @ 2015-07-23  7:55 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth, Glenn Ruben Bakke

On Mon, Jul 13, 2015 at 01:50:33PM +0200, Lukasz Duda wrote:
> Allocating context table for stateful compression when interface is set
> up. Removing context table when interface is being unregistered.
> 
> Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
> Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
> ---
>  net/bluetooth/6lowpan.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index 2fb7b30..60c7e80 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -871,6 +871,9 @@ static int setup_netdev(struct l2cap_chan *chan, struct lowpan_dev **dev)
>  	       &chan->src, chan->src_type);
>  	set_bit(__LINK_STATE_PRESENT, &netdev->state);
>  
> +	/* Allocate context table for stateful compression. */
> +	lowpan_context_table_alloc(netdev);
> +
>  	*dev = netdev_priv(netdev);
>  	(*dev)->netdev = netdev;
>  	(*dev)->hdev = chan->conn->hcon->hdev;
> @@ -1415,6 +1418,7 @@ static int device_event(struct notifier_block *unused,
>  			if (entry->netdev == netdev) {
>  				BT_DBG("Unregistered netdev %s %p",
>  				       netdev->name, netdev);
> +				lowpan_context_table_free(netdev);
>  				list_del(&entry->list);
>  				break;
>  			}

Don't we need similar allocation in net/ieee802154/6lowpan/core.c ?

- Alex

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

* RE: [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan
  2015-07-23  7:55   ` Alexander Aring
@ 2015-07-23  8:09     ` Duda, Lukasz
  0 siblings, 0 replies; 20+ messages in thread
From: Duda, Lukasz @ 2015-07-23  8:09 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	Bakke, Glenn Ruben

Hi Aring,

> On Mon, Jul 13, 2015 at 01:50:33PM +0200, Lukasz Duda wrote:
> > Allocating context table for stateful compression when interface is
> > set up. Removing context table when interface is being unregistered.
> >
> > Signed-off-by: Lukasz Duda <lukasz.duda@nordicsemi.no>
> > Signed-off-by: Glenn Ruben Bakke <glenn.ruben.bakke@nordicsemi.no>
> > ---
> >  net/bluetooth/6lowpan.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index
> > 2fb7b30..60c7e80 100644
> > --- a/net/bluetooth/6lowpan.c
> > +++ b/net/bluetooth/6lowpan.c
> > @@ -871,6 +871,9 @@ static int setup_netdev(struct l2cap_chan *chan,
> struct lowpan_dev **dev)
> >  	       &chan->src, chan->src_type);
> >  	set_bit(__LINK_STATE_PRESENT, &netdev->state);
> >
> > +	/* Allocate context table for stateful compression. */
> > +	lowpan_context_table_alloc(netdev);
> > +
> >  	*dev = netdev_priv(netdev);
> >  	(*dev)->netdev = netdev;
> >  	(*dev)->hdev = chan->conn->hcon->hdev; @@ -1415,6 +1418,7 @@
> static
> > int device_event(struct notifier_block *unused,
> >  			if (entry->netdev == netdev) {
> >  				BT_DBG("Unregistered netdev %s %p",
> >  				       netdev->name, netdev);
> > +				lowpan_context_table_free(netdev);
> >  				list_del(&entry->list);
> >  				break;
> >  			}
> 
> Don't we need similar allocation in net/ieee802154/6lowpan/core.c ?
> 
> - Alex

Yes we need the same for IEEE802.15.4. Since I can test stateful compression feature only with BT-LE right now, 
I decided to not touch this part. However this is an obvious goal to create generic stateful compression for all
set of 6LoWPAN protocols.

Regards,
Łukasz

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

* RE: [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c
  2015-07-23  7:48   ` Alexander Aring
@ 2015-07-23  8:20     ` Duda, Lukasz
  0 siblings, 0 replies; 20+ messages in thread
From: Duda, Lukasz @ 2015-07-23  8:20 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	Bakke, Glenn Ruben

Hi Alex,

> On Mon, Jul 13, 2015 at 01:50:32PM +0200, Lukasz Duda wrote:
> ...
> >  		} else {
> >  			pr_debug("dest address unicast %pI6c\n", &hdr-
> >daddr);
> >  			lowpan_push_hc_data(&hc_ptr, hdr->daddr.s6_addr,
> 16); @@ -619,6
> > +774,9 @@ static int __init lowpan_module_init(void)  {
> >  	lowpan_debugfs = debugfs_create_dir("6lowpan", NULL);
> >
> > +	/* Initialize stateful compression. */
> > +	lowpan_context_init();
> > +
> 
> Error handling here, too. You need to change lowpan_context_init for
> returning return value from debugfs_create_file then.
> 
> The rest will I review when I get it running. :-)
> 
> - Alex

Thanks for comment! 

I am certainly going to add this check on debugfs_create. The reason being is that I tried to fit into /net/bluetooth code part as
much as I can and I didn't find such checks there :)

Regards,
Łukasz

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

* Re: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
                   ` (6 preceding siblings ...)
  2015-07-15  9:55 ` Duda, Lukasz
@ 2015-07-23  8:22 ` Alexander Aring
  2015-10-12 16:15 ` Alexander Aring
  8 siblings, 0 replies; 20+ messages in thread
From: Alexander Aring @ 2015-07-23  8:22 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth

Hi,

On Mon, Jul 13, 2015 at 01:50:29PM +0200, Lukasz Duda wrote:
> Hi all,
> 
> Resending the RFC patch set after some clean up.
> 
> The following patches introduce support for stateful compression in IPHC.
> 
> Patch #1: Introduce debugfs entry for 6lowpan module
> Patch #2: Add stateful compression component of 6lowpan module
> Patch #3: Add stateful compression support for iphc.c
> Patch #4: Enable stateful compression in bluetooth_6lowpan
> 
> Usage of stateful compression is described in Patch #2.
> 
> Notes
> =====
> 
> RFC v2:
>     - Splitting patch into smaller incremental patches
>     - Moving debugfs logic from iphc.c into context.c
>     - Updating patch set to align with style and coding guideline
>     
> RFC v1:
>     - Initial patch set: http://www.spinics.net/lists/linux-bluetooth/msg62930.html
> 
> Limitations
> ===========
> 
> - Temporarly use of debugfs to be able to add context table entries.
> - Current module does not support context time-outs.
> - No support for multicast addresses stateful compression.
> 
> Discussion: Idea on how to make the 6lowpan context tables out of debug mode
> ============================================================================
> 
> The patch provided here is just a temporary solution where the contexts are
> manually added. The proper way of adding/removing contexts would be to make
> ndisc.c parse 6CO options and act upon it. Generated Router Advertisement
> packets with 6CO option (for example from RADVD) will be handled in the
> ndisc_router_discovery function.
> 

I think it's currently fine to bring such functionality over debugfs for
introduce the functionality. If we have it done, then we talk about more
detailed process how to make the dealing with 6CO, ipv6 stack and 6lowpan
module.

> Also, other flags like ARO, ABRO etc. which are specified in RFC6775 need
> to be handled in the ipv6 module in order to do it in a neat way.
> Potentially the RFC6775 extensions could also be used by other protocols
> than BLE and 802.15.4.
> 
> There is a need for parsing the 6CO option in ndisc.c which parses Router
> Advertisement messages. Today the function calls to IPHC (6lowpan module) are
> quite hard to implement in ndisc.c, as there is no guarantee that the 6lowpan
> module will be loaded or not. It would make sense to build 6lowpan module
> as in-build part of IPv6. The features could be compiled in by using #defines.
> 
> What do you think about moving 6lowpan as a component of IPv6 and modify ndisc.c

The initial idea of 6lowpan is to work as adaptation layer. This is exactly
what we try to do with the virtual lowpan interface, doing compression/uncompression
and such stuff before we give the packet to the next layer. 6LoWPAN has also
some differents between btle/802.15.4 e.g. we need to run some fragmentation/reassemble
before ipv6 layer.

To run as adaptation layer this isn't always possible, like in your case
the 6CO field. Also in 802.15.4 we need a special handling for different
mac address types short and extended (which is not supported right now).

I think we should try to handling it still as seperate module so far we
can, if the netdev people says something that we can't do and the
solution would be a move of 6lowpan branch into ipv6, then we should do
it. E.g. Maybe they don't like function dependencies from the 6lowpan
module, like lookup cid value. I don't know how possible that is for
accepting such dependency mainline for the ipv6 community. But maybe we
can implement the lookup functions then in ipv6 module (which should
always available when loading the 6lowpan module).


For your support to handle the 6CO functionality:

 - First, prepare all steps that it works with the debugfs and we have it
   mainline. (Inclusive all review notes, that we have the basic
   functionality).

 - Second, send patches for review to bluetooth, wpan mailinglist which
   adds functionality to net/ipv6 -> Then we can talk about the possible
   handling "how we can add such functionality". The netdev people
   looking much into things which slows down other stack e.g. ethernet
   and they are not happy when adding more runtime decision stuff (I
   suppose that and never tried that out). So if we add more runtime
   decision we should make this so small as possible, otherwise I don't
   see that we have a change to get it in.

   Sometimes in IPv6 stack you already see several times
   "switch (dev->type)", that's a good example that we can do our stuff
   there by checking on ARPHRD_6LOWPAN and making different stuff on
   hot paths. Then we have no additional runtime costs. But I think such
   "switch (dev->type)" doesn't exist at places were do you need the
   handling now.


I don't see that we can talk much about "how possible things are to
implement" - without any code. The 6LoWPAN code we can touch every time,
the IPv6 code is more complicated right now. :-)

- Alex

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

* Re: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
                   ` (7 preceding siblings ...)
  2015-07-23  8:22 ` Alexander Aring
@ 2015-10-12 16:15 ` Alexander Aring
  2015-10-13  8:36   ` Duda, Lukasz
  8 siblings, 1 reply; 20+ messages in thread
From: Alexander Aring @ 2015-10-12 16:15 UTC (permalink / raw)
  To: Lukasz Duda; +Cc: linux-wpan, linux-bluetooth, martin.gergeleit

Hi Lukasz,

do you still working on sateful compression, otherwise I would try to
implement this stuff based on your work.

When I have something I will keep you in cc for reviewing process.

Thanks.

- Alex

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

* RE: [RFC v2 0/4] Adding stateful compression to IPHC
  2015-10-12 16:15 ` Alexander Aring
@ 2015-10-13  8:36   ` Duda, Lukasz
  0 siblings, 0 replies; 20+ messages in thread
From: Duda, Lukasz @ 2015-10-13  8:36 UTC (permalink / raw)
  To: Alexander Aring
  Cc: linux-wpan@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	martin.gergeleit@hs-rm.de, Bakke, Glenn Ruben

Hello Alex,

First of all, I want to thank you for changes that you have done on
6LoWPAN architecture.

Due to a large workload we have not been able to do further patches
to the Stateful Compression RFC.

We would be more than happy if you want to continue on what we started.
While we are busy in next few weeks with other priorities, we should still
be able to make time for reviews if you want to take over where we left.

One note I would like to add is from the latest BTLE draft:
(https://tools.ietf.org/html/draft-ietf-6lo-btle-17#section-3.2.4).
This defines some different (non generic) approach with respect to compression
of IID, that comes from ARO messages (not implemented in Linux) instead of
MAC layer, which is not handled by the initial Stateful Compression RFC we
proposed.

Best regards,
Łukasz

> -----Original Message-----
> From: Alexander Aring [mailto:alex.aring@gmail.com]
> Sent: Monday, October 12, 2015 18:16
> To: Duda, Lukasz
> Cc: linux-wpan@vger.kernel.org; linux-bluetooth@vger.kernel.org;
> martin.gergeleit@hs-rm.de
> Subject: Re: [RFC v2 0/4] Adding stateful compression to IPHC
> 
> Hi Lukasz,
> 
> do you still working on sateful compression, otherwise I would try to
> implement this stuff based on your work.
> 
> When I have something I will keep you in cc for reviewing process.
> 
> Thanks.
> 
> - Alex

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

end of thread, other threads:[~2015-10-13  8:46 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-13 11:50 [RFC v2 0/4] Adding stateful compression to IPHC Lukasz Duda
2015-07-13 11:50 ` [RFC v2 1/4] 6lowpan: Introduce debugfs entry for 6lowpan module Lukasz Duda
2015-07-23  6:07   ` Alexander Aring
2015-07-13 11:50 ` [RFC v2 2/4] 6lowpan: Add stateful compression component of " Lukasz Duda
2015-07-23  6:52   ` Alexander Aring
2015-07-23  6:59     ` Alexander Aring
2015-07-13 11:50 ` [RFC v2 3/4] 6lowpan: Add stateful compression support for iphc.c Lukasz Duda
2015-07-23  7:48   ` Alexander Aring
2015-07-23  8:20     ` Duda, Lukasz
2015-07-13 11:50 ` [RFC v2 4/4] Bluetooth: 6lowpan: Enable stateful compression in bluetooth_6lowpan Lukasz Duda
2015-07-23  7:55   ` Alexander Aring
2015-07-23  8:09     ` Duda, Lukasz
2015-07-13 13:09 ` [RFC v2 0/4] Adding stateful compression to IPHC Jukka Rissanen
2015-07-15 13:42   ` Duda, Lukasz
2015-07-16  7:25     ` Jukka Rissanen
2015-07-13 14:15 ` Michael Richardson
2015-07-15  9:55 ` Duda, Lukasz
2015-07-23  8:22 ` Alexander Aring
2015-10-12 16:15 ` Alexander Aring
2015-10-13  8:36   ` Duda, Lukasz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).