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

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

[Chwee-Lin Choong]

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.

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.

Fixes: 35b288d6e3d4 ("net: ethtool: mm: sanitize some UAPI configurations")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chwee-Lin Choong <chwee.lin.choong@intel.com>
Reviewed-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
Reviewed-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
---
 net/ethtool/mm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ethtool/mm.c b/net/ethtool/mm.c
index 2816bb23c3ad..8a66ea3148d1 100644
--- a/net/ethtool/mm.c
+++ b/net/ethtool/mm.c
@@ -214,8 +214,8 @@ static int ethnl_set_mm(struct ethnl_req_info *req_info, struct genl_info *info)
 		return -ERANGE;
 	}
 
-	if (cfg.verify_enabled && !cfg.tx_enabled) {
-		NL_SET_ERR_MSG(extack, "Verification requires TX enabled");
+	if (cfg.verify_enabled && !cfg.pmac_enabled) {
+		NL_SET_ERR_MSG(extack, "Verify enabled requires pMAC enabled");
 		return -EINVAL;
 	}
 
-- 
2.34.1

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

[Furong Xu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB18030, Size: 1521 bytes --]

On Wed, 15 Jan 2025 14:59:31 +0800, Chwee-Lin Choong <chwee.lin.choong@intel.com> 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.
> 
> 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 :)

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@intel.com> 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

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

[Choong, Chwee Lin]

On Wednesday, January 15, 2025 5:49 PM, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>Thanks for copying me, Furong.

Hi Vladimir,

>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@intel.com> 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.
>
The intent of this patch is tied to the earlier discussion about avoiding LLDP operations 
that forcefully enable verification whenever preemption support is advertised by the link partner.

Since we are currently seeking clarification from Avnu on their test requirements regarding 
scenarios with LLDP + verification disabled, I’ll provide an update once we hear back from Avnu.

>> > 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

Thanks,
CL