[dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API

[Chandran, Sugesh]

Hi Adrien,
Thank you for your comments,
Please see the reply below.

Regards
_Sugesh


> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Thursday, December 8, 2016 3:09 PM
> To: Chandran, Sugesh <sugesh.chandran at intel.com>
> Cc: Kevin Traynor <ktraynor at redhat.com>; dev at dpdk.org; Thomas
> Monjalon <thomas.monjalon at 6wind.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; Olivier Matz <olivier.matz at 6wind.com>;
> sugesh.chandran at intel.comn
> Subject: Re: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
> 
> Hi Sugesh,
> 
> On Tue, Dec 06, 2016 at 06:11:38PM +0000, Chandran, Sugesh wrote:
> [...]
> > > >>> +int
> > > >>> +rte_flow_validate(uint8_t port_id,
> > > >>> +		  const struct rte_flow_attr *attr,
> > > >>> +		  const struct rte_flow_item pattern[],
> > > >>> +		  const struct rte_flow_action actions[],
> > > >>> +		  struct rte_flow_error *error);
> > > >>
> > > >> Why not just use rte_flow_create() and get an error? Is it less
> > > >> disruptive to do a validate and find the rule cannot be created,
> > > >> than using a create directly?
> > > >
> > > > The rationale can be found in the original RFC, which I'll convert
> > > > to actual documentation in v2. In short:
> > > >
> > > > - Calling rte_flow_validate() before rte_flow_create() is useless since
> > > >   rte_flow_create() also performs validation.
> > > >
> > > > - We cannot possibly express a full static set of allowed flow rules, even
> > > >   if we could, it usually depends on the current hardware configuration
> > > >   therefore would not be static.
> > > >
> > > > - rte_flow_validate() is thus provided as a replacement for capability
> > > >   flags. It can be used to determine during initialization if the underlying
> > > >   device can support the typical flow rules an application might want to
> > > >   provide later and do something useful with that information (e.g.
> always
> > > >   use software fallback due to HW limitations).
> > > >
> > > > - rte_flow_validate() being a subset of rte_flow_create(), it is
> essentially
> > > >   free to expose.
> > >
> > > make sense now, thanks.
> > [Sugesh] : We had this discussion earlier at the design stage about
> > the time taken for programming the hardware, and how to make it
> > deterministic. How about having a timeout parameter as well for the
> > rte_flow_* If the hardware flow insert is timed out, error out than
> > waiting indefinitely, so that application have some control over The
> > time to program the flow. It can be another set of APIs something
> > like, rte_flow_create_timeout()
> 
> Yes as discussed the existing API does not provide any timing constraints to
> PMDs, validate() and create() may take forever to complete, although PMDs
> are strongly encouraged to take as little time as possible.
> 
> Like you suggested, this could be done through distinct API calls. The
> validate() function would also have its _timeout() counterpart since the set
> of possible rules could be restricted in that mode.
[Sugesh] Thanks!. Looking forward to see an api set with that implementation as well 
in the future :). I feel it's a must from the user application point of view.
> 
> > Are you going to provide any control over the initialization of NIC
> > to define the capability matrices For eg; To operate in a L3 router mode,
> software wanted to initialize the NIC port only to consider the L2 and L3
> fields.
> > I assume the initialization is done based on the first rules that are
> programmed into the NIC.?
> 
> Precisely, PMDs are supposed to determine the most appropriate device
> mode to use in order to handle the requested rules. They may even switch
> to another mode if necessary assuming this does not break existing
> constraints.
> 
> I think we've discussed an atomic (commit-based) mode of operation
> through separate functions as well, where the application would attempt to
> create a bunch of rules at once, possibly making it easier for PMDs to
> determine the most appropriate mode of operation for the device.
> 
> All of these may be added later according to users feedback once the basic
> API has settled.
[Sugesh] Yes , we discussed about this before. However I feel that, it make sense
to provide some flexibility to the user/application to define a profile/mode of the device.
This way the complexity of determining the mode by itself will be taken away from PMD.
Looking at the P4 enablement patches in OVS, the mode definition APIs can be used in conjunction
P4 behavioral model. 
For eg: A P4 model for a L2 switch operate OVS as a L2 switch. Using the mode definition APIs
Its possible to impose the same behavioral model in the hardware too. 
This way its simple, clean and very predictive though it needs to define an additional profile_define APIs.
I am sorry to provide the comment at this stage,  However looking at the adoption of ebpf, P4 make me
to think this way.
What do you think?
> 
> --
> Adrien Mazarguil
> 6WIND