Netdev List
 help / color / mirror / Atom feed
* [PATCH 2/2] net: ethernet: xilinx: Do not use NO_IRQ in axienet
From: Michal Simek @ 2013-01-10 16:58 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1357837123-18996-1-git-send-email-michal.simek@xilinx.com>

This driver is used on Microblaze and will be used
on Arm Zynq.
Microblaze doesn't define NO_IRQ and no IRQ is 0.
Arm still uses NO_IRQ as -1 and there is no option
to connect IRQ to irq 0.

That's why <= 0 is only one option how to find out
undefined IRQ.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index d9f69b8..6f47100 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1590,7 +1590,7 @@ static int axienet_of_probe(struct platform_device *op)
 	lp->rx_irq = irq_of_parse_and_map(np, 1);
 	lp->tx_irq = irq_of_parse_and_map(np, 0);
 	of_node_put(np);
-	if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
+	if ((lp->rx_irq <= 0) || (lp->tx_irq <= 0)) {
 		dev_err(&op->dev, "could not determine irqs\n");
 		ret = -ENOMEM;
 		goto err_iounmap_2;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH] tcp: splice: fix an infinite loop in tcp_read_sock()
From: Eric Dumazet @ 2013-01-10 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Willy Tarreau

From: Eric Dumazet <edumazet@google.com>

commit 02275a2ee7c0 (tcp: don't abort splice() after small transfers)
added a regression.


[   83.843570] INFO: rcu_sched self-detected stall on CPU
[   83.844575] INFO: rcu_sched detected stalls on CPUs/tasks: { 6} (detected by 0, t=21002 jiffies, g=4457, c=4456, q=13132)
[   83.844582] Task dump for CPU 6:
[   83.844584] netperf         R  running task        0  8966   8952 0x0000000c
[   83.844587]  0000000000000000 0000000000000006 0000000000006c6c 0000000000000000
[   83.844589]  000000000000006c 0000000000000096 ffffffff819ce2bc ffffffffffffff10
[   83.844592]  ffffffff81088679 0000000000000010 0000000000000246 ffff880c4b9ddcd8
[   83.844594] Call Trace:
[   83.844596]  [<ffffffff81088679>] ? vprintk_emit+0x1c9/0x4c0
[   83.844601]  [<ffffffff815ad449>] ? schedule+0x29/0x70
[   83.844606]  [<ffffffff81537bd2>] ? tcp_splice_data_recv+0x42/0x50
[   83.844610]  [<ffffffff8153beaa>] ? tcp_read_sock+0xda/0x260
[   83.844613]  [<ffffffff81537b90>] ? tcp_prequeue_process+0xb0/0xb0
[   83.844615]  [<ffffffff8153c0f0>] ? tcp_splice_read+0xc0/0x250
[   83.844618]  [<ffffffff814dc0c2>] ? sock_splice_read+0x22/0x30
[   83.844622]  [<ffffffff811b820b>] ? do_splice_to+0x7b/0xa0
[   83.844627]  [<ffffffff811ba4bc>] ? sys_splice+0x59c/0x5d0
[   83.844630]  [<ffffffff8119745b>] ? putname+0x2b/0x40
[   83.844633]  [<ffffffff8118bcb4>] ? do_sys_open+0x174/0x1e0
[   83.844636]  [<ffffffff815b6202>] ? system_call_fastpath+0x16/0x1b


if recv_actor() returns 0, we should stop immediately,
because looping wont give a chance to drain the pipe.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willy Tarreau <w@1wt.eu>
---
 net/ipv4/tcp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1ca2536..5f173dc 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1482,7 +1482,7 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 					break;
 			}
 			used = recv_actor(desc, skb, offset, len);
