netdev - RE: [E1000-devel] [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware 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]

Date:	Mon, 18 Jul 2016 22:30:29 +0000
From:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
To:	"zyjzyj2000@...il.com" <zyjzyj2000@...il.com>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
Subject: RE: [E1000-devel] [PATCH 1/1] ixgbevf: avoid checking hang when
	performing hardware reset

> -----Original Message-----
> From: zyjzyj2000@...il.com [mailto:zyjzyj2000@...il.com]
> Sent: Monday, July 18, 2016 6:45 AM
> To: e1000-devel@...ts.sourceforge.net; netdev@...r.kernel.org; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@...el.com>
> Subject: [E1000-devel] [PATCH 1/1] ixgbevf: avoid checking hang when
> performing hardware reset
> 
> From: Zhu Yanjun <zyjzyj2000@...il.com>
> 
> When performing hardware reset, it is not necessary to check hang.
> Or else, the call trace will appear.
> 
> Signed-off-by: Zhu Yanjun <zyjzyj2000@...il.com>
> ---
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index acc2401..d563d24 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -2792,9 +2792,14 @@ static void ixgbevf_reset_subtask(struct
> ixgbevf_adapter *adapter)  static void ixgbevf_check_hang_subtask(struct
> ixgbevf_adapter *adapter)  {
>  	struct ixgbe_hw *hw = &adapter->hw;
> +	struct ixgbe_mbx_info *mbx = &hw->mbx;
>  	u32 eics = 0;
>  	int i;
> 
> +	/* When performing hardware reset, unnecessary to check hang. */
> +	if (mbx->ops.check_for_rst(hw))
> +		return;
> +
>  	/* If we're down or resetting, just bail */
>  	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
>  	    test_bit(__IXGBEVF_RESETTING, &adapter->state))
> --
> 2.7.4
> 
> 

My concern with this patch is that the check_for_rst does a read to clear on the RSTD and RSTI bits.  So reading it in the service task may mean we miss this transition in the mailbox protocol.  Likewise it is possible they may have already been cleared and you won't even catch the state your looking for.

-Don Skidmore <donald.c.skidmore@...el.com>