diff mbox series

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

Message ID 20230614140714.14443-4-florian.kauer@linutronix.de
State New
Headers
Series igc: Fix corner cases for TSN offload |

Commit Message

Florian Kauer June 14, 2023, 2:07 p.m. UTC 
  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@linutronix.de>
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Zulkifli, Muhammad Husaini June 16, 2023, 4:06 p.m. UTC | #1 
Dear Florian,

> -----Original Message-----
> From: Florian Kauer <florian.kauer@linutronix.de>
> Sent: Wednesday, 14 June, 2023 10:07 PM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Tan Tee Min <tee.min.tan@linux.intel.com>; Zulkifli,
> Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; Gunasekaran,
> Aravindhan <aravindhan.gunasekaran@intel.com>; Chilakala, Mallikarjuna
> <mallikarjuna.chilakala@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; kurt@linutronix.de; florian.kauer@linutronix.de
> 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@linutronix.de>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> ---
>  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
diff mbox series

Patch

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;
 
 	if (!validate_schedule(adapter, qopt))