On Mon, Dec 19, 2022 at 01:54:02PM +0200, Vladimir Oltean wrote:
Hi Sasha,
On Sun, Dec 18, 2022 at 11:12:39AM -0500, Sasha Levin wrote:
From: Vladimir Oltean <vladimir.oltean@xxxxxxx>
[ Upstream commit 29811d6e19d795efcf26644b66c4152abbac35a6 ]
DPNIs and DPSW objects can connect and disconnect at runtime from DPMAC
objects on the same fsl-mc bus. The DPMAC object also holds "ethtool -S"
unstructured counters. Those counters are only shown for the entity
owning the netdev (DPNI, DPSW) if it's connected to a DPMAC.
The ethtool stringset code path is split into multiple callbacks, but
currently, connecting and disconnecting the DPMAC takes the rtnl_lock().
This blocks the entire ethtool code path from running, see
ethnl_default_doit() -> rtnl_lock() -> ops->prepare_data() ->
strset_prepare_data().
This is going to be a problem if we are going to no longer require
rtnl_lock() when connecting/disconnecting the DPMAC, because the DPMAC
could appear between ops->get_sset_count() and ops->get_strings().
If it appears out of the blue, we will provide a stringset into an array
that was dimensioned thinking the DPMAC wouldn't be there => array
accessed out of bounds.
There isn't really a good way to work around that, and I don't want to
put too much pressure on the ethtool framework by playing locking games.
Just make the DPMAC counters be always available. They'll be zeroes if
the DPNI or DPSW isn't connected to a DPMAC.
Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
Reviewed-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
Tested-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
I think the algorithm has a problem in that it has a tendency to
auto-pick preparatory patches which eliminate limitations that are
preventing future development from taking place, rather than patches
which fix present issues in the given code base.
Yeah, I'd agree. I think that the tricky part is that preperatory
patches usually resolve an issue, but it's not clear whether it's
something that affects users, or is just a theoretical limitation needed
by future patches.
In this case, the patch is part of a larger series which was at the
boundary between "next" work and "stable" work (patch 07/12 of this)
https://patchwork.kernel.org/project/netdevbpf/cover/20221129141221.872653-1-vladimir.oltean@xxxxxxx/
Due to the volume of that rework, I intended it to go to "next", even
though backporting the entire series to "stable" could have its own
merits. But picking just patch 07/12 out of that series is pointless,
so please drop this patch from the queue for 5.15, 6.0 and 6.1, please.
Now dropped, thanks!
--
Thanks,
Sasha