-			if (used < 0) {
+			if (used <= 0) {
 				if (!copied)
 					copied = used;
 				break;

^ permalink raw reply related

* Re: [PATCH 2/2] Fix phy_attach - forward dev_flags for phy_attach
From: Florian Fainelli @ 2013-01-10 17:12 UTC (permalink / raw)
  To: Kosta Zertsekel
  Cc: netdev@vger.kernel.org, zertsekel@gmail.com, andrew@lunn.ch,
	Eran Ben-Avi, linux-arm-kernel@lists.infradead.org, Lior Amsalem
In-Reply-To: <BC373419EB4337418B2B595BAEDC155F015896370062@IL-MB01.marvell.com>

Le 01/10/13 16:57, Kosta Zertsekel a écrit :
>> By the way, most, if not all of the phy_connect() users in drivers/net/ethernet/ also do not ensure they pass the phy device flags, so you might
> want to fix this globally and not just for Marvell driver.
> Indeed, phy_connect() mostly just pass zero intead of phy_dev->dev_flags.
> But, I think, the guy that calls phy_connect() in its driver should know what he does, and,
> probably, we should rely on him knowing his stuff.
> The only evidence of the bug is when phy_dev->dev_flags was actually changed by PHY fixup callback
> (see dns323-setup.c for example) and was *not* propagated to phy_connect() or phy_attach() as in pxa168_eth.c phy_init().
> The code conforming to the former should be fixed IMHO.

Actually, I wonder if we should not rather remove entirely the flags 
argument, let the phy_connect() or phy_attach() callers modify the phy 
device dev_flags like it does today (e.g: tg3) and modify phy_connect() 
and phy_attach() to pass phy->dev_flags to phy_attach_direct().
--
Florian

^ permalink raw reply

* Fw: [Bug 52481] New: Occasional NULL pointer dereference during ftp networking since 3.7
From: Stephen Hemminger @ 2013-01-10 17:21 UTC (permalink / raw)
  To: netdev



Begin forwarded message:

Date: Tue,  8 Jan 2013 14:29:18 +0000 (UTC)
From: bugzilla-daemon@bugzilla.kernel.org
To: shemminger@linux-foundation.org
Subject: [Bug 52481] New: Occasional NULL pointer dereference during ftp networking since 3.7


https://bugzilla.kernel.org/show_bug.cgi?id=52481

           Summary: Occasional NULL pointer dereference during ftp
                    networking since 3.7
           Product: Networking
           Version: 2.5
    Kernel Version: 3.7.0
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: IPV4
        AssignedTo: shemminger@linux-foundation.org
        ReportedBy: itumaykin@gmail.com
        Regression: Yes


Created an attachment (id=90771)
 --> (https://bugzilla.kernel.org/attachment.cgi?id=90771)
lspci -vvv

I am experiencing occasional panics during ftp networking after installing
kernel 3.7.0.

I have vsftpd-3.0.2 up and running on port 21 with configuration not changed
for months, I use passive FTP with nf_conntrack_ftp helper. My distro is
Hardened Gentoo amd64 and I use not vanilla kernel, but with some Gentoo
specific and grsec specific patches.

Anyway, with kernel 3.5.4 I had no problems with my FTP at all, but with 3.7.0
there are random panics during or after FTP sessions. It happens more often
after sessions with lots of data transferred, but sometimes only `ls` is enough
to hang my machine.
The nature of these errors is unknown to me, but the issue is definetely within
the kernel as I tried to downgrade solely kernel back to 3.5.4 and everything
returned back to normal.

Also, I don't have traces of these panics in my logs, but I took photos of one
of such cases. Here are links to them:
https://docs.google.com/folder/d/0B9NAm8_0mptgU2RrMTZWX0NucUk/edit

I understand there is probably not enough info to investigate the source of
this issue, but I am ready to provide any additional info or do some testing.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply

* BUG: NULL pointer dereference in netif_carrier_off
From: Tom Parkin @ 2013-01-10 17:50 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev

Hi Eric,

I've just had a chance to try your fix for the veth oops I previously
reported[1], but I still see some problems (please see oops below).

If I revert the original stats overhead fix (2681128f0ced8) and the
subsequent NULL deref patch (d0e2c55e7c94) this problem goes away for
me.

I've reproduced this bug using a script intended to stress test L2TP
network namespace code, which uses veth tunnels to bridge namespaces.

Apologies for the late report!

Thanks,
Tom

[1]
http://www.spinics.net/lists/netdev/msg221284.html

BUG: unable to handle kernel NULL pointer dereference at 0000002c
IP: [<c151f5f8>] netif_carrier_off+0x8/0x30
*pde = 00000000
Oops: 0002 [#1] SMP
Modules linked in: veth bridge stp llc l2tp_ip6 l2tp_ip l2tp_ppp pppox
l2tp_eth l2tp_netlink l2tp_core kvm_amd 9
Pid: 5778, comm: ip Tainted: G        W    3.8.0-rc1-tpdev-33-lockdep+
#42 Gigabyte Technology Co., Ltd. GA-MA62
EIP: 0060:[<c151f5f8>] EFLAGS: 00010202 CPU: 0
EIP is at netif_carrier_off+0x8/0x30
EAX: 00000000 EBX: ee422800 ECX: 00000001 EDX: c18e4b80
ESI: efec7c44 EDI: ee422840 EBP: efec7b38 ESP: efec7b38
 DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 80050033 CR2: 0000002c CR3: 2fdb2000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process ip (pid: 5778, ti=efec6000 task=f1d4bf00 task.ti=efec6000)
Stack:
 efec7b44 f810e23d ee422800 efec7b60 c14fc169 ffffffff ffffffff f594c210
 ee422800 efec7c44 efec7b80 c14fc25e efec7b6c ee422040 ee422040 efec7c04
 ee422800 efec7c44 efec7ba0 c14fc357 f180dd00 efec7c44 ee422000 efec7c44
Call Trace:
 [<f810e23d>] veth_close+0x2d/0x60 [veth]
 [<c14fc169>] __dev_close_many+0x69/0xb0
 [<c14fc25e>] dev_close_many+0x6e/0xc0
 [<c14fc357>] rollback_registered_many+0xa7/0x1c0
 [<c14fc484>] unregister_netdevice_many+0x14/0x60
 [<c1510e0e>] rtnl_dellink+0x9e/0x100
 [<c10461a8>] ? ns_capable+0x38/0x70
 [<c1510d70>] ? rtnetlink_put_metrics+0xf0/0xf0
 [<c1512ae3>] rtnetlink_rcv_msg+0x153/0x2a0
 [<c160db1a>] ? mutex_lock_nested+0x21a/0x2e0
 [<c150f924>] ? rtnl_lock+0x14/0x20
 [<c1512990>] ? __rtnl_unlock+0x20/0x20
 [<c152a31e>] netlink_rcv_skb+0x8e/0xb0
 [<c150f94c>] rtnetlink_rcv+0x1c/0x30
 [<c1529cdd>] netlink_unicast+0x17d/0x1f0
 [<c1529f74>] netlink_sendmsg+0x224/0x390
 [<c14e8791>] sock_sendmsg+0xd1/0xf0
 [<c1135b89>] ? might_fault+0x89/0x90
 [<c12fa932>] ? _copy_from_user+0x42/0x60
 [<c14f5c64>] ? verify_iovec+0x44/0xb0
 [<c14e96a2>] __sys_sendmsg+0x262/0x270
 [<c1073e4f>] ? sched_clock_cpu+0xcf/0x150
 [<c109c02b>] ? trace_hardirqs_off+0xb/0x10
 [<c1073f35>] ? local_clock+0x65/0x70
 [<c109c69c>] ? lock_release_holdtime.part.23+0xbc/0xf0
 [<c10a15ed>] ? lock_release_non_nested+0x29d/0x2e0
 [<c1073f35>] ? local_clock+0x65/0x70
 [<c1178ab1>] ? fget_light+0x371/0x450
 [<c14ead9b>] sys_sendmsg+0x3b/0x60
 [<c14eb443>] sys_socketcall+0x283/0x2e0
 [<c1611310>] ? restore_all+0xf/0xf
 [<c1614bc0>] ? __do_page_fault+0x4e0/0x4e0
 [<c12fa548>] ? trace_hardirqs_on_thunk+0xc/0x10
 [<c1618acd>] sysenter_do_call+0x12/0x38
Code: 00 00 00 e8 ab 2f fd ff 8b 75 f8 b8 01 00 00 00 83 83 9c 00 00
00 01 8b 5d f4 8b 7d fc 89 ec 5d c3 66 90 3
EIP: [<c151f5f8>] netif_carrier_off+0x8/0x30 SS:ESP 0068:efec7b38
CR2: 000000000000002c
---[ end trace e9f7b433da03329a ]---

--
Tom Parkin
www.thesecretdogproject.com
The competent programmer is fully aware of the strictly limited size
of his own skull; therefore he approaches the programming task in full
humility, and among other things he avoids clever tricks like the
plague. /E. W. Dijkstra/

^ permalink raw reply

* Re: [PATCH net-next] team: add ethtool support
From: Stephen Hemminger @ 2013-01-10 17:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Flavio Leitner, netdev, Jiri Pirko
In-Reply-To: <1356834716.21409.6258.camel@edumazet-glaptop>

On Sat, 29 Dec 2012 18:31:56 -0800
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Sat, 2012-12-29 at 23:44 -0200, Flavio Leitner wrote:
> 
> > Speaking as a support engineer, it's a lot easier to grab ethtool -S and
> > see everything than grab two or more outputs.
> > 
> 
> I agree its very convenient.
> 
> I have a patch to add GRO statistics at the core layer, in the ethtool
> -S stats. 
> 
> I was about to ask netdev guys what they think of this idea ?
> 
> 
> net-gro: Add GRO counters to ethtool -S
>     
> In order to get an idea of how effective is GRO aggregation on a machine,
> we need appropriate counters. Preferably use "ethtool -S" to display them
> on a per device basis, or even per RX queue.
>     
> In this implementation, I chose to not change NIC drivers.
> 
> Core network stack adds the gro counters at the end of the counters
> each NIC driver provides for ethtool -S
>     
> There are 5 counters per RX queue :
>     
>     gro_complete:  number of time the NAPI handler did not consume its budget
>                    (This force a flush of all GRO packets in the GRO queue)
>     gro_overflows: number of time a segment could not be stored in GRO queue
>                    because current number or messages is too high
>     gro_nogro:     number of time a segment was not stored in GRO queue.
>                    (Because its not a TCP packet, or it includes a
>                    SYN/FIN/RST/PSH flag)
>     gro_msgs:      number of GRO messages (might contain 1 to 17 segments)
>     gro_segs:      number of GRO segments
>     
> Example:
>     
>      On receiver machine, with 8 RX queues :
> 
>      ethtool -S eth4 | tail -n 10
>          gro_complete[7]: 56635
>          gro_overflows[7]: 0
>          gro_nogro[7]: 212
>          gro_msgs[7]: 129410
>          gro_segs[7]: 1434925
>          gro_complete: 699479
>          gro_overflows: 0
>          gro_nogro: 2455
>          gro_msgs: 1626470
>          gro_segs: 17876794
>     
>     In this example, we can compute average number of segments per GRO message :
>     
>     17876794/17876794 = 10.99
>     
>     Or more precisely : 17876794/(17876794+2455) = 10.97
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ethtool is awkward because it is only available through ioctl, no netlink or /proc.
If you use ethtool for GRO, please make it a separate ioctl not an add-on to
existing device statistics.  ethtool --gro ??

^ permalink raw reply

* Re: BUG: NULL pointer dereference in netif_carrier_off
From: Eric Dumazet @ 2013-01-10 17:56 UTC (permalink / raw)
  To: Tom Parkin; +Cc: netdev
In-Reply-To: <CACzvQmaj1_r4DHJjU7K4JN7jywX8y0_j_EQ_6tC8VkGryMmQPQ@mail.gmail.com>

On Thu, 2013-01-10 at 17:50 +0000, Tom Parkin wrote:
> Hi Eric,
> 
> I've just had a chance to try your fix for the veth oops I previously
> reported[1], but I still see some problems (please see oops below).
> 
> If I revert the original stats overhead fix (2681128f0ced8) and the
> subsequent NULL deref patch (d0e2c55e7c94) this problem goes away for
> me.
> 
> I've reproduced this bug using a script intended to stress test L2TP
> network namespace code, which uses veth tunnels to bridge namespaces.
> 
> Apologies for the late report!
> 
> Thanks,
> Tom
> 
> [1]
> http://www.spinics.net/lists/netdev/msg221284.html
> 
> BUG: unable to handle kernel NULL pointer dereference at 0000002c
> IP: [<c151f5f8>] netif_carrier_off+0x8/0x30
> *pde = 00000000
> Oops: 0002 [#1] SMP
> Modules linked in: veth bridge stp llc l2tp_ip6 l2tp_ip l2tp_ppp pppox
> l2tp_eth l2tp_netlink l2tp_core kvm_amd 9
> Pid: 5778, comm: ip Tainted: G        W    3.8.0-rc1-tpdev-33-lockdep+
> #42 Gigabyte Technology Co., Ltd. GA-MA62
> EIP: 0060:[<c151f5f8>] EFLAGS: 00010202 CPU: 0
> EIP is at netif_carrier_off+0x8/0x30
> EAX: 00000000 EBX: ee422800 ECX: 00000001 EDX: c18e4b80
> ESI: efec7c44 EDI: ee422840 EBP: efec7b38 ESP: efec7b38
>  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> CR0: 80050033 CR2: 0000002c CR3: 2fdb2000 CR4: 000007d0
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Process ip (pid: 5778, ti=efec6000 task=f1d4bf00 task.ti=efec6000)
> Stack:
>  efec7b44 f810e23d ee422800 efec7b60 c14fc169 ffffffff ffffffff f594c210
>  ee422800 efec7c44 efec7b80 c14fc25e efec7b6c ee422040 ee422040 efec7c04
>  ee422800 efec7c44 efec7ba0 c14fc357 f180dd00 efec7c44 ee422000 efec7c44
> Call Trace:
>  [<f810e23d>] veth_close+0x2d/0x60 [veth]
>  [<c14fc169>] __dev_close_many+0x69/0xb0
>  [<c14fc25e>] dev_close_many+0x6e/0xc0
>  [<c14fc357>] rollback_registered_many+0xa7/0x1c0
>  [<c14fc484>] unregister_netdevice_many+0x14/0x60
>  [<c1510e0e>] rtnl_dellink+0x9e/0x100
>  [<c10461a8>] ? ns_capable+0x38/0x70
>  [<c1510d70>] ? rtnetlink_put_metrics+0xf0/0xf0
>  [<c1512ae3>] rtnetlink_rcv_msg+0x153/0x2a0
>  [<c160db1a>] ? mutex_lock_nested+0x21a/0x2e0
>  [<c150f924>] ? rtnl_lock+0x14/0x20
>  [<c1512990>] ? __rtnl_unlock+0x20/0x20
>  [<c152a31e>] netlink_rcv_skb+0x8e/0xb0
>  [<c150f94c>] rtnetlink_rcv+0x1c/0x30
>  [<c1529cdd>] netlink_unicast+0x17d/0x1f0
>  [<c1529f74>] netlink_sendmsg+0x224/0x390
>  [<c14e8791>] sock_sendmsg+0xd1/0xf0
>  [<c1135b89>] ? might_fault+0x89/0x90
>  [<c12fa932>] ? _copy_from_user+0x42/0x60
>  [<c14f5c64>] ? verify_iovec+0x44/0xb0
>  [<c14e96a2>] __sys_sendmsg+0x262/0x270
>  [<c1073e4f>] ? sched_clock_cpu+0xcf/0x150
>  [<c109c02b>] ? trace_hardirqs_off+0xb/0x10
>  [<c1073f35>] ? local_clock+0x65/0x70
>  [<c109c69c>] ? lock_release_holdtime.part.23+0xbc/0xf0
>  [<c10a15ed>] ? lock_release_non_nested+0x29d/0x2e0
>  [<c1073f35>] ? local_clock+0x65/0x70
>  [<c1178ab1>] ? fget_light+0x371/0x450
>  [<c14ead9b>] sys_sendmsg+0x3b/0x60
>  [<c14eb443>] sys_socketcall+0x283/0x2e0
>  [<c1611310>] ? restore_all+0xf/0xf
>  [<c1614bc0>] ? __do_page_fault+0x4e0/0x4e0
>  [<c12fa548>] ? trace_hardirqs_on_thunk+0xc/0x10
>  [<c1618acd>] sysenter_do_call+0x12/0x38
> Code: 00 00 00 e8 ab 2f fd ff 8b 75 f8 b8 01 00 00 00 83 83 9c 00 00
> 00 01 8b 5d f4 8b 7d fc 89 ec 5d c3 66 90 3
> EIP: [<c151f5f8>] netif_carrier_off+0x8/0x30 SS:ESP 0068:efec7b38
> CR2: 000000000000002c
> ---[ end trace e9f7b433da03329a ]---
> 

Hi Tom, thanks for the report, I'll take a look.

^ permalink raw reply

* Re: R: Re: mtu issue with ipsec tunnel and netfilter snat
From: Jan Engelhardt @ 2013-01-10 17:57 UTC (permalink / raw)
  To: pupilla@libero.it; +Cc: netdev
In-Reply-To: <8364295.258481357836417891.JavaMail.defaultUser@defaultHost>

On Thursday 2013-01-10 17:46, pupilla@libero.it wrote:

>jengelh@inai.de wrote:
>>>
>>>But why linux_gw_snat is not sending icmp need to frag packets to
>>>10.148.12.23?
>>
>>Perhaps because ICMP was blocked erroneously?
>
>well, I don't see the icmp packets because tcpdump 'see' only the 
>incoming ipsec clear packets. Is there a way to see the outgoing clear 
>ipsec packets with tcpdump?

Not with AF_PACKET sockets (tcpdump uses such), but you could with 
iptables -j LOG, NFLOG and TRACE they can be made visible. If you need 
the full packet, you can either patch up LOG to call the kernel hexdump 
functions, or use NFLOG - I think - to deliver it to any userspace 
program to do further processing. If all else fails, there is also 
NFQUEUE with which the packet can also be copied to userspace.

^ permalink raw reply

* Re: tainted warnings with tcp splicing in 3.7.1
From: Rick Jones @ 2013-01-10 18:22 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Willy Tarreau, Christian Becker, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <1357834825.27446.2205.camel@edumazet-glaptop>

On 01/10/2013 08:20 AM, Eric Dumazet wrote:
> I also want to thanks Rick, as the latest netperf has splice() support.
>
> Thanks Rick !

You are quite welcome - and thank you for helping me get it to actually 
work :)

Those wishing to try it themselves should grab the top-of-trunk netperf 
bits from http://www.netperf.org/svn/netperf2/trunk .  The use of 
splice() is gated by a test-specific -V option:

raj@tardy:~/netperf2_trunk/src$ ./netperf -t omni -- -d recv -V
OMNI Receive TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
localhost.localdomain () port 0 AF_INET : copy avoidance : demo
Remote      Local       Remote Elapsed Throughput Throughput
Send Socket Recv Socket Send   Time               Units
Size        Size        Size   (sec)
Final       Final
1661688     4194304     16384  10.00   26103.14   10^6bits/s

You should see that "copy avoidance" appearing in the test banner.  It 
will also "take" for things like a migrated TCP_mumble test.  For those 
cases where you don't see a throughput change, enabling CPU utilization 
measurement and looking at that and service demand should show a difference.

happy benchmarking,

rick jones

^ permalink raw reply

* Re: [PATCH net-next v5 01/14] vlan: wrap hw-acceleration calls in separate functions.
From: Stephen Hemminger @ 2013-01-10 18:25 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: mst, netdev, stephen, bridge, shmulik.ladkani, davem
In-Reply-To: <1357751882-8619-2-git-send-email-vyasevic@redhat.com>

On Wed,  9 Jan 2013 12:17:48 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:

>  
>  /**
> + * vlan_hw_buggy - Check to see if VLAN hw acceleration is supported.
> + * @dev: netdevice of the lowerdev/hw nic
> + *
> + * Checks to see if HW and driver report VLAN acceleration correctly.
> + */
> +static inline bool vlan_hw_buggy(const struct net_device *dev)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid))
> +		return true;
> +
> +	return false;
> +}
> +
> +/**
> + * vlan_vid_add_hw - Add the VLAN vid to the HW filter
> + * @dev: netdevice of the lowerdev/hw nic
> + * @vid: vlan id.
> + *
> + * Inserts the vid into the HW vlan filter table if hw supports it.
> + */
> +static inline int vlan_vid_add_hw(struct net_device *dev,
> +				  unsigned short vid)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +	int err = 0;
> +
> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    ops->ndo_vlan_rx_add_vid)
> +		err = ops->ndo_vlan_rx_add_vid(dev, vid);
> +
> +	return err;
> +}
> +
> +/**
> + * vlan_vid_del_hw - Delete the VLAN vid from the HW filter
> + * @dev: netdevice of the lowerdev/hw nic
> + * @vid: vlan id.
> + *
> + * Delete the vid from the HW vlan filter table if hw supports it.
> + */
> +static inline int vlan_vid_del_hw(struct net_device *dev,
> +				  unsigned short vid)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +	int err = 0;
> +
> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    ops->ndo_vlan_rx_kill_vid)
> +		err = ops->ndo_vlan_rx_add_vid(dev, vid);
> +
> +	return err;
> +}
> +

