Re: [dpdk-dev] [PATCH 1/3] ethdev: introduce flow pre-configuration hints

[Ori Kam <orika@nvidia.com>]

Hi Ajit,

> -----Original Message-----
> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Sent: Sunday, October 31, 2021 7:27 PM
> Subject: Re: [PATCH 1/3] ethdev: introduce flow pre-configuration hints
> 
> On Wed, Oct 13, 2021 at 6:15 AM Ori Kam <orika@nvidia.com> wrote:
> >
> > Hi Ajit,
> >
> > > -----Original Message-----
> > > From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > > Sent: Wednesday, October 13, 2021 7:11 AM
> > > Subject: Re: [PATCH 1/3] ethdev: introduce flow pre-configuration hints
> > >
> > > On Tue, Oct 5, 2021 at 9:48 PM Alexander Kozyrev <akozyrev@nvidia.com> wrote:
> > > >
> > > > The flow rules creation/destruction at a large scale incurs a
> > > > performance penalty and may negatively impact the packet processing
> > > > when used as part of the datapath logic. This is mainly because
> > > > software/hardware resources are allocated and prepared during the flow rule creation.
> > > >
> > > > In order to optimize the insertion rate, PMD may use some hints
> > > > provided by the application at the initialization phase. The
> > > > rte_flow_configure() function allows to pre-allocate all the needed resources beforehand.
> > > > These resources can be used at a later stage without costly allocations.
> > > > Every PMD may use only the subset of hints and ignore unused ones.
> > > This could get tricky. An application can avoid attempts to create flows for items/actions if it can get
> a
> > > hint that PMD cannot satisfy some of the hints provided.
> > > Also what if the application tries to configure a higher count than what the PMD/HW can support?
> > > It will be good if the hints can be negotiated.
> > > Something like this?
> > > Application could start with a set of hints.
> > > PMD can check what can be supported and return an updated set.
> > > Application stays within those values till it needs to resize.
> > >
> >
> > I don't like the negotaion approach since soon it will be impossible to maintain.
> > I don't know how many possible option there will and what each PMD will implement.
> > Also negotiation means that the PMD think he knows what is best and this is the opposite of
> > this feature as giving the application as much control as possible.
> >
> > Just like configure ethdev may fail and the application just get the error message.
> > What application should do with the hits is PMD dependent.
> >
> > For example application request 1M counter, this can even be system specific (to much memory)
> > if PMD can't allocate such number of counter it should fail with error message that lets the
> application
> > know if possible the max number of counters.
> How will the application know that the failure was because of counters
> and not because of meters or something else?
> 
He can check the error description, 

Even now when you insert rule you don't know why it failed.
I assume that if only some of the parameters are not supported the
error returned will be EINVAL.

> >
> > Best,
> > Ori
> >
> > > >
> > > > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> > > > Suggested-by: Ori Kam <orika@nvidia.com>
> > > > ---
> > > >  lib/ethdev/rte_flow.h | 70
> > > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 70 insertions(+)
> > > >
> > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> > > > 7b1ed7f110..c69d503b90 100644
> > > > --- a/lib/ethdev/rte_flow.h
> > > > +++ b/lib/ethdev/rte_flow.h
> > > > @@ -4288,6 +4288,76 @@ rte_flow_tunnel_item_release(uint16_t port_id,
> > > >                              struct rte_flow_item *items,
> > > >                              uint32_t num_of_items,
> > > >                              struct rte_flow_error *error);
> > > > +
> > > > +/**
> > > > + * Flow engine configuration.
> > > > + */
> > > > +__extension__
> > > > +struct rte_flow_port_attr {
> > > > +       /**
> > > > +        * Version of the struct layout, should be 0.
> > > > +        */
> > > > +       uint32_t version;
> > > > +       /**
> > > > +        * Memory size allocated for the flow rules management.
> > > > +        * If set to 0, memory is allocated dynamically.
> > > > +        */
> > > > +       uint32_t mem_size;
> > > > +       /**
> > > > +        * Number of counter actions pre-configured.
> > > > +        * If set to 0, PMD will allocate counters dynamically.
> > > > +        * @see RTE_FLOW_ACTION_TYPE_COUNT
> > > > +        */
> > > > +       uint32_t nb_counters;
> > > > +       /**
> > > > +        * Number of aging actions pre-configured.
> > > > +        * If set to 0, PMD will allocate aging dynamically.
> > > > +        * @see RTE_FLOW_ACTION_TYPE_AGE
> > > > +        */
> > > > +       uint32_t nb_aging;
> > > > +       /**
> > > > +        * Number of traffic metering actions pre-configured.
> > > > +        * If set to 0, PMD will allocate meters dynamically.
> > > > +        * @see RTE_FLOW_ACTION_TYPE_METER
> > > > +        */
> > > > +       uint32_t nb_meters;
> > > > +       /**
> > > > +        * Resources reallocation strategy.
> > > > +        * If set to 1, PMD is not allowed to allocate more resources on demand.
> > > > +        * An application can only allocate more resources by calling the
> > > > +        * configure API again with new values (may not be supported by PMD).
> > > > +        */
> > > > +       uint32_t fixed_resource_size:1; };
> > > > +
> > > > +/**
> > > > + * @warning
> > > > + * @b EXPERIMENTAL: this API may change without prior notice.
> > > > + *
> > > > + * Configure flow rules module.
> > > > + * To pre-allocate resources as per the flow port attributes,
> > > > + * this configuration function must be called before any flow rule is created.
> > > > + * No other rte_flow function should be called while this function is invoked.
> > > > + * This function can be called again to change the configuration.
> > > > + * Some PMDs may not support re-configuration at all,
> > > > + * or may only allow increasing the number of resources allocated.
> > > > + *
> > > > + * @param port_id
> > > > + *   Port identifier of Ethernet device.
> > > > + * @param[in] port_attr
> > > > + *   Port configuration attributes.
> > > > + * @param[out] error
> > > > + *   Perform verbose error reporting if not NULL.
> > > > + *   PMDs initialize this structure in case of error only.
> > > > + *
> > > > + * @return
> > > > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > > > + */
> > > > +__rte_experimental
> > > > +int
> > > > +rte_flow_configure(uint16_t port_id,
> > > > +                  const struct rte_flow_port_attr *port_attr,
> > > > +                  struct rte_flow_error *error);
> > > >  #ifdef __cplusplus
> > > >  }
> > > >  #endif
> > > > --
> > > > 2.18.2
> > > >