From: Ido Schimmel <idosch@idosch.org>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
dsahern@gmail.com, Nikolay Aleksandrov <nikolay@nvidia.com>
Subject: Re: [PATCH net 2/3] net: nexthop: release IPv6 per-cpu dsts when replacing a nexthop group
Date: Sun, 21 Nov 2021 19:35:17 +0200 [thread overview]
Message-ID: <YZqDVQiqSCMsDEZh@shredder> (raw)
In-Reply-To: <YZp/MvIX/YCHJY9K@shredder>
On Sun, Nov 21, 2021 at 07:17:41PM +0200, Ido Schimmel wrote:
> On Sun, Nov 21, 2021 at 05:24:52PM +0200, Nikolay Aleksandrov wrote:
> > From: Nikolay Aleksandrov <nikolay@nvidia.com>
> 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
next prev parent reply other threads:[~2021-11-21 17:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-21 15:24 [PATCH net 0/3] net: nexthop: fix refcount issues when replacing groups Nikolay Aleksandrov
2021-11-21 15:24 ` [PATCH net 1/3] net: ipv6: add fib6_nh_release_dsts stub Nikolay Aleksandrov
2021-11-21 15:24 ` [PATCH net 2/3] net: nexthop: release IPv6 per-cpu dsts when replacing a nexthop group Nikolay Aleksandrov
2021-11-21 17:17 ` Ido Schimmel
2021-11-21 17:35 ` Ido Schimmel [this message]
2021-11-21 18:02 ` Nikolay Aleksandrov
2021-11-21 15:24 ` [PATCH net 3/3] selftests: net: fib_nexthops: add test for group refcount imbalance bug Nikolay Aleksandrov
2021-11-21 17:53 ` Ido Schimmel
2021-11-21 17:59 ` Nikolay Aleksandrov
2021-11-21 17:55 ` [PATCH net 0/3] net: nexthop: fix refcount issues when replacing groups Ido Schimmel
2021-11-21 18:17 ` Nikolay Aleksandrov
2021-11-22 9:48 ` Ido Schimmel
2021-11-22 9:53 ` Nikolay Aleksandrov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YZqDVQiqSCMsDEZh@shredder \
--to=idosch@idosch.org \
--cc=davem@davemloft.net \
--cc=dsahern@gmail.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nikolay@nvidia.com \
--cc=razor@blackwall.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).