From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net-next-2.6] ixgbe: Fix TX stats accounting Date: Tue, 24 Nov 2009 14:23:10 +0100 Message-ID: <4B0BDE3E.2020406@gmail.com> References: <20091123064630.7385.30498.stgit@ppwaskie-hc2.jf.intel.com> <2674af740911222332i65c0d066h79bf2c1ca1d5e4f0@mail.gmail.com> <1258968980.2697.9.camel@ppwaskie-mobl2> <4B0A6218.9040303@gmail.com> <4B0A9E4E.9010804@gmail.com> <19210.54486.353397.804028@gargle.gargle.HOWL> <4B0ABF6D.9000103@gmail.com> <4B0B8F52.3010005@gmail.com> <1259053673.2631.30.camel@ppwaskie-mobl2> <4B0BADA6.7080602@gmail.com> <1259057164.2631.36.camel@ppwaskie-mobl2> <4B0BC56D.1000909@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: "robert@herjulf.net" , Jesper Dangaard Brouer , Linux Netdev List , "David S. Miller" To: Peter P Waskiewicz Jr , Jeff Kirsher Return-path: Received: from gw1.cosmosbay.com ([212.99.114.194]:51976 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758060AbZKXNX1 (ORCPT ); Tue, 24 Nov 2009 08:23:27 -0500 In-Reply-To: <4B0BC56D.1000909@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: Here is an updated version, because ixgbe_get_ethtool_stats() needs to call dev_get_stats() or "ethtool -S" wont give correct tx_bytes/tx_packets values. [PATCH net-next-2.6] ixgbe: Fix TX stats accounting Several cpus can update netdev->stats.tx_bytes & netdev->stats.tx_packets in parallel. In this case, TX stats are under estimated and false sharing takes place. After a pktgen session sending exactly 200000000 packets : # ifconfig fiber0 | grep TX TX packets:198501982 errors:0 dropped:0 overruns:0 carrier:0 Multi queue devices should instead use txq->tx_bytes & txq->tx_packets in their xmit() method (appropriate txq lock already held by caller, no cache line miss), or use appropriate locking. After patch, same pktgen session gives : # ifconfig fiber0 | grep TX TX packets:200000000 errors:0 dropped:0 overruns:0 carrier:0 Signed-off-by: Eric Dumazet --- drivers/net/ixgbe/ixgbe_ethtool.c | 1 + drivers/net/ixgbe/ixgbe_main.c | 20 ++++---------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c index 74f04e1..7b7f8f6 100644 --- a/drivers/net/ixgbe/ixgbe_ethtool.c +++ b/drivers/net/ixgbe/ixgbe_ethtool.c @@ -944,6 +944,7 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev, char *p = NULL; ixgbe_update_stats(adapter); + dev_get_stats(netdev); for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++) { switch (ixgbe_gstrings_stats[i].type) { case NETDEV_STATS: diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c index ebcec30..1cea120 100644 --- a/drivers/net/ixgbe/ixgbe_main.c +++ b/drivers/net/ixgbe/ixgbe_main.c @@ -425,8 +425,6 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector, tx_ring->total_packets += total_packets; tx_ring->stats.packets += total_packets; tx_ring->stats.bytes += total_bytes; - netdev->stats.tx_bytes += total_bytes; - netdev->stats.tx_packets += total_packets; return (count < tx_ring->work_limit); } @@ -5249,6 +5247,7 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb, { struct ixgbe_adapter *adapter = netdev_priv(netdev); struct ixgbe_ring *tx_ring; + struct netdev_queue *txq; unsigned int first; unsigned int tx_flags = 0; u8 hdr_len = 0; @@ -5345,6 +5344,9 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb, tx_ring->atr_count = 0; } } + txq = netdev_get_tx_queue(netdev, r_idx); + txq->tx_bytes += skb->len; + txq->tx_packets++; ixgbe_tx_queue(adapter, tx_ring, tx_flags, count, skb->len, hdr_len); ixgbe_maybe_stop_tx(netdev, tx_ring, DESC_NEEDED); @@ -5359,19 +5361,6 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb, } /** - * ixgbe_get_stats - Get System Network Statistics - * @netdev: network interface device structure - * - * Returns the address of the device statistics structure. - * The statistics are actually updated from the timer callback. - **/ -static struct net_device_stats *ixgbe_get_stats(struct net_device *netdev) -{ - /* only return the current stats */ - return &netdev->stats; -} - -/** * ixgbe_set_mac - Change the Ethernet Address of the NIC * @netdev: network interface device structure * @p: pointer to an address structure @@ -5501,7 +5490,6 @@ static const struct net_device_ops ixgbe_netdev_ops = { .ndo_stop = ixgbe_close, .ndo_start_xmit = ixgbe_xmit_frame, .ndo_select_queue = ixgbe_select_queue, - .ndo_get_stats = ixgbe_get_stats, .ndo_set_rx_mode = ixgbe_set_rx_mode, .ndo_set_multicast_list = ixgbe_set_rx_mode, .ndo_validate_addr = eth_validate_addr,