On 6/2/2025 11:04 pm, Vladimir Oltean wrote:
On Thu, Feb 06, 2025 at 10:40:11PM +0800, Abdul Rahim, Faizal wrote:
Hi Vladimir,
Thanks for the quick review, appreciate your help.
On 6/2/2025 1:12 am, Vladimir Oltean wrote:
On Wed, Feb 05, 2025 at 05:05:20AM -0500, Faizal Rahim wrote:
This patch implements the "ethtool --set-mm" callback to trigger the
frame preemption verification handshake.
Uses the MAC Merge Software Verification (mmsv) mechanism in ethtool
to perform the verification handshake for igc.
The structure fpe.mmsv is set by mmsv in ethtool and should remain
read-only for the driver.
igc does not use two mmsv callbacks:
a) configure_tx()
- igc lacks registers to configure FPE in the transmit direction.
Yes, maybe, but it's still important to handle this. It tells you when
the preemptible traffic classes should be sent as preemptible on the wire
(i.e. when the verification is either disabled, or it succeeded).
There is a selftest called manual_failed_verification() which supposedly
tests this exact condition: if verification fails, then packets sent to
TC0 are supposed to bump the eMAC's TX counters, even though TC0 is
configured as preemptible. Otherwise stated: even if the tc program says
that a certain traffic class is preemptible, you don't want to actually
send preemptible packets if you haven't verified the link partner can
handle them, since it will likely drop them on RX otherwise.
Even though fpe in tx direction isn't set in igc, it still checks
ethtool_mmsv_is_tx_active() before setting a queue as preemptible.
This is done in :
igc_tsn_enable_offload(struct igc_adapter *adapter) {
{
....
if (ethtool_mmsv_is_tx_active(&adapter->fpe.mmsv) &&
ring->preemptible)
txqctl |= IGC_TXQCTL_PREEMPTIBLE;
Wouldn't this handle the situation mentioned ?
Sorry if I miss something here.
And what if tx_active becomes true after you had already configured the
queues with tc (and the above check caused IGC_TXQCTL_PREEMPTIBLE to not
be set)? Shouldn't you set IGC_TXQCTL_PREEMPTIBLE now? Isn't
ethtool_mmsv_configure_tx() exactly the function that notifies you of
changes to tx_active, and hence, aren't you interested in setting up a
callback for it?
Ahh okay, got it. I sent v3 that also included this update. Thanks!
