The current code uses the RT_TOS macro to cut off the 6 DSCP
bits, down to the original 3 TOS bits.
Do not use this macro to get the prio for inheriting purposes.
Signed-off-by: Matthias May <[email protected]>
---
drivers/net/geneve.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 4c380c06f178..e1a4480e6f17 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -877,8 +877,7 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
use_cache = false;
}
- fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio),
- info->key.label);
+ fl6->flowlabel = ip6_make_flowinfo(prio, info->key.label);
dst_cache = (struct dst_cache *)&info->dst_cache;
if (use_cache) {
dst = dst_cache_get_ip6(dst_cache, &fl6->saddr);
--
2.35.1
On Sun, Jul 24, 2022 at 02:37:41AM +0200, Matthias May wrote:
> The current code uses the RT_TOS macro to cut off the 6 DSCP
> bits, down to the original 3 TOS bits.
>
> Do not use this macro to get the prio for inheriting purposes.
Honestly, this patch is a bug fix and is suitable for the net tree
(with appropriate 'Fixes' tag).
Ideally, we'd also fix ip6_dst_lookup_tunnel() (used by bareudp
tunnels) and vxlan6_get_route().
Also, mlx5e_tc_tun_update_header_ipv6() and
mlx5e_tc_tun_create_header_ipv6() both call RT_TOS() inside
ip6_make_flowinfo() and certainly need to be fixed too.
> Signed-off-by: Matthias May <[email protected]>
> ---
> drivers/net/geneve.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 4c380c06f178..e1a4480e6f17 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -877,8 +877,7 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
> use_cache = false;
> }
>
> - fl6->flowlabel = ip6_make_flowinfo(RT_TOS(prio),
> - info->key.label);
> + fl6->flowlabel = ip6_make_flowinfo(prio, info->key.label);
> dst_cache = (struct dst_cache *)&info->dst_cache;
> if (use_cache) {
> dst = dst_cache_get_ip6(dst_cache, &fl6->saddr);
> --
> 2.35.1
>
On 25/07/2022 19:05, Guillaume Nault wrote:
> On Sun, Jul 24, 2022 at 02:37:41AM +0200, Matthias May wrote:
>> The current code uses the RT_TOS macro to cut off the 6 DSCP
>> bits, down to the original 3 TOS bits.
>>
>> Do not use this macro to get the prio for inheriting purposes.
>
> Honestly, this patch is a bug fix and is suitable for the net tree
> (with appropriate 'Fixes' tag).
>
> Ideally, we'd also fix ip6_dst_lookup_tunnel() (used by bareudp
> tunnels) and vxlan6_get_route().
>
> Also, mlx5e_tc_tun_update_header_ipv6() and
> mlx5e_tc_tun_create_header_ipv6() both call RT_TOS() inside
> ip6_make_flowinfo() and certainly need to be fixed too.
>
Hi Guillaume
How would i do that?
Send a v2 to net with the fixes tag on 95caf6f71a999?
Or just resend to net with the fixes tag on 95caf6f71a999?
Since there are no actual changes to the patch.
This kind of contradicts the statement that IPv4 and IPv6 should behave the same.
--> v6 would be fixed, but v4 not.
BR
Matthias
On Tue, Jul 26, 2022 at 06:29:56PM +0200, Matthias May wrote:
> On 25/07/2022 19:05, Guillaume Nault wrote:
> > On Sun, Jul 24, 2022 at 02:37:41AM +0200, Matthias May wrote:
> > > The current code uses the RT_TOS macro to cut off the 6 DSCP
> > > bits, down to the original 3 TOS bits.
> > >
> > > Do not use this macro to get the prio for inheriting purposes.
> >
> > Honestly, this patch is a bug fix and is suitable for the net tree
> > (with appropriate 'Fixes' tag).
> >
> > Ideally, we'd also fix ip6_dst_lookup_tunnel() (used by bareudp
> > tunnels) and vxlan6_get_route().
> >
> > Also, mlx5e_tc_tun_update_header_ipv6() and
> > mlx5e_tc_tun_create_header_ipv6() both call RT_TOS() inside
> > ip6_make_flowinfo() and certainly need to be fixed too.
> >
>
> Hi Guillaume
> How would i do that?
> Send a v2 to net with the fixes tag on 95caf6f71a999?
> Or just resend to net with the fixes tag on 95caf6f71a999?
> Since there are no actual changes to the patch.
Hi Matthias,
Ideally, send a patch series to net that'd removes RT_TOS() from the
ip6_make_flowinfo() calls in geneve, vxlan and bareudp (one patch for
each protocol, with the appropriate Fixes tag). You can add the IPv4
patch in that series or send it separately, as you see fit.
Alternatively you can just repost this series to net, with a proper
Fixes tag for each patch (and I'll take care of vxlan and bareudp in
a future series).
> This kind of contradicts the statement that IPv4 and IPv6 should behave the same.
> --> v6 would be fixed, but v4 not.
I personally consider the current IPv4 behaviour for TOS inherit option
to be a bug, so, in this case, we can have both IPv4 and IPv6 fixed in
the same tree.
But generally speaking, we have some divergence in how IPv4 and IPv6
treat tos/dsfield. That's because of some historical reasons and it's
not easy to reconciliate both implementations (because of backward
compatibility).
> BR
> Matthias