DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ori Kam <orika@nvidia.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
	"Morten Brørup" <mb@smartsharesystems.com>,
	"Jerin Jacob" <jerinjacobk@gmail.com>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"ferruh.yigit@amd.com" <ferruh.yigit@amd.com>,
	"techboard@dpdk.org" <techboard@dpdk.org>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	"Zhang, Helin" <helin.zhang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [PATCH] ethdev: introduce generic flow item and action
Date: Thu, 3 Aug 2023 08:39:46 +0000	[thread overview]
Message-ID: <MW2PR12MB46668013CC0C585BDCC92C66D608A@MW2PR12MB4666.namprd12.prod.outlook.com> (raw)
In-Reply-To: <PH7PR11MB74506D4D1DCB0263C5E27401EB0BA@PH7PR11MB7450.namprd11.prod.outlook.com>

Hi Cristian,

> -----Original Message-----
> From: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Sent: Wednesday, August 2, 2023 7:57 PM
> 
> 
> 
> > -----Original Message-----
> > From: Ori Kam <orika@nvidia.com>
> > Sent: Wednesday, August 2, 2023 4:47 PM
> > To: Morten Brørup <mb@smartsharesystems.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>; Jerin Jacob <jerinjacobk@gmail.com>
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; NBU-Contact-Thomas Monjalon
> > (EXTERNAL) <thomas@monjalon.net>; david.marchand@redhat.com;
> > Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> > ferruh.yigit@amd.com; techboard@dpdk.org; Mcnamara, John
> > <john.mcnamara@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> > dev@dpdk.org
> > Subject: RE: [PATCH] ethdev: introduce generic flow item and action
> >
> > Hi Qi
> 
> Hi Ori,
> 
> Thanks for your input!
> 
> >
> > > -----Original Message-----
> > > From: Morten Brørup <mb@smartsharesystems.com>
> > > Sent: Wednesday, August 2, 2023 6:25 PM
> > >
> > > > From: Dumitrescu, Cristian [mailto:cristian.dumitrescu@intel.com]
> > > > Sent: Wednesday, 2 August 2023 16.06
> > > >
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Wednesday, August 2, 2023 12:22 PM
> > > > >
> > > > > On Wed, Aug 2, 2023 at 4:31 PM Morten Brørup
> > > > > <mb@smartsharesystems.com> wrote:
> > > > > >
> > > > > > > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > > > > > > Sent: Wednesday, 2 August 2023 12.25
> > > > > > >
> > > > > > > > From: Qi Zhang [mailto:qi.z.zhang@intel.com]
> > > > > > > > Sent: Wednesday, 2 August 2023 19.35
> > > > > > > >
> > > > > > > > From: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > > > > > >
> > > > > > > > For network devices that are programmable through languages
> > such as
> > > > > > > > the P4 language, there are no pre-defined flow items and actions.
> > > > > > > >
> > > > > > > > The format of the protocol header and metadata fields that are
> > used to
> > > > > > > > specify the flow items that make up the flow pattern, as well as the
> > > > > > > > flow actions, are all defined by the program, with an infinity of
> > > > > > > > possible combinations, as opposed to being selected from a finite
> > > > > > > > pre-defined list.
> > > > > > > >
> > > > > > > > It is virtually impossible to pre-define all the flow items and the
> > > > > > > > flow actions that programs might ever use, as these are only
> > limited
> > > > > > > > by the set of HW resources and the program developer's
> > imagination.
> > > > > > > >
> > > > > > > > To support the programmable network devices, we are introducing:
> > > > > > > >
> > > > > > > > * A generic flow item: The flow item is expressed as an array of
> > bytes
> > > > > > > > of a given length, whose meaning is defined by the program loaded
> > by
> > > > > > > > the network device.
> > > > > > >
> > > > > > > The flow item is not "generic", it is "opaque": Only the application
> > > > knows
> > > > > > > what this flow item does.
> > > > > > >
> > > > > > > I hate the concept for two reasons:
> > > > > > > 1. The inability for applications to detect which flow items the
> > > > underlying
> > > > > > > hardware supports.
> > > > > > > 2. The risk that vendors will use this instead of introducing new flow
> > > > item
> > > > > > > types, available for anyone to implement.
> > > > > >
> > > > > > After further consideration, there might be a middle ground.
> > > > > >
> > > > > > Consider Vendor-Specific attributes for DHCP and RADIUS, or SNMP
> > > MIBs...
> > > > > >
> > > > > > Any vendor is free to add his own, proprietary special-purpose
> > attributes,
> > > > > without going through the standardization process. (This is the key
> > > > challenge
> > > > > this patch seems to be aiming at.)
> > > > > >
> > > > > > The vendor might publish these attributes, and other vendors may
> > > > > implement them too.
> > > > > >
> > > > > > And in order to prevent collisions, the Vendor-Specific attributes
> > contain
> > > > a
> > > > > globally unique vendor ID, such as the Private Enterprise Number [1]
> > > > > managed by IANA.
> > > > > >
> > > > > > If similar principles can be worked into the patch, I can support it.
> > > > >
> > > > > +1
> > > > >
> > +1 I understand that this is supposed to be generic, but how can it?
> 
> What exactly it not generic?
> 
> > how do you know if PMD supports this?
> 
> What is the current RTE_FLOW mechanism to find out whether a device/PMD
> supports a certain flow? The only mechanism available is to try to create the
> flow (rte_flow_create) or mimic the creation of the flow (rte_flow_validate)
> and see if you get success or error, right? Same mechanism to be used here.
> Of course, we would be happy to support the addition of a more complex
> query mechanism in RTE_FLOW.
> 

