Re: [Intel-wired-lan] [PATCH net-next 2/5] net/sched: taprio: replace tc_taprio_qopt_offload :: enable with a "cmd" enum

From: Kurt Kanzenbach <kurt@linutronix.de>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, netdev@vger.kernel.org
Cc: Andrew Lunn <andrew@lunn.ch>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Ferenc Fejes <ferenc.fejes@ericsson.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Eric Dumazet <edumazet@google.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Gerhard Engleder <gerhard@engleder-embedded.com>,
	UNGLinuxDriver@microchip.com,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com>,
	intel-wired-lan@lists.osuosl.org,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Pranavi Somisetty <pranavi.somisetty@amd.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Roger Quadros <rogerq@kernel.org>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>,
	Harini Katakam <harini.katakam@amd.com>,
	linux-kernel@vger.kernel.org, Jose Abreu <joabreu@synopsys.com>,
	Oleksij Rempel <linux@rempel-privat.de>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH net-next 2/5] net/sched: taprio: replace tc_taprio_qopt_offload :: enable with a "cmd" enum
Date: Tue, 30 May 2023 14:20:50 +0200	[thread overview]
Message-ID: <87leh6qacd.fsf@kurt> (raw)
In-Reply-To: <20230530091948.1408477-3-vladimir.oltean@nxp.com>

[-- Attachment #1.1: Type: text/plain, Size: 2060 bytes --]

On Tue May 30 2023, Vladimir Oltean wrote:
> Inspired from struct flow_cls_offload :: cmd, in order for taprio to be
> able to report statistics (which is future work), it seems that we need
> to drill one step further with the ndo_setup_tc(TC_SETUP_QDISC_TAPRIO)
> multiplexing, and pass the command as part of the common portion of the
> muxed structure.
>
> Since we already have an "enable" variable in tc_taprio_qopt_offload,
> refactor all drivers to check for "cmd" instead of "enable", and reject
> every other command except "replace" and "destroy" - to be future proof.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

[...]

> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 595a548bb0a8..af50001ccdd4 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> @@ -1885,13 +1885,17 @@ static int hellcreek_port_setup_tc(struct dsa_switch *ds, int port,
>  	case TC_SETUP_QDISC_TAPRIO: {
>  		struct tc_taprio_qopt_offload *taprio = type_data;
>  
> -		if (!hellcreek_validate_schedule(hellcreek, taprio))
> -			return -EOPNOTSUPP;
> +		switch (taprio->cmd) {
> +		case TAPRIO_CMD_REPLACE:
> +			if (!hellcreek_validate_schedule(hellcreek, taprio))
> +				return -EOPNOTSUPP;
>  
> -		if (taprio->enable)
>  			return hellcreek_port_set_schedule(ds, port, taprio);
> -
> -		return hellcreek_port_del_schedule(ds, port);
> +		case TAPRIO_CMD_DESTROY:
> +			return hellcreek_port_del_schedule(ds, port);
> +		default:
> +			return -EOPNOTSUPP;
> +		}
>  	}
>  	default:
>  		return -EOPNOTSUPP;

Uhm, seems like the current code validates the schedule even for
removing a schedule which seems a bit odd. With your changes it looks
correct.

Acked-by: Kurt Kanzenbach <kurt@linutronix.de> # hellcreek

Anyway, the hellcreek device has Tx overrun counters per TC. Even though
they should be zero, simply because the hardware Length Aware Shaper is
enabled by default.

Thanks,
Kurt

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

[-- Attachment #2: Type: text/plain, Size: 162 bytes --]

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan