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

Message-ID: <VI1PR0501MB2784A00EB05152522E2BF8A0BF6A0@VI1PR0501MB2784.eurprd05.prod.outlook.com>
Date:   Thu, 24 May 2018 11:25:22 +0000
From:   Yuval Mintz <yuvalm@...lanox.com>
To:     Sudarsana Reddy Kalluru <sudarsana.kalluru@...ium.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.

> 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/ 

> 
> --
> 1.8.3.1