[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y90zwMijGCw79Ulw@corigine.com>
Date: Fri, 3 Feb 2023 17:18:08 +0100
From: Simon Horman <simon.horman@...igine.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Claudiu Manoil <claudiu.manoil@....com>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>,
Kurt Kanzenbach <kurt@...utronix.de>,
Jacob Keller <jacob.e.keller@...el.com>,
Jamal Hadi Salim <jhs@...atatu.com>,
Cong Wang <xiyou.wangcong@...il.com>,
Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH v5 net-next 08/17] net/sched: mqprio: allow reverse
TC:TXQ mappings
On Thu, Feb 02, 2023 at 02:36:12AM +0200, Vladimir Oltean wrote:
> By imposing that the last TXQ of TC i is smaller than the first TXQ of
> any TC j (j := i+1 .. n), mqprio imposes a strict ordering condition for
> the TXQ indices (they must increase as TCs increase).
>
> Claudiu points out that the complexity of the TXQ count validation is
> too high for this logic, i.e. instead of iterating over j, it is
> sufficient that the TXQ indices of TC i and i + 1 are ordered, and that
> will eventually ensure global ordering.
>
> This is true, however it doesn't appear to me that is what the code
> really intended to do. Instead, based on the comments, it just wanted to
> check for overlaps (and this isn't how one does that).
>
> So the following mqprio configuration, which I had recommended to
> Vinicius more than once for igb/igc (to account for the fact that on
> this hardware, lower numbered TXQs have higher dequeue priority than
> higher ones):
>
> num_tc 4 map 0 1 2 3 queues 1@3 1@2 1@1 1@0
>
> is in fact denied today by mqprio.
>
> The full story is that in fact, it's only denied with "hw 0"; if
> hardware offloading is requested, mqprio defers TXQ range overlap
> validation to the device driver (a strange decision in itself).
>
> This is most certainly a bug, but it's not one that has any merit for
> being fixed on "stable" as far as I can tell. This is because mqprio
> always rejected a configuration which was in fact valid, and this has
> shaped the way in which mqprio configuration scripts got built for
> various hardware (see igb/igc in the link below). Therefore, one could
> consider it to be merely an improvement for mqprio to allow reverse
> TC:TXQ mappings.
>
> Link: https://patchwork.kernel.org/project/netdevbpf/patch/20230130173145.475943-9-vladimir.oltean@nxp.com/#25188310
> Link: https://patchwork.kernel.org/project/netdevbpf/patch/20230128010719.2182346-6-vladimir.oltean@nxp.com/#25186442
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
Reviewed-by: Simon Horman <simon.horman@...igine.com>
Powered by blists - more mailing lists