Skip to content

Commit bd11ff4

Browse files
q2venPaolo Abeni
authored andcommitted
ipv6: Get rid of RTNL for SIOCDELRT and RTM_DELROUTE.
Basically, removing an IPv6 route does not require RTNL because the IPv6 routing tables are protected by per table lock. inet6_rtm_delroute() calls nexthop_find_by_id() to check if the nexthop specified by RTA_NH_ID exists. nexthop uses rbtree and the top-down walk can be safely performed under RCU. ip6_route_del() already relies on RCU and the table lock, but we need to extend the RCU critical section a bit more to cover __ip6_del_rt(). For example, nexthop_for_each_fib6_nh() and inet6_rt_notify() needs RCU. Let's call nexthop_find_by_id() and __ip6_del_rt() under RCU and get rid of RTNL from inet6_rtm_delroute() and SIOCDELRT. Even if the nexthop is removed after rcu_read_unlock() in inet6_rtm_delroute(), __remove_nexthop_fib() cleans up the routes tied to the nexthop, and ip6_route_del() returns -ESRCH. So the request was at least valid as of nexthop_find_by_id(), and it's just a matter of timing. Note that we need to pass false to lwtunnel_valid_encap_type_attr(). The following patches also use the newroute bool. Note also that fib6_get_table() does not require RCU because once allocated fib6_table is not freed until netns dismantle. I will post a follow-up series to convert such callers to RCU-lockless version. [0] Link: https://lore.kernel.org/netdev/[email protected]/ #[0] Signed-off-by: Kuniyuki Iwashima <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent 4cb4861 commit bd11ff4

File tree

1 file changed

+28
-20
lines changed

1 file changed

+28
-20
lines changed

net/ipv6/route.c

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4125,9 +4125,9 @@ static int ip6_route_del(struct fib6_config *cfg,
41254125
if (rt->nh) {
41264126
if (!fib6_info_hold_safe(rt))
41274127
continue;
4128-
rcu_read_unlock();
41294128

4130-
return __ip6_del_rt(rt, &cfg->fc_nlinfo);
4129+
err = __ip6_del_rt(rt, &cfg->fc_nlinfo);
4130+
break;
41314131
}
41324132
if (cfg->fc_nh_id)
41334133
continue;
@@ -4142,13 +4142,13 @@ static int ip6_route_del(struct fib6_config *cfg,
41424142
continue;
41434143
if (!fib6_info_hold_safe(rt))
41444144
continue;
4145-
rcu_read_unlock();
41464145

41474146
/* if gateway was specified only delete the one hop */
41484147
if (cfg->fc_flags & RTF_GATEWAY)
4149-
return __ip6_del_rt(rt, &cfg->fc_nlinfo);
4150-
4151-
return __ip6_del_rt_siblings(rt, cfg);
4148+
err = __ip6_del_rt(rt, &cfg->fc_nlinfo);
4149+
else
4150+
err = __ip6_del_rt_siblings(rt, cfg);
4151+
break;
41524152
}
41534153
}
41544154
rcu_read_unlock();
@@ -4517,19 +4517,20 @@ int ipv6_route_ioctl(struct net *net, unsigned int cmd, struct in6_rtmsg *rtmsg)
45174517

45184518
rtmsg_to_fib6_config(net, rtmsg, &cfg);
45194519

4520-
rtnl_lock();
45214520
switch (cmd) {
45224521
case SIOCADDRT:
4522+
rtnl_lock();
45234523
/* Only do the default setting of fc_metric in route adding */
45244524
if (cfg.fc_metric == 0)
45254525
cfg.fc_metric = IP6_RT_PRIO_USER;
45264526
err = ip6_route_add(&cfg, GFP_KERNEL, NULL);
4527+
rtnl_unlock();
45274528
break;
45284529
case SIOCDELRT:
45294530
err = ip6_route_del(&cfg, NULL);
45304531
break;
45314532
}
4532-
rtnl_unlock();
4533+
45334534
return err;
45344535
}
45354536

@@ -5052,7 +5053,8 @@ static const struct nla_policy rtm_ipv6_policy[RTA_MAX+1] = {
50525053
};
50535054

