[PATCH net-next 0/5] net: flower: validate encapsulation control flags

[PATCH net-next 0/5] net: flower: validate encapsulation control flags

[Asbjørn Sloth Tønnesen]

Now that all drivers properly rejects unsupported flower control flags
used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar
checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL.

There are currently just 4 drivers supporting this key, and
3 of those currently doesn't validate encapsulated control flags.

Encapsulation control flags may currently be unused, but they should
still be validated by the drivers, so that drivers will properly
reject any new flags when they are introduced.

This series adds some helper functions, and implements them in all
4 drivers.

NB: It is currently discussed[1] to use encapsulation control flags
for tunnel flags instead of the new FLOW_DISSECTOR_KEY_ENC_FLAGS.

[1] https://lore.kernel.org/netdev/ZmFuxElwZiYJzBkh@dcaratti.users.ipa.redhat.com/

Asbjørn Sloth Tønnesen (5):
  flow_offload: add encapsulation control flag helpers
  sfc: use flow_rule_is_supp_enc_control_flags()
  net/mlx5e: flower: validate encapsulation control flags
  nfp: flower: validate encapsulation control flags
  ice: flower: validate encapsulation control flags

 drivers/net/ethernet/intel/ice/ice_tc_lib.c   |  4 +++
 .../ethernet/mellanox/mlx5/core/en/tc_tun.c   |  6 ++++
 .../ethernet/netronome/nfp/flower/offload.c   |  4 +++
 drivers/net/ethernet/sfc/tc.c                 |  5 +--
 include/net/flow_offload.h                    | 35 +++++++++++++++++++
 5 files changed, 50 insertions(+), 4 deletions(-)

-- 
2.45.1

[PATCH net-next 1/5] flow_offload: add encapsulation control flag helpers

[Asbjørn Sloth Tønnesen]

This patch adds two new helper functions:
  flow_rule_is_supp_enc_control_flags()
  flow_rule_has_enc_control_flags()

They are intended to be used for validating encapsulation control
flags, and compliment the similar helpers without "enc_" in the name.

The only difference is that they have their own error message,
to make it obvious if an unsupported flag error is related to
FLOW_DISSECTOR_KEY_CONTROL or FLOW_DISSECTOR_KEY_ENC_CONTROL.

flow_rule_has_enc_control_flags() is for drivers supporting
FLOW_DISSECTOR_KEY_ENC_CONTROL, but not supporting any
encapsulation control flags.
(Currently all 4 drivers fits this category)

flow_rule_is_supp_enc_control_flags() is currently only used
for the above helper, but should also be used by drivers once
they implement at least one encapsulation control flag.

There is AFAICT currently no need for an "enc_" variant of
flow_rule_match_has_control_flags(), as all drivers currently
supporting FLOW_DISSECTOR_KEY_ENC_CONTROL, are already calling
flow_rule_match_enc_control() directly.

Only compile tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 include/net/flow_offload.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index ec9f80509f60c..292cd8f4b762a 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -471,6 +471,28 @@ static inline bool flow_rule_is_supp_control_flags(const u32 supp_flags,
 	return false;
 }
 
