On Fri, Feb 7, 2025 at 11:46 AM Breno Leitao [off-list ref] wrote:

Hello Kuniyuki,

On Fri, Feb 07, 2025 at 12:38:22PM +0900, Kuniyuki Iwashima wrote:

quoted

From: Breno Leitao <leitao@debian.org>
Date: Thu, 6 Feb 2025 07:51:55 -0800

quoted

quoted

Are there better approaches to silence these warnings when RTNL is held?
Any suggestions would be appreciated.

We can't use lockdep_rtnl_net_is_held() there yet because most users are
not converted to per-netns RTNL, so it will complain loudly.

Right, so, I understand the best approach is to leverage
lockdep_rtnl_is_held() only right now. Something as:

        diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
        index 1dcc76af75203..0deee1313f23a 100644
        --- a/include/linux/netdevice.h
        +++ b/include/linux/netdevice.h
        @@ -3217,7 +3217,8 @@ int call_netdevice_notifiers_info(unsigned long val,
        #define for_each_netdev_reverse(net, d)        \
                        list_for_each_entry_reverse(d, &(net)->dev_base_head, dev_list)
        #define for_each_netdev_rcu(net, d)            \
        -               list_for_each_entry_rcu(d, &(net)->dev_base_head, dev_list)
        +               list_for_each_entry_rcu(d, &(net)->dev_base_head, dev_list, \
        +                                       lockdep_rtnl_is_held())
        #define for_each_netdev_safe(net, d, n)        \
                        list_for_each_entry_safe(d, n, &(net)->dev_base_head, dev_list)
        #define for_each_netdev_continue(net, d)               \

Which brings another problem:

lockdep_rtnl_is_held() is defined in include/linux/rtnetlink.h, so,
we'll need to include 'linux/rtnetlink.h' in linux/netdevice.h, which
doesn't seem correct (!?).

Otherwise drivers using for_each_netdev_rcu() will not be able to find
lockdep_rtnl_is_held().

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

Note that we have different accessors like rtnl_dereference() and
rcu_dereference_rtnl()
It helps to differentiate expectations, and as self describing code.

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

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