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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 16 Jun 2023 16:06:14 +0000
From: "Zulkifli, Muhammad Husaini" <muhammad.husaini.zulkifli@...el.com>
To: Florian Kauer <florian.kauer@...utronix.de>, "Brandeburg, Jesse"
	<jesse.brandeburg@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "Gomes, Vinicius" <vinicius.gomes@...el.com>,
	"David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "Tan Tee
 Min" <tee.min.tan@...ux.intel.com>, "Gunasekaran, Aravindhan"
	<aravindhan.gunasekaran@...el.com>, "Chilakala, Mallikarjuna"
	<mallikarjuna.chilakala@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kurt@...utronix.de" <kurt@...utronix.de>
Subject: RE: [PATCH net-next 3/6] igc: Handle already enabled taprio offload
 for basetime 0

Dear Florian,

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