Re: [RFC PATCH] net: introduce HW Rate Limiting Driver API

On Wed, 2024-05-08 at 23:47 +0200, Andrew Lunn wrote:

quoted

+ * 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.

quoted

+	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.

quoted

+ * 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