Hello Eric,

On Fri, Feb 07, 2025 at 11:56:53AM +0100, Eric Dumazet wrote:

quoted

I suppose we will need to move some of definitions around, but, I am
NOT confident in which way.

Note that we have different accessors like rtnl_dereference() and
rcu_dereference_rtnl()

Makes sense. I suppose that would be a for_each_netdev_rtnl().

It helps to differentiate expectations, and as self describing code.

The problem with this approach, is that we don't know what lock the
caller of dev_getbyhwaddr_rcu() is using, thus, we cannot leverage
a possible for_each_netdev_rtnl() inside dev_getbyhwaddr_rcu().

I would not change  for_each_netdev_rcu(), and instead add a new
dev_getbyhwaddr_rtnl()
function for contexts holding RTNL.

Initially, I had reservations about this approach, but after further
consideration, it seems that creating separate variants of
dev_getbyhwaddr() might be the most effective solution.

By doing so, we can introduce dev_getbyhwaddr_rcu() and
dev_getbyhwaddr_rtnl(), each tailored to specific locking mechanisms.

To explore this idea further, I'll create a proof-of-concept
implementation to see how these new functions would look in practice.

This will help us determine whether this approach is indeed the best way
forward. Thanks for the suggestion.

Alternatively, add one rcu_read_lock()/rcu_read_unlock() pair as some
dev_getbyhwaddr_rcu() callers already do.

Fair, we can do that as well, but, it seemed weird to me to have
something like:

	rtnl_lock();
	rcu_read_lock();
	dev_getbyhwaddr_rcu();

Thanks for chiming in
--breno