On 6/20/22 10:04 PM, Jakub Kicinski wrote:

On Mon, 20 Jun 2022 18:26:42 +0300 Dima Chumak wrote:

quoted

Currently, kernel provides a way to limit tx rate of a VF via devlink
rate function of a port. The underlying mechanism is a shaper applied to
all traffic passing through the target VF or a group of VFs. By its
essence, a shaper naturally works with outbound traffic, and in
practice, it's rarely seen to be implemented for inbound traffic.
Nevertheless, there is a user request to have a mechanism for limiting
inbound traffic as well. It is usually done by using some form of
traffic policing, dropping excess packets over the configured limit that
set by a user. Thus, introducing another limiting mechanism to the port
function can help close this gap.

This series introduces devlink attrs, along with their ops, to manage
rate policing of a single port as well as a port group. It is based on
the existing notion of leaf and node rate objects, and extends their
attributes to support both RX and TX limiting, for a number of packets
per second and/or a number of bytes per second. Additionally, there is a
second set of parameters for specifying the size of buffering performed,
called "burst", that controls the allowed level of spikes in traffic
before it starts getting dropped.

A new sub-type of a devlink_rate object is introduced, called
"limit_type". It can be either "shaping", the default, or "police".
A single leaf or a node object can be switched from one limit type to
another, but it cannot do both types of rate limiting simultaneously.
A node and a leaf object that have parent-child relationship must have
the same limit type. In other words, it's only possible to group rate
objects of the same limit type as their group's limit_type.

TC already has the police action. Your previous patches were accepted
because there was no exact match for shaping / admission. Now you're
"extending" that API to duplicate existing TC APIs. Infuriating.

I'm sorry for not being able to reply promptly.

I've re-read more carefully the cover letter of the original 'devlink:
rate objects API' series by Dmytro Linkin, off of which I based my
patches, though my understanding still might be incomplete/incorrect
here.

It seems that TC, being ingress only, doesn't cover the full spectrum of
rate-limiting that's possible to achieve with devlink. TC works only
with representors and doesn't allow to configure "the other side of the
wire", where devlink port function seems to be a better match as it
connects directly to a VF.

Also, for the existing devlink-rate mechanism of VF grouping, it would be
challenging to achieve similar functionality with TC flows, as groups don't
have a net device instance where flows can be attached.

I want to apologize in case my proposed changes have come across as
being bluntly ignoring some of the pre-established agreements and
understandings of TC / devlink responsibility separation, it wasn't
intentional.