Re: [PATCH v5 6/6] 6lowpan: Fix IID format for Bluetooth

2017-02-28 Thread Jukka Rissanen 
Hi Luiz,

On Fri, 2017-02-24 at 14:14 +0200, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz 
> 
> Accourding to RFC 7668 U/L bit shall not be used:
> 
> https://wiki.tools.ietf.org/html/rfc7668#section-3.2.2 [Page 10]:
> 
>    In the figure, letter 'b' represents a bit from the
>    Bluetooth device address, copied as is without any changes on any
>    bit.  This means that no bit in the IID indicates whether the
>    underlying Bluetooth device address is public or random.
> 
>    |0  1|1  3|3  4|4 
>  6|
>    |0  5|6  1|2  7|8 
>  3|
>    ++++
> +
>    |||1110|bb
> bb|
>    ++++
> +
> 
> Because of this the code cannot figure out the address type from the
> IP
> address anymore thus it makes no sense to use peer_lookup_ba as it
> needs
> the peer address type.
> 
> Signed-off-by: Luiz Augusto von Dentz 
> ---
>  include/net/6lowpan.h   |  4 ---
> 

Looks good.

Acked-by: Jukka Rissanen 


Cheers,
Jukka

Re: [PATCH v5 6/6] 6lowpan: Fix IID format for Bluetooth

2017-02-25 Thread Alexander Aring 

Hi,

On 02/26/2017 07:05 AM, Alexander Aring wrote:
> 
> Hi,
> 
> On 02/24/2017 01:14 PM, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz 
>>
>> Accourding to RFC 7668 U/L bit shall not be used:
>>
>> https://wiki.tools.ietf.org/html/rfc7668#section-3.2.2 [Page 10]:
>>
>>In the figure, letter 'b' represents a bit from the
>>Bluetooth device address, copied as is without any changes on any
>>bit.  This means that no bit in the IID indicates whether the
>>underlying Bluetooth device address is public or random.
>>
>>|0  1|1  3|3  4|4  6|
>>|0  5|6  1|2  7|8  3|
>>+++++
>>|||1110||
>>+++++
>>
>> Because of this the code cannot figure out the address type from the IP
>> address anymore thus it makes no sense to use peer_lookup_ba as it needs
>> the peer address type.
>>
> 
> I am still not quite 100% of this and want to leave my opinion about this
> handling which can be interpreted in a different way.
> 
> The RFC says here:
> 
> Following the guidance of [RFC7136], a 64-bit
> Interface Identifier (IID) is formed from the 48-bit Bluetooth device
> address by inserting two octets, with hexadecimal values of 0xFF and
> 0xFE in the middle of the 48-bit Bluetooth device address as shown in
> Figure 6. 

Okay, they said from IID from the 48-bit address. 

And IID is what you need here and what [RFC7136] describes as result,
so I think you are right.

There is no need for special u/l bitflip or link-layer multicast handling.

- Alex

Re: [PATCH v5 6/6] 6lowpan: Fix IID format for Bluetooth

2017-02-25 Thread Alexander Aring 

Hi,

On 02/24/2017 01:14 PM, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz 
> 
> Accourding to RFC 7668 U/L bit shall not be used:
> 
> https://wiki.tools.ietf.org/html/rfc7668#section-3.2.2 [Page 10]:
> 
>In the figure, letter 'b' represents a bit from the
>Bluetooth device address, copied as is without any changes on any
>bit.  This means that no bit in the IID indicates whether the
>underlying Bluetooth device address is public or random.
> 
>|0  1|1  3|3  4|4  6|
>|0  5|6  1|2  7|8  3|
>+++++
>|||1110||
>+++++
> 
> Because of this the code cannot figure out the address type from the IP
> address anymore thus it makes no sense to use peer_lookup_ba as it needs
> the peer address type.
> 

I am still not quite 100% of this and want to leave my opinion about this
handling which can be interpreted in a different way.

The RFC says here:

Following the guidance of [RFC7136], a 64-bit
Interface Identifier (IID) is formed from the 48-bit Bluetooth device
address by inserting two octets, with hexadecimal values of 0xFF and
0xFE in the middle of the 48-bit Bluetooth device address as shown in
Figure 6.  In the figure, letter 'b' represents a bit from the
Bluetooth device address, copied as is without any changes on any
bit.

You cut the important part "Following the guidance of [RFC7136]".

---

What you describe is "How to see the link layer address from L3 layer"
and then it clearly said "there is no bit which indicate something
special that the address is public/random, whatever".

---

At my point of you the RFC tells here: grab the L2 layer in the way how
you quote above, but then apply [RFC7136] which is L3 handling.

I simple cc now some 6lo ietf stuff and others 6lowpan hackers, maybe we
have luck and somebody can answer this question.

---

And when we already about to start a discussion about that, what is do
to with the multicast bit of L2 address? I know you told it's not an
EUI-48 address. Are you sure?