I would rather not have all these inline's. This isn't performance critical.
Also, the check for buggy devices should be done inside the vlan code,
not repeated in the functions using the add/remove API. When device is
registered the flag and add/kill should be checked, and if the device driver
is buggy it should fail the register_netdevice.

^ permalink raw reply

* Re: BUG: NULL pointer dereference in netif_carrier_off
From: Eric Dumazet @ 2013-01-10 18:32 UTC (permalink / raw)
  To: Tom Parkin, David Miller; +Cc: netdev
In-Reply-To: <1357840590.27446.2331.camel@edumazet-glaptop>

From: Eric Dumazet <edumazet@google.com>

On Thu, 2013-01-10 at 09:56 -0800, Eric Dumazet wrote:

> 
> Hi Tom, thanks for the report, I'll take a look.

Oh well, fix seems straightforward...

Thanks !

[PATCH net-next] veth: fix a NULL deref in netif_carrier_off

In commit d0e2c55e7c94 (veth: avoid a NULL deref in veth_stats_one)
we now clear the peer pointers in veth_dellink()

veth_close() must therefore make sure the peer pointer is set.

Reported-by: Tom Parkin <tom.parkin@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/veth.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 0f71a4f..e1da42a 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -206,9 +206,11 @@ static int veth_open(struct net_device *dev)
 static int veth_close(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer = rtnl_dereference(priv->peer);
 
 	netif_carrier_off(dev);
-	netif_carrier_off(rtnl_dereference(priv->peer));
+	if (peer)
+		netif_carrier_off(peer);
 
 	return 0;
 }

^ permalink raw reply related

* RE: tainted warnings with tcp splicing in 3.7.1
From: Lukas Tribus @ 2013-01-10 18:27 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1357801149.27446.1142.camel@edumazet-glaptop>


Hi Eric,

this is probably a dumb question ... but since the fix is in net/ipv4/tcp.c I was asking myself if this can affect IPv6 as well?



Thanks,

Lukas


> [PATCH] tcp: fix splice() and tcp collapsing interaction
> 
> Under unusual circumstances, TCP collapse can split a big GRO TCP packet
> while its being used in a splice(socket->pipe) operation.
> 
> skb_splice_bits() releases the socket lock before calling
> splice_to_pipe().
> 
> [ 1081.353685] WARNING: at net/ipv4/tcp.c:1330 tcp_cleanup_rbuf+0x4d/0xfc()
> [ 1081.371956] Hardware name: System x3690 X5 -[7148Z68]-
> [ 1081.391820] cleanup rbuf bug: copied AD3BCF1 seq AD370AF rcvnxt AD3CF13
> 
> To fix this problem, we must eat skbs in tcp_recv_skb().
> 
> Remove the inline keyword from tcp_recv_skb() definition since
> it has three call sites.
> 
> Reported-by: Christian Becker <c.becker@traviangames.com>
> Cc: Willy Tarreau <w@1wt.eu>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv4/tcp.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1ca2536..1f901be 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1428,12 +1428,12 @@ static void tcp_service_net_dma(struct sock *sk, bool wait)
>  }
>  #endif
>  
> -static inline struct sk_buff *tcp_recv_skb(struct sock *sk, u32 seq, u32 *off)
> +static struct sk_buff *tcp_recv_skb(struct sock *sk, u32 seq, u32 *off)
>  {
>  	struct sk_buff *skb;
>  	u32 offset;
>  
> -	skb_queue_walk(&sk->sk_receive_queue, skb) {
> +	while ((skb = skb_peek(&sk->sk_receive_queue)) != NULL) {
>  		offset = seq - TCP_SKB_CB(skb)->seq;
>  		if (tcp_hdr(skb)->syn)
>  			offset--;
> @@ -1441,6 +1441,11 @@ static inline struct sk_buff *tcp_recv_skb(struct sock *sk, u32 seq, u32 *off)
>  			*off = offset;
>  			return skb;
>  		}
> +		/* This looks weird, but this can happen if TCP collapsing
> +		 * splitted a fat GRO packet, while we released socket lock
> +		 * in skb_splice_bits()
> +		 */
> +		sk_eat_skb(sk, skb, false);
>  	}
>  	return NULL;
>  }
> @@ -1520,8 +1525,10 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
>  	tcp_rcv_space_adjust(sk);
>  
>  	/* Clean up data we have read: This will do ACK frames. */
> -	if (copied > 0)
> +	if (copied > 0) {
> +		tcp_recv_skb(sk, seq, &offset);
>  		tcp_cleanup_rbuf(sk, copied);
> +	}
>  	return copied;
>  }
>  EXPORT_SYMBOL(tcp_read_sock);
> 
 		 	   		  

^ permalink raw reply

* Re: [PATCH net-next v5 02/14] bridge: Add vlan filtering infrastructure
From: Stephen Hemminger @ 2013-01-10 18:36 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: mst, netdev, stephen, bridge, shmulik.ladkani, davem
In-Reply-To: <1357751882-8619-3-git-send-email-vyasevic@redhat.com>

This patch has some minor whitespace and spelling errors:

WARNING: line over 80 characters
#429: FILE: net/bridge/br_private.h:205:
+static inline struct net_bridge_port *vlans_to_port(struct net_port_vlans *vlans)

ERROR: trailing whitespace
#432: FILE: net/bridge/br_private.h:208:
+       $