My bad explanation, you are right the way to know if some PMD supports a feature
is trial and error, what I ment is how PMD knows how to phrase the data, it should be 
in a known format.

> > what if each PMD needs different configurations?
> 
> Not sure what you're referring to exactly. The format of the flow items and
> flow actions is defined by the P4 program; so all the HW devices that are
> able to successfully load a given P4 program, potentially from different
> vendors, will have the same understanding of each flow item and action.
> 

So you don't suggest generic, you suggest p4_action/item which will get
a blob based on standard.

> >
> > In addition how can you handle number of those action and items?
> > For example if I have match on protocol X and Y and do actions Z and W
> > each one of those can be generic item.
> > if you have a way to define a standard why to read such actions then we have
> > something to talk about.
> 
> Yes, we do. The format of all flow items and flow actions available on the HW
> device is fully defined by the P4 program, therefore all the HW devices,
> potentially from different vendors, that are able to successfully load a given P4
> program, will have the same understanding of them.
> 

Again this was not clear from the RFC, see my above comment
> >
> >
> > > >
> > > > Morten, Jerin,
> > > >
> > > > I think there is a fundamental misunderstanding here: we are not trying to
> > > > provide support for some non-standard vendor-specific features here.
> > What
> > > > we are trying to do is add generic multi-vendor support in RTE_FLOW for
> > > > P4 programmable network devices, which requires supporting flow items
> > > > and actions that are defined directly by the user through their P4
> > programs
> > > > as opposed to being selected from a pre-defined list.
> > > >
> > > > There are an infinity of flow items and actions that the users can define
> > > > through
> > > > their P4 programs, and they cannot be supported with a finite list of
> > > RTE_FLOW
> > > > items and actions:
> > > >
> > > > 1/ Some flow items map directly to the IETF defined protocols, while some
> > > > others do not, and only the user writing the program knows the exact
> > answer;
> > > >
> > > > 2/ Some flow items are simply application-specific (not vendor specific)
> > > > meta-data that (I hope we all accept) is outside of the standardization
> > > > process.
> > >
> > > Such items can use a special "reserved" vendor-id.
> > >
> >
> > Can you show me what items/actions are missing in rte_flow?
> 
> The number of flow items (protocol headers, but also metadata) and flow
> actions
> that users can define in their P4 programs is infinite, so unfortunately Ori, as
> much
> as I want to grant you this wish, I am not going to be able to 😊. Also, it is
> important to note that the users write their P4 program (defining their data
> path
> pipeline) without any involvement from their device vendor, so the vendor is
> simply
> not aware of meaning of the user's flow items and actions either.
> 

Maybe we should add an API to register actions/items, but at the end we don't want to write code
Twice, meaning if action is already supported by the current API there is no reason to use different
API.
maybe we can add a small translation function the a PMD can convert actions and output the right
rte_flow actions/items, and if it can't, it can create a user action 

> >
> > > >
> > > > 3/ Some flow actions map directly to the existing RTE_FLOW actions
> > > (especially
> > > > the more straightforward actions such as: packet drop, packet redirection
> > to
> > > > an
> > > > output queue, some specific packet modifications, etc), while the vast
> > > > majority
> > > > of possible actions do not.
> > > >
> > > > Are you saying that the P4 programmable network devices should NOT be
> > > > supported by DPDK and RTE_FLOW?
> > >
> > > No, I get the need for this. And I understand that since P4 is compiled to
> > > hardware-specific binary blobs, there is a need to put such blobs into DPDK
> > as
> > > flow items and actions, instead of the "uncompiled" P4 program.
> > >
> > > I am suggesting that instead of adding a completely opaque data type:
> > >
> > > Struct item {
> > > Int len;      // Length of value in bytes.
> > > Char value[]; // Application specific meaning.
> > > };
> > >
> >
> > But since you didn't define a known protocol for PMD to read the data how
> > 2 pmds can use the same action?
> 
> The format of all flow items and flow actions available on the HW device is fully
> defined by the P4 program, therefore all the HW devices, even if from different
> vendors, will have the same understanding of them.
> 
Like I said above it is not generic it is p4 format.

