* [PATCH net-next] net: dsa: Add support for 64-bit statistics
@ 2017-08-01 22:00 Florian Fainelli
2017-08-01 22:42 ` Florian Fainelli
2017-08-02 23:49 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-08-01 22:00 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli, Andrew Lunn, Vivien Didelot, open list
DSA slave network devices maintain a pair of bytes and packets counters
for each directions, but these are not 64-bit capable. Re-use
pcpu_sw_netstats which contains exactly what we need for that purpose
and update the code path to report 64-bit capable statistics.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 8 ++++++--
net/dsa/dsa_priv.h | 2 ++
net/dsa/slave.c | 38 +++++++++++++++++++++++++++++++-------
3 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index a55e2e4087a4..0ba842c08dd3 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -190,6 +190,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct sk_buff *nskb = NULL;
+ struct dsa_slave_priv *p;
if (unlikely(dst == NULL)) {
kfree_skb(skb);
@@ -207,12 +208,15 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
}
skb = nskb;
+ p = netdev_priv(skb->dev);
skb_push(skb, ETH_HLEN);
skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, skb->dev);
- skb->dev->stats.rx_packets++;
- skb->dev->stats.rx_bytes += skb->len;
+ u64_stats_update_begin(&p->stats64.syncp);
+ p->stats64.rx_packets++;
+ p->stats64.rx_bytes += skb->len;
+ u64_stats_update_end(&p->stats64.syncp);
netif_receive_skb(skb);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 55982cc39b24..7aa0656296c2 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -77,6 +77,8 @@ struct dsa_slave_priv {
struct sk_buff * (*xmit)(struct sk_buff *skb,
struct net_device *dev);
+ struct pcpu_sw_netstats stats64;
+
/* DSA port data, such as switch, port index, etc. */
struct dsa_port *dp;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9507bd38cf04..65f3cef85976 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -354,8 +354,10 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
struct dsa_slave_priv *p = netdev_priv(dev);
struct sk_buff *nskb;
- dev->stats.tx_packets++;
- dev->stats.tx_bytes += skb->len;
+ u64_stats_update_begin(&p->stats64.syncp);
+ p->stats64.tx_packets++;
+ p->stats64.tx_bytes += skb->len;
+ u64_stats_update_end(&p->stats64.syncp);
/* Transmit function may have to reallocate the original SKB,
* in which case it must have freed it. Only free it here on error.
@@ -594,11 +596,15 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->dp->ds;
-
- data[0] = dev->stats.tx_packets;
- data[1] = dev->stats.tx_bytes;
- data[2] = dev->stats.rx_packets;
- data[3] = dev->stats.rx_bytes;
+ unsigned int start;
+
+ do {
+ start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
+ data[0] = p->stats64.tx_packets;
+ data[1] = p->stats64.tx_bytes;
+ data[2] = p->stats64.rx_packets;
+ data[3] = p->stats64.rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
if (ds->ops->get_ethtool_stats)
ds->ops->get_ethtool_stats(ds, p->dp->index, data + 4);
}
@@ -861,6 +867,22 @@ static int dsa_slave_setup_tc(struct net_device *dev, u32 handle,
}
}
+static void dsa_slave_get_stats64(struct net_device *dev,
+ struct rtnl_link_stats64 *stats)
+{
+ struct dsa_slave_priv *p = netdev_priv(dev);
+ unsigned int start;
+
+ netdev_stats_to_stats64(stats, &dev->stats);
+ do {
+ start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
+ stats->tx_packets = p->stats64.tx_packets;
+ stats->tx_bytes = p->stats64.tx_bytes;
+ stats->rx_packets = p->stats64.rx_packets;
+ stats->rx_bytes = p->stats64.rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
+}
+
void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
{
ops->get_sset_count = dsa_cpu_port_get_sset_count;
@@ -936,6 +958,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_bridge_dellink = switchdev_port_bridge_dellink,
.ndo_get_phys_port_name = dsa_slave_get_phys_port_name,
.ndo_setup_tc = dsa_slave_setup_tc,
+ .ndo_get_stats64 = dsa_slave_get_stats64,
};
static const struct switchdev_ops dsa_slave_switchdev_ops = {
@@ -1171,6 +1194,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
slave_dev->vlan_features = master->vlan_features;
p = netdev_priv(slave_dev);
+ u64_stats_init(&p->stats64.syncp);
p->dp = &ds->ports[port];
INIT_LIST_HEAD(&p->mall_tc_list);
p->xmit = dst->tag_ops->xmit;
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
2017-08-01 22:00 [PATCH net-next] net: dsa: Add support for 64-bit statistics Florian Fainelli
@ 2017-08-01 22:42 ` Florian Fainelli
2017-08-02 23:49 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-08-01 22:42 UTC (permalink / raw)
To: netdev; +Cc: davem, Andrew Lunn, Vivien Didelot, open list
On 08/01/2017 03:00 PM, Florian Fainelli wrote:
> DSA slave network devices maintain a pair of bytes and packets counters
> for each directions, but these are not 64-bit capable. Re-use
> pcpu_sw_netstats which contains exactly what we need for that purpose
> and update the code path to report 64-bit capable statistics.
Humm, I do see a long time for ifconfig/ethtool -S to complete after
having transfered over 250GiB of data, the locking looks correct to me
in that statistics for the RX path are updated from softIRQ context
(napi_gro_receive/netif_receive_skb) but maybe I did misunderstand
whether the softirq/IRQ context applies to the producer and not the
reader...
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/dsa/dsa.c | 8 ++++++--
> net/dsa/dsa_priv.h | 2 ++
> net/dsa/slave.c | 38 +++++++++++++++++++++++++++++++-------
> 3 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index a55e2e4087a4..0ba842c08dd3 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -190,6 +190,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
> {
> struct dsa_switch_tree *dst = dev->dsa_ptr;
> struct sk_buff *nskb = NULL;
> + struct dsa_slave_priv *p;
>
> if (unlikely(dst == NULL)) {
> kfree_skb(skb);
> @@ -207,12 +208,15 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
> }
>
> skb = nskb;
> + p = netdev_priv(skb->dev);
> skb_push(skb, ETH_HLEN);
> skb->pkt_type = PACKET_HOST;
> skb->protocol = eth_type_trans(skb, skb->dev);
>
> - skb->dev->stats.rx_packets++;
> - skb->dev->stats.rx_bytes += skb->len;
> + u64_stats_update_begin(&p->stats64.syncp);
> + p->stats64.rx_packets++;
> + p->stats64.rx_bytes += skb->len;
> + u64_stats_update_end(&p->stats64.syncp);
>
> netif_receive_skb(skb);
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 55982cc39b24..7aa0656296c2 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -77,6 +77,8 @@ struct dsa_slave_priv {
> struct sk_buff * (*xmit)(struct sk_buff *skb,
> struct net_device *dev);
>
> + struct pcpu_sw_netstats stats64;
> +
> /* DSA port data, such as switch, port index, etc. */
> struct dsa_port *dp;
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 9507bd38cf04..65f3cef85976 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -354,8 +354,10 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct sk_buff *nskb;
>
> - dev->stats.tx_packets++;
> - dev->stats.tx_bytes += skb->len;
> + u64_stats_update_begin(&p->stats64.syncp);
> + p->stats64.tx_packets++;
> + p->stats64.tx_bytes += skb->len;
> + u64_stats_update_end(&p->stats64.syncp);
>
> /* Transmit function may have to reallocate the original SKB,
> * in which case it must have freed it. Only free it here on error.
> @@ -594,11 +596,15 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct dsa_switch *ds = p->dp->ds;
> -
> - data[0] = dev->stats.tx_packets;
> - data[1] = dev->stats.tx_bytes;
> - data[2] = dev->stats.rx_packets;
> - data[3] = dev->stats.rx_bytes;
> + unsigned int start;
> +
> + do {
> + start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
> + data[0] = p->stats64.tx_packets;
> + data[1] = p->stats64.tx_bytes;
> + data[2] = p->stats64.rx_packets;
> + data[3] = p->stats64.rx_bytes;
> + } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
> if (ds->ops->get_ethtool_stats)
> ds->ops->get_ethtool_stats(ds, p->dp->index, data + 4);
> }
> @@ -861,6 +867,22 @@ static int dsa_slave_setup_tc(struct net_device *dev, u32 handle,
> }
> }
>
> +static void dsa_slave_get_stats64(struct net_device *dev,
> + struct rtnl_link_stats64 *stats)
> +{
> + struct dsa_slave_priv *p = netdev_priv(dev);
> + unsigned int start;
> +
> + netdev_stats_to_stats64(stats, &dev->stats);
> + do {
> + start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
> + stats->tx_packets = p->stats64.tx_packets;
> + stats->tx_bytes = p->stats64.tx_bytes;
> + stats->rx_packets = p->stats64.rx_packets;
> + stats->rx_bytes = p->stats64.rx_bytes;
> + } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
> +}
> +
> void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
> {
> ops->get_sset_count = dsa_cpu_port_get_sset_count;
> @@ -936,6 +958,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
> .ndo_bridge_dellink = switchdev_port_bridge_dellink,
> .ndo_get_phys_port_name = dsa_slave_get_phys_port_name,
> .ndo_setup_tc = dsa_slave_setup_tc,
> + .ndo_get_stats64 = dsa_slave_get_stats64,
> };
>
> static const struct switchdev_ops dsa_slave_switchdev_ops = {
> @@ -1171,6 +1194,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
> slave_dev->vlan_features = master->vlan_features;
>
> p = netdev_priv(slave_dev);
> + u64_stats_init(&p->stats64.syncp);
> p->dp = &ds->ports[port];
> INIT_LIST_HEAD(&p->mall_tc_list);
> p->xmit = dst->tag_ops->xmit;
>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
2017-08-01 22:00 [PATCH net-next] net: dsa: Add support for 64-bit statistics Florian Fainelli
2017-08-01 22:42 ` Florian Fainelli
@ 2017-08-02 23:49 ` David Miller
2017-08-03 17:30 ` Florian Fainelli
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2017-08-02 23:49 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, linux-kernel
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 1 Aug 2017 15:00:36 -0700
> DSA slave network devices maintain a pair of bytes and packets counters
> for each directions, but these are not 64-bit capable. Re-use
> pcpu_sw_netstats which contains exactly what we need for that purpose
> and update the code path to report 64-bit capable statistics.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks.
I would run ethtool -S and ifconfig under perf to see where it is
spending so much time.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
2017-08-02 23:49 ` David Miller
@ 2017-08-03 17:30 ` Florian Fainelli
2017-08-03 18:11 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2017-08-03 17:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev, andrew, vivien.didelot, linux-kernel
On 08/02/2017 04:49 PM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Tue, 1 Aug 2017 15:00:36 -0700
>
>> DSA slave network devices maintain a pair of bytes and packets counters
>> for each directions, but these are not 64-bit capable. Re-use
>> pcpu_sw_netstats which contains exactly what we need for that purpose
>> and update the code path to report 64-bit capable statistics.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Applied, thanks.
>
> I would run ethtool -S and ifconfig under perf to see where it is
> spending so much time.
>
This appears to be way worse than I thought, will keep digging, but for
now, I may have to send a revert. Andrew, Vivien can you see if you have
the same problems on your boards? Thanks!
# killall iperf
# [ ID] Interval Transfer Bandwidth
[ 3] 0.0-19.1 sec 500 MBytes 220 Mbits/sec
# while true; do ethtool -S gphy; ifconfig gphy; done
^C^C
[ 64.566226] INFO: rcu_sched self-detected stall on CPU
[ 64.571487] 0-...: (25999 ticks this GP) idle=006/140000000000001/0
softirq=965/965 fqs=6495
[ 64.580214] (t=26000 jiffies g=205 c=204 q=51)
[ 64.584958] NMI backtrace for cpu 0
[ 64.588571] CPU: 0 PID: 1515 Comm: ethtool Not tainted
4.13.0-rc3-00534-g5a4d148f0d78 #328
[ 64.596951] Hardware name: Broadcom STB (Flattened Device Tree)
[ 64.602973] [<c0211fe4>] (unwind_backtrace) from [<c020cc4c>]
(show_stack+0x10/0x14)
[ 64.610836] [<c020cc4c>] (show_stack) from [<c0a42bac>]
(dump_stack+0xb0/0xdc)
[ 64.618172] [<c0a42bac>] (dump_stack) from [<c0a48f48>]
(nmi_cpu_backtrace+0x11c/0x120)
[ 64.626295] [<c0a48f48>] (nmi_cpu_backtrace) from [<c0a49064>]
(nmi_trigger_cpumask_backtrace+0x118/0x158)
[ 64.636087] [<c0a49064>] (nmi_trigger_cpumask_backtrace) from
[<c02a8730>] (rcu_dump_cpu_stacks+0xa0/0xd4)
[ 64.645875] [<c02a8730>] (rcu_dump_cpu_stacks) from [<c02a7cdc>]
(rcu_check_callbacks+0xb8c/0xbfc)
[ 64.654965] [<c02a7cdc>] (rcu_check_callbacks) from [<c02add00>]
(update_process_times+0x30/0x5c)
[ 64.663966] [<c02add00>] (update_process_times) from [<c02c1ce0>]
(tick_sched_timer+0x40/0x90)
[ 64.672702] [<c02c1ce0>] (tick_sched_timer) from [<c02aeb58>]
(__hrtimer_run_queues+0x198/0x6f0)
[ 64.681615] [<c02aeb58>] (__hrtimer_run_queues) from [<c02afa90>]
(hrtimer_interrupt+0x98/0x1f4)
[ 64.690530] [<c02afa90>] (hrtimer_interrupt) from [<c0875760>]
(arch_timer_handler_virt+0x28/0x30)
[ 64.699628] [<c0875760>] (arch_timer_handler_virt) from [<c0292328>]
(handle_percpu_devid_irq+0xc8/0x484)
[ 64.709329] [<c0292328>] (handle_percpu_devid_irq) from [<c028cebc>]
(generic_handle_irq+0x24/0x34)
[ 64.718502] [<c028cebc>] (generic_handle_irq) from [<c028d418>]
(__handle_domain_irq+0x5c/0xb0)
[ 64.727325] [<c028d418>] (__handle_domain_irq) from [<c02014e8>]
(gic_handle_irq+0x48/0x8c)
[ 64.735794] [<c02014e8>] (gic_handle_irq) from [<c020d97c>]
(__irq_svc+0x5c/0x7c)
[ 64.743384] Exception stack(0xc283fd40 to 0xc283fd88)
[ 64.748510] fd40: 00000001 0011a9ad 00000000 edf37000 ecddb800
f0d95000 ecddbe6c c08e87f4
[ 64.756801] fd60: ed6b8010 00000660 00000658 600f0013 00000001
c283fd90 c027a39c c09a8c24
[ 64.765091] fd80: 200f0013 ffffffff
[ 64.768647] [<c020d97c>] (__irq_svc) from [<c09a8c24>]
(dsa_slave_get_ethtool_stats+0x100/0x104)
[ 64.777562] [<c09a8c24>] (dsa_slave_get_ethtool_stats) from
[<c08e87f4>] (dev_ethtool+0x768/0x2840)
[ 64.786742] [<c08e87f4>] (dev_ethtool) from [<c09029ec>]
(dev_ioctl+0x5f8/0xa50)
[ 64.794251] [<c09029ec>] (dev_ioctl) from [<c038ffb4>]
(do_vfs_ioctl+0xac/0x8d0)
[ 64.801755] [<c038ffb4>] (do_vfs_ioctl) from [<c039080c>]
(SyS_ioctl+0x34/0x5c)
[ 64.809175] [<c039080c>] (SyS_ioctl) from [<c02085a0>]
(ret_fast_syscall+0x0/0x1c)
[ 64.816901] INFO: rcu_sched detected stalls on CPUs/tasks:
[ 64.822480] 0-...: (26006 ticks this GP) idle=006/140000000000000/0
softirq=965/965 fqs=6495
[ 64.831206] (detected by 2, t=26264 jiffies, g=205, c=204, q=51)
[ 64.837390] Sending NMI from CPU 2 to CPUs 0:
[ 64.841811] NMI backtrace for cpu 0
[ 64.841818] CPU: 0 PID: 1515 Comm: ethtool Not tainted
4.13.0-rc3-00534-g5a4d148f0d78 #328
[ 64.841821] Hardware name: Broadcom STB (Flattened Device Tree)
[ 64.841824] task: edf37000 task.stack: c283e000
[ 64.841832] PC is at dsa_slave_get_ethtool_stats+0x100/0x104
[ 64.841842] LR is at mark_held_locks+0x68/0x90
[ 64.841846] pc : [<c09a8c24>] lr : [<c027a39c>] psr: 200f0013
[ 64.841850] sp : c283fd90 ip : 00000001 fp : 600f0013
[ 64.841853] r10: 00000658 r9 : 00000660 r8 : ed6b8010
[ 64.841856] r7 : c08e87f4 r6 : ecddbe6c r5 : f0d95000 r4 : ecddb800
[ 64.841860] r3 : edf37000 r2 : 00000000 r1 : 0011a9ad r0 : 00000001
[ 64.841865] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment user
[ 64.841869] Control: 30c5387d Table: 2dddf500 DAC: fffffffd
[ 64.841874] CPU: 0 PID: 1515 Comm: ethtool Not tainted
4.13.0-rc3-00534-g5a4d148f0d78 #328
[ 64.841876] Hardware name: Broadcom STB (Flattened Device Tree)
[ 64.841885] [<c0211fe4>] (unwind_backtrace) from [<c020cc4c>]
(show_stack+0x10/0x14)
[ 64.841892] [<c020cc4c>] (show_stack) from [<c0a42bac>]
(dump_stack+0xb0/0xdc)
[ 64.841900] [<c0a42bac>] (dump_stack) from [<c0a48eec>]
(nmi_cpu_backtrace+0xc0/0x120)
[ 64.841907] [<c0a48eec>] (nmi_cpu_backtrace) from [<c0210b74>]
(handle_IPI+0x108/0x424)
[ 64.841914] [<c0210b74>] (handle_IPI) from [<c0201528>]
(gic_handle_irq+0x88/0x8c)
[ 64.841919] [<c0201528>] (gic_handle_irq) from [<c020d97c>]
(__irq_svc+0x5c/0x7c)
[ 64.841922] Exception stack(0xc283fd40 to 0xc283fd88)
[ 64.841928] fd40: 00000001 0011a9ad 00000000 edf37000 ecddb800
f0d95000 ecddbe6c c08e87f4
[ 64.841932] fd60: ed6b8010 00000660 00000658 600f0013 00000001
c283fd90 c027a39c c09a8c24
[ 64.841935] fd80: 200f0013 ffffffff
[ 64.841942] [<c020d97c>] (__irq_svc) from [<c09a8c24>]
(dsa_slave_get_ethtool_stats+0x100/0x104)
[ 64.841950] [<c09a8c24>] (dsa_slave_get_ethtool_stats) from
[<c08e87f4>] (dev_ethtool+0x768/0x2840)
[ 64.841958] [<c08e87f4>] (dev_ethtool) from [<c09029ec>]
(dev_ioctl+0x5f8/0xa50)
[ 64.841965] [<c09029ec>] (dev_ioctl) from [<c038ffb4>]
(do_vfs_ioctl+0xac/0x8d0)
[ 64.841972] [<c038ffb4>] (do_vfs_ioctl) from [<c039080c>]
(SyS_ioctl+0x34/0x5c)
[ 64.841979] [<c039080c>] (SyS_ioctl) from [<c02085a0>]
(ret_fast_syscall+0x0/0x1c)
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
2017-08-03 17:30 ` Florian Fainelli
@ 2017-08-03 18:11 ` Andrew Lunn
2017-08-03 19:43 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2017-08-03 18:11 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, netdev, vivien.didelot, linux-kernel
On Thu, Aug 03, 2017 at 10:30:56AM -0700, Florian Fainelli wrote:
> On 08/02/2017 04:49 PM, David Miller wrote:
> > From: Florian Fainelli <f.fainelli@gmail.com>
> > Date: Tue, 1 Aug 2017 15:00:36 -0700
> >
> >> DSA slave network devices maintain a pair of bytes and packets counters
> >> for each directions, but these are not 64-bit capable. Re-use
> >> pcpu_sw_netstats which contains exactly what we need for that purpose
> >> and update the code path to report 64-bit capable statistics.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >
> > Applied, thanks.
> >
> > I would run ethtool -S and ifconfig under perf to see where it is
> > spending so much time.
> >
>
> This appears to be way worse than I thought, will keep digging, but for
> now, I may have to send a revert. Andrew, Vivien can you see if you have
> the same problems on your boards? Thanks!
>
> # killall iperf
> # [ ID] Interval Transfer Bandwidth
> [ 3] 0.0-19.1 sec 500 MBytes 220 Mbits/sec
> # while true; do ethtool -S gphy; ifconfig gphy; done
> ^C^C
>
>
> [ 64.566226] INFO: rcu_sched self-detected stall on CPU
> [ 64.571487] 0-...: (25999 ticks this GP) idle=006/140000000000001/0
Hi Florian
I don't get anything so bad, but i think that is because of hardware
restrictions. I see the ethtool; ifconfig loop goes a lot slower when
there is iperf traffic, but i don't get an RCU stall. However, the
board i tested on only has a 100Mbps CPU interface, and it can handle
all that traffic without pushing the CPU to 100%. What is the CPU load
when you run your test? Even if you are going to 100% CPU load, we
still don't want RCU stalls.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
2017-08-03 18:11 ` Andrew Lunn
@ 2017-08-03 19:43 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-08-03 19:43 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, vivien.didelot, linux-kernel
On 08/03/2017 11:11 AM, Andrew Lunn wrote:
> On Thu, Aug 03, 2017 at 10:30:56AM -0700, Florian Fainelli wrote:
>> On 08/02/2017 04:49 PM, David Miller wrote:
>>> From: Florian Fainelli <f.fainelli@gmail.com>
>>> Date: Tue, 1 Aug 2017 15:00:36 -0700
>>>
>>>> DSA slave network devices maintain a pair of bytes and packets counters
>>>> for each directions, but these are not 64-bit capable. Re-use
>>>> pcpu_sw_netstats which contains exactly what we need for that purpose
>>>> and update the code path to report 64-bit capable statistics.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>
>>> Applied, thanks.
>>>
>>> I would run ethtool -S and ifconfig under perf to see where it is
>>> spending so much time.
>>>
>>
>> This appears to be way worse than I thought, will keep digging, but for
>> now, I may have to send a revert. Andrew, Vivien can you see if you have
>> the same problems on your boards? Thanks!
>>
>> # killall iperf
>> # [ ID] Interval Transfer Bandwidth
>> [ 3] 0.0-19.1 sec 500 MBytes 220 Mbits/sec
>> # while true; do ethtool -S gphy; ifconfig gphy; done
>> ^C^C
>>
>>
>> [ 64.566226] INFO: rcu_sched self-detected stall on CPU
>> [ 64.571487] 0-...: (25999 ticks this GP) idle=006/140000000000001/0
>
> Hi Florian
Hi Andrew,
>
> I don't get anything so bad, but i think that is because of hardware
> restrictions. I see the ethtool; ifconfig loop goes a lot slower when
> there is iperf traffic, but i don't get an RCU stall. However, the
> board i tested on only has a 100Mbps CPU interface, and it can handle
> all that traffic without pushing the CPU to 100%. What is the CPU load
> when you run your test? Even if you are going to 100% CPU load, we
> still don't want RCU stalls.
This is a quad core 1.5 Ghz board pushing 1Gbit/sec worth of traffic,
this is about 25% loaded. What is needed to reproduce this is basically:
iperf -c 192.168.1.1 -t 30 &
while true; do ifconfig gphy; ethtool -S gphy; done
when iperf terminates, the lock-up reliably occurs. I just converted
net/dsa/ to use per-cpu statistics and of course, I can no longer
reproduce this problem now...
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-03 19:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-01 22:00 [PATCH net-next] net: dsa: Add support for 64-bit statistics Florian Fainelli
2017-08-01 22:42 ` Florian Fainelli
2017-08-02 23:49 ` David Miller
2017-08-03 17:30 ` Florian Fainelli
2017-08-03 18:11 ` Andrew Lunn
2017-08-03 19:43 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).