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

Kurt Kanzenbach
2023-05-30T12:20:58Z

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]