WARNING: please, no spaces at the start of a line
#432: FILE: net/bridge/br_private.h:208:
+       $

+/* Must be protected by RTNL */
+static void br_vlan_del(struct net_bridge_vlan *vlan)

+	/* Drop the self-ref to trigger descrution. */
                                        ^^^^^^^^^^

Also, the data structure vlan's seems inverted. Why do you keep a hash list
of vlan's and then a bitmap of ports. Seems more natural to just put a bitmap
on each port that has vlan filtering rather than introducing yet another list
to manage.

^ permalink raw reply

* RE: tainted warnings with tcp splicing in 3.7.1
From: Eric Dumazet @ 2013-01-10 18:37 UTC (permalink / raw)
  To: Lukas Tribus; +Cc: netdev
In-Reply-To: <DUB107-W210F9409862AAA949CB15ED2A0@phx.gbl>

On Thu, 2013-01-10 at 19:27 +0100, Lukas Tribus wrote:
> Hi Eric,
> 
> this is probably a dumb question ... but since the fix is in net/ipv4/tcp.c I was asking myself if this can affect IPv6 as well?
> 

Yes it can, net/ipv4 contains generic TCP code.

^ permalink raw reply

* Re: [PATCH net-next v5 01/14] vlan: wrap hw-acceleration calls in separate functions.
From: Vlad Yasevich @ 2013-01-10 18:41 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, davem, stephen, bridge, shmulik.ladkani, mst
In-Reply-To: <20130110102525.2ff40c12@nehalam.linuxnetplumber.net>

On 01/10/2013 01:25 PM, Stephen Hemminger wrote:
> On Wed,  9 Jan 2013 12:17:48 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>>
>>   /**
>> + * vlan_hw_buggy - Check to see if VLAN hw acceleration is supported.
>> + * @dev: netdevice of the lowerdev/hw nic
>> + *
>> + * Checks to see if HW and driver report VLAN acceleration correctly.
>> + */
>> +static inline bool vlan_hw_buggy(const struct net_device *dev)
>> +{
>> +	const struct net_device_ops *ops = dev->netdev_ops;
>> +
>> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +	    (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/**
>> + * vlan_vid_add_hw - Add the VLAN vid to the HW filter
>> + * @dev: netdevice of the lowerdev/hw nic
>> + * @vid: vlan id.
>> + *
>> + * Inserts the vid into the HW vlan filter table if hw supports it.
>> + */
>> +static inline int vlan_vid_add_hw(struct net_device *dev,
>> +				  unsigned short vid)
>> +{
>> +	const struct net_device_ops *ops = dev->netdev_ops;
>> +	int err = 0;
>> +
>> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +	    ops->ndo_vlan_rx_add_vid)
>> +		err = ops->ndo_vlan_rx_add_vid(dev, vid);
>> +
>> +	return err;
>> +}
>> +
>> +/**
>> + * vlan_vid_del_hw - Delete the VLAN vid from the HW filter
>> + * @dev: netdevice of the lowerdev/hw nic
>> + * @vid: vlan id.
>> + *
>> + * Delete the vid from the HW vlan filter table if hw supports it.
>> + */
>> +static inline int vlan_vid_del_hw(struct net_device *dev,
>> +				  unsigned short vid)
>> +{
>> +	const struct net_device_ops *ops = dev->netdev_ops;
>> +	int err = 0;
>> +
>> +	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +	    ops->ndo_vlan_rx_kill_vid)
>> +		err = ops->ndo_vlan_rx_add_vid(dev, vid);
>> +
>> +	return err;
>> +}
>> +
>
> I would rather not have all these inline's. This isn't performance critical.

I kind of need to keep them as inlines because of the VLAN support is 
built.  Right now, none of the VLAN files are build if VLAN support is
turned off.  So all we get access to are inlines from if_vlan.h.

I suppose I can change how VLANs get built, but not if that's the right 
thing.  It looks like it is set up the way it is on purpose.

> Also, the check for buggy devices should be done inside the vlan code,
> not repeated in the functions using the add/remove API. When device is
> registered the flag and add/kill should be checked, and if the device driver
> is buggy it should fail the register_netdevice.
>

Not sure what you mean here.  I don't check if it's buggy again.  I 
check that the device supports filter and the pointer is set.  I does
exactly what the code used to do.  I suppose that the checks for valid
function pointers may be a little redundant since otherwise 
vlan_hw_buggy() would have triggered, but it's safer to have them since 
we can't guarantee that other users have checked for buggy implementations.

-vlad

^ permalink raw reply

* Re: tainted warnings with tcp splicing in 3.7.1
From: Eric Dumazet @ 2013-01-10 18:42 UTC (permalink / raw)
  To: Rick Jones
  Cc: Willy Tarreau, Christian Becker, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <50EF06DD.7070207@hp.com>

On Thu, 2013-01-10 at 10:22 -0800, Rick Jones wrote:
> On 01/10/2013 08:20 AM, Eric Dumazet wrote:
> > I also want to thanks Rick, as the latest netperf has splice() support.
> >
> > Thanks Rick !
> 
> You are quite welcome - and thank you for helping me get it to actually 
> work :)
> 
> Those wishing to try it themselves should grab the top-of-trunk netperf 
> bits from http://www.netperf.org/svn/netperf2/trunk .  The use of 
> splice() is gated by a test-specific -V option:
> 
> raj@tardy:~/netperf2_trunk/src$ ./netperf -t omni -- -d recv -V
> OMNI Receive TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
> localhost.localdomain () port 0 AF_INET : copy avoidance : demo
> Remote      Local       Remote Elapsed Throughput Throughput
> Send Socket Recv Socket Send   Time               Units
> Size        Size        Size   (sec)
> Final       Final
> 1661688     4194304     16384  10.00   26103.14   10^6bits/s
> 
> You should see that "copy avoidance" appearing in the test banner.  It 
> will also "take" for things like a migrated TCP_mumble test.  For those 
> cases where you don't see a throughput change, enabling CPU utilization 
> measurement and looking at that and service demand should show a difference.

Thanks Rick !

Can we use zero copy for the sender as well (sendfile() or vmsplice()) ?

Here are some results :

Reference time (no splice())

# ./netperf -H 127.0.0.1 -t omni -- -d recv     
OMNI Receive TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 127.0.0.1 ()
port 0 AF_INET
catcher: timer popped with times_up != 0
Remote      Local       Remote Elapsed Throughput Throughput  
Send Socket Recv Socket Send   Time               Units       
Size        Size        Size   (sec)                          
Final       Final                                             
2097152     2097152     16384  10.00   33237.74   10^6bits/s  


zero copy at receiver (splice(socket ->pipe), splice(pipe -> /dev/null))

# ./netperf -H 127.0.0.1 -t omni -- -d recv -V
OMNI Receive TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 127.0.0.1 ()
port 0 AF_INET : copy avoidance
catcher: timer popped with times_up != 0
Remote      Local       Remote Elapsed Throughput Throughput  
Send Socket Recv Socket Send   Time               Units       
Size        Size        Size   (sec)                          
Final       Final                                             
1325580     2097152     16384  10.00   51980.60   10^6bits/s  

^ permalink raw reply

* Re: tainted warnings with tcp splicing in 3.7.1
From: Rick Jones @ 2013-01-10 18:49 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Willy Tarreau, Christian Becker, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <1357843328.27446.2417.camel@edumazet-glaptop>

On 01/10/2013 10:42 AM, Eric Dumazet wrote:
> Can we use zero copy for the sender as well (sendfile() or vmsplice()) ?

Not at present.  The TCP_SENDFILE test has not been "migrated" and there 
is no corresponding sendfile() test in the omni code, so the attempt to 
enable copy-avoidance won't "take."

I'll take that as a feature request for netperf.

rick

^ permalink raw reply

* [PATCH v2 00/10] Make XPS usable within ixgbe
From: Alexander Duyck @ 2013-01-10 18:56 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem

The following patch series makes it so that the ixgbe driver can support
ATR even when the number of queues is less than the number of CPUs.  To do
this I have updated the kernel to support letting drivers set their own XPS
configuration.  To do this it was necessary to move the code out of the
sysfs specific code and into the dev specific regions.

It has been almost 6 months since the last time I released these patches.  I
have been meaning to get back to them but I just haven't had a chance to get
this work done until recently.

Only the core net patches are meant for acceptance at this time.  I will
submit the ixgbe patches through Jeff Kirsher's tree.  For this reason I have
tagged the ixgbe related patches as RFC.

---

rc2: Updated first patch to address changes from dev_pick_tx to netdev_pick_tx.
     Made it so that we only update XPS map on queue number change.
     Minor tweaks and cleanups to get caught up with recent changes in the tree.

Alexander Duyck (10):
      net: Split core bits of netdev_pick_tx into __netdev_pick_tx
      net: Add functions netif_reset_xps_queue and netif_set_xps_queue
      net: Rewrite netif_reset_xps_queue to allow for better code reuse
      net: Rewrite netif_set_xps_queues to address several issues
      net: Add support for XPS without sysfs being defined
      ixgbe: Define FCoE and Flow director limits much sooner to allow for changes
      ixgbe: Add function for setting XPS queue mapping
      ixgbe: Update ixgbe driver to use __netdev_pick_tx in ixgbe_select_queue
      ixgbe: Add support for displaying the number of Tx/Rx channels
      ixgbe: Add support for set_channels ethtool operation


 drivers/net/ethernet/intel/ixgbe/ixgbe.h         |    3 
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c |  112 +++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c     |   21 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |   77 ++++--
 include/linux/netdevice.h                        |   13 +
 net/Kconfig                                      |    2 
 net/core/dev.c                                   |  289 ++++++++++++++++++++--
 net/core/net-sysfs.c                             |  160 ------------
 8 files changed, 454 insertions(+), 223 deletions(-)

-- 

^ permalink raw reply

* [PATCH v2 01/10] net: Split core bits of netdev_pick_tx into __netdev_pick_tx
From: Alexander Duyck @ 2013-01-10 18:56 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