If it's really EUI-48 then I think [RFC7136] should be applied.

- Alex

Re: [PATCH v5 6/6] 6lowpan: Fix IID format for Bluetooth

2017-02-24 Thread Stefan Schmidt 

Hello.

On 02/24/2017 01:14 PM, Luiz Augusto von Dentz wrote:

From: Luiz Augusto von Dentz 

Accourding to RFC 7668 U/L bit shall not be used:

https://wiki.tools.ietf.org/html/rfc7668#section-3.2.2 [Page 10]:

   In the figure, letter 'b' represents a bit from the
   Bluetooth device address, copied as is without any changes on any
   bit.  This means that no bit in the IID indicates whether the
   underlying Bluetooth device address is public or random.

   |0  1|1  3|3  4|4  6|
   |0  5|6  1|2  7|8  3|
   +++++
   |||1110||
   +++++

Because of this the code cannot figure out the address type from the IP
address anymore thus it makes no sense to use peer_lookup_ba as it needs
the peer address type.

Signed-off-by: Luiz Augusto von Dentz 
---
 include/net/6lowpan.h   |  4 ---
 net/bluetooth/6lowpan.c | 79 -
 2 files changed, 12 insertions(+), 71 deletions(-)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index c5792cb..a713780 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -211,10 +211,6 @@ static inline void 
lowpan_iphc_uncompress_eui48_lladdr(struct in6_addr *ipaddr,
ipaddr->s6_addr[11] = 0xFF;
ipaddr->s6_addr[12] = 0xFE;
memcpy(>s6_addr[13], lladdr + 3, 3);
-   /* second bit-flip (Universe/Local)
-* is done according RFC2464
-*/
-   ipaddr->s6_addr[8] ^= 0x02;
 }

 #ifdef DEBUG
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 0b68cfc..ec89c55 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -398,37 +398,6 @@ static int chan_recv_cb(struct l2cap_chan *chan, struct 
sk_buff *skb)
return err;
 }

