RE: [PATCH v4 2/2] ethdev: add quota flow action and item

[Gregory Etelson <getelson@nvidia.com>]

Hello Thomas,

> Is this patch related to query/update of indirect flow action?
> It looks like it should be in a separate series.
> 

Quota flow action updates relay on rte_flow_action_handle_query_update().
Also, the function is used in the testmd patch section.

> 18/01/2023 11:31, Gregory Etelson:
> > Quota action limits traffic according to pre-defined configuration.
> > Quota reflects overall traffic usage regardless bandwidth.
> > Quota flow action initialized with signed tokens number value.
> > Quota flow action updates tokens number according to
> > these rules:
> > 1. if quota was configured to count packet length, for each packet
> >    of size S, tokens number reduced by S.
> > 2. If quota was configured to count packets, each packet decrements
> >    tokens number.
> > quota action sets packet metadata according to a number of remaining
> > tokens number:
> >   PASS - remaining tokens number is non-negative.
> >   BLOCK - remaining tokens number is negative.
> >
> > Quota flow item matches on that data
> >
> > Application updates tokens number in quota flow action
> > with SET or ADD calls:
> >  SET(QUOTA, val) - arm quota with new tokens number set to val
> >  ADD(QUOTA, val) - increase existing quota tokens number by val
> >
> > Both SET and ADD return to application number of tokens stored in port
> > before update.
> >
> > Application must create a rule with quota action to mark flow and
> > match on the mark with quota item in following flow rule.
> >
> > Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> > ---
> > v2: Remove RTE_FLOW_QU_DEFAULT query-update mode.
> > v3: Update release notes.
> > ---
> >  app/test-pmd/cmdline_flow.c            | 394 ++++++++++++++++++++++++-
> >  app/test-pmd/config.c                  |  82 ++++-
> >  app/test-pmd/testpmd.h                 |  11 +
> >  doc/guides/nics/features/default.ini   |   2 +
> >  doc/guides/nics/features/mlx5.ini      |   2 +
> >  doc/guides/nics/mlx5.rst               |  12 +
> 
> mlx5 feature should be documented and marked in the patch
> having the implementation code, not in ethdev API.
>

Will fix in v5.
 
> >  doc/guides/prog_guide/rte_flow.rst     |  41 +++
> >  doc/guides/rel_notes/release_23_03.rst |  27 ++
> >  lib/ethdev/rte_flow.c                  |   2 +
> >  lib/ethdev/rte_flow.h                  | 123 ++++++++
> >  10 files changed, 686 insertions(+), 10 deletions(-)
> [...]
> > +     /**
> > +      * Apply quota verdict - PASS or BLOCK to a flow.
> 
> Is "to" really wanted here?
> What about "pass or block a flow"?
> 

The flow action provides information only - it does not affect flow in any way.
Application needs to match on quota flow item after quota action to discover quota state.
It's also up to application how to react on quota state. 

> > +      *
> > +      * @see struct rte_flow_action_quota
> > +      * @see struct rte_flow_query_quota
> > +      * @see struct rte_flow_update_quota
> > +      */
> > +      RTE_FLOW_ACTION_TYPE_QUOTA,
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * QUOTA operational mode.
> > + *
> > + * @see struct rte_flow_action_quota
> > + */
> > +enum rte_flow_quota_mode {
> > +     RTE_FLOW_QUOTA_MODE_PACKET = 1, /** Count packets */
> > +     RTE_FLOW_QUOTA_MODE_L2 = 2, /** Count packet bytes starting
> from L2 */
> > +     RTE_FLOW_QUOTA_MODE_L3 = 3, /** Count packet bytes starting
> from L3 */
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Create QUOTA action.
> > + *
> > + * @see RTE_FLOW_ACTION_TYPE_QUOTA
> > + */
> > +struct rte_flow_action_quota {
> > +     enum rte_flow_quota_mode mode; /** quota operational mode */
> > +     int64_t quota;                 /** quota value */
> 
> What means a negative quota?
> 

Negative quota is kind of a loan extended by a hardware if it provides such option.
If quota value is -T, application must provide at least (T+1) tokens to return quota to PASS state. 

> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Query indirect QUOTA action.
> > + *
> > + * @see RTE_FLOW_ACTION_TYPE_QUOTA
> > + *
> > + */
> > +struct rte_flow_query_quota {
> > +     int64_t quota; /** quota value */
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Indirect QUOTA update operations.
> > + *
> > + * @see struct rte_flow_update_quota
> > + */
> > +enum rte_flow_update_quota_op {
> > +     RTE_FLOW_UPDATE_QUOTA_SET, /** set new quota value */
> > +     RTE_FLOW_UPDATE_QUOTA_ADD, /** increase existing quota with
> new value */
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * @see RTE_FLOW_ACTION_TYPE_QUOTA
> > + *
> > + * Update indirect QUOTA action.
> > + */
> > +struct rte_flow_update_quota {
> > +     enum rte_flow_update_quota_op op; /** update operation */
> > +     int64_t quota;                    /** quota value */
> >  };
> >
> >  /**
> > @@ -4854,6 +4967,11 @@ struct rte_flow_port_info {
> >        * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
> >        */
> >       uint32_t max_nb_conn_tracks;
> > +     /**
> > +      * Maximum number of quota actions.
> > +      * @see RTE_FLOW_ACTION_TYPE_QUOTA
> > +      */
> > +     uint32_t max_nb_quotas;
> >       /**
> >        * Port supported flags (RTE_FLOW_PORT_FLAG_*).
> >        */
> > @@ -4932,6 +5050,11 @@ struct rte_flow_port_attr {
> >        * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
> >        */
> >       uint32_t nb_conn_tracks;
> > +     /**
> > +      * Maximum number of quota actions.
> > +      * @see RTE_FLOW_ACTION_TYPE_QUOTA
> > +      */
> > +     uint32_t nb_quotas;
> >       /**
> >        * Port flags (RTE_FLOW_PORT_FLAG_*).
> >        */
> 
>