From: Michael Pfeiffer <[email protected]>

For a MODIFY_FIELD action, flow_hw_validate_action_modify_field() is
invoked and enforces class_id == 0 in the action's source and
destination, if the modified field is none of
RTE_FLOW_FIELD_GENEVE_OPT_*, as the value is used solely for GENEVE
fields.

However, this check is flawed due to the way rte_flow_field_data is
initialized. As it consists of unions and anonymous structs as members,
empty initialization of this struct or initializing just the tag_index
only guarantees initialization of the first union member, while the
remaining member's default initialization behavior is unspecified.
Therefore, depending on the compiler type, version and configuration,
the remaining members may either be default-initialized as well or
contain bytes from uninitialized memory. This causes the check to fail
depending on how the struct is initialized wherever it is used.

For example, rte_flow_configure() sometimes fails on mlx5 under these
circumstances with an error "destination class id is not supported"
during creation of representor tagging rules, as these internally use
MODIFY_FIELD actions in the following call stack:

  1. rte_flow_configure
  2. mlx5_flow_port_configure
  3. flow_hw_configure
  4. __flow_hw_configure
  5. flow_hw_setup_tx_repr_tagging
  6. flow_hw_create_tx_repr_tag_jump_acts_tmpl
     --> various rte_flow_action_modify_field are initialized here, but
         class_id remains uninitialized
  7. __flow_hw_actions_template_create
  8. mlx5_flow_hw_actions_validate
  9. flow_hw_validate_action_modify_field
     --> invoked with class_id containing uninitialized bytes and
         non-GENEVE field type

Remove the two checks for class_id in the non-GENEVE case, as this field
is unused for these actions and avoids additional implicit dependencies
on the correct ordering of union members.

Fixes: 1caa89ec1891 ("net/mlx5: support GENEVE options modification")
Cc: [email protected]

Signed-off-by: Michael Pfeiffer <[email protected]>
Signed-off-by: Adrian Schollmeyer <[email protected]>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index bca5b2769e..add48d7c8f 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -6264,10 +6264,6 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev 
*dev,
                        return rte_flow_error_set(error, EINVAL,
                                        RTE_FLOW_ERROR_TYPE_ACTION, action,
                                        "destination tag index is not 
supported");
-               if (action_conf->dst.class_id)
-                       return rte_flow_error_set(error, EINVAL,
-                                       RTE_FLOW_ERROR_TYPE_ACTION, action,
-                                       "destination class id is not 
supported");
        }
        if (mask_conf->dst.level != UINT8_MAX)
                return rte_flow_error_set(error, EINVAL,
@@ -6290,10 +6286,6 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev 
*dev,
                                return rte_flow_error_set(error, EINVAL,
                                        RTE_FLOW_ERROR_TYPE_ACTION, action,
                                        "source tag index is not supported");
-                       if (action_conf->src.class_id)
-                               return rte_flow_error_set(error, EINVAL,
-                                       RTE_FLOW_ERROR_TYPE_ACTION, action,
-                                       "source class id is not supported");
                }
                if (mask_conf->src.level != UINT8_MAX)
                        return rte_flow_error_set(error, EINVAL,
-- 
2.34.1

Reply via email to