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.
Best regards, Ilya Maximets.
> VLOG_DEFINE_THIS_MODULE(netdev_linux);
>
> COVERAGE_DEFINE(netdev_set_policing);
> @@ -2673,9 +2684,9 @@ netdev_linux_read_features(struct netdev_linux *netdev)
> netdev->current = ecmd.duplex ? NETDEV_F_1GB_FD : NETDEV_F_1GB_HD;
> } else if (netdev->current_speed == SPEED_10000) {
> netdev->current = NETDEV_F_10GB_FD;
> - } else if (netdev->current_speed == 40000) {
> + } else if (netdev->current_speed == SPEED_40000) {
> netdev->current = NETDEV_F_40GB_FD;
> - } else if (netdev->current_speed == 100000) {
> + } else if (netdev->current_speed == SPEED_100000) {
> netdev->current = NETDEV_F_100GB_FD;
> } else if (netdev->current_speed == 1000000) {
> netdev->current = NETDEV_F_1TB_FD;
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
