[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250516022759.44392-6-kuniyu@amazon.com>
Date: Thu, 15 May 2025 19:27:21 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: "David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, "Paolo
Abeni" <pabeni@...hat.com>
CC: Simon Horman <horms@...nel.org>, Kuniyuki Iwashima <kuniyu@...zon.com>,
Kuniyuki Iwashima <kuni1840@...il.com>, <netdev@...r.kernel.org>
Subject: [PATCH v2 net-next 5/7] Revert "ipv6: Factorise ip6_route_multipath_add()."
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@...zon.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
Powered by blists - more mailing lists