[PATCH net-next v2 0/4] net: sparx5: flower: validate control flags

[PATCH net-next v2 0/4] net: sparx5: flower: validate control flags

[Asbjørn Sloth Tønnesen]

This series adds flower control flags validation to the
sparx5 driver, and changes it from assuming that it handles
all control flags, to instead reject rules if they have
masked any unknown/unsupported control flags.

---

Changelog:

v2:
* Split first patch into 3 (requested by Jiri)
* Convert a missed extack usage (noticed by Daniel)
* Added cover letter (requested by Simon and Daniel)

v1: https://lore.kernel.org/netdev/20240423102728.228765-1-ast@fiberby.net/

Asbjørn Sloth Tønnesen (4):
  net: sparx5: flower: only do lookup if fragment flags are set
  net: sparx5: flower: add extack to
    sparx5_tc_flower_handler_control_usage()
  net: sparx5: flower: remove goto in
    sparx5_tc_flower_handler_control_usage()
  net: sparx5: flower: check for unsupported control flags

 .../microchip/sparx5/sparx5_tc_flower.c       | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH net-next v2 1/4] net: sparx5: flower: only do lookup if fragment flags are set

[Asbjørn Sloth Tønnesen]

The fragment lookup should only be performed, when
at least one of the fragment flags are set.

This change was deliberately not included in commit
68aba00483c7 ("net: sparx5: flower: fix fragment flags handling")
as it's only needed for future proffing the code, since
"mask" is currently only set in conjunction with the
fragment flags.
(The 3rd flag FLOW_DIS_ENCAPSULATION is only used with "key")

Only compile tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index 663571fe7b2d..ca9e69d39439 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -165,7 +165,7 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
 
 	flow_rule_match_control(st->frule, &mt);
 
-	if (mt.mask->flags) {
+	if (mt.mask->flags & (FLOW_DIS_IS_FRAGMENT | FLOW_DIS_FIRST_FRAG)) {
 		u8 is_frag_key = !!(mt.key->flags & FLOW_DIS_IS_FRAGMENT);
 		u8 is_frag_mask = !!(mt.mask->flags & FLOW_DIS_IS_FRAGMENT);
 		u8 is_frag_idx = (is_frag_key << 1) | is_frag_mask;
-- 
2.43.0

[PATCH net-next v2 2/4] net: sparx5: flower: add extack to sparx5_tc_flower_handler_control_usage()

[Asbjørn Sloth Tønnesen]

Define extack locally, to reduce line lengths and aid future users.

Only compile tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index ca9e69d39439..d0dca65f8e76 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -159,6 +159,7 @@ sparx5_tc_flower_handler_basic_usage(struct vcap_tc_flower_parse_usage *st)
 static int
 sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
 {
+	struct netlink_ext_ack *extack = st->fco->common.extack;
 	struct flow_match_control mt;
 	u32 value, mask;
 	int err = 0;
@@ -178,7 +179,7 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
 		u8 vdt = sparx5_vcap_frag_map[is_frag_idx][first_frag_idx];
 
 		if (vdt == FRAG_INVAL) {
-			NL_SET_ERR_MSG_MOD(st->fco->common.extack,
+			NL_SET_ERR_MSG_MOD(extack,
 					   "Match on invalid fragment flag combination");
 			return -EINVAL;
 		}
@@ -199,7 +200,7 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
 	return err;
 
 out:
-	NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ip_frag parse error");
+	NL_SET_ERR_MSG_MOD(extack, "ip_frag parse error");
 	return err;
 }
 
-- 
2.43.0

[PATCH net-next v2 3/4] net: sparx5: flower: remove goto in sparx5_tc_flower_handler_control_usage()

[Asbjørn Sloth Tønnesen]

Remove goto, as it's only used once, and the error message is
specific to that context.

Only compile tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 .../net/ethernet/microchip/sparx5/sparx5_tc_flower.c   | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index d0dca65f8e76..22f6c778afb0 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -191,17 +191,15 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
 		err = vcap_rule_add_key_u32(st->vrule,
 					    VCAP_KF_L3_FRAGMENT_TYPE,
 					    value, mask);
-		if (err)
-			goto out;
+		if (err) {
+			NL_SET_ERR_MSG_MOD(extack, "ip_frag parse error");
+			return err;
+		}
 	}
 
 	st->used_keys |= BIT_ULL(FLOW_DISSECTOR_KEY_CONTROL);
 
 	return err;
-
-out:
-	NL_SET_ERR_MSG_MOD(extack, "ip_frag parse error");
-	return err;
 }
 
 static int
-- 
2.43.0

[PATCH net-next v2 4/4] net: sparx5: flower: check for unsupported control flags

[Asbjørn Sloth Tønnesen]

Use flow_rule_is_supp_control_flags() to reject filters with
unsupported control flags.

In case any unsupported control flags are masked,
flow_rule_is_supp_control_flags() sets a NL extended
error message, and we return -EOPNOTSUPP.

Only compile-tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index 22f6c778afb0..8d67d9f24c76 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -197,6 +197,11 @@ sparx5_tc_flower_handler_control_usage(struct vcap_tc_flower_parse_usage *st)
 		}
 	}
 
