Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for stateful compression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 25/11/15 19:07, Alexander Aring wrote:
Hi,

On Wed, Nov 25, 2015 at 06:12:25PM +0100, Stefan Schmidt wrote:
Helllo.

On 17/11/15 23:33, Alexander Aring wrote:
This patch introduce support for IPHC stateful address compression. It
will offer three debugfs per interface entries, which are:

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

Example to setup a context id:

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

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

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

Hint: Simple copy one line and then maniuplate it.

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

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

On transmit side:

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

Example:

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

On Receive side:

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

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

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index e484898..6e8d30e 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -75,6 +75,8 @@
  #define LOWPAN_IPHC_MAX_HC_BUF_LEN	(sizeof(struct ipv6hdr) +	\
  					 LOWPAN_IPHC_MAX_HEADER_LEN +	\
  					 LOWPAN_NHC_MAX_HDR_LEN)
+/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
+#define LOWPAN_IPHC_CI_TABLE_SIZE	(1 << 4)
  #define LOWPAN_DISPATCH_IPV6		0x41 /* 01000001 = 65 */
  #define LOWPAN_DISPATCH_IPHC		0x60 /* 011xxxxx = ... */
@@ -98,9 +100,45 @@ enum lowpan_lltypes {
  	LOWPAN_LLTYPE_IEEE802154,
  };
+enum lowpan_iphc_ctx_states {
+	LOWPAN_IPHC_CTX_STATE_DISABLED,
+	LOWPAN_IPHC_CTX_STATE_ENABLED,
+
+	__LOWPAN_IPHC_CTX_STATE_INVALID,
+	LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
+};
+
You are expecting more states here besides enabled and disabled? Because
normally a bool would be fine here and an enum overkill. If you have more
states pending it makes sense to have the enum though.

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

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

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

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

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

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

Thanks.

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

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

ok.

- Alex

regards
Stefan Schmidt
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux