netdev - Re: [PATCH net] net_sched: act_mirred: full rcu conversion

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:   Fri, 09 Sep 2016 05:23:57 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        John Fastabend <john.fastabend@...il.com>,
        Hadar Hen Zion <hadarh@...lanox.com>,
        Amir Vadai <amirva@...lanox.com>
Subject: Re: [PATCH net] net_sched: act_mirred: full rcu conversion

On Thu, 2016-09-08 at 22:24 -0700, Cong Wang wrote:
> On Thu, Sep 8, 2016 at 8:35 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> > From: Eric Dumazet <edumazet@...gle.com>
> >
> > As reported by Cong Wang, I was lazy when I did initial RCU conversion
> > of tc_mirred, as I thought I could avoid allocation/freeing of a
> > parameter block.
> 
> Quote from Eric Dumazet:
> 
> https://www.mail-archive.com/netdev@vger.kernel.org/msg115482.html
> 
> <Quote>
> Well, I added a READ_ONCE() to read tcf_action once.
> 
> Adding rcu here would mean adding a pointer and extra cache line, to
> deref the values.
> 
> IMHO the race here has no effect . You either read the old or new value.
> </Quote>
> 
> Me with facepalm... ;-)


Point is still valid. Show me a real case where it was a serious
problem, instead of simply theoretical.

tc_mirred + ifb patches allowed us to reach a milestone, removing the
last contended spinlocks, and you are catching up with this one year
later.

I wont backport this fix in Google prod kernels, because there is
absolutely no way we need it, and the extra memory cache line might hurt
latencies.

Since you did not write a fix on your side since June 17th, I presume
you do not care that much.