Re: [PATCH] batman-adv: fix throughput detection for VLAN interfaces

Sven Eckelmann <[email protected]> Sun, 05 Jul 2026 14:41:51 +0200
Newsgroups org.open-mesh.lists.batman,org.kernel.vger.linux-kernel
Message-ID <178325531132.27339.5219063208417341297.b4-review@b4>
On Sun, 05 Jul 2026 17:51:34 +0530, Amitesh Singh <[email protected]> wrote:
> batadv_v_elp_get_throughput() calls __ethtool_get_link_ksettings() to
> determine the link speed of an Ethernet interface. Virtual interfaces
> such as a VLAN on a bridge return success but with SPEED_UNKNOWN, so the
> function falls through to the hardcoded default even when the underlying
> physical device has a known speed.
> 
> Move rtnl_unlock() into each exit branch so the lock is still held
> after the initial query. When the speed is unknown, walk the lower
> device stack with netdev_walk_all_lower_dev() to find the first real
> device that reports a valid speed, which covers DSA ports and physical
> NICs sitting below a VLAN or bridge interface.

Has to be reviewed by someone with B.A.T.M.A.N. V expertise, but:

What makes you think that the found port is then the port something is sent 
out (towards this specific neighbor)? The neighbor could be behind a 10Mbit/
Half port and you just found a 25000Mbit/Full port and stopped
netdev_walk_all_lower_dev

>
>
> diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
> index 6ad6042a..3d4d82c5 100644
> --- a/net/batman-adv/bat_v_elp.c
> +++ b/net/batman-adv/bat_v_elp.c
> @@ -70,6 +70,32 @@ static void batadv_v_elp_start_timer(struct batadv_hard_iface *hard_iface)
> [ ... skip 23 lines ... ]
> +		return 0;
> +
> +	*(u32 *)priv->data = speed * 10;
> +	return 1;
> +}
> +

Duplex handling was not copied to this function

> @@ -177,9 +202,30 @@ static bool batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh,
> [ ... skip 14 lines ... ]
> +			struct netdev_nested_priv priv = { .data = &throughput };
> +
> +			netdev_walk_all_lower_dev(hard_iface->net_dev,
> +						  batadv_v_elp_lower_dev_throughput,
> +						  &priv);
> +		}

Why this extra scope?

> +		rtnl_unlock();
> +		if (throughput) {
> +			*pthroughput = throughput;
> +			return true;
> +		}

Sashiko review:
https://sashiko.dev/#/patchset/20260705122134.62809-1-singh.amitesh%40gmail.com

-- 
Sven Eckelmann <[email protected]>