On Fri,  9 Oct 2026 11:58:50 +0100
Anatoly Burakov <[email protected]> wrote:

> even though 99% of what people use rte_flow parsers for is parsing protocol
> graphs, no parser is written explicitly as a graph. This patchset attempts to
> suggest a viable model to build rte_flow parsers as graphs, by offering a
> library to build rte_flow parser graphs without too much boilerplate and
> complexity.
> 
> Most of the patchset is about Intel drivers, but they are meant as
> reimplementations as well as examples for the rest of the community to assess
> how to build parsers using this new infrastructure. I expect the first three
> patches will be of most interest to non-Intel reviewers, as they deal with
> building two reusable parser architecture pieces.
> 
> The first piece is a new flow graph helper in ethdev. Its purpose is
> deliberately narrow: it targets the protocol-graph part of rte_flow pattern
> parsing, where drivers walk packet headers and validate legal item sequences 
> and
> parameters. That does not cover all possible rte_flow features, especially 
> more
> exotic flow items, but it does cover a large and widely shared part of what
> existing drivers need to do. Or, to put it in other words, the only flow items
> this infrastructure *doesn't* cover is things that do not lend themselves well
> to be parsed as a graph of protocol headers (e.g. conntrack items). Everything
> else should be covered or cover-able. In practice, just about all drivers will
> benefit from graph parsing as all but one of them implement only the protocol
> stack parts, which are the ones targeted by the graph helper.
> 
> The second piece is a reusable flow engine framework for Intel Ethernet 
> drivers.
> This is kept Intel-local because I do not feel it is even appropriate to 
> define
> such a framework for all drivers to use in the first place. Even so, the 
> intent
> is to establish a cleaner parser architecture with a defined interaction 
> model,
> explicit memory ownership rules, locking, initialization sequence,
> implementations of rte_flow API entry points, flow replay and memory cleanup,
> and engine definitions that do not block secondary-process-safe usage. It is 
> my
> hope that this would serve as a model for other drivers to follow, expand on,
> rework, and improve, so that maybe down the line we *might* have a common
> rte_flow infrastructure for drivers to use.
> 
> Most of the rest of the series is parser reimplementation, but that is mainly
> the vehicle for demonstrating and validating those two pieces. ixgbe and i40e
> are wired into the new common parsing path, and their existing parsers are
> migrated incrementally to the graph-based model. Besides reducing ad hoc 
> parser
> code, this also makes validation more explicit and more consistent. In a few
> places that means invalid inputs that were previously ignored, deferred, or
> interpreted loosely are now rejected earlier and more strictly, without any
> increase in code complexity (in fact, with marked *decrease* of it!).
> 
> v5 -> v6:
> - Renamed all underscored functions in flow graph
> - Reclassified cleanup/replay as engine conf operations
> - Added better logging to flow graph parse function


AI review still saw some open issues.

Subject: Re: [PATCH v6 00/25] Building a better rte_flow parser

The series does not apply to main; 10/25, 15/25 and 19/25 conflict
(based on next-net-intel). After resolving those by hand every commit
builds with -Dwerror=true for ethdev, ixgbe and i40e.

The flow_graph/engine framework is a good direction and the per-engine
split is much easier to follow than the old parsers. Most of the
problems below are in the conversion, not the framework. The biggest
ones: i40e rte_flow_validate() uses the wrong private pointer, the new
hash table names collide between ports of the same NIC, the ixgbe
tunnel FDIR graph rejects every inner-ETH pattern, and the i40e hash
VLAN graph reads past the end of its node array.


[PATCH v6 02/25] ethdev: add flow graph API

Info: typos in flow_graph.rst: "The the ``END`` node" and "used by the
driver to programming hardware with".

Info: copyright year in flow_graph.c/.h is 2025, new files in 2026.


[PATCH v6 03/25] net/intel/common: add flow engines infrastructure

Info: ci_flow_flush() logs uninstall failures at DEBUG and returns 0,
so a filter left in hardware is invisible. Log at ERR and return the
first error after the loop has finished.

Info: typo "commont" in ci_flow_alloc().


[PATCH v6 06/25] net/ixgbe: make ethertype filter table dynamic

