[PATCH net-next] rtnetlink: comment the RCU and SRCU waits in unregister paths

[PATCH net-next] rtnetlink: comment the RCU and SRCU waits in unregister paths

[Andrea Parri]

rtnl_link_unregister(), rtnl_af_unregister() and rtnl_unregister_all()
wait for readers before tearing down what those readers use, but most
of these waits do not say which readers they are waiting for.

A missing wait is then easy to overlook: rtnl_link_unregister() lacked
its synchronize_rcu() until commit 3b70edea393d ("rtnetlink: wait for
RCU readers in rtnl_link_unregister()"). Add a comment to each wait
that lacks one, naming the readers it waits for.

Suggested-by: Eric Dumazet <edumazet@kernel.org>
Link: https://lore.kernel.org/CAL4WiioFMQN4troVjQjCmE6Z9kwwb95SnoEupjD=PPSK6Q5f6w@mail.gmail.com
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 net/core/rtnetlink.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 15bffafbf0a9c..80f8577816760 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -526,6 +526,9 @@ void rtnl_unregister_all(int protocol)
 	}
 	rtnl_unlock();
 
+	/* Wait for rtnl_get_link() callers under RCU that may still
+	 * dereference tab.
+	 */
 	synchronize_net();
 
 	kfree(tab);
@@ -711,6 +714,7 @@ void rtnl_link_unregister(struct rtnl_link_ops *ops)
 	 * not yet taken ops->srcu.
 	 */
 	synchronize_rcu();
+	/* Wait for rtnl_link_ops_get() callers to call rtnl_link_ops_put(). */
 	synchronize_srcu(&ops->srcu);
 	cleanup_srcu_struct(&ops->srcu);
 
@@ -843,7 +847,11 @@ void rtnl_af_unregister(struct rtnl_af_ops *ops)
 	list_del_rcu(&ops->list);
 	spin_unlock(&rtnl_af_ops_lock);
 
+	/* Wait for RCU walkers of rtnl_af_ops, including rtnl_af_lookup()
+	 * readers that found ops but have not yet taken ops->srcu.
+	 */
 	synchronize_rcu();
+	/* Wait for rtnl_af_lookup() callers to call rtnl_af_put(). */
 	synchronize_srcu(&ops->srcu);
 	cleanup_srcu_struct(&ops->srcu);
 }
-- 
2.53.0

Re: [PATCH net-next] rtnetlink: comment the RCU and SRCU waits in unregister paths

[Kuniyuki Iwashima]

On Thu, Oct 8, 2026 at 5:44 AM Andrea Parri <parri.andrea@gmail.com> wrote:
>
> rtnl_link_unregister(), rtnl_af_unregister() and rtnl_unregister_all()
> wait for readers before tearing down what those readers use, but most
> of these waits do not say which readers they are waiting for.
>
> A missing wait is then easy to overlook: rtnl_link_unregister() lacked
> its synchronize_rcu() until commit 3b70edea393d ("rtnetlink: wait for
> RCU readers in rtnl_link_unregister()"). Add a comment to each wait
> that lacks one, naming the readers it waits for.
>
> Suggested-by: Eric Dumazet <edumazet@kernel.org>
> Link: https://lore.kernel.org/CAL4WiioFMQN4troVjQjCmE6Z9kwwb95SnoEupjD=PPSK6Q5f6w@mail.gmail.com
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
>  net/core/rtnetlink.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 15bffafbf0a9c..80f8577816760 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -526,6 +526,9 @@ void rtnl_unregister_all(int protocol)
>         }
>         rtnl_unlock();
>
> +       /* Wait for rtnl_get_link() callers under RCU that may still
> +        * dereference tab.
> +        */
>         synchronize_net();

More specifically, this only applies to callers before try_module_get().

>
>         kfree(tab);
> @@ -711,6 +714,7 @@ void rtnl_link_unregister(struct rtnl_link_ops *ops)
>          * not yet taken ops->srcu.
>          */
>         synchronize_rcu();
> +       /* Wait for rtnl_link_ops_get() callers to call rtnl_link_ops_put(). */
>         synchronize_srcu(&ops->srcu);

I think we don't need comment for srcu as the relation is
clear via the argument.

>         cleanup_srcu_struct(&ops->srcu);
>
> @@ -843,7 +847,11 @@ void rtnl_af_unregister(struct rtnl_af_ops *ops)
>         list_del_rcu(&ops->list);
>         spin_unlock(&rtnl_af_ops_lock);
>
> +       /* Wait for RCU walkers of rtnl_af_ops, including rtnl_af_lookup()
> +        * readers that found ops but have not yet taken ops->srcu.
> +        */

this can be simply /* see rtnl_link_unregister() */ instead of duplicating
comments.

>         synchronize_rcu();
> +       /* Wait for rtnl_af_lookup() callers to call rtnl_af_put(). */

ditto.

>         synchronize_srcu(&ops->srcu);
>         cleanup_srcu_struct(&ops->srcu);
>  }
> --
> 2.53.0
>

Re: [PATCH net-next] rtnetlink: comment the RCU and SRCU waits in unregister paths

[Andrea Parri]

On Mon, Oct 12, 2026 at 06:48:22PM -0700, Kuniyuki Iwashima wrote:
> > +       /* Wait for rtnl_get_link() callers under RCU that may still
> > +        * dereference tab.
> > +        */
> >         synchronize_net();
>
> More specifically, this only applies to callers before try_module_get().

Right, the later lookup in rtnetlink_rcv_msg() runs under RTNL.  I'll
name rtnetlink_rcv_msg() and try_module_get() in v2.


> > +       /* Wait for rtnl_link_ops_get() callers to call rtnl_link_ops_put(). */
> >         synchronize_srcu(&ops->srcu);
>
> I think we don't need comment for srcu as the relation is
> clear via the argument.

Agreed, I'll drop both srcu comments.


> > +       /* Wait for RCU walkers of rtnl_af_ops, including rtnl_af_lookup()
> > +        * readers that found ops but have not yet taken ops->srcu.
> > +        */
>
> this can be simply /* see rtnl_link_unregister() */ instead of duplicating
> comments.

The two waits cover different readers, though.  Besides
rtnl_af_lookup(), the synchronize_rcu() in rtnl_af_unregister() also
waits for the RCU walkers of rtnl_af_ops that invoke callbacks without
ops->srcu: rtnl_link_get_af_size(), rtnl_fill_link_af() and the
IFLA_STATS_AF_SPEC paths.  link_ops has no analogous RCU-only list
walkers, so a pointer there would miss them.  I'd prefer to keep a
local comment, reworded in v2 to mention those walkers.

Thanks,
  Andrea