Re: [PATCH net 2/3] net: nexthop: release IPv6 per-cpu dsts when replacing a nexthop group

On 21/11/2021 19:35, Ido Schimmel wrote:

On Sun, Nov 21, 2021 at 07:17:41PM +0200, Ido Schimmel wrote:

quoted

On Sun, Nov 21, 2021 at 05:24:52PM +0200, Nikolay Aleksandrov wrote:

quoted

From: Nikolay Aleksandrov <redacted>

Can we avoid two synchronize_net() per resilient group by removing the
one added here and instead do:

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index a69a9e76f99f..a47ce43ab1ff 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c

@@ -2002,9 +2002,10 @@ static int replace_nexthop_grp(struct net *net, struct nexthop *old,
 
        rcu_assign_pointer(old->nh_grp, newg);
 
+       /* Make sure concurrent readers are not using 'oldg' anymore. */
+       synchronize_net();
+
        if (newg->resilient) {
-               /* Make sure concurrent readers are not using 'oldg' anymore. */
-               synchronize_net();
                rcu_assign_pointer(oldg->res_table, tmp_table);
                rcu_assign_pointer(oldg->spare->res_table, tmp_table);
        }

Discussed this with Nik. It is possible and would be a good cleanup for
net-next. For net it is best to leave synchronize_net() where it is so
that the patch will be easier to backport. Resilient nexthop groups were
only added in 5.13 whereas nexthop objects were added in 5.3

Indeed, thank you for the review. I'll send patches for this suggestion and
the IPv6 optimization (that is one of the optimizations I was referring to
in the cover letter) for net-next after net is merged.