netdev - Re: [PATCH] net: Adjust softirq raising in __napi

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, 22 Oct 2009 10:27:01 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Jarek Poplawski <jarkao2@...il.com>
Cc:	Tilman Schmidt <tilman@...p.cc>,
	David Miller <davem@...emloft.net>, hidave.darkstar@...il.com,
	linux-kernel@...r.kernel.org, tglx@...utronix.de,
	linux-wireless@...r.kernel.org, linux-ppp@...r.kernel.org,
	netdev@...r.kernel.org, paulus@...ba.org,
	Michael Buesch <mb@...sch.de>,
	Oliver Hartkopp <oliver@...tkopp.net>
Subject: Re: [PATCH] net: Adjust softirq raising in __napi_schedule

On Wed, 2009-10-21 at 23:39 +0200, Jarek Poplawski wrote:

> > > -	__raise_softirq_irqoff(NET_RX_SOFTIRQ);
> > > +	raise_softirq_irqoff(NET_RX_SOFTIRQ);
> > 
> > This still doesn't make any sense.
> > 
> > There may or may not be a lot of code that assumes that everything else
> > is run with other tasklets disabled, and that it cannot be interrupted
> > by a tasklet and thus create a race.
> > 
> > Can you prove that is not the case, across the entire networking layer?
> 
> I'm not sure I can understand your question. This patch is mainly to
> avoid using netif_rx()/netif_rx_ni() pair as a test of proper process
> context handling; IMHO there're better tools for this (lockdep,
> WARN_ON's).

And how exactly does that matter to the patch at hand?!

I'm saying that it seems to me, as indicated by the API (and without
proof otherwise that's how it is) the networking layer needs to have
packets handed to it with softirqs disabled. Therefore, this patch is
not needed. While it may not be _wrong_, it'll definitely introduce a
performance regression.

This really should be obvious. You're fixing the warning at the source
of the warning, rather than the source of the problem.

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)