| 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
| ||
|
Date: Tue, 30 Aug 2022 16:01:56 -0500
From: Thinh Tran <thinhtr@...ux.vnet.ibm.com>
To: Manish Chopra <manishc@...vell.com>,
"kuba@...nel.org" <kuba@...nel.org>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Ariel Elior <aelior@...vell.com>,
"davem@...emloft.net" <davem@...emloft.net>,
Sudarsana Reddy Kalluru <skalluru@...vell.com>,
Alok Prasad <palok@...vell.com>
Subject: Re: [EXT] [PATCH] bnx2x: Fix error recovering in switch configuration
On 8/30/2022 4:19 AM, Manish Chopra wrote:
>> -----Original Message-----
>> From: Thinh Tran <thinhtr@...ux.vnet.ibm.com>
>> Sent: Friday, August 26, 2022 1:30 AM
>> To: kuba@...nel.org
>> Cc: netdev@...r.kernel.org; Ariel Elior <aelior@...vell.com>;
>> davem@...emloft.net; Manish Chopra <manishc@...vell.com>; Sudarsana
>> Reddy Kalluru <skalluru@...vell.com>; Thinh Tran
>> <thinhtr@...ux.vnet.ibm.com>
>> Subject: [EXT] [PATCH] bnx2x: Fix error recovering in switch configuration
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> As the BCM57810 and other I/O adapters are connected through a PCIe
>> switch, the bnx2x driver causes unexpected system hang/crash while handling
>> PCIe switch errors, if its error handler is called after other drivers' handlers.
>>
>> In this case, after numbers of bnx2x_tx_timout(), the
>> bnx2x_nic_unload() is called, frees up resources and calls
>> bnx2x_napi_disable(). Then when EEH calls its error handler, the
>> bnx2x_io_error_detected() and
>> bnx2x_io_slot_reset() also calling bnx2x_napi_disable() and freeing the
>> resources.
>>
>> This patch will:
>> - reduce the numbers of bnx2x_panic_dump() while in
>> bnx2x_tx_timeout(), avoid filling up dmesg buffer.
>> - use checking new napi_enable flags to prevent calling
>> disable again which causing system hangs.
>> - cheking if fp->page_pool already freed avoid system
>> crash.
>>
>> Signed-off-by: Thinh Tran <thinhtr@...ux.vnet.ibm.com>
>> ---
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 4 ++++
>> .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 24 ++++++++++++++++++-
>> .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 3 +++
>> 3 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
>> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
>> index dd5945c4bfec..7fa23d47907a 100644
>> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
>> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
>> @@ -1509,6 +1509,10 @@ struct bnx2x {
>> bool cnic_loaded;
>> struct cnic_eth_dev *(*cnic_probe)(struct net_device *);
>>
>> + bool napi_enable;
>> + bool cnic_napi_enable;
>> + int tx_timeout_cnt;
>> +
>> /* Flag that indicates that we can start looking for FCoE L2 queue
>> * completions in the default status block.
>> */
>> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>> index 712b5595bc39..bb8d91f44642 100644
>> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
>> @@ -1860,37 +1860,49 @@ static int bnx2x_setup_irqs(struct bnx2x *bp)
>> static void bnx2x_napi_enable_cnic(struct bnx2x *bp) {
>> int i;
>> + if (bp->cnic_napi_enable)
>> + return;
>>
>> for_each_rx_queue_cnic(bp, i) {
>> napi_enable(&bnx2x_fp(bp, i, napi));
>> }
>> + bp->cnic_napi_enable = true;
>> }
>>
>> static void bnx2x_napi_enable(struct bnx2x *bp) {
>> int i;
>> + if (bp->napi_enable)
>> + return;
>>
>> for_each_eth_queue(bp, i) {
>> napi_enable(&bnx2x_fp(bp, i, napi));
>> }
>> + bp->napi_enable = true;
>> }
>>
>> static void bnx2x_napi_disable_cnic(struct bnx2x *bp) {
>> int i;
>> + if (!bp->cnic_napi_enable)
>> + return;
>>
>> for_each_rx_queue_cnic(bp, i) {
>> napi_disable(&bnx2x_fp(bp, i, napi));
>> }
>> + bp->cnic_napi_enable = false;
>> }
>>
>> static void bnx2x_napi_disable(struct bnx2x *bp) {
>> int i;
>> + if (!bp->napi_enable)
>> + return;
>>
>> for_each_eth_queue(bp, i) {
>> napi_disable(&bnx2x_fp(bp, i, napi));
>> }
>> + bp->napi_enable = false;
>> }
>>
>> void bnx2x_netif_start(struct bnx2x *bp) @@ -2554,6 +2566,7 @@ int
>> bnx2x_load_cnic(struct bnx2x *bp)
>> }
>>
>> /* Add all CNIC NAPI objects */
>> + bp->cnic_napi_enable = false;
>> bnx2x_add_all_napi_cnic(bp);
>> DP(NETIF_MSG_IFUP, "cnic napi added\n");
>> bnx2x_napi_enable_cnic(bp);
>> @@ -2701,7 +2714,9 @@ int bnx2x_nic_load(struct bnx2x *bp, int
>> load_mode)
>> */
>> bnx2x_setup_tc(bp->dev, bp->max_cos);
>>
>> + bp->tx_timeout_cnt = 0;
>> /* Add all NAPI objects */
>> + bp->napi_enable = false;
>> bnx2x_add_all_napi(bp);
>> DP(NETIF_MSG_IFUP, "napi added\n");
>> bnx2x_napi_enable(bp);
>> @@ -4982,7 +4997,14 @@ void bnx2x_tx_timeout(struct net_device *dev,
>> unsigned int txqueue)
>> */
>> if (!bp->panic)
>> #ifndef BNX2X_STOP_ON_ERROR
>> - bnx2x_panic_dump(bp, false);
>> + {
>> + if (++bp->tx_timeout_cnt > 3) {
>> + bnx2x_panic_dump(bp, false);
>> + bp->tx_timeout_cnt = 0;
>> + } else {
>> + netdev_err(bp->dev, "TX timeout %d times\n", bp-
>>> tx_timeout_cnt);
>> + }
>> +
>
> Hello Trinh,
> bnx2x_panic_dump() dumps quite of some useful debug (fastpath, hw related) info to look at in the logs,
> I think they should be dumped at very first occurrence of tx timeout rather than waiting on next few tx timeout
> event (which may not even occur in some cases). One another way to prevent it could be by turning off the
> carrier (netif_carrier_off()) at very first place in bnx2x_tx_timeout() to prevent such repeated occurrences of tx timeout on the netdev.
>
> Thanks,
> Manish
will do
Thanks,
Thinh Tran
Powered by blists - more mailing lists