netdev - Re: [Intel-wired-lan] [PATCH iwl-net] idpf: fix transaction timeouts on reset

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]

Message-ID: <1102746b-58a8-44fb-98a9-a96db1096fe4@intel.com>
Date: Wed, 18 Dec 2024 16:13:22 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Emil Tantilov <emil.s.tantilov@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<larysa.zaremba@...el.com>, <decot@...gle.com>, <willemb@...gle.com>,
	<anthony.l.nguyen@...el.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net] idpf: fix transaction timeouts
 on reset

From: Emil Tantilov <emil.s.tantilov@...el.com>
Date: Tue, 17 Dec 2024 17:44:17 -0800

> Restore the call to idpf_vc_xn_shutdown() at the beginning of
> idpf_vc_core_deinit() provided the function is not called on
> remove. In the reset path this call is needed to prevent mailbox
> transactions from timing out.
> 
> Fixes: 09d0fb5cb30e ("idpf: deinit virtchnl transaction manager after vport and vectors")
> Reviewed-by: Larysa Zaremba <larysa.zaremba@...el.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
> ---
> Testing hints:
> echo 1 > /sys/class/net/<netif>/device/reset
> ---
>  drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index d46c95f91b0d..0387794daf17 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -3080,9 +3080,15 @@ void idpf_vc_core_deinit(struct idpf_adapter *adapter)
>  	if (!test_bit(IDPF_VC_CORE_INIT, adapter->flags))
>  		return;
>  
> +	/* Avoid transaction timeouts when called during reset */
> +	if (!test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
> +		idpf_vc_xn_shutdown(adapter->vcxn_mngr);
> +
>  	idpf_deinit_task(adapter);
>  	idpf_intr_rel(adapter);
> -	idpf_vc_xn_shutdown(adapter->vcxn_mngr);
> +
> +	if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
> +		idpf_vc_xn_shutdown(adapter->vcxn_mngr);

Why test it two times...

	bool reset;

	...

	reset = test_bit(REMOVE_IN_PROG);
	if (!reset)
		vc_xn_shutdown();

	deinit_task();
	intr_rel();

	if (reset)
		vc_xn_shutdown();

BTW can't we just move that call unconditionally?

>  
>  	cancel_delayed_work_sync(&adapter->serv_task);
>  	cancel_delayed_work_sync(&adapter->mbx_task);

Thanks,
Olek