* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Peter P Waskiewicz Jr @ 2009-11-24 19:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, peterz@infradead.org, arjan@linux.intel.com,
yong.zhang0@gmail.com, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, netdev@vger.kernel.org
In-Reply-To: <4B0C2D85.7020200@gmail.com>
On Tue, 2009-11-24 at 11:01 -0800, Eric Dumazet wrote:
> Peter P Waskiewicz Jr a écrit :
>
> > That's exactly what we're doing in our 10GbE driver right now (isn't
> > pushed upstream yet, still finalizing our testing). We spread to all
> > NUMA nodes in a semi-intelligent fashion when allocating our rings and
> > buffers. The last piece is ensuring the interrupts tied to the various
> > queues all route to the NUMA nodes those CPUs belong to. irqbalance
> > needs some kind of hint to make sure it does the right thing, which
> > today it does not.
>
> sk_buff allocations should be done on the node of the cpu handling rx interrupts.
Yes, but we preallocate the buffers to minimize overhead when running
our interrupt routines. Regardless, whatever queue we're filling with
those sk_buff's has an interrupt vector attached. So wherever the
descriptor ring/queue and its associated buffers were allocated, that is
where the interrupt's affinity needs to be set to.
> For rings, I am ok for irqbalance and driver cooperation, in case admin
> doesnt want to change the defaults.
>
> >
> > I don't see how this is complex though. Driver loads, allocates across
> > the NUMA nodes for optimal throughput, then writes CPU masks for the
> > NUMA nodes each interrupt belongs to. irqbalance comes along and looks
> > at the new mask "hint," and then balances that interrupt within that
> > hinted mask.
>
> So NUMA policy is given by the driver at load time ?
I think it would have to. Nobody else has insight how the driver
allocated its resources. So the driver can be told where to allocate
(see below), or the driver needs to indicate upwards how it allocated
resources.
> An admin might chose to direct all NIC trafic to a given node, because
> its machine has mixed workload. 3 nodes out of 4 for database workload,
> one node for network IO...
>
> So if an admin changes smp_affinity, is your driver able to reconfigure itself
> and re-allocate all its rings to be on NUMA node chosen by admin ? This is
> what I qualify as complex.
No, we don't want to go this route of reallocation. This, I agree, is
very complex, and can be very devastating. We'd basically be resetting
the driver whenever an interrupt moved, so this could be a terrible DoS
vulnerability.
Jesse Brandeburg has a set of patches he's working on that will allow us
to bind an interface to a single node. So in your example of 3 nodes
for DB workload and 1 for network I/O, the driver can be loaded and
directly bound to that 4th node. Then the node_affinity mask would be
set by the driver for the CPU mask of that single node. But in these
deployments, a sysadmin changing affinity that will fly directly in the
face of how resources are laid out is poor system administration. I
know it will happen, but I don't know how far we need to protect the
sysadmin from shooting themselves in the foot in terms of performance
tuning.
Cheers,
-PJ
^ permalink raw reply
* Re: NUMA and multiQ interation
From: Tom Herbert @ 2009-11-24 19:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091124.100613.240222762.davem@davemloft.net>
On Tue, Nov 24, 2009 at 10:06 AM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Tue, 24 Nov 2009 09:04:41 -0800
>
>> What is the expected interaction here, and would these results be
>> typical? If so, would this warrant the need to associate each RX
>> queue to a numa node, instead of just the device?
>
> Yes we are fully aware of this and discussed it at netconf this year, see
> in particular:
>
> http://vger.kernel.org/netconf2009_slides/netconf2009_numa_discussion.odp
>
> PJ is also currently trying to pass upstream some changes such that these
> NUMA allocation bits can be exported to userspace and thus irqbalanced
> can pick IRQ targetting more intelligently when a driver allocates per-queue
> memory resources on different NUMA nodes. The thread discussing this has
> been going active for the past few days, maybe you missed it.
>
I saw that thread, and it looks compelling. But we have applications
that are network bound such that we want to use multiple queues across
multiple nodes for scaling-- and trying keep all queues on the same
node does not scale very well. Maybe ignoring NUMA allocation could
be a fall-back mode in a dynamic allocation with heavy load?
Tom
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Eric Dumazet @ 2009-11-24 19:01 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: David Miller, peterz@infradead.org, arjan@linux.intel.com,
yong.zhang0@gmail.com, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, netdev@vger.kernel.org
In-Reply-To: <1259087601.2631.56.camel@ppwaskie-mobl2>
Peter P Waskiewicz Jr a écrit :
> That's exactly what we're doing in our 10GbE driver right now (isn't
> pushed upstream yet, still finalizing our testing). We spread to all
> NUMA nodes in a semi-intelligent fashion when allocating our rings and
> buffers. The last piece is ensuring the interrupts tied to the various
> queues all route to the NUMA nodes those CPUs belong to. irqbalance
> needs some kind of hint to make sure it does the right thing, which
> today it does not.
sk_buff allocations should be done on the node of the cpu handling rx interrupts.
For rings, I am ok for irqbalance and driver cooperation, in case admin
doesnt want to change the defaults.
>
> I don't see how this is complex though. Driver loads, allocates across
> the NUMA nodes for optimal throughput, then writes CPU masks for the
> NUMA nodes each interrupt belongs to. irqbalance comes along and looks
> at the new mask "hint," and then balances that interrupt within that
> hinted mask.
So NUMA policy is given by the driver at load time ?
An admin might chose to direct all NIC trafic to a given node, because
its machine has mixed workload. 3 nodes out of 4 for database workload,
one node for network IO...
So if an admin changes smp_affinity, is your driver able to reconfigure itself
and re-allocate all its rings to be on NUMA node chosen by admin ? This is
what I qualify as complex.
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Eric Dumazet @ 2009-11-24 18:58 UTC (permalink / raw)
To: David Miller
Cc: peter.p.waskiewicz.jr, peterz, arjan, yong.zhang0, linux-kernel,
arjan, netdev
In-Reply-To: <20091124.105442.06273019.davem@davemloft.net>
David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 24 Nov 2009 19:26:15 +0100
>
>> It seems complex to me, maybe optimal thing would be to use a NUMA policy to
>> spread vmalloc() allocations to all nodes to get a good bandwidth...
>
> vmalloc() and sk_buff's don't currently mix and I really don't see us
> every allowing them to :-)
I think Peter was referring to tx/rx rings buffers, not sk_buffs.
They (ring buffers) are allocated with vmalloc() at driver init time.
And Tom pointed out that our rx sk_buff allocation should be using the node
of requester, no need to hardcode node number per rx queue (or per device as of today)
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: David Miller @ 2009-11-24 18:54 UTC (permalink / raw)
To: eric.dumazet
Cc: peter.p.waskiewicz.jr, peterz, arjan, yong.zhang0, linux-kernel,
arjan, netdev
In-Reply-To: <4B0C2547.8030408@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 24 Nov 2009 19:26:15 +0100
> It seems complex to me, maybe optimal thing would be to use a NUMA policy to
> spread vmalloc() allocations to all nodes to get a good bandwidth...
vmalloc() and sk_buff's don't currently mix and I really don't see us
every allowing them to :-)
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Patrick McHardy @ 2009-11-24 18:38 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Arnd Bergmann, virtualization, Herbert Xu, Eric Dumazet,
Anna Fischer, netdev, bridge, linux-kernel, Mark Smith,
Gerhard Stenzel, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <m1aayb6bfg.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
>
>> In the layered case (macvlan -> eth0) its common behaviour to
>> keep the mark however. But in case of different namespaces,
>> I think macvlan should also clear the mark on the dev_queue_xmit()
>> path since this is just a shortcut to looping the packets
>> through veth. In fact probably both of them should also clear
>> skb->priority so other namespaces don't accidentally misclassify
>> packets.
>
> That is why I pushed for what is becoming dev_forward_skb. So that
> we have one place where we can make all of those tweaks. It seems
> like in every review we find another field that should be cleared/handled
> specially.
>
> I don't quite follow what you intend with dev_queue_xmit when the macvlan
> is in one namespace and the real physical device is in another. Are
> you mentioning that the packet classifier runs in the namespace where
> the primary device lives with packets from a different namespace?
Exactly. And I think we should make sure that the namespace of
the macvlan device can't (deliberately or accidentally) cause
misclassification.
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Peter P Waskiewicz Jr @ 2009-11-24 18:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, peterz@infradead.org, arjan@linux.intel.com,
yong.zhang0@gmail.com, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, netdev@vger.kernel.org
In-Reply-To: <4B0C2547.8030408@gmail.com>
On Tue, 2009-11-24 at 10:26 -0800, Eric Dumazet wrote:
> Peter P Waskiewicz Jr a écrit :
> That's the kind of thing PJ is trying to make available.
> >
> > Yes, that's exactly what I'm trying to do. Even further, we want to
> > allocate the ring SW struct itself and descriptor structures on other
> > NUMA nodes, and make sure the interrupt lines up with those allocations.
> >
>
> Say you allocate ring buffers on NUMA node of the CPU handling interrupt
> on a particular queue.
>
> If irqbalance or an admin changes /proc/irq/{number}/smp_affinities,
> do you want to realloc ring buffer to another NUMA node ?
>
That's why I'm trying to add the node_affinity mechanism that irqbalance
can use to prevent the interrupt being moved to another node.
> It seems complex to me, maybe optimal thing would be to use a NUMA policy to
> spread vmalloc() allocations to all nodes to get a good bandwidth...
That's exactly what we're doing in our 10GbE driver right now (isn't
pushed upstream yet, still finalizing our testing). We spread to all
NUMA nodes in a semi-intelligent fashion when allocating our rings and
buffers. The last piece is ensuring the interrupts tied to the various
queues all route to the NUMA nodes those CPUs belong to. irqbalance
needs some kind of hint to make sure it does the right thing, which
today it does not.
I don't see how this is complex though. Driver loads, allocates across
the NUMA nodes for optimal throughput, then writes CPU masks for the
NUMA nodes each interrupt belongs to. irqbalance comes along and looks
at the new mask "hint," and then balances that interrupt within that
hinted mask.
Cheers,
-PJ
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Arnd Bergmann @ 2009-11-24 18:28 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Patrick McHardy, virtualization, Herbert Xu, Eric Dumazet,
Anna Fischer, netdev, bridge, linux-kernel, Mark Smith,
Gerhard Stenzel, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <m1aayb6bfg.fsf@fess.ebiederm.org>
On Tuesday 24 November 2009, Eric W. Biederman wrote:
> I don't quite follow what you intend with dev_queue_xmit when the macvlan
> is in one namespace and the real physical device is in another. Are
> you mentioning that the packet classifier runs in the namespace where
> the primary device lives with packets from a different namespace?
I treat internal and external delivery very differently, the three
cases are:
1. skb from real device to macvlan (macvlan_handle_frame): basically
unchanged from before, except avoiding duplicate broadcasts. All
skbs end up in netif_rx(vlan->dev) without clearing any data.
We catch the frame in netif_receive_skb before it interacts with the
namespace of the real device.
2. skb to external device (macvlan_start_xmit): if the destination
is external, we just end up in dev_queue_xmit, with skb->dev set to
the external device but no other changes. The data is already on the
way out at this stage, so the namespace should not matter any more.
3. internal delivery: an skb from one macvlan to another gets always
sent through dev_forward_skb, which is supposed to clear anything
that must not leave the namespace.
Does this make sense?
Arnd <><
^ permalink raw reply
* Re: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-24 18:27 UTC (permalink / raw)
To: Allan, Bruce W
Cc: Andi Kleen, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, Frans Pop, Brandeburg, Jesse,
linux-kernel@vger.kernel.org, Caleb Cushing, Kirsher, Jeffrey T
In-Reply-To: <8DD2590731AB5D4C9DBF71A877482A903E2AA445@orsmsx509.amr.corp.intel.com>
On Tue, Nov 24, 2009 at 07:57:41AM -0800, Allan, Bruce W wrote:
> The "pci_enable_pcie_error_reporting failed" message is a non-fatal warning that has recently been removed.
>
Thanks for the explanation,
Jarek P.
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Eric Dumazet @ 2009-11-24 18:26 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: David Miller, peterz@infradead.org, arjan@linux.intel.com,
yong.zhang0@gmail.com, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, netdev@vger.kernel.org
In-Reply-To: <1259085412.2631.48.camel@ppwaskie-mobl2>
Peter P Waskiewicz Jr a écrit :
That's the kind of thing PJ is trying to make available.
>
> Yes, that's exactly what I'm trying to do. Even further, we want to
> allocate the ring SW struct itself and descriptor structures on other
> NUMA nodes, and make sure the interrupt lines up with those allocations.
>
Say you allocate ring buffers on NUMA node of the CPU handling interrupt
on a particular queue.
If irqbalance or an admin changes /proc/irq/{number}/smp_affinities,
do you want to realloc ring buffer to another NUMA node ?
It seems complex to me, maybe optimal thing would be to use a NUMA policy to
spread vmalloc() allocations to all nodes to get a good bandwidth...
^ permalink raw reply
* [PATCH] r8169: restore mac addr in rtl8169_remove_one and rtl_shutdown
From: Ivan Vecera @ 2009-11-24 18:26 UTC (permalink / raw)
To: netdev; +Cc: romieu
The newer chipsets (all PCI-E) are known that they need full power cycle
(AC or battery removal) to reset MAC address to a hardwired one. Previous
patch to address this problem loads the original MAC address from EEPROM.
But it brought other problem for which it is necessary to introduce a new
module parameter.
However, it might suffice to restore the initial MAC address before
shutdown/reboot/kexec and when removing the module.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/net/r8169.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index b9221bd..0fe2fc9 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3235,6 +3235,10 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
flush_scheduled_work();
unregister_netdev(dev);
+
+ /* restore original MAC address */
+ rtl_rar_set(tp, dev->perm_addr);
+
rtl_disable_msi(pdev, tp);
rtl8169_release_board(pdev, dev, tp->mmio_addr);
pci_set_drvdata(pdev, NULL);
@@ -4881,6 +4885,9 @@ static void rtl_shutdown(struct pci_dev *pdev)
rtl8169_net_suspend(dev);
+ /* restore original MAC address */
+ rtl_rar_set(tp, dev->perm_addr);
+
spin_lock_irq(&tp->lock);
rtl8169_asic_down(ioaddr);
--
1.6.4.4
^ permalink raw reply related
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Eric W. Biederman @ 2009-11-24 18:10 UTC (permalink / raw)
To: Patrick McHardy
Cc: Arnd Bergmann, virtualization, Herbert Xu, Eric Dumazet,
Anna Fischer, netdev, bridge, linux-kernel, Mark Smith,
Gerhard Stenzel, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <4B0C1031.4050803@trash.net>
Patrick McHardy <kaber@trash.net> writes:
> Eric W. Biederman wrote:
>> Patrick McHardy <kaber@trash.net> writes:
>>
>>>>>> I did all my testing with macvlan interfaces in separate namespaces
>>>>>> communicating with each other, so I'd assume that we should always
>>>>>> clear skb->mark and skb->dst in this function.
>>>>> Good point, in that case we probably should clear it as well. But
>>>>> in the non-namespace case the TC classification currently works and
>>>>> this is consistent with any other virtual device driver, so it
>>>>> should continue to work.
>>>> Do you think we should be able to use TC to direct traffic between
>>>> macvlans on the same underlying device in bridge mode? It does sound
>>>> useful, but I'm not sure how to implement that or if you'd expect
>>>> it to work with the current code. If we support that, it should probably
>>>> also work with namespaces, by consuming the mark in the macvlan
>>>> and veth drivers.
>>> I don't think its necessary, we bypass outgoing queuing anyways.
>>> But if you'd want to add it, just keeping the skb->mark clearing
>>> in veth should work from what I can tell.
>>
>> veth doesn't have an outgoing queue. The reason we clear skb->mark
>> in veth is because when reentering the networking stack the packet
>> needs to be reclassified. At the point of loopback we are talking
>> a packet that has at least logically gone out of the machine on a
>> wire and come back into the machine on another physical interface.
>>
>> So it seems to me we should have consistent handling for macvlans,
>> veth, for the cases where we are looping packets back around. In
>> practice I expect all of those cases are going to be cross namespace
>> as otherwise we would have intercepted the packet before going
>> out a physical interface.
>
> Agreed on the looping case, that's what we're doing now.
>
> In the layered case (macvlan -> eth0) its common behaviour to
> keep the mark however. But in case of different namespaces,
> I think macvlan should also clear the mark on the dev_queue_xmit()
> path since this is just a shortcut to looping the packets
> through veth. In fact probably both of them should also clear
> skb->priority so other namespaces don't accidentally misclassify
> packets.
That is why I pushed for what is becoming dev_forward_skb. So that
we have one place where we can make all of those tweaks. It seems
like in every review we find another field that should be cleared/handled
specially.
I don't quite follow what you intend with dev_queue_xmit when the macvlan
is in one namespace and the real physical device is in another. Are
you mentioning that the packet classifier runs in the namespace where
the primary device lives with packets from a different namespace?
Eric
^ permalink raw reply
* Re: NUMA and multiQ interation
From: David Miller @ 2009-11-24 18:06 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <65634d660911240904y294ea6fj4cf2e4ac757e619b@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Tue, 24 Nov 2009 09:04:41 -0800
> What is the expected interaction here, and would these results be
> typical? If so, would this warrant the need to associate each RX
> queue to a numa node, instead of just the device?
Yes we are fully aware of this and discussed it at netconf this year, see
in particular:
http://vger.kernel.org/netconf2009_slides/netconf2009_numa_discussion.odp
PJ is also currently trying to pass upstream some changes such that these
NUMA allocation bits can be exported to userspace and thus irqbalanced
can pick IRQ targetting more intelligently when a driver allocates per-queue
memory resources on different NUMA nodes. The thread discussing this has
been going active for the past few days, maybe you missed it.
^ permalink raw reply
* how expensive are mallocs?
From: Andrew Grover @ 2009-11-24 17:57 UTC (permalink / raw)
To: netdev
How much effort generally makes sense to avoid mallocs? For example,
rds has a function that does a kmalloc for an array of scatterlist
entries, which is freed at the bottom of the function. We couldn't
allocate off the stack in all cases (too big), but we could allocate
an array of say 8 scatterlists, and use it if that's big enough,
falling back to kmalloc if it's not.
Is this a good idea?
Also, RDS has its own per-cpu page remainder allocator (see
net/rds/page.c) for kernel send buffers. Would cutting this code and
just using kmalloc be recommended? Doesn't SL?B already do per-cpu
pools?
Does this stuff even matter enough to rise above the noise in benchmarks?
Thanks -- Regards -- Andy
^ permalink raw reply
* Re: [PATCH v2] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: David Miller @ 2009-11-24 17:57 UTC (permalink / raw)
To: tglx; +Cc: peter.p.waskiewicz.jr, linux-kernel, arjan, mingo, yong.zhang0,
netdev
In-Reply-To: <alpine.LFD.2.00.0911241151360.24119@localhost.localdomain>
From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 24 Nov 2009 12:07:35 +0100 (CET)
> And what does the kernel do with this information and why are we not
> using the existing device/numa_node information ?
It's a different problem space Thomas.
If the device lives on NUMA node X, we still end up wanting to
allocate memory resources (RX ring buffers) on other NUMA nodes on a
per-queue basis.
Otherwise a network card's forwarding performance is limited by the
memory bandwidth of a single NUMA node, and on a multiqueue cards we
therefore fare much better by allocating each device RX queue's memory
resources on a different NUMA node.
It is this NUMA usage that PJ is trying to export somehow to userspace
so that irqbalanced and friends can choose the IRQ cpu masks more
intelligently.
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Peter P Waskiewicz Jr @ 2009-11-24 17:56 UTC (permalink / raw)
To: David Miller
Cc: peterz@infradead.org, arjan@linux.intel.com,
yong.zhang0@gmail.com, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, netdev@vger.kernel.org
In-Reply-To: <20091124.093956.247147202.davem@davemloft.net>
On Tue, 2009-11-24 at 09:39 -0800, David Miller wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Tue, 24 Nov 2009 09:39:46 +0100
>
> > On Mon, 2009-11-23 at 22:07 -0800, Arjan van de Ven wrote:
> >> the problem is that there is no way currently that the driver can communicate
> >> "I allocated all my metadata on THIS numa node". irqbalance and sysadmins need
> >> that to not make really stupid decisions.....
> >
> > And what exactly is struct device::numa_node good for then?
>
> device->numa_node just says where the device is.
>
> For better performance, it can make sense to, for example, allocate the ring
> buffers for different device queues on other NUMA nodes.
>
> That's the kind of thing PJ is trying to make available.
Yes, that's exactly what I'm trying to do. Even further, we want to
allocate the ring SW struct itself and descriptor structures on other
NUMA nodes, and make sure the interrupt lines up with those allocations.
Cheers,
-PJ
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Peter P Waskiewicz Jr @ 2009-11-24 17:55 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, Yong Zhang, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, davem@davemloft.net,
netdev@vger.kernel.org, Jesse Barnes
In-Reply-To: <alpine.LFD.2.00.0911241027140.24119@localhost.localdomain>
On Tue, 2009-11-24 at 03:07 -0700, Thomas Gleixner wrote:
> On Tue, 24 Nov 2009, Peter P Waskiewicz Jr wrote:
> > On Tue, 2009-11-24 at 01:38 -0700, Peter Zijlstra wrote:
> > > On Mon, 2009-11-23 at 15:32 -0800, Waskiewicz Jr, Peter P wrote:
> > >
> > > > Unfortunately, a driver can't. The irq_set_affinity() function isn't
> > > > exported. I proposed a patch on netdev to export it, and then to tie down
> > > > an interrupt using IRQF_NOBALANCING, so irqbalance won't touch it. That
> > > > was rejected, since the driver is enforcing policy of the interrupt
> > > > balancing, not irqbalance.
> > >
> > > Why would a patch touching the irq subsystem go to netdev?
> >
> > The only change to the IRQ subsystem was:
> >
> > EXPORT_SYMBOL(irq_set_affinity);
>
> Which is still touching the generic irq subsystem and needs the ack of
> the relevant maintainer. If there is a need to expose such an
> interface to drivers then the maintainer wants to know exactly why and
> needs to be part of the discussion of alternative solutions. Otherwise
> you waste time on implementing stuff like the current patch which is
> definitely not going anywhere near the irq subsystem.
>
Understood, and duly noted.
> > > If all you want is to expose policy to userspace then you don't need any
> > > of this, simply expose the NICs home node through a sysfs device thingy
> > > (I was under the impression its already there somewhere, but I can't
> > > ever find anything in /sys).
> > >
> > > No need what so ever to poke at the IRQ subsystem.
> >
> > The point is we need something common that the kernel side (whether a
> > driver or /proc can modify) that irqbalance can use.
>
> /sys/class/net/ethX/device/numa_node
>
> perhaps ?
What I'm trying to do though is one to many NUMA node assignments. See
below for a better overview of what the issue is we're trying to solve.
>
> > > > Also, if you use the /proc interface to change smp_affinity on an
> > > > interrupt without any of these changes, irqbalance will override it on its
> > > > next poll interval. This also is not desirable.
> > >
> > > This all sounds backwards.. we've got a perfectly functional interface
> > > for affinity -- which people object to being used for some reason. So
> > > you add another interface on top, and that is ok?
> > >
> >
> > But it's not functional. If I set the affinity in smp_affinity, then
> > irqbalance will override it 10 seconds later.
>
> And to work around the brain wreckage of irqbalanced you want to
> fiddle in the irq code instead of teaching irqbalanced to handle node
> affinities ?
>
> The only thing which is worth to investigate is whether the irq core
> code should honour the dev->numa_node setting and restrict the
> possible irq affinity settings to that node. If a device is tied to a
> node it makes a certain amount of sense to do that.
>
> But such a change would not need a new interface in the irq core and
> definitely not a new cpumask_t member in the irq_desc structure to
> store a node affinity which can be expressed with a simple
> integer.
>
> But this needs more thoughts and I want to know more about the
> background and the reasoning for such a change.
>
I'll use the ixgbe driver as my example, since that is where my
immediate problems are. This is our 10GbE device, and supports 128 Rx
queues, 128 Tx queues, and has a maximum of 64 MSI-X vectors. In a
typical case, let's say an 8-core machine (Nehalem-EP with
hyperthreading off) brings one port online. We'll allocate 8 Rx and 8
Tx queues. When these allocations occur, we want to allocate the memory
for our descriptor rings and buffer structs and DMA areas onto the
various NUMA nodes. This will promote spreading of the load not just
across CPUs, but also the memory controllers.
If we were to just run like that and have irqbalance move our vectors to
a single node, then we'd have half of our network resources creating
cross-node traffic, which is undesirable, since the OS may have to take
locks node to node to get the memory it's looking for.
The bottom line is we need some mechanism that allows a driver/user to
deterministically assign the underlying interrupt resources to the
correct NUMA node for each interrupt. And in the example above, we may
have more than one NUMA node we need to balance into.
Please let me know if I've explained this well enough. I appreciate the
time.
Cheers,
-PJ Waskiewicz
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: David Miller @ 2009-11-24 17:39 UTC (permalink / raw)
To: peterz
Cc: arjan, peter.p.waskiewicz.jr, yong.zhang0, linux-kernel, arjan,
netdev
In-Reply-To: <1259051986.4531.1057.camel@laptop>
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 24 Nov 2009 09:39:46 +0100
> On Mon, 2009-11-23 at 22:07 -0800, Arjan van de Ven wrote:
>> the problem is that there is no way currently that the driver can communicate
>> "I allocated all my metadata on THIS numa node". irqbalance and sysadmins need
>> that to not make really stupid decisions.....
>
> And what exactly is struct device::numa_node good for then?
device->numa_node just says where the device is.
For better performance, it can make sense to, for example, allocate the ring
buffers for different device queues on other NUMA nodes.
That's the kind of thing PJ is trying to make available.
^ permalink raw reply
* Re: NUMA and multiQ interation
From: Eric Dumazet @ 2009-11-24 17:36 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Netdev List
In-Reply-To: <65634d660911240904y294ea6fj4cf2e4ac757e619b@mail.gmail.com>
Tom Herbert a écrit :
> This is a question about the expected interaction between NUMA and
> receive multi queue. Our test setup is a 16 core AMD system with 4
> sockets, one NUMA node per socket and a bnx2x. The test is running
> 500 streams in netperf RR with response/request of one byte using
> net-next-2.6.
>
> Highest throughput we are seeing is with 4 queues (1 queue processed
> per socket) giving 361862 tps at 67% of cpu. 16 queues (1 queue per
> cpu) gives 226722 tps at 30.43% cpu.
>
> However, with a modified kernel that does RX skb allocations from
> local node rather than the devices numa node, I'm getting 923422 tps
> at 100% cpu. This is much higher tps and better cpu utilization than
> the case where allocations are coming from the device numa node. It
> appears that cross node allocations are a causing a significant
> performance hit. For a 2.5 times performance improvement I'm kind of
> motivated to revert netdev_alloc_skb to when it did not pay attention
> to numa node :-)
>
> What is the expected interaction here, and would these results be
> typical? If so, would this warrant the need to associate each RX
> queue to a numa node, instead of just the device?
>
I believe you answer to your own question Tom.
I always had doubts about forcing RX buffers to be 'close to the device'
And RPS clearly shows that the hard work is done on a CPU close to application,
so it would make sense to allocate RX buffer on the local node (of cpu handling
the RX queue), eg not necessarly on the device numa node.
When packet is transfered from NIC to memory, the NUMA distance is only hit
one time per cache line, no cache needed.
Then, when processing packet by host cpus, we might need many transferts,
because of TCP coalescing and copying to user space.
SLUB/SLAB also pay an extra fee when cross node allocation/deallocation are performed.
Another point to look is the vmalloc() that various drivers use at NIC initialization.
module loading is performed with poor NUMA property (forcing all allocation to one single node)
For example, on this IXGBE adapter, all working space (tx queue rings) is allocated on node 0,
on my dual node machine.
# grep ixgbe_setup_rx_resources /proc/vmallocinfo
0xffffc90006c67000-0xffffc90006c72000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006c73000-0xffffc90006c7e000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d02000-0xffffc90006d0d000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d0e000-0xffffc90006d19000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d1a000-0xffffc90006d25000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d26000-0xffffc90006d31000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d32000-0xffffc90006d3d000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d3e000-0xffffc90006d49000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d4a000-0xffffc90006d55000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d56000-0xffffc90006d61000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d62000-0xffffc90006d6d000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d6e000-0xffffc90006d79000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d7a000-0xffffc90006d85000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d86000-0xffffc90006d91000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d92000-0xffffc90006d9d000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006d9e000-0xffffc90006da9000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e4a000-0xffffc90006e55000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e56000-0xffffc90006e61000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e62000-0xffffc90006e6d000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e6e000-0xffffc90006e79000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e7a000-0xffffc90006e85000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e86000-0xffffc90006e91000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e92000-0xffffc90006e9d000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006e9e000-0xffffc90006ea9000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006eaa000-0xffffc90006eb5000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006eb6000-0xffffc90006ec1000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006ec2000-0xffffc90006ecd000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006ece000-0xffffc90006ed9000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006eda000-0xffffc90006ee5000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006ee6000-0xffffc90006ef1000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006ef2000-0xffffc90006efd000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
0xffffc90006efe000-0xffffc90006f09000 45056 ixgbe_setup_rx_resources+0x45/0x1e0 [ixgbe] pages=10 vmalloc N0=10
alloc_large_system_hash() has better NUMA properties, spreading large hash tables to all nodes.
^ permalink raw reply
* NUMA and multiQ interation
From: Tom Herbert @ 2009-11-24 17:04 UTC (permalink / raw)
To: Linux Netdev List
This is a question about the expected interaction between NUMA and
receive multi queue. Our test setup is a 16 core AMD system with 4
sockets, one NUMA node per socket and a bnx2x. The test is running
500 streams in netperf RR with response/request of one byte using
net-next-2.6.
Highest throughput we are seeing is with 4 queues (1 queue processed
per socket) giving 361862 tps at 67% of cpu. 16 queues (1 queue per
cpu) gives 226722 tps at 30.43% cpu.
However, with a modified kernel that does RX skb allocations from
local node rather than the devices numa node, I'm getting 923422 tps
at 100% cpu. This is much higher tps and better cpu utilization than
the case where allocations are coming from the device numa node. It
appears that cross node allocations are a causing a significant
performance hit. For a 2.5 times performance improvement I'm kind of
motivated to revert netdev_alloc_skb to when it did not pay attention
to numa node :-)
What is the expected interaction here, and would these results be
typical? If so, would this warrant the need to associate each RX
queue to a numa node, instead of just the device?
Thanks,
Tom
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Patrick McHardy @ 2009-11-24 16:56 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Arnd Bergmann, virtualization, Herbert Xu, Eric Dumazet,
Anna Fischer, netdev, bridge, linux-kernel, Mark Smith,
Gerhard Stenzel, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <m1aaybc1s5.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
>
>>>>> I did all my testing with macvlan interfaces in separate namespaces
>>>>> communicating with each other, so I'd assume that we should always
>>>>> clear skb->mark and skb->dst in this function.
>>>> Good point, in that case we probably should clear it as well. But
>>>> in the non-namespace case the TC classification currently works and
>>>> this is consistent with any other virtual device driver, so it
>>>> should continue to work.
>>> Do you think we should be able to use TC to direct traffic between
>>> macvlans on the same underlying device in bridge mode? It does sound
>>> useful, but I'm not sure how to implement that or if you'd expect
>>> it to work with the current code. If we support that, it should probably
>>> also work with namespaces, by consuming the mark in the macvlan
>>> and veth drivers.
>> I don't think its necessary, we bypass outgoing queuing anyways.
>> But if you'd want to add it, just keeping the skb->mark clearing
>> in veth should work from what I can tell.
>
> veth doesn't have an outgoing queue. The reason we clear skb->mark
> in veth is because when reentering the networking stack the packet
> needs to be reclassified. At the point of loopback we are talking
> a packet that has at least logically gone out of the machine on a
> wire and come back into the machine on another physical interface.
>
> So it seems to me we should have consistent handling for macvlans,
> veth, for the cases where we are looping packets back around. In
> practice I expect all of those cases are going to be cross namespace
> as otherwise we would have intercepted the packet before going
> out a physical interface.
Agreed on the looping case, that's what we're doing now.
In the layered case (macvlan -> eth0) its common behaviour to
keep the mark however. But in case of different namespaces,
I think macvlan should also clear the mark on the dev_queue_xmit()
path since this is just a shortcut to looping the packets
through veth. In fact probably both of them should also clear
skb->priority so other namespaces don't accidentally misclassify
packets.
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Eric W. Biederman @ 2009-11-24 16:42 UTC (permalink / raw)
To: Patrick McHardy
Cc: Arnd Bergmann, virtualization, Herbert Xu, Eric Dumazet,
Anna Fischer, netdev, bridge, linux-kernel, Mark Smith,
Gerhard Stenzel, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <4B0BB818.6090509@trash.net>
Patrick McHardy <kaber@trash.net> writes:
>>>> I did all my testing with macvlan interfaces in separate namespaces
>>>> communicating with each other, so I'd assume that we should always
>>>> clear skb->mark and skb->dst in this function.
>>> Good point, in that case we probably should clear it as well. But
>>> in the non-namespace case the TC classification currently works and
>>> this is consistent with any other virtual device driver, so it
>>> should continue to work.
>>
>> Do you think we should be able to use TC to direct traffic between
>> macvlans on the same underlying device in bridge mode? It does sound
>> useful, but I'm not sure how to implement that or if you'd expect
>> it to work with the current code. If we support that, it should probably
>> also work with namespaces, by consuming the mark in the macvlan
>> and veth drivers.
>
> I don't think its necessary, we bypass outgoing queuing anyways.
> But if you'd want to add it, just keeping the skb->mark clearing
> in veth should work from what I can tell.
veth doesn't have an outgoing queue. The reason we clear skb->mark
in veth is because when reentering the networking stack the packet
needs to be reclassified. At the point of loopback we are talking
a packet that has at least logically gone out of the machine on a
wire and come back into the machine on another physical interface.
So it seems to me we should have consistent handling for macvlans,
veth, for the cases where we are looping packets back around. In
practice I expect all of those cases are going to be cross namespace
as otherwise we would have intercepted the packet before going
out a physical interface.
Eric
^ permalink raw reply
* RE: [RFC PATCH 1/4] net: Add support to netdev ops for changing hardware queue MAC and VLAN filters
From: Williams, Mitch A @ 2009-11-24 16:14 UTC (permalink / raw)
To: Ben Hutchings
Cc: Kirsher, Jeffrey T, davem@davemloft.net, shemminger@vyatta.com,
netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <1259072292.2940.16.camel@achroite.uk.solarflarecom.com>
>From: Ben Hutchings [mailto:bhutchings@solarflare.com]
>Sent: Tuesday, November 24, 2009 6:18 AM
>> [snip]
>>
>> >
>> >How does this interact with use of multiple queues within a single
>> >function? Are the specified queue numbers really interpreted as RX
>> >queue indices or as function numbers?
>> >
>> >Ben.
>>
>> Yeah, that is ambiguous. Would it be better if we changed the name of
>the parameter to 'vf' instead of 'queue' to make it explicit?
>>
>> This would give us:
>> $ ip link set eth1 vf 1 mac <blah>
>>
>> The issue of which VF goes with which PF device can be deduced in
>userspace via sysfs.
>>
>> If we want to make this apply to non SR-IOV queues, then we'll add a new
>parameter later.
>>
>> Works for you?
>
>OK, this sounds reasonable.
>
>Ben.
>
Thanks for looking at this stuff, Ben. I appreciate it. I'll rework the patches and send them out in a few days.
-Mitch
^ permalink raw reply
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Michael S. Tsirkin @ 2009-11-24 16:04 UTC (permalink / raw)
To: Anthony Liguori
Cc: Rusty Russell, Shirley Ma, Eric Dumazet, Avi Kivity, netdev, kvm,
linux-kernel, Hollis Blanchard
In-Reply-To: <4B0BEF70.5070104@codemonkey.ws>
On Tue, Nov 24, 2009 at 08:36:32AM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
>>
>>> On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
>>>
>>>>>> + skb = (struct sk_buff *)buf;
>>>>>>
>>>>> This cast is unnecessary, but a comment would be nice:
>>>>>
>>>> Without this cast there is a compile warning.
>>> Hi Shirley,
>>>
>>> Looks like buf is a void *, so no cast should be necessary. But I could
>>> be reading the patch wrong.
>>>
>>>
>>>>> However, I question whether making it 16 byte is the right thing: the
>>>>> ethernet header is 14 bytes long, so don't we want 8 bytes of padding?
>>>>>
>>>> Because in QEMU it requires 10 bytes header in a separately, so one page
>>>> is used to share between virtio_net_hdr header which is 10 bytes head
>>>> and rest of data. So I put 6 bytes offset here between two buffers. I
>>>> didn't look at the reason why a seperate buf is used for virtio_net_hdr
>>>> in QEMU.
>>>>
>>> It's a qemu bug. It insists the header be an element in the scatterlist by
>>> itself. Unfortunately we have to accommodate it.
>>>
>>
>> We do? Let's just fix this?
>>
>
> So does lguest.
It does? All I see it doing is writev/readv,
and this passes things to tap which handles
this correctly.
> It's been that way since the beginning. Fixing this
> would result in breaking older guests.
If you look at my patch, it handles old guests just fine :).
> We really need to introduce a feature bit if we want to change this.
I am not sure I agree: we can't add feature bits
for all bugs, can we?
--
MST
^ permalink raw reply
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Michael S. Tsirkin @ 2009-11-24 16:04 UTC (permalink / raw)
To: Anthony Liguori
Cc: Rusty Russell, Shirley Ma, Eric Dumazet, Avi Kivity, netdev, kvm,
linux-kernel, Hollis Blanchard
In-Reply-To: <4B0BEF70.5070104@codemonkey.ws>
On Tue, Nov 24, 2009 at 08:36:32AM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
>>
>>> On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
>>>
>>>>>> + skb = (struct sk_buff *)buf;
>>>>>>
>>>>> This cast is unnecessary, but a comment would be nice:
>>>>>
>>>> Without this cast there is a compile warning.
>>> Hi Shirley,
>>>
>>> Looks like buf is a void *, so no cast should be necessary. But I could
>>> be reading the patch wrong.
>>>
>>>
>>>>> However, I question whether making it 16 byte is the right thing: the
>>>>> ethernet header is 14 bytes long, so don't we want 8 bytes of padding?
>>>>>
>>>> Because in QEMU it requires 10 bytes header in a separately, so one page
>>>> is used to share between virtio_net_hdr header which is 10 bytes head
>>>> and rest of data. So I put 6 bytes offset here between two buffers. I
>>>> didn't look at the reason why a seperate buf is used for virtio_net_hdr
>>>> in QEMU.
>>>>
>>> It's a qemu bug. It insists the header be an element in the scatterlist by
>>> itself. Unfortunately we have to accommodate it.
>>>
>>
>> We do? Let's just fix this?
>>
>
> So does lguest. It's been that way since the beginning. Fixing this
> would result in breaking older guests.
The patch you are replying to fixes this in a way that does not break older guests.
> We really need to introduce a feature bit if we want to change this.
--
MST
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox