<div dir="ltr"><div dir="ltr">pá 13. 2. 2026 v 1:43 odesílatel Stephen Hemminger <<a href="mailto:stephen@networkplumber.org">stephen@networkplumber.org</a>> napsal:</div><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 6 Feb 2026 16:40:39 +0100<br>
Lukáš Šišmiš <<a href="mailto:sismis@dyna-nic.com" target="_blank">sismis@dyna-nic.com</a>> wrote:<br>
<br>
> pá 6. 2. 2026 v 15:02 odesílatel Thomas Monjalon <<a href="mailto:thomas@monjalon.net" target="_blank">thomas@monjalon.net</a>><br>
> napsal:<br>
> <br>
> > 04/02/2026 15:53, Stephen Hemminger:  <br>
> > > On Tue, 3 Feb 2026 09:34:26 +0100<br>
> > > Lukáš Šišmiš <<a href="mailto:sismis@dyna-nic.com" target="_blank">sismis@dyna-nic.com</a>> wrote:<br>
> > >  <br>
> > > > ><br>
> > > > > The kernel version of checkpatch complains here. The DPDK shell  <br>
> > script  <br>
> > > > > seems to be set to ignore this but.<br>
> > > > ><br>
> > > > > WARNING: break is not useful after a return<br>
> > > > > #15008: FILE: lib/flow_parser/rte_flow_parser.c:14763:<br>
> > > > > +               return cmd_flow_parsed(out);<br>
> > > > > +               break;<br>
> > > > ><br>
> > > > > Should I create a new patch set or just let it be at this moment?  <br>
> > > > Lukas  <br>
> > ><br>
> > ><br>
> > > I am ok with it as is.  <br>
> ><br>
> > Better to update.<br>
> ><br>
> > There are other warnings:<br>
> ><br>
> > WARNING:STRNCPY: Prefer strscpy, strscpy_pad, or __nonstring over strncpy<br>
> > - see: <a href="https://github.com/KSPP/linux/issues/90" rel="noreferrer" target="_blank">https://github.com/KSPP/linux/issues/90</a><br>
> > #13052 <<a href="https://github.com/KSPP/linux/issues/90#13052" rel="noreferrer" target="_blank">https://github.com/KSPP/linux/issues/90#13052</a>>: FILE:<br>
> > lib/flow_parser/rte_flow_parser.c:12825:<br>
> > +       strncpy(buf, str, len);<br>
> ><br>
> > and a lot of WARNING:LONG_LINE<br>
> ><br>
> > I can have a look after the decision.  <br>
> <br>
> ><br>
> > And on a more general note, I would have expected to ask the opinion<br>
> > of rte_flow maintainers, but they are not Cc'ed in these patches.<br>
> >  <br>
> I communicated primarily with Stephen, and will CC Ori too. Anyone else?<br>
> <br>
> <br>
> > I'm a bit skeptical about adding this outside of ethdev library<br>
> > which defines the flow API.<br>
> >  <br>
> <br>
> CCing Ori to make a decision. I don't mind putting it directly into ethdev<br>
> as well, I just thought the parser could be its own separate lib as it is<br>
> just consuming strings and producing rte_flow structures. I can see the<br>
> heavy ties to the flow library, though. Thomas, Stephen, what are your<br>
> opinions?<br>
<br>
I am beginning to agree with Thomas, this belongs in the ethdev directory<br>
next to rte_flow.<br>
<br>
Also, not sure what the purpose of parser_ops is. It says to pass NULL, but<br>
test-pmd is doing something else. It would be better to have an initializer<br>
(i.e RTE_INIT) instead? Maybe<br> </blockquote><div> </div><div>Ok, I can move it there then. Should I proceed with Patch v11 - that would</div><div>include moving the ethdev directory and addressing the kernel's checkpatch</div><div>issues?</div><div><br></div><div>The initializer doesn't seem like a bad idea. I didn't know about that one.</div><div>The parser_ops are not needed for purely parsing, but to remain compatible</div><div>with the testpmd's code, which is using, e.g., the hints on the commandline</div><div>I added parser_ops.</div><div>The simple public library API is for converting strings to rte_flow structures,</div><div>the parser_ops is for hooking up the testpmd. </div></div></div>