[PATCH] net: rocker: Add basic netdev counters - v2

[PATCH] net: rocker: Add basic netdev counters - v2

[David Ahern]

Add packet and byte counters for RX and TX paths.

$ ifconfig eth1
eth1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet6 fe80::5054:ff:fe12:3501  prefixlen 64  scopeid 0x20<link>
        ether 52:54:00:12:35:01  txqueuelen 1000  (Ethernet)
        RX packets 63  bytes 15813 (15.4 KiB)
        RX errors 1  dropped 0  overruns 0  frame 0
        TX packets 79  bytes 17991 (17.5 KiB)
        TX errors 7  dropped 0 overruns 0  carrier 0  collisions 0

Rx / Tx errors tested by injecting faults in qemu's hardware model for Rocker.

v2:
- moved counter locations to avoid potential use after free per Florian's comment

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Scott Feldman <sfeldma@gmail.com>
Cc: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/ethernet/rocker/rocker.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index c5548164d1a8..f9daf67117b9 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -3915,6 +3915,8 @@ static netdev_tx_t rocker_port_xmit(struct sk_buff *skb, struct net_device *dev)
 	rocker_tlv_nest_cancel(desc_info, frags);
 out:
 	dev_kfree_skb(skb);
+	dev->stats.tx_dropped++;
+
 	return NETDEV_TX_OK;
 }
 
@@ -4238,12 +4240,22 @@ static int rocker_port_poll_tx(struct napi_struct *napi, int budget)
 
 	/* Cleanup tx descriptors */
 	while ((desc_info = rocker_desc_tail_get(&rocker_port->tx_ring))) {
+		struct sk_buff *skb;
+
 		err = rocker_desc_err(desc_info);
 		if (err && net_ratelimit())
 			netdev_err(rocker_port->dev, "tx desc received with err %d\n",
 				   err);
 		rocker_tx_desc_frags_unmap(rocker_port, desc_info);
-		dev_kfree_skb_any(rocker_desc_cookie_ptr_get(desc_info));
+
+		skb = rocker_desc_cookie_ptr_get(desc_info);
+		if (err == 0) {
+			rocker_port->dev->stats.tx_packets++;
+			rocker_port->dev->stats.tx_bytes += skb->len;
+		} else
+			rocker_port->dev->stats.tx_errors++;
+
+		dev_kfree_skb_any(skb);
 		credits++;
 	}
 
@@ -4276,6 +4288,10 @@ static int rocker_port_rx_proc(struct rocker *rocker,
 	rx_len = rocker_tlv_get_u16(attrs[ROCKER_TLV_RX_FRAG_LEN]);
 	skb_put(skb, rx_len);
 	skb->protocol = eth_type_trans(skb, rocker_port->dev);
+
+	rocker_port->dev->stats.rx_packets++;
+	rocker_port->dev->stats.rx_bytes += skb->len;
+
 	netif_receive_skb(skb);
 
 	return rocker_dma_rx_ring_skb_alloc(rocker, rocker_port, desc_info);
@@ -4309,6 +4325,9 @@ static int rocker_port_poll_rx(struct napi_struct *napi, int budget)
 				netdev_err(rocker_port->dev, "rx processing failed with err %d\n",
 					   err);
 		}
+		if (err)
+			rocker_port->dev->stats.rx_errors++;
+
 		rocker_desc_gen_clear(desc_info);
 		rocker_desc_head_set(rocker, &rocker_port->rx_ring, desc_info);
 		credits++;
-- 
1.9.3 (Apple Git-50)

Re: [PATCH] net: rocker: Add basic netdev counters - v2

[Scott Feldman]

On Fri, Jan 16, 2015 at 1:22 PM, David Ahern <dsahern@gmail.com> wrote:
> Add packet and byte counters for RX and TX paths.
>
> $ ifconfig eth1
> eth1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet6 fe80::5054:ff:fe12:3501  prefixlen 64  scopeid 0x20<link>
>         ether 52:54:00:12:35:01  txqueuelen 1000  (Ethernet)
>         RX packets 63  bytes 15813 (15.4 KiB)
>         RX errors 1  dropped 0  overruns 0  frame 0
>         TX packets 79  bytes 17991 (17.5 KiB)
>         TX errors 7  dropped 0 overruns 0  carrier 0  collisions 0
>
> Rx / Tx errors tested by injecting faults in qemu's hardware model for Rocker.
>
> v2:
> - moved counter locations to avoid potential use after free per Florian's comment
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Scott Feldman <sfeldma@gmail.com>
> Cc: Jiri Pirko <jiri@resnulli.us>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>

