* [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos)
@ 2022-02-10 15:08 Guillaume Nault
2022-02-10 18:11 ` David Ahern
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Guillaume Nault @ 2022-02-10 15:08 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, Hideaki YOSHIFUJI, David Ahern, Shuah Khan,
linux-kselftest, Toke Høiland-Jørgensen
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
}
--
2.21.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos)
2022-02-10 15:08 [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos) Guillaume Nault
@ 2022-02-10 18:11 ` David Ahern
2022-02-10 18:23 ` Shuah Khan
2022-02-11 12:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2022-02-10 18:11 UTC (permalink / raw)
To: Guillaume Nault, David Miller, Jakub Kicinski
Cc: netdev, Hideaki YOSHIFUJI, David Ahern, Shuah Khan,
linux-kselftest, Toke Høiland-Jørgensen
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos)
2022-02-10 15:08 [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos) Guillaume Nault
2022-02-10 18:11 ` David Ahern
@ 2022-02-10 18:23 ` Shuah Khan
2022-02-10 22:05 ` Guillaume Nault
2022-02-11 12:00 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2022-02-10 18:23 UTC (permalink / raw)
To: Guillaume Nault, David Miller, Jakub Kicinski
Cc: netdev, Hideaki YOSHIFUJI, David Ahern, Shuah Khan,
linux-kselftest, Toke Høiland-Jørgensen, Shuah Khan
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos)
2022-02-10 18:23 ` Shuah Khan
@ 2022-02-10 22:05 ` Guillaume Nault
2022-02-10 22:15 ` Shuah Khan
0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2022-02-10 22:05 UTC (permalink / raw)
To: Shuah Khan
Cc: David Miller, Jakub Kicinski, netdev, Hideaki YOSHIFUJI,
David Ahern, Shuah Khan, linux-kselftest,
Toke Høiland-Jørgensen
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos)
2022-02-10 22:05 ` Guillaume Nault
@ 2022-02-10 22:15 ` Shuah Khan
0 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2022-02-10 22:15 UTC (permalink / raw)
To: Guillaume Nault
Cc: David Miller, Jakub Kicinski, netdev, Hideaki YOSHIFUJI,
David Ahern, Shuah Khan, linux-kselftest,
Toke Høiland-Jørgensen, Shuah Khan
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos)
2022-02-10 15:08 [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos) Guillaume Nault
2022-02-10 18:11 ` David Ahern
2022-02-10 18:23 ` Shuah Khan
@ 2022-02-11 12:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-11 12:00 UTC (permalink / raw)
To: Guillaume Nault
Cc: davem, kuba, netdev, yoshfuji, dsahern, shuah, linux-kselftest, toke
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-11 12:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 15:08 [PATCH net-next] ipv6: Reject routes configurations that specify dsfield (tos) Guillaume Nault
2022-02-10 18:11 ` David Ahern
2022-02-10 18:23 ` Shuah Khan
2022-02-10 22:05 ` Guillaume Nault
2022-02-10 22:15 ` Shuah Khan
2022-02-11 12:00 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.