On Mon, 6 Oct 2025 at 20:10, Ilya Maximets <[email protected]> wrote:
>
> On 9/16/25 3:56 PM, David Marchand wrote:
> > SPEED_UNKNOWN was introduced in Linux kernel v3.2.
> >
> > While at it, add some defines for 40G and 100G that were added in more
> > recent kernels.
> >
> > Fixes: 6240c0b4c80e ("netdev: Add netdev_get_speed() to netdev API.")
> > Signed-off-by: David Marchand <[email protected]>
> > Acked-by: Mike Pattrick <[email protected]>
> > ---
> >  lib/netdev-linux.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
>
> Thanks, David!  This change mostly looks good to me, see a couple
> comments below.
>
> >
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > index c383663dde..2147659485 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -80,6 +80,17 @@
> >  #include "userspace-tso.h"
> >  #include "util.h"
> >
> > +/* For older kernels that don't define those macros in linux/ethtool.h. */
> > +#ifndef SPEED_UNKNOWN
> > +#define SPEED_UNKNOWN -1
> > +#endif
> > +#ifndef SPEED_40000
> > +#define SPEED_40000 40000
> > +#endif
> > +#ifndef SPEED_100000
> > +#define SPEED_100000 100000
> > +#endif
> > +
>
> Hmm, it doesn't look right to have these defined above the logging and
> the coverage.  I'd suggest to move the definitions a bit lower in the
> code, maybe close to the re-definitions of SUPPORTED_*/ADVERTISED_*.
>
> It would also be good to add the kernel versions when these were introduced.
> The reason is to make things easier if we'll decide to drop support for
> some older kernels, so we can just read the code and remove definitions
> that are no longer necessary.

I had not noticed those other compat macros, ack, will do.


-- 
David Marchand

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to