The ->rtm_tos option is normally used to route packets based on both the destination address and the DS field. However it's ignored for IPv6 routes. Setting ->rtm_tos for IPv6 is thus invalid as the route is going to work only on the destination address anyway, so it won't behave as specified.
Suggested-by: Toke Høiland-Jørgensen toke@redhat.com Signed-off-by: Guillaume Nault gnault@redhat.com --- The same problem exists for ->rtm_scope. I'm working only on ->rtm_tos here because IPv4 recently started to validate this option too (as part of the DSCP/ECN clarification effort). I'll give this patch some soak time, then send another one for rejecting ->rtm_scope in IPv6 routes if nobody complains.
net/ipv6/route.c | 6 ++++++ tools/testing/selftests/net/fib_tests.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index f4884cda13b9..dd98a11fbdb6 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -5009,6 +5009,12 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh, err = -EINVAL; rtm = nlmsg_data(nlh);
+ if (rtm->rtm_tos) { + NL_SET_ERR_MSG(extack, + "Invalid dsfield (tos): option not available for IPv6"); + goto errout; + } + *cfg = (struct fib6_config){ .fc_table = rtm->rtm_table, .fc_dst_len = rtm->rtm_dst_len, diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index bb73235976b3..e2690cc42da3 100755 --- a/tools/testing/selftests/net/fib_tests.sh +++ b/tools/testing/selftests/net/fib_tests.sh @@ -988,12 +988,25 @@ ipv6_rt_replace() ipv6_rt_replace_mpath }
+ipv6_rt_dsfield() +{ + echo + echo "IPv6 route with dsfield tests" + + run_cmd "$IP -6 route flush 2001:db8:102::/64" + + # IPv6 doesn't support routing based on dsfield + run_cmd "$IP -6 route add 2001:db8:102::/64 dsfield 0x04 via 2001:db8:101::2" + log_test $? 2 "Reject route with dsfield" +} + ipv6_route_test() { route_setup
ipv6_rt_add ipv6_rt_replace + ipv6_rt_dsfield
route_cleanup }
On 2/10/22 7:08 AM, Guillaume Nault wrote:
The ->rtm_tos option is normally used to route packets based on both the destination address and the DS field. However it's ignored for IPv6 routes. Setting ->rtm_tos for IPv6 is thus invalid as the route is going to work only on the destination address anyway, so it won't behave as specified.
Suggested-by: Toke Høiland-Jørgensen toke@redhat.com Signed-off-by: Guillaume Nault gnault@redhat.com
The same problem exists for ->rtm_scope. I'm working only on ->rtm_tos here because IPv4 recently started to validate this option too (as part of the DSCP/ECN clarification effort). I'll give this patch some soak time, then send another one for rejecting ->rtm_scope in IPv6 routes if nobody complains.
net/ipv6/route.c | 6 ++++++ tools/testing/selftests/net/fib_tests.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+)
Reviewed-by: David Ahern dsahern@kernel.org
On 2/10/22 8:08 AM, Guillaume Nault wrote:
The ->rtm_tos option is normally used to route packets based on both the destination address and the DS field. However it's ignored for IPv6 routes. Setting ->rtm_tos for IPv6 is thus invalid as the route is going to work only on the destination address anyway, so it won't behave as specified.
Suggested-by: Toke Høiland-Jørgensen toke@redhat.com Signed-off-by: Guillaume Nault gnault@redhat.com
The same problem exists for ->rtm_scope. I'm working only on ->rtm_tos here because IPv4 recently started to validate this option too (as part of the DSCP/ECN clarification effort). I'll give this patch some soak time, then send another one for rejecting ->rtm_scope in IPv6 routes if nobody complains.
net/ipv6/route.c | 6 ++++++ tools/testing/selftests/net/fib_tests.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index f4884cda13b9..dd98a11fbdb6 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -5009,6 +5009,12 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh, err = -EINVAL; rtm = nlmsg_data(nlh);
- if (rtm->rtm_tos) {
NL_SET_ERR_MSG(extack,"Invalid dsfield (tos): option not available for IPv6");
Is this an expected failure on ipv6, in which case should this test report pass? Should it print "failed as expected" or is returning fail from errout is what should happen?
goto errout;- }
- *cfg = (struct fib6_config){ .fc_table = rtm->rtm_table, .fc_dst_len = rtm->rtm_dst_len,
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index bb73235976b3..e2690cc42da3 100755 --- a/tools/testing/selftests/net/fib_tests.sh +++ b/tools/testing/selftests/net/fib_tests.sh @@ -988,12 +988,25 @@ ipv6_rt_replace() ipv6_rt_replace_mpath } +ipv6_rt_dsfield() +{
- echo
- echo "IPv6 route with dsfield tests"
- run_cmd "$IP -6 route flush 2001:db8:102::/64"
- # IPv6 doesn't support routing based on dsfield
- run_cmd "$IP -6 route add 2001:db8:102::/64 dsfield 0x04 via 2001:db8:101::2"
- log_test $? 2 "Reject route with dsfield"
+}
- ipv6_route_test() { route_setup
ipv6_rt_add ipv6_rt_replace
- ipv6_rt_dsfield
route_cleanup }
With the above comment addressed or explained.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
On Thu, Feb 10, 2022 at 11:23:20AM -0700, Shuah Khan wrote:
On 2/10/22 8:08 AM, Guillaume Nault wrote:
The ->rtm_tos option is normally used to route packets based on both the destination address and the DS field. However it's ignored for IPv6 routes. Setting ->rtm_tos for IPv6 is thus invalid as the route is going to work only on the destination address anyway, so it won't behave as specified.
Suggested-by: Toke Høiland-Jørgensen toke@redhat.com Signed-off-by: Guillaume Nault gnault@redhat.com
The same problem exists for ->rtm_scope. I'm working only on ->rtm_tos here because IPv4 recently started to validate this option too (as part of the DSCP/ECN clarification effort). I'll give this patch some soak time, then send another one for rejecting ->rtm_scope in IPv6 routes if nobody complains.
net/ipv6/route.c | 6 ++++++ tools/testing/selftests/net/fib_tests.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index f4884cda13b9..dd98a11fbdb6 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -5009,6 +5009,12 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh, err = -EINVAL; rtm = nlmsg_data(nlh);
- if (rtm->rtm_tos) {
NL_SET_ERR_MSG(extack,"Invalid dsfield (tos): option not available for IPv6");Is this an expected failure on ipv6, in which case should this test report pass? Should it print "failed as expected" or is returning fail from errout is what should happen?
This is an expected failure. When ->rtm_tos is set, iproute2 fails with error code 2 and prints "Error: Invalid dsfield (tos): option not available for IPv6.".
The selftest redirects stderr to /dev/null by default (unless -v is passed on the command line) and expects the command to fail and return 2. So the default output is just:
IPv6 route with dsfield tests TEST: Reject route with dsfield [ OK ]
Of course, on a kernel that accepts non-null ->rtm_tos, "[ OK ]" becomes "[FAIL]", and the the failed tests couter is incremented.
goto errout;- }
- *cfg = (struct fib6_config){ .fc_table = rtm->rtm_table, .fc_dst_len = rtm->rtm_dst_len,
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index bb73235976b3..e2690cc42da3 100755 --- a/tools/testing/selftests/net/fib_tests.sh +++ b/tools/testing/selftests/net/fib_tests.sh @@ -988,12 +988,25 @@ ipv6_rt_replace() ipv6_rt_replace_mpath } +ipv6_rt_dsfield() +{
- echo
- echo "IPv6 route with dsfield tests"
- run_cmd "$IP -6 route flush 2001:db8:102::/64"
- # IPv6 doesn't support routing based on dsfield
- run_cmd "$IP -6 route add 2001:db8:102::/64 dsfield 0x04 via 2001:db8:101::2"
- log_test $? 2 "Reject route with dsfield"
+}
- ipv6_route_test() { route_setup ipv6_rt_add ipv6_rt_replace
- ipv6_rt_dsfield route_cleanup }
With the above comment addressed or explained.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
On 2/10/22 3:05 PM, Guillaume Nault wrote:
On Thu, Feb 10, 2022 at 11:23:20AM -0700, Shuah Khan wrote:
On 2/10/22 8:08 AM, Guillaume Nault wrote:
The ->rtm_tos option is normally used to route packets based on both the destination address and the DS field. However it's ignored for IPv6 routes. Setting ->rtm_tos for IPv6 is thus invalid as the route is going to work only on the destination address anyway, so it won't behave as specified.
Suggested-by: Toke Høiland-Jørgensen toke@redhat.com Signed-off-by: Guillaume Nault gnault@redhat.com
The same problem exists for ->rtm_scope. I'm working only on ->rtm_tos here because IPv4 recently started to validate this option too (as part of the DSCP/ECN clarification effort). I'll give this patch some soak time, then send another one for rejecting ->rtm_scope in IPv6 routes if nobody complains.
net/ipv6/route.c | 6 ++++++ tools/testing/selftests/net/fib_tests.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index f4884cda13b9..dd98a11fbdb6 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -5009,6 +5009,12 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh, err = -EINVAL; rtm = nlmsg_data(nlh);
- if (rtm->rtm_tos) {
NL_SET_ERR_MSG(extack,"Invalid dsfield (tos): option not available for IPv6");Is this an expected failure on ipv6, in which case should this test report pass? Should it print "failed as expected" or is returning fail from errout is what should happen?
This is an expected failure. When ->rtm_tos is set, iproute2 fails with error code 2 and prints "Error: Invalid dsfield (tos): option not available for IPv6.".
The selftest redirects stderr to /dev/null by default (unless -v is passed on the command line) and expects the command to fail and return 2. So the default output is just:
IPv6 route with dsfield tests TEST: Reject route with dsfield [ OK ]
Of course, on a kernel that accepts non-null ->rtm_tos, "[ OK ]" becomes "[FAIL]", and the the failed tests couter is incremented.
Sounds good to me.
With the above comment addressed or explained.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
Hello:
This patch was applied to netdev/net-next.git (master) by David S. Miller davem@davemloft.net:
On Thu, 10 Feb 2022 16:08:08 +0100 you wrote:
The ->rtm_tos option is normally used to route packets based on both the destination address and the DS field. However it's ignored for IPv6 routes. Setting ->rtm_tos for IPv6 is thus invalid as the route is going to work only on the destination address anyway, so it won't behave as specified.
Suggested-by: Toke Høiland-Jørgensen toke@redhat.com Signed-off-by: Guillaume Nault gnault@redhat.com
[...]
Here is the summary with links: - [net-next] ipv6: Reject routes configurations that specify dsfield (tos) https://git.kernel.org/netdev/net-next/c/b9605161e7be
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org
-
David Ahern -
Guillaume Nault -
patchwork-bot+netdevbpf＠kernel.org -
Shuah Khan