This change splits the core bits of netdev_pick_tx into a separate function.
The main idea behind this is to make this code accessible to select queue
functions when they decide to process the standard path instead of their
own custom path in their select queue routine.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 include/linux/netdevice.h |    1 +
 net/core/dev.c            |   57 +++++++++++++++++++++++++--------------------
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0209ac3..608c3ac 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1403,6 +1403,7 @@ static inline void netdev_for_each_tx_queue(struct net_device *dev,
 
 extern struct netdev_queue *netdev_pick_tx(struct net_device *dev,
 					   struct sk_buff *skb);
+extern u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb);
 
 /*
  * Net namespace inlines
diff --git a/net/core/dev.c b/net/core/dev.c
index 594830e..04f04e0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2495,37 +2495,44 @@ static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
 #endif
 }
 
-struct netdev_queue *netdev_pick_tx(struct net_device *dev,
-				    struct sk_buff *skb)
+u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb)
 {
-	int queue_index;
-	const struct net_device_ops *ops = dev->netdev_ops;
-
-	if (dev->real_num_tx_queues == 1)
-		queue_index = 0;
-	else if (ops->ndo_select_queue) {
-		queue_index = ops->ndo_select_queue(dev, skb);
-		queue_index = dev_cap_txqueue(dev, queue_index);
-	} else {
-		struct sock *sk = skb->sk;
-		queue_index = sk_tx_queue_get(sk);
+	struct sock *sk = skb->sk;
+	int queue_index = sk_tx_queue_get(sk);
 
-		if (queue_index < 0 || skb->ooo_okay ||
-		    queue_index >= dev->real_num_tx_queues) {
-			int old_index = queue_index;
+	if (queue_index < 0 || skb->ooo_okay ||
+	    queue_index >= dev->real_num_tx_queues) {
+		int new_index = get_xps_queue(dev, skb);
+		if (new_index < 0)
+			new_index = skb_tx_hash(dev, skb);
 
-			queue_index = get_xps_queue(dev, skb);
-			if (queue_index < 0)
-				queue_index = skb_tx_hash(dev, skb);
-
-			if (queue_index != old_index && sk) {
-				struct dst_entry *dst =
+		if (queue_index != new_index && sk) {
+			struct dst_entry *dst =
 				    rcu_dereference_check(sk->sk_dst_cache, 1);
 
-				if (dst && skb_dst(skb) == dst)
-					sk_tx_queue_set(sk, queue_index);
-			}
+			if (dst && skb_dst(skb) == dst)
+				sk_tx_queue_set(sk, queue_index);
+
 		}
+
+		queue_index = new_index;
+	}
+
+	return queue_index;
+}
+
+struct netdev_queue *netdev_pick_tx(struct net_device *dev,
+				    struct sk_buff *skb)
+{
+	int queue_index = 0;
+
+	if (dev->real_num_tx_queues != 1) {
+		const struct net_device_ops *ops = dev->netdev_ops;
+		if (ops->ndo_select_queue)
+			queue_index = ops->ndo_select_queue(dev, skb);
+		else
+			queue_index = __netdev_pick_tx(dev, skb);
+		queue_index = dev_cap_txqueue(dev, queue_index);
 	}
 
 	skb_set_queue_mapping(skb, queue_index);

^ permalink raw reply related

* [PATCH v2 02/10] net: Add functions netif_reset_xps_queue and netif_set_xps_queue
From: Alexander Duyck @ 2013-01-10 18:57 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

This patch adds two functions, netif_reset_xps_queue and
netif_set_xps_queue.  The main idea behind these to functions is to provide
a mechanism through which drivers can update their defaults in regards to
XPS.

Currently no such mechanism exists and as a result we cannot use XPS for
things such as ATR which would require a basic configuration to start in
which the Tx queues are mapped to CPUs via a 1:1 mapping.  With this change
I am making it possible for drivers such as ixgbe to be able to use the XPS
feature by controlling the default configuration.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 include/linux/netdevice.h |   13 ++++
 net/core/dev.c            |  155 +++++++++++++++++++++++++++++++++++++++++++++
 net/core/net-sysfs.c      |  148 +------------------------------------------
 3 files changed, 173 insertions(+), 143 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 608c3ac..59fe9da 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2103,6 +2103,19 @@ static inline void netif_wake_subqueue(struct net_device *dev, u16 queue_index)
 		__netif_schedule(txq->qdisc);
 }
 
+#ifdef CONFIG_XPS
+extern void netif_reset_xps_queue(struct net_device *dev, u16 index);
+extern int netif_set_xps_queue(struct net_device *dev, struct cpumask *mask,
+			       u16 index);
+#else
+static inline int netif_set_xps_queue(struct net_device *dev,
+				      struct cpumask *mask,
+				      u16 index)
+{
+	return 0;
+}
+#endif
+
 /*
  * Returns a Tx hash for the given packet when dev->real_num_tx_queues is used
  * as a distribution range limit for the returned value.
diff --git a/net/core/dev.c b/net/core/dev.c
index 04f04e0..76126fb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1857,6 +1857,161 @@ static void netif_setup_tc(struct net_device *dev, unsigned int txq)
 	}
 }
 
+#ifdef CONFIG_XPS
+static DEFINE_MUTEX(xps_map_mutex);
+#define xmap_dereference(P)		\
+	rcu_dereference_protected((P), lockdep_is_held(&xps_map_mutex))
+
+void netif_reset_xps_queue(struct net_device *dev, u16 index)
+{
+	struct xps_dev_maps *dev_maps;
+	struct xps_map *map;
+	int i, pos, nonempty = 0;
+
+	mutex_lock(&xps_map_mutex);
+	dev_maps = xmap_dereference(dev->xps_maps);
+
+	if (!dev_maps)
+		goto out_no_maps;
+
+	for_each_possible_cpu(i) {
+		map = xmap_dereference(dev_maps->cpu_map[i]);
+		if (!map)
+			continue;
+
+		for (pos = 0; pos < map->len; pos++)
+			if (map->queues[pos] == index)
+				break;
+
+		if (pos < map->len) {
+			if (map->len > 1) {
+				map->queues[pos] = map->queues[--map->len];
+			} else {
+				RCU_INIT_POINTER(dev_maps->cpu_map[i], NULL);
+				kfree_rcu(map, rcu);
+				map = NULL;
+			}
+		}
+		if (map)
+			nonempty = 1;
+	}
+
+	if (!nonempty) {
+		RCU_INIT_POINTER(dev->xps_maps, NULL);
+		kfree_rcu(dev_maps, rcu);
+	}
+
+out_no_maps:
+	mutex_unlock(&xps_map_mutex);
+}
+
+int netif_set_xps_queue(struct net_device *dev, struct cpumask *mask, u16 index)
+{
+	int i, cpu, pos, map_len, alloc_len, need_set;
+	struct xps_map *map, *new_map;
+	struct xps_dev_maps *dev_maps, *new_dev_maps;
+	int nonempty = 0;
+	int numa_node_id = -2;
+	int maps_sz = max_t(unsigned int, XPS_DEV_MAPS_SIZE, L1_CACHE_BYTES);
+
+	new_dev_maps = kzalloc(maps_sz, GFP_KERNEL);
+	if (!new_dev_maps)
+		return -ENOMEM;
+
+	mutex_lock(&xps_map_mutex);
+
+	dev_maps = xmap_dereference(dev->xps_maps);
+
+	for_each_possible_cpu(cpu) {
+		map = dev_maps ?
+			xmap_dereference(dev_maps->cpu_map[cpu]) : NULL;
+		new_map = map;
+		if (map) {
+			for (pos = 0; pos < map->len; pos++)
+				if (map->queues[pos] == index)
+					break;
+			map_len = map->len;
+			alloc_len = map->alloc_len;
+		} else
+			pos = map_len = alloc_len = 0;
+
+		need_set = cpumask_test_cpu(cpu, mask) && cpu_online(cpu);
+#ifdef CONFIG_NUMA
+		if (need_set) {
+			if (numa_node_id == -2)
+				numa_node_id = cpu_to_node(cpu);
+			else if (numa_node_id != cpu_to_node(cpu))
+				numa_node_id = -1;
+		}
+#endif
+		if (need_set && pos >= map_len) {
+			/* Need to add queue to this CPU's map */
+			if (map_len >= alloc_len) {
+				alloc_len = alloc_len ?
+				    2 * alloc_len : XPS_MIN_MAP_ALLOC;
+				new_map = kzalloc_node(XPS_MAP_SIZE(alloc_len),
+						       GFP_KERNEL,
+						       cpu_to_node(cpu));
+				if (!new_map)
+					goto error;
+				new_map->alloc_len = alloc_len;
+				for (i = 0; i < map_len; i++)
+					new_map->queues[i] = map->queues[i];
+				new_map->len = map_len;
+			}
+			new_map->queues[new_map->len++] = index;
+		} else if (!need_set && pos < map_len) {
+			/* Need to remove queue from this CPU's map */
+			if (map_len > 1)
+				new_map->queues[pos] =
+				    new_map->queues[--new_map->len];
+			else
+				new_map = NULL;
+		}
+		RCU_INIT_POINTER(new_dev_maps->cpu_map[cpu], new_map);
+	}
+
+	/* Cleanup old maps */
+	for_each_possible_cpu(cpu) {
+		map = dev_maps ?
+			xmap_dereference(dev_maps->cpu_map[cpu]) : NULL;
+		if (map && xmap_dereference(new_dev_maps->cpu_map[cpu]) != map)
+			kfree_rcu(map, rcu);
+		if (new_dev_maps->cpu_map[cpu])
+			nonempty = 1;
+	}
+
+	if (nonempty) {
+		rcu_assign_pointer(dev->xps_maps, new_dev_maps);
+	} else {
+		kfree(new_dev_maps);
+		RCU_INIT_POINTER(dev->xps_maps, NULL);
+	}
+
+	if (dev_maps)
+		kfree_rcu(dev_maps, rcu);
+
+	netdev_queue_numa_node_write(netdev_get_tx_queue(dev, index),
+				     (numa_node_id >= 0) ? numa_node_id :
+				     NUMA_NO_NODE);
+
+	mutex_unlock(&xps_map_mutex);
+
+	return 0;
+error:
+	mutex_unlock(&xps_map_mutex);
+
+	if (new_dev_maps)
+		for_each_possible_cpu(i)
+			kfree(rcu_dereference_protected(
+				new_dev_maps->cpu_map[i],
+				1));
+	kfree(new_dev_maps);
+	return -ENOMEM;
+}
+EXPORT_SYMBOL(netif_set_xps_queue);
+
+#endif
 /*
  * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
  * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 29c884a..5ad489d 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1002,54 +1002,14 @@ static ssize_t show_xps_map(struct netdev_queue *queue,
 	return len;
 }
 
-static DEFINE_MUTEX(xps_map_mutex);
-#define xmap_dereference(P)		\
-	rcu_dereference_protected((P), lockdep_is_held(&xps_map_mutex))
-
 static void xps_queue_release(struct netdev_queue *queue)
 {
 	struct net_device *dev = queue->dev;
-	struct xps_dev_maps *dev_maps;
-	struct xps_map *map;
 	unsigned long index;
-	int i, pos, nonempty = 0;
 
 	index = get_netdev_queue_index(queue);
 
-	mutex_lock(&xps_map_mutex);
-	dev_maps = xmap_dereference(dev->xps_maps);
-
-	if (dev_maps) {
-		for_each_possible_cpu(i) {
-			map = xmap_dereference(dev_maps->cpu_map[i]);
-			if (!map)
-				continue;
-
-			for (pos = 0; pos < map->len; pos++)
-				if (map->queues[pos] == index)
-					break;
-
-			if (pos < map->len) {
-				if (map->len > 1)
-					map->queues[pos] =
-					    map->queues[--map->len];
-				else {
-					RCU_INIT_POINTER(dev_maps->cpu_map[i],
-					    NULL);
-					kfree_rcu(map, rcu);
-					map = NULL;
-				}
-			}
-			if (map)
-				nonempty = 1;
-		}
-
-		if (!nonempty) {
-			RCU_INIT_POINTER(dev->xps_maps, NULL);
-			kfree_rcu(dev_maps, rcu);
-		}
-	}
-	mutex_unlock(&xps_map_mutex);
+	netif_reset_xps_queue(dev, index);
 }
 
 static ssize_t store_xps_map(struct netdev_queue *queue,
@@ -1057,13 +1017,9 @@ static ssize_t store_xps_map(struct netdev_queue *queue,
 		      const char *buf, size_t len)
 {
 	struct net_device *dev = queue->dev;
-	cpumask_var_t mask;
-	int err, i, cpu, pos, map_len, alloc_len, need_set;
 	unsigned long index;
-	struct xps_map *map, *new_map;
-	struct xps_dev_maps *dev_maps, *new_dev_maps;
-	int nonempty = 0;
-	int numa_node_id = -2;
+	cpumask_var_t mask;
+	int err;
 
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;
@@ -1079,105 +1035,11 @@ static ssize_t store_xps_map(struct netdev_queue *queue,
 		return err;
 	}
 
-	new_dev_maps = kzalloc(max_t(unsigned int,
-	    XPS_DEV_MAPS_SIZE, L1_CACHE_BYTES), GFP_KERNEL);
-	if (!new_dev_maps) {
-		free_cpumask_var(mask);
-		return -ENOMEM;
-	}
-
-	mutex_lock(&xps_map_mutex);
-
-	dev_maps = xmap_dereference(dev->xps_maps);
-
-	for_each_possible_cpu(cpu) {
-		map = dev_maps ?
-			xmap_dereference(dev_maps->cpu_map[cpu]) : NULL;
-		new_map = map;
-		if (map) {
-			for (pos = 0; pos < map->len; pos++)
-				if (map->queues[pos] == index)
-					break;
-			map_len = map->len;
-			alloc_len = map->alloc_len;
-		} else
-			pos = map_len = alloc_len = 0;
-
-		need_set = cpumask_test_cpu(cpu, mask) && cpu_online(cpu);
-#ifdef CONFIG_NUMA
-		if (need_set) {
-			if (numa_node_id == -2)
-				numa_node_id = cpu_to_node(cpu);
-			else if (numa_node_id != cpu_to_node(cpu))
-				numa_node_id = -1;
-		}
-#endif
-		if (need_set && pos >= map_len) {
-			/* Need to add queue to this CPU's map */
-			if (map_len >= alloc_len) {
-				alloc_len = alloc_len ?
-				    2 * alloc_len : XPS_MIN_MAP_ALLOC;
-				new_map = kzalloc_node(XPS_MAP_SIZE(alloc_len),
-						       GFP_KERNEL,
-						       cpu_to_node(cpu));
-				if (!new_map)
-					goto error;
-				new_map->alloc_len = alloc_len;
-				for (i = 0; i < map_len; i++)
-					new_map->queues[i] = map->queues[i];
-				new_map->len = map_len;
-			}
-			new_map->queues[new_map->len++] = index;
-		} else if (!need_set && pos < map_len) {
-			/* Need to remove queue from this CPU's map */
-			if (map_len > 1)
-				new_map->queues[pos] =
-				    new_map->queues[--new_map->len];
-			else
-				new_map = NULL;
-		}
-		RCU_INIT_POINTER(new_dev_maps->cpu_map[cpu], new_map);
-	}
-
-	/* Cleanup old maps */
-	for_each_possible_cpu(cpu) {
-		map = dev_maps ?
-			xmap_dereference(dev_maps->cpu_map[cpu]) : NULL;
-		if (map && xmap_dereference(new_dev_maps->cpu_map[cpu]) != map)
-			kfree_rcu(map, rcu);
-		if (new_dev_maps->cpu_map[cpu])
-			nonempty = 1;
-	}
-
-	if (nonempty) {
-		rcu_assign_pointer(dev->xps_maps, new_dev_maps);
-	} else {
-		kfree(new_dev_maps);
-		RCU_INIT_POINTER(dev->xps_maps, NULL);
-	}
-
-	if (dev_maps)
-		kfree_rcu(dev_maps, rcu);
-
-	netdev_queue_numa_node_write(queue, (numa_node_id >= 0) ? numa_node_id :
-					    NUMA_NO_NODE);
-
-	mutex_unlock(&xps_map_mutex);
+	err = netif_set_xps_queue(dev, mask, index);
 
 	free_cpumask_var(mask);
