sfc: per-queue stats
Subsystems (3)
v2 cover sfc: per-queue stats
From: Edward Cree <[email protected]>
This series implements the netdev_stat_ops interface for per-queue
statistics in the sfc driver, mostly using existing counters that were originally added for ethtool -S output.
Changed in v2:
* exclude (dedicated) XDP TXQ stats from per-queue TX stats
* explain patch #3 better
Edward Cree (6):
sfc: remove obsolete counters from struct efx_channel
sfc: implement basic per-queue stats
sfc: add n_rx_overlength to ethtool stats
sfc: implement per-queue rx drop and overrun stats
sfc: implement per-queue TSO (hw_gso) stats
sfc: add per-queue RX and TX bytes stats
drivers/net/ethernet/sfc/ef100_rx.c | 5 +- drivers/net/ethernet/sfc/ef100_tx.c | 1 + drivers/net/ethernet/sfc/efx.c | 109 ++++++++++++++++++++++ drivers/net/ethernet/sfc/efx_channels.c | 4 + drivers/net/ethernet/sfc/efx_channels.h | 8 +- drivers/net/ethernet/sfc/ethtool_common.c | 3 +- drivers/net/ethernet/sfc/net_driver.h | 31 +++++- drivers/net/ethernet/sfc/rx.c | 5 +- drivers/net/ethernet/sfc/rx_common.c | 3 + drivers/net/ethernet/sfc/tx.c | 2 + drivers/net/ethernet/sfc/tx_common.c | 5 + 11 files changed, 165 insertions(+), 11 deletions(-)
PATCH v2 2/6 sfc: implement basic per-queue stats
From: Edward Cree <[email protected]>
Just RX and TX packet counts for now. We do not have per-queue
byte counts, which causes us to fail stats.pkt_byte_sum selftest with "Drivers should always report basic keys" error.
Per-queue counts are since the last time the queue was inited
(typically by efx_start_datapath(), on ifup or reconfiguration); device-wide total (efx_get_base_stats()) is since driver probe. This is not the same lifetime as rtnl_link_stats64, which uses firmware stats which count since FW (re)booted; this can cause a "Qstats are lower" or "RTNL stats are lower" failure in stats.pkt_byte_sum selftest.
Move the increment of rx_queue->rx_packets to match the semantics
specified for netdev per-queue stats, i.e. just before handing the packet to XDP (if present) or the netstack (through GRO). This will affect the existing ethtool -S output which also reports these counters.
--- drivers/net/ethernet/sfc/ef100_rx.c | 4 +- drivers/net/ethernet/sfc/efx.c | 74 +++++++++++++++++++++++++ drivers/net/ethernet/sfc/efx_channels.h | 1 - drivers/net/ethernet/sfc/net_driver.h | 6 ++ drivers/net/ethernet/sfc/rx.c | 4 +- drivers/net/ethernet/sfc/rx_common.c | 2 + drivers/net/ethernet/sfc/tx_common.c | 2 + 7 files changed, 88 insertions(+), 5 deletions(-)
drivers/net/ethernet/sfc/ef100_rx.c +2 -2
@@ -134,6 +134,8 @@ void __ef100_rx_packet(struct efx_channel *channel) goto free_rx_buffer; } + ++rx_queue->rx_packets; +efx_rx_packet_gro (channel, rx_buf, channel->rx_pkt_n_frags, eh, csum); goto out;
@@ -149,8 +151,6 @@ static void ef100_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index)struct efx_channel *channel =efx_rx_queue_channel (rx_queue);struct efx_nic *efx = rx_queue->efx; - ++rx_queue->rx_packets; -netif_vdbg (efx, rx_status, efx->net_dev,"RX queue %d received id %x\n" ,efx_rx_queue_index (rx_queue), index);
drivers/net/ethernet/sfc/efx.c +74 -0
92 lines not on this page · raw message
drivers/net/ethernet/sfc/efx_channels.h +0 -1
@@ -49,5 +49,4 @@ void efx_fini_napi_channel(struct efx_channel *channel);void efx_fini_napi (struct efx_nic *efx);void efx_channel_dummy_op_void (struct efx_channel *channel); -#endif
drivers/net/ethernet/sfc/net_driver.h +6 -0
@@ -232,6 +232,8 @@ struct efx_tx_buffer {* @xmit_pending: Are any packets waiting to be pushed to the NIC * @cb_packets: Number of times the TX copybreak feature has been used * @notify_count: Count of notified descriptors to the NIC +* @tx_packets: Number of packets sent since this struct was created +* @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue() * @empty_read_count: If the completion path has seen the queue as empty * and the transmission path has not yet checked this, the value of * @read_count bitwise-added to %EFX_EMPTY_COUNT_VALID; otherwise 0.
@@ -281,6 +283,7 @@ struct efx_tx_queue {unsigned int notify_count;/* Statistics to supplement MAC stats */ unsigned long tx_packets; +unsigned long old_tx_packets;/* Members shared between paths and sometimes updated */ unsigned int empty_read_count____cacheline_aligned_in_smp ;
@@ -370,6 +373,8 @@ struct efx_rx_page_state {* @recycle_count: RX buffer recycle counter. * @slow_fill: Timer used to defer efx_nic_generate_fill_event(). * @grant_work: workitem used to grant credits to the MAE if @grant_credits +* @rx_packets: Number of packets received since this struct was created +* @old_rx_packets: Value of @rx_packets as of last efx_init_rx_queue() * @xdp_rxq_info: XDP specific RX queue information. * @xdp_rxq_info_valid: Is xdp_rxq_info valid data?. */
@@ -406,6 +411,7 @@ struct efx_rx_queue {struct work_struct grant_work;/* Statistics to supplement MAC stats */ unsigned long rx_packets; +unsigned long old_rx_packets;struct xdp_rxq_info xdp_rxq_info;bool xdp_rxq_info_valid; };
drivers/net/ethernet/sfc/rx.c +2 -2
@@ -125,8 +125,6 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,struct efx_channel *channel =efx_rx_queue_channel (rx_queue);struct efx_rx_buffer *rx_buf; - rx_queue->rx_packets++; - rx_buf =efx_rx_buffer (rx_queue, index); rx_buf->flags |= flags;
@@ -394,6 +392,8 @@ void __efx_rx_packet(struct efx_channel *channel) goto out; } + rx_queue->rx_packets++; +if (!efx_do_xdp (efx, channel, rx_buf, &eh)) goto out;
drivers/net/ethernet/sfc/rx_common.c +2 -0
@@ -241,6 +241,8 @@ void efx_init_rx_queue(struct efx_rx_queue *rx_queue) rx_queue->page_recycle_failed =0 ; rx_queue->page_recycle_full =0 ; + rx_queue->old_rx_packets = rx_queue->rx_packets; +/* Initialise limit fields */ max_fill = efx->rxq_entries -EFX_RXD_HEAD_ROOM ; max_trigger =
drivers/net/ethernet/sfc/tx_common.c +2 -0
@@ -86,6 +86,8 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue) tx_queue->completed_timestamp_major =0 ; tx_queue->completed_timestamp_minor =0 ; + tx_queue->old_tx_packets = tx_queue->tx_packets; + tx_queue->xdp_tx =efx_channel_is_xdp_tx (tx_queue->channel); tx_queue->tso_version =0 ;
PATCH v2 1/6 sfc: remove obsolete counters from struct efx_channel
In reply to [email protected]From: Edward Cree <[email protected]>
The n_rx_tobe_disc and n_rx_mcast_mismatch counters are a legacy
from farch, and are never written in EF10 or EF100 code. Remove them from the struct and from ethtool -S output, saving a bit of memory and avoiding user confusion.
--- drivers/net/ethernet/sfc/ethtool_common.c | 2 -- drivers/net/ethernet/sfc/net_driver.h | 4 ---- 2 files changed, 6 deletions(-)
drivers/net/ethernet/sfc/ethtool_common.c +0 -2
@@ -75,7 +75,6 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {EFX_ETHTOOL_UINT_TXQ_STAT (pio_packets),EFX_ETHTOOL_UINT_TXQ_STAT (cb_packets),EFX_ETHTOOL_ATOMIC_NIC_ERROR_STAT (rx_reset), -EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_tobe_disc),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_ip_hdr_chksum_err),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_tcp_udp_chksum_err), EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_inner_ip_hdr_chksum_err),
@@ -83,7 +82,6 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_outer_ip_hdr_chksum_err),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_outer_tcp_udp_chksum_err),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_eth_crc_err), -EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_mcast_mismatch),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_frm_trunc),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_merge_events), EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_packets),
drivers/net/ethernet/sfc/net_driver.h +0 -4
@@ -451,10 +451,8 @@ enum efx_sync_events_state {* @filter_work: Work item for efx_filter_rfs_expire() * @rps_flow_id: Flow IDs of filters allocated for accelerated RFS, * indexed by filter ID -* @n_rx_tobe_disc: Count of RX_TOBE_DISC errors * @n_rx_ip_hdr_chksum_err: Count of RX IP header checksum errors * @n_rx_tcp_udp_chksum_err: Count of RX TCP and UDP checksum errors -* @n_rx_mcast_mismatch: Count of unmatched multicast frames * @n_rx_frm_trunc: Count of RX_FRM_TRUNC errors * @n_rx_overlength: Count of RX_OVERLENGTH errors * @n_skbuff_leaks: Count of skbuffs leaked due to RX overrun
@@ -511,7 +509,6 @@ struct efx_channel {u32 *rps_flow_id;#endif -unsigned int n_rx_tobe_disc;unsigned int n_rx_ip_hdr_chksum_err;unsigned int n_rx_tcp_udp_chksum_err;unsigned int n_rx_outer_ip_hdr_chksum_err;
@@ -519,7 +516,6 @@ struct efx_channel {unsigned int n_rx_inner_ip_hdr_chksum_err;unsigned int n_rx_inner_tcp_udp_chksum_err;unsigned int n_rx_eth_crc_err; -unsigned int n_rx_mcast_mismatch;unsigned int n_rx_frm_trunc;unsigned int n_rx_overlength;unsigned int n_skbuff_leaks;
PATCH v2 4/6 sfc: implement per-queue rx drop and overrun stats
In reply to [email protected]From: Edward Cree <[email protected]>
--- drivers/net/ethernet/sfc/efx.c | 15 +++++++++++++-- drivers/net/ethernet/sfc/efx_channels.c | 4 ++++ drivers/net/ethernet/sfc/efx_channels.h | 7 +++++++ drivers/net/ethernet/sfc/net_driver.h | 7 +++++++ 4 files changed, 31 insertions(+), 2 deletions(-)
drivers/net/ethernet/sfc/efx.c +13 -2
@@ -638,6 +638,10 @@ static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx, rx_queue =efx_channel_get_rx_queue (channel);/* Count only packets since last time datapath was started */ stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets; + stats->hw_drops =efx_get_queue_stat_rx_hw_drops (channel) - + channel->old_n_rx_hw_drops; + stats->hw_drop_overruns = channel->n_rx_nodesc_trunc - + channel->old_n_rx_hw_drop_overruns; } static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
@@ -670,6 +674,8 @@ static void efx_get_base_stats(struct net_device *net_dev,struct efx_channel *channel; rx->packets =0 ; + rx->hw_drops =0 ; + rx->hw_drop_overruns =0 ; tx->packets =0 ;/* Count all packets on non-core queues, and packets before last
@@ -677,10 +683,15 @@ static void efx_get_base_stats(struct net_device *net_dev,*/ efx_for_each_channel(channel, efx) { rx_queue = efx_channel_get_rx_queue(channel); - if (channel->channel >= net_dev->real_num_rx_queues) + if (channel->channel >= net_dev->real_num_rx_queues) { rx->packets += rx_queue->rx_packets; - else + rx->hw_drops += efx_get_queue_stat_rx_hw_drops(channel); + rx->hw_drop_overruns += channel->n_rx_nodesc_trunc; + } else { rx->packets += rx_queue->old_rx_packets; + rx->hw_drops += channel->old_n_rx_hw_drops; + rx->hw_drop_overruns += channel->old_n_rx_hw_drop_overruns; + } efx_for_each_channel_tx_queue(tx_queue, channel) { if (channel->channel < efx->tx_channel_offset || channel->channel >= efx->tx_channel_offset +
drivers/net/ethernet/sfc/efx_channels.c +4 -0
@@ -1100,6 +1100,10 @@ void efx_start_channels(struct efx_nic *efx)atomic_inc (&efx->active_queues); } +/* reset per-queue stats */ + channel->old_n_rx_hw_drops =efx_get_queue_stat_rx_hw_drops (channel); + channel->old_n_rx_hw_drop_overruns = channel->n_rx_nodesc_trunc; +efx_for_each_channel_rx_queue (rx_queue, channel) {efx_init_rx_queue (rx_queue);atomic_inc (&efx->active_queues);
drivers/net/ethernet/sfc/efx_channels.h +7 -0
@@ -43,6 +43,13 @@ struct efx_channel *efx_copy_channel(const struct efx_channel *old_channel);void efx_start_channels (struct efx_nic *efx);void efx_stop_channels (struct efx_nic *efx); +static inline u64 efx_get_queue_stat_rx_hw_drops (struct efx_channel *channel) +{ +return channel->n_rx_eth_crc_err + channel->n_rx_frm_trunc + + channel->n_rx_overlength + channel->n_rx_nodesc_trunc + + channel->n_rx_mport_bad; +} +void efx_init_napi_channel (struct efx_channel *channel);void efx_init_napi (struct efx_nic *efx);void efx_fini_napi_channel (struct efx_channel *channel);
drivers/net/ethernet/sfc/net_driver.h +7 -0
@@ -472,6 +472,10 @@ enum efx_sync_events_state {* @n_rx_xdp_redirect: Count of RX packets redirected to a different NIC by XDP * @n_rx_mport_bad: Count of RX packets dropped because their ingress mport was * not recognised +* @old_n_rx_hw_drops: Count of all RX packets dropped for any reason as of last +* efx_start_channels() +* @old_n_rx_hw_drop_overruns: Value of @n_rx_nodesc_trunc as of last +* efx_start_channels() * @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
@@ -534,6 +538,9 @@ struct efx_channel {unsigned int n_rx_xdp_redirect;unsigned int n_rx_mport_bad; +unsigned int old_n_rx_hw_drops; +unsigned int old_n_rx_hw_drop_overruns; +unsigned int rx_pkt_n_frags;unsigned int rx_pkt_index;
PATCH v2 3/6 sfc: add n_rx_overlength to ethtool stats
In reply to [email protected]From: Edward Cree <[email protected]>
The previous patch changed when we increment the RX queue's rx_packets
counter, to match the semantics of netdev per-queue stats. The differences between the old and new counts are scatter errors (which produce a WARN_ON) and this counter, which is incremented by efx_rx_packet__check_len() when an RX packet (which was placed in a single buffer by SG, i.e. n_frags == 1) has a length (from the RX event) which is too long to fit in the RX buffer. If this occurs, we drop the packet and fire a ratelimited netif_err().
The counter previously was not reported anywhere; add it to ethtool -S
output to ensure users still have this information.
--- drivers/net/ethernet/sfc/ethtool_common.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/ethernet/sfc/ethtool_common.c +1 -0
@@ -83,6 +83,7 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_outer_tcp_udp_chksum_err),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_eth_crc_err),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_frm_trunc), +EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_overlength),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_merge_events),EFX_ETHTOOL_UINT_CHANNEL_STAT (rx_merge_packets), EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_drops),
PATCH v2 5/6 sfc: implement per-queue TSO (hw_gso) stats
In reply to [email protected]From: Edward Cree <[email protected]>
--- drivers/net/ethernet/sfc/efx.c | 20 +++++++++++++++++--- drivers/net/ethernet/sfc/net_driver.h | 4 ++++ drivers/net/ethernet/sfc/tx_common.c | 2 ++ 3 files changed, 23 insertions(+), 3 deletions(-)
drivers/net/ethernet/sfc/efx.c +17 -3
@@ -653,15 +653,22 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx, channel =efx_get_tx_channel (efx, idx); stats->packets =0 ; + stats->hw_gso_packets =0 ; + stats->hw_gso_wire_packets =0 ;/* If a TX channel has XDP TXQs, the stats for these will be counted * in base stats; however, in EFX_XDP_TX_QUEUES_BORROWED mode we use * the same TXQ as the core, and thus XDP TXes get included in these * per-queue stats. */ efx_for_each_channel_tx_queue (tx_queue, channel) -if (!tx_queue->xdp_tx) +if (!tx_queue->xdp_tx) { stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets; + stats->hw_gso_packets += tx_queue->tso_bursts - + tx_queue->old_tso_bursts; + stats->hw_gso_wire_packets += tx_queue->tso_packets - + tx_queue->old_tso_packets; + } } static void efx_get_base_stats(struct net_device *net_dev,
@@ -677,6 +684,8 @@ static void efx_get_base_stats(struct net_device *net_dev, rx->hw_drops =0 ; rx->hw_drop_overruns =0 ; tx->packets =0 ; + tx->hw_gso_packets =0 ; + tx->hw_gso_wire_packets =0 ;/* Count all packets on non-core queues, and packets before last * datapath start on core queues.
@@ -696,10 +705,15 @@ static void efx_get_base_stats(struct net_device *net_dev,if (channel->channel < efx->tx_channel_offset || channel->channel >= efx->tx_channel_offset + net_dev->real_num_tx_queues || - tx_queue->xdp_tx) + tx_queue->xdp_tx) { tx->packets += tx_queue->tx_packets; -else + tx->hw_gso_packets += tx_queue->tso_bursts; + tx->hw_gso_wire_packets += tx_queue->tso_packets; + }else { tx->packets += tx_queue->old_tx_packets; + tx->hw_gso_packets += tx_queue->old_tso_bursts; + tx->hw_gso_wire_packets += tx_queue->old_tso_packets; + } } } }
drivers/net/ethernet/sfc/net_driver.h +4 -0
@@ -234,6 +234,8 @@ struct efx_tx_buffer {* @notify_count: Count of notified descriptors to the NIC * @tx_packets: Number of packets sent since this struct was created * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue() +* @old_tso_bursts: Value of @tso_bursts as of last efx_init_tx_queue() +* @old_tso_packets: Value of @tso_packets as of last efx_init_tx_queue() * @empty_read_count: If the completion path has seen the queue as empty * and the transmission path has not yet checked this, the value of * @read_count bitwise-added to %EFX_EMPTY_COUNT_VALID; otherwise 0.
@@ -284,6 +286,8 @@ struct efx_tx_queue {/* Statistics to supplement MAC stats */ unsigned long tx_packets;unsigned long old_tx_packets; +unsigned int old_tso_bursts; +unsigned int old_tso_packets;/* Members shared between paths and sometimes updated */ unsigned int empty_read_count____cacheline_aligned_in_smp ;
drivers/net/ethernet/sfc/tx_common.c +2 -0
@@ -87,6 +87,8 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue) tx_queue->completed_timestamp_minor =0 ; tx_queue->old_tx_packets = tx_queue->tx_packets; + tx_queue->old_tso_bursts = tx_queue->tso_bursts; + tx_queue->old_tso_packets = tx_queue->tso_packets; tx_queue->xdp_tx =efx_channel_is_xdp_tx (tx_queue->channel); tx_queue->tso_version =0 ;
PATCH v2 6/6 sfc: add per-queue RX and TX bytes stats
In reply to [email protected]From: Edward Cree <[email protected]>
While this does add overhead to the fast path, it should be minimal
as the cacheline should already be held for write from updating the queue's [tr]x_packets stat.
--- drivers/net/ethernet/sfc/ef100_rx.c | 1 + drivers/net/ethernet/sfc/ef100_tx.c | 1 + drivers/net/ethernet/sfc/efx.c | 10 ++++++++++ drivers/net/ethernet/sfc/net_driver.h | 10 ++++++++++ drivers/net/ethernet/sfc/rx.c | 1 + drivers/net/ethernet/sfc/rx_common.c | 1 + drivers/net/ethernet/sfc/tx.c | 2 ++ drivers/net/ethernet/sfc/tx_common.c | 1 + 8 files changed, 27 insertions(+)
drivers/net/ethernet/sfc/ef100_rx.c +1 -0
@@ -135,6 +135,7 @@ void __ef100_rx_packet(struct efx_channel *channel) } ++rx_queue->rx_packets; + rx_queue->rx_bytes += rx_buf->len;efx_rx_packet_gro (channel, rx_buf, channel->rx_pkt_n_frags, eh, csum); goto out;
drivers/net/ethernet/sfc/ef100_tx.c +1 -0
@@ -493,6 +493,7 @@ int __ef100_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb, }else { tx_queue->tx_packets++; } + tx_queue->tx_bytes += skb->len;return 0 ; err:
drivers/net/ethernet/sfc/efx.c +10 -0
@@ -638,6 +638,7 @@ static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx, rx_queue =efx_channel_get_rx_queue (channel);/* Count only packets since last time datapath was started */ stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets; + stats->bytes = rx_queue->rx_bytes - rx_queue->old_rx_bytes; stats->hw_drops =efx_get_queue_stat_rx_hw_drops (channel) - channel->old_n_rx_hw_drops; stats->hw_drop_overruns = channel->n_rx_nodesc_trunc -
@@ -653,6 +654,7 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx, channel =efx_get_tx_channel (efx, idx); stats->packets =0 ; + stats->bytes =0 ; stats->hw_gso_packets =0 ; stats->hw_gso_wire_packets =0 ;/* If a TX channel has XDP TXQs, the stats for these will be counted
@@ -664,6 +666,8 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,if (!tx_queue->xdp_tx) { stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets; + stats->bytes += tx_queue->tx_bytes - + tx_queue->old_tx_bytes; stats->hw_gso_packets += tx_queue->tso_bursts - tx_queue->old_tso_bursts; stats->hw_gso_wire_packets += tx_queue->tso_packets -
@@ -681,9 +685,11 @@ static void efx_get_base_stats(struct net_device *net_dev,struct efx_channel *channel; rx->packets =0 ; + rx->bytes =0 ; rx->hw_drops =0 ; rx->hw_drop_overruns =0 ; tx->packets =0 ; + tx->bytes =0 ; tx->hw_gso_packets =0 ; tx->hw_gso_wire_packets =0 ;
@@ -694,10 +700,12 @@ static void efx_get_base_stats(struct net_device *net_dev, rx_queue =efx_channel_get_rx_queue (channel);if (channel->channel >= net_dev->real_num_rx_queues) { rx->packets += rx_queue->rx_packets; + rx->bytes += rx_queue->rx_bytes; rx->hw_drops +=efx_get_queue_stat_rx_hw_drops (channel); rx->hw_drop_overruns += channel->n_rx_nodesc_trunc; }else { rx->packets += rx_queue->old_rx_packets; + rx->bytes += rx_queue->old_rx_bytes; rx->hw_drops += channel->old_n_rx_hw_drops; rx->hw_drop_overruns += channel->old_n_rx_hw_drop_overruns; }
@@ -707,10 +715,12 @@ static void efx_get_base_stats(struct net_device *net_dev, net_dev->real_num_tx_queues || tx_queue->xdp_tx) { tx->packets += tx_queue->tx_packets; + tx->bytes += tx_queue->tx_bytes; tx->hw_gso_packets += tx_queue->tso_bursts; tx->hw_gso_wire_packets += tx_queue->tso_packets; } else { tx->packets += tx_queue->old_tx_packets; + tx->bytes += tx_queue->old_tx_bytes; tx->hw_gso_packets += tx_queue->old_tso_bursts; tx->hw_gso_wire_packets += tx_queue->old_tso_packets; }
drivers/net/ethernet/sfc/net_driver.h +10 -0
@@ -233,7 +233,11 @@ struct efx_tx_buffer {* @cb_packets: Number of times the TX copybreak feature has been used * @notify_count: Count of notified descriptors to the NIC * @tx_packets: Number of packets sent since this struct was created +* @tx_bytes: Number of bytes sent since this struct was created. For TSO, +* counts the superframe size, not the sizes of generated frames on the +* wire (i.e. the headers are only counted once) * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue() +* @old_tx_bytes: Value of @tx_bytes as of last efx_init_tx_queue() * @old_tso_bursts: Value of @tso_bursts as of last efx_init_tx_queue() * @old_tso_packets: Value of @tso_packets as of last efx_init_tx_queue() * @empty_read_count: If the completion path has seen the queue as empty
@@ -285,7 +289,9 @@ struct efx_tx_queue {unsigned int notify_count;/* Statistics to supplement MAC stats */ unsigned long tx_packets; +unsigned long tx_bytes;unsigned long old_tx_packets; +unsigned long old_tx_bytes;unsigned int old_tso_bursts;unsigned int old_tso_packets;
@@ -378,7 +384,9 @@ struct efx_rx_page_state {* @slow_fill: Timer used to defer efx_nic_generate_fill_event(). * @grant_work: workitem used to grant credits to the MAE if @grant_credits * @rx_packets: Number of packets received since this struct was created +* @rx_bytes: Number of bytes received since this struct was created * @old_rx_packets: Value of @rx_packets as of last efx_init_rx_queue() +* @old_rx_bytes: Value of @rx_bytes as of last efx_init_rx_queue() * @xdp_rxq_info: XDP specific RX queue information. * @xdp_rxq_info_valid: Is xdp_rxq_info valid data?. */
@@ -415,7 +423,9 @@ struct efx_rx_queue {struct work_struct grant_work;/* Statistics to supplement MAC stats */ unsigned long rx_packets; +unsigned long rx_bytes;unsigned long old_rx_packets; +unsigned long old_rx_bytes;struct xdp_rxq_info xdp_rxq_info;bool xdp_rxq_info_valid; };
drivers/net/ethernet/sfc/rx.c +1 -0
@@ -393,6 +393,7 @@ void __efx_rx_packet(struct efx_channel *channel) } rx_queue->rx_packets++; + rx_queue->rx_bytes += rx_buf->len;if (!efx_do_xdp (efx, channel, rx_buf, &eh)) goto out;
drivers/net/ethernet/sfc/rx_common.c +1 -0
@@ -242,6 +242,7 @@ void efx_init_rx_queue(struct efx_rx_queue *rx_queue) rx_queue->page_recycle_full =0 ; rx_queue->old_rx_packets = rx_queue->rx_packets; + rx_queue->old_rx_bytes = rx_queue->rx_bytes;/* Initialise limit fields */ max_fill = efx->rxq_entries -EFX_RXD_HEAD_ROOM ;
drivers/net/ethernet/sfc/tx.c +2 -0
@@ -394,6 +394,7 @@ netdev_tx_t __efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb }else { tx_queue->tx_packets++; } + tx_queue->tx_bytes += skb_len;return NETDEV_TX_OK ;
@@ -490,6 +491,7 @@ int efx_xdp_tx_buffers(struct efx_nic *efx, int n, struct xdp_frame **xdpfs, tx_buffer->dma_offset =0 ; tx_buffer->unmap_len = len; tx_queue->tx_packets++; + tx_queue->tx_bytes += len; }/* Pass mapped frames to hardware. */
drivers/net/ethernet/sfc/tx_common.c +1 -0
@@ -87,6 +87,7 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue) tx_queue->completed_timestamp_minor =0 ; tx_queue->old_tx_packets = tx_queue->tx_packets; + tx_queue->old_tx_bytes = tx_queue->tx_bytes; tx_queue->old_tso_bursts = tx_queue->tso_bursts; tx_queue->old_tso_packets = tx_queue->tso_packets;
Jakub Kicinski
maintainerRe: [PATCH v2 net-next 6/6] sfc: add per-queue RX and TX bytes stats
Quoted 1 line · On Thu, 5 Sep 2024 16:41:35 +0100 [email protected] wrote:
* @tx_packets: Number of packets sent since this struct was created
I think it's number of packets "enqueued", but the doc says:
name: tx-packets
doc: |
Number of wire packets successfully sent. Packet is considered to be successfully sent once it is in device memory (usually this means the device has issued a DMA completion for the packet).
Not the end of the world if you prefer to keep as is, but if so maybe just acknowledge in commit message or a code comment that this is not 100% in line with the definition?
Quoted 3 lines
+ * @tx_bytes: Number of bytes sent since this struct was created. For TSO,
+ * counts the superframe size, not the sizes of generated frames on the
+ * wire (i.e. the headers are only counted once)
Hm. Hm. This is technically not documented but my intuition is that tx_bytes should count wire bytes. tx_packets counts segments / wire packets, looking at ef100_tx.c
qstats "bytes" should be the same kind of bytes as counted by the MAC. That way we can hopefully see how many packets "enter" the device from queues, and how many "leave" via the MAC. Helping to calculate drops at various stages. That matters more for packets than bytes, but still..
Signature · 1 line
pw-bot: cr
Edward Cree
maintainerRe: [PATCH v2 net-next 6/6] sfc: add per-queue RX and TX bytes stats
Quoted 4 lines · Jakub Kicinski
Quoted 1 line · On Thu, 5 Sep 2024 16:41:35 +0100 [email protected] wrote:
* @tx_packets: Number of packets sent since this struct was created
I think it's number of packets "enqueued",
You're correct.
Quoted 7 lines
but the doc says:
name: tx-packets
doc: |
Number of wire packets successfully sent. Packet is considered to be successfully sent once it is in device memory (usually this means the device has issued a DMA completion for the packet).
Fair point. We *do* have tx_queue->pkts_compl but that's reset every
NAPI poll — it exists for BQL's sake. That said, if it's the completions you want to count why isn't there just a hook in BQL to provide those stats automatically without driver involvement?
Quoted 3 lines
Not the end of the world if you prefer to keep as is, but if so maybe just acknowledge in commit message or a code comment that this is not 100% in line with the definition?
I think it's probably better if I change the code to match the doc.
Quoted 8 lines
Quoted 3 lines
+ * @tx_bytes: Number of bytes sent since this struct was created. For TSO,
+ * counts the superframe size, not the sizes of generated frames on the
+ * wire (i.e. the headers are only counted once)Hm. Hm. This is technically not documented but my intuition is that
tx_bytesshould count wire bytes.tx_packetscounts segments / wire packets, looking atef100_tx.c
qstats "bytes" should be the same kind of bytes as counted by the MAC.
Well, even if we calculated the wire bytes, the figures still wouldn't
match entirely because the MAC counts the FCS, which isn't included here. We can add that in too, but then one would expect the same thing on RX, which would require an extra branch in the datapath checking NETIF_F_RXFCS and I didn't want to take that performance hit.
So my preference here would be to keep this as skb bytes rather than
wire bytes, since as you say it's the packet count that really matters here.
Jakub Kicinski
maintainerRe: [PATCH v2 net-next 6/6] sfc: add per-queue RX and TX bytes stats
Quoted 9 lines · On Tue, 10 Sep 2024 16:03:04 +0100 Edward Cree wrote:
Quoted 8 lines
Quoted 3 lines
+ * @tx_bytes: Number of bytes sent since this struct was created. For TSO,
+ * counts the superframe size, not the sizes of generated frames on the
+ * wire (i.e. the headers are only counted once)Hm. Hm. This is technically not documented but my intuition is that
tx_bytesshould count wire bytes.tx_packetscounts segments / wire packets, looking atef100_tx.c
qstats "bytes" should be the same kind of bytes as counted by the MAC.
Quoted 8 lines
Well, even if we calculated the wire bytes, the figures still wouldn't
match entirely because the MAC counts the FCS, which isn't included here. We can add that in too, but then one would expect the same thing on RX, which would require an extra branch in the datapath checkingNETIF_F_RXFCSand I didn't want to take that performance hit.
So my preference here would be to keep this as skb bytes rather than
wire bytes, since as you say it's the packet count that really matters here.
Right, that's fine. But just to state the obvious - adding / subtracting FCS bytes is relatively easy for user space to do (assuming RXFCS handling is correct, as you mention). Converting from LSO bytes to wire bytes is impossible, FCS is fixed size while header length varies.