netdev - Re: [PATCH net v2] rps: Correct wrong skb_flow

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] [day] [month] [year] [list]

Date:   Fri, 11 May 2018 17:47:00 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Gao Feng <gfree.wind@....163.com>
Cc:     David Miller <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Willem de Bruijn <willemb@...gle.com>,
        jakub.kicinski@...ronome.com, ktkhai@...tuozzo.com,
        Alexei Starovoitov <ast@...nel.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        John Fastabend <john.fastabend@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net v2] rps: Correct wrong skb_flow_limit check when
 enable RPS

On Thu, May 10, 2018 at 6:09 PM,  <gfree.wind@....163.com> wrote:
> From: Gao Feng <gfree.wind@....163.com>
>
> The skb flow limit is implemented for each CPU independently. In the
> current codes, the function skb_flow_limit gets the softnet_data by
> this_cpu_ptr. But the target cpu of enqueue_to_backlog would be not
> the current cpu when enable RPS. As the result, the skb_flow_limit checks
> the stats of current CPU, while the skb is going to append the queue of
> another CPU. It isn't the expected behavior.
>
> Now pass the softnet_data as a param to make consistent.
>
> Fixes: 99bbc7074190 ("rps: selective flow shedding during softnet overflow")
> Signed-off-by: Gao Feng <gfree.wind@....163.com>

See also the discussion in the v1 of this patch.

The merits of moving flow_limit state from irq to rps cpu can
be argued, but the existing behavior is intentional and correct,
so this should not be applied to net and be backported to stable
branches.

My bad for reviving the discussion in the v1 thread while v2 was
already pending, sorry.