Re: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for stateful compression

2015-12-17 Thread Alexander Aring
On Thu, Dec 17, 2015 at 12:26:37PM +, Duda, Lukasz wrote:
> Hi Alex!
> 
> > -Original Message-
> > From: Alexander Aring [mailto:alex.ar...@gmail.com]
> > Sent: Tuesday, December 15, 2015 12:09
> > To: Duda, Lukasz
> > Cc: linux-w...@vger.kernel.org; linux-blueto...@vger.kernel.org;
> > netdev@vger.kernel.org; ker...@pengutronix.de; m...@sandelman.ca;
> > martin.gergel...@hs-rm.de
> > Subject: Re: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for
> > stateful compression
> > 
> > On Tue, Dec 15, 2015 at 10:29:51AM +, Duda, Lukasz wrote:
> > > First of all great work for your series of patches on 6lowpan improvements
> > and
> > > stateful compression!
> > >
> > > I have just done some testing of this patch (without RADVD modifications),
> > and
> > > I can share my experiments using 6LoWPAN over BT-LE by sending simple
> > ICMPv6
> > > messages. Contexts for BTLE device has been added manually.
> > >
> > 
> > did you test that with linux <-> linux? Or linux <->
> > $SOME_OTHER_6LOWPAN_BTLE_STACK.
> > 
> > I tested it on my side with RIOT, it has 802.15.4 6LoWPAN support and
> > also manipulate manually the context table.
> > 
> 
> I have tested it with nRF52 BTLE device from Nordic Semiconductor 
> with IoT SDK, and Linux Ubuntu with BTLE Dongle that acts as Router/Master.
> 
> > > Experiment 1:
> > >
> > > Router: 2001:db8::1/64 BTLE: 2001:db8::211:22FF:FE33:4455/64
> > > CID 1: 2001:db8::/64
> > >
> > > Works fine, I see that CID 1 is used for both addresses. Router has 64 
> > > bits of
> > > IID inline and BTLE node has 0.
> > >
> > > Experiment 2:
> > >
> > > Router: 2001:db8::1/64 BTLE: 2001:db8::211:22FF:FE33:4455/64
> > > CID 3: 2001:db8::/64 CID 5: 2001:db8::1/128
> > >
> > > Works also fine, I see that both CID 3 and 5 are used, as well as both 
> > > sides
> > > compress its IID in the best possible way. So the patch appears to work 
> > > fine
> > on
> > > 6LoWPAN over BT-LE.
> > >
> > 
> > ok.
> > 
> > >
> > > However, I notice that the folder created in the sys/kernel/debug/6lowpan/
> > for
> > > my bluetooth network interface is called "bt%d". And I would imagine this
> > > should be "bt0", "bt1", ... and not the template?
> > >
> > 
> > urgh, this should not happen. I use "dev->name" for that and dev is the
> > netdevice structure. This should be an _unique_ interface name,
> > otherwise you will getting trouble if you have two btle 6lowpan
> > interfaces.
> > 
> > I didn't realized it because I create my interface with:
> > 
> > ip link add link wpan0 name lowpan0 type lowpan
> > 
> > but should change it into:
> > 
> > ip link add link wpan0 name lowpan%d type lowpan
> > 
> > I realized that the dev->name will be changed from template into "real"
> > name after registering. This should do the job:
> > 
> > diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> > index c7f06f5..faf65ba 100644
> > --- a/net/6lowpan/core.c
> > +++ b/net/6lowpan/core.c
> > @@ -29,13 +29,13 @@ int lowpan_register_netdevice(struct net_device
> > *dev,
> > 
> > lowpan_priv(dev)->lltype = lltype;
> > 
> > -   ret = lowpan_dev_debugfs_init(dev);
> > +   ret = register_netdevice(dev);
> > if (ret < 0)
> > return ret;
> > 
> > -   ret = register_netdevice(dev);
> > +   ret = lowpan_dev_debugfs_init(dev);
> > if (ret < 0)
> > -   lowpan_dev_debugfs_exit(dev);
> > +   unregister_netdevice(dev);
> > 
> > return ret;
> >  }
> > 
> > I think it should be safe to do that after registering because we held the
> > RTNL lock. And the interface isn't up after registering.
> > 
> 
> Thanks! Your patch helped, and I acked it in separate mail thread.
> 
> > > Also, I notice that the compression for Flow Control and Traffic Label in 
> > > IPv6
> > > header has been modified, these fields are no longer compressed in any
> > packets
> > > (0b11 value) that comes from Linux Kernel (e.g. ICMP Echo Request,
> > > Router Advertisement), instead I get three extra bytes (0b01 value).
> > > I would like to understand reason for this modification a little better.
> > 
&

