* 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
* [RFC PATCH v2 08/10] ixgbe: Update ixgbe driver to use __netdev_pick_tx in ixgbe_select_queue
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 updates the ixgbe driver to use __netdev_pick_tx instead of
the current logic it is using to select a queue. The main result of this
change is that ixgbe can now fully support XPS, and in the case of non-FCoE
enabled configs it means we don't need to have our own ndo_select_queue.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 48 ++++++++++++++-----------
1 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 8ca9794..4c0bcd8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6363,38 +6363,40 @@ static inline int ixgbe_maybe_stop_tx(struct ixgbe_ring *tx_ring, u16 size)
return __ixgbe_maybe_stop_tx(tx_ring, size);
}
+#ifdef IXGBE_FCOE
static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
{
- struct ixgbe_adapter *adapter = netdev_priv(dev);
- int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
- smp_processor_id();
-#ifdef IXGBE_FCOE
- __be16 protocol = vlan_get_protocol(skb);
+ struct ixgbe_adapter *adapter;
+ struct ixgbe_ring_feature *f;
+ int txq;
- if (((protocol == htons(ETH_P_FCOE)) ||
- (protocol == htons(ETH_P_FIP))) &&
- (adapter->flags & IXGBE_FLAG_FCOE_ENABLED)) {
- struct ixgbe_ring_feature *f;
+ /*
+ * only execute the code below if protocol is FCoE
+ * or FIP and we have FCoE enabled on the adapter
+ */
+ switch (vlan_get_protocol(skb)) {
+ case __constant_htons(ETH_P_FCOE):
+ case __constant_htons(ETH_P_FIP):
+ adapter = netdev_priv(dev);
- f = &adapter->ring_feature[RING_F_FCOE];
+ if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED)
+ break;
+ default:
+ return __netdev_pick_tx(dev, skb);
+ }
- while (txq >= f->indices)
- txq -= f->indices;
- txq += adapter->ring_feature[RING_F_FCOE].offset;
+ f = &adapter->ring_feature[RING_F_FCOE];
- return txq;
- }
-#endif
+ txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
+ smp_processor_id();
- if (adapter->flags & IXGBE_FLAG_FDIR_HASH_CAPABLE) {
- while (unlikely(txq >= dev->real_num_tx_queues))
- txq -= dev->real_num_tx_queues;
- return txq;
- }
+ while (txq >= f->indices)
+ txq -= f->indices;
- return skb_tx_hash(dev, skb);
+ return txq + f->offset;
}
+#endif
netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
struct ixgbe_adapter *adapter,
struct ixgbe_ring *tx_ring)
@@ -7092,7 +7094,9 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_open = ixgbe_open,
.ndo_stop = ixgbe_close,
.ndo_start_xmit = ixgbe_xmit_frame,
+#ifdef IXGBE_FCOE
.ndo_select_queue = ixgbe_select_queue,
+#endif
.ndo_set_rx_mode = ixgbe_set_rx_mode,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = ixgbe_set_mac,
^ permalink raw reply related
* [RFC PATCH v2 09/10] ixgbe: Add support for displaying the number of Tx/Rx channels
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 patch adds support for the ethtool get_channels operation.
Since the ixgbe driver has to support DCB as well as the other modes the
assumption I made here is that the number of channels in DCB modes refers
to the number of queues per traffic class, not the number of queues total.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 72 ++++++++++++++++++++++
1 files changed, 72 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 3268584..688effc 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2704,6 +2704,77 @@ static int ixgbe_get_ts_info(struct net_device *dev,
return 0;
}
+static unsigned int ixgbe_max_channels(struct ixgbe_adapter *adapter)
+{
+ unsigned int max_combined;
+ u8 tcs = netdev_get_num_tc(adapter->netdev);
+
+ if (!(adapter->flags & IXGBE_FLAG_MSIX_ENABLED)) {
+ /* We only support one q_vector without MSI-X */
+ max_combined = 1;
+ } else if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED) {
+ /* SR-IOV currently only allows one queue on the PF */
+ max_combined = 1;
+ } else if (tcs > 1) {
+ /* For DCB report channels per traffic class */
+ if (adapter->hw.mac.type == ixgbe_mac_82598EB) {
+ /* 8 TC w/ 4 queues per TC */
+ max_combined = 4;
+ } else if (tcs > 4) {
+ /* 8 TC w/ 8 queues per TC */
+ max_combined = 8;
+ } else {
+ /* 4 TC w/ 16 queues per TC */
+ max_combined = 16;
+ }
+ } else if (adapter->atr_sample_rate) {
+ /* support up to 64 queues with ATR */
+ max_combined = IXGBE_MAX_FDIR_INDICES;
+ } else {
+ /* support up to 16 queues with RSS */
+ max_combined = IXGBE_MAX_RSS_INDICES;
+ }
+
+ return max_combined;
+}
+
+static void ixgbe_get_channels(struct net_device *dev,
+ struct ethtool_channels *ch)
+{
+ struct ixgbe_adapter *adapter = netdev_priv(dev);
+
+ /* report maximum channels */
+ ch->max_combined = ixgbe_max_channels(adapter);
+
+ /* report info for other vector */
+ if (adapter->flags & IXGBE_FLAG_MSIX_ENABLED) {
+ ch->max_other = NON_Q_VECTORS;
+ ch->other_count = NON_Q_VECTORS;
+ }
+
+ /* record RSS queues */
+ ch->combined_count = adapter->ring_feature[RING_F_RSS].indices;
+
+ /* nothing else to report if RSS is disabled */
+ if (ch->combined_count == 1)
+ return;
+
+ /* we do not support ATR queueing if SR-IOV is enabled */
+ if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
+ return;
+
+ /* same thing goes for being DCB enabled */
+ if (netdev_get_num_tc(dev) > 1)
+ return;
+
+ /* if ATR is disabled we can exit */
+ if (!adapter->atr_sample_rate)
+ return;
+
+ /* report flow director queues as maximum channels */
+ ch->combined_count = adapter->ring_feature[RING_F_FDIR].indices;
+}
+
static const struct ethtool_ops ixgbe_ethtool_ops = {
.get_settings = ixgbe_get_settings,
.set_settings = ixgbe_set_settings,
@@ -2733,6 +2804,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = {
.get_rxnfc = ixgbe_get_rxnfc,
.set_rxnfc = ixgbe_set_rxnfc,
.get_ts_info = ixgbe_get_ts_info,
+ .get_channels = ixgbe_get_channels,
};
void ixgbe_set_ethtool_ops(struct net_device *netdev)
^ permalink raw reply related
* [RFC PATCH v2 10/10] ixgbe: Add support for set_channels ethtool operation
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 the ethtool set_channels operation.
Since the ixgbe driver has to support DCB as well as the other modes the
assumption I made here is that the number of channels in DCB modes refers
to the number of queues per traffic class, not the number of queues total.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 +
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 40 ++++++++++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 +++-
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 590f042..a431628 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -695,8 +695,8 @@ extern bool ixgbe_verify_lesm_fw_enabled_82599(struct ixgbe_hw *hw);
extern void ixgbe_set_rx_mode(struct net_device *netdev);
#ifdef CONFIG_IXGBE_DCB
extern void ixgbe_set_rx_drop_en(struct ixgbe_adapter *adapter);
-extern int ixgbe_setup_tc(struct net_device *dev, u8 tc);
#endif
+extern int ixgbe_setup_tc(struct net_device *dev, u8 tc);
extern void ixgbe_tx_ctxtdesc(struct ixgbe_ring *, u32, u32, u32, u32);
extern void ixgbe_do_reset(struct net_device *netdev);
#ifdef CONFIG_IXGBE_HWMON
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 688effc..31998a3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2775,6 +2775,45 @@ static void ixgbe_get_channels(struct net_device *dev,
ch->combined_count = adapter->ring_feature[RING_F_FDIR].indices;
}
+static int ixgbe_set_channels(struct net_device *dev,
+ struct ethtool_channels *ch)
+{
+ struct ixgbe_adapter *adapter = netdev_priv(dev);
+ unsigned int count = ch->combined_count;
+
+ /* verify they are not requesting separate vectors */
+ if (ch->rx_count || ch->tx_count)
+ return -EINVAL;
+
+ /* ignore other_count since it is not changeable */
+
+ /* verify we have at least one channel requested */
+ if (!count)
+ return -EINVAL;
+
+ /* verify the number of channels does not exceed hardware limits */
+ if (count > ixgbe_max_channels(adapter))
+ return -EINVAL;
+
+ /* update feature limits from largest to smallest supported values */
+ adapter->ring_feature[RING_F_FDIR].limit = count;
+
+ /* cap RSS limit at 16 */
+ if (count > IXGBE_MAX_RSS_INDICES)
+ count = IXGBE_MAX_RSS_INDICES;
+ adapter->ring_feature[RING_F_RSS].limit = count;
+
+#ifdef IXGBE_FCOE
+ /* cap FCoE limit at 8 */
+ if (count > IXGBE_FCRETA_SIZE)
+ count = IXGBE_FCRETA_SIZE;
+ adapter->ring_feature[RING_F_FCOE].limit = count;
+
+#endif
+ /* use setup TC to update any traffic class queue mapping */
+ return ixgbe_setup_tc(dev, netdev_get_num_tc(dev));
+}
+
static const struct ethtool_ops ixgbe_ethtool_ops = {
.get_settings = ixgbe_get_settings,
.set_settings = ixgbe_set_settings,
@@ -2805,6 +2844,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = {
.set_rxnfc = ixgbe_set_rxnfc,
.get_ts_info = ixgbe_get_ts_info,
.get_channels = ixgbe_get_channels,
+ .set_channels = ixgbe_set_channels,
};
void ixgbe_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 4c0bcd8..798bbf6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6796,6 +6796,7 @@ static void ixgbe_set_prio_tc_map(struct ixgbe_adapter *adapter)
}
}
+#endif /* CONFIG_IXGBE_DCB */
/**
* ixgbe_setup_tc - configure net_device for multiple traffic classes
*
@@ -6821,6 +6822,7 @@ int ixgbe_setup_tc(struct net_device *dev, u8 tc)
ixgbe_close(dev);
ixgbe_clear_interrupt_scheme(adapter);
+#ifdef CONFIG_IXGBE_DCB
if (tc) {
netdev_set_num_tc(dev, tc);
ixgbe_set_prio_tc_map(adapter);
@@ -6843,15 +6845,16 @@ int ixgbe_setup_tc(struct net_device *dev, u8 tc)
adapter->dcb_cfg.pfc_mode_enable = false;
}
- ixgbe_init_interrupt_scheme(adapter);
ixgbe_validate_rtr(adapter, tc);
+
+#endif /* CONFIG_IXGBE_DCB */
+ ixgbe_init_interrupt_scheme(adapter);
if (netif_running(dev))
ixgbe_open(dev);
return 0;
}
-#endif /* CONFIG_IXGBE_DCB */
void ixgbe_do_reset(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
^ permalink raw reply related
* Re: [PATCH net-next v5 02/14] bridge: Add vlan filtering infrastructure
From: Vlad Yasevich @ 2013-01-10 19:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, davem, stephen, bridge, shmulik.ladkani, mst
In-Reply-To: <20130110103614.23383079@nehalam.linuxnetplumber.net>
On 01/10/2013 01:36 PM, Stephen Hemminger wrote:
> 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. */
> ^^^^^^^^^^
>
sorry, will fix. I thought I ran everything through checkpatch. I'll
re-run them.
> 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.
>
I originally had it this way. I found that it wasted space and made
other things more difficult to do. For instance, to do egress policy, I
would have needed another bitmap of vlans... With 4096 vlans, that's
512 bytes per port (1024 with policy).
I could probably remove the per-port list. It would make 2 things a
little harder:
1) detecting if the port has any vlan info at all.
2) dumping the vlan information
I could pursue this if you think it'll be better.
-vlad
^ permalink raw reply
* Re: [PATCH V3 1/2] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Ben Hutchings @ 2013-01-10 19:12 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <87a9sh3lru.fsf@rustcorp.com.au>
On Thu, 2013-01-10 at 11:19 +1030, Rusty Russell wrote:
> Wanlong Gao <gaowanlong@cn.fujitsu.com> writes:
> > On 01/09/2013 07:31 AM, Rusty Russell wrote:
> >> Wanlong Gao <gaowanlong@cn.fujitsu.com> writes:
> >>> */
> >>> static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
> >>> {
> >>> - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
> >>> - smp_processor_id();
> >>> + int txq = 0;
> >>> +
> >>> + if (skb_rx_queue_recorded(skb))
> >>> + txq = skb_get_rx_queue(skb);
> >>> + else if ((txq = per_cpu(vq_index, smp_processor_id())) == -1)
> >>> + txq = 0;
> >>
> >> You should use __get_cpu_var() instead of smp_processor_id() here, ie:
> >>
> >> else if ((txq = __get_cpu_var(vq_index)) == -1)
> >>
> >> And AFAICT, no reason to initialize txq to 0 to start with.
> >>
> >> So:
> >>
> >> int txq;
> >>
> >> if (skb_rx_queue_recorded(skb))
> >> txq = skb_get_rx_queue(skb);
> >> else {
> >> txq = __get_cpu_var(vq_index);
> >> if (txq == -1)
> >> txq = 0;
> >> }
> >
> > Got it, thank you.
> >
> >>
> >> Now, just to confirm, I assume this can happen even if we use vq_index,
> >> right, because of races with virtnet_set_channels?
> >
> > I still can't understand this race, could you explain more? thank you.
>
> I assume that someone can call virtnet_set_channels() while we are
> inside virtnet_select_queue(), so they reduce dev->real_num_tx_queues,
> causing virtnet_set_channels to do:
>
> while (unlikely(txq >= dev->real_num_tx_queues))
> txq -= dev->real_num_tx_queues;
>
> Otherwise, when is this loop called?
In fact, this race can result in the TX scheduler using a queue that has
been disabled, or other weirdness (consider what happens if
real_num_tx_queues increases between those two uses).
virtnet_set_channels() really must disable TX temporarily:
netif_tx_lock(dev);
netif_device_detach(dev);
netif_tx_unlock(dev);
...
netif_device_attach(dev);
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
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
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