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

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

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.

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