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@xxxxxxxxxx>
> > Signed-off-by: Guillaume Nault <gnault@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxx>
>
> thanks,
> -- Shuah
>
