netdev - Re: [PATCH net 2/2] sfc: limit ARFS workitems in flight per channel

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]

Date:   Thu, 12 Apr 2018 16:24:46 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     David Miller <davem@...emloft.net>
CC:     <linux-net-drivers@...arflare.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net 2/2] sfc: limit ARFS workitems in flight per channel

On 12/04/18 16:11, David Miller wrote:
> From: Edward Cree <ecree@...arflare.com>
> Date: Thu, 12 Apr 2018 15:02:50 +0100
>
>> A misconfigured system (e.g. with all interrupts affinitised to all CPUs)
>>  may produce a storm of ARFS steering events.  With the existing sfc ARFS
>>  implementation, that could create a backlog of workitems that grinds the
>>  system to a halt.  To prevent this, limit the number of workitems that
>>  may be in flight for a given SFC device to 8 (EFX_RPS_MAX_IN_FLIGHT), and
>>  return EBUSY from our ndo_rx_flow_steer method if the limit is reached.
>> Given this limit, also store the workitems in an array of slots within the
>>  struct efx_nic, rather than dynamically allocating for each request.
>>
>> Signed-off-by: Edward Cree <ecree@...arflare.com>
> I don't think this behavior is all that great.
>
> If you really have to queue up these operations because they take a long
> time, I think it is better to enter a synchronous mode and sleep once
> you hit this in-flight limit of 8.
I don't think we can sleep at this point, ndo_rx_flow_steer is called from
 the RX path (netif_receive_skb_internal() -> get_rps_cpu() ->
 set_rps_cpu()).

> Either that or make the expiration work smarter when it has lots of events
> to process.
I'm afraid I don't understand what you mean here.
This code is not handling expiration of old ARFS filters, it's inserting
 new ones.

-Ed