diff mbox

[RFC,bluetooth-next,18/20] 6lowpan: move multicast flags to generic

Message ID 20160711195044.25343-19-aar@pengutronix.de (mailing list archive)
State Superseded
Headers show

Commit Message

Alexander Aring July 11, 2016, 7:50 p.m. UTC
These flags should be all the same for 6LoWPAN so move it to 6LoWPAN generic.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
---
 net/6lowpan/core.c            | 1 +
 net/ieee802154/6lowpan/core.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

Comments

Alexander Aring July 12, 2016, 8:34 p.m. UTC | #1
Hi,

On 07/11/2016 09:50 PM, Alexander Aring wrote:
> These flags should be all the same for 6LoWPAN so move it to 6LoWPAN generic.
> 
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> ---
>  net/6lowpan/core.c            | 1 +
>  net/ieee802154/6lowpan/core.c | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> index a978000..6b7de14 100644
> --- a/net/6lowpan/core.c
> +++ b/net/6lowpan/core.c
> @@ -62,6 +62,7 @@ int lowpan_register_netdevice(struct net_device *dev,
>  	dev->type = ARPHRD_6LOWPAN;
>  	dev->mtu = IPV6_MIN_MTU;
>  	dev->priv_flags |= IFF_NO_QUEUE;
> +	dev->flags = IFF_BROADCAST | IFF_MULTICAST;
>  
>  	dev->header_ops = &header_ops;
>  
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 228a711..a60abad 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -92,7 +92,6 @@ static void lowpan_setup(struct net_device *ldev)
>  	memset(ldev->broadcast, 0xff, IEEE802154_ADDR_LEN);
>  	/* We need an ipv6hdr as minimum len when calling xmit */
>  	ldev->hard_header_len	= sizeof(struct ipv6hdr);

This should be moved to generic 6lowpan as well.

This says at least, the skb->len at xmit callback must be at least a
length of "sizeof(struct ipv6hdr)" which should be correct.

BUT...

I still have (more than years) the use-case that somebody sends an
AF_PACKET raw socket over lowpan interface.

I didn't test it yet, but I think this is a simple way to crash the
kernel on all lowpan interfaces. I currently not sure if I can break
something there, but I am sure it will send garbage data.

The xmit callback needs data which is available in skb_headroom, this
data is set by header_create callback which will not called on AF_PACKET
RAW sockets.

The root of this issue is that we don't have L2 here for creating mac
headers. The header_create callback should do that, but we do 6LoWPAN
adaptation here and the header_create callback will be used by ndisc to
say "here are the addresses, generate a mac header" and AF_PACKET
_DGRAM_ (for putting mac header in front of AF_PACKET payload).

We use this callback for the first use-case of ndisc only.

AF_PACKET RAW receive make sense, because tcpdump/wireshark needs to
capture data. Sending AF_PACKET RAW makes no sense and will I suppose
crash the kernel and I think every user can do that.

DGRAM sockets maybe makes sense, but I would disable that also for
(receive and transmit). You need to use PF_INET6 socket types for lowpan
interfaces only. That issue is somehow described at [0].

> -	ldev->flags		= IFF_BROADCAST | IFF_MULTICAST;
>  
>  	ldev->netdev_ops	= &lowpan_netdev_ops;
>  	ldev->destructor	= free_netdev;
> 

- Alex

[0] http://lxr.free-electrons.com/source/net/ieee802154/6lowpan/tx.c#L43
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jukka Rissanen July 13, 2016, 11:15 a.m. UTC | #2
Hi Alex,

On Tue, 2016-07-12 at 22:34 +0200, Alexander Aring wrote:
> Hi,
> 
> On 07/11/2016 09:50 PM, Alexander Aring wrote:
> > 
> > These flags should be all the same for 6LoWPAN so move it to
> > 6LoWPAN generic.
> > 
> > Signed-off-by: Alexander Aring <aar@pengutronix.de>
> > ---
> >  net/6lowpan/core.c            | 1 +
> >  net/ieee802154/6lowpan/core.c | 1 -
> >  2 files changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
> > index a978000..6b7de14 100644
> > --- a/net/6lowpan/core.c
> > +++ b/net/6lowpan/core.c
> > @@ -62,6 +62,7 @@ int lowpan_register_netdevice(struct net_device
> > *dev,
> >  	dev->type = ARPHRD_6LOWPAN;
> >  	dev->mtu = IPV6_MIN_MTU;
> >  	dev->priv_flags |= IFF_NO_QUEUE;
> > +	dev->flags = IFF_BROADCAST | IFF_MULTICAST;
> >  
> >  	dev->header_ops = &header_ops;
> >  
> > diff --git a/net/ieee802154/6lowpan/core.c
> > b/net/ieee802154/6lowpan/core.c
> > index 228a711..a60abad 100644
> > --- a/net/ieee802154/6lowpan/core.c
> > +++ b/net/ieee802154/6lowpan/core.c
> > @@ -92,7 +92,6 @@ static void lowpan_setup(struct net_device *ldev)
> >  	memset(ldev->broadcast, 0xff, IEEE802154_ADDR_LEN);
> >  	/* We need an ipv6hdr as minimum len when calling xmit */
> >  	ldev->hard_header_len	= sizeof(struct ipv6hdr);
> This should be moved to generic 6lowpan as well.
> 
> This says at least, the skb->len at xmit callback must be at least a
> length of "sizeof(struct ipv6hdr)" which should be correct.
> 
> BUT...
> 
> I still have (more than years) the use-case that somebody sends an
> AF_PACKET raw socket over lowpan interface.

According to packet(7), the "Packet sockets are used to receive or send
raw packets at the device driver (OSI Layer 2) level."
But lowpan0 interface is meant to compress IPv6 header so it is working
on L3 data (or more precisely between L2 and L3).
So I am wondering how this is suppose to work and what kind of use case
you have for this?

> 
> I didn't test it yet, but I think this is a simple way to crash the
> kernel on all lowpan interfaces. I currently not sure if I can break
> something there, but I am sure it will send garbage data.
> 
> The xmit callback needs data which is available in skb_headroom, this
> data is set by header_create callback which will not called on
> AF_PACKET
> RAW sockets.
> 
> The root of this issue is that we don't have L2 here for creating mac
> headers. The header_create callback should do that, but we do 6LoWPAN
> adaptation here and the header_create callback will be used by ndisc
> to
> say "here are the addresses, generate a mac header" and AF_PACKET
> _DGRAM_ (for putting mac header in front of AF_PACKET payload).
> 
> We use this callback for the first use-case of ndisc only.
> 
> AF_PACKET RAW receive make sense, because tcpdump/wireshark needs to
> capture data. Sending AF_PACKET RAW makes no sense and will I suppose
> crash the kernel and I think every user can do that.
> 
> DGRAM sockets maybe makes sense, but I would disable that also for
> (receive and transmit). You need to use PF_INET6 socket types for
> lowpan
> interfaces only. That issue is somehow described at [0].
> 
> > 
> > -	ldev->flags		= IFF_BROADCAST |
> > IFF_MULTICAST;
> >  
> >  	ldev->netdev_ops	= &lowpan_netdev_ops;
> >  	ldev->destructor	= free_netdev;
> > 
> - Alex
> 
> [0] http://lxr.free-electrons.com/source/net/ieee802154/6lowpan/tx.c#
> L43


Cheers,
Jukka


--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Aring July 14, 2016, 8:21 a.m. UTC | #3
Hi,

On 07/13/2016 01:15 PM, Jukka Rissanen wrote:
> Hi Alex,
> 
> On Tue, 2016-07-12 at 22:34 +0200, Alexander Aring wrote:
>> Hi,
>>
>> On 07/11/2016 09:50 PM, Alexander Aring wrote:
>>>
>>> These flags should be all the same for 6LoWPAN so move it to
>>> 6LoWPAN generic.
>>>
>>> Signed-off-by: Alexander Aring <aar@pengutronix.de>
>>> ---
>>>  net/6lowpan/core.c            | 1 +
>>>  net/ieee802154/6lowpan/core.c | 1 -
>>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
>>> index a978000..6b7de14 100644
>>> --- a/net/6lowpan/core.c
>>> +++ b/net/6lowpan/core.c
>>> @@ -62,6 +62,7 @@ int lowpan_register_netdevice(struct net_device
>>> *dev,
>>>  	dev->type = ARPHRD_6LOWPAN;
>>>  	dev->mtu = IPV6_MIN_MTU;
>>>  	dev->priv_flags |= IFF_NO_QUEUE;
>>> +	dev->flags = IFF_BROADCAST | IFF_MULTICAST;
>>>  
>>>  	dev->header_ops = &header_ops;
>>>  
>>> diff --git a/net/ieee802154/6lowpan/core.c
>>> b/net/ieee802154/6lowpan/core.c
>>> index 228a711..a60abad 100644
>>> --- a/net/ieee802154/6lowpan/core.c
>>> +++ b/net/ieee802154/6lowpan/core.c
>>> @@ -92,7 +92,6 @@ static void lowpan_setup(struct net_device *ldev)
>>>  	memset(ldev->broadcast, 0xff, IEEE802154_ADDR_LEN);
>>>  	/* We need an ipv6hdr as minimum len when calling xmit */
>>>  	ldev->hard_header_len	= sizeof(struct ipv6hdr);
>> This should be moved to generic 6lowpan as well.
>>
>> This says at least, the skb->len at xmit callback must be at least a
>> length of "sizeof(struct ipv6hdr)" which should be correct.
>>
>> BUT...
>>
>> I still have (more than years) the use-case that somebody sends an
>> AF_PACKET raw socket over lowpan interface.
> 
> According to packet(7), the "Packet sockets are used to receive or send
> raw packets at the device driver (OSI Layer 2) level."
> But lowpan0 interface is meant to compress IPv6 header so it is working
> on L3 data (or more precisely between L2 and L3).
> So I am wondering how this is suppose to work and what kind of use case
> you have for this?
> 

I think the issue is more complex, forget what the man page says here.
We use this callback for something which isn't made for, because the
IPv6 ndisc will tell us over this callback "what's the destination L2
address".

Sorry, I wrote something here which confused you. I meant more I think
about the use-case that users could try to using AF_PACKET RAW sockets
here and I think they can kill the kernel with the right parameters.

Sending AF_PACKET RAW here makes completely no sense and should be
disabled. Because we set in this callback something in the skb_headroom
and xmit callback will use it.

IMPORTANT NOTE: this handling is weird, because we except here that
between header_create and xmit the IPv6 subsytem will not do
skb_put/skb_push anymore. I currently think they will never do that
because the header_create callback is normally there to put a mac header
to the socket buffer. So IPv6 creation should be mostly done.

Sending DGRAM sockets it could make sense somehow for a crazy use-case
which maybe under 0.1% users wants. You can give the ndisc information
from userspace over that, source and destination address.

Nevertheless I would disable AF_PACKET DGRAM/RAW handling, EXCEPT RAW
RECEIVE handling. This will already be used by wireshark/tcpdump/etc.

---

Alternative half solution:

I already thought about to simple make a re-lookup on ipv6 ndisc
cache for L3 destination address. Like we do for short address handling.

But this will not work for broadcast addresses, I would more use the
normally functionality in IPv6 to tell us the L2 destination address
which is "header_create".

I need to think about the real solution to handle everything, you could
also except that somebody sends IPv6 raw over AF_PACKET here, then
parsing L3 daddr and do such lookup on ndisc for L2 daddr, but there
exists this problem with broadcast. I need to look into IPv6 how we
otherwise can detect unicast or multicast, maybe it's just when L3 is
multicast, but I would not bet on that. I need to look into IPv6 to
get an answer.

---

The reason why it should be removed is also below:

>>
>> I didn't test it yet, but I think this is a simple way to crash the
>> kernel on all lowpan interfaces. I currently not sure if I can break
>> something there, but I am sure it will send garbage data.
>>
>> The xmit callback needs data which is available in skb_headroom, this
>> data is set by header_create callback which will not called on
>> AF_PACKET
>> RAW sockets.
>>
>> The root of this issue is that we don't have L2 here for creating mac
>> headers. The header_create callback should do that, but we do 6LoWPAN
>> adaptation here and the header_create callback will be used by ndisc
>> to
>> say "here are the addresses, generate a mac header" and AF_PACKET
>> _DGRAM_ (for putting mac header in front of AF_PACKET payload).
>>
>> We use this callback for the first use-case of ndisc only.
>>
>> AF_PACKET RAW receive make sense, because tcpdump/wireshark needs to
>> capture data. Sending AF_PACKET RAW makes no sense and will I suppose
>> crash the kernel and I think every user can do that.
>>
>> DGRAM sockets maybe makes sense, but I would disable that also for
>> (receive and transmit). You need to use PF_INET6 socket types for
>> lowpan
>> interfaces only. That issue is somehow described at [0].
>>

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Aring July 14, 2016, 8:36 a.m. UTC | #4
Hi,

On 07/14/2016 10:21 AM, Alexander Aring wrote:
> 
> Hi,
> 
> On 07/13/2016 01:15 PM, Jukka Rissanen wrote:
>> Hi Alex,
>>
>> On Tue, 2016-07-12 at 22:34 +0200, Alexander Aring wrote:
>>> Hi,
>>>
>>> On 07/11/2016 09:50 PM, Alexander Aring wrote:
>>>>
>>>> These flags should be all the same for 6LoWPAN so move it to
>>>> 6LoWPAN generic.
>>>>
>>>> Signed-off-by: Alexander Aring <aar@pengutronix.de>
>>>> ---
>>>>  net/6lowpan/core.c            | 1 +
>>>>  net/ieee802154/6lowpan/core.c | 1 -
>>>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
>>>> index a978000..6b7de14 100644
>>>> --- a/net/6lowpan/core.c
>>>> +++ b/net/6lowpan/core.c
>>>> @@ -62,6 +62,7 @@ int lowpan_register_netdevice(struct net_device
>>>> *dev,
>>>>  	dev->type = ARPHRD_6LOWPAN;
>>>>  	dev->mtu = IPV6_MIN_MTU;
>>>>  	dev->priv_flags |= IFF_NO_QUEUE;
>>>> +	dev->flags = IFF_BROADCAST | IFF_MULTICAST;
>>>>  
>>>>  	dev->header_ops = &header_ops;
>>>>  
>>>> diff --git a/net/ieee802154/6lowpan/core.c
>>>> b/net/ieee802154/6lowpan/core.c
>>>> index 228a711..a60abad 100644
>>>> --- a/net/ieee802154/6lowpan/core.c
>>>> +++ b/net/ieee802154/6lowpan/core.c
>>>> @@ -92,7 +92,6 @@ static void lowpan_setup(struct net_device *ldev)
>>>>  	memset(ldev->broadcast, 0xff, IEEE802154_ADDR_LEN);
>>>>  	/* We need an ipv6hdr as minimum len when calling xmit */
>>>>  	ldev->hard_header_len	= sizeof(struct ipv6hdr);
>>> This should be moved to generic 6lowpan as well.
>>>
>>> This says at least, the skb->len at xmit callback must be at least a
>>> length of "sizeof(struct ipv6hdr)" which should be correct.
>>>
>>> BUT...
>>>
>>> I still have (more than years) the use-case that somebody sends an
>>> AF_PACKET raw socket over lowpan interface.
>>
>> According to packet(7), the "Packet sockets are used to receive or send
>> raw packets at the device driver (OSI Layer 2) level."
>> But lowpan0 interface is meant to compress IPv6 header so it is working
>> on L3 data (or more precisely between L2 and L3).
>> So I am wondering how this is suppose to work and what kind of use case
>> you have for this?
>>
> 
> I think the issue is more complex, forget what the man page says here.
> We use this callback for something which isn't made for, because the
> IPv6 ndisc will tell us over this callback "what's the destination L2
> address".
> 
> Sorry, I wrote something here which confused you. I meant more I think
> about the use-case that users could try to using AF_PACKET RAW sockets
> here and I think they can kill the kernel with the right parameters.
> 
> Sending AF_PACKET RAW here makes completely no sense and should be
> disabled. Because we set in this callback something in the skb_headroom
> and xmit callback will use it.
> 
> IMPORTANT NOTE: this handling is weird, because we except here that
> between header_create and xmit the IPv6 subsytem will not do
> skb_put/skb_push anymore. I currently think they will never do that
> because the header_create callback is normally there to put a mac header
> to the socket buffer. So IPv6 creation should be mostly done.
> 
> Sending DGRAM sockets it could make sense somehow for a crazy use-case
> which maybe under 0.1% users wants. You can give the ndisc information
> from userspace over that, source and destination address.
> 
> Nevertheless I would disable AF_PACKET DGRAM/RAW handling, EXCEPT RAW
> RECEIVE handling. This will already be used by wireshark/tcpdump/etc.
> 
> ---
> 
> Alternative half solution:
> 
> I already thought about to simple make a re-lookup on ipv6 ndisc
> cache for L3 destination address. Like we do for short address handling.
> 
> But this will not work for broadcast addresses, I would more use the
> normally functionality in IPv6 to tell us the L2 destination address
> which is "header_create".
> 
> I need to think about the real solution to handle everything, you could
> also except that somebody sends IPv6 raw over AF_PACKET here, then
> parsing L3 daddr and do such lookup on ndisc for L2 daddr, but there
> exists this problem with broadcast. I need to look into IPv6 how we
> otherwise can detect unicast or multicast, maybe it's just when L3 is
> multicast, but I would not bet on that. I need to look into IPv6 to
> get an answer.
> 

But this handling to send RAW IPv6 on AF_PACKET should not be done over
AF_PACKET, there exists sockets in AF_INET6 to do that. So I think
disable AF_PACKET (except RAW rx functionality) would be okay...
using AF_PACKET for such use-case is wrong anyway.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Richardson July 19, 2016, 2:51 p.m. UTC | #5
Jukka Rissanen <jukka.rissanen@linux.intel.com> wrote:
    > According to packet(7), the "Packet sockets are used to receive or send
    > raw packets at the device driver (OSI Layer 2) level."
    > But lowpan0 interface is meant to compress IPv6 header so it is working
    > on L3 data (or more precisely between L2 and L3).
    > So I am wondering how this is suppose to work and what kind of use case
    > you have for this?

First, one might want to send 802.15.4 packets with custom Information
Elements. So you ask why we would want to do this through the lowpan
interface... well, because lowpan will perhaps grow support for the 6tisch
(802.15.4e) mode of operation, and the custom IEs need to be scheduled out
with the other packets.   In particular, you don't want to jump the 6lowpan
queue of fragments.

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mcr@sandelman.ca  http://www.sandelman.ca/        |   ruby on rails    [

--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Aring July 19, 2016, 6:20 p.m. UTC | #6
Hi,

On 07/19/2016 04:51 PM, Michael Richardson wrote:
> Jukka Rissanen <jukka.rissanen@linux.intel.com> wrote:
>     > According to packet(7), the "Packet sockets are used to receive or send
>     > raw packets at the device driver (OSI Layer 2) level."
>     > But lowpan0 interface is meant to compress IPv6 header so it is working
>     > on L3 data (or more precisely between L2 and L3).
>     > So I am wondering how this is suppose to work and what kind of use case
>     > you have for this?
> 
> First, one might want to send 802.15.4 packets with custom Information
> Elements. So you ask why we would want to do this through the lowpan
> interface... well, because lowpan will perhaps grow support for the 6tisch
> (802.15.4e) mode of operation, and the custom IEs need to be scheduled out
> with the other packets.   In particular, you don't want to jump the 6lowpan
> queue of fragments.
> 

I currently think this discussion goes into the wrong direction.

Here exists an issue that transmit AF_PACKET RAW will not work,
DGRAM (tx/rx) sockets has also no sense.

The root of this issue is the "header_create" callback and what's made
for. This callback is used to tell the subsystem "Please add a mac
header here and as parameter you have L2 source and destination address".

This callback will be used by AF_PACKET _DGRAM_ and IPv6 ndisc cache.
Both of them has some point "please add a mac header, here are source
and destination L2 addresses".

So we don't put any mac header there, our use-case is "IPv6 ndisc cache"
only. We need the destination address from the ndisc and this callback
is the super generic way to tel us that by IPv6 stack and I would use
this callback as well, because it seems to be the normal IPv6 way.

I already told that we don't put a mac header there, what we do is
adding into a hidden skb space (skb_headroom) the address information
(important here the L2 daddr). This hidden skb space will be evaluated
inside the xmit callback where we finally replace the IPv6 header with
6LoWPAN and put the mac header in there.

So why we don't replace the header in header_create? This has several
issues:

 - We need to have a unshared skb buf and we cannot unshare in this callback.
 - We need to have the IPv6 view in AF_PACKET RAW receive side and this
   will be caputered between header_create and xmit.

Sending on AF_PACKET RAW sockets will occur that "create header"
callback will not be called and xmit callback directly. The xmit callback will
evaluate the "hidden skb room" (skb_headroom), we don't called "create_header"
before so there is garbage inside this skb_headroom.

btw:

What I mentioned before, this handling is weird but it works for IPv6.
We assume here that nobody runs skb_put/skb_push between "header_create"
and "xmit".

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
index a978000..6b7de14 100644
--- a/net/6lowpan/core.c
+++ b/net/6lowpan/core.c
@@ -62,6 +62,7 @@  int lowpan_register_netdevice(struct net_device *dev,
 	dev->type = ARPHRD_6LOWPAN;
 	dev->mtu = IPV6_MIN_MTU;
 	dev->priv_flags |= IFF_NO_QUEUE;
+	dev->flags = IFF_BROADCAST | IFF_MULTICAST;
 
 	dev->header_ops = &header_ops;
 
diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 228a711..a60abad 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -92,7 +92,6 @@  static void lowpan_setup(struct net_device *ldev)
 	memset(ldev->broadcast, 0xff, IEEE802154_ADDR_LEN);
 	/* We need an ipv6hdr as minimum len when calling xmit */
 	ldev->hard_header_len	= sizeof(struct ipv6hdr);
-	ldev->flags		= IFF_BROADCAST | IFF_MULTICAST;
 
 	ldev->netdev_ops	= &lowpan_netdev_ops;
 	ldev->destructor	= free_netdev;