+	if (!flow_rule_is_supp_control_flags(FLOW_DIS_IS_FRAGMENT |
+					     FLOW_DIS_FIRST_FRAG,
+					     mt.mask->flags, extack))
+		return -EOPNOTSUPP;
+
 	st->used_keys |= BIT_ULL(FLOW_DISSECTOR_KEY_CONTROL);
 
 	return err;
-- 
2.43.0

Re: [PATCH net-next v2 0/4] net: sparx5: flower: validate control flags

[Daniel Machon]

> This series adds flower control flags validation to the
> sparx5 driver, and changes it from assuming that it handles
> all control flags, to instead reject rules if they have
> masked any unknown/unsupported control flags.
> 
> ---
> 
> Changelog:
> 
> v2:
> * Split first patch into 3 (requested by Jiri)
> * Convert a missed extack usage (noticed by Daniel)
> * Added cover letter (requested by Simon and Daniel)
> 
> v1: https://lore.kernel.org/netdev/20240423102728.228765-1-ast@fiberby.net/
> 
> Asbjørn Sloth Tønnesen (4):
>   net: sparx5: flower: only do lookup if fragment flags are set
>   net: sparx5: flower: add extack to
>     sparx5_tc_flower_handler_control_usage()
>   net: sparx5: flower: remove goto in
>     sparx5_tc_flower_handler_control_usage()
>   net: sparx5: flower: check for unsupported control flags
> 
>  .../microchip/sparx5/sparx5_tc_flower.c       | 20 +++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> --
> 2.43.0
>

For the series:

Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
Tested-by: Daniel Machon <daniel.machon@microchip.com>

Re: [PATCH net-next v2 0/4] net: sparx5: flower: validate control flags

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 24 Apr 2024 12:16:21 +0000 you wrote:
> This series adds flower control flags validation to the
> sparx5 driver, and changes it from assuming that it handles
> all control flags, to instead reject rules if they have
> masked any unknown/unsupported control flags.
> 
> ---
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/4] net: sparx5: flower: only do lookup if fragment flags are set
    https://git.kernel.org/netdev/net-next/c/bcf303c62c98
  - [net-next,v2,2/4] net: sparx5: flower: add extack to sparx5_tc_flower_handler_control_usage()
    https://git.kernel.org/netdev/net-next/c/8cd1b6c0bf31
  - [net-next,v2,3/4] net: sparx5: flower: remove goto in sparx5_tc_flower_handler_control_usage()
    https://git.kernel.org/netdev/net-next/c/b92eb1ac13f0
  - [net-next,v2,4/4] net: sparx5: flower: check for unsupported control flags
    https://git.kernel.org/netdev/net-next/c/8ef631e9c995

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html