netdev - RE: [PATCH net-next 1/1] bnx2x: Collect the device debug information during Tx timeout.

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] [day] [month] [year] [list]

Message-ID: <MW2PR07MB4139E55F48CF140F788EFABF8A6A0@MW2PR07MB4139.namprd07.prod.outlook.com>
Date:   Thu, 24 May 2018 11:40:34 +0000
From:   "Kalluru, Sudarsana" <Sudarsana.Kalluru@...ium.com>
To:     Yuval Mintz <yuvalm@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 1/1] bnx2x: Collect the device debug information
 during Tx timeout.


-----Original Message-----
From: Yuval Mintz [mailto:yuvalm@...lanox.com] 
Sent: 24 May 2018 16:55
To: Kalluru, Sudarsana <Sudarsana.Kalluru@...ium.com>; davem@...emloft.net
Cc: netdev@...r.kernel.org
Subject: RE: [PATCH net-next 1/1] bnx2x: Collect the device debug information during Tx timeout.

> Tx-timeout mostly happens due to some issue in the device. In such 
> cases, debug dump would be helpful for identifying the cause of the issue.
> This patch adds support to spill debug data during the Tx timeout. 
> Here
> bnx2x_panic_dump() API is used instead of bnx2x_panic(), since we 
> still want to allow the Tx-timeout recovery a chance to succeed.
> 
> Please consider applying this to "net-next".
> 
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@...ium.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 95871576..182d5e1 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -4962,8 +4962,13 @@ void bnx2x_tx_timeout(struct net_device *dev)  
> {
>  	struct bnx2x *bp = netdev_priv(dev);
> 
> -#ifdef BNX2X_STOP_ON_ERROR
> +	/* We want the information of the dump logged,
> +	 * but calling bnx2x_panic() would kill all chances of recovery.
> +	 */
>  	if (!bp->panic)
> +#ifdef BNX2X_STOP_ON_ERROR
> +		bnx2x_panic_dump(bp, false);
> +#else
>  		bnx2x_panic();
>  #endif

This looks backward to me; When BNX2X_STOP_ON_ERROR is defined you *want* bnx2x_panic() to fatally stop the device, not the other way around.
I.e., s/ifdef/ifndef/ 


Yuval, thanks a lot for your review. Will post the updated changes.

> 
> --
> 1.8.3.1