linux-kernel - Re: [PATCH net-next v21 02/14] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <49709595-5012-4fa3-9616-839dcdbf6b09@bootlin.com>
Date: Tue, 2 Dec 2025 16:48:39 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Paolo Abeni <pabeni@...hat.com>, davem@...emloft.net
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, thomas.petazzoni@...tlin.com,
 Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
 Eric Dumazet <edumazet@...gle.com>, Russell King <linux@...linux.org.uk>,
 linux-arm-kernel@...ts.infradead.org,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Herve Codina <herve.codina@...tlin.com>,
 Florian Fainelli <f.fainelli@...il.com>,
 Heiner Kallweit <hkallweit1@...il.com>,
 Vladimir Oltean <vladimir.oltean@....com>,
 Köry Maincent <kory.maincent@...tlin.com>,
 Marek Behún <kabel@...nel.org>,
 Oleksij Rempel <o.rempel@...gutronix.de>,
 Nicolò Veronese <nicveronese@...il.com>,
 Simon Horman <horms@...nel.org>, mwojtas@...omium.org,
 Antoine Tenart <atenart@...nel.org>, devicetree@...r.kernel.org,
 Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
 Romain Gantois <romain.gantois@...tlin.com>,
 Daniel Golle <daniel@...rotopia.org>,
 Dimitri Fedrau <dimitri.fedrau@...bherr.com>,
 Tariq Toukan <tariqt@...dia.com>
Subject: Re: [PATCH net-next v21 02/14] net: ethtool: Introduce
 ETHTOOL_LINK_MEDIUM_* values

Hi Paolo

On 02/12/2025 14:03, Paolo Abeni wrote:
> On 11/29/25 9:22 AM, Maxime Chevallier wrote:
>> @@ -298,138 +321,149 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
>>  		.speed	= SPEED_UNKNOWN, \
>>  		.lanes	= 0, \
>>  		.duplex	= DUPLEX_UNKNOWN, \
>> +		.mediums = BIT(ETHTOOL_LINK_MEDIUM_NONE), \
>>  	}
>>  
>>  const struct link_mode_info link_mode_params[] = {
>> -	__DEFINE_LINK_MODE_PARAMS(10, T, Half),
>> -	__DEFINE_LINK_MODE_PARAMS(10, T, Full),
>> -	__DEFINE_LINK_MODE_PARAMS(100, T, Half),
>> -	__DEFINE_LINK_MODE_PARAMS(100, T, Full),
>> -	__DEFINE_LINK_MODE_PARAMS(1000, T, Half),
>> -	__DEFINE_LINK_MODE_PARAMS(1000, T, Full),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(10, T, 2, 4, Half, T),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(10, T, 2, 4, Full, T),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(100, T, 2, 4, Half, T),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(100, T, 2, 4, Full, T),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(1000, T, 4, 4, Half, T),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(1000, T, 4, 4, Full, T),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(Autoneg),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(TP),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(AUI),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(MII),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(FIBRE),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(BNC),
>> -	__DEFINE_LINK_MODE_PARAMS(10000, T, Full),
>> +	__DEFINE_LINK_MODE_PARAMS_PAIRS(10000, T, 4, 4, Full, T),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(Pause),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause),
>> -	__DEFINE_LINK_MODE_PARAMS(2500, X, Full),
>> +	__DEFINE_LINK_MODE_PARAMS_MEDIUMS(2500, X, Full,
>> +					  __MED(C) | __MED(S) | __MED(L)),
>>  	__DEFINE_SPECIAL_MODE_PARAMS(Backplane),
>> -	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full),
>> -	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full),
>> -	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full),
>> +	__DEFINE_LINK_MODE_PARAMS(1000, KX, Full, K),
>> +	__DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K),
>> +	__DEFINE_LINK_MODE_PARAMS(10000, KR, Full, K),
>>  	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = {
>>  		.speed	= SPEED_10000,
>>  		.lanes	= 1,
>>  		.duplex = DUPLEX_FULL,
> 
> The AI review points that medium is not initialized here:
> 
> https://netdev-ai.bots.linux.dev/ai-review.html?id=437cd013-c6a6-49e1-bec1-de4869930c7a#patch-1
> 
> Is that intentional? It should deserve at least an explanation in the
> commit message.

Yes it is OK, however I don't really know how to answer AI on that. I'm
sorry it's still a bit blurry to me what's the right way to proceed with
these reviews.

Should I paste the AI report, then reply to it ?

I'd rather add more comments to the code than say in my commit log "AI
says xxx, it's wrong because blabla" though.

> 
> Somewhat related, AI raised on the first patch the same question raised
> on a previous iteration, and I assumed you considered that valid,
> according to:
> 
> https://lore.kernel.org/netdev/f753719e-2370-401d-a001-821bdd5ee838@bootlin.com/

So I don't know either how to proceed with this. dt_binding_check is
fine with the current state, and Rob acked the patch. I am not sure how
it's going to be received if I reach out to DT maintainers saying "the
netdev LLM said XXX, is this correct or hallucination ?", but it may
very well have a good point. I did try to dive into the yaml and then
json schema specs, but I wasn't able to go far enough to reach a proper
conclusion on wether we must remove "contains" for scalar :(

All of that would be a bit clearer if the AI review was on the ML, but I
also understand the risk for pollution with that, especially at an early
stage of adoption.

> 
> Otherwise I think some wording in the commit message explaining why the
> AI feedback is incorrect would be useful.
> 
> /P
> 

I'll update the commits and add comments anyways. I guess in a month
though, with net-next closed then the end of year.

Thanks,

Maxime