* Re: [PATCH 4/4] macvlan: export macvlan mode through netlink
From: Arnd Bergmann @ 2009-11-24 12:57 UTC (permalink / raw)
To: virtualization
Cc: Patrick McHardy, Herbert Xu, Eric Dumazet, Anna Fischer, netdev,
bridge, linux-kernel, Mark Smith, Gerhard Stenzel,
Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <4B0BBB2E.8020502@trash.net>
On Tuesday 24 November 2009, Patrick McHardy wrote:
> Arnd Bergmann wrote:
> > @@ -600,6 +594,18 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> > if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
> > return -EADDRNOTAVAIL;
> > }
> > +
> > + if (data && data[IFLA_MACVLAN_MODE]) {
> > + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> > + switch (mode) {
> > + case MACVLAN_MODE_PRIVATE:
> > + case MACVLAN_MODE_VEPA:
> > + case MACVLAN_MODE_BRIDGE:
> > + break;
> > + default:
> > + return -EINVAL;
>
> EINVAL is quite unspecific. In this case I think EOPNOTSUPP would
> be fine and provide more information.
ok
> > + }
> > + }
> > return 0;
> > }
> > @@ -664,6 +670,13 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
> > vlan->dev = dev;
> > vlan->port = port;
> >
> > + vlan->mode = MACVLAN_MODE_VEPA;
> > + if (data && data[IFLA_MACVLAN_MODE]) {
> > + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> > +
> > + vlan->mode = mode;
>
> This looks a bit strange, like cut-and-paste without reformatting :)
> I'd suggest to simply use "vlan->mode = nla_get_u32(...)".
Yep.
Thanks for the review. Combined changes below.
Arnd <><
---
drivers/net/macvlan.c | 47 +++++++++++++++++++++++------------------------
1 files changed, 23 insertions(+), 24 deletions(-)
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -118,15 +118,16 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
return 0;
}
-static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
- bool success, bool multicast)
+static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
+ unsigned int len, bool success,
+ bool multicast)
{
struct macvlan_rx_stats *rx_stats;
rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
if (likely(success)) {
rx_stats->rx_packets++;;
- rx_stats->rx_bytes += length;
+ rx_stats->rx_bytes += len;
if (multicast)
rx_stats->multicast++;
} else {
@@ -170,7 +171,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
- if ((vlan->dev == src) || !(vlan->mode & mode))
+ if (vlan->dev == src || !(vlan->mode & mode))
continue;
nskb = skb_clone(skb, GFP_ATOMIC);
@@ -190,7 +191,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
const struct macvlan_dev *vlan;
const struct macvlan_dev *src;
struct net_device *dev;
- int len;
+ unsigned int len;
port = rcu_dereference(skb->dev->macvlan_port);
if (port == NULL)
@@ -200,17 +201,22 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
src = macvlan_hash_lookup(port, eth->h_source);
if (!src)
/* frame comes from an external address */
- macvlan_broadcast(skb, port, NULL, MACVLAN_MODE_PRIVATE
- | MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
+ macvlan_broadcast(skb, port, NULL,
+ MACVLAN_MODE_PRIVATE |
+ MACVLAN_MODE_VEPA |
+ MACVLAN_MODE_BRIDGE);
else if (src->mode == MACVLAN_MODE_VEPA)
/* flood to everyone except source */
macvlan_broadcast(skb, port, src->dev,
- MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
+ MACVLAN_MODE_VEPA |
+ MACVLAN_MODE_BRIDGE);
else if (src->mode == MACVLAN_MODE_BRIDGE)
- /* flood only to VEPA ports, bridge ports
- already saw the frame */
+ /*
+ * flood only to VEPA ports, bridge ports
+ * already saw the frame on the way out.
+ */
macvlan_broadcast(skb, port, src->dev,
- MACVLAN_MODE_VEPA);
+ MACVLAN_MODE_VEPA);
return skb;
}
@@ -253,7 +259,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
dest = macvlan_hash_lookup(port, eth->h_dest);
if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
- int length = skb->len + ETH_HLEN;
+ unsigned int length = skb->len + ETH_HLEN;
int ret = dev_forward_skb(dest->dev, skb);
macvlan_count_rx(dest, length,
ret == NET_RX_SUCCESS, 0);
@@ -604,8 +610,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
}
if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- switch (mode) {
+ switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
case MACVLAN_MODE_PRIVATE:
case MACVLAN_MODE_VEPA:
case MACVLAN_MODE_BRIDGE:
@@ -679,11 +684,8 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
vlan->port = port;
vlan->mode = MACVLAN_MODE_VEPA;
- if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
-
- vlan->mode = mode;
- }
+ if (data && data[IFLA_MACVLAN_MODE])
+ vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
err = register_netdevice(dev);
if (err < 0)
@@ -710,11 +712,8 @@ static int macvlan_changelink(struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
{
struct macvlan_dev *vlan = netdev_priv(dev);
- if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- vlan->mode = mode;
- }
-
+ if (data && data[IFLA_MACVLAN_MODE])
+ vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
return 0;
}
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Arnd Bergmann @ 2009-11-24 13:13 UTC (permalink / raw)
To: Patrick McHardy
Cc: virtualization, Herbert Xu, Eric Dumazet, Anna Fischer, netdev,
bridge, linux-kernel, Mark Smith, Gerhard Stenzel,
Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <4B0BB818.6090509@trash.net>
On Tuesday 24 November 2009, Patrick McHardy wrote:
> 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.
Ok, I won't bother with it for now then.
Arnd <><
^ permalink raw reply
* Re: [PATCH net-next-2.6] ixgbe: Fix TX stats accounting
From: Eric Dumazet @ 2009-11-24 13:23 UTC (permalink / raw)
To: Peter P Waskiewicz Jr, Jeff Kirsher
Cc: robert@herjulf.net, Jesper Dangaard Brouer, Linux Netdev List,
David S. Miller
In-Reply-To: <4B0BC56D.1000909@gmail.com>
Here is an updated version, because ixgbe_get_ethtool_stats()
needs to call dev_get_stats() or "ethtool -S" wont give
correct tx_bytes/tx_packets values.
[PATCH net-next-2.6] ixgbe: Fix TX stats accounting
Several cpus can update netdev->stats.tx_bytes & netdev->stats.tx_packets
in parallel. In this case, TX stats are under estimated and false sharing
takes place.
After a pktgen session sending exactly 200000000 packets :
# ifconfig fiber0 | grep TX
TX packets:198501982 errors:0 dropped:0 overruns:0 carrier:0
Multi queue devices should instead use txq->tx_bytes & txq->tx_packets
in their xmit() method (appropriate txq lock already held by caller, no
cache line miss), or use appropriate locking.
After patch, same pktgen session gives :
# ifconfig fiber0 | grep TX
TX packets:200000000 errors:0 dropped:0 overruns:0 carrier:0
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/ixgbe/ixgbe_ethtool.c | 1 +
drivers/net/ixgbe/ixgbe_main.c | 20 ++++----------------
2 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index 74f04e1..7b7f8f6 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -944,6 +944,7 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
char *p = NULL;
ixgbe_update_stats(adapter);
+ dev_get_stats(netdev);
for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++) {
switch (ixgbe_gstrings_stats[i].type) {
case NETDEV_STATS:
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index ebcec30..1cea120 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -425,8 +425,6 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
tx_ring->total_packets += total_packets;
tx_ring->stats.packets += total_packets;
tx_ring->stats.bytes += total_bytes;
- netdev->stats.tx_bytes += total_bytes;
- netdev->stats.tx_packets += total_packets;
return (count < tx_ring->work_limit);
}
@@ -5249,6 +5247,7 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_ring *tx_ring;
+ struct netdev_queue *txq;
unsigned int first;
unsigned int tx_flags = 0;
u8 hdr_len = 0;
@@ -5345,6 +5344,9 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
tx_ring->atr_count = 0;
}
}
+ txq = netdev_get_tx_queue(netdev, r_idx);
+ txq->tx_bytes += skb->len;
+ txq->tx_packets++;
ixgbe_tx_queue(adapter, tx_ring, tx_flags, count, skb->len,
hdr_len);
ixgbe_maybe_stop_tx(netdev, tx_ring, DESC_NEEDED);
@@ -5359,19 +5361,6 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
}
/**
- * ixgbe_get_stats - Get System Network Statistics
- * @netdev: network interface device structure
- *
- * Returns the address of the device statistics structure.
- * The statistics are actually updated from the timer callback.
- **/
-static struct net_device_stats *ixgbe_get_stats(struct net_device *netdev)
-{
- /* only return the current stats */
- return &netdev->stats;
-}
-
-/**
* ixgbe_set_mac - Change the Ethernet Address of the NIC
* @netdev: network interface device structure
* @p: pointer to an address structure
@@ -5501,7 +5490,6 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_stop = ixgbe_close,
.ndo_start_xmit = ixgbe_xmit_frame,
.ndo_select_queue = ixgbe_select_queue,
- .ndo_get_stats = ixgbe_get_stats,
.ndo_set_rx_mode = ixgbe_set_rx_mode,
.ndo_set_multicast_list = ixgbe_set_rx_mode,
.ndo_validate_addr = eth_validate_addr,
^ permalink raw reply related
* Re: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-24 13:46 UTC (permalink / raw)
To: Caleb Cushing
Cc: Frans Pop, Andi Kleen, linux-kernel, netdev, Jeff Kirsher,
Jesse Brandeburg, e1000-devel
In-Reply-To: <20091124111946.GA7883@ff.dom.local>
On Tue, Nov 24, 2009 at 11:19:46AM +0000, Jarek Poplawski wrote:
...
> Alas it's not all information I asked. E.g. "netstat -s before faulty
> kernel" and "netstat -s after faulty kernel" seem to be the same file:
> netstat_after.slave4.log.gz.
On the other hand, there is a lot of tcp retransmits there:
Tcp:
17 active connections openings
0 passive connection openings
14 failed connection attempts
0 connection resets received
0 connections established
45 segments received
49 segments send out
19 segments retransmited
0 bad segments received.
19 resets sent
So it might point at the driver yet. It would be interesting to see
more of this: could you repeat "netstat -s" and "ethtool -S eth0"
after rebooting with both kernels and doing a few minutes of similar
tcp activities (against the router or some other "good" site). Btw,
please remind us the exact kernel versions. If you can, try 2.6.32-rc8
instead of 2.6.31.
Jarek P.
^ permalink raw reply
* Re: [PATCH 4/4] macvlan: export macvlan mode through netlink
From: Patrick McHardy @ 2009-11-24 13:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: virtualization, Herbert Xu, Eric Dumazet, Anna Fischer, netdev,
bridge, linux-kernel, Mark Smith, Gerhard Stenzel,
Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <200911241357.46690.arnd@arndb.de>
Arnd Bergmann wrote:
> Thanks for the review. Combined changes below.
Looks good, thanks.
>
> Arnd <><
> ---
>
> drivers/net/macvlan.c | 47 +++++++++++++++++++++++------------------------
> 1 files changed, 23 insertions(+), 24 deletions(-)
>
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -118,15 +118,16 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
> return 0;
> }
>
> -static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
> - bool success, bool multicast)
> +static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
> + unsigned int len, bool success,
> + bool multicast)
> {
> struct macvlan_rx_stats *rx_stats;
>
> rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
> if (likely(success)) {
> rx_stats->rx_packets++;;
> - rx_stats->rx_bytes += length;
> + rx_stats->rx_bytes += len;
> if (multicast)
> rx_stats->multicast++;
> } else {
> @@ -170,7 +171,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
>
> for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
> hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
> - if ((vlan->dev == src) || !(vlan->mode & mode))
> + if (vlan->dev == src || !(vlan->mode & mode))
> continue;
>
> nskb = skb_clone(skb, GFP_ATOMIC);
> @@ -190,7 +191,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> const struct macvlan_dev *vlan;
> const struct macvlan_dev *src;
> struct net_device *dev;
> - int len;
> + unsigned int len;
>
> port = rcu_dereference(skb->dev->macvlan_port);
> if (port == NULL)
> @@ -200,17 +201,22 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> src = macvlan_hash_lookup(port, eth->h_source);
> if (!src)
> /* frame comes from an external address */
> - macvlan_broadcast(skb, port, NULL, MACVLAN_MODE_PRIVATE
> - | MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
> + macvlan_broadcast(skb, port, NULL,
> + MACVLAN_MODE_PRIVATE |
> + MACVLAN_MODE_VEPA |
> + MACVLAN_MODE_BRIDGE);
> else if (src->mode == MACVLAN_MODE_VEPA)
> /* flood to everyone except source */
> macvlan_broadcast(skb, port, src->dev,
> - MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
> + MACVLAN_MODE_VEPA |
> + MACVLAN_MODE_BRIDGE);
> else if (src->mode == MACVLAN_MODE_BRIDGE)
> - /* flood only to VEPA ports, bridge ports
> - already saw the frame */
> + /*
> + * flood only to VEPA ports, bridge ports
> + * already saw the frame on the way out.
> + */
> macvlan_broadcast(skb, port, src->dev,
> - MACVLAN_MODE_VEPA);
> + MACVLAN_MODE_VEPA);
> return skb;
> }
>
> @@ -253,7 +259,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
>
> dest = macvlan_hash_lookup(port, eth->h_dest);
> if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
> - int length = skb->len + ETH_HLEN;
> + unsigned int length = skb->len + ETH_HLEN;
> int ret = dev_forward_skb(dest->dev, skb);
> macvlan_count_rx(dest, length,
> ret == NET_RX_SUCCESS, 0);
> @@ -604,8 +610,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> }
>
> if (data && data[IFLA_MACVLAN_MODE]) {
> - u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> - switch (mode) {
> + switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
> case MACVLAN_MODE_PRIVATE:
> case MACVLAN_MODE_VEPA:
> case MACVLAN_MODE_BRIDGE:
> @@ -679,11 +684,8 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
> vlan->port = port;
>
> vlan->mode = MACVLAN_MODE_VEPA;
> - if (data && data[IFLA_MACVLAN_MODE]) {
> - u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> -
> - vlan->mode = mode;
> - }
> + if (data && data[IFLA_MACVLAN_MODE])
> + vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
>
> err = register_netdevice(dev);
> if (err < 0)
> @@ -710,11 +712,8 @@ static int macvlan_changelink(struct net_device *dev,
> struct nlattr *tb[], struct nlattr *data[])
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> - if (data && data[IFLA_MACVLAN_MODE]) {
> - u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> - vlan->mode = mode;
> - }
> -
> + if (data && data[IFLA_MACVLAN_MODE])
> + vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> return 0;
> }
>
>
^ permalink raw reply
* RE: [RFC PATCH 1/4] net: Add support to netdev ops for changing hardware queue MAC and VLAN filters
From: Ben Hutchings @ 2009-11-24 14:18 UTC (permalink / raw)
To: Williams, Mitch A
Cc: Kirsher, Jeffrey T, davem@davemloft.net, shemminger@vyatta.com,
netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A1365FA6A9617F@rrsmsx501.amr.corp.intel.com>
On Mon, 2009-11-23 at 12:52 -0700, Williams, Mitch A wrote:
> >From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> >Sent: Monday, November 23, 2009 5:23 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.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Anthony Liguori @ 2009-11-24 14:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Rusty Russell, Shirley Ma, Eric Dumazet, Avi Kivity, netdev, kvm,
linux-kernel, Hollis Blanchard
In-Reply-To: <20091124113754.GB2405@redhat.com>
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.
We really need to introduce a feature bit if we want to change this.
Regards,
Anthony Liguori
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Arjan van de Ven @ 2009-11-24 14:42 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Peter P Waskiewicz Jr, Yong Zhang, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, davem@davemloft.net,
netdev@vger.kernel.org
In-Reply-To: <1259051986.4531.1057.camel@laptop>
Peter Zijlstra wrote:
>>> Same for userspace.
>> 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?
>
and that is exported to userspace.. how?
... that has nothing to do with an irq, and also falls flat for a driver that
supports multiple irqs, and assigns one to each numa node.
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Arjan van de Ven @ 2009-11-24 14:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Peter P Waskiewicz Jr, Yong Zhang, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, davem@davemloft.net,
netdev@vger.kernel.org, Thomas Gleixner
In-Reply-To: <1259053736.4531.1097.camel@laptop>
Peter Zijlstra wrote:
> On Tue, 2009-11-24 at 00:59 -0800, Peter P Waskiewicz Jr wrote:
>>> 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 here I was thinking the kernel round-robins IRQ delivery on the mask
> specified there.
the kernel does no such thing, nor has code to do so.
> Are you talking about some daft userspace thing that
> writes into the irq smp_affinity to effect irq balancing?
thanks ;)
^ permalink raw reply
* Re: [patch]USB autosuspend for cdc-ether
From: Steve.Glendinning @ 2009-11-24 15:11 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, David Brownell, linux-usb, netdev, stern,
Torgny Johansson
In-Reply-To: <200911231621.04499.oliver@neukum.org>
Hi Oliver,
Interesting patch, I'm going to try and add support for this
to smsc95xx.
A few things I noticed:
> @@ -929,7 +961,11 @@ kevent (struct work_struct *work)
> int retval = 0;
>
> clear_bit (EVENT_LINK_RESET, &dev->flags);
> + status = usb_autopm_get_interface(dev->intf);
> + if (status < 0)
> + goto skip_reset;
> if(info->link_reset && (retval = info->link_reset(dev)) < 0) {
> +skip_reset:
> devinfo(dev, "link reset failed (%d) usbnet usb-%s-%s, %s",
> retval,
> dev->udev->bus->bus_name, dev->udev->devpath,
on EVENT_LINK_RESET you call usb_autopm_get_interface with no matching
usb_autopm_put_interface, so you leak 1 reference count and the device
never autosuspends.
> @@ -958,7 +994,7 @@ static void tx_complete (struct urb *urb)
>
> switch (urb->status) {
> case -EPIPE:
> - usbnet_defer_kevent (dev, EVENT_TX_HALT);
> + usbnet_defer_kevent (dev, EVENT_TX_HALT);
> break;
>
> /* software-driven interface shutdown */
The only change here is the addition of trailing whitespace (which both
checkpatch.pl and git complain about). You should run your patch through
scripts/checkpatch.pl and fix these up.
--
Steve
^ permalink raw reply
* RE: [E1000-devel] large packet loss take2 2.6.31.x
From: Allan, Bruce W @ 2009-11-24 15:57 UTC (permalink / raw)
To: Jarek Poplawski, Caleb Cushing
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Frans Pop, Brandeburg, Jesse, linux-kernel@vger.kernel.org,
Andi Kleen, Kirsher, Jeffrey T
In-Reply-To: <20091124111946.GA7883@ff.dom.local>
>-----Original Message-----
>From: Jarek Poplawski [mailto:jarkao2@gmail.com]
>Sent: Tuesday, November 24, 2009 3:20 AM
>To: Caleb Cushing
>Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Frans Pop;
>Brandeburg, Jesse; linux-kernel@vger.kernel.org; Andi Kleen; Kirsher,
>Jeffrey T
>Subject: Re: [E1000-devel] large packet loss take2 2.6.31.x
>
>On Tue, Nov 24, 2009 at 01:17:09AM -0500, Caleb Cushing wrote:
>> > Btw, currently I don't consider this dropping means there has to be
>> > a bug. It could be otherwise - a feature... e.g. when a new kernel
>> > can transmit faster (then dropping in some other, slower place can
>> > happen).
>>
>> um... where would it be dropping that we wouldn't have a bug? I mean
>> sure faster is great... but if it makes my network not work right...
>
>E.g. if it were dropped because of a queue overflow (but it doesn't
>seem to be the case, at least at your box) or because of memory
>problems while handling a lot of traffic.
>
>>
>> I've added all (I think) information you've asked for to the bug
>> http://bugzilla.kernel.org/show_bug.cgi?id=13835 except for ethtool
>> and netstat on the router side. ethtool complains about not having
>> driver or capability (maybe because it's a 2.4 kernel?) and the
>> version of netstat doesn't support -s. I disabled everything that I
>> can think of that would send/receive packets before doing the test
>> client side, except dhcp/dns windows box's were probably sending some
>> broadcasts too. but the traffic should be pretty low. I did remember
>> to set the txqueuelen didn't seem to make a difference
>
>Alas it's not all information I asked. E.g. "netstat -s before faulty
>kernel" and "netstat -s after faulty kernel" seem to be the same file:
>netstat_after.slave4.log.gz. Anyway, since there are problems with
>getting stats from the router we still can't compare them, or check
>for the dropped stats. (Btw, could you check for /proc/net/softnet_stat
>yet?)
>
>So, it might be the kernel problem you reported, but there is not
>enough data to prove it. Then my proposal is to try to repeat this
>problem in more "testing friendly" conditions - preferably against
>some other, more up-to-date linux box, if possible?
>
>> only error in dmesg I see is
>>
>> e1000e 0000:00:19.0: pci_enable_pcie_error_reporting failed 0xfffffffb
>
>I added e1000e maintainers to CC to have a look at this warning.
>
>Jarek P.
The "pci_enable_pcie_error_reporting failed" message is a non-fatal warning that has recently been removed.
^ permalink raw reply
* Re: Diagnostic Monitoring Interface Monitoring (DOM) PATCH 4/5 for net-next-2.6
From: robert @ 2009-11-24 18:12 UTC (permalink / raw)
To: Joe Perches; +Cc: Robert Olsson, David Miller, netdev
In-Reply-To: <1259017659.16503.138.camel@Joe-Laptop.home>
Joe Perches writes:
> Can you run ./scripts/checkpatch.pl on your patches please?
Yes there were many objections... but my linux quantum is for over for now
> > + if((res = read_phy_diag(hw, 0x1, DOM_A2_TEMP_SLOPE, &pd->temp_slope)))
> > + goto out;
> [etc...]
> > + if((res = read_phy_diag(hw, 0x1, DOM_A2_TX_I_OFFSET, &pd->tx_bias_offset)))
> > + goto out;
>
> perhaps this is clearer as:
>
> if (read_phy_diag(hw, 0x1, reg, loc) ||
> read_phy_diag(hw, 0x1, reg2, loc2) ||
> ...
> read_phy_diag(hw, 0x1, regN, locN))
> goto out;
Feel free to attack it...
Cheers
--ro
^ 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
* 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: [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/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: [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
* 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: 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
* 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: [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: 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 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
* 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: 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
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