From f9b76ebd49f97458857568918c305a17fa7c6567 Mon Sep 17 00:00:00 2001 From: Michael Chan Date: Tue, 11 Jul 2017 13:05:34 -0400 Subject: bnxt_en: Fix race conditions in .ndo_get_stats64(). .ndo_get_stats64() may not be protected by RTNL and can race with .ndo_stop() or other ethtool operations that can free the statistics memory. Fix it by setting a new flag BNXT_STATE_READ_STATS and then proceeding to read statistics memory only if the state is OPEN. The close path that frees the memory clears the OPEN state and then waits for the BNXT_STATE_READ_STATS to clear before proceeding to free the statistics memory. Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.") Signed-off-by: Michael Chan Signed-off-by: David S. Miller --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 18 ++++++++++++++++-- drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index a19f68f5862d..415694d37989 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -6279,6 +6279,12 @@ static int bnxt_open(struct net_device *dev) return __bnxt_open_nic(bp, true, true); } +static bool bnxt_drv_busy(struct bnxt *bp) +{ + return (test_bit(BNXT_STATE_IN_SP_TASK, &bp->state) || + test_bit(BNXT_STATE_READ_STATS, &bp->state)); +} + int bnxt_close_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init) { int rc = 0; @@ -6297,7 +6303,7 @@ int bnxt_close_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init) clear_bit(BNXT_STATE_OPEN, &bp->state); smp_mb__after_atomic(); - while (test_bit(BNXT_STATE_IN_SP_TASK, &bp->state)) + while (bnxt_drv_busy(bp)) msleep(20); /* Flush rings and and disable interrupts */ @@ -6358,8 +6364,15 @@ bnxt_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats) u32 i; struct bnxt *bp = netdev_priv(dev); - if (!bp->bnapi) + set_bit(BNXT_STATE_READ_STATS, &bp->state); + /* Make sure bnxt_close_nic() sees that we are reading stats before + * we check the BNXT_STATE_OPEN flag. + */ + smp_mb__after_atomic(); + if (!test_bit(BNXT_STATE_OPEN, &bp->state)) { + clear_bit(BNXT_STATE_READ_STATS, &bp->state); return; + } /* TODO check if we need to synchronize with bnxt_close path */ for (i = 0; i < bp->cp_nr_rings; i++) { @@ -6406,6 +6419,7 @@ bnxt_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats) stats->tx_fifo_errors = le64_to_cpu(tx->tx_fifo_underruns); stats->tx_errors = le64_to_cpu(tx->tx_err); } + clear_bit(BNXT_STATE_READ_STATS, &bp->state); } static bool bnxt_mc_list_updated(struct bnxt *bp, u32 *rx_mask) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h index f872a7db2ca8..3c9d484dbd4e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h @@ -1117,6 +1117,7 @@ struct bnxt { unsigned long state; #define BNXT_STATE_OPEN 0 #define BNXT_STATE_IN_SP_TASK 1 +#define BNXT_STATE_READ_STATS 2 struct bnxt_irq *irq_tbl; int total_irqs; -- cgit From 3b6b34df342553a7522561e34288f5bb803aa9aa Mon Sep 17 00:00:00 2001 From: Michael Chan Date: Tue, 11 Jul 2017 13:05:35 -0400 Subject: bnxt_en: Fix bug in ethtool -L. When changing channels from combined to rx/tx or vice versa, the code uses the wrong "sh" parameter to determine if we are reserving rings for shared or non-shared mode. It should be using the ethtool requested "sh" parameter instead of the current "sh" parameter. Fix it by passing the "sh" parameter to bnxt_reserve_rings(). For ethtool, we will pass in the requested "sh" parameter. Fixes: 391be5c27364 ("bnxt_en: Implement new scheme to reserve tx rings.") Signed-off-by: Michael Chan Signed-off-by: David S. Miller --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++------ drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 ++- drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 3 ++- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 415694d37989..d9830d09e6c3 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -6918,16 +6918,13 @@ static void bnxt_sp_task(struct work_struct *work) } /* Under rtnl_lock */ -int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs, int tx_xdp) +int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs, + int tx_xdp) { int max_rx, max_tx, tx_sets = 1; int tx_rings_needed; - bool sh = true; int rc; - if (!(bp->flags & BNXT_FLAG_SHARED_RINGS)) - sh = false; - if (tcs) tx_sets = tcs; @@ -7135,7 +7132,7 @@ int bnxt_setup_mq_tc(struct net_device *dev, u8 tc) sh = true; rc = bnxt_reserve_rings(bp, bp->tx_nr_rings_per_tc, bp->rx_nr_rings, - tc, bp->tx_nr_rings_xdp); + sh, tc, bp->tx_nr_rings_xdp); if (rc) return rc; diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h index 3c9d484dbd4e..f34691f85602 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h @@ -1301,7 +1301,8 @@ int bnxt_open_nic(struct bnxt *, bool, bool); int bnxt_half_open_nic(struct bnxt *bp); void bnxt_half_close_nic(struct bnxt *bp); int bnxt_close_nic(struct bnxt *, bool, bool); -int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs, int tx_xdp); +int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs, + int tx_xdp); int bnxt_setup_mq_tc(struct net_device *dev, u8 tc); int bnxt_get_max_rings(struct bnxt *, int *, int *, bool); void bnxt_restore_pf_fw_resources(struct bnxt *bp); diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index fd1181510b65..be6acadcb202 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -432,7 +432,8 @@ static int bnxt_set_channels(struct net_device *dev, } tx_xdp = req_rx_rings; } - rc = bnxt_reserve_rings(bp, req_tx_rings, req_rx_rings, tcs, tx_xdp); + rc = bnxt_reserve_rings(bp, req_tx_rings, req_rx_rings, sh, tcs, + tx_xdp); if (rc) { netdev_warn(dev, "Unable to allocate the requested rings\n"); return rc; diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c index 7d67552e70d7..3961a6807454 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c @@ -170,7 +170,7 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog) if (!tc) tc = 1; rc = bnxt_reserve_rings(bp, bp->tx_nr_rings_per_tc, bp->rx_nr_rings, - tc, tx_xdp); + true, tc, tx_xdp); if (rc) { netdev_warn(dev, "Unable to reserve enough TX rings to support XDP.\n"); return rc; -- cgit From 9b0436c3f29483ca91d890b0072c0c02e2e535ed Mon Sep 17 00:00:00 2001 From: Michael Chan Date: Tue, 11 Jul 2017 13:05:36 -0400 Subject: bnxt_en: Fix SRIOV on big-endian architecture. The PF driver sets up a list of firmware commands from the VF driver that needs to be forwarded to the PF for approval. This list is a 256-bit bitmap. The code that sets up the bitmap falls apart on big-endian architecture. __set_bit() does not work because it operates on long types whereas the firmware interface is defined in u32 types, causing bits in the wrong 32-bit word to be set. Fix it by setting the proper bits on an array of u32. Fixes: de68f5de5651 ("bnxt_en: Fix bitmap declaration to work on 32-bit arches.") Reported-by: Shannon Nelson Signed-off-by: Michael Chan Signed-off-by: David S. Miller --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index d9830d09e6c3..e7c8539cbddf 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -3458,13 +3458,18 @@ static int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp) req.ver_upd = DRV_VER_UPD; if (BNXT_PF(bp)) { - DECLARE_BITMAP(vf_req_snif_bmap, 256); - u32 *data = (u32 *)vf_req_snif_bmap; + u32 data[8]; int i; - memset(vf_req_snif_bmap, 0, sizeof(vf_req_snif_bmap)); - for (i = 0; i < ARRAY_SIZE(bnxt_vf_req_snif); i++) - __set_bit(bnxt_vf_req_snif[i], vf_req_snif_bmap); + memset(data, 0, sizeof(data)); + for (i = 0; i < ARRAY_SIZE(bnxt_vf_req_snif); i++) { + u16 cmd = bnxt_vf_req_snif[i]; + unsigned int bit, idx; + + idx = cmd / 32; + bit = cmd % 32; + data[idx] |= 1 << bit; + } for (i = 0; i < 8; i++) req.vf_req_fwd[i] = cpu_to_le32(data[i]); -- cgit