[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <57139951-daf7-42c3-b7a6-e4870a3f71fa@linux.intel.com>
Date: Mon, 23 Dec 2024 17:52:37 +0800
From: "Abdul Rahim, Faizal" <faizal.abdul.rahim@...ux.intel.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S . Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>,
intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH iwl-next 9/9] igc: Add support to get frame preemption
statistics via ethtool
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@0 1@1 1@2 1@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@0 1@1 1@2 1@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.
Powered by blists - more mailing lists