RE: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for stateful compression

2015-12-17 Thread Duda, Lukasz
Hi Alex!

> -Original Message-
> From: Alexander Aring [mailto:alex.ar...@gmail.com]
> Sent: Tuesday, December 15, 2015 12:09
> To: Duda, Lukasz
> Cc: linux-w...@vger.kernel.org; linux-blueto...@vger.kernel.org;
> netdev@vger.kernel.org; ker...@pengutronix.de; m...@sandelman.ca;
> martin.gergel...@hs-rm.de
> Subject: Re: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for
> stateful compression
> 
> On Tue, Dec 15, 2015 at 10:29:51AM +, Duda, Lukasz wrote:
> > First of all great work for your series of patches on 6lowpan improvements
> and
> > stateful compression!
> >
> > I have just done some testing of this patch (without RADVD modifications),
> and
> > I can share my experiments using 6LoWPAN over BT-LE by sending simple
> ICMPv6
> > messages. Contexts for BTLE device has been added manually.
> >
> 
> did you test that with linux <-> linux? Or linux <->
> $SOME_OTHER_6LOWPAN_BTLE_STACK.
> 
> I tested it on my side with RIOT, it has 802.15.4 6LoWPAN support and
> also manipulate manually the context table.
> 

I have tested it with nRF52 BTLE device from Nordic Semiconductor 
with IoT SDK, and Linux Ubuntu with BTLE Dongle that acts as Router/Master.

> > Experiment 1:
> >
> > Router: 2001:db8::1/64 BTLE: 2001:db8::211:22FF:FE33:4455/64
> > CID 1: 2001:db8::/64
> >
> > Works fine, I see that CID 1 is used for both addresses. Router has 64 bits 
> > of
> > IID inline and BTLE node has 0.
> >
> > Experiment 2:
> >
> > Router: 2001:db8::1/64 BTLE: 2001:db8::211:22FF:FE33:4455/64
> > CID 3: 2001:db8::/64 CID 5: 2001:db8::1/128
> >
> > Works also fine, I see that both CID 3 and 5 are used, as well as both sides
> > compress its IID in the best possible way. So the patch appears to work fine
> on
> > 6LoWPAN over BT-LE.
> >
> 
> ok.
> 
> >
> > However, I notice that the folder created in the sys/kernel/debug/6lowpan/
> for
> > my bluetooth network interface is called "bt%d". And I would imagine this
> > should be "bt0", "bt1", ... and not the template?
> >
> 
> urgh, this should not happen. I use "dev->name" for that and dev is the
> netdevice structure. This should be an _unique_ interface name,
> otherwise you will getting trouble if you have two btle 6lowpan
> interfaces.
> 
> I didn't realized it because I create my interface with:
> 
> ip link add link wpan0 name lowpan0 type lowpan
> 
> but should change it into:
> 
> ip link add link wpan0 name lowpan%d type lowpan
> 
> I realized that the dev->name will be changed from template into "real"
> name after registering. This should do the job:
> 
> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> index c7f06f5..faf65ba 100644
> --- a/net/6lowpan/core.c
> +++ b/net/6lowpan/core.c
> @@ -29,13 +29,13 @@ int lowpan_register_netdevice(struct net_device
> *dev,
> 
> lowpan_priv(dev)->lltype = lltype;
> 
> -   ret = lowpan_dev_debugfs_init(dev);
> +   ret = register_netdevice(dev);
> if (ret < 0)
> return ret;
> 
> -   ret = register_netdevice(dev);
> +   ret = lowpan_dev_debugfs_init(dev);
> if (ret < 0)
> -   lowpan_dev_debugfs_exit(dev);
> +   unregister_netdevice(dev);
> 
> return ret;
>  }
> 
> I think it should be safe to do that after registering because we held the
> RTNL lock. And the interface isn't up after registering.
> 

