On Wed, 16 Sep 2026 13:18:08 +0100 Anatoly Burakov <[email protected]> wrote:
> + > +/* > + * This is a common header for Intel Ethernet drivers' flow engine > + * implementations. It defines the interfaces and data structures required to > + * implement flow rule engines that can be plugged into the drivers' flow > + * handling logic. > + * > + * Design considerations: > + * > + * 1. Ease of implementation > + * > + * The flow engine interface is designed to be as simple as possible with > + * obvious defaults (i.e. not specifying something leads to behavior that > + * would've been the most expected in context). The point is not to produce a > + * monstrous driver-within-a-driver framework, but rather to make engine > + * definitions follow semantic expectations of what the engine actually does. > + * > + * All the boilerplate (flow management, engine enablement tracking, etc.) is > + * handled by the common flow infrastructure, so the engine implementation > only > + * needs to focus on the actual logic of parsing and installing/uninstalling > + * flow rules, and defining each step of the process as it pertains to each > flow > + * engine. > + * > + * It is expected that drivers will use other utility functions from the > common > + * flow-related code where applicable (e.g. flow_util.h, flow_check.h, etc.). > + * > + * 2. Full secondary process compatibility > + * > + * In order to support rte_flow operations in secondary processes, we need to > + * store which engines are enabled for particular driver instance, and > resolve > + * them at runtime. The engine index (its position in the engine list) is > used as > + * a bit position in a driver-specific 64-bit field of enabled engines. This > + * way, the engine definitions can be stored in read-only memory, and > referenced > + * by both primary and secondary processes without issues. > + * > + * For this to remain safe, flow engine lists and engine definitions must be > + * immutable for process lifetime (declare them as const). > + * > + * Note that this does not imply that all drivers are therefore able to > support > + * rte_flow-related operations in secondary processes - that is still up to > each > + * driver to implement. This just ensures that the flow engine framework does > + * not prevent it. > + * > + * Engine callbacks must not access or retain an `struct rte_eth_dev *` > pointer, > + * as that object is process-local; use the process-independent > + * `struct rte_eth_dev_data *` provided by the framework instead. > + * > + * The per-instance engine configuration is set up and torn down exclusively > by > + * `ci_flow_engine_conf_init()` and `ci_flow_engine_conf_reset()`. These > functions > + * should only be called at device setup/teardown by primary process. > + * > + * 3. Flow object lifecycle is framework-owned > + * > + * Engines are expected to treat framework-provided context and flow objects > as > + * storage they fill in, not storage they own. In other words, engine logic > + * should focus on contents of flow data, while object lifetime is managed by > + * the framework. Engines may still allocate auxiliary data, but only in > places > + * where the framework guarantees a matching teardown call, which will give > the > + * engine the opportunity to release said auxiliary data. > + * > + * 4. Pattern parsing: flow_graph and pattern_parse callback > + * > + * The flow engine framework is designed to work hand-in-hand with the > + * `flow_graph` parsing infrastructure. Each engine may provide a pattern > + * graph that is used to match the flow pattern, and extract relevant data > + * into the engine context provided by the framework. > + * > + * Engines may also provide a `pattern_parse` callback that is invoked before > + * the graph parser runs. This allows engines to handle pattern items that > + * don't fit neatly into the graph model (e.g. FUZZY items that can appear at > + * any position), as well as ignoring the graph parser entirely and > implementing > + * custom pattern parsing. > + * > + * There is no way to completely ignore pattern contents for the engine > except > + * for defining a noop `pattern_parse` callback. This is by design, as such > case > + * is considered rte_flow API misuse. By default, even for empty fallback > case, > + * a meaningful pattern (one that is not empty or ANY) will be treated as > error. > + * > + * 5. Setup, teardown, and flow list lifecycle ordering > + * > + * `ci_flow_engine_conf_init()` and `ci_flow_engine_conf_reset()` are > + * primary-process-only (see point 2), and the framework does not serialize > + * them against concurrent flow operations or against each other - the driver > + * must do so. > + * > + * The expected sequence of calls for a driver instance is: > + * > + * - `ci_flow_engine_conf_init()` should run from the driver's `dev_init` > path. > + * > + * - At `dev_close`, `ci_flow_cleanup()` should be run first to drop all > flows > + * and their internal tracking. Then, `ci_flow_engine_conf_reset()` can be > run. > + * > + * - Devices may or may not advertise `RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP`, > + * i.e. support for keeping flow rules across a `dev_stop`/`dev_start` > + * cycle. > + * > + * - If the device does not advertise this capability, flows must be > + * flushed via `ci_flow_flush()` as the first step of `dev_stop()` (doing > so > + * later may interfere with flow uninstall). > + * > + * - If rule replay is needed (i.e. flows were kept rather than flushed at > + * `dev_stop`), the driver should call `ci_flow_replay()` from `dev_start` > + * to re-install the kept flows to hardware as last step. > + * This is excess commenting, which is the kind of thing AI likes to generate unless you tell to get to the point. Even AI evaluating itself said: Warning Comments are far longer than the code needs. Roughly a third of flow_engine.h (587 of 1620 lines) and flow_graph.h (175 of 507) is comment text, and many comments narrate the obvious or restate the design document. Examples: flow_graph.h, _flow_graph_node_is_expected(): /* * In the interest of everyone debugging flow parsing code, we should * provide the user with meaningful messages about exactly what failed, * as no one likes non-descript "node constraints not met" errors with * no clear indication of where this is even coming from. What follows * is us building said meaningful error messages. It's a bit ugly, but * it is for the greater good. */ flow_engine.h: the file header and the struct ci_flow_engine_ops comment together run about 290 lines of design essay. ci_flow_parse() repeats the same match-mode table that already appears above ci_flow_engine_ops. ixgbe_flow_dev_dump() and i40e_flow_dev_dump() (patches 4 and 13) carry a 13-line comment explaining one if statement. i40e_fdir_flow_install() and i40e_fdir_flow_register() (patch 16) open with paragraph-length comments before a single condition. Short one-line comments are enough for straightforward code. Put the design description in flow_graph.rst (or a short block at the top of flow_engine.h) once, and drop the per-function restatements. Trivial comments such as "/* success */", "/* is the pointer valid? */" and "/* engine looks valid */" can go.
