Re: [RFC v2 0/4] flow_compile: textual flow rule compiler

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, May 06, 2026 at 05:06:47PM -0700, Stephen Hemminger wrote:
> Background
> ----------
> 
> Multiple efforts over the past few cycles have tried to make
> testpmd's flow rule grammar reusable from outside testpmd.
> External applications that need rte_flow want a documented way
> to turn human-written rules into the rte_flow_attr/item/action
> arrays accepted by rte_flow_create().
> 
> The most recent attempt is Lukas Sismis's series, currently at
> v12:
> 
>   http://patches.dpdk.org/project/dpdk/list/?series=37384
> 
> That series factors testpmd's existing cmdline_flow.c into a
> library and updates testpmd to consume it.  It works, but
> inherits two properties of cmdline_flow.c that I think are worth
> avoiding in a reusable library:
> 
>   - Coupling to librte_cmdline.  Even after the v12 split into
>     a "simple" part and a "cmdline" part, the parser is still
>     organized around testpmd's command interpreter, and v12 has
>     cmdline depending on ethdev to break a previous circular
>     dependency.  A library used by daemons, control planes, or
>     unit tests should not need that.
> 
>   - Ad-hoc grammar.  cmdline_flow.c implements parsing per-token
>     in long dispatch logic; the grammar emerges from the code
>     rather than being stated, and adding a new flow item
>     requires touching the parser.
> 
> This RFC explores a different shape and is posted to ask the
> list which one is preferred before more work goes into either.
> 
> I started a new green-field library for parsing flow rules
> (with AI assistance for the boilerplate).  It is young but
> passes tests and reviews clean under the project's AI review
> guidelines.
> 
> This series
> -----------
> 
> lib/flow_compile -- a small new library providing the same
> service via a pcap_compile()-style API:
> 
>     char errbuf[RTE_FLOW_COMPILE_ERRBUF_SIZE];
>     struct rte_flow_compile *fc = rte_flow_compile(rule, errbuf);
>     if (fc == NULL)
>             fail(errbuf);            /* "line:col: message" */
> 
>     rte_flow_compile_create(port_id, fc, &flow_error);
>     rte_flow_compile_free(fc);
> 
> Design properties:
> 
>   - Flex lexer plus bison grammar.  Both are reentrant
>     (%option reentrant, %define api.pure full), so multiple
>     compilations may run concurrently and the parser holds no
>     static mutable state.  The grammar itself is short
>     (~200 lines) because all per-type knowledge lives in
>     descriptor tables, not in productions.
> 
>   - Parser is driven entirely by descriptor tables of items and
>     actions.  Adding a new flow item is a table edit, not a
>     grammar change.  A custom-setter hook on each field is the
>     escape valve for layouts that don't fit a plain byte range
>     (bitfields, indirect arrays).
> 
>   - Dependencies: rte_ethdev (for rte_flow.h) and rte_net (for
>     MAC parsing).  No librte_cmdline.  Flex and bison are
>     required at build time to regenerate the lexer and parser;
>     if either tool is missing the library is silently skipped
>     via meson's has_flex_bison check, the same pattern other
>     DPDK components use for optional generators.
> 
>   - Per-allocation rte_zmalloc for spec/mask/last/conf payloads;
>     rte_flow_compile_free() walks the pattern and action arrays
>     and releases every non-NULL slot before freeing the arrays.
>     Parse-error paths use the same walker, so partially
>     constructed rules clean up uniformly.  ASan/LSan run clean
>     on the autotest, including the failure cases.
> 
> The grammar follows testpmd's syntax closely so familiar rules
> carry over:
> 
>     ingress pattern eth / ipv4 src is 10.0.0.1 / end
>     actions queue index 3 / count / end
> 
> and is documented as a formal BNF in the programmer's guide
> chapter (patch 2).
> 
> Initial coverage: eth, vlan, ipv4, ipv6, tcp, udp, vxlan,
> port_id, port_representor, represented_port items; drop,
> passthru, queue, mark, jump, count, port_id and representor
> variants, of_pop_vlan, vxlan_decap actions.  Variable-conf
> items and actions (RSS, RAW) need custom setters and are
> deferred to a follow-up.
> 
> What this RFC is *not*
> ----------------------
> 
> Not a replacement for cmdline_flow.c in testpmd.  If the shape
> here is acceptable, the next step is a separate series adding a
> "flow compile <port> <rule>" command in testpmd alongside the
> existing parser, so users can adopt the library incrementally
> without breaking scripts that depend on the current syntax.
> 
> What I'd like feedback on
> -------------------------
> 
> 1. API shape.  pcap_compile-style (one string -> opaque object ->
>    arrays) versus the three-call attr/pattern/actions form
>    Sismis's v12 exposes.  What does your application actually
>    want?
> 

For this, I wonder if we also could do with a second API for the creation
which takes a list of tokens rather than just a single string. Thinking
about integration with testpmd, or with apps which already have some
commandline interface which produces a list of tokens, having to re-stitch
the tokens together into one string seems awkward.

Also, have you already investigated how this might be integrated into
testpmd? Do we have the capability to pass multi-token strings via cmdline?

> 2. Library placement.  Stand-alone at lib/flow_compile/ versus
>    addition to lib/ethdev.  This series treats it as a
>    control-path parser layered on top of ethdev rather than
>    part of ethdev itself; v12 places its parser inside ethdev.
> 

+1 to external to ethdev

> 3. Table-driven extension model.  Is "to add a new flow item,
>    add a row to the descriptor table" the right contract?
>    Should the tables live alongside each rte_flow_item_*
>    definition in rte_flow.h, or in their own file as here?
> 
> 4. Build-tool dependency.  Flex and bison are not currently
>    required to build DPDK.  Adding a library that needs them
>    (with a clean has_flex_bison fallback so the rest of DPDK
>    still builds without them) is the cleanest way I see to get
>    a real grammar. If this gets used by testpmd then
>    what is now an optional dependency would get hardened in.
> 

Flex and bison are very common build tools. I don't see an issue with this
dependency.

> 5. Convergence.  If this design is preferred, I'm happy to
>    coordinate with Lukas to fold in the testpmd-side changes
>    from his series.
> 
> 6. Readability. AI generated code like this tends to be
>    either opaque or too verbose for humans. Often have to
>    nudge it into submission.
> 

For readability, can you (or the AI's working for you :-) ) split the main
patch into a couple of patches for easier review and comment. It's a very
large single patch to go through in one go.

/Bruce