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

From: Gregory Etelson <getelson@nvidia.com>
To: "NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Matan Azrad <matan@nvidia.com>,
	Raslan Darawsheh <rasland@nvidia.com>, Ori Kam <orika@nvidia.com>,
	Aman Singh <aman.deep.singh@intel.com>,
	Yuying Zhang <yuying.zhang@intel.com>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	Slava Ovsiienko <viacheslavo@nvidia.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	"ivan.malov@oktetlabs.ru" <ivan.malov@oktetlabs.ru>,
	"jerinj@marvell.com" <jerinj@marvell.com>
Subject: RE: [PATCH v4 2/2] ethdev: add quota flow action and item
Date: Thu, 19 Jan 2023 09:39:54 +0000	[thread overview]
Message-ID: <IA1PR12MB63321A8ECC2DC2EE2180CAEFA5C49@IA1PR12MB6332.namprd12.prod.outlook.com> (raw)
In-Reply-To: <2597127.MzOyzyC30C@thomas>

Hello Thomas,

[]

> > > > +     /**
> > > > +      * 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.
> 
> I am still not sure to understand the comment.
> Is this one better?
> "Apply the quota verdict (PASS or BLOCK) to a flow."
> 
> 

Yes. I'll update the v5.

> > > > +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.
> 
> OK. Is it documented?
> 
> 

I'll add missing documentation.