[Intel-wired-lan] [PATCH iwl-next 9/9] igc: Add support to get frame preemption statistics via ethtool

Abdul Rahim, Faizal faizal.abdul.rahim at linux.intel.com
Mon Dec 23 09:52:37 UTC 2024



On 17/12/2024 12:05 am, Vladimir Oltean wrote:
> On Mon, Dec 16, 2024 at 01:47:20AM -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
>> 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 0 1 2 3 0 0 0 0 0 0 0 0 0 0 0 0 \
>>        queues 1 at 0 1 at 1 1 at 2 1 at 3 base-time 0 sched-entry S F 100000 \
>>        fp E E P P
> 
> Hmm, the prio_tc_map pattern changed since the last time I looked at igc
> examples? It was in decreasing order before? How do you handle backwards
> compatibility with the Tx ring strict priority default configuration?
> I haven't downloaded the entire set locally, will do so later.
> 

I tested like this for i226:
     CMD=(
         "tc qdisc replace dev $IFACE 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 at 0 1 at 1 1 at 2 1 at 3 "
         "base-time $BASE "
         "${SCHED_ENTRY[*]} "
         "flags 0x1 "
         "txtime-delay $TXTIME_DELAY "
         "clockid CLOCK_TAI "

But I mistakenly copied the mapping from a different scenario where socket 
priority -> tc -> hw_queue mapping is not important to my test objective in 
that scenario.

I'll update the description to use decreasing order then.

>> +
>> +	return ooo_smdc + ooo_frame_cnt + ooo_frag_cnt + miss_frame_frag_cnt;
>> +}
>> +
>> +static void igc_ethtool_get_mm_stats(struct net_device *dev,
>> +				     struct ethtool_mm_stats *stats)
>> +{
>> +	struct igc_adapter *adapter = netdev_priv(dev);
>> +	struct igc_hw *hw = &adapter->hw;
>> +
>> +	stats->MACMergeFrameAssErrorCount = igc_ethtool_get_frame_ass_error(dev);
>> +	stats->MACMergeFrameSmdErrorCount = 0; /* Not available in IGC */
>> +	stats->MACMergeFrameAssOkCount = rd32(IGC_PRMPTDRCNT);
>> +	stats->MACMergeFragCountRx =  rd32(IGC_PRMEVNTRCNT);
>> +	stats->MACMergeFragCountTx = rd32(IGC_PRMEVNTTCNT);
>> +	stats->MACMergeHoldCount = 0; /* Not available in IGC */
> 
> Don't report counters as zero when in reality you don't know.
> 
> Just don't assign values to these. mm_prepare_data() -> ethtool_stats_init()
> presets them to 0xffffffffffffffff (ETHTOOL_STAT_NOT_SET), and
> mm_put_stats() -> mm_put_stat() detects whether they are still equal to
> this value, and if they are, does not report netlink attributes for them.
> 

Got it.


>> diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
>> index 12ddc5793651..f40946cce35a 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_regs.h
>> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
>> @@ -222,6 +222,25 @@
>>   
>>   #define IGC_FTQF(_n)	(0x059E0 + (4 * (_n)))  /* 5-tuple Queue Fltr */
>>   
>> +/* Time sync registers - preemption statistics */
>> +#define IGC_PRMEVNTTCNT		0x04298	/* TX Preemption event counter */
>> +#define IGC_PRMEVNTRCNT		0x0429C	/* RX Preemption event counter */
>> +#define IGC_PRMPTDRCNT		0x04284	/* Good RX Preempted Packets */
>> +
>> + /* Preemption Exception Counter */
>> +#define IGC_PRMEXPRCNT					0x042A0
>> +/* Received out of order packets with SMD-C and NOT ReumeRx */
>> +#define IGC_PRMEXPRCNT_OOO_SMDC 0x000000FF
>> +/* Received out of order packets with SMD-C and wrong Frame CNT */
>> +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT			0x0000FF00
>> +#define IGC_PRMEXPRCNT_OOO_FRAME_CNT_SHIFT		8
>> +/* Received out of order packets with SMD-C and wrong Frag CNT */
>> +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT			0x00FF0000
>> +#define IGC_PRMEXPRCNT_OOO_FRAG_CNT_SHIFT		16
>> +/* Received packets with SMD-S and ReumeRx */
> 
> What is ReumeRx?

Resume receive. It’s a typo in the i226 documentation that I (shamelessly) 
copied into the code without checking properly. It's meant to indicate that 
an RX flag in the i226 firmware is set. I’ll remove the 'ResumeRX' part 
since it adds confusion.



More information about the Intel-wired-lan mailing list