[PATCH ipsec-next v2 0/4] xfrm: Convert __xfrm4_dst_lookup() and its callers to dscp_t.

[PATCH ipsec-next v2 0/4] xfrm: Convert __xfrm4_dst_lookup() and its callers to dscp_t.

[Guillaume Nault]

This patch series continues to prepare users of ->flowi4_tos to a
future conversion of this field (__u8 to dscp_t). This time, we convert
__xfrm4_dst_lookup() and its call chain.

The objective is to eventually make all users of ->flowi4_tos use a
dscp_t value. Making ->flowi4_tos a dscp_t field will help avoiding
regressions where ECN bits are erroneously interpreted as DSCP bits.

Changes since v1:
  * Rebase on top of ipsec-next. Now we don't convert the ->dst_lookup()
    callback handlers since they they don't have any "tos" parameter
    anymore. Therefore, the original patches 4, 5 and 6 are dropped and
    replaced with the new patch 4, which just converts the "tos" field
    in struct xfrm_dst_lookup_params.

Guillaume Nault (4):
  xfrm: Convert xfrm_get_tos() to dscp_t.
  xfrm: Convert xfrm_bundle_create() to dscp_t.
  xfrm: Convert xfrm_dst_lookup() to dscp_t.
  xfrm: Convert struct xfrm_dst_lookup_params -> tos to dscp_t.

 include/net/xfrm.h      |  3 ++-
 net/ipv4/xfrm4_policy.c |  3 ++-
 net/xfrm/xfrm_policy.c  | 16 ++++++++--------
 3 files changed, 12 insertions(+), 10 deletions(-)

-- 
2.39.2

[PATCH ipsec-next v2 1/4] xfrm: Convert xfrm_get_tos() to dscp_t.

[Guillaume Nault]

Return a dscp_t variable to prepare for the future conversion of
xfrm_bundle_create() to dscp_t.

While there, rename the function "xfrm_get_dscp", to align its name
with the new return type.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/xfrm/xfrm_policy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index a2ea9dbac90b..077e1c9b2025 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2576,10 +2576,10 @@ xfrm_tmpl_resolve(struct xfrm_policy **pols, int npols, const struct flowi *fl,
 
 }
 
-static int xfrm_get_tos(const struct flowi *fl, int family)
+static dscp_t xfrm_get_dscp(const struct flowi *fl, int family)
 {
 	if (family == AF_INET)
-		return fl->u.ip4.flowi4_tos & INET_DSCP_MASK;
+		return inet_dsfield_to_dscp(fl->u.ip4.flowi4_tos);
 
 	return 0;
 }
@@ -2673,7 +2673,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 
 	xfrm_flowi_addr_get(fl, &saddr, &daddr, family);
 
-	tos = xfrm_get_tos(fl, family);
+	tos = inet_dscp_to_dsfield(xfrm_get_dscp(fl, family));
 
 	dst_hold(dst);
 
-- 
2.39.2

Re: [PATCH ipsec-next v2 1/4] xfrm: Convert xfrm_get_tos() to dscp_t.

[Ido Schimmel]

On Thu, Oct 31, 2024 at 04:52:36PM +0100, Guillaume Nault wrote:
> Return a dscp_t variable to prepare for the future conversion of
> xfrm_bundle_create() to dscp_t.
> 
> While there, rename the function "xfrm_get_dscp", to align its name
> with the new return type.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[PATCH ipsec-next v2 2/4] xfrm: Convert xfrm_bundle_create() to dscp_t.

[Guillaume Nault]

Use a dscp_t variable to store the result of xfrm_get_dscp().
This prepares for the future conversion of xfrm_dst_lookup().

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/xfrm/xfrm_policy.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 077e1c9b2025..222410fa43e7 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2667,13 +2667,13 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 	int header_len = 0;
 	int nfheader_len = 0;
 	int trailer_len = 0;
-	int tos;
 	int family = policy->selector.family;
 	xfrm_address_t saddr, daddr;
+	dscp_t dscp;
 
 	xfrm_flowi_addr_get(fl, &saddr, &daddr, family);
 
-	tos = inet_dscp_to_dsfield(xfrm_get_dscp(fl, family));
+	dscp = xfrm_get_dscp(fl, family);
 
 	dst_hold(dst);
 
