Re: iptables very slow after commit 784544739a25c30637397ace5489eeb6e15d7d49

Stephen Hemminger
2009-04-12T16:06:25Z

On Sat, 11 Apr 2009 17:34:45 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Sat, Apr 11, 2009 at 11:57:16AM -0700, Linus Torvalds wrote:
> >
> >
> > On Fri, 10 Apr 2009, Paul E. McKenney wrote:
> > >
> > > 1. Assuming that the synchronize_net() is intended to guarantee
> > > that the new rules will be in effect before returning to
> > > user space:
> >
> > Btw, I think that's a bad assumption.
>
> It does indeed appear to be!
>
> > The thing is, nobody can really care if the new rules are in effect or
> > not, because the thing you race with is not the "return to user space"
> > part, but the incoming packets.
> >
> > And those incoming packets might have been incoming before the rules were
> > set up too.
> >
> > So I seriously doubt you need to synchronize with any returning to user
> > space. What you want to synchronize with is then later actions that do
> > things like turning on the interface that the rules are attached to etc!
> >
> > So I would suggest:
> >
> > - remove the synchronize_net() entirely. Replace it with just freeing the
> > old rules using RCU.
> >
> > - new packets will always end up seeing the new rules. That includes the
> > case of somebody doing "ifconfig eth0 up" that enables a new source of
> > packets, so there are no real security issues.
> >
> > - if you enabled your network interfaces before you updated your packet
> > filtering rules, you already had a window where packets would come in
> > with the old rules, so doing a "synchronize_net()" in no way protects
> > against any race conditions anyway.
> >
> > Am I missing something?
>
> The issue at this point seems to be the need to get accurate snapshots
> of various counters -- there are a number of Linux networking users who
> need to account for every byte flowing through their systems. However,
> it is also necessary to update these counters very efficiently, given
> that they are updated on a per-packet basis. The current approach is
> as follows:
>
> 1. Install a new set of counters.
>
> 2. Wait for a grace period to elapse.
>
> 3. At this point, we know that all subsequent counting will happen
> on the new set of counters.
>
> 4. Add the value of the old set of counters to the new set of
> counters.
>
> 5. Copy the old set of counters up to user space.
>
> So we get a good snapshot in #5, while #4 ensures that we don't lose
> any counts when taking future snapshots. Unfortunately, #2 hits us
> with grace-period latencies on the critical path.
>
> We are going through the following possibilities:
>
> o Stick with the current approach, and ask people to move to
> new batch-oriented interfaces. However, a 30x decrease in
> performance is pretty grim, even for an old-style interface.
>
> o Use various atomic tricks to get an immediate snapshot of the
> old counters after step 1. Make step 3 use call_rcu() instead
> of synchronize_rcu(), and then step 4 happens off the
> critical path.
>
> This approach moves the RCU grace period off of the critical
> path, but the atomic tricks are extremely ugly on 32-bit SMP
> machines. 32-bit UP machines and 64-bit machines are not
> too bad, though the 32-bit UP case does add preemption-disable
> overhead on the counter-update fastpath.
>
> o Provide some sort of expedited synchronize_rcu(). This might
> be able to decrease the hit from 30x down to maybe 5x.
> But I might need to do this for the fast-boot folks anyway,
> though I am first trying to get away with just speeding
> up synchronized_rcu(). Though I was not thinking in terms
> of 6x, let alone 30x.
>
> Please note that this would not be a drop-in replacement for
> synchronize_rcu(). One would use synchronize_rcu_expedited()
> (or whatever) only when the system really could not get any
> useful work done while the grace period was in progress.
> The general approach would be to keep the whole machine busy
> trying to get the grace period done as soon as possible.
>
> Thanx, Paul

We could also try:
* per-cpu spinlock on counters (instead of synchronize_net).
When doing update, just acquire
lock on that cpu and futz with counters then. Overhead should
still be less than 2.6.29 and earlier global rwlock

* synchonize_rcu/synchronize_net is more guarantee than needed?

* use on_each_cpu() somehow to do grace periood?

* Add a cond_resched() into net_rx_action which might cause rx processing
to get out of rcu sooner? also in transmit packet scheduler.