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]>