Re: [PATCH v2] ethdev: add Linux ethtool link mode conversion

[Thomas Monjalon <thomas@monjalon.net>]

29/02/2024 18:38, Stephen Hemminger:
> On Thu, 29 Feb 2024 17:58:13 +0100
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Thursday, 29 February 2024 17.45
> > > 
> > > On Thu, 29 Feb 2024 16:42:56 +0100
> > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > >   
> > > > +/* Link modes sorted with index as defined in ethtool.
> > > > + * Values are speed in Mbps with LSB indicating duplex.
> > > > + *
> > > > + * The ethtool bits definition should not change as it is a kernel  
> > > API.  
> > > > + * Using raw numbers directly avoids checking API availability
> > > > + * and allows to compile with new bits included even on an old  
> > > kernel.  
> > > > + *
> > > > + * The array below is built from bit definitions with this shell  
> > > command:  
> > > > + *   sed -rn 's;.*(ETHTOOL_LINK_MODE_)([0-9]+)([0-9a-zA-Z_]*).*=  
> > > *([0-9]*).*;'\  
> > > > + *           '[\4] = \2, /\* \1\2\3 *\/;p'  
> > > /usr/include/linux/ethtool.h |  
> > > > + *   awk '/_Half_/{$3=$3+1","}1'
> > > > + */
> > > > +static uint32_t link_modes[] = {  
> > > 
> > > Make it const please.

Yes


> > > You could add meson rule to generate it and then use non-numeric tags.  
> > 
> > However you do it, make sure it cross builds. The kernel/ethtool on the target system may differ from the one on the build system.
> > 
> 
> If the build system is older, the speed table will be smaller. And the code should just print "Unknown"
> If the build system is newer, then the table will be larger than kernel ever returns which is Ok.

There is no benefit in having a smaller table.
That's why I prefer using numeric indices with the best coverage possible.