netdev - Re: [PATCH net 1/6] igc: Rename qbv_enable to taprio_offload

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: Tue, 11 Jul 2023 10:01:30 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
	edumazet@...gle.com, netdev@...r.kernel.org,
	Florian Kauer <florian.kauer@...utronix.de>, kurt@...utronix.de,
	vinicius.gomes@...el.com, muhammad.husaini.zulkifli@...el.com,
	tee.min.tan@...ux.intel.com, aravindhan.gunasekaran@...el.com,
	sasha.neftin@...el.com, Naama Meir <naamax.meir@...ux.intel.com>
Subject: Re: [PATCH net 1/6] igc: Rename qbv_enable to taprio_offload_enable

On Mon, Jul 10, 2023 at 09:34:58AM -0700, Tony Nguyen wrote:
> From: Florian Kauer <florian.kauer@...utronix.de>
> 
> In the current implementation the flags adapter->qbv_enable
> and IGC_FLAG_TSN_QBV_ENABLED have a similar name, but do not
> have the same meaning. The first one is used only to indicate
> taprio offload (i.e. when igc_save_qbv_schedule was called),
> while the second one corresponds to the Qbv mode of the hardware.
> However, the second one is also used to support the TX launchtime
> feature, i.e. ETF qdisc offload. This leads to situations where
> adapter->qbv_enable is false, but the flag IGC_FLAG_TSN_QBV_ENABLED
> is set. This is prone to confusion.
> 
> The rename should reduce this confusion. Since it is a pure
> rename, it has no impact on functionality.

And shouldn't be sent to net, but to net-next.

Thanks

> 
> 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>
> Tested-by: Naama Meir <naamax.meir@...ux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
>  drivers/net/ethernet/intel/igc/igc.h      | 2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c | 6 +++---
>  drivers/net/ethernet/intel/igc/igc_tsn.c  | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index 639a50c02537..9db384f66a8e 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -191,7 +191,7 @@ struct igc_adapter {
>  	int tc_setup_type;
>  	ktime_t base_time;
>  	ktime_t cycle_time;
> -	bool qbv_enable;
> +	bool taprio_offload_enable;
>  	u32 qbv_config_change_errors;
>  	bool qbv_transition;
>  	unsigned int qbv_count;
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 281a0e35b9d1..fae534ef1c4f 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6126,16 +6126,16 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
>  
>  	switch (qopt->cmd) {
>  	case TAPRIO_CMD_REPLACE:
> -		adapter->qbv_enable = true;
> +		adapter->taprio_offload_enable = true;
>  		break;
>  	case TAPRIO_CMD_DESTROY:
> -		adapter->qbv_enable = false;
> +		adapter->taprio_offload_enable = false;
>  		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
>  
> -	if (!adapter->qbv_enable)
> +	if (!adapter->taprio_offload_enable)
>  		return igc_tsn_clear_schedule(adapter);
>  
>  	if (qopt->base_time < 0)
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 3cdb0c988728..b76ebfc10b1d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -37,7 +37,7 @@ static unsigned int igc_tsn_new_flags(struct igc_adapter *adapter)
>  {
>  	unsigned int new_flags = adapter->flags & ~IGC_FLAG_TSN_ANY_ENABLED;
>  
> -	if (adapter->qbv_enable)
> +	if (adapter->taprio_offload_enable)
>  		new_flags |= IGC_FLAG_TSN_QBV_ENABLED;
>  
>  	if (is_any_launchtime(adapter))
> -- 
> 2.38.1
> 
>