From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: "David S. Miller" <davem@davemloft.net>,
David Ahern <dsahern@kernel.org>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>,
Kuniyuki Iwashima <kuniyu@amazon.com>,
Kuniyuki Iwashima <kuni1840@gmail.com>, <netdev@vger.kernel.org>
Subject: [PATCH v1 net-next 5/7] Revert "ipv6: Factorise ip6_route_multipath_add()."
Date: Wed, 14 May 2025 13:18:58 -0700 [thread overview]
Message-ID: <20250514201943.74456-6-kuniyu@amazon.com> (raw)
In-Reply-To: <20250514201943.74456-1-kuniyu@amazon.com>
Commit 71c0efb6d12f ("ipv6: Factorise ip6_route_multipath_add().") split
a loop in ip6_route_multipath_add() so that we can put rcu_read_lock()
between ip6_route_info_create() and ip6_route_info_create_nh().
We no longer need to do so as ip6_route_info_create_nh() does not require
RCU now.
Let's revert the commit to simplify the code.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
net/ipv6/route.c | 193 +++++++++++++++++------------------------------
1 file changed, 70 insertions(+), 123 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index a87091dd06b1..96ae21da9961 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5335,131 +5335,29 @@ struct rt6_nh {
struct fib6_info *fib6_info;
struct fib6_config r_cfg;
struct list_head list;
- int weight;
};
-static void ip6_route_mpath_info_cleanup(struct list_head *rt6_nh_list)
+static int ip6_route_info_append(struct list_head *rt6_nh_list,
+ struct fib6_info *rt,
+ struct fib6_config *r_cfg)
{
- struct rt6_nh *nh, *nh_next;
+ struct rt6_nh *nh;
- list_for_each_entry_safe(nh, nh_next, rt6_nh_list, list) {
- struct fib6_info *rt = nh->fib6_info;
-
- if (rt) {
- free_percpu(rt->fib6_nh->nh_common.nhc_pcpu_rth_output);
- free_percpu(rt->fib6_nh->rt6i_pcpu);
- ip_fib_metrics_put(rt->fib6_metrics);
- kfree(rt);
- }
-
- list_del(&nh->list);
- kfree(nh);
+ list_for_each_entry(nh, rt6_nh_list, list) {
+ /* check if fib6_info already exists */
+ if (rt6_duplicate_nexthop(nh->fib6_info, rt))
+ return -EEXIST;
}
-}
-
-static int ip6_route_mpath_info_create(struct list_head *rt6_nh_list,
- struct fib6_config *cfg,
- struct netlink_ext_ack *extack)
-{
- struct rtnexthop *rtnh;
- int remaining;
- int err;
-
- remaining = cfg->fc_mp_len;
- rtnh = (struct rtnexthop *)cfg->fc_mp;
-
- /* Parse a Multipath Entry and build a list (rt6_nh_list) of
- * fib6_info structs per nexthop
- */
- while (rtnh_ok(rtnh, remaining)) {
- struct fib6_config r_cfg;
- struct fib6_info *rt;
- struct rt6_nh *nh;
- int attrlen;
-
- nh = kzalloc(sizeof(*nh), GFP_KERNEL);
- if (!nh) {
- err = -ENOMEM;
- goto err;
- }
- list_add_tail(&nh->list, rt6_nh_list);
-
- memcpy(&r_cfg, cfg, sizeof(*cfg));
- if (rtnh->rtnh_ifindex)
- r_cfg.fc_ifindex = rtnh->rtnh_ifindex;
-
- attrlen = rtnh_attrlen(rtnh);
- if (attrlen > 0) {
- struct nlattr *nla, *attrs = rtnh_attrs(rtnh);
-
- nla = nla_find(attrs, attrlen, RTA_GATEWAY);
- if (nla) {
- r_cfg.fc_gateway = nla_get_in6_addr(nla);
- r_cfg.fc_flags |= RTF_GATEWAY;
- }
-
- r_cfg.fc_encap = nla_find(attrs, attrlen, RTA_ENCAP);
- nla = nla_find(attrs, attrlen, RTA_ENCAP_TYPE);
- if (nla)
- r_cfg.fc_encap_type = nla_get_u16(nla);
- }
-
- r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK);
-
- rt = ip6_route_info_create(&r_cfg, GFP_KERNEL, extack);
- if (IS_ERR(rt)) {
- err = PTR_ERR(rt);
- goto err;
- }
-
- nh->fib6_info = rt;
- nh->weight = rtnh->rtnh_hops + 1;
- memcpy(&nh->r_cfg, &r_cfg, sizeof(r_cfg));
+ nh = kzalloc(sizeof(*nh), GFP_KERNEL);
+ if (!nh)
+ return -ENOMEM;
- rtnh = rtnh_next(rtnh, &remaining);
- }
+ nh->fib6_info = rt;
+ memcpy(&nh->r_cfg, r_cfg, sizeof(*r_cfg));
+ list_add_tail(&nh->list, rt6_nh_list);
return 0;
-err:
- ip6_route_mpath_info_cleanup(rt6_nh_list);
- return err;
-}
-
-static int ip6_route_mpath_info_create_nh(struct list_head *rt6_nh_list,
- struct netlink_ext_ack *extack)
-{
- struct rt6_nh *nh, *nh_next, *nh_tmp;
- LIST_HEAD(tmp);
- int err;
-
- list_for_each_entry_safe(nh, nh_next, rt6_nh_list, list) {
- struct fib6_info *rt = nh->fib6_info;
-
- err = ip6_route_info_create_nh(rt, &nh->r_cfg, extack);
- if (err) {
- nh->fib6_info = NULL;
- goto err;
- }
-
- rt->fib6_nh->fib_nh_weight = nh->weight;
-
- list_move_tail(&nh->list, &tmp);
-
- list_for_each_entry(nh_tmp, rt6_nh_list, list) {
- /* check if fib6_info already exists */
- if (rt6_duplicate_nexthop(nh_tmp->fib6_info, rt)) {
- err = -EEXIST;
- goto err;
- }
- }
- }
-out:
- list_splice(&tmp, rt6_nh_list);
- return err;
-err:
- ip6_route_mpath_info_cleanup(rt6_nh_list);
- goto out;
}
static void ip6_route_mpath_notify(struct fib6_info *rt,
@@ -5519,11 +5417,16 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
struct fib6_info *rt_notif = NULL, *rt_last = NULL;
struct nl_info *info = &cfg->fc_nlinfo;
struct rt6_nh *nh, *nh_safe;
+ struct fib6_config r_cfg;
+ struct rtnexthop *rtnh;
LIST_HEAD(rt6_nh_list);
struct rt6_nh *err_nh;
+ struct fib6_info *rt;
__u16 nlflags;
- int nhn = 0;
+ int remaining;
+ int attrlen;
int replace;
+ int nhn = 0;
int err;
replace = (cfg->fc_nlinfo.nlh &&
@@ -5533,13 +5436,57 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
if (info->nlh && info->nlh->nlmsg_flags & NLM_F_APPEND)
nlflags |= NLM_F_APPEND;
- err = ip6_route_mpath_info_create(&rt6_nh_list, cfg, extack);
- if (err)
- return err;
+ remaining = cfg->fc_mp_len;
+ rtnh = (struct rtnexthop *)cfg->fc_mp;
- err = ip6_route_mpath_info_create_nh(&rt6_nh_list, extack);
- if (err)
- goto cleanup;
+ /* Parse a Multipath Entry and build a list (rt6_nh_list) of
+ * fib6_info structs per nexthop
+ */
+ while (rtnh_ok(rtnh, remaining)) {
+ memcpy(&r_cfg, cfg, sizeof(*cfg));
+ if (rtnh->rtnh_ifindex)
+ r_cfg.fc_ifindex = rtnh->rtnh_ifindex;
+
+ attrlen = rtnh_attrlen(rtnh);
+ if (attrlen > 0) {
+ struct nlattr *nla, *attrs = rtnh_attrs(rtnh);
+
+ nla = nla_find(attrs, attrlen, RTA_GATEWAY);
+ if (nla) {
+ r_cfg.fc_gateway = nla_get_in6_addr(nla);
+ r_cfg.fc_flags |= RTF_GATEWAY;
+ }
+
+ r_cfg.fc_encap = nla_find(attrs, attrlen, RTA_ENCAP);
+ nla = nla_find(attrs, attrlen, RTA_ENCAP_TYPE);
+ if (nla)
+ r_cfg.fc_encap_type = nla_get_u16(nla);
+ }
+
+ r_cfg.fc_flags |= (rtnh->rtnh_flags & RTNH_F_ONLINK);
+ rt = ip6_route_info_create(&r_cfg, GFP_KERNEL, extack);
+ if (IS_ERR(rt)) {
+ err = PTR_ERR(rt);
+ rt = NULL;
+ goto cleanup;
+ }
+
+ err = ip6_route_info_create_nh(rt, &r_cfg, extack);
+ if (err) {
+ rt = NULL;
+ goto cleanup;
+ }
+
+ rt->fib6_nh->fib_nh_weight = rtnh->rtnh_hops + 1;
+
+ err = ip6_route_info_append(&rt6_nh_list, rt, &r_cfg);
+ if (err) {
+ fib6_info_release(rt);
+ goto cleanup;
+ }
+
+ rtnh = rtnh_next(rtnh, &remaining);
+ }
/* for add and replace send one notification with all nexthops.
* Skip the notification in fib6_add_rt2node and send one with
--
2.49.0
next prev parent reply other threads:[~2025-05-14 20:21 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-14 20:18 [PATCH v1 net-next 0/7] ipv6: Follow up for RTNL-free RTM_NEWROUTE series Kuniyuki Iwashima
2025-05-14 20:18 ` [PATCH v1 net-next 1/7] ipv6: Remove rcu_read_lock() in fib6_get_table() Kuniyuki Iwashima
2025-05-14 20:18 ` [PATCH v1 net-next 2/7] inet: Remove rtnl_is_held arg of lwtunnel_valid_encap_type(_attr)?() Kuniyuki Iwashima
2025-05-14 20:18 ` [PATCH v1 net-next 3/7] ipv6: Narrow down RCU critical section in inet6_rtm_newroute() Kuniyuki Iwashima
2025-05-14 20:18 ` [PATCH v1 net-next 4/7] Revert "ipv6: sr: switch to GFP_ATOMIC flag to allocate memory during seg6local LWT setup" Kuniyuki Iwashima
2025-05-14 20:18 ` Kuniyuki Iwashima [this message]
2025-05-14 20:18 ` [PATCH v1 net-next 6/7] ipv6: Pass gfp_flags down to ip6_route_info_create_nh() Kuniyuki Iwashima
2025-05-14 20:19 ` [PATCH v1 net-next 7/7] ipv6: Revert two per-cpu var allocation for RTM_NEWROUTE Kuniyuki Iwashima
2025-05-15 1:45 ` [PATCH v1 net-next 0/7] ipv6: Follow up for RTNL-free RTM_NEWROUTE series Jakub Kicinski
2025-05-15 2:05 ` Kuniyuki Iwashima
2025-05-15 2:22 ` Jakub Kicinski
2025-05-15 9:02 ` Paolo Abeni
2025-05-15 16:46 ` Kuniyuki Iwashima
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=20250514201943.74456-6-kuniyu@amazon.com \
--to=kuniyu@amazon.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=kuni1840@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/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).