On 2022-04-29 22:42, Jakub Kicinski wrote:

On Sat, 30 Apr 2022 02:00:05 +0000 Nambiar, Amritha wrote:

quoted

IIUC, currently the action skbedit queue_mapping is for transmit queue selection,
and the bound checking is w.r.to dev->real_num_tx_queues. Also, based on my
discussion with Alex (https://www.spinics.net/lists/netdev/msg761581.html), it
looks like this currently applies at the qdisc enqueue stage and not at the
classifier level.

They both apply at enqueue stage, AFAIU. Setting classid on ingress
does exactly nothing, no? :)

Neither is perfect, at least skbedit seems more straightforward.
I suspect modern DC operator may have little familiarity with classful
qdiscs and what classid is. Plus, again, you're assuming mqprio's
interpretation like it's a TC-wide thing.

skbedit OTOH is used with a clsact qdisc.

Also it would be good if what we did had some applicability to SW.
Maybe extend skbedit with a way of calling skb_record_rx_queue()?

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.

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

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.

cheers,
jamal