Error: ixgbe_ethertype_filter_restore() skips slots by index without
checking whether timesync/antispoof are installed:

        if (i == filter_info->timesync_idx ||
                        i == filter_info->antispoof_idx||

Both indexes default to 0 (and stay stale after timesync_disable), so
an rte_flow ethertype filter in slot 0 is not restored on dev_start.
This patch is Cc stable, so it matters even though 07/25 removes the
function. ixgbe_clear_all_ethertype_filter() already does it right:

        if ((filter_info->timesync_installed &&
             i == filter_info->timesync_idx) ||
            (filter_info->antispoof_installed &&
             i == filter_info->antispoof_idx) ||
            !filter_info->ethertype_table.entries[i].used)
                continue;


[PATCH v6 13/25] net/ixgbe: reimplement ntuple parser

Warning: the L4 protocol is only set when the TCP/UDP/SCTP item has no
spec. "eth / ipv4 / tcp dst is 80" (no next_proto_id in IPv4) leaves
proto_mask 0, so the 5-tuple filter matches UDP and SCTP port 80 too.
This is pre-existing, but 12/25 only fixes the empty-item case. Call
ixgbe_ntuple_set_l4_proto() unconditionally in the three process
callbacks, then fill the ports when the mask is non-NULL.

Info: the priority conversion comment has broken indentation on its
first two lines.


[PATCH v6 14/25] net/ixgbe: reimplement security parser

Error: ixgbe_crypto_clear_ipsec_tables() now clears only entry 0:

        *priv->tx_sa_tbl = (struct ixgbe_crypto_tx_sa_table){0};

tx_sa_tbl is an array of IPSEC_MAX_SA_COUNT; the other entries keep
used=1 and leak Tx SA slots. Keep the memset:

        memset(priv->tx_sa_tbl, 0, sizeof(priv->tx_sa_tbl));


[PATCH v6 15/25] net/ixgbe: reimplement FDIR parser

Error: IXGBE_FDIR_TUNNEL_NODE_INNER_IPV4 has no edges entry.
flow_graph_find_next_node() runs flow_graph_node_is_valid() on each
candidate edge before comparing types, and INNER_ETH lists INNER_IPV4
before END. So every tunnel pattern that does not continue with VLAN
after the inner ETH (e.g. eth / ipv4 / udp / vxlan / eth / end) fails
with "Flow graph edge list is not defined for non-END node". Add:

        [IXGBE_FDIR_TUNNEL_NODE_INNER_IPV4] = {
                .next = (size_t[]) {
                        IXGBE_FDIR_TUNNEL_NODE_END,
                        FLOW_GRAPH_NODE_EDGE_END
                }
        },

Error: "ixgbe_fdir_hash_%s" with a PCI name is 28 characters. rte_hash
names its ring "HT_<name>" and RTE_RING_NAMESIZE is 29, so the name is
cut to "HT_ixgbe_fdir_hash_0000:81:0" and port 1 of a dual-port NIC
fails with EEXIST. Verified with rte_hash_create() for 0000:81:00.0
and 0000:81:00.1. The old "fdir_%s" fits. The usable name length is
26 characters; keep the old prefix or use the port id.


[PATCH v6 19/25] net/i40e: add support for common flow parsing

Error: i40e_flow_validate() (and i40e_flow_query() in this patch) use

        struct i40e_pf *pf = dev->data->dev_private;

dev_private is struct i40e_adapter, whose first member is struct
i40e_hw, so &pf->flow_engine_conf points into the hw struct. 24/25
fixes query; validate is still wrong at the end of the series. Use
I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private) as the other ops do.


[PATCH v6 20/25] net/i40e: reimplement ethertype parser

Error: "i40e_ethertype_hash_%s" is 32 characters with a PCI name. The
snprintf already truncates it to "i40e_ethertype_hash_0000:81:00.", so
both ports of a NIC get the same name and the second hash create
fails. Same fix as 15/25.


[PATCH v6 21/25] net/i40e: refactor FDIR engine infrastructure

