netdev - Re: [PATCH net-next 1/4] net: dev: Remove the preempt_disable() in netif_rx

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, 3 Feb 2022 13:17:57 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Toke Høiland-Jørgensen <toke@...e.dk>
Cc:     Eric Dumazet <edumazet@...gle.com>, bpf <bpf@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH net-next 1/4] net: dev: Remove the preempt_disable() in
 netif_rx_internal().

On 2022-02-03 13:00:06 [+0100], Toke Høiland-Jørgensen wrote:
> > Here is the code in larger context:
> >
> > #ifdef CONFIG_RPS
> >     if (static_branch_unlikely(&rps_needed)) {
> >         struct rps_dev_flow voidflow, *rflow = &voidflow;
> >         int cpu;
> >
> >         preempt_disable();
> >         rcu_read_lock();
> >
> >         cpu = get_rps_cpu(skb->dev, skb, &rflow);
> >         if (cpu < 0)
> >             cpu = smp_processor_id();
> >
> >         ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
> >
> >         rcu_read_unlock();
> >         preempt_enable();
> >     } else
> > #endif
> >
> > This code needs the preempt_disable().
> 
> This is mostly so that the CPU ID stays the same throughout that section
> of code, though, right? So wouldn't it work to replace the
> preempt_disable() with a migrate_disable()? That should keep _RT happy,
> no?

It would but as mentioned previously: BH is disabled and
smp_processor_id() is stable.

> -Toke

Sebastian