The current code retrieves the TOS field after the lookup
on the ipv4 routing table. The routing process currently
only allows routing based on the original 3 TOS bits, and
not on the full 6 DSCP bits.
As a result the retrieved TOS is cut to the 3 bits.
However for inheriting purposes the full 6 bits should be used.
Extract the full 6 bits before the route lookup and use
that instead of the cut off 3 TOS bits.
This patch is the functional equivalent for IPv4 to the patch
"geneve: do not use RT_TOS for IPv6 flowlabel"
Fixes: e305ac6cf5a1 ("geneve: Add support to collect tunnel metadata.")
Signed-off-by: Matthias May <[email protected]>
---
v1 -> v2:
- Fix typo in "Fixes" tag
v2 -> v3:
- Add missing CCs
---
drivers/net/geneve.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 2495a5719e1c..4c380c06f178 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -797,7 +797,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
struct geneve_sock *gs4,
struct flowi4 *fl4,
const struct ip_tunnel_info *info,
- __be16 dport, __be16 sport)
+ __be16 dport, __be16 sport,
+ __u8 *full_tos)
{
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct geneve_dev *geneve = netdev_priv(dev);
@@ -822,6 +823,7 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
use_cache = false;
}
fl4->flowi4_tos = RT_TOS(tos);
+ *full_tos = tos;
dst_cache = (struct dst_cache *)&info->dst_cache;
if (use_cache) {
@@ -910,6 +912,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
const struct ip_tunnel_key *key = &info->key;
struct rtable *rt;
struct flowi4 fl4;
+ __u8 full_tos;
__u8 tos, ttl;
__be16 df = 0;
__be16 sport;
@@ -920,7 +923,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
rt = geneve_get_v4_rt(skb, dev, gs4, &fl4, info,
- geneve->cfg.info.key.tp_dst, sport);
+ geneve->cfg.info.key.tp_dst, sport, &full_tos);
if (IS_ERR(rt))
return PTR_ERR(rt);
@@ -964,7 +967,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0;
} else {
- tos = ip_tunnel_ecn_encap(fl4.flowi4_tos, ip_hdr(skb), skb);
+ tos = ip_tunnel_ecn_encap(full_tos, ip_hdr(skb), skb);
if (geneve->cfg.ttl_inherit)
ttl = ip_tunnel_get_ttl(ip_hdr(skb), skb);
else
@@ -1137,6 +1140,7 @@ static int geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
{
struct ip_tunnel_info *info = skb_tunnel_info(skb);
struct geneve_dev *geneve = netdev_priv(dev);
+ __u8 full_tos;
__be16 sport;
if (ip_tunnel_info_af(info) == AF_INET) {
@@ -1148,7 +1152,8 @@ static int geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
1, USHRT_MAX, true);
rt = geneve_get_v4_rt(skb, dev, gs4, &fl4, info,
- geneve->cfg.info.key.tp_dst, sport);
+ geneve->cfg.info.key.tp_dst, sport,
+ &full_tos);
if (IS_ERR(rt))
return PTR_ERR(rt);
--
2.35.1
On Tue, Aug 02, 2022 at 02:20:25PM +0200, Matthias May wrote:
> The current code retrieves the TOS field after the lookup
> on the ipv4 routing table. The routing process currently
> only allows routing based on the original 3 TOS bits, and
> not on the full 6 DSCP bits.
> As a result the retrieved TOS is cut to the 3 bits.
> However for inheriting purposes the full 6 bits should be used.
>
> Extract the full 6 bits before the route lookup and use
> that instead of the cut off 3 TOS bits.
>
> This patch is the functional equivalent for IPv4 to the patch
> "geneve: do not use RT_TOS for IPv6 flowlabel"
This last sentence assumes this patch and your IPv6 series are going to
be merged roughly at the same time and with the same title. There's
no such guarantee though. So I think we can just drop the reference to
the IPv6 patch. But wait a day or two before sending a new version:
others might have different opinion, maintainers might want to apply
the patch as is or adjust the message manually, someone may have other
points to comment on...
Anyway, the rest of the commit message and the code look good to me.
Thanks for fixing this!
Acked-by: Guillaume Nault <[email protected]>
On 8/2/22 6:20 AM, Matthias May wrote:
> @@ -822,6 +823,7 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
> use_cache = false;
> }
> fl4->flowi4_tos = RT_TOS(tos);
Could make this optional with
if (full_tos)
> + *full_tos = tos;
>
> dst_cache = (struct dst_cache *)&info->dst_cache;
> if (use_cache) {
...
> @@ -1137,6 +1140,7 @@ static int geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
> {
> struct ip_tunnel_info *info = skb_tunnel_info(skb);
> struct geneve_dev *geneve = netdev_priv(dev);
> + __u8 full_tos;
> __be16 sport;
>
> if (ip_tunnel_info_af(info) == AF_INET) {
> @@ -1148,7 +1152,8 @@ static int geneve_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
> 1, USHRT_MAX, true);
>
> rt = geneve_get_v4_rt(skb, dev, gs4, &fl4, info,
> - geneve->cfg.info.key.tp_dst, sport);
> + geneve->cfg.info.key.tp_dst, sport,
> + &full_tos);
> if (IS_ERR(rt))
> return PTR_ERR(rt);
>
since this one is not used