public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: "Lukáš Šišmiš" <sismis@dyna-nic.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	dev@dpdk.org, Ori Kam <orika@nvidia.com>
Subject: Re: [PATCH v10 3/6] flow_parser: add shared parser library
Date: Thu, 12 Feb 2026 16:43:41 -0800	[thread overview]
Message-ID: <20260212164341.074639cd@phoenix.local> (raw)
In-Reply-To: <CAPQRu6FR+RojmsKhcsegastxJHLBAbmtx5Qurg79Wn_w94yYpg@mail.gmail.com>

On Fri, 6 Feb 2026 16:40:39 +0100
Lukáš Šišmiš <sismis@dyna-nic.com> wrote:

> pá 6. 2. 2026 v 15:02 odesílatel Thomas Monjalon <thomas@monjalon.net>
> napsal:
> 
> > 04/02/2026 15:53, Stephen Hemminger:  
> > > On Tue, 3 Feb 2026 09:34:26 +0100
> > > Lukáš Šišmiš <sismis@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




  reply	other threads:[~2026-02-13  0:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-02 11:36 [PATCH v10 0/6] flow_parser: add shared parser library Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 1/6] cmdline: include stddef.h for MSVC compatibility Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 2/6] ethdev: add RSS type helper APIs Lukas Sismis
2026-02-17 14:40   ` Stephen Hemminger
2026-02-02 11:36 ` [PATCH v10 4/6] app/testpmd: use shared flow parser library Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 5/6] examples/flow_parsing: add flow parser demo Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 6/6] test: add flow parser library functional tests Lukas Sismis
2026-02-02 18:37 ` [PATCH v10 0/6] flow_parser: add shared parser library Stephen Hemminger
     [not found] ` <20260202113659.24052-4-sismis@dyna-nic.com>
2026-02-02 20:03   ` [PATCH v10 3/6] " Stephen Hemminger
2026-02-03  8:34     ` Lukáš Šišmiš
2026-02-04 14:53       ` Stephen Hemminger
2026-02-06 14:01         ` Thomas Monjalon
2026-02-06 15:40           ` Lukáš Šišmiš
2026-02-13  0:43             ` Stephen Hemminger [this message]
2026-02-13  7:46               ` Lukáš Šišmiš
2026-02-13 19:16                 ` Stephen Hemminger
2026-02-14  4:35                 ` Stephen Hemminger
2026-02-10 14:44   ` Stephen Hemminger
2026-02-10 14:45 ` [PATCH v10 0/6] " Stephen Hemminger
2026-02-12  8:56   ` Lukáš Šišmiš

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260212164341.074639cd@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=orika@nvidia.com \
    --cc=sismis@dyna-nic.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox