| Message ID | 20240514201102055dD2Ba45qKbLlUMxu_DTHP@zte.com.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bb487272380d120295e955ad8acfcbb281b57642 |
| Headers | show |
| Series | net/ipv6: Fix route deleting failure when metric equals 0 | expand |
On 5/14/24 6:11 AM, xu.xin16@zte.com.cn wrote: > From: xu xin <xu.xin16@zte.com.cn> > > Problem > ========= > After commit 67f695134703 ("ipv6: Move setting default metric for routes"), > we noticed that the logic of assigning the default value of fc_metirc > changed in the ioctl process. That is, when users use ioctl(fd, SIOCADDRT, > rt) with a non-zero metric to add a route, then they may fail to delete a > route with passing in a metric value of 0 to the kernel by ioctl(fd, > SIOCDELRT, rt). But iproute can succeed in deleting it. > > As a reference, when using iproute tools by netlink to delete routes with > a metric parameter equals 0, like the command as follows: > > ip -6 route del fe80::/64 via fe81::5054:ff:fe11:3451 dev eth0 metric 0 > > the user can still succeed in deleting the route entry with the smallest > metric. > > Root Reason > =========== > After commit 67f695134703 ("ipv6: Move setting default metric for routes"), > When ioctl() pass in SIOCDELRT with a zero metric, rtmsg_to_fib6_config() > will set a defalut value (1024) to cfg->fc_metric in kernel, and in > ip6_route_del() and the line 4074 at net/ipv3/route.c, it will check by > > if (cfg->fc_metric && cfg->fc_metric != rt->fib6_metric) > continue; > > and the condition is true and skip the later procedure (deleting route) > because cfg->fc_metric != rt->fib6_metric. But before that commit, > cfg->fc_metric is still zero there, so the condition is false and it > will do the following procedure (deleting). > > Solution > ======== > In order to keep a consistent behaviour across netlink() and ioctl(), we > should allow to delete a route with a metric value of 0. So we only do > the default setting of fc_metric in route adding. > > CC: stable@vger.kernel.org # 5.4+ > Fixes: 67f695134703 ("ipv6: Move setting default metric for routes") > Co-developed-by: Fan Yu <fan.yu9@zte.com.cn> > Signed-off-by: Fan Yu <fan.yu9@zte.com.cn> > Signed-off-by: xu xin <xu.xin16@zte.com.cn> > --- > net/ipv6/route.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > Reviewed-by: David Ahern <dsahern@kernel.org>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 14 May 2024 20:11:02 +0800 (CST) you wrote: > From: xu xin <xu.xin16@zte.com.cn> > > Problem > ========= > After commit 67f695134703 ("ipv6: Move setting default metric for routes"), > we noticed that the logic of assigning the default value of fc_metirc > changed in the ioctl process. That is, when users use ioctl(fd, SIOCADDRT, > rt) with a non-zero metric to add a route, then they may fail to delete a > route with passing in a metric value of 0 to the kernel by ioctl(fd, > SIOCDELRT, rt). But iproute can succeed in deleting it. > > [...] Here is the summary with links: - net/ipv6: Fix route deleting failure when metric equals 0 https://git.kernel.org/netdev/net/c/bb487272380d You are awesome, thank you!
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index c43b0616742e..bbc2a0dd9314 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4445,7 +4445,7 @@ static void rtmsg_to_fib6_config(struct net *net, .fc_table = l3mdev_fib_table_by_index(net, rtmsg->rtmsg_ifindex) ? : RT6_TABLE_MAIN, .fc_ifindex = rtmsg->rtmsg_ifindex, - .fc_metric = rtmsg->rtmsg_metric ? : IP6_RT_PRIO_USER, + .fc_metric = rtmsg->rtmsg_metric, .fc_expires = rtmsg->rtmsg_info, .fc_dst_len = rtmsg->rtmsg_dst_len, .fc_src_len = rtmsg->rtmsg_src_len, @@ -4475,6 +4475,9 @@ int ipv6_route_ioctl(struct net *net, unsigned int cmd, struct in6_rtmsg *rtmsg) rtnl_lock(); switch (cmd) { case SIOCADDRT: + /* Only do the default setting of fc_metric in route adding */ + if (cfg.fc_metric == 0) + cfg.fc_metric = IP6_RT_PRIO_USER; err = ip6_route_add(&cfg, GFP_KERNEL, NULL); break; case SIOCDELRT: