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

[Alexander Aring <aar@pengutronix.de>]

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