On Mon, Jan 24, 2022 at 6:37 AM Jerin Jacob <jerinjac...@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 9:01 PM Alexander Kozyrev <akozy...@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 or
> > fail in case the requested configuration is not supported.
> >
> > Signed-off-by: Alexander Kozyrev <akozy...@nvidia.com>
> > ---
>
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Flow engine port configuration attributes.
> > + */
> > +__extension__
>
> Is this __extension__ required ?
>
>
> > +struct rte_flow_port_attr {
> > + /**
> > + * Version of the struct layout, should be 0.
> > + */
> > + uint32_t version;
>
> Why version number? Across DPDK, we are using dynamic function
> versioning, I think, that would
> be sufficient for ABI versioning
>
> > + /**
> > + * 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;
> > +};
> > +
> > +/**
> > + * @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.
> > + * Must be called only after Ethernet device is configured, but may be
> > called
> > + * before or after the device is started as long as there are no flow
> > rules.
> > + * 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.
>
> Following comment from Ivan looks good to me
>
> * Pre-configure the port's flow API engine.
> *
> * This API can only be invoked before the application
> * starts using the rest of the flow library functions.
> *
> * The API can be invoked multiple times to change the
> * settings. The port, however, may reject the changes.
>
> > + *
> > + * @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,
>
> Should we couple, setting resource limit hint to configure function as
> if we add future items in
> configuration, we may pain to manage all state. Instead how about,
> rte_flow_resource_reserve_hint_set()?
+1
>
>
> > + const struct rte_flow_port_attr *port_attr,
> > + struct rte_flow_error *error);
>
> I think, we should have _get function to get those limit numbers otherwise,
> we can not write portable applications as the return value is kind of
> boolean now if
> don't define exact values for rte_errno for reasons.
+1
