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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250115094839.xjudiq2japopdqza@skbuf>
Date: Wed, 15 Jan 2025 11:48:39 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Furong Xu <0x1207@...il.com>
Cc: Chwee-Lin Choong <chwee.lin.choong@...el.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 1/1] net: ethtool: mm: Allow Verify Enabled before Tx
 Enabled

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@...el.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