Messages in this thread | | | Date | Mon, 1 Apr 2024 19:48:20 +0200 | From | Piotr Wejman <> | Subject | Re: [PATCH v3] net: stmmac: fix rx queue priority assignment |
| |
On Mon, Mar 11, 2024 at 01:41:44PM -0700, Jakub Kicinski wrote: > On Sun, 3 Mar 2024 20:03:38 +0100 Piotr Wejman wrote: > > The driver should ensure that same priority is not mapped to multiple > > rx queues. Currently rx_queue_priority() function is adding > > priorities for a queue without clearing them from others. > > Do you know what user-visible mis-behavior this may result in?
When changing priority to rx queue mapping with tc qdisc taprio command (man tc-taprio), all packets with priority assigned to multiple queues are dropped.
> > > From DesignWare Cores Ethernet Quality-of-Service > > Databook, section 17.1.29 MAC_RxQ_Ctrl2: > > "[...]The software must ensure that the content of this field is > > mutually exclusive to the PSRQ fields for other queues, that is, > > the same priority is not mapped to multiple Rx queues[...]" > > > > After this patch, rx_queue_priority() function will: > > - assign desired priorities to a queue > > - remove those priorities from all other queues > > But also you seem to remove clearing all other prios from the queue: > > - value &= ~GMAC_RXQCTRL_PSRQX_MASK(queue); > > and > > - value &= ~XGMAC_PSRQ(queue); > > is that intentional? Commit message should explain why.
Yes, that keeps other priorities assigned to that queue and only clears the same priorities from all other queues.
> > > The write sequence of CTRL2 and CTRL3 registers is done in the way to > > ensure this order. > > Ensure which order? Looks like you're actually writing in the opposite > order than what I'd expect :S First the register you want to assign to, > and then the register you only clear from. >
I meant the order you wrote: first assign new priorities to a queue, then clear them from others queues.
> When you repost please include a Fixes tag. > -- > pw-bot: cr
| |