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