-static u8 get_addr_type_from_eui64(u8 byte)
-{
-   /* Is universal(0) or local(1) bit */
-   return ((byte & 0x02) ? BDADDR_LE_RANDOM : BDADDR_LE_PUBLIC);
-}
-
-static void copy_to_bdaddr(struct in6_addr *ip6_daddr, bdaddr_t *addr)
-{
-   u8 *eui64 = ip6_daddr->s6_addr + 8;
-
-   addr->b[0] = eui64[7];
-   addr->b[1] = eui64[6];
-   addr->b[2] = eui64[5];
-   addr->b[3] = eui64[2];
-   addr->b[4] = eui64[1];
-   addr->b[5] = eui64[0];
-}
-
-static void convert_dest_bdaddr(struct in6_addr *ip6_daddr,
-   bdaddr_t *addr, u8 *addr_type)
-{
-   copy_to_bdaddr(ip6_daddr, addr);
-
-   /* We need to toggle the U/L bit that we got from IPv6 address
-* so that we get the proper address and type of the BD address.
-*/
-   addr->b[5] ^= 0x02;
-
-   *addr_type = get_addr_type_from_eui64(addr->b[5]);
-}
-
 static int setup_header(struct sk_buff *skb, struct net_device *netdev,
bdaddr_t *peer_addr, u8 *peer_addr_type)
 {
@@ -436,8 +405,7 @@ static int setup_header(struct sk_buff *skb, struct 
net_device *netdev,
struct ipv6hdr *hdr;
struct lowpan_btle_dev *dev;
struct lowpan_peer *peer;
-   bdaddr_t addr, *any = BDADDR_ANY;
-   u8 *daddr = any->b;
+   u8 *daddr;
int err, status = 0;

hdr = ipv6_hdr(skb);
@@ -448,34 +416,24 @@ static int setup_header(struct sk_buff *skb, struct 
net_device *netdev,

if (ipv6_addr_is_multicast(_daddr)) {
lowpan_cb(skb)->chan = NULL;
+   daddr = NULL;
} else {
-   u8 addr_type;
+   BT_DBG("dest IP %pI6c", _daddr);

-   /* Get destination BT device from skb.
-* If there is no such peer then discard the packet.
+   /* The packet might be sent to 6lowpan interface
+* because of routing (either via default route
+* or user set route) so get peer according to
+* the destination address.
 */
-   convert_dest_bdaddr(_daddr, , _type);
-
-   BT_DBG("dest addr %pMR type %d IP %pI6c", ,
-  addr_type, _daddr);
-
-   peer = peer_lookup_ba(dev, , addr_type);
+   peer = peer_lookup_dst(dev, _daddr, skb);
if (!peer) {
-   /* The packet might be sent to 6lowpan interface
-* because of routing (either via default route
-* or user set route) so get peer according to
-* the destination address.
-*/
-   peer = peer_lookup_dst(dev, _daddr, skb);
-   if (!peer) {
-   BT_DBG("no such peer %pMR found", );
-   return -ENOENT;
-   }
+   BT_DBG("no

[PATCH v5 6/6] 6lowpan: Fix IID format for Bluetooth

2017-02-24 Thread Luiz Augusto von Dentz 
From: Luiz Augusto von Dentz 

Accourding to RFC 7668 U/L bit shall not be used:

https://wiki.tools.ietf.org/html/rfc7668#section-3.2.2 [Page 10]:

   In the figure, letter 'b' represents a bit from the
   Bluetooth device address, copied as is without any changes on any
   bit.  This means that no bit in the IID indicates whether the
   underlying Bluetooth device address is public or random.

   |0  1|1  3|3  4|4  6|
   |0  5|6  1|2  7|8  3|
   +++++
   |||1110||
   +++++

Because of this the code cannot figure out the address type from the IP
address anymore thus it makes no sense to use peer_lookup_ba as it needs
the peer address type.

Signed-off-by: Luiz Augusto von Dentz 
---
 include/net/6lowpan.h   |  4 ---
 net/bluetooth/6lowpan.c | 79 -
 2 files changed, 12 insertions(+), 71 deletions(-)

diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index c5792cb..a713780 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -211,10 +211,6 @@ static inline void 
lowpan_iphc_uncompress_eui48_lladdr(struct in6_addr *ipaddr,
ipaddr->s6_addr[11] = 0xFF;
ipaddr->s6_addr[12] = 0xFE;
memcpy(>s6_addr[13], lladdr + 3, 3);
-   /* second bit-flip (Universe/Local)
-* is done according RFC2464
-*/
-   ipaddr->s6_addr[8] ^= 0x02;
 }
 
 #ifdef DEBUG
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 0b68cfc..ec89c55 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -398,37 +398,6 @@ static int chan_recv_cb(struct l2cap_chan *chan, struct 
sk_buff *skb)
return err;
 }
 
-static u8 get_addr_type_from_eui64(u8 byte)
-{
-   /* Is universal(0) or local(1) bit */
-   return ((byte & 0x02) ? BDADDR_LE_RANDOM : BDADDR_LE_PUBLIC);
-}
-
-static void copy_to_bdaddr(struct in6_addr *ip6_daddr, bdaddr_t *addr)
-{
-   u8 *eui64 = ip6_daddr->s6_addr + 8;
-
-   addr->b[0] = eui64[7];
-   addr->b[1] = eui64[6];
-   addr->b[2] = eui64[5];
-   addr->b[3] = eui64[2];
-   addr->b[4] = eui64[1];
-   addr->b[5] = eui64[0];
-}
-
-static void convert_dest_bdaddr(struct in6_addr *ip6_daddr,
-   bdaddr_t *addr, u8 *addr_type)
-{
-   copy_to_bdaddr(ip6_daddr, addr);
-
-   /* We need to toggle the U/L bit that we got from IPv6 address
-* so that we get the proper address and type of the BD address.
-*/
-   addr->b[5] ^= 0x02;
-
-   *addr_type = get_addr_type_from_eui64(addr->b[5]);
-}
-
 static int setup_header(struct sk_buff *skb, struct net_device *netdev,
bdaddr_t *peer_addr, u8 *peer_addr_type)
 {
@@ -436,8 +405,7 @@ static int setup_header(struct sk_buff *skb, struct 
net_device *netdev,
struct ipv6hdr *hdr;
struct lowpan_btle_dev *dev;
struct lowpan_peer *peer;
-   bdaddr_t addr, *any = BDADDR_ANY;
-   u8 *daddr = any->b;
+   u8 *daddr;
int err, status = 0;
 
hdr = ipv6_hdr(skb);
@@ -448,34 +416,24 @@ static int setup_header(struct sk_buff *skb, struct 
net_device *netdev,
 
if (ipv6_addr_is_multicast(_daddr)) {
lowpan_cb(skb)->chan = NULL;
+   daddr = NULL;
} else {
-   u8 addr_type;
+   BT_DBG("dest IP %pI6c", _daddr);
 
-   /* Get destination BT device from skb.
-* If there is no such peer then discard the packet.
+   /* The packet might be sent to 6lowpan interface
+* because of routing (either via default route
+* or user set route) so get peer according to
+* the destination address.
 */
-   convert_dest_bdaddr(_daddr, , _type);
-
-   BT_DBG("dest addr %pMR type %d IP %pI6c", ,
-  addr_type, _daddr);
-
-   peer = peer_lookup_ba(dev, , addr_type);
+   peer = peer_lookup_dst(dev, _daddr, skb);
if (!peer) {
-   /* The packet might be sent to 6lowpan interface
-* because of routing (either via default route
-* or user set route) so get peer according to
-* the destination address.
-*/
-   peer = peer_lookup_dst(dev, _daddr, skb);
-   if (!peer) {
-   BT_DBG("no such peer %pMR found", );
-   return -ENOENT;
-   }
+   BT_DBG("no such peer");
+   return -ENOENT;