| Message ID | 1602411781-6012-4-git-send-email-michael.chan@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | bnxt_en: Updates for net-next. | expand |
On Sun, 11 Oct 2020 06:22:55 -0400 Michael Chan wrote: > Currently, bp->msg_enable has default value of 0. It is more useful > to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by > default. > > Reviewed-by: Edwin Peer <edwin.peer@broadcom.com> > Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> > Signed-off-by: Michael Chan <michael.chan@broadcom.com> This will add whole bunch of output for "RX buffer error 4000[45]", no? That one needs to switch to a silent reset first, I'd think.
On Sun, Oct 11, 2020 at 12:34 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Sun, 11 Oct 2020 06:22:55 -0400 Michael Chan wrote: > > Currently, bp->msg_enable has default value of 0. It is more useful > > to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by > > default. > > > > Reviewed-by: Edwin Peer <edwin.peer@broadcom.com> > > Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> > > Signed-off-by: Michael Chan <michael.chan@broadcom.com> > > This will add whole bunch of output for "RX buffer error 4000[45]", no? > That one needs to switch to a silent reset first, I'd think. The last round of net-next patches have made changes to reduce this noise already. If the firmware supports the new RING_MONITOR scheme, There will be no more messages. The reset will be counted in a new ethtool -S counter only. If the firmware is older and does not support the new scheme, we've changed the logging to warn_once() in addition to the ethtool -S counter. There is no point in warning more than once.
On Sun, 11 Oct 2020 14:13:04 -0700 Michael Chan wrote: > On Sun, Oct 11, 2020 at 12:34 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Sun, 11 Oct 2020 06:22:55 -0400 Michael Chan wrote: > > > Currently, bp->msg_enable has default value of 0. It is more useful > > > to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by > > > default. > > > > > > Reviewed-by: Edwin Peer <edwin.peer@broadcom.com> > > > Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com> > > > Signed-off-by: Michael Chan <michael.chan@broadcom.com> > > > > This will add whole bunch of output for "RX buffer error 4000[45]", no? > > That one needs to switch to a silent reset first, I'd think. > > The last round of net-next patches have made changes to reduce this > noise already. If the firmware supports the new RING_MONITOR scheme, > There will be no more messages. The reset will be counted in a new > ethtool -S counter only. > > If the firmware is older and does not support the new scheme, we've > changed the logging to warn_once() in addition to the ethtool -S > counter. There is no point in warning more than once. I'm talking about bnxt_dbg_dump_states specifically. I'm looking at net-next: bnxt_rx_pkt bnxt_sched_reset set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event); bnxt_sp_task bnxt_reset bnxt_reset_task bnxt_dbg_dump_states
On Sun, Oct 11, 2020 at 2:53 PM Jakub Kicinski <kuba@kernel.org> wrote: > > I'm talking about bnxt_dbg_dump_states specifically. > > I'm looking at net-next: > > bnxt_rx_pkt > bnxt_sched_reset > set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event); > > > bnxt_sp_task > bnxt_reset > bnxt_reset_task > bnxt_dbg_dump_states > Yes, you are right. Although, we actually go through a slightly different path with a different event when we encounter these RX buffer errors: bnxt_sched_reset() set_bit(BNXT_RST_RING_SP_EVENT, &bp->sp_event) bnxt_rx_ring_reset() bnxt_hwrm_rx_ring_reset() But with the oldest firmware, this call would fail and we would fall back to bnxt_reset_task() with non-silent mode. I will respin and change this call to silent mode. Thanks.
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index d4402a2cd07f..0da6a1138eee 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -69,6 +69,7 @@ #include "bnxt_debugfs.h" #define BNXT_TX_TIMEOUT (5 * HZ) +#define BNXT_DEF_MSG_ENABLE (NETIF_MSG_DRV | NETIF_MSG_HW) MODULE_LICENSE("GPL"); MODULE_DESCRIPTION("Broadcom BCM573xx network driver"); @@ -12510,6 +12511,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) return -ENOMEM; bp = netdev_priv(dev); + bp->msg_enable = BNXT_DEF_MSG_ENABLE; bnxt_set_max_func_irqs(bp, max_irqs); if (bnxt_vf_pciid(ent->driver_data))