[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65634d660904202026r7d73f810s700bacb8756e0967@mail.gmail.com>
Date: Mon, 20 Apr 2009 20:26:13 -0700
From: Tom Herbert <therbert@...gle.com>
To: Andi Kleen <andi@...stfloor.org>
Cc: netdev@...r.kernel.org, David Miller <davem@...emloft.net>
Subject: Re: [PATCH] Software receive packet steering
On Mon, Apr 20, 2009 at 3:32 AM, Andi Kleen <andi@...stfloor.org> wrote:
>
> Tom Herbert <therbert@...gle.com> writes:
>
> > +static int netif_cpu_for_rps(struct net_device *dev, struct sk_buff *skb)
> > +{
> > + cpumask_t mask;
> > + unsigned int hash;
> > + int cpu, count = 0;
> > +
> > + cpus_and(mask, dev->soft_rps_cpus, cpu_online_map);
> > + if (cpus_empty(mask))
> > + return smp_processor_id();
>
> There's a race here with CPU hotunplug I think. When a CPU is hotunplugged
> in parallel you can still push packets to it even though they are not
> drained. You probably need some kind of drain callback in a CPU hotunplug
> notifier that eats all packets left over.
>
We will look at that, the hotplug support may very well be lacking in the patch.
> > +got_hash:
> > + hash %= cpus_weight_nr(mask);
>
> That looks rather heavyweight even on modern CPUs. I bet it's 40-50+ cycles
> alone forth the hweight and the division. Surely that can be done better?
>
Agreed, I will try to pull in the RX hash from Dave Miller's remote
softirq patch.
> Also I suspect some kind of runtime switch for this would be useful.
>
> Also the manual set up of the receive mask seems really clumpsy. Couldn't
> you set that up dynamically based on where processes executing recvmsg()
> are running?
>
We have done exactly that. It works very well in many cases
(application + platform combinations), but I haven't found it to be
better than doing the hash in all cases. I could provide the patch,
but it might be more of a follow patch to this base one.
Thanks,
Tom
--
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
Powered by blists - more mailing lists