[PATCH v10 3/6] flow_parser: add shared parser library

[Lukáš Šišmiš]

pá 13. 2. 2026 v 1:43 odesílatel Stephen Hemminger <
stephen at networkplumber.org> napsal:

> On Fri, 6 Feb 2026 16:40:39 +0100
> Lukáš Šišmiš <sismis at dyna-nic.com> wrote:
>
> > pá 6. 2. 2026 v 15:02 odesílatel Thomas Monjalon <thomas at monjalon.net>
> > napsal:
> >
> > > 04/02/2026 15:53, Stephen Hemminger:
> > > > On Tue, 3 Feb 2026 09:34:26 +0100
> > > > Lukáš Šišmiš <sismis at dyna-nic.com> wrote:
> > > >
> > > > > >
> > > > > > The kernel version of checkpatch complains here. The DPDK shell
> > > script
> > > > > > seems to be set to ignore this but.
> > > > > >
> > > > > > WARNING: break is not useful after a return
> > > > > > #15008: FILE: lib/flow_parser/rte_flow_parser.c:14763:
> > > > > > +               return cmd_flow_parsed(out);
> > > > > > +               break;
> > > > > >
> > > > > > Should I create a new patch set or just let it be at this
> moment?
> > > > > Lukas
> > > >
> > > >
> > > > I am ok with it as is.
> > >
> > > Better to update.
> > >
> > > There are other warnings:
> > >
> > > WARNING:STRNCPY: Prefer strscpy, strscpy_pad, or __nonstring over
> strncpy
> > > - see: https://github.com/KSPP/linux/issues/90
> > > #13052 <https://github.com/KSPP/linux/issues/90#13052>: FILE:
> > > lib/flow_parser/rte_flow_parser.c:12825:
> > > +       strncpy(buf, str, len);
> > >
> > > and a lot of WARNING:LONG_LINE
> > >
> > > I can have a look after the decision.
> >
> > >
> > > And on a more general note, I would have expected to ask the opinion
> > > of rte_flow maintainers, but they are not Cc'ed in these patches.
> > >
> > I communicated primarily with Stephen, and will CC Ori too. Anyone else?
> >
> >
> > > I'm a bit skeptical about adding this outside of ethdev library
> > > which defines the flow API.
> > >
> >
> > CCing Ori to make a decision. I don't mind putting it directly into
> ethdev
> > as well, I just thought the parser could be its own separate lib as it is
> > just consuming strings and producing rte_flow structures. I can see the
> > heavy ties to the flow library, though. Thomas, Stephen, what are your
> > opinions?
>
> I am beginning to agree with Thomas, this belongs in the ethdev directory
> next to rte_flow.
>
> Also, not sure what the purpose of parser_ops is. It says to pass NULL, but
> test-pmd is doing something else. It would be better to have an initializer
> (i.e RTE_INIT) instead? Maybe
>


Ok, I can move it there then. Should I proceed with Patch v11 - that would
include moving the ethdev directory and addressing the kernel's checkpatch
issues?

The initializer doesn't seem like a bad idea. I didn't know about that one.
The parser_ops are not needed for purely parsing, but to remain compatible
with the testpmd's code, which is using, e.g., the hints on the commandline
I added parser_ops.
The simple public library API is for converting strings to rte_flow
structures,
the parser_ops is for hooking up the testpmd.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20260213/041fa7a8/attachment-0001.htm>