Re: [PATCH net 1/1] net: ethtool: mm: Allow Verify Enabled before Tx Enabled

[Vladimir Oltean]

Thanks for copying me, Furong.

On Wed, Jan 15, 2025 at 05:42:04PM +0800, Furong Xu wrote:
> On Wed, 15 Jan 2025 14:59:31 +0800, Chwee-Lin Choong <chwee.lin.choong@xxxxxxxxx> wrote:
> 
> > The current implementation of ethtool --set-mm restricts
> > enabling the "verify_enabled" flag unless Tx preemption
> > (tx_enabled) is already enabled. By default, verification
> > is disabled, and enabling Tx preemption immediately activates
> > preemption.
> > 
> > When verification is intended, users can only enable verification
> > after enabling tx_enabled, which temporarily deactivates preemption
> > until verification completes. This creates an inconsistent and
> > restrictive workflow.

Where the premise of the patch is wrong is here. Users don't have to
enable verification _after_ enabling TX. They can also enable
verification _at the same time_ as TX, aka within the same netlink
message. They just can't enable TX verification while TX in general is
disabled. It just doesn't make sense.

> > This patch modifies ethtool --set-mm to allow users to pre-enable
> > verification locally using ethtool before Tx preemption is enabled
> > via ethtool or negotiated through LLDP with a link partner.
> > 
> > Current Workflow:
> > 1. Enable pmac_enabled → Preemption supported
> > 2. Enable tx_enabled → Preemption Tx enabled
> > 3. verify_enabled defaults to off → Preemption active
> > 4. Enable verify_enabled → Preemption deactivates → Verification starts
> >                          → Verification success → Preemption active.
> > 
> > Proposed Workflow:
> > 1. Enable pmac_enabled → Preemption supported
> > 2. Enable verify_enabled → Preemption supported and Verify enabled
> > 3. Enable tx_enabled → Preemption Tx enabled → Verification starts
> >                      → Verification success → Preemption active.
> > 
> 
> Maybe you misunderstand the parameters of ethtool --set-mm.
> 
> tools/testing/selftests/drivers/net/hw/ethtool_mm.sh will help you :)

Yes, see manual_with_verification() there:

ethtool --set-mm $tx verify-enabled on tx-enabled on