Thanks David.  I think this is good enough for first pass.  Longer
term, I'd like to see this replaced by stats read from device for each
port.

-scott

Re: [PATCH] net: rocker: Add basic netdev counters - v2

[David Ahern]

On 1/16/15 4:41 PM, Scott Feldman wrote:
>
> Thanks David.  I think this is good enough for first pass.  Longer
> term, I'd like to see this replaced by stats read from device for each
> port.

Aren't there 2 sets of stats at play here?
1. packets punted to the control plane / CPU,
2. packets forwarded through the ports.

If you have the netdev counters resemble packets handled through the 
port it could be confusing to someone looking at packets handled by the OS.

What about having detailed counters local to the driver (i.e., ethtool 
-S) show the low level stats like packets handled through the port?

David

Re: [PATCH] net: rocker: Add basic netdev counters - v2

[Scott Feldman]

On Fri, Jan 16, 2015 at 3:48 PM, David Ahern <dsahern@gmail.com> wrote:
> On 1/16/15 4:41 PM, Scott Feldman wrote:
>>
>>
>> Thanks David.  I think this is good enough for first pass.  Longer
>> term, I'd like to see this replaced by stats read from device for each
>> port.
>
>
> Aren't there 2 sets of stats at play here?
> 1. packets punted to the control plane / CPU,
> 2. packets forwarded through the ports.

You're right.

> If you have the netdev counters resemble packets handled through the port it
> could be confusing to someone looking at packets handled by the OS.

Agreed, keep traditional netdev stats for pkts to/from OS.  Maybe
we're done then?

> What about having detailed counters local to the driver (i.e., ethtool -S)
> show the low level stats like packets handled through the port?

That sounds perfect, to use ethtool -S for extended device stats.

Re: [PATCH] net: rocker: Add basic netdev counters - v2

[Jiri Pirko]

Fri, Jan 16, 2015 at 10:22:29PM CET, dsahern@gmail.com wrote:
>Add packet and byte counters for RX and TX paths.
>
>$ ifconfig eth1
>eth1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>        inet6 fe80::5054:ff:fe12:3501  prefixlen 64  scopeid 0x20<link>
>        ether 52:54:00:12:35:01  txqueuelen 1000  (Ethernet)
>        RX packets 63  bytes 15813 (15.4 KiB)
>        RX errors 1  dropped 0  overruns 0  frame 0
>        TX packets 79  bytes 17991 (17.5 KiB)
>        TX errors 7  dropped 0 overruns 0  carrier 0  collisions 0
>
>Rx / Tx errors tested by injecting faults in qemu's hardware model for Rocker.
>
>v2:
>- moved counter locations to avoid potential use after free per Florian's comment
>
>Signed-off-by: David Ahern <dsahern@gmail.com>

Acked-by: Jiri Pirko <jiri@resnulli.us>

Thanks David.

Re: [PATCH] net: rocker: Add basic netdev counters - v2

[David Miller]

From: Scott Feldman <sfeldma@gmail.com>
Date: Fri, 16 Jan 2015 15:41:41 -0800

> On Fri, Jan 16, 2015 at 1:22 PM, David Ahern <dsahern@gmail.com> wrote:
>> Add packet and byte counters for RX and TX paths.
>>
>> $ ifconfig eth1
>> eth1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>>         inet6 fe80::5054:ff:fe12:3501  prefixlen 64  scopeid 0x20<link>
>>         ether 52:54:00:12:35:01  txqueuelen 1000  (Ethernet)
>>         RX packets 63  bytes 15813 (15.4 KiB)
>>         RX errors 1  dropped 0  overruns 0  frame 0
>>         TX packets 79  bytes 17991 (17.5 KiB)
>>         TX errors 7  dropped 0 overruns 0  carrier 0  collisions 0
>>
>> Rx / Tx errors tested by injecting faults in qemu's hardware model for Rocker.
>>
>> v2:
>> - moved counter locations to avoid potential use after free per Florian's comment
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> Cc: Scott Feldman <sfeldma@gmail.com>
>> Cc: Jiri Pirko <jiri@resnulli.us>
> 
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> 
> Thanks David.  I think this is good enough for first pass.  Longer
> term, I'd like to see this replaced by stats read from device for each
> port.

Ok, applied, thanks everyone.