50545055
static int rtm_to_fib6_multipath_config(struct fib6_config *cfg,
5055-
struct netlink_ext_ack *extack)
5056+
struct netlink_ext_ack *extack,
5057+
bool newroute)
50565058
{
50575059
struct rtnexthop *rtnh;
50585060
int remaining;
@@ -5086,15 +5088,16 @@ static int rtm_to_fib6_multipath_config(struct fib6_config *cfg,
50865088
} while (rtnh_ok(rtnh, remaining));
50875089

50885090
return lwtunnel_valid_encap_type_attr(cfg->fc_mp, cfg->fc_mp_len,
5089-
extack, true);
5091+
extack, newroute);
50905092
}
50915093

50925094
static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
50935095
struct fib6_config *cfg,
50945096
struct netlink_ext_ack *extack)
50955097
{
5096-
struct rtmsg *rtm;
5098+
bool newroute = nlh->nlmsg_type == RTM_NEWROUTE;
50975099
struct nlattr *tb[RTA_MAX+1];
5100+
struct rtmsg *rtm;
50985101
unsigned int pref;
50995102
int err;
51005103

@@ -5203,7 +5206,7 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
52035206
cfg->fc_mp = nla_data(tb[RTA_MULTIPATH]);
52045207
cfg->fc_mp_len = nla_len(tb[RTA_MULTIPATH]);
52055208

5206-
err = rtm_to_fib6_multipath_config(cfg, extack);
5209+
err = rtm_to_fib6_multipath_config(cfg, extack, newroute);
52075210
if (err < 0)
52085211
goto errout;
52095212
}
@@ -5223,7 +5226,7 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
52235226
cfg->fc_encap_type = nla_get_u16(tb[RTA_ENCAP_TYPE]);
52245227

52255228
err = lwtunnel_valid_encap_type(cfg->fc_encap_type,
5226-
extack, true);
5229+
extack, newroute);
52275230
if (err < 0)
52285231
goto errout;
52295232
}
@@ -5546,15 +5549,20 @@ static int inet6_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh,
55465549
if (err < 0)
55475550
return err;
55485551

5549-
if (cfg.fc_nh_id &&
5550-
!nexthop_find_by_id(sock_net(skb->sk), cfg.fc_nh_id)) {
5551-
NL_SET_ERR_MSG(extack, "Nexthop id does not exist");
5552-
return -EINVAL;
5552+
if (cfg.fc_nh_id) {
5553+
rcu_read_lock();
5554+
err = !nexthop_find_by_id(sock_net(skb->sk), cfg.fc_nh_id);
5555+
rcu_read_unlock();
5556+
5557+
if (err) {
5558+
NL_SET_ERR_MSG(extack, "Nexthop id does not exist");
5559+
return -EINVAL;
5560+
}
55535561
}
55545562

5555-
if (cfg.fc_mp)
5563+
if (cfg.fc_mp) {
55565564
return ip6_route_multipath_del(&cfg, extack);
5557-
else {
5565+
} else {
55585566
cfg.fc_delete_all_nh = 1;
55595567
return ip6_route_del(&cfg, extack);
55605568
}
@@ -6766,7 +6774,7 @@ static const struct rtnl_msg_handler ip6_route_rtnl_msg_handlers[] __initconst_o
67666774
{.owner = THIS_MODULE, .protocol = PF_INET6, .msgtype = RTM_NEWROUTE,
67676775
.doit = inet6_rtm_newroute},
67686776
{.owner = THIS_MODULE, .protocol = PF_INET6, .msgtype = RTM_DELROUTE,
6769-
.doit = inet6_rtm_delroute},
6777+
.doit = inet6_rtm_delroute, .flags = RTNL_FLAG_DOIT_UNLOCKED},
67706778
{.owner = THIS_MODULE, .protocol = PF_INET6, .msgtype = RTM_GETROUTE,
67716779
.doit = inet6_rtm_getroute, .flags = RTNL_FLAG_DOIT_UNLOCKED},
67726780
};

0 commit comments

Comments
 (0)