Hi Alex,
On Fri, Feb 17, 2017 at 9:40 AM, Alexander Aring <aar@xxxxxxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> iphc code looks fine and better as before so far I see. Thanks!
>
> On 02/16/2017 06:19 PM, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>
>> This allow technologies such as Bluetooth to use its native lladdr which
>> is eui48 instead of eui64 which was expected by functions like
>> lowpan_header_decompress and lowpan_header_compress.
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>> ---
>> include/net/6lowpan.h | 19 +++++++++++++++++++
>> net/6lowpan/iphc.c | 49 +++++++++++++++++++++++++++++++++++++------------
>> net/bluetooth/6lowpan.c | 43 ++++++++++---------------------------------
>> 3 files changed, 66 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
>> index 5ab4c99..c5792cb 100644
>> --- a/include/net/6lowpan.h
>> +++ b/include/net/6lowpan.h
>> @@ -198,6 +198,25 @@ static inline void lowpan_iphc_uncompress_eui64_lladdr(struct in6_addr *ipaddr,
>> ipaddr->s6_addr[8] ^= 0x02;
>> }
>>
>> +static inline void lowpan_iphc_uncompress_eui48_lladdr(struct in6_addr *ipaddr,
>> + const void *lladdr)
>> +{
>> + /* fe:80::XXXX:XXff:feXX:XXXX
>> + * \_________________/
>> + * hwaddr
>> + */
>> + ipaddr->s6_addr[0] = 0xFE;
>> + ipaddr->s6_addr[1] = 0x80;
>> + memcpy(&ipaddr->s6_addr[8], lladdr, 3);
>> + ipaddr->s6_addr[11] = 0xFF;
>> + ipaddr->s6_addr[12] = 0xFE;
>> + memcpy(&ipaddr->s6_addr[13], lladdr + 3, 3);
>> + /* second bit-flip (Universe/Local)
>> + * is done according RFC2464
>> + */
>> + ipaddr->s6_addr[8] ^= 0x02;
>> +}
>> +
>> #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 fb5f6fa..ddbeeb7 100644
>> --- a/net/6lowpan/iphc.c
>> +++ b/net/6lowpan/iphc.c
>> @@ -278,6 +278,22 @@ lowpan_iphc_ctx_get_by_mcast_addr(const struct net_device *dev,
>> return ret;
>> }
>>
>> +static void lowpan_iphc_uncompress_lladdr(const struct net_device *dev,
>> + struct in6_addr *ipaddr,
>> + const void *lladdr)
>> +{
>> + switch (dev->addr_len) {
>> + case ETH_ALEN:
>> + lowpan_iphc_uncompress_eui48_lladdr(ipaddr, lladdr);
>> + break;
>> + case EUI64_ADDR_LEN:
>> + lowpan_iphc_uncompress_eui64_lladdr(ipaddr, lladdr);
>> + break;
>> + default:
>> + WARN_ON_ONCE(1);
>> + }
>> +}
>> +
>> /* Uncompress address function for source and
>> * destination address(non-multicast).
>> *
>> @@ -320,8 +336,7 @@ static int lowpan_iphc_uncompress_addr(struct sk_buff *skb,
>> lowpan_iphc_uncompress_802154_lladdr(ipaddr, lladdr);
>> break;
>> default:
>> - lowpan_iphc_uncompress_eui64_lladdr(ipaddr, lladdr);
>> - break;
>
> why removing this break?
>
>> + lowpan_iphc_uncompress_lladdr(dev, ipaddr, lladdr);
>> }
>> break;
>> default:
>> @@ -381,7 +396,7 @@ static int lowpan_iphc_uncompress_ctx_addr(struct sk_buff *skb,
>> lowpan_iphc_uncompress_802154_lladdr(ipaddr, lladdr);
>> break;
>> default:
>> - lowpan_iphc_uncompress_eui64_lladdr(ipaddr, lladdr);
>> + lowpan_iphc_uncompress_lladdr(dev, ipaddr, lladdr);
>> break;
>> }
>> ipv6_addr_prefix_copy(ipaddr, &ctx->pfx, ctx->plen);
>> @@ -810,6 +825,21 @@ lowpan_iphc_compress_ctx_802154_lladdr(const struct in6_addr *ipaddr,
>> return lladdr_compress;
>> }
>>
>> +static bool lowpan_iphc_addr_equal(const struct net_device *dev,
>> + const struct lowpan_iphc_ctx *ctx,
>> + const struct in6_addr *ipaddr,
>> + const void *lladdr)
>> +{
>> + struct in6_addr tmp = {};
>> +
>> + lowpan_iphc_uncompress_lladdr(dev, &tmp, lladdr);
>> +
>> + if (ctx)
>> + ipv6_addr_prefix_copy(&tmp, &ctx->pfx, ctx->plen);
>> +
>> + return ipv6_addr_equal(&tmp, ipaddr);
>> +}
>> +
>> static u8 lowpan_compress_ctx_addr(u8 **hc_ptr, const struct net_device *dev,
>> const struct in6_addr *ipaddr,
>> const struct lowpan_iphc_ctx *ctx,
>> @@ -827,13 +857,7 @@ static u8 lowpan_compress_ctx_addr(u8 **hc_ptr, const struct net_device *dev,
>> }
>> break;
>> default:
>> - /* check for SAM/DAM = 11 */
>> - memcpy(&tmp.s6_addr[8], lladdr, EUI64_ADDR_LEN);
>> - /* second bit-flip (Universe/Local) is done according RFC2464 */
>> - tmp.s6_addr[8] ^= 0x02;
>> - /* context information are always used */
>> - ipv6_addr_prefix_copy(&tmp, &ctx->pfx, ctx->plen);
>> - if (ipv6_addr_equal(&tmp, ipaddr)) {
>> + if (lowpan_iphc_addr_equal(dev, ctx, ipaddr, lladdr)) {
>> dam = LOWPAN_IPHC_DAM_11;
>> goto out;
>> }
>> @@ -929,11 +953,12 @@ static u8 lowpan_compress_addr_64(u8 **hc_ptr, const struct net_device *dev,
>> }
>> break;
>> default:
>> - if (is_addr_mac_addr_based(ipaddr, lladdr)) {
>> - dam = LOWPAN_IPHC_DAM_11; /* 0-bits */
>> + if (lowpan_iphc_addr_equal(dev, NULL, ipaddr, lladdr)) {
>> + dam = LOWPAN_IPHC_DAM_11;
>> pr_debug("address compression 0 bits\n");
>> goto out;
>> }
>> +
>> break;
>> }
>>
>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> index 1456b01..d2a1aa5 100644
>> --- a/net/bluetooth/6lowpan.c
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -64,7 +64,7 @@ struct lowpan_peer {
>> struct l2cap_chan *chan;
>>
>> /* peer addresses in various formats */
>> - unsigned char eui64_addr[EUI64_ADDR_LEN];
>> + unsigned char lladdr[ETH_ALEN];
>> struct in6_addr peer_addr;
>> };
>>
>> @@ -80,8 +80,6 @@ struct lowpan_btle_dev {
>> struct delayed_work notify_peers;
>> };
>>
>> -static void set_addr(u8 *eui, u8 *addr, u8 addr_type);
>> -
>> static inline struct lowpan_btle_dev *
>> lowpan_btle_dev(const struct net_device *netdev)
>> {
>> @@ -277,7 +275,6 @@ static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev,
>> const u8 *saddr;
>> struct lowpan_btle_dev *dev;
>> struct lowpan_peer *peer;
>> - unsigned char eui64_daddr[EUI64_ADDR_LEN];
>>
>> dev = lowpan_btle_dev(netdev);
>>
>> @@ -287,10 +284,9 @@ static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev,
>> if (!peer)
>> return -EINVAL;
>>
>> - saddr = peer->eui64_addr;
>> - set_addr(&eui64_daddr[0], chan->src.b, chan->src_type);
>> + saddr = peer->lladdr;
>>
>> - return lowpan_header_decompress(skb, netdev, &eui64_daddr, saddr);
>> + return lowpan_header_decompress(skb, netdev, netdev->dev_addr, saddr);
>> }
>>
>
> This is something which I don't understand.
>
> protoype of this function:
>
> int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
> const void *daddr, const void *saddr)
>
> your daddr pointer is "netdev->dev_addr", but this contains normally the L2 source
> address of your transceiver.
> You don't set the destination L2 address in your dev->dev_addr. (I hope)
>
> For me it looks like daddr == saddr, or should saddr be daddr, but then
> saddr (meaning here the saddr which should named daddr then, if it is daddr)
> should be as 3. parameter.
This is for decompressing a received packet and it actually don't
change the order of the parameters, the dev_addr is in fact the
chan->src byte swapped to be used as a MAC address if the interface,
there is nothing new in this regard. The only change that caused by
this is that we no longer convert the MAC into an IID.
>
>> static void ifup(struct net_device *netdev)
>> {
>> int err;
>> @@ -762,14 +737,16 @@ static struct l2cap_chan *add_peer_chan(struct l2cap_chan *chan,
>> peer->chan = chan;
>> memset(&peer->peer_addr, 0, sizeof(struct in6_addr));
>>
>> + baswap((void *)peer->lladdr, &chan->dst);
>> +
>> /* RFC 2464 ch. 5 */
>> peer->peer_addr.s6_addr[0] = 0xFE;
>> peer->peer_addr.s6_addr[1] = 0x80;
>> - set_addr((u8 *)&peer->peer_addr.s6_addr + 8, chan->dst.b,
>> - chan->dst_type);
>>
>> - memcpy(&peer->eui64_addr, (u8 *)&peer->peer_addr.s6_addr + 8,
>> - EUI64_ADDR_LEN);
>> + memcpy(&peer->peer_addr.s6_addr[8], peer->lladdr, 3);
>> + peer->peer_addr.s6_addr[11] = 0xFF;
>> + peer->peer_addr.s6_addr[12] = 0xFE;
>> + memcpy(&peer->peer_addr.s6_addr[13], peer->lladdr + 3, 3);
>>
>
> You need to explain me why you add FFFE here again, why you need to
> generate an address here on ifup?
This is just maintaining the same logic as there was in set_addr since
we no longer have the address as 8 bytes it needed to introduce the
filler bytes, I can actually replace this with a call to
lowpan_iphc_uncompress_eui48_lladdr which does contain the exact same
code.
> Normally this should not be needed inside 6LoWPAN L2 implementation...
> but I suppose this is a next step to fixing BTLE 6LoWPAN.
It is perhaps not necessary to generate an IP address here, but I did
not want to add unrelated changes, this might actually come in a
following patch if we deem necessary.
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
