On 9/16/2026 1:18 PM, Anatoly Burakov wrote:
Current implementation of flow engines in various drivers have a few issues
that need to be corrected.
For one, some of the
seems like a missed word?
are fundamentally incompatible with secondary
processes, because the flow engine registration and creation will
allocate structures in shared memory but use process-local pointers to
point to flow engines and pattern tables.
For another, a lot of them are needlessly complicated and rely on a
separation between patterns and parsing that is hard to reason about and
maintain: they do not define memory ownership model, they do not define the
way in which we approach parameter and pattern parsing, and they
occasionally do weird things like passing around pointers-to-void-pointers
or even using pointers as integer values.
Another common problem is extremely convoluted internal tracking, flow
installation, flow replay, and cleanup code. This infrastructure is usually
done in an ad-hoc manner that has a lot of boilerplate.
These issues can be corrected, but because of how much code there is to the
current infrastructure and how tightly coupled it is, it would be easier to
just build new one from scratch, and gradually migrate all engines to use
it. This patch is intended as a first step towards that goal, and defines
both common data types to be used by all rte_flow parsers, as well as the
interaction model that is to be followed by all drivers.
We define a set of structures that will represent:
- Defined rte_flow parsing interaction model and code flow (ops struct)
- Defined memory allocation and ownership model for all engines
- Scratch space format for all engines (variably allocated typed struct)
- Flow rule format for all engines (variably allocated typed struct)
- Engine definitions that are compatible with secondary process model
- Implementations of common rte_flow operations
- Various supporting infrastructure for parser customization, e.g. hooks
- Support for using custom allocation (e.g. for mempool-based alloc)
- Support for replaying all flows to restore HW state
- Support for removing all flows without modifying HW state
The design intent is heavily documented right inside the header and is to
be considered authoritative design document for how to build rte_flow
parsers for Intel Ethernet drivers going forward.
Signed-off-by: Anatoly Burakov<[email protected]>
---
<snip>
+/* enable all engines for a specific driver instance - caller must serialize
initialization */
+static inline int
+ci_flow_engine_conf_init(struct ci_flow_engine_conf *engine_conf,
+ const struct ci_flow_engine_list *engine_list,
+ struct rte_eth_dev_data *dev_data)
+{
+ struct ci_flow_engine_ref engine_ref;
+
+ /* reject invalid configuration */
+ if (engine_conf == NULL || engine_list == NULL || dev_data == NULL)
+ return -1;
return -EINVAL?
+
+ /* init the lock */
+ rte_rwlock_init(&engine_conf->config_lock);
+
<snip>
+/* parse a flow using a specific engine - caller must hold config lock */
+static inline int
+ci_flow_parse(const struct ci_flow_engine_conf *engine_conf,
+ const struct ci_flow_engine *engine,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ struct ci_flow *flow,
+ struct rte_flow_error *error)
+{
+ enum ci_match_type match_type;
+ struct ci_flow_engine_ctx *ctx;
+ int ret = 0;
+
+ /*
+ * Determine the type of matching we are going to perform based on the
+ * presence of pattern graph and pattern_parse callback. The logic is as
+ * follows:
+ *
+ * - if graph but no callback, match against graph
+ *
+ * Expected default case: pattern matching is graph based, no special
+ * handling for any pattern items.
+ *
+ * - if both graph and callback, match against callback + graph
+ *
+ * Preprocessor case, i.e. preprocess the pattern with the callback
+ * before handling the matching to the graph engine. The assumption is
+ * that the graph will be set up with a proper ignore list to skip over
+ * nodes that weren't meant for the graph processing.
+ *
+ * - if no graph but callback, match against callback
+ *
+ * Fully custom pattern parsing case.
+ *
+ * - if no graph and no callback, match against empty graph
+ *
+ * "Pattern is not meaningful" case, for engines that do not care about
+ * the pattern at all. A default matching behavior against empty
+ * patterns is provided (i.e. allow NULL pattern, and allow END or ANY
+ * -> END patterns). Note that this is not the same as ignoring pattern
+ * entirely: the engine will still reject patterns that are not empty.
+ */
+ match_type = engine->graph == NULL ?
+ (engine->ops->pattern_parse == NULL ? CI_MATCH_EMPTY :
CI_MATCH_CALLBACK) :
+ (engine->ops->pattern_parse == NULL ? CI_MATCH_GRAPH :
CI_MATCH_ALL);
+
+ CI_DRV_LOG(DEBUG, "engine '%s': parsing flow", engine->name);
+
+ /* allocate context */
+ ctx = (struct ci_flow_engine_ctx *)calloc(1,
+ RTE_MAX(engine->ctx_size, sizeof(struct
ci_flow_engine_ctx)));
+ if (ctx == NULL) {
+ return rte_flow_error_set(error, ENOMEM,
+ RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+ "Failed to allocate memory for rule engine
context");
+ }
+ ctx->dev_data = engine_conf->dev_data;
+ ctx->attr = attr;
+ ctx->pattern = pattern;
+ ctx->actions = actions;
+ flow->dev_data = engine_conf->dev_data;
it was set in ci_flow_alloc()
+
+ /* parse flow parameters */
+ ret = engine->ops->ctx_init(actions, attr, ctx, error);
<snip>
+
+ /* no engine could handle this flow */
+ CI_DRV_LOG(DEBUG, "no engine accepted the flow");
+ flow = NULL;
+ rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+ "No flow engine could handle the requested flow");
This unconditionally rewrites error string, don't we want to keep the
last reason why flow wasn't created? Same is applied for validate.
+unlock:
+ rte_rwlock_write_unlock(&engine_conf->config_lock);
+
+ return (struct rte_flow *)flow;
+}
+
<snip>
--
Regards,
Vladimir
