[PATCH v6 0/6] flow_parser: add shared parser library
Stephen Hemminger
stephen at networkplumber.org
Tue Jan 27 23:18:05 CET 2026
On Tue, 27 Jan 2026 22:05:25 +0100
Lukas Sismis <sismis at dyna-nic.com> wrote:
> This series extracts the testpmd flow CLI parser into a reusable library,
> enabling external applications to parse rte_flow rules using testpmd syntax.
>
> Motivation
> ----------
> External applications like Suricata IDS [1] need to express hardware filtering
> rules in a consistent, human-readable format. Rather than inventing custom
> syntax, reusing testpmd's well-tested flow grammar provides immediate
> compatibility with existing documentation and user knowledge.
>
> Note: This library provides only one way to create rte_flow structures.
> Applications can also construct rte_flow_attr, rte_flow_item[], and
> rte_flow_action[] directly in C code.
>
> Design
> ------
> The library (librte_flow_parser) exposes the following APIs:
> - rte_flow_parser_parse_attr_str(): Parse attributes only
> - rte_flow_parser_parse_pattern_str(): Parse patterns only
> - rte_flow_parser_parse_actions_str(): Parse actions only
>
> Testpmd is updated to use the library, ensuring a single
> maintained parser implementation.
>
> Testing and Demo
> -------
> - Functional tests in dpdk-test
> - Example application: examples/flow_parsing
>
> Changes
> -------
>
> v6:
> - Inconsistent Experimental API Version adjusted
> - Fixes Tag added to MSVC build commit
> - Non-Standard Header Guards updated
> - Implicit Pointer Comparison and Return Type issues addressed in many places
> - commit message in patch 6 updated
>
> v5:
> - removed/replaced (f)printf code from the library
> - reverted back to exporting the internal/private API as it is needed by
> testpmd and cannot be easily split further.
> - adjusted length of certain lines
> - marking port/queue id typedef as experimental
> - updated release rel_notes
> - copyeright adjustments
>
>
> v4:
> - ethdev changes in separate commit
> - library's public API only exposes attribute, pattern and action parsing,
> while the full command parsing is kept internal for testpmd usage only.
> - Addressed Stephen's comments from V3
> - dpdk-test now have tests focused on public and internal library functions
>
> v3:
> - Add more functional tests
> - More concise MAINTAINERS updates
> - Updated license headers
> - A thing to note: When playing with flow commands, I figured, some may use
> non-flow commands, such as raw decap/encap, policy meter and others.
> Flow parser library itself now supports `set` command to set e.g. the decap/
> encap parameters, as the flow syntax only supports defining the index of the
> encap/decap configs. The library, however, does not support e.g. `create`
> command to create policy meters, as that is just an ID and it can be created
> separately using rte_meter APIs.
>
> [1] https://github.com/OISF/suricata/pull/13950
>
> Lukas Sismis (6):
> cmdline: include stddef.h for MSVC compatibility
> ethdev: add RSS type helper APIs
> flow_parser: add shared parser library
> app/testpmd: use shared flow parser library
> examples/flow_parsing: add flow parser demo
> test: add flow parser library functional tests
>
> MAINTAINERS | 6 +-
> app/test-pmd/cmd_flex_item.c | 41 +-
> app/test-pmd/cmdline.c | 267 +-
> app/test-pmd/config.c | 112 +-
> app/test-pmd/flow_parser.c | 409 +
> app/test-pmd/flow_parser_cli.c | 153 +
> app/test-pmd/meson.build | 5 +-
> app/test-pmd/testpmd.c | 4 +
> app/test-pmd/testpmd.h | 126 +-
> app/test/meson.build | 1 +
> app/test/test_ethdev_api.c | 51 +
> app/test/test_flow_parser.c | 923 ++
> doc/api/doxy-api-index.md | 1 +
> doc/api/doxy-api.conf.in | 1 +
> doc/guides/prog_guide/flow_parser_lib.rst | 111 +
> doc/guides/prog_guide/index.rst | 1 +
> doc/guides/rel_notes/release_26_03.rst | 21 +
> examples/flow_parsing/main.c | 291 +
> examples/flow_parsing/meson.build | 11 +
> examples/meson.build | 1 +
> lib/cmdline/cmdline_parse.h | 2 +
> lib/ethdev/rte_ethdev.c | 107 +
> lib/ethdev/rte_ethdev.h | 77 +
> lib/flow_parser/meson.build | 7 +
> .../flow_parser/rte_flow_parser.c | 11430 ++++++++--------
> lib/flow_parser/rte_flow_parser.h | 124 +
> lib/flow_parser/rte_flow_parser_private.h | 1247 ++
> lib/meson.build | 2 +
> 28 files changed, 9698 insertions(+), 5834 deletions(-)
> create mode 100644 app/test-pmd/flow_parser.c
> create mode 100644 app/test-pmd/flow_parser_cli.c
> create mode 100644 app/test/test_flow_parser.c
> create mode 100644 doc/guides/prog_guide/flow_parser_lib.rst
> create mode 100644 examples/flow_parsing/main.c
> create mode 100644 examples/flow_parsing/meson.build
> create mode 100644 lib/flow_parser/meson.build
> rename app/test-pmd/cmdline_flow.c => lib/flow_parser/rte_flow_parser.c (55%)
> create mode 100644 lib/flow_parser/rte_flow_parser.h
> create mode 100644 lib/flow_parser/rte_flow_parser_private.h
>
Automated review is now happy, and the bot is certainly pickier than me :-)
The word fixes in changelog tends to be treated special by check-git-log,
checkpatches and the AGENTS file.
Series-acked-by: Stephen Hemminger <stephen at networkplumber.org>
---
## DPDK Patch Review: flow_parser v6
### Summary
This is a 6-patch series that extracts the testpmd flow CLI parser into a reusable library (`librte_flow_parser`). Comparing v6 to v5, the changelog claims several fixes were addressed.
---
### Patch 1/6: `cmdline: include stddef.h for MSVC compatibility`
**Commit Message**: ✅ Good
- Subject: 49 chars, lowercase, proper `cmdline:` prefix
- Body wrapped correctly, describes the change well
- Has `Signed-off-by`
**Issue (Warning)**: The v6 changelog claims "Fixes Tag added to MSVC build commit", but **no Fixes tag is present** in the commit message. Since this is a preventive compatibility improvement rather than fixing a specific regression, a Fixes tag may not be required. However, the changelog description is misleading.
**Code**: ✅ Simple, correct change adding `#include <stddef.h>` before the fallback `offsetof` macro.
---
### Patch 2/6: `ethdev: add RSS type helper APIs`
**Commit Message**: ✅ Good
- Subject: 36 chars, correct prefix
- Has release notes update
**Code**: ✅
- New APIs properly marked `__rte_experimental`
- Unit tests added in `test_ethdev_api.c`
- Includes `rte_port_id_t` and `rte_queue_id_t` typedefs
---
### Patch 3/6: `flow_parser: add shared parser library`
**Commit Message**: ✅ Good
- Subject: 42 chars, correct prefix
- Comprehensive description of the library design
- Has release notes, documentation, and MAINTAINERS update
**Code Review**:
| Check | Status |
|-------|--------|
| SPDX license | ✅ Present on all new files |
| Header guards | ✅ Standard format (`_RTE_FLOW_PARSER_H_`) |
| `__rte_experimental` markers | ✅ All public APIs marked |
| Naming conventions | ✅ All public symbols use `rte_` prefix |
| Documentation | ✅ `flow_parser_lib.rst` added |
**Minor Observations**:
- Line 16018-16019: Header guards follow DPDK standard style
- Public API is minimal (4 functions) - good design
- Internal/private API properly separated in `rte_flow_parser_private.h`
---
### Patch 4/6: `app/testpmd: use shared flow parser library`
**Commit Message**: ✅ Good
- Subject: 45 chars, proper `app/testpmd:` prefix
**Code**: ✅
- Properly removes old `cmdline_flow.c` and adds new files
- Updates meson.build to depend on `flow_parser` library
---
### Patch 5/6: `examples/flow_parsing: add flow parser demo`
**Commit Message**: ✅ Good
- Subject: 44 chars, proper `examples/flow_parsing:` prefix
- Includes build instructions
**Code**: ✅
- SPDX license correct
- meson.build has `allow_experimental_apis = true`
- MAINTAINERS entry added
---
### Patch 6/6: `test: add flow parser library functional tests`
**Commit Message**: ✅ Good
- Subject: 50 chars, proper `test:` prefix
**Code**: ✅ Excellent test coverage
- Uses `TEST_ASSERT` macros as required by AGENTS.md
- Uses `unit_test_suite_runner` infrastructure
- `REGISTER_FAST_TEST` with proper flags
- Tests both public and internal APIs
- Includes invalid input/syntax tests
---
### V5 → V6 Changes Verification
| Claimed Change | Verified? |
|----------------|-----------|
| "Inconsistent Experimental API Version adjusted" | ✅ All `__rte_experimental` markers appear consistent |
| "Fixes Tag added to MSVC build commit" | ❌ **Not present** - misleading changelog |
| "Non-Standard Header Guards updated" | ✅ Guards now use standard `_RTE_*_H_` format |
| "Implicit Pointer Comparison issues addressed" | ✅ Old code had `!buf`, new code uses explicit `!= NULL` comparisons |
---
### Overall Assessment
**Ready for merge with one minor clarification needed:**
1. **Changelog accuracy** (Info): The v6 cover letter claims "Fixes Tag added to MSVC build commit" but patch 1/6 has no Fixes tag. The author should either add a Fixes tag if one applies, or correct the changelog. Since the change is a preventive improvement and not fixing a specific commit's regression, no Fixes tag is likely needed - just update the changelog.
The patches are well-structured, follow DPDK coding standards, include proper documentation, release notes, and comprehensive tests. The library design with a minimal public API and separated private headers is clean.
More information about the dev
mailing list