From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
"Ori Kam" <orika@nvidia.com>, <dev@dpdk.org>,
"NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
<david.marchand@redhat.com>, <jerinj@marvell.com>,
<jerinjacobk@gmail.com>, <ferruh.yigit@amd.com>,
"Zhang, Helin" <helin.zhang@intel.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"Mcnamara, John" <john.mcnamara@intel.com>
Cc: <techboard@dpdk.org>, "Zhang, Qi Z" <qi.z.zhang@intel.com>
Subject: RE: [PATCH] ethdev: add flow API support for P4-programmable devices
Date: Mon, 25 Sep 2023 14:35:18 +0200 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87C07@smartserver.smartshare.dk> (raw)
In-Reply-To: <DS0PR11MB7442DFFE8451EBA656045430EBFCA@DS0PR11MB7442.namprd11.prod.outlook.com>
> From: Dumitrescu, Cristian [mailto:cristian.dumitrescu@intel.com]
> Sent: Monday, 25 September 2023 13.52
>
> Hi Ori,
>
> Will implement your comments and send the V2 straight away. Answers inline.
>
> > From: Ori Kam <orika@nvidia.com>
> > Sent: Thursday, September 21, 2023 5:39 PM
> >
> > Hi Cristian,
> >
> > > From: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > Sent: Friday, September 15, 2023 10:00 PM
> > >
> > > This patch introduces the new "program" action type to enable flow API
> > > support for P4-programmable devices.
> > >
> > > In the case of P4-programmable devices, the device is initially blank.
> > > The flow items and actions are defined by the user (outside of any
> > > vendor control) through the P4 program, which is typically compiled
> > > into firmware that is loaded on the device at init time. These flow
> > > items and actions are then used during the run-time phase to add flows
> > > on the device.
> > >
> > > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > > ---
[...]
> > > +struct rte_flow_action_prog_argument {
> > > + /** Argument name. */
> > > + const char *arg_name;
> >
> > Maybe uint32 id?
>
> The reason for having the argument name also specified here is purely related
> to
> allowing the control plane to specify the action arguments in a potentially
> different
> order than in the P4 program, or to skip some arguments, implying a default
> value
> to be used for these arguments.
>
> Since the arguments have a string identifier in the P4 program, we kept a
> string
> name here for this reason. Having a numerical ID would imply the position of
> the
> argument within the argument list in the P4 program, which is IMO less useful.
Having an index would imply the position within the argument list.
However, an ID is more like an enum, and does not imply any position.
The application would probably need to treat the possible string values like enum values anyway. In other words: It's not really a string, it's an enum represented by a string. So we might as well represent it by an integer type (uint32_t).
>
> I agree we can remove this name, but then we lose the ability explained above.
> Therefore, I am going to keep this for now, in case you agree with my
> explanation,
> But we can remove it if you're strongly against it.
Intuitively, a numerical ID seems easier and faster to handle than a string.
I don’t feel strongly about this. And I'm not really involved with RTE_FLOW or P4, so not really qualified. ;-)
next prev parent reply other threads:[~2023-09-25 12:35 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-15 18:59 Cristian Dumitrescu
2023-09-21 16:38 ` Ori Kam
2023-09-25 11:52 ` Dumitrescu, Cristian
2023-09-25 12:35 ` Morten Brørup [this message]
2023-09-25 12:33 ` [PATCH V2] " Cristian Dumitrescu
2023-09-26 9:25 ` Ori Kam
2023-09-28 9:19 ` Ferruh Yigit
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=98CBD80474FA8B44BF855DF32C47DC35D87C07@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=cristian.dumitrescu@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=helin.zhang@intel.com \
--cc=jerinj@marvell.com \
--cc=jerinjacobk@gmail.com \
--cc=john.mcnamara@intel.com \
--cc=orika@nvidia.com \
--cc=qi.z.zhang@intel.com \
--cc=techboard@dpdk.org \
--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;
as well as URLs for NNTP newsgroup(s).