-	return len;
 
-error:
-	mutex_unlock(&xps_map_mutex);
-
-	if (new_dev_maps)
-		for_each_possible_cpu(i)
-			kfree(rcu_dereference_protected(
-				new_dev_maps->cpu_map[i],
-				1));
-	kfree(new_dev_maps);
-	free_cpumask_var(mask);
-	return -ENOMEM;
+	return err ? : len;
 }
 
 static struct netdev_queue_attribute xps_cpus_attribute =

^ permalink raw reply related

* [PATCH v2 03/10] net: Rewrite netif_reset_xps_queue to allow for better code reuse
From: Alexander Duyck @ 2013-01-10 18:57 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

This patch does a minor refactor on netif_reset_xps_queue to address a few
items I noticed.

First is the fact that we are doing removal of queues in both
netif_reset_xps_queue and netif_set_xps_queue.  Since there is no need to
have the code in two places I am pushing it out into a separate function
and will come back in another patch and reuse the code in
netif_set_xps_queue.

The second item this change addresses is the fact that the Tx queues were
not getting their numa_node value cleared as a part of the XPS queue reset.
This patch resolves that by resetting the numa_node value if the dev_maps
value is set.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 net/core/dev.c |   56 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 76126fb..fccee52 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1862,45 +1862,55 @@ static DEFINE_MUTEX(xps_map_mutex);
 #define xmap_dereference(P)		\
 	rcu_dereference_protected((P), lockdep_is_held(&xps_map_mutex))
 
-void netif_reset_xps_queue(struct net_device *dev, u16 index)
+static struct xps_map *remove_xps_queue(struct xps_dev_maps *dev_maps,
+					int cpu, u16 index)
 {
-	struct xps_dev_maps *dev_maps;
-	struct xps_map *map;
-	int i, pos, nonempty = 0;
-
-	mutex_lock(&xps_map_mutex);
-	dev_maps = xmap_dereference(dev->xps_maps);
-
-	if (!dev_maps)
-		goto out_no_maps;
+	struct xps_map *map = NULL;
+	int pos;
 
-	for_each_possible_cpu(i) {
-		map = xmap_dereference(dev_maps->cpu_map[i]);
-		if (!map)
-			continue;
-
-		for (pos = 0; pos < map->len; pos++)
-			if (map->queues[pos] == index)
-				break;
+	if (dev_maps)
+		map = xmap_dereference(dev_maps->cpu_map[cpu]);
 
-		if (pos < map->len) {
+	for (pos = 0; map && pos < map->len; pos++) {
+		if (map->queues[pos] == index) {
 			if (map->len > 1) {
 				map->queues[pos] = map->queues[--map->len];
 			} else {
-				RCU_INIT_POINTER(dev_maps->cpu_map[i], NULL);
+				RCU_INIT_POINTER(dev_maps->cpu_map[cpu], NULL);
 				kfree_rcu(map, rcu);
 				map = NULL;
 			}
+			break;
 		}
-		if (map)
-			nonempty = 1;
 	}
 
-	if (!nonempty) {
+	return map;
+}
+
+void netif_reset_xps_queue(struct net_device *dev, u16 index)
+{
+	struct xps_dev_maps *dev_maps;
+	int cpu;
+	bool active = false;
+
+	mutex_lock(&xps_map_mutex);
+	dev_maps = xmap_dereference(dev->xps_maps);
+
+	if (!dev_maps)
+		goto out_no_maps;
+
+	for_each_possible_cpu(cpu) {
+		if (remove_xps_queue(dev_maps, cpu, index))
+			active = true;
+	}
+
+	if (!active) {
 		RCU_INIT_POINTER(dev->xps_maps, NULL);
 		kfree_rcu(dev_maps, rcu);
 	}
 
+	netdev_queue_numa_node_write(netdev_get_tx_queue(dev, index),
+				     NUMA_NO_NODE);
 out_no_maps:
 	mutex_unlock(&xps_map_mutex);
 }

^ permalink raw reply related

* [PATCH v2 04/10] net: Rewrite netif_set_xps_queues to address several issues
From: Alexander Duyck @ 2013-01-10 18:57 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

This change is meant to address several issues I found within the
netif_set_xps_queues function.

If the allocation of one of the maps to be assigned to new_dev_maps failed
we could end up with the device map in an inconsistent state since we had
already worked through a number of CPUs and removed or added the queue.  To
address that I split the process into several steps.  The first of which is
just the allocation of updated maps for CPUs that will need larger maps to
store the queue.  By doing this we can fail gracefully without actually
altering the contents of the current device map.

The second issue I found was the fact that we were always allocating a new
device map even if we were not adding any queues.  I have updated the code
so that we only allocate a new device map if we are adding queues,
otherwise if we are not adding any queues to CPUs we just skip to the
removal process.

The last change I made was to reuse the code from remove_xps_queue to remove
the queue from the CPU.  By making this change we can be consistent in how
we go about adding and removing the queues from the CPUs.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 net/core/dev.c |  183 ++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 117 insertions(+), 66 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index fccee52..715d8d4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1915,107 +1915,158 @@ out_no_maps:
 	mutex_unlock(&xps_map_mutex);
 }
 
+static struct xps_map *expand_xps_map(struct xps_map *map,
+				      int cpu, u16 index)
+{
+	struct xps_map *new_map;
+	int alloc_len = XPS_MIN_MAP_ALLOC;
+	int i, pos;
+
+	for (pos = 0; map && pos < map->len; pos++) {
+		if (map->queues[pos] != index)
+			continue;
+		return map;
+	}
+
+	/* Need to add queue to this CPU's existing map */
+	if (map) {
+		if (pos < map->alloc_len)
+			return map;
+
+		alloc_len = map->alloc_len * 2;
+	}
+
+	/* Need to allocate new map to store queue on this CPU's map */
+	new_map = kzalloc_node(XPS_MAP_SIZE(alloc_len), GFP_KERNEL,
+			       cpu_to_node(cpu));
+	if (!new_map)
+		return NULL;
+
+	for (i = 0; i < pos; i++)
+		new_map->queues[i] = map->queues[i];
+	new_map->alloc_len = alloc_len;
+	new_map->len = pos;
+
+	return new_map;
+}
+
 int netif_set_xps_queue(struct net_device *dev, struct cpumask *mask, u16 index)
 {
-	int i, cpu, pos, map_len, alloc_len, need_set;
+	struct xps_dev_maps *dev_maps, *new_dev_maps = NULL;
 	struct xps_map *map, *new_map;
-	struct xps_dev_maps *dev_maps, *new_dev_maps;
-	int nonempty = 0;
-	int numa_node_id = -2;
 	int maps_sz = max_t(unsigned int, XPS_DEV_MAPS_SIZE, L1_CACHE_BYTES);
-
-	new_dev_maps = kzalloc(maps_sz, GFP_KERNEL);
-	if (!new_dev_maps)
-		return -ENOMEM;
+	int cpu, numa_node_id = -2;
+	bool active = false;
 
 	mutex_lock(&xps_map_mutex);
 
 	dev_maps = xmap_dereference(dev->xps_maps);
 
+	/* allocate memory for queue storage */
+	for_each_online_cpu(cpu) {
+		if (!cpumask_test_cpu(cpu, mask))
+			continue;
+
+		if (!new_dev_maps)
+			new_dev_maps = kzalloc(maps_sz, GFP_KERNEL);
+		if (!new_dev_maps)
+			return -ENOMEM;
+
+		map = dev_maps ? xmap_dereference(dev_maps->cpu_map[cpu]) :
+				 NULL;
+
+		map = expand_xps_map(map, cpu, index);
+		if (!map)
+			goto error;
+
+		RCU_INIT_POINTER(new_dev_maps->cpu_map[cpu], map);
+	}
+
+	if (!new_dev_maps)
+		goto out_no_new_maps;
+
 	for_each_possible_cpu(cpu) {
-		map = dev_maps ?
-			xmap_dereference(dev_maps->cpu_map[cpu]) : NULL;
-		new_map = map;
-		if (map) {
-			for (pos = 0; pos < map->len; pos++)
-				if (map->queues[pos] == index)
-					break;
-			map_len = map->len;
-			alloc_len = map->alloc_len;
-		} else
-			pos = map_len = alloc_len = 0;
+		if (cpumask_test_cpu(cpu, mask) && cpu_online(cpu)) {
+			/* add queue to CPU maps */
+			int pos = 0;
 
-		need_set = cpumask_test_cpu(cpu, mask) && cpu_online(cpu);
+			map = xmap_dereference(new_dev_maps->cpu_map[cpu]);
+			while ((pos < map->len) && (map->queues[pos] != index))
+				pos++;
+
+			if (pos == map->len)
+				map->queues[map->len++] = index;
 #ifdef CONFIG_NUMA
-		if (need_set) {
 			if (numa_node_id == -2)
 				numa_node_id = cpu_to_node(cpu);
 			else if (numa_node_id != cpu_to_node(cpu))
 				numa_node_id = -1;
-		}
 #endif
-		if (need_set && pos >= map_len) {
-			/* Need to add queue to this CPU's map */
-			if (map_len >= alloc_len) {
-				alloc_len = alloc_len ?
-				    2 * alloc_len : XPS_MIN_MAP_ALLOC;
-				new_map = kzalloc_node(XPS_MAP_SIZE(alloc_len),
-						       GFP_KERNEL,
-						       cpu_to_node(cpu));
-				if (!new_map)
-					goto error;
-				new_map->alloc_len = alloc_len;
-				for (i = 0; i < map_len; i++)
-					new_map->queues[i] = map->queues[i];
-				new_map->len = map_len;
-			}
-			new_map->queues[new_map->len++] = index;
-		} else if (!need_set && pos < map_len) {
-			/* Need to remove queue from this CPU's map */
-			if (map_len > 1)
-				new_map->queues[pos] =
-				    new_map->queues[--new_map->len];
-			else
-				new_map = NULL;
+		} else if (dev_maps) {
+			/* fill in the new device map from the old device map */
+			map = xmap_dereference(dev_maps->cpu_map[cpu]);
+			RCU_INIT_POINTER(new_dev_maps->cpu_map[cpu], map);
 		}
-		RCU_INIT_POINTER(new_dev_maps->cpu_map[cpu], new_map);
+
 	}
 
+	rcu_assign_pointer(dev->xps_maps, new_dev_maps);
+
 	/* Cleanup old maps */
-	for_each_possible_cpu(cpu) {
-		map = dev_maps ?
-			xmap_dereference(dev_maps->cpu_map[cpu]) : NULL;
-		if (map && xmap_dereference(new_dev_maps->cpu_map[cpu]) != map)
-			kfree_rcu(map, rcu);
-		if (new_dev_maps->cpu_map[cpu])
-			nonempty = 1;
-	}
+	if (dev_maps) {
+		for_each_possible_cpu(cpu) {
+			new_map = xmap_dereference(new_dev_maps->cpu_map[cpu]);
+			map = xmap_dereference(dev_maps->cpu_map[cpu]);
+			if (map && map != new_map)
+				kfree_rcu(map, rcu);
+		}
 
-	if (nonempty) {
-		rcu_assign_pointer(dev->xps_maps, new_dev_maps);
-	} else {
-		kfree(new_dev_maps);
-		RCU_INIT_POINTER(dev->xps_maps, NULL);
+		kfree_rcu(dev_maps, rcu);
 	}
 
-	if (dev_maps)
-		kfree_rcu(dev_maps, rcu);
+	dev_maps = new_dev_maps;
+	active = true;
 
+out_no_new_maps:
+	/* update Tx queue numa node */
 	netdev_queue_numa_node_write(netdev_get_tx_queue(dev, index),
 				     (numa_node_id >= 0) ? numa_node_id :
 				     NUMA_NO_NODE);
 
+	if (!dev_maps)
+		goto out_no_maps;
+
+	/* removes queue from unused CPUs */
+	for_each_possible_cpu(cpu) {
+		if (cpumask_test_cpu(cpu, mask) && cpu_online(cpu))
+			continue;
+
+		if (remove_xps_queue(dev_maps, cpu, index))
+			active = true;
+	}
+
+	/* free map if not active */
+	if (!active) {
+		RCU_INIT_POINTER(dev->xps_maps, NULL);
+		kfree_rcu(dev_maps, rcu);
+	}
+
+out_no_maps:
 	mutex_unlock(&xps_map_mutex);
 
 	return 0;
 error:
+	/* remove any maps that we added */
+	for_each_possible_cpu(cpu) {
+		new_map = xmap_dereference(new_dev_maps->cpu_map[cpu]);
+		map = dev_maps ? xmap_dereference(dev_maps->cpu_map[cpu]) :
+				 NULL;
+		if (new_map && new_map != map)
+			kfree(new_map);
+	}
+
 	mutex_unlock(&xps_map_mutex);
 
-	if (new_dev_maps)
-		for_each_possible_cpu(i)
-			kfree(rcu_dereference_protected(
-				new_dev_maps->cpu_map[i],
-				1));
 	kfree(new_dev_maps);
 	return -ENOMEM;
 }

^ permalink raw reply related

* [PATCH v2 05/10] net: Add support for XPS without sysfs being defined
From: Alexander Duyck @ 2013-01-10 18:57 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

This patch makes it so that we can support transmit packet steering without
sysfs needing to be enabled.  The reason for making this change is to make
it so that a driver can make use of the XPS even while the sysfs portion of
the interface is not present.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 include/linux/netdevice.h |    1 -
 net/Kconfig               |    2 +-
 net/core/dev.c            |   26 ++++++++++++++++++++------
 net/core/net-sysfs.c      |   14 --------------
 4 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 59fe9da..aa7ad8a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2104,7 +2104,6 @@ static inline void netif_wake_subqueue(struct net_device *dev, u16 queue_index)
 }
 
 #ifdef CONFIG_XPS
-extern void netif_reset_xps_queue(struct net_device *dev, u16 index);
 extern int netif_set_xps_queue(struct net_device *dev, struct cpumask *mask,
 			       u16 index);
 #else
diff --git a/net/Kconfig b/net/Kconfig
index 30b48f5..3cc5be0 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -232,7 +232,7 @@ config RFS_ACCEL
 
 config XPS
 	boolean
-	depends on SMP && SYSFS && USE_GENERIC_SMP_HELPERS
+	depends on SMP && USE_GENERIC_SMP_HELPERS
 	default y
 
 config NETPRIO_CGROUP
diff --git a/net/core/dev.c b/net/core/dev.c
index 715d8d4..095d22f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1887,10 +1887,10 @@ static struct xps_map *remove_xps_queue(struct xps_dev_maps *dev_maps,
 	return map;
 }
 
-void netif_reset_xps_queue(struct net_device *dev, u16 index)
+static void netif_reset_xps_queues_gt(struct net_device *dev, u16 index)
 {
 	struct xps_dev_maps *dev_maps;
-	int cpu;
+	int cpu, i;
 	bool active = false;
 
 	mutex_lock(&xps_map_mutex);
@@ -1900,7 +1900,11 @@ void netif_reset_xps_queue(struct net_device *dev, u16 index)
 		goto out_no_maps;
 
 	for_each_possible_cpu(cpu) {
-		if (remove_xps_queue(dev_maps, cpu, index))
+		for (i = index; i < dev->num_tx_queues; i++) {
+			if (!remove_xps_queue(dev_maps, cpu, i))
+				break;
+		}
+		if (i == dev->num_tx_queues)
 			active = true;
 	}
 
@@ -1909,8 +1913,10 @@ void netif_reset_xps_queue(struct net_device *dev, u16 index)
 		kfree_rcu(dev_maps, rcu);
 	}
 
-	netdev_queue_numa_node_write(netdev_get_tx_queue(dev, index),
-				     NUMA_NO_NODE);
+	for (i = index; i < dev->num_tx_queues; i++)
+		netdev_queue_numa_node_write(netdev_get_tx_queue(dev, i),
+					     NUMA_NO_NODE);
+
 out_no_maps:
 	mutex_unlock(&xps_map_mutex);
 }
@@ -2096,8 +2102,12 @@ int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
 		if (dev->num_tc)
 			netif_setup_tc(dev, txq);
 
-		if (txq < dev->real_num_tx_queues)
+		if (txq < dev->real_num_tx_queues) {
 			qdisc_reset_all_tx_gt(dev, txq);
+#ifdef CONFIG_XPS
+			netif_reset_xps_queues_gt(dev, txq);
+#endif
+		}
 	}
 
 	dev->real_num_tx_queues = txq;
@@ -5899,6 +5909,10 @@ static void rollback_registered_many(struct list_head *head)
 
 		/* Remove entries from kobject tree */
 		netdev_unregister_kobject(dev);
+#ifdef CONFIG_XPS
+		/* Remove XPS queueing entries */
+		netif_reset_xps_queues_gt(dev, 0);
+#endif
 	}
 
 	synchronize_net();
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 5ad489d..a5b89a6 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1002,16 +1002,6 @@ static ssize_t show_xps_map(struct netdev_queue *queue,
 	return len;
 }
 
