netdev - Re: [PATCH net-2.6] pkt_sched: gen

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]

Message-ID: <1275971457.2775.40.camel@edumazet-laptop>
Date:	Tue, 08 Jun 2010 06:30:57 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Changli Gao <xiaosuo@...il.com>
Cc:	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Stephen Hemminger <shemminger@...tta.com>,
	Jarek Poplawski <jarkao2@...il.com>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH net-2.6] pkt_sched: gen_estimator: add a new lock

Le mardi 08 juin 2010 à 09:00 +0800, Changli Gao a écrit :
> On Tue, Jun 8, 2010 at 1:18 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> >
> >
> > [PATCH net-2.6] pkt_sched: gen_estimator: add a new lock
> >
> > gen_kill_estimator() / gen_new_estimator() is not always called with
> > RTNL held.
> >
> > net/netfilter/xt_RATEEST.c is one user of these API that do not hold
> > RTNL, so random corruptions can occur between "tc" and "iptables"
> >
> > Add a new fine grained lock instead of trying to use RTNL in xt_RATEEST
> >
> 
> Why not use RTNL in xt_RATEEST? It seems xt_RATEEST misuse the APIs.
> 
> and I think gen_replace_estimator is expected to be an atomic operation.
> 
> And gen_estimator_active() is also assumed to be called with RTNL locked.
> 

Thank you for asking this question.

Because I want to kill RTNL when possible, I dont even want to try
adding RTNL to xt_RATEEST and solve all lock dependencies it might
raise.

RTNL = Big and Horrible Network LOCK

You never got blocked because of this RTNL thing, dont you ?

I did. And it sucks, because when you hit this, you are in a hurry and
locating the bottleneck takes lot of time.

RTNL is the thing we must hold during device register / unregister.
Its locked for long delays because of all synchronize_rcu() that must be
done, and that is already a big problem on some setups.

Every time someone adds a RTNL requirement, you can be sure another guy
will zap it during following ten years.

Let's do this right now, not later.

For an example of horrible rtnl behavior, take a look at following
construct :

if (!rtnl_trylock())
	return restart_syscall();

I saw hundred of udev looping, trying to get rtnl to dump some
information. (Patrick added a rtnl requirement to all dump operations,
and it sucks)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html