+/**
+ * flow_rule_is_supp_enc_control_flags() - check for supported control flags
+ * @supp_enc_flags: encapsulation control flags supported by driver
+ * @enc_ctrl_flags: encapsulation control flags present in rule
+ * @extack: The netlink extended ACK for reporting errors.
+ *
+ * Return: true if only supported control flags are set, false otherwise.
+ */
+static inline bool flow_rule_is_supp_enc_control_flags(const u32 supp_enc_flags,
+						       const u32 enc_ctrl_flags,
+						       struct netlink_ext_ack *extack)
+{
+	if (likely((enc_ctrl_flags & ~supp_enc_flags) == 0))
+		return true;
+
+	NL_SET_ERR_MSG_FMT_MOD(extack,
+			       "Unsupported match on enc_control.flags %#x",
+			       enc_ctrl_flags);
+
+	return false;
+}
+
 /**
  * flow_rule_has_control_flags() - check for presence of any control flags
  * @ctrl_flags: control flags present in rule
@@ -484,6 +506,19 @@ static inline bool flow_rule_has_control_flags(const u32 ctrl_flags,
 	return !flow_rule_is_supp_control_flags(0, ctrl_flags, extack);
 }
 
+/**
+ * flow_rule_has_enc_control_flags() - check for presence of any control flags
+ * @enc_ctrl_flags: encapsulation control flags present in rule
+ * @extack: The netlink extended ACK for reporting errors.
+ *
+ * Return: true if control flags are set, false otherwise.
+ */
+static inline bool flow_rule_has_enc_control_flags(const u32 enc_ctrl_flags,
+						   struct netlink_ext_ack *extack)
+{
+	return !flow_rule_is_supp_enc_control_flags(0, enc_ctrl_flags, extack);
+}
+
 /**
  * flow_rule_match_has_control_flags() - match and check for any control flags
  * @rule: The flow_rule under evaluation.
-- 
2.45.1

[PATCH net-next 2/5] sfc: use flow_rule_is_supp_enc_control_flags()

[Asbjørn Sloth Tønnesen]

Change the existing check for unsupported encapsulation control flags,
to use the new helper flow_rule_is_supp_enc_control_flags().

No functional change, only compile tested.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 drivers/net/ethernet/sfc/tc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index 9d140203e273a..0d93164988fc6 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -387,11 +387,8 @@ static int efx_tc_flower_parse_match(struct efx_nic *efx,
 		struct flow_match_control fm;
 
 		flow_rule_match_enc_control(rule, &fm);
-		if (fm.mask->flags) {
-			NL_SET_ERR_MSG_FMT_MOD(extack, "Unsupported match on enc_control.flags %#x",
-					       fm.mask->flags);
+		if (flow_rule_has_enc_control_flags(fm.mask->flags, extack))
 			return -EOPNOTSUPP;
-		}
 		if (!IS_ALL_ONES(fm.mask->addr_type)) {
 			NL_SET_ERR_MSG_FMT_MOD(extack, "Unsupported enc addr_type mask %u (key %u)",
 					       fm.mask->addr_type,
-- 
2.45.1

Re: [PATCH net-next 2/5] sfc: use flow_rule_is_supp_enc_control_flags()

[Edward Cree]

On 09/06/2024 18:33, Asbjørn Sloth Tønnesen wrote:
> Change the existing check for unsupported encapsulation control flags,
> to use the new helper flow_rule_is_supp_enc_control_flags().
> 
> No functional change, only compile tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

Acked-by: Edward Cree <ecree.xilinx@gmail.com>

> ---
>  drivers/net/ethernet/sfc/tc.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
> index 9d140203e273a..0d93164988fc6 100644
> --- a/drivers/net/ethernet/sfc/tc.c
> +++ b/drivers/net/ethernet/sfc/tc.c
> @@ -387,11 +387,8 @@ static int efx_tc_flower_parse_match(struct efx_nic *efx,
>  		struct flow_match_control fm;
>  
>  		flow_rule_match_enc_control(rule, &fm);
> -		if (fm.mask->flags) {
> -			NL_SET_ERR_MSG_FMT_MOD(extack, "Unsupported match on enc_control.flags %#x",
> -					       fm.mask->flags);
> +		if (flow_rule_has_enc_control_flags(fm.mask->flags, extack))
>  			return -EOPNOTSUPP;
> -		}
>  		if (!IS_ALL_ONES(fm.mask->addr_type)) {
>  			NL_SET_ERR_MSG_FMT_MOD(extack, "Unsupported enc addr_type mask %u (key %u)",
>  					       fm.mask->addr_type,
> 

[PATCH net-next 3/5] net/mlx5e: flower: validate encapsulation control flags

[Asbjørn Sloth Tønnesen]

Encapsulation control flags are currently not used anywhere,
so all flags are currently unsupported by all drivers.

This patch adds validation of this assumption, so that
encapsulation flags may be used in the future.

In case any encapsulation control flags are masked,
flow_rule_match_has_enc_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/mellanox/mlx5/core/en/tc_tun.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index 8dfb57f712b0d..721f35e597579 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -850,6 +850,12 @@ int mlx5e_tc_tun_parse(struct net_device *filter_dev,
 		flow_rule_match_enc_control(rule, &match);
 		addr_type = match.key->addr_type;
 
+		if (flow_rule_has_enc_control_flags(match.mask->flags,
+						    extack)) {
+			err = -EOPNOTSUPP;
+			goto out;
+		}
+
 		/* For tunnel addr_type used same key id`s as for non-tunnel */
 		if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
 			struct flow_match_ipv4_addrs match;
-- 
2.45.1

[PATCH net-next 4/5] nfp: flower: validate encapsulation control flags

[Asbjørn Sloth Tønnesen]

Encapsulation control flags are currently not used anywhere,
so all flags are currently unsupported by all drivers.

This patch adds validation of this assumption, so that
encapsulation flags may be used in the future.

In case any encapsulation control flags are masked,
flow_rule_match_has_enc_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/netronome/nfp/flower/offload.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 8e0a890381b60..46ffc2c208930 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -321,6 +321,10 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
 
 		flow_rule_match_enc_control(rule, &enc_ctl);
 