Error: i40e_fdir_tmpl_del() and the error path of i40e_fdir_tmpl_add()
call i40e_fdir_tmpl_unregister(), which decrements fdir_rule_count,
before checking ci_usage_count_is_last(). After the decrement,
is_last means one other rule remains:

 - deleting the only rule leaves count 0, is_last is false, and FDIR
   is never torn down (hw_configured stays true);
 - deleting a template while one other FDIR rule exists tears FDIR
   down under that rule.

i40e_fdir_flow_uninstall() does it correctly because it runs before
unregister. Check before unregistering:

        bool last = ci_usage_count_is_last(&pf->fdir.fdir_rule_count);

        ret = i40e_fdir_tmpl_unregister(dev, node);
        if (ret < 0)
                return ret;
        TAILQ_REMOVE(&pf->fdir.tmpls.list, node, rules);
        if (last && pf->fdir.hw_configured) {
                i40e_fdir_teardown(pf);
                pf->fdir.hw_configured = false;
        }

Same for the error path in i40e_fdir_tmpl_add(). Alternatively, test
fdir_rule_count == 0 after unregister.

Error: "i40e_fdir_hash_%s" (27 characters) collides the same way as
in 15/25: the ring name becomes "HT_i40e_fdir_hash_0000:81:00".


[PATCH v6 22/25] net/i40e: reimplement FDIR parser

Error: i40e_fdir_configure() now writes the default flex layout through
i40e_fdir_flex_pit_program(), which writes GLQF_ORT(33 + layer) for
all three layers. GLQF_ORT is global and shared by every PF on the
NIC. The old i40e_init_flx_pld() only wrote the per-port PRTQF_FLX_PIT
registers and touched GLQF_ORT only for a flex rule. Now the first FDIR
rule of any kind on PF1 resets the ORT entries a flex rule on PF0
depends on, and this also happens with support-multi-driver set.
Teardown and the last-flex-rule path do the same. Program only
PRTQF_FLX_PIT for the default layout, and write GLQF_ORT only when a
flex rule needs it (and not at all with support_multi_driver, or
only when the value matches).

Warning: i40e_fdir_node_eth_validate() rejects an ETH mask with both
MAC masks zero. The old parser accepted ether_type-only L2 FDIR rules
(eth type is 0x88f7 / end with MARK or PASSTHRU, which the ethertype
engine does not take). Reject only when MACs and ether_type are all
unmasked.

Warning: RAW validation rejects any non-zero mask on relative, search,
offset or limit. testpmd "raw relative is 1 ... offset is 2 ..." sets
those masks to all-ones, so the usual flex-payload command now fails.
The old parser ignored the control-field masks. Accept zero or full
masks there (or ignore them as before); only the spec values matter.


[PATCH v6 23/25] net/i40e: reimplement tunnel parsers

Error: "i40e_tunnel_hash_%s" (29 characters) collides the same way as
in 15/25: the ring name becomes "HT_i40e_tunnel_hash_0000:81:".


[PATCH v6 24/25] net/i40e: reimplement hash parser

Error: i40e_hash_vlan_graph.nodes defines only START and VLAN, but the
VLAN edge list points at I40E_HASH_VLAN_NODE_END. The compound literal
has two elements, so &graph->nodes[2] is out of bounds for any
"vlan / end" RSS rule. Add the END node:

        [I40E_HASH_VLAN_NODE_END] = {
                .name = "END",
                .type = RTE_FLOW_ITEM_TYPE_END,
        },

Warning: the GTPC and GTPU rows are duplicated with IPv4 and IPv6 masks,
but i40e_hash_pattern_ctx_finalize() returns an error on the first row
whose flags do not cover the requested types. GTPC/GTPU over IPv6 with
IPv6 RSS types never reaches the second row and is rejected; the old
match loop tried each row. Use a single row per pctype with
I40E_HASH_IPV4_L234_RSS_MASK | I40E_HASH_IPV6_L234_RSS_MASK, or pick
the mask from the outer IP version.

Warning: eth / ipv4 and eth / ipv6 now map only to NONF_IPV*_OTHER. The
old table also configured FRAG_IPV4/FRAG_IPV6 for these patterns, so
"rss types ipv4 end" no longer hashes fragmented packets and
"rss types ipv4-frag end" on eth / ipv4 is rejected. Configure both
pctypes as before (for IPv6 the FRAG_EXT node can stay as is).

Reply via email to