Thanks! Your patch helped, and I acked it in separate mail thread.

> > Also, I notice that the compression for Flow Control and Traffic Label in 
> > IPv6
> > header has been modified, these fields are no longer compressed in any
> packets
> > (0b11 value) that comes from Linux Kernel (e.g. ICMP Echo Request,
> > Router Advertisement), instead I get three extra bytes (0b01 value).
> > I would like to understand reason for this modification a little better.
> 
> 0b11 means that traffic class and flow label are zero. Are you sure that
> these fields are zero inside the IPv6 header when you transmit
> "e.g. ICMP Echo Request, RA"?
> 
> Can you verify this by running tcpdump/wireshark? Or instruments some
> printk's at [0] for hdr->flow_lbl array and hdr->priority?
> 
> - Alex
> 
> [0] http://lxr.free-electrons.com/source/net/6lowpan/iphc.c#L428

I did some more linux debugging, and indeed, its IPv6 stack that already gives 
ip6hdr
with flow label set to some strange number. Do you know maybe the reason of 
this?
On Kernel version < 4.2 that field was always set to 0, thus better compression 
can
be applied.

Best regards,
Ɓukasz


RE: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for stateful compression

2015-12-15 Thread Duda, Lukasz
Hi Alex,

> -Original Message-
> From: Alexander Aring [mailto:alex.ar...@gmail.com]
> Sent: Monday, December 14, 2015 15:01
> To: linux-w...@vger.kernel.org
> Cc: linux-blueto...@vger.kernel.org; netdev@vger.kernel.org;
> ker...@pengutronix.de; m...@sandelman.ca; Duda, Lukasz;
> martin.gergel...@hs-rm.de; Alexander Aring
> Subject: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for stateful
> compression
> 
> This patch introduce support for IPHC stateful address compression. It will
> offer the context table via one debugfs entry.
> 
> Example to setup a context id:
> 
> A "cat /sys/kernel/debug/6lowpan/lowpan0/ctx_table" will display all
> contexts which are available. Example:
> 
> ID ipv6-address/prefix-length  flags
> 0  :::::::/0   0
> 1  :::::::/0   0
> 2  :::::::/0   0
> 3  :::::::/0   0
> 4  :::::::/0   0
> 5  :::::::/0   0
> 6  :::::::/0   0
> 7  :::::::/0   0
> 8  :::::::/0   0
> 9  :::::::/0   0
> 10 :::::::/0   0
> 11 :::::::/0   0
> 12 :::::::/0   0
> 13 :::::::/0   0
> 14 :::::::/0   0
> 15 :::::::/0   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:::::::/64 3" >
> /sys/kernel/debug/6lowpan/lowpan0/ctx_table
> 
> The flags are currently two:
> 
>  - BIT(0) - active: entry is added or deleted to the ctx_table.
>  - BIT(1) - c: compression flag according rfc6775.
> 
> 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 with the best compression
> method. 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.ar...@gmail.com>
> ---
>  include/net/6lowpan.h |  31 
>  net/6lowpan/core.c|   6 +-
>  net/6lowpan/debugfs.c |  97 
>  net/6lowpan/iphc.c| 420
> +++---
>  4 files changed, 496 insertions(+), 58 deletions(-)
> 
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h index
> 2f6a3f2..db636c8 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 /* 0101 = 65 */
>  #define LOWPAN_DISPATCH_IPHC 0x60 /* 011x = ... */
> @@ -98,9 +100,38 @@ enum lowpan_lltypes {
>   LOWPAN_LLTYPE_IEEE802154,
>  };
> 
> +enum lowpan_iphc_ctx_flags {
> + LOWPAN_IPHC_CTX_FLAG_ACTIVE = BIT(0),
> + LOWPAN_IPHC_CTX_FLAG_C  = BIT(1),
> +};
> +
> +struct lowpan_iphc_ctx {
> + u8 id;
> + struct in6_addr pfx;
> + u8 plen;
> + u32 flags;
> +};
> +
> +struct lowpan_iphc_ctx_table {
> + spinlock_t lock;
> + const struct lowpan_iphc_ctx_ops *ops;
> + struct lowpan_iphc_ctx table[LOWPAN_IPHC_CI_TABLE_SIZE]; };
> +
> +static inline bool lowpan_iphc_ctx_is_active(const struct
> +lowpan_iphc_ctx *ctx) {
> + return ctx->flags & LOWPAN_IPHC_CTX_FLAG_ACTIVE; }
> +
> +static inline bool lowpan_iphc_ctx_is_c(const struct lowpan_iphc_ctx
> +*ctx) {
> + return ctx->flags & LOWPAN_IPHC_CTX_FLAG_C; }
> +
>  struct lowpan_priv {
>   enum lowpan_lltypes lltype;
>   s

Re: [RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for stateful compression

2015-12-15 Thread Alexander Aring
On Tue, Dec 15, 2015 at 10:29:51AM +, Duda, Lukasz wrote:
> First of all great work for your series of patches on 6lowpan improvements and
> stateful compression! 
> 
> I have just done some testing of this patch (without RADVD modifications), and
> I can share my experiments using 6LoWPAN over BT-LE by sending simple ICMPv6
> messages. Contexts for BTLE device has been added manually.
> 

did you test that with linux <-> linux? Or linux <->
$SOME_OTHER_6LOWPAN_BTLE_STACK.

I tested it on my side with RIOT, it has 802.15.4 6LoWPAN support and
also manipulate manually the context table.

> Experiment 1:
> 
> Router: 2001:db8::1/64 BTLE: 2001:db8::211:22FF:FE33:4455/64
> CID 1: 2001:db8::/64
> 
> Works fine, I see that CID 1 is used for both addresses. Router has 64 bits of
> IID inline and BTLE node has 0.
> 
> Experiment 2:
> 
> Router: 2001:db8::1/64 BTLE: 2001:db8::211:22FF:FE33:4455/64
> CID 3: 2001:db8::/64 CID 5: 2001:db8::1/128
> 
> Works also fine, I see that both CID 3 and 5 are used, as well as both sides
> compress its IID in the best possible way. So the patch appears to work fine 
> on
> 6LoWPAN over BT-LE.
> 

ok.

> 
> However, I notice that the folder created in the sys/kernel/debug/6lowpan/ for
> my bluetooth network interface is called "bt%d". And I would imagine this
> should be "bt0", "bt1", ... and not the template?
> 

urgh, this should not happen. I use "dev->name" for that and dev is the
netdevice structure. This should be an _unique_ interface name,
otherwise you will getting trouble if you have two btle 6lowpan
interfaces.

I didn't realized it because I create my interface with:

ip link add link wpan0 name lowpan0 type lowpan

but should change it into:

ip link add link wpan0 name lowpan%d type lowpan

I realized that the dev->name will be changed from template into "real"
name after registering. This should do the job:

diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
index c7f06f5..faf65ba 100644
--- a/net/6lowpan/core.c
+++ b/net/6lowpan/core.c
@@ -29,13 +29,13 @@ int lowpan_register_netdevice(struct net_device *dev,
 
lowpan_priv(dev)->lltype = lltype;
 
-   ret = lowpan_dev_debugfs_init(dev);
+   ret = register_netdevice(dev);
if (ret < 0)
return ret;
 
-   ret = register_netdevice(dev);
+   ret = lowpan_dev_debugfs_init(dev);
if (ret < 0)
-   lowpan_dev_debugfs_exit(dev);
+   unregister_netdevice(dev);
 
return ret;
 }

I think it should be safe to do that after registering because we held the
RTNL lock. And the interface isn't up after registering.

> Also, I notice that the compression for Flow Control and Traffic Label in IPv6
> header has been modified, these fields are no longer compressed in any packets
> (0b11 value) that comes from Linux Kernel (e.g. ICMP Echo Request,
> Router Advertisement), instead I get three extra bytes (0b01 value).
> I would like to understand reason for this modification a little better.

0b11 means that traffic class and flow label are zero. Are you sure that
these fields are zero inside the IPv6 header when you transmit
"e.g. ICMP Echo Request, RA"?

Can you verify this by running tcpdump/wireshark? Or instruments some
printk's at [0] for hdr->flow_lbl array and hdr->priority?

- Alex

[0] http://lxr.free-electrons.com/source/net/6lowpan/iphc.c#L428
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFCv4 bluetooth-next 1/2] 6lowpan: iphc: add support for stateful compression

2015-12-14 Thread Alexander Aring
This patch introduce support for IPHC stateful address compression. It
will offer the context table via one debugfs entry.

Example to setup a context id:

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

ID ipv6-address/prefix-length  flags
0  :::::::/0   0
1  :::::::/0   0
2  :::::::/0   0
3  :::::::/0   0
4  :::::::/0   0
5  :::::::/0   0
6  :::::::/0   0
7  :::::::/0   0
8  :::::::/0   0
9  :::::::/0   0
10 :::::::/0   0
11 :::::::/0   0
12 :::::::/0   0
13 :::::::/0   0
14 :::::::/0   0
15 :::::::/0   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:::::::/64 3" >
/sys/kernel/debug/6lowpan/lowpan0/ctx_table

The flags are currently two:

 - BIT(0) - active: entry is added or deleted to the ctx_table.
 - BIT(1) - c: compression flag according rfc6775.

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 with the
best compression method. 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 
---
 include/net/6lowpan.h |  31 
 net/6lowpan/core.c|   6 +-
 net/6lowpan/debugfs.c |  97 
 net/6lowpan/iphc.c| 420 +++---
 4 files changed, 496 insertions(+), 58 deletions(-)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index 2f6a3f2..db636c8 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 /* 0101 = 65 */
 #define LOWPAN_DISPATCH_IPHC   0x60 /* 011x = ... */
@@ -98,9 +100,38 @@ enum lowpan_lltypes {
LOWPAN_LLTYPE_IEEE802154,
 };
 
+enum lowpan_iphc_ctx_flags {
+   LOWPAN_IPHC_CTX_FLAG_ACTIVE = BIT(0),
+   LOWPAN_IPHC_CTX_FLAG_C  = BIT(1),
+};
+
+struct lowpan_iphc_ctx {
+   u8 id;
+   struct in6_addr pfx;
+   u8 plen;
+   u32 flags;
+};
+
+struct lowpan_iphc_ctx_table {
+   spinlock_t lock;
+   const struct lowpan_iphc_ctx_ops *ops;
+   struct lowpan_iphc_ctx table[LOWPAN_IPHC_CI_TABLE_SIZE];
+};
+
+static inline bool lowpan_iphc_ctx_is_active(const struct lowpan_iphc_ctx *ctx)
+{
+   return ctx->flags & LOWPAN_IPHC_CTX_FLAG_ACTIVE;
+}
+
+static inline bool lowpan_iphc_ctx_is_c(const struct lowpan_iphc_ctx *ctx)
+{
+   return ctx->flags & LOWPAN_IPHC_CTX_FLAG_C;
+}
+
 struct lowpan_priv {
enum lowpan_lltypes lltype;
struct dentry *iface_debugfs;
+   struct lowpan_iphc_ctx_table ctx;
 
/* must be last */
u8 priv[0] __aligned(sizeof(void *));
diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
index c7f06f5..772f51c 100644
--- a/net/6lowpan/core.c
+++ b/net/6lowpan/core.c
@@ -20,7 +20,7 @@
 int lowpan_register_netdevice(struct net_device *dev,
  enum lowpan_lltypes lltype)
 {
-   int ret;
+   int i, ret;
 
dev->addr_len = EUI64_ADDR_LEN;
dev->type = ARPHRD_6LOWPAN;
@@ -29,6 +29,10 @@ int lowpan_register_netdevice(struct net_device *dev,
 
lowpan_priv(dev)->lltype = lltype;
 
+   spin_lock_init(_priv(dev)->ctx.lock);
+   for (i = 0; i < LOWPAN_IPHC_CI_TABLE_SIZE; i++)
+   lowpan_priv(dev)->ctx.table[i].id = i;
+
ret = lowpan_dev_debugfs_init(dev);
if (ret < 0)
return ret;
diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
index 88eef84..5270fa1 100644
--- a/net/6lowpan/debugfs.c
+++ b/net/6lowpan/debugfs.c
@@