-static void xps_queue_release(struct netdev_queue *queue)
-{
-	struct net_device *dev = queue->dev;
-	unsigned long index;
-
-	index = get_netdev_queue_index(queue);
-
-	netif_reset_xps_queue(dev, index);
-}
-
 static ssize_t store_xps_map(struct netdev_queue *queue,
 		      struct netdev_queue_attribute *attribute,
 		      const char *buf, size_t len)
@@ -1058,10 +1048,6 @@ static void netdev_queue_release(struct kobject *kobj)
 {
 	struct netdev_queue *queue = to_netdev_queue(kobj);
 
-#ifdef CONFIG_XPS
-	xps_queue_release(queue);
-#endif
-
 	memset(kobj, 0, sizeof(*kobj));
 	dev_put(queue->dev);
 }

^ permalink raw reply related

* [RFC PATCH v2 06/10] ixgbe: Define FCoE and Flow director limits much sooner to allow for changes
From: Alexander Duyck @ 2013-01-10 18:57 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

Instead of adjusting the FCoE and Flow director limits based on the number
of CPUs we can define them much sooner.  This allows the user to come
through later and adjust them once we have updated the code to support the
set_channels ethtool operation.

I am still allowing for FCoE and RSS queues to be separated if the number
queues is less than the number of CPUs.  This essentially treats the two
groupings like they are two separate traffic classes.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c  |    7 +------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   12 ++++++++----
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 8c74f73..4b1eeb6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -386,7 +386,6 @@ static bool ixgbe_set_dcb_sriov_queues(struct ixgbe_adapter *adapter)
 		fcoe = &adapter->ring_feature[RING_F_FCOE];
 
 		/* limit ourselves based on feature limits */
-		fcoe_i = min_t(u16, fcoe_i, num_online_cpus());
 		fcoe_i = min_t(u16, fcoe_i, fcoe->limit);
 
 		if (fcoe_i) {
@@ -562,9 +561,6 @@ static bool ixgbe_set_sriov_queues(struct ixgbe_adapter *adapter)
 		fcoe_i = min_t(u16, fcoe_i, fcoe->limit);
 
 		if (vmdq_i > 1 && fcoe_i) {
-			/* reserve no more than number of CPUs */
-			fcoe_i = min_t(u16, fcoe_i, num_online_cpus());
-
 			/* alloc queues for FCoE separately */
 			fcoe->indices = fcoe_i;
 			fcoe->offset = vmdq_i * rss_i;
@@ -623,8 +619,7 @@ static bool ixgbe_set_rss_queues(struct ixgbe_adapter *adapter)
 	if (rss_i > 1 && adapter->atr_sample_rate) {
 		f = &adapter->ring_feature[RING_F_FDIR];
 
-		f->indices = min_t(u16, num_online_cpus(), f->limit);
-		rss_i = max_t(u16, rss_i, f->indices);
+		rss_i = f->indices = f->limit;
 
 		if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE))
 			adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 20d6764..810f2b8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4466,7 +4466,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
 	struct pci_dev *pdev = adapter->pdev;
-	unsigned int rss;
+	unsigned int rss, fdir;
 	u32 fwsm;
 #ifdef CONFIG_IXGBE_DCB
 	int j;
@@ -4502,8 +4502,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 			adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
 		/* Flow Director hash filters enabled */
 		adapter->atr_sample_rate = 20;
-		adapter->ring_feature[RING_F_FDIR].limit =
-							 IXGBE_MAX_FDIR_INDICES;
+		fdir = min_t(int, IXGBE_MAX_FDIR_INDICES, num_online_cpus());
+		adapter->ring_feature[RING_F_FDIR].limit = fdir;
 		adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_64K;
 #ifdef IXGBE_FCOE
 		adapter->flags |= IXGBE_FLAG_FCOE_CAPABLE;
@@ -7410,13 +7410,17 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 #ifdef IXGBE_FCOE
 	if (adapter->flags & IXGBE_FLAG_FCOE_CAPABLE) {
+		unsigned int fcoe_l;
+
 		if (hw->mac.ops.get_device_caps) {
 			hw->mac.ops.get_device_caps(hw, &device_caps);
 			if (device_caps & IXGBE_DEVICE_CAPS_FCOE_OFFLOADS)
 				adapter->flags &= ~IXGBE_FLAG_FCOE_CAPABLE;
 		}
 
-		adapter->ring_feature[RING_F_FCOE].limit = IXGBE_FCRETA_SIZE;
+
+		fcoe_l = min_t(int, IXGBE_FCRETA_SIZE, num_online_cpus());
+		adapter->ring_feature[RING_F_FCOE].limit = fcoe_l;
 
 		netdev->features |= NETIF_F_FSO |
 				    NETIF_F_FCOE_CRC;

^ permalink raw reply related

* [RFC PATCH v2 07/10] ixgbe: Add function for setting XPS queue mapping
From: Alexander Duyck @ 2013-01-10 18:58 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110184115.29578.85768.stgit@ahduyck-cp1.jf.intel.com>

This change adds support for ixgbe to configure the XPS queue mapping on
load.  The result of this change is that on open we will now be resetting
the number of Tx queues, and then setting the default configuration for XPS
based on if ATR is enabled or disabled.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c  |   14 +++++++++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   10 ++++++++++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 8e78676..590f042 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -195,6 +195,7 @@ struct ixgbe_rx_queue_stats {
 
 enum ixgbe_ring_state_t {
 	__IXGBE_TX_FDIR_INIT_DONE,
+	__IXGBE_TX_XPS_INIT_DONE,
 	__IXGBE_TX_DETECT_HANG,
 	__IXGBE_HANG_CHECK_ARMED,
 	__IXGBE_RX_RSC_ENABLED,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 4b1eeb6..ab01662 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -771,19 +771,23 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
 {
 	struct ixgbe_q_vector *q_vector;
 	struct ixgbe_ring *ring;
-	int node = -1;
+	int node = NUMA_NO_NODE;
 	int cpu = -1;
 	int ring_count, size;
+	u8 tcs = netdev_get_num_tc(adapter->netdev);
 
 	ring_count = txr_count + rxr_count;
 	size = sizeof(struct ixgbe_q_vector) +
 	       (sizeof(struct ixgbe_ring) * ring_count);
 
 	/* customize cpu for Flow Director mapping */
-	if (adapter->flags & IXGBE_FLAG_FDIR_HASH_CAPABLE) {
-		if (cpu_online(v_idx)) {
-			cpu = v_idx;
-			node = cpu_to_node(cpu);
+	if ((tcs <= 1) && !(adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)) {
+		u16 rss_i = adapter->ring_feature[RING_F_RSS].indices;
+		if (rss_i > 1 && adapter->atr_sample_rate) {
+			if (cpu_online(v_idx)) {
+				cpu = v_idx;
+				node = cpu_to_node(cpu);
+			}
 		}
 	}
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 810f2b8..8ca9794 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2814,6 +2814,16 @@ void ixgbe_configure_tx_ring(struct ixgbe_adapter *adapter,
 		ring->atr_sample_rate = 0;
 	}
 
+	/* initialize XPS */
+	if (!test_and_set_bit(__IXGBE_TX_XPS_INIT_DONE, &ring->state)) {
+		struct ixgbe_q_vector *q_vector = ring->q_vector;
+
+		if (q_vector)
+			netif_set_xps_queue(adapter->netdev,
+					    &q_vector->affinity_mask,
+					    ring->queue_index);
+	}
+
 	clear_bit(__IXGBE_HANG_CHECK_ARMED, &ring->state);
 
 	/* enable queue */

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox