RE: [PATCH net-next 3/6] igc: Handle already enabled taprio offload for basetime 0

[Zulkifli, Muhammad Husaini]

Dear Florian,

> -----Original Message-----
> From: Florian Kauer <florian.kauer@xxxxxxxxxxxxx>
> Sent: Wednesday, 14 June, 2023 10:07 PM
> To: Brandeburg, Jesse <jesse.brandeburg@xxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Gomes, Vinicius <vinicius.gomes@xxxxxxxxx>;
> David S . Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Tan Tee Min <tee.min.tan@xxxxxxxxxxxxxxx>; Zulkifli,
> Muhammad Husaini <muhammad.husaini.zulkifli@xxxxxxxxx>; Gunasekaran,
> Aravindhan <aravindhan.gunasekaran@xxxxxxxxx>; Chilakala, Mallikarjuna
> <mallikarjuna.chilakala@xxxxxxxxx>
> Cc: intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; kurt@xxxxxxxxxxxxx; florian.kauer@xxxxxxxxxxxxx
> Subject: [PATCH net-next 3/6] igc: Handle already enabled taprio offload for
> basetime 0
> 
> Since commit e17090eb2494 ("igc: allow BaseTime 0 enrollment for Qbv") it is
> possible to enable taprio offload with a basetime of 0.
> However, the check if taprio offload is already enabled (and thus -EALREADY
> should be returned for igc_save_qbv_schedule) still relied on
> adapter->base_time > 0.
> 
> This can be reproduced as follows:
> 
>     # TAPRIO offload (flags == 0x2) and base-time = 0
>     sudo tc qdisc replace dev enp1s0 parent root handle 100 stab overhead 24
> taprio \
> 	    num_tc 1 \
> 	    map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
> 	    queues 1@0 \
> 	    base-time 0 \
> 	    sched-entry S 01 300000 \
> 	    flags 0x2
> 
>     # The second call should fail with "Error: Device failed to setup taprio offload."
>     # But that only happens if base-time was != 0
>     sudo tc qdisc replace dev enp1s0 parent root handle 100 stab overhead 24
> taprio \
> 	    num_tc 1 \
> 	    map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
> 	    queues 1@0 \
> 	    base-time 0 \
> 	    sched-entry S 01 300000 \
> 	    flags 0x2
> 
> Fixes: e17090eb2494 ("igc: allow BaseTime 0 enrollment for Qbv")
> Signed-off-by: Florian Kauer <florian.kauer@xxxxxxxxxxxxx>
> Reviewed-by: Kurt Kanzenbach <kurt@xxxxxxxxxxxxx>
> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index 122158b321d5..35ace8d338a5 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6123,7 +6123,7 @@ static int igc_save_qbv_schedule(struct igc_adapter
> *adapter,
>  	if (qopt->base_time < 0)
>  		return -ERANGE;
> 
> -	if (igc_is_device_id_i225(hw) && adapter->base_time)
> +	if (igc_is_device_id_i225(hw) && adapter->taprio_offload_enable)
>  		return -EALREADY;

I appreciate you catching that. Only i225 is affected by this; i226 is unaffected.

> 
>  	if (!validate_schedule(adapter, qopt))
> --
> 2.39.2