@@ -2721,7 +2721,8 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 				family = xfrm[i]->props.family;
 
 			oif = fl->flowi_oif ? : fl->flowi_l3mdev;
-			dst = xfrm_dst_lookup(xfrm[i], tos, oif,
+			dst = xfrm_dst_lookup(xfrm[i],
+					      inet_dscp_to_dsfield(dscp), oif,
 					      &saddr, &daddr, family, mark);
 			err = PTR_ERR(dst);
 			if (IS_ERR(dst))
-- 
2.39.2

Re: [PATCH ipsec-next v2 2/4] xfrm: Convert xfrm_bundle_create() to dscp_t.

[Ido Schimmel]

On Thu, Oct 31, 2024 at 04:52:43PM +0100, Guillaume Nault wrote:
> Use a dscp_t variable to store the result of xfrm_get_dscp().
> This prepares for the future conversion of xfrm_dst_lookup().
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[PATCH ipsec-next v2 3/4] xfrm: Convert xfrm_dst_lookup() to dscp_t.

[Guillaume Nault]

Pass a dscp_t variable to xfrm_dst_lookup(), instead of an int, to
prevent accidental setting of ECN bits in ->flowi4_tos.

Only xfrm_bundle_create() actually calls xfrm_dst_lookup(). Since it
already has a dscp_t variable to pass as parameter, we only need to
remove the inet_dscp_to_dsfield() conversion.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/xfrm/xfrm_policy.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 222410fa43e7..9e231c1ece3f 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -289,7 +289,7 @@ struct dst_entry *__xfrm_dst_lookup(int family,
 EXPORT_SYMBOL(__xfrm_dst_lookup);
 
 static inline struct dst_entry *xfrm_dst_lookup(struct xfrm_state *x,
-						int tos, int oif,
+						dscp_t dscp, int oif,
 						xfrm_address_t *prev_saddr,
 						xfrm_address_t *prev_daddr,
 						int family, u32 mark)
@@ -312,7 +312,7 @@ static inline struct dst_entry *xfrm_dst_lookup(struct xfrm_state *x,
 	params.net = net;
 	params.saddr = saddr;
 	params.daddr = daddr;
-	params.tos = tos;
+	params.tos = inet_dscp_to_dsfield(dscp);
 	params.oif = oif;
 	params.mark = mark;
 	params.ipproto = x->id.proto;
@@ -2721,9 +2721,8 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 				family = xfrm[i]->props.family;
 
 			oif = fl->flowi_oif ? : fl->flowi_l3mdev;
-			dst = xfrm_dst_lookup(xfrm[i],
-					      inet_dscp_to_dsfield(dscp), oif,
-					      &saddr, &daddr, family, mark);
+			dst = xfrm_dst_lookup(xfrm[i], dscp, oif, &saddr,
+					      &daddr, family, mark);
 			err = PTR_ERR(dst);
 			if (IS_ERR(dst))
 				goto put_states;
-- 
2.39.2

Re: [PATCH ipsec-next v2 3/4] xfrm: Convert xfrm_dst_lookup() to dscp_t.

[Ido Schimmel]

On Thu, Oct 31, 2024 at 04:52:49PM +0100, Guillaume Nault wrote:
> Pass a dscp_t variable to xfrm_dst_lookup(), instead of an int, to
> prevent accidental setting of ECN bits in ->flowi4_tos.
> 
> Only xfrm_bundle_create() actually calls xfrm_dst_lookup(). Since it
> already has a dscp_t variable to pass as parameter, we only need to
> remove the inet_dscp_to_dsfield() conversion.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[PATCH ipsec-next v2 4/4] xfrm: Convert struct xfrm_dst_lookup_params -> tos to dscp_t.

[Guillaume Nault]

Add type annotation to the "tos" field of struct xfrm_dst_lookup_params,
to ensure that the ECN bits aren't mistakenly taken into account when
doing route lookups. Rename that field (tos -> dscp) to make that
change explicit.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 include/net/xfrm.h      | 3 ++-
 net/ipv4/xfrm4_policy.c | 3 ++-
 net/xfrm/xfrm_policy.c  | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index a0bdd58f401c..48ec4c415e98 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -19,6 +19,7 @@
 
 #include <net/sock.h>
 #include <net/dst.h>
+#include <net/inet_dscp.h>
 #include <net/ip.h>
 #include <net/route.h>
 #include <net/ipv6.h>
@@ -351,7 +352,7 @@ void xfrm_if_unregister_cb(void);
 
 struct xfrm_dst_lookup_params {
 	struct net *net;
-	int tos;
+	dscp_t dscp;
 	int oif;
 	xfrm_address_t *saddr;
 	xfrm_address_t *daddr;
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 7e1c2faed1ff..7fb6205619e7 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -14,6 +14,7 @@
 #include <linux/inetdevice.h>
 #include <net/dst.h>
 #include <net/xfrm.h>
+#include <net/inet_dscp.h>
 #include <net/ip.h>
 #include <net/l3mdev.h>
 
@@ -24,7 +25,7 @@ static struct dst_entry *__xfrm4_dst_lookup(struct flowi4 *fl4,
 
 	memset(fl4, 0, sizeof(*fl4));
 	fl4->daddr = params->daddr->a4;
-	fl4->flowi4_tos = params->tos;
+	fl4->flowi4_tos = inet_dscp_to_dsfield(params->dscp);
 	fl4->flowi4_l3mdev = l3mdev_master_ifindex_by_index(params->net,
 							    params->oif);
 	fl4->flowi4_mark = params->mark;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 9e231c1ece3f..e91607fe45ba 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -312,7 +312,7 @@ static inline struct dst_entry *xfrm_dst_lookup(struct xfrm_state *x,
 	params.net = net;
 	params.saddr = saddr;
 	params.daddr = daddr;
-	params.tos = inet_dscp_to_dsfield(dscp);
+	params.dscp = dscp;
 	params.oif = oif;
 	params.mark = mark;
 	params.ipproto = x->id.proto;
-- 
2.39.2

Re: [PATCH ipsec-next v2 4/4] xfrm: Convert struct xfrm_dst_lookup_params -> tos to dscp_t.

[Ido Schimmel]

On Thu, Oct 31, 2024 at 04:52:57PM +0100, Guillaume Nault wrote:
> Add type annotation to the "tos" field of struct xfrm_dst_lookup_params,
> to ensure that the ECN bits aren't mistakenly taken into account when
> doing route lookups. Rename that field (tos -> dscp) to make that
> change explicit.
> 
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH ipsec-next v2 0/4] xfrm: Convert __xfrm4_dst_lookup() and its callers to dscp_t.

[Steffen Klassert]

On Thu, Oct 31, 2024 at 04:52:27PM +0100, Guillaume Nault wrote:
> This patch series continues to prepare users of ->flowi4_tos to a
> future conversion of this field (__u8 to dscp_t). This time, we convert
> __xfrm4_dst_lookup() and its call chain.
> 
> The objective is to eventually make all users of ->flowi4_tos use a
> dscp_t value. Making ->flowi4_tos a dscp_t field will help avoiding
> regressions where ECN bits are erroneously interpreted as DSCP bits.
> 
> Changes since v1:
>   * Rebase on top of ipsec-next. Now we don't convert the ->dst_lookup()
>     callback handlers since they they don't have any "tos" parameter
>     anymore. Therefore, the original patches 4, 5 and 6 are dropped and
>     replaced with the new patch 4, which just converts the "tos" field
>     in struct xfrm_dst_lookup_params.
> 
> Guillaume Nault (4):
>   xfrm: Convert xfrm_get_tos() to dscp_t.
>   xfrm: Convert xfrm_bundle_create() to dscp_t.
>   xfrm: Convert xfrm_dst_lookup() to dscp_t.
>   xfrm: Convert struct xfrm_dst_lookup_params -> tos to dscp_t.
> 
>  include/net/xfrm.h      |  3 ++-
>  net/ipv4/xfrm4_policy.c |  3 ++-
>  net/xfrm/xfrm_policy.c  | 16 ++++++++--------
>  3 files changed, 12 insertions(+), 10 deletions(-)

Series applied to ipsec-next, thanks a lot!