[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151125180716.GB4652@omega>
Date: Wed, 25 Nov 2015 19:07:21 +0100
From: Alexander Aring <alex.aring@...il.com>
To: Stefan Schmidt <stefan@....samsung.com>
Cc: linux-wpan@...r.kernel.org, linux-bluetooth@...r.kernel.org,
netdev@...r.kernel.org, kernel@...gutronix.de, mcr@...delman.ca,
lukasz.duda@...dicsemi.no, martin.gergeleit@...rm.de
Subject: Re: [RFCv2 bluetooth-next 3/3] 6lowpan: iphc: add support for
stateful compression
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@...il.com>
> >---
> > include/net/6lowpan.h | 62 ++++++
> > net/6lowpan/6lowpan_i.h | 3 +
> > net/6lowpan/Kconfig | 1 +
> > net/6lowpan/core.c | 17 ++
> > net/6lowpan/debugfs.c | 109 +++++++++++
> > net/6lowpan/iphc.c | 500 ++++++++++++++++++++++++++++++++++++++++++------
> > 6 files changed, 635 insertions(+), 57 deletions(-)
> >
> >diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> >index e484898..6e8d30e 100644
> >--- a/include/net/6lowpan.h
> >+++ b/include/net/6lowpan.h
> >@@ -75,6 +75,8 @@
> > #define LOWPAN_IPHC_MAX_HC_BUF_LEN (sizeof(struct ipv6hdr) + \
> > LOWPAN_IPHC_MAX_HEADER_LEN + \
> > LOWPAN_NHC_MAX_HDR_LEN)
> >+/* SCI/DCI is 4 bit width, so we have maximum 16 entries */
> >+#define LOWPAN_IPHC_CI_TABLE_SIZE (1 << 4)
> > #define LOWPAN_DISPATCH_IPV6 0x41 /* 01000001 = 65 */
> > #define LOWPAN_DISPATCH_IPHC 0x60 /* 011xxxxx = ... */
> >@@ -98,9 +100,45 @@ enum lowpan_lltypes {
> > LOWPAN_LLTYPE_IEEE802154,
> > };
> >+enum lowpan_iphc_ctx_states {
> >+ LOWPAN_IPHC_CTX_STATE_DISABLED,
> >+ LOWPAN_IPHC_CTX_STATE_ENABLED,
> >+
> >+ __LOWPAN_IPHC_CTX_STATE_INVALID,
> >+ LOWPAN_IPHC_CTX_STATE_MAX = __LOWPAN_IPHC_CTX_STATE_INVALID - 1,
> >+};
> >+
>
> You are expecting more states here besides enabled and disabled? Because
> normally a bool would be fine here and an enum overkill. If you have more
> states pending it makes sense to have the enum though.
>
Yea, I thought about more states than just these ones. E.g. from which
sources came the context which was set, from ICMPv6 option field, manually
added from debugfs/netlink? Such things...
Nevertheless I will change it to bool here, we can still change it if we
want that. But then we need to talk about that again if doing userspace
API for that.
> >+struct lowpan_iphc_ctx {
> >+ enum lowpan_iphc_ctx_states state;
> >+ u8 id;
> >+ struct in6_addr addr;
> >+ u8 prefix_len;
> >+};
> >+
...
> > #include <linux/netdevice.h>
> >+extern const struct lowpan_iphc_ctx_ops iphc_ctx_unicast_ops;
> >+extern const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops;
> >+
> > #ifdef CONFIG_6LOWPAN_DEBUGFS
> > int lowpan_dev_debugfs_init(struct net_device *dev);
> > void lowpan_dev_debugfs_uninit(struct net_device *dev);
> >diff --git a/net/6lowpan/Kconfig b/net/6lowpan/Kconfig
> >index 01c901c..7ecedd7 100644
> >--- a/net/6lowpan/Kconfig
> >+++ b/net/6lowpan/Kconfig
> >@@ -8,6 +8,7 @@ menuconfig 6LOWPAN
> > config 6LOWPAN_DEBUGFS
> > bool "6LoWPAN debugfs support"
> > depends on 6LOWPAN
> >+ depends on DEBUG_FS
>
> This looks like a fix that should be merged into the first patch, no?
yes. I will fix that.
> > ---help---
> > This enables 6LoWPAN debugfs support. For example to manipulate
> > IPHC context information at runtime.
> >diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> >index fe58509..d354c5b 100644
> >--- a/net/6lowpan/core.c
> >+++ b/net/6lowpan/core.c
> >@@ -19,6 +19,8 @@
> > int lowpan_netdev_setup(struct net_device *dev, enum lowpan_lltypes lltype)
> > {
> >+ int i;
> >+
> > dev->addr_len = EUI64_ADDR_LEN;
> > dev->type = ARPHRD_6LOWPAN;
> > dev->mtu = IPV6_MIN_MTU;
...
> >+static ssize_t lowpan_context_dbgfs_write(struct file *fp,
> >+ const char __user *user_buf,
> >+ size_t count, loff_t *ppos)
> >+{
> >+ char buf[128] = { };
> >+ struct seq_file *file = fp->private_data;
> >+ struct lowpan_iphc_ctx_table *t = file->private;
> >+ struct lowpan_iphc_ctx ctx;
> >+ int status = count, n, id, state, i, prefix_len, ret;
> >+ unsigned int addr[8];
> >+
> >+ if (copy_from_user(&buf, user_buf, min_t(size_t, sizeof(buf) - 1,
> >+ count))) {
> >+ status = -EFAULT;
> >+ goto out;
> >+ }
> >+
> >+ n = sscanf(buf, "%d %04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x/%d %d",
> >+ &id, &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
> >+ &addr[5], &addr[6], &addr[7], &prefix_len, &state);
> >+ if (n != 11) {
>
> 11 more like a magic number here. Not to bad, but a define could be nice.
ok.
...
> >+
> >+const struct lowpan_iphc_ctx_ops iphc_ctx_mcast_ops = {
> >+ .valid_prefix = lowpan_iphc_mcast_ctx_valid_prefix,
> >+ .update = lowpan_iphc_ctx_update,
> >+ .get_by_id = lowpan_iphc_ctx_get_by_id,
> >+ .get_by_addr = lowpan_iphc_ctx_get_by_mcast_addr,
> >+};
>
> These are the comments from a first look over it. I will have a more
> detailed review and actual testing once you are happy enough with it to move
> it from RFC to PATCH.
>
ok.
- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists