On 5/28/2026 3:13 PM, Bruce Richardson wrote:
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"?
Sure, yes, a rulle cannot be both egress and ingress.
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?
Rules cannot be both ingress and egress, and they cannot be neither
ingress and egress. That leaves two options: either the rule is ingress,
or the rule is egress.
If the rule isn't ingress, it by definition will be egress in order to
pass earlier checks. The intent is also that a rule that is "expected to
be egress" must be egress to be accepted.
However, now that I think of it, you're right in that the condition is
subtly wrong. By default, we expect the rule to be ingress (i.e. not
setting "expect egress" implies we are expecting ingress), but we only
*check* if the rule is egress - we would *not* check if the rule is
egress. I think the condition should be replaced with:
1) if attr_param == NULL, rule *cannot* ever be expected to be egress
because we specify egress expectations in attr_param - so, rule being
egress when attr_param == NULL is an error
2) if attr_param != NULL, egress *must* match expect_egress
So, basically:
if ((attr_param == NULL && attr->egress) || (attr->egress !=
attr_param->expect_egress)
this should better match the intent.
/Bruce
--
Thanks,
Anatoly
