Date: Thu, 09 May 2024 16:19:52 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>, Jiri Pirko
 <jiri@...nulli.us>, Madhu Chittim <madhu.chittim@...el.com>, Sridhar
 Samudrala <sridhar.samudrala@...el.com>, Simon Horman <horms@...nel.org>,
 John Fastabend <john.fastabend@...il.com>, Sunil Kovvuri Goutham
 <sgoutham@...vell.com>,  Jamal Hadi Salim <jhs@...atatu.com>
Subject: Re: [RFC PATCH] net: introduce HW Rate Limiting Driver API

On Wed, 2024-05-08 at 23:47 +0200, Andrew Lunn wrote:
> > + * struct net_shaper_info - represents a shaping node on the NIC H/W
> > + * @metric: Specify if the bw limits refers to PPS or BPS
> > + * @bw_min: Minimum guaranteed rate for this shaper
> > + * @bw_max: Maximum peak bw allowed for this shaper
> > + * @burst: Maximum burst for the peek rate of this shaper
> > + * @priority: Scheduling priority for this shaper
> > + * @weight: Scheduling weight for this shaper
> > + */
> > +struct net_shaper_info {
> > +	enum net_shaper_metric metric;
> > +	u64 bw_min;	/* minimum guaranteed bandwidth, according to metric */
> > +	u64 bw_max;	/* maximum allowed bandwidth */
> > +	u32 burst;	/* maximum burst in bytes for bw_max */
> > +	u32 priority;	/* scheduling strict priority */
> 
> Above it say priority. Here is strict priority? Is there a difference?

the 'priority' field is the strict priority for scheduling. We will
clarify the first comment.

> 
> > +	u32 weight;	/* scheduling WRR weight*/
> > +};
> 
> Are there any special semantics for weight? Looking at the hardware i
> have, which has 8 queues for a switch port, i can either set it to
> strict priority, meaning queue 7 needs to be empty before it look at
> queue 6, and it will only look at queue 5 when 6 is empty etc. Or i
> can set weights per queue. How would i expect strict priority?

The expected behaviour is that schedulers at the same level with the
same priority will be served according to their weight.

I'm unsure if I read your question correctly. My understanding is that
the your H/W don't allow strict priority and WRR simultaneously. In
such case, the set/create operations should reject calls setting both
non zero weight and priority values.

> Shaping itself is not performed on the queues, but the port. So it
> seems like i should create 8 net_shaper_info and set the weight in
> each, and everything else to 0? And then create a queue group shaper,
> put the 8 queue shapers into it, and then configure bw_max for the
> group? Everything else is 0, because that is all i can configure.
> 
> Does this sound correct?

It sounds correct. You can avoid creating the queue group and set the
rate at the NETDEV scope, or possibly not even set/create such shaper:
the transmission is expected to be limited by the line rate.

> > + * NET_SHAPER_SCOPE_NETDEV, NET_SHAPER_SCOPE_QUEUE_GROUP and
> > + * NET_SHAPER_SCOPE_QUEUE are available on both PFs and VFs devices.
> 
> Are they also available on plain boring devices which don't have PFs
> or VFs?

Yes, a driver is free to implement/support an arbitrary subset of the
available feature. Operations attempting to set an unsupported state
should fail with a relevant extended ack.

> Would i be correct in assuming my driver should just create these
> shapers. There will be some netlink calls to allow user space to
> enumerate them, and display the relationships between them?

Yes, the idea/plan/roadmap is to implement yml-based netlink API to
fully expose the feature to the user-space.

>  And
> netlink calls to set values in a shaper? Will there be a way to say
> which fields are actually settable, since i doubt most hardware will
> have everything? 

This was not planned yet, even because some (most?) H/W can have non-
trivial constraints to expose. e.g. WRR is available at the queue scope
only, and only if there is at most one SP shaper somewhere else. 

> In my case, only one field appears to be relevant in
> each shaper, and maybe we want to give a hint about that to userspace?

Any suggestion on how to expose that in reasonable way more then
welcome!

Thanks,

Paolo

Powered by blists - more mailing lists