From: Jesper Dangaard Brouer <brouer@redhat•com>
To: "Björn Töpel" <bjorn.topel@intel•com>
Cc: jeffrey.t.kirsher@intel•com, intel-wired-lan@lists•osuosl.org,
magnus.karlsson@intel•com, magnus.karlsson@gmail•com,
jacob.e.keller@intel•com, bjorn.topel@gmail•com,
brouer@redhat•com,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>
Subject: Re: [PATCH] i40e: report correct statistics when XDP is enabled
Date: Fri, 24 Aug 2018 16:00:16 +0200 [thread overview]
Message-ID: <20180824160016.245b08d8@redhat.com> (raw)
In-Reply-To: <20180824112159.32298-1-bjorn.topel@intel.com>
On Fri, 24 Aug 2018 13:21:59 +0200
Björn Töpel <bjorn.topel@intel•com> wrote:
> When XDP is enabled, the driver will report incorrect
> statistics. Received frames will reported as transmitted frames.
>
> This commits fixes the i40e implementation of ndo_get_stats64 (struct
> net_device_ops), so that iproute2 will report correct statistics
> (e.g. when running "ip -stats link show dev eth0") even when XDP is
> enabled.
>
> Reported-by: Jesper Dangaard Brouer <brouer@redhat•com>
> Fixes: 74608d17fe29 ("i40e: add support for XDP_TX action")
Stable candidate:
$ git describe --contains 74608d17fe29
v4.13-rc1~157^2~128^2~13
> Signed-off-by: Björn Töpel <bjorn.topel@intel•com>
It works for me:
Tested-by: Jesper Dangaard Brouer <brouer@redhat•com>
I'm explicitly _not_ ACK'ing the patch, as I think the your code changes
below makes it harder to follow whether a TX or RX ring is getting
updated. But it is 100% up to the driver maintainers to say if this is
acceptable from a maintenance PoV.
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 24 +++++++++++----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index e40c023cc7b6..7c122dd3faa1 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -425,9 +425,9 @@ static void i40e_get_netdev_stats_struct(struct net_device *netdev,
> struct rtnl_link_stats64 *stats)
> {
> struct i40e_netdev_priv *np = netdev_priv(netdev);
> - struct i40e_ring *tx_ring, *rx_ring;
> struct i40e_vsi *vsi = np->vsi;
> struct rtnl_link_stats64 *vsi_stats = i40e_get_vsi_stats_struct(vsi);
> + struct i40e_ring *ring;
> int i;
>
> if (test_bit(__I40E_VSI_DOWN, vsi->state))
> @@ -441,24 +441,26 @@ static void i40e_get_netdev_stats_struct(struct net_device *netdev,
> u64 bytes, packets;
> unsigned int start;
>
> - tx_ring = READ_ONCE(vsi->tx_rings[i]);
> - if (!tx_ring)
> + ring = READ_ONCE(vsi->tx_rings[i]);
> + if (!ring)
> continue;
> - i40e_get_netdev_stats_struct_tx(tx_ring, stats);
> + i40e_get_netdev_stats_struct_tx(ring, stats);
>
> - rx_ring = &tx_ring[1];
> + if (i40e_enabled_xdp_vsi(vsi)) {
> + ring++;
> + i40e_get_netdev_stats_struct_tx(ring, stats);
> + }
>
> + ring++;
> do {
> - start = u64_stats_fetch_begin_irq(&rx_ring->syncp);
> - packets = rx_ring->stats.packets;
> - bytes = rx_ring->stats.bytes;
> - } while (u64_stats_fetch_retry_irq(&rx_ring->syncp, start));
> + start = u64_stats_fetch_begin_irq(&ring->syncp);
> + packets = ring->stats.packets;
> + bytes = ring->stats.bytes;
> + } while (u64_stats_fetch_retry_irq(&ring->syncp, start));
>
> stats->rx_packets += packets;
> stats->rx_bytes += bytes;
>
> - if (i40e_enabled_xdp_vsi(vsi))
> - i40e_get_netdev_stats_struct_tx(&rx_ring[1], stats);
> }
> rcu_read_unlock();
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
next parent reply other threads:[~2018-08-24 17:35 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20180824112159.32298-1-bjorn.topel@intel.com>
2018-08-24 14:00 ` Jesper Dangaard Brouer [this message]
2018-08-28 17:00 ` [Intel-wired-lan] [PATCH] i40e: report correct statistics when XDP is enabled Paul Menzel
2018-08-28 17:10 ` Björn Töpel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180824160016.245b08d8@redhat.com \
--to=brouer@redhat$(echo .)com \
--cc=bjorn.topel@gmail$(echo .)com \
--cc=bjorn.topel@intel$(echo .)com \
--cc=intel-wired-lan@lists$(echo .)osuosl.org \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=jeffrey.t.kirsher@intel$(echo .)com \
--cc=magnus.karlsson@gmail$(echo .)com \
--cc=magnus.karlsson@intel$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox