[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1a7f0aa0-47ae-4936-9e55-576cdf71f4cc@linux.dev>
Date: Tue, 29 Jul 2025 17:01:06 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Andrew Lunn <andrew@...n.ch>
Cc: Michael Chan <michael.chan@...adcom.com>,
Pavan Chebbi <pavan.chebbi@...adcom.com>, Tariq Toukan <tariqt@...dia.com>,
Gal Pressman <gal@...dia.com>, intel-wired-lan@...ts.osuosl.org,
Donald Hunter <donald.hunter@...il.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
netdev@...r.kernel.org
Subject: Re: [RFC PATCH] ethtool: add FEC bins histogramm report
On 29/07/2025 14:48, Andrew Lunn wrote:
>> + name: fec-hist-bin-low
>> + type: s32
>
> Signed 32 bit
>
>> +struct ethtool_fec_hist_range {
>> + s16 low;
>
> Signed 16 bit.
>
>> + if (nla_put_u32(skb, ETHTOOL_A_FEC_STAT_FEC_HIST_BIN_LOW,
>> + ranges[i].low) ||
>
> Unsigned 32 bit.
>
> Could we have some consistency with the types.
Yeah, it looks a bit messy. AFAIK, any type of integer less than 32 bits
will be extended to 32 bits anyway, so I believe it's ok to keep smaller
memory footprint for the histogram definition in the driver but still
use s32 as netlink attr type. I'll change the code to use nla_put_s32()
to keep sign info.
Does it look OK?
>
> Andrew
Powered by blists - more mailing lists