[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191031211300.7e5c1e7c@carbon>
Date: Thu, 31 Oct 2019 21:13:00 +0100
From: Jesper Dangaard Brouer <brouer@...hat.com>
To: Charles McLachlan <cmclachlan@...arflare.com>
Cc: <davem@...emloft.net>, <netdev@...r.kernel.org>,
<linux-net-drivers@...arflare.com>, brouer@...hat.com,
David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net-next v4 6/6] sfc: add XDP counters to ethtool stats
On Thu, 31 Oct 2019 10:24:23 +0000
Charles McLachlan <cmclachlan@...arflare.com> wrote:
> Count XDP packet drops, error drops, transmissions and redirects and
> expose these counters via the ethtool stats command.
>
> Signed-off-by: Charles McLachlan <cmclachlan@...arflare.com>
I'm going to ACK this even-though I don't like these driver specific
counters. Given we have failed to standardize this, I really cannot
complain. Hopefully we will get this standardized later
Acked-by: Jesper Dangaard Brouer <brouer@...hat.com>
Left code below, if Ahern want to complain ;-)
> drivers/net/ethernet/sfc/ethtool.c | 25 +++++++++++++++++++++++++
> drivers/net/ethernet/sfc/net_driver.h | 8 ++++++++
> drivers/net/ethernet/sfc/rx.c | 9 +++++++++
> 3 files changed, 42 insertions(+)
>
> diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
> index 86b965875540..8db593fb9699 100644
> --- a/drivers/net/ethernet/sfc/ethtool.c
> +++ b/drivers/net/ethernet/sfc/ethtool.c
> @@ -83,6 +83,10 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {
> EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_frm_trunc),
> EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_events),
> EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_packets),
> + EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_drops),
> + EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_bad_drops),
> + EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_tx),
> + EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_redirect),
> };
>
> #define EFX_ETHTOOL_SW_STAT_COUNT ARRAY_SIZE(efx_sw_stat_desc)
> @@ -399,6 +403,19 @@ static size_t efx_describe_per_queue_stats(struct efx_nic *efx, u8 *strings)
> }
> }
> }
> + if (efx->xdp_tx_queue_count && efx->xdp_tx_queues) {
> + unsigned short xdp;
> +
> + for (xdp = 0; xdp < efx->xdp_tx_queue_count; xdp++) {
> + n_stats++;
> + if (strings) {
> + snprintf(strings, ETH_GSTRING_LEN,
> + "tx-xdp-cpu-%hu.tx_packets", xdp);
> + strings += ETH_GSTRING_LEN;
> + }
> + }
> + }
> +
> return n_stats;
> }
>
> @@ -509,6 +526,14 @@ static void efx_ethtool_get_stats(struct net_device *net_dev,
> data++;
> }
> }
> + if (efx->xdp_tx_queue_count && efx->xdp_tx_queues) {
> + int xdp;
> +
> + for (xdp = 0; xdp < efx->xdp_tx_queue_count; xdp++) {
> + data[0] = efx->xdp_tx_queues[xdp]->tx_packets;
> + data++;
> + }
> + }
>
> efx_ptp_update_stats(efx, data);
> }
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 505ddc060e64..04e49eac7327 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -453,6 +453,10 @@ enum efx_sync_events_state {
> * lack of descriptors
> * @n_rx_merge_events: Number of RX merged completion events
> * @n_rx_merge_packets: Number of RX packets completed by merged events
> + * @n_rx_xdp_drops: Count of RX packets intentionally dropped due to XDP
> + * @n_rx_xdp_bad_drops: Count of RX packets dropped due to XDP errors
> + * @n_rx_xdp_tx: Count of RX packets retransmitted due to XDP
> + * @n_rx_xdp_redirect: Count of RX packets redirected to a different NIC by XDP
> * @rx_pkt_n_frags: Number of fragments in next packet to be delivered by
> * __efx_rx_packet(), or zero if there is none
> * @rx_pkt_index: Ring index of first buffer for next packet to be delivered
> @@ -506,6 +510,10 @@ struct efx_channel {
> unsigned int n_rx_nodesc_trunc;
> unsigned int n_rx_merge_events;
> unsigned int n_rx_merge_packets;
> + unsigned int n_rx_xdp_drops;
> + unsigned int n_rx_xdp_bad_drops;
> + unsigned int n_rx_xdp_tx;
> + unsigned int n_rx_xdp_redirect;
>
> unsigned int rx_pkt_n_frags;
> unsigned int rx_pkt_index;
> diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
> index 91f6d5b9ceac..a7d9841105d8 100644
> --- a/drivers/net/ethernet/sfc/rx.c
> +++ b/drivers/net/ethernet/sfc/rx.c
> @@ -677,6 +677,7 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,
> netif_err(efx, rx_err, efx->net_dev,
> "XDP is not possible with multiple receive fragments (%d)\n",
> channel->rx_pkt_n_frags);
> + channel->n_rx_xdp_bad_drops++;
> return false;
> }
>
> @@ -722,6 +723,9 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,
> if (net_ratelimit())
> netif_err(efx, rx_err, efx->net_dev,
> "XDP TX failed (%d)\n", err);
> + channel->n_rx_xdp_bad_drops++;
> + } else {
> + channel->n_rx_xdp_tx++;
> }
> break;
>
> @@ -732,12 +736,16 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,
> if (net_ratelimit())
> netif_err(efx, rx_err, efx->net_dev,
> "XDP redirect failed (%d)\n", err);
> + channel->n_rx_xdp_bad_drops++;
> + } else {
> + channel->n_rx_xdp_redirect++;
> }
> break;
>
> default:
> bpf_warn_invalid_xdp_action(xdp_act);
> efx_free_rx_buffers(rx_queue, rx_buf, 1);
> + channel->n_rx_xdp_bad_drops++;
> break;
>
> case XDP_ABORTED:
> @@ -745,6 +753,7 @@ static bool efx_do_xdp(struct efx_nic *efx, struct efx_channel *channel,
> /* Fall through */
> case XDP_DROP:
> efx_free_rx_buffers(rx_queue, rx_buf, 1);
> + channel->n_rx_xdp_drops++;
> break;
> }
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
Powered by blists - more mailing lists