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: 14+ 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
-- strict thread matches above, loose matches on Subject: below --
2025-05-15 11:04 [PATCH v1 net-next 5/7] Revert "ipv6: Factorise ip6_route_multipath_add()." kernel test robot
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.