On 2022-05-03 11:47, Jakub Kicinski wrote:

On Tue, 3 May 2022 06:32:01 -0400 Jamal Hadi Salim wrote:

quoted

I am on the fence of "six of one, half a dozen of the other" ;->

TC classids have *always been used to identify queues* (or hierarchy of
but always leading to a single queue). Essentially a classid identity
is equivalent to a built-in action which says which queue to pick.
The data structure is very much engrained in the tc psyche.

When TX side HW queues(IIRC, mqprio) showed up there was ambiguity to
distinguish between a s/w queue vs a h/w queue hence queue_mapping
which allows us to override the *HW TX queue* selection - or at least
that was the intended goal.
Note: There are other ways to tell the h/w what queues to use on TX
(eg skb->priority) i.e there's no monopoly by queue_mapping.

Given lack of s/w queues on RX (hence lack of ambiguity) it seemed
natural that the classid could be used to describe the RX queue
identity for offload, it's just there.
I thought it was brilliant when Amritha first posted.

Is it just me or is TC generally considered highly confusing?

Hopefully we can discuss what is confusing (otherwise this becomes the
internet troll discussion of "tc sucks").
The fact that folks find ways to use s/w in an unintended ways is
not unique to tc. The architecture is clean.

IMO using a qdisc cls construct in clsact is only going to add
to that.

Assigning classid can still be meaningful on ingress in case
of a switch where there are actual qdiscs offloaded.

quoted

I think we should pick the simpler of "half-dozen or six".
The posted patch seems driver-only change i.e no core code
changes required (which other vendors could follow).
But i am not sure if that defines "simpler".

No core changes is not an argument we should take seriously upstream.

I am afraid, that sounds like a blanket statement though.
It should be taken seriously if it helps maintainability
but like i said i wasnt sure having not seen the alternative.

quoted

BTW:
Didnt follow the skb_record_rx_queue() thought - isnt that
always set by the driver based on which h/w queue the packet
arrived at? Whereas the semantics of this is to tell the h/w
what queue to use.

Overriding the queue mapping in the SW could still be useful
if TC wants to override the actual queue ID assigned by the NIC.

This way whether the action gets offloaded or not the resulting
skb will have the same metadata (in case of offload because it
came on the right queue and the driver set the mapping, in case
of sw because we overwrote the mapping).

IIUC, you are suggesting that using the same semantics for ingress and
egress is more intuitive - and that is a fair arguement.
Hope you understand my view when I said it was half a dozen or six
given we have multiple approaches today on egress that signal the
hardware what queue to use (and classid's intent is/was to
select a queue).

cheers,
jamal