Best,
Ori

> >
> > > ...add a semi-opaque data type:
> > >
> > > Struct tlv {
> > > Int type;     // Vendor specific type.
> > > Int len;      // Length of value in bytes.
> > > Char value[]; // (Vendor, Type) specific meaning.
> > > };
> > >
> > > Struct item {
> > > Int vendor;          // Vendor ID.
> > > Int len;             // Length of values in bytes.
> > > Struct tlv values[]; // Array of TLVs.
> > > };
> > >
> > > Like RADIUS Vendor-Specific attributes:
> > > https://datatracker.ietf.org/doc/html/rfc2138#section-5.26
> > >
> > > Then some (Vendor, Type) fields can be documented (and thus generally
> > > understood by DPDK), and some undocumented.
> > >
> > > E.g. like Microsoft documented some of theirs in RFC 2548:
> > > https://datatracker.ietf.org/doc/html/rfc2548
> > >
> > >
> > > Another benefit is that these new "VENDOR-SPECIFIC" flow types can be
> > reused
> > > for other purposes than compiled P4 programs.
> > >
> > > >
> > > > >
> > > > > >
> > > > > > Preferably, there should also be a means for applications to query if
> > > > specific
> > > > > Vendor-Specific flow items and actions are supported or not.
> > > > > >
> > > > > >
> > > > > > [1]: https://www.iana.org/assignments/enterprise-numbers/
> > > > > >
> > > >
> > > > Regards,
> > > > Cristian
> 
> Regards,
> Cristian

  reply	other threads:[~2023-08-03  8:39 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-02 17:34 Qi Zhang
2023-08-02  9:37 ` Zhang, Qi Z
2023-08-02 10:25 ` Morten Brørup
2023-08-02 11:01   ` Morten Brørup
2023-08-02 11:21     ` Jerin Jacob
2023-08-02 14:06       ` Dumitrescu, Cristian
2023-08-02 15:24         ` Morten Brørup
2023-08-02 15:47           ` Ori Kam
2023-08-02 16:06             ` Ori Kam
2023-08-02 17:22               ` Dumitrescu, Cristian
2023-08-02 17:56                 ` Morten Brørup
2023-08-03  1:05                   ` Zhang, Qi Z
2023-08-03 13:57                     ` Morten Brørup
2023-08-16 13:23                       ` Dumitrescu, Cristian
2023-08-16 14:20                         ` Morten Brørup
2023-08-16 17:08                           ` Ferruh Yigit
2023-08-16 17:18                             ` Dumitrescu, Cristian
2023-08-16 16:19                         ` DPDK community: RTE_FLOW support for P4-programmable devices Dumitrescu, Cristian
2023-08-27  7:48                           ` Ori Kam
2023-08-28 16:12                             ` Dumitrescu, Cristian
2023-08-29  7:38                               ` Jerin Jacob
2023-08-29 10:18                                 ` Ferruh Yigit
2023-08-31 10:32                                   ` Ori Kam
2023-08-31 13:42                                     ` Stephen Hemminger
2023-09-01  2:07                                     ` Jerin Jacob
2023-09-01  6:57                                       ` Ori Kam
2023-09-01  9:52                                         ` Jerin Jacob
2023-09-04  7:45                                           ` Ferruh Yigit
2023-09-06  8:30                                             ` Ori Kam
2023-09-11 20:20                                         ` Dumitrescu, Cristian
2023-08-02 16:56             ` [PATCH] ethdev: introduce generic flow item and action Dumitrescu, Cristian
2023-08-03  8:39               ` Ori Kam [this message]
2023-08-16 13:12                 ` Dumitrescu, Cristian
2023-08-02 16:14           ` Dumitrescu, Cristian
2023-08-02 14:06   ` Dumitrescu, Cristian
2023-08-02 14:54     ` Morten Brørup

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=MW2PR12MB46668013CC0C585BDCC92C66D608A@MW2PR12MB4666.namprd12.prod.outlook.com \
    --to=orika@nvidia.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=mb@smartsharesystems.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).