Message-ID: <20250214113815.37ttoor3isrt34dg@skbuf>
Date: Fri, 14 Feb 2025 13:38:15 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: "Abdul Rahim, Faizal" <faizal.abdul.rahim@...ux.intel.com>
Cc: Kurt Kanzenbach <kurt@...utronix.de>,
	Tony Nguyen <anthony.l.nguyen@...el.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Simon Horman <horms@...nel.org>,
	Russell King <linux@...linux.org.uk>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Furong Xu <0x1207@...il.com>,
	Russell King <rmk+kernel@...linux.org.uk>,
	Serge Semin <fancer.lancer@...il.com>,
	Xiaolei Wang <xiaolei.wang@...driver.com>,
	Suraj Jaiswal <quic_jsuraj@...cinc.com>,
	Kory Maincent <kory.maincent@...tlin.com>,
	Gal Pressman <gal@...dia.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Choong Yong Liang <yong.liang.choong@...ux.intel.com>,
	Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
	Vinicius Costa Gomes <vinicius.gomes@...el.com>,
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org
Subject: Re: [PATCH iwl-next v4 0/9] igc: Add support for Frame Preemption
 feature in IGC

On Fri, Feb 14, 2025 at 07:20:08PM +0800, Abdul Rahim, Faizal wrote:
> Okay. I can look into this in a separate patch submission, but just an
> FYI—this adds another dependency to the second part of the igc fpe
> submission (preemptible tc on taprio + mqprio). This new patch
> (driver-specific priv flag to control 2 different priority scheme) would
> need to be accepted first before the second part of igc fpe can be
> submitted.

So perhaps now you're starting to understand why I had initially
suggested you'd better draw the line now at just the MAC Merge layer
and focus on harmonizing taprio and mqprio TX scheduling in a separate
patch set.

I would expect that for uniform behavior, you would force the users a
little bit to adopt the new TX scheduling mode in taprio, otherwise any
configuration with preemptible traffic classes would be rejected by the
driver. So, if preemption is used, then the scheduling model is the same
between mqprio and taprio, and you don't have to handle preemptible
traffic classes over the old scheduling model.

I would also expect that you replace the current patch which handles
preemptible traffic classes in taprio with another one which explicitly
returns -EOPNOTSUPP if preemptible traffic classes exist - just like
mqprio. When Kurt added that condition in mqprio, it wasn't strictly
necessary, because mqprio_parse_tc_entries() already checks
ethtool_dev_mm_supported() in the core and rejects the configuration.
But after your MAC Merge series is accepted, you still won't be able to
handle preemptible traffic classes even though the core will let you, so
you will have to impose the restriction yourself, just like Kurt did.
I'm not trying to be negative, but it's imaginable that there's a chance
you won't succeed to bring the whole feature set to completion right
away, and if you do abandon things in the middle (MAC Merge layer
supported but preemptible traffic classes unsupported), it would be good
if the driver at least rejected a configuration it doesn't support,
instead of accepting it and not acting on it. Because if a significant
amount of time passes in such an inconsistent state, it would be very
difficult for anybody else to pick up from that position and not change
the behavior in incompatible ways that are user-visible.

Powered by blists - more mailing lists