On Wed, 9 Nov 2022 19:54:52 +0100 Wilczynski, Michal wrote:

On 11/8/2022 11:39 PM, Jakub Kicinski wrote:

quoted

On Mon,  7 Nov 2022 19:13:26 +0100 Michal Wilczynski wrote:  

quoted

Add documentation to a newly added devlink-rate feature. Provide some
examples on how to use the features, which netlink attributes are
supported and descriptions of the attributes.
+Devlink Rate
+==========
+
+The ``ice`` driver implements devlink-rate API. It allows for offload of
+the Hierarchical QoS to the hardware. It enables user to group Virtual
+Functions in a tree structure and assign supported parameters: tx_share,
+tx_max, tx_priority and tx_weight to each node in a tree. So effectively
+user gains an ability to control how much bandwidth is allocated for each
+VF group. This is later enforced by the HW.
+
+It is assumed that this feature is mutually exclusive with DCB and ADQ, or
+any driver feature that would trigger changes in QoS, for example creation
+of the new traffic class.  

Meaning? Will the devlink API no longer reflect reality once one of
the VFs enables DCB for example?  

By DCB I mean the DCB that's implemented in the FW, and I'm not aware
of any flow that would enable the VF to tweak FW DCB on/off. Additionally
there is a commit in this patch series that should prevent any devlink-rate
changes if the FW DCB is enabled, and should prevent enabling FW DCB
enablement if any changes were made with the devlink-rate.

Nice, but in case DCB or TC/ADQ gets enabled devlink rate will just
show a stale hierarchy?

We need to document clearly that the driver is supposed to prevent
multiple APIs being used, and how we decide which API takes precedence.

I don't think there is a way to detect that the SW DCB is enabled though.
In that case the software would try to enforce it's own settings in the SW
stack and the HW would try to enforce devlink-rate settings.

quoted

quoted

+        consumed by the tree Node. Rate Limit is an absolute number
+        specifying a maximum amount of bytes a Node may consume during
+        the course of one second. Rate limit guarantees that a link will
+        not oversaturate the receiver on the remote end and also enforces
+        an SLA between the subscriber and network provider.
+    * - ``tx_share``  

Wouldn't it be more common to call this tx_min, like in the old VF API
and the cgroup APIs?  

I agree on this one, I'm not really sure why this attribute is called
tx_share. In it's iproute documentation tx_share is described as:
"specifies minimal tx rate value shared among all rate objects. If rate
object is a part of some rate group, then this value shared with rate
objects of this rate group.".
So tx_min is more intuitive, but I suspect that the original author
wanted to emphasize that this BW is shared among all the children
nodes.

Ah :/ I missed you're not adding this one :S