On Tue, Jul 29, 2025 at 05:01:06PM +0100, Vadim Fedorenko wrote:
> 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,
sign extended, not just extended. That makes things more fun.
> so I believe it's ok to keep smaller
> memory footprint
.../net/ethernet/broadcom/bnxt/bnxt_ethtool.c
.../ethernet/fungible/funeth/funeth_ethtool.c
.../ethernet/hisilicon/hns3/hns3_ethtool.c
drivers/net/ethernet/intel/ice/ice_ethtool.c
.../marvell/octeontx2/nic/otx2_ethtool.c
.../ethernet/mellanox/mlx5/core/en_ethtool.c
drivers/net/ethernet/sfc/ethtool.c
drivers/net/ethernet/sfc/siena/ethtool.c
These are all huge drivers, with extensive memory footprint. How many
bins are we talking about? 5? One per PCS? I suspect the size
difference it deep in the noise.
> 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.
So bins can have negative low/high values?
Andrew
