Re: [PATCH] RPS: support 802.1q and pppoe session

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 25 mars 2010 à 13:12 +0800, Changli Gao a écrit :
> On Thu, Mar 25, 2010 at 1:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > While this might sounds a good idea, you really should split this in two
> > parts.
> >
> > By the way, why not handling IPIP too ?
> 
> I'm not sure if it is a good idea to support VLAN and PPPOE, and
> actually David don't like it. :(
> 
> >
> > Because I believe 802.1q part has no added value for instance, since
> > packet handled by CPUX will be decoded and passed to VLAN device, having
> > a chance to be fully taken by RPS, since we go back to netif_rx().
> >
> > Probably same thing for IPIP / PPPOE can be discussed.
> 
> It is useful when Linux is run as a bridge.
> 

I am not saying its not useful. BTW, for routers/bridges, RPS is not a
good idea, unless you must add netfilter complex rules.

> >
> > I agree we might need a flag or something to reset rxhash to 0 somewhere
> > (probably in non accelerated vlan rx handling) to force second
> > get_rps_cpu() invocation to recompute it. This small correction has no
> > cost if put outside of get_rps_cpus().
> >
> > If get_rps_cpus() is too complex, it might become too slow for typical
> > use. We should find smart ways to solve your performance problem if they
> > ever exist.
> >
> 
> It means that more than one IPI will be sent for just a single
> packets, I don't think the cost is acceptable.
> 

I believe you dont _fully_ understand how RPS currently works.

I am very surprised you send RPS patches if you dont master it.

Please read again get_rps_cpus(), line 2238

        default:
                goto done;   <<<< HERE skb->rxhash unchanged >>>>
        }
        ports = 0;
        switch (ip_proto) {


This means that unknown protocols are directly handled by THIS cpu, and
not given to another cpu. No IPI involved.

In case of tunnels or vlan, we then reenter lowlevel stack and at this
point, we can fully use RPS, because we are able to find IPV4/IPV6
headers.

Your patch is not necessary, since next time we call get_rps_cpus(),
rxhash being still null, we compute the correct non null hash and at
this point can choose an appropriate target cpu.

(All you need is to set /sys/class/net/vlan.825/queues/rx-0/rps_cpus to
needed mask)