+		if (flow_rule_has_enc_control_flags(enc_ctl.mask->flags,
+						    extack))
+			return -EOPNOTSUPP;
+
 		if (enc_ctl.mask->addr_type != 0xffff) {
 			NL_SET_ERR_MSG_MOD(extack, "unsupported offload: wildcarded protocols on tunnels are not supported");
 			return -EOPNOTSUPP;
-- 
2.45.1

Re: [PATCH net-next 4/5] nfp: flower: validate encapsulation control flags

[Louis Peens]

On Sun, Jun 09, 2024 at 05:33:54PM +0000, Asbjørn Sloth Tønnesen wrote:
> Encapsulation control flags are currently not used anywhere,
> so all flags are currently unsupported by all drivers.
> 
> This patch adds validation of this assumption, so that
> encapsulation flags may be used in the future.
> 
> In case any encapsulation control flags are masked,
> flow_rule_match_has_enc_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>
Looks fair to me, thanks.

Signed-off-by: Louis Peens <louis.peens@corigine.com>

> ---
>  drivers/net/ethernet/netronome/nfp/flower/offload.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
> index 8e0a890381b60..46ffc2c208930 100644
> --- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
> +++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
> @@ -321,6 +321,10 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
>  
>  		flow_rule_match_enc_control(rule, &enc_ctl);
>  
> +		if (flow_rule_has_enc_control_flags(enc_ctl.mask->flags,
> +						    extack))
> +			return -EOPNOTSUPP;
> +
>  		if (enc_ctl.mask->addr_type != 0xffff) {
>  			NL_SET_ERR_MSG_MOD(extack, "unsupported offload: wildcarded protocols on tunnels are not supported");
>  			return -EOPNOTSUPP;
> -- 
> 2.45.1
> 

[PATCH net-next 5/5] ice: flower: validate encapsulation control flags

[Asbjørn Sloth Tønnesen]

Encapsulation control flags are currently not used anywhere,
so all flags are currently unsupported by all drivers.

This patch adds validation of this assumption, so that
encapsulation flags may be used in the future.

In case any encapsulation control flags are masked,
flow_rule_match_has_enc_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/intel/ice/ice_tc_lib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
index 8bd24b33f3a67..e6923f8121a99 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
@@ -1353,6 +1353,7 @@ ice_parse_tunnel_attr(struct net_device *dev, struct flow_rule *rule,
 		      struct ice_tc_flower_fltr *fltr)
 {
 	struct ice_tc_flower_lyr_2_4_hdrs *headers = &fltr->outer_headers;
+	struct netlink_ext_ack *extack = fltr->extack;
 	struct flow_match_control enc_control;
 
 	fltr->tunnel_type = ice_tc_tun_get_type(dev);
@@ -1373,6 +1374,9 @@ ice_parse_tunnel_attr(struct net_device *dev, struct flow_rule *rule,
 
 	flow_rule_match_enc_control(rule, &enc_control);
 
+	if (flow_rule_has_enc_control_flags(enc_control.mask->flags, extack))
+		return -EOPNOTSUPP;
+
 	if (enc_control.key->addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
 		struct flow_match_ipv4_addrs match;
 
-- 
2.45.1

Re: [Intel-wired-lan] [PATCH net-next 5/5] ice: flower: validate encapsulation control flags

[Marcin Szycik]

On 09.06.2024 19:33, Asbjørn Sloth Tønnesen wrote:
> Encapsulation control flags are currently not used anywhere,
> so all flags are currently unsupported by all drivers.
> 
> This patch adds validation of this assumption, so that
> encapsulation flags may be used in the future.
> 
> In case any encapsulation control flags are masked,
> flow_rule_match_has_enc_control_flags() sets a NL extended
> error message, and we return -EOPNOTSUPP.
> 
> Only compile tested.

Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>

> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
> ---
>  drivers/net/ethernet/intel/ice/ice_tc_lib.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> index 8bd24b33f3a67..e6923f8121a99 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> @@ -1353,6 +1353,7 @@ ice_parse_tunnel_attr(struct net_device *dev, struct flow_rule *rule,
>  		      struct ice_tc_flower_fltr *fltr)
>  {
>  	struct ice_tc_flower_lyr_2_4_hdrs *headers = &fltr->outer_headers;
> +	struct netlink_ext_ack *extack = fltr->extack;
>  	struct flow_match_control enc_control;
>  
>  	fltr->tunnel_type = ice_tc_tun_get_type(dev);
> @@ -1373,6 +1374,9 @@ ice_parse_tunnel_attr(struct net_device *dev, struct flow_rule *rule,
>  
>  	flow_rule_match_enc_control(rule, &enc_control);
>  
> +	if (flow_rule_has_enc_control_flags(enc_control.mask->flags, extack))
> +		return -EOPNOTSUPP;
> +
>  	if (enc_control.key->addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
>  		struct flow_match_ipv4_addrs match;
>  

Re: [PATCH net-next 0/5] net: flower: validate encapsulation control flags

[Davide Caratti]

On Sun, Jun 09, 2024 at 05:33:50PM +0000, Asbjørn Sloth Tønnesen wrote:
> Now that all drivers properly rejects unsupported flower control flags
> used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar
> checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL.
> 
> There are currently just 4 drivers supporting this key, and
> 3 of those currently doesn't validate encapsulated control flags.
> 
> Encapsulation control flags may currently be unused, but they should
> still be validated by the drivers, so that drivers will properly
> reject any new flags when they are introduced.
> 
> This series adds some helper functions, and implements them in all
> 4 drivers.
>

Reviewed-by: Davide Caratti <dcaratti@redhat.com>

Re: [PATCH net-next 0/5] net: flower: validate encapsulation control flags

[Jakub Kicinski]

On Sun,  9 Jun 2024 17:33:50 +0000 Asbjørn Sloth Tønnesen wrote:
> Now that all drivers properly rejects unsupported flower control flags
> used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar
> checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL.

Thanks for doing this work!
Do you have any more of such series left? Could we perhaps consider
recording the driver support somewhere in struct net_device and do 
the rejecting in the core? That's much easier to extend when adding
new flags than updating all the drivers.
This series I think may not be a great first candidate as IIUC all
drivers would reject so the flag would be half-dead...

Re: [PATCH net-next 0/5] net: flower: validate encapsulation control flags

[Asbjørn Sloth Tønnesen]

Hi Jakub,

On 6/13/24 1:04 AM, Jakub Kicinski wrote:
> On Sun,  9 Jun 2024 17:33:50 +0000 Asbjørn Sloth Tønnesen wrote:
>> Now that all drivers properly rejects unsupported flower control flags
>> used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar
>> checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL.
> 
> Thanks for doing this work!

Thank you, for maintaining the tree!

> Do you have any more of such series left?

Not at the moment, I only stumbled upon this, because I read the code
with an eye on adding a new IS_JUMBO_FRAME flag (patches for which are
almost ready).

Once this[1] currently RFC patch goes in, I might try to move all
control flags in under FLOW_DIS_F_*, to get rid of the then
FLOW_DIS_(IS_FRAGMENT|FIRST_FRAG|ENCAPSULATION|F_*).

[1] [RFC PATCH net-next 1/9] net/sched: flower: define new tunnel flags
https://lore.kernel.org/netdev/20240611235355.177667-2-ast@fiberby.net/

> Could we perhaps consider
> recording the driver support somewhere in struct net_device and do
> the rejecting in the core?

Sure, it could work for the control flags, and used_keys validation,
but I am not sure if it is worth it, as most of the validation is
very specific to the limitations of the different hardware. An easy
first step in that direction would be to move the used_keys checks
behind a helper, and possibly storing used_keys in struct net_device.

> That's much easier to extend when adding
> new flags than updating all the drivers.

That's how it is now, with the new helpers, as all flags are
unsupported, unless the driver specifically supports it.

Any new flag only needs to be added to the core, and drivers only needs
to be updated when they implement offloading support for a flag.

> This series I think may not be a great first candidate as IIUC all
> drivers would reject so the flag would be half-dead...

Correct. I don't know if there is any hardware support planned for the
tunnel-related encapsulation control flags.

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541

Re: [PATCH net-next 0/5] net: flower: validate encapsulation control flags

[patchwork-bot+netdevbpf]

Hello:

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

On Sun,  9 Jun 2024 17:33:50 +0000 you wrote:
> Now that all drivers properly rejects unsupported flower control flags
> used with FLOW_DISSECTOR_KEY_CONTROL, then time has come to add similar
> checks to the drivers supporting FLOW_DISSECTOR_KEY_ENC_CONTROL.
> 
> There are currently just 4 drivers supporting this key, and
> 3 of those currently doesn't validate encapsulated control flags.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] flow_offload: add encapsulation control flag helpers
    https://git.kernel.org/netdev/net-next/c/b48a1540b73a
  - [net-next,2/5] sfc: use flow_rule_is_supp_enc_control_flags()
    https://git.kernel.org/netdev/net-next/c/2ede54f8786f
  - [net-next,3/5] net/mlx5e: flower: validate encapsulation control flags
    https://git.kernel.org/netdev/net-next/c/28d19ec91755
  - [net-next,4/5] nfp: flower: validate encapsulation control flags
    https://git.kernel.org/netdev/net-next/c/34cdd9847820
  - [net-next,5/5] ice: flower: validate encapsulation control flags
    https://git.kernel.org/netdev/net-next/c/5a1b015d521d

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