On Thu, May 28, 2026 at 02:39:47PM +0200, Burakov, Anatoly wrote: > On 5/27/2026 3:28 PM, Bruce Richardson wrote: > > On Mon, May 25, 2026 at 03:06:23PM +0100, Anatoly Burakov wrote: > > > There are a lot of commonalities between what kinds of flow attr each > > > Intel > > > driver supports. Add a helper function that will validate attr based on > > > common requirements and (optional) parameter checks. > > > > > > Things we check for: > > > - Rejecting NULL attr (obviously) > > > - Default to ingress flows > > > - Transfer, group, priority, and egress are not allowed unless requested > > > > > > Signed-off-by: Anatoly Burakov <[email protected]> > > > --- > > > drivers/net/intel/common/flow_check.h | 69 +++++++++++++++++++++++++++ > > > 1 file changed, 69 insertions(+) > > > > > > diff --git a/drivers/net/intel/common/flow_check.h > > > b/drivers/net/intel/common/flow_check.h > > > index 74fb28ae3d..0572028664 100644 > > > --- a/drivers/net/intel/common/flow_check.h > > > +++ b/drivers/net/intel/common/flow_check.h > > > @@ -54,6 +54,7 @@ ci_flow_action_type_in_list(const enum > > > rte_flow_action_type type, > > > /* Forward declarations */ > > > struct ci_flow_actions; > > > struct ci_flow_actions_check_param; > > > +struct ci_flow_attr_check_param; > > > static inline const char * > > > ci_flow_action_type_to_str(enum rte_flow_action_type type) > > > @@ -271,6 +272,74 @@ ci_flow_check_actions(const struct rte_flow_action > > > *actions, > > > return parsed_actions->count == 0 ? -EINVAL : 0; > > > } > > > +/** > > > + * Parameter structure for attr check. > > > + */ > > > +struct ci_flow_attr_check_param { > > > + bool allow_priority; /**< True if priority attribute is allowed. */ > > > + bool allow_transfer; /**< True if transfer attribute is allowed. */ > > > + bool allow_group; /**< True if group attribute is allowed. */ > > > + bool expect_egress; /**< True if egress attribute is expected. */ > > > +}; > > > + > > > +/** > > > + * Validate rte_flow_attr structure against specified constraints. > > > + * > > > + * @param attr Pointer to rte_flow_attr structure to validate. > > > + * @param attr_param Pointer to ci_flow_attr_check_param structure > > > specifying constraints. > > > + * @param error Pointer to rte_flow_error structure for error reporting. > > > + * > > > + * @return 0 on success, negative errno on failure. > > > + */ > > > +static inline int > > > +ci_flow_check_attr(const struct rte_flow_attr *attr, > > > + const struct ci_flow_attr_check_param *attr_param, > > > + struct rte_flow_error *error) > > > +{ > > > + if (attr == NULL) { > > > + return rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ATTR, attr, > > > + "NULL attribute"); > > > + } > > > + > > > + /* Direction must be either ingress or egress */ > > > + if (attr->ingress == attr->egress) { > > > + return rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ATTR, attr, > > > + "Either ingress or egress must be > > > set"); > > > + } > > > + > > > + /* Expect ingress by default */ > > > + if (attr->egress && (attr_param == NULL || !attr_param->expect_egress)) > > > { > > > + return rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, attr, > > > + "Egress not supported"); > > > + } > > > > I think "allow_egress" is possibly a better name here. "expect_egress" > > implies that egress must be present, but the logic here seems to be only > > allowing egress if the flag is provided. > > No, the check right above this prevents unspecified egress *if* ingress is > also specified.
So the error message above should be "Either ingress or egress must be set, but not both"? > Meaning, one of egress/ingress *must* be specified, and if > it's egress, it is only allowed when we are expecting egress. > Yes, so it's either ingress or egress but egress is only allowed in some cases. But nothing in this check prevents ingress alone when expect_egress is set, so egress is allowed but not required, so I still think "allow_egress" is a better name. "expect_egress" is a bit ambivilent, in that it's unclear how strong that expectation is - is it an error, or just a warning if the expectation is not met, for example? /Bruce
