Re: [PATCH iwl-next v4 9/9] igc: Add support to get frame preemption statistics via ethtool

Vladimir Oltean
2025-02-12T21:54:16Z

On Mon, Feb 10, 2025 at 02:02:07AM -0500, Faizal Rahim wrote:
> Implemented "ethtool --include-statistics --show-mm" callback for IGC.
>
> Tested preemption scenario to check preemption statistics:
> 1) Trigger verification handshake on both boards:
> $ sudo ethtool --set-mm enp1s0 pmac-enabled on
> $ sudo ethtool --set-mm enp1s0 tx-enabled on
> $ sudo ethtool --set-mm enp1s0 verify-enabled on

For the record, all these can be enabled at the same time through a
single ethtool command.

> 2) Set preemptible or express queue in taprio for tx board:
> $ sudo tc qdisc replace dev enp1s0 parent root handle 100 taprio \
> num_tc 4 map 3 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 \
> queues 1@0 1@1 1@2 1@3 base-time 0 sched-entry S F 100000 \
> fp E E P P
> 3) Send large size packets on preemptible queue
> 4) Send small size packets on express queue to preempt packets in
> preemptible queue
> 5) Show preemption statistics on the receiving board:
> $ ethtool --include-statistics --show-mm enp1s0
> MAC Merge layer state for enp1s0:
> pMAC enabled: on
> TX enabled: on
> TX active: on
> TX minimum fragment size: 64
> RX minimum fragment size: 60
> Verify enabled: on
> Verify time: 128
> Max verify time: 128
> Verification status: SUCCEEDED
> Statistics:
> MACMergeFrameAssErrorCount: 0
> MACMergeFrameSmdErrorCount: 0
> MACMergeFrameAssOkCount: 511
> MACMergeFragCountRx: 764
> MACMergeFragCountTx: 0
> MACMergeHoldCount: 0

nitpick: mix of tabs and spaces.

> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index f15ac7565fbd..cd5160315993 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3076,6 +3076,7 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget)
> break;
>
> if (static_branch_unlikely(&igc_fpe_enabled) &&
> + adapter->fpe.mmsv.pmac_enabled &&

This bit is misplaced in this patch.
Also, both conditions, being repeated twice as they are, could be placed
in an igc_pmac_enabled() helper function or similar.

> igc_fpe_transmitted_smd_v(tx_desc))
> ethtool_mmsv_event_handle(&adapter->fpe.mmsv,
> ETHTOOL_MMSV_LD_SENT_VERIFY_MPACKET);