netdev - Re: Fighting out-of-order reception with RPS?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALx6S37TMbi0s4LK_j=VyZB0-vEHBWF4nFJsNSfQOz5D-Q0ksw@mail.gmail.com>
Date:	Tue, 14 Jul 2015 11:02:16 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	Oliver Hartkopp <socketcan@...tkopp.net>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	Sunil Kovvuri <sunil.kovvuri@...il.com>,
	Jonathon Reinhart <jonathon.reinhart@...il.com>
Subject: Re: Fighting out-of-order reception with RPS?

On Tue, Jul 14, 2015 at 10:09 AM, Oliver Hartkopp
<socketcan@...tkopp.net> wrote:
> On 13.07.2015 06:57, Eric Dumazet wrote:
>>
>> On Sun, 2015-07-12 at 21:15 +0200, Oliver Hartkopp wrote:
>>
>>> E.g. with
>>>
>>>         skb_set_hash(skb, dev->ifindex, PKT_HASH_TYPE_L2);
>>>
>>> and
>>>
>>>         echo f > /sys/class/net/can0/queues/rx-0/rps_cpus
>>>
>>> I get properly ordered CAN frames - even with netif_rx() processed skbs.
>>> I
>>> just want to have this stuff to be enabled by default for CAN interfaces
>>> to
>>> kill the OOO frame issue.
>>
>>
>> I doubt your skb_set_hash() makes any difference.
>>
>> RPS prefers a L4 hash anyway (skb_get_hash()), so flow dissection
>> happens.
>>
>
> Please take a look into netif_rx_internal() in net/core/dev.c
>
> http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/net/core/dev.c?id=v4.2-rc1#n3486
>
> with CONFIG_RPS netif_rx() takes care about the rps cpu and puts the skb
> into the correct hash specific queue.
>
> As we usually have several PF_CAN sockets which get CAN frames from a
> specific CAN interface it makes no sense to enqueue packets into queues
> sorted by receiving sockets or L4 hash (we don't have L4 addressing on CAN).
>
> The skb_set_hash(skb, dev->ifindex, PKT_HASH_TYPE_L2) makes sure that the
> skbs from a specific CAN netdev are always processed in the same queue.
>
> When this is not wanted in 'fastpath netif_rx()' why is the CONFIG_RPS
> section in there?
>
> What is the advantage of implementing NAPI and a 'private sk_buf queue'
> suggested by Tom in http://marc.info/?l=linux-can&m=143681458003381&w=2 to
> set the hash as shown and enable rps_cpus?
>
> The latter just looks just like a complexity boost to have a functionality
> that already exists in netif_rx(). I just want to understand it.
>

netif_rx allows OOO packets and the way you're fixing that with RPS
and setting a "flow hash" are nothing more than hacks. Sorry, but
there is no way we are going to change RPS just to satisfy this your
case. If you want to do this right and get something in the kernel,
then implement NAPI for CAN drivers as has been suggested now by three
very experienced developers. This solves the your OOO problem and
moves drivers to NAPI which is the greatly preferred interface. I
really don't see why there needs to be any more discussion on this.

Tom



> Regards,
> Oliver
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html