All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/3] bnxt_en: 3 bug fixes.
@ 2020-07-12  0:48 Michael Chan
  2020-07-12  0:48 ` [PATCH net 1/3] bnxt_en: Fix race when modifying pause settings Michael Chan
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Michael Chan @ 2020-07-12  0:48 UTC (permalink / raw
  To: davem; +Cc: netdev, kuba

2 Fixes related to PHY/link settings.  The last one fixes the sizing of
the completion ring.

Please also queue for -stable.  Thanks.

Michael Chan (1):
  bnxt_en: Fix completion ring sizing with TPA enabled.

Vasundhara Volam (2):
  bnxt_en: Fix race when modifying pause settings.
  bnxt_en: Init ethtool link settings after reading updated PHY
    configuration.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 22 +++++++++++++++-------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  5 ++++-
 2 files changed, 19 insertions(+), 8 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 1/3] bnxt_en: Fix race when modifying pause settings.
  2020-07-12  0:48 [PATCH net 0/3] bnxt_en: 3 bug fixes Michael Chan
@ 2020-07-12  0:48 ` Michael Chan
  2020-07-12  0:48 ` [PATCH net 2/3] bnxt_en: Init ethtool link settings after reading updated PHY configuration Michael Chan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Chan @ 2020-07-12  0:48 UTC (permalink / raw
  To: davem; +Cc: netdev, kuba

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

The driver was modified to not rely on rtnl lock to protect link
settings about 2 years ago.  The pause setting was missed when
making that change.  Fix it by acquiring link_lock mutex before
calling bnxt_hwrm_set_pause().

Fixes: e2dc9b6e38fa ("bnxt_en: Don't use rtnl lock to protect link change logic in workqueue.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 6b88143..b4aa56d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1765,8 +1765,11 @@ static int bnxt_set_pauseparam(struct net_device *dev,
 	if (epause->tx_pause)
 		link_info->req_flow_ctrl |= BNXT_LINK_PAUSE_TX;
 
-	if (netif_running(dev))
+	if (netif_running(dev)) {
+		mutex_lock(&bp->link_lock);
 		rc = bnxt_hwrm_set_pause(bp);
+		mutex_unlock(&bp->link_lock);
+	}
 	return rc;
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 2/3] bnxt_en: Init ethtool link settings after reading updated PHY configuration.
  2020-07-12  0:48 [PATCH net 0/3] bnxt_en: 3 bug fixes Michael Chan
  2020-07-12  0:48 ` [PATCH net 1/3] bnxt_en: Fix race when modifying pause settings Michael Chan
@ 2020-07-12  0:48 ` Michael Chan
  2020-07-12  0:48 ` [PATCH net 3/3] bnxt_en: Fix completion ring sizing with TPA enabled Michael Chan
  2020-07-12 22:29 ` [PATCH net 0/3] bnxt_en: 3 bug fixes David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Chan @ 2020-07-12  0:48 UTC (permalink / raw
  To: davem; +Cc: netdev, kuba

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

In a shared port PHY configuration, async event is received when any of the
port modifies the configuration. Ethtool link settings should be
initialised after updated PHY configuration from firmware.

Fixes: b1613e78e98d ("bnxt_en: Add async. event logic for PHY configuration changes.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6a884df..28147e4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10385,15 +10385,15 @@ static void bnxt_sp_task(struct work_struct *work)
 				       &bp->sp_event))
 			bnxt_hwrm_phy_qcaps(bp);
 
-		if (test_and_clear_bit(BNXT_LINK_CFG_CHANGE_SP_EVENT,
-				       &bp->sp_event))
-			bnxt_init_ethtool_link_settings(bp);
-
 		rc = bnxt_update_link(bp, true);
-		mutex_unlock(&bp->link_lock);
 		if (rc)
 			netdev_err(bp->dev, "SP task can't update link (rc: %x)\n",
 				   rc);
+
+		if (test_and_clear_bit(BNXT_LINK_CFG_CHANGE_SP_EVENT,
+				       &bp->sp_event))
+			bnxt_init_ethtool_link_settings(bp);
+		mutex_unlock(&bp->link_lock);
 	}
 	if (test_and_clear_bit(BNXT_UPDATE_PHY_SP_EVENT, &bp->sp_event)) {
 		int rc;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 3/3] bnxt_en: Fix completion ring sizing with TPA enabled.
  2020-07-12  0:48 [PATCH net 0/3] bnxt_en: 3 bug fixes Michael Chan
  2020-07-12  0:48 ` [PATCH net 1/3] bnxt_en: Fix race when modifying pause settings Michael Chan
  2020-07-12  0:48 ` [PATCH net 2/3] bnxt_en: Init ethtool link settings after reading updated PHY configuration Michael Chan
@ 2020-07-12  0:48 ` Michael Chan
  2020-07-12 22:29 ` [PATCH net 0/3] bnxt_en: 3 bug fixes David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Michael Chan @ 2020-07-12  0:48 UTC (permalink / raw
  To: davem; +Cc: netdev, kuba

The current completion ring sizing formula is wrong with TPA enabled.
The formula assumes that the number of TPA completions are bound by the
RX ring size, but that's not true.  TPA_START completions are immediately
recycled so they are not bound by the RX ring size.  We must add
bp->max_tpa to the worst case maximum RX and TPA completions.

The completion ring can overflow because of this mistake.  This will
cause hardware to disable the completion ring when this happens,
leading to RX and TX traffic to stall on that ring.  This issue is
generally exposed only when the RX ring size is set very small.

Fix the formula by adding bp->max_tpa to the number of RX completions
if TPA is enabled.

Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.");
Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 28147e4..7463a18 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3418,7 +3418,7 @@ void bnxt_set_tpa_flags(struct bnxt *bp)
  */
 void bnxt_set_ring_params(struct bnxt *bp)
 {
-	u32 ring_size, rx_size, rx_space;
+	u32 ring_size, rx_size, rx_space, max_rx_cmpl;
 	u32 agg_factor = 0, agg_ring_size = 0;
 
 	/* 8 for CRC and VLAN */
@@ -3474,7 +3474,15 @@ void bnxt_set_ring_params(struct bnxt *bp)
 	bp->tx_nr_pages = bnxt_calc_nr_ring_pages(ring_size, TX_DESC_CNT);
 	bp->tx_ring_mask = (bp->tx_nr_pages * TX_DESC_CNT) - 1;
 
-	ring_size = bp->rx_ring_size * (2 + agg_factor) + bp->tx_ring_size;
+	max_rx_cmpl = bp->rx_ring_size;
+	/* MAX TPA needs to be added because TPA_START completions are
+	 * immediately recycled, so the TPA completions are not bound by
+	 * the RX ring size.
+	 */
+	if (bp->flags & BNXT_FLAG_TPA)
+		max_rx_cmpl += bp->max_tpa;
+	/* RX and TPA completions are 32-byte, all others are 16-byte */
+	ring_size = max_rx_cmpl * 2 + agg_ring_size + bp->tx_ring_size;
 	bp->cp_ring_size = ring_size;
 
 	bp->cp_nr_pages = bnxt_calc_nr_ring_pages(ring_size, CP_DESC_CNT);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net 0/3] bnxt_en: 3 bug fixes.
  2020-07-12  0:48 [PATCH net 0/3] bnxt_en: 3 bug fixes Michael Chan
                   ` (2 preceding siblings ...)
  2020-07-12  0:48 ` [PATCH net 3/3] bnxt_en: Fix completion ring sizing with TPA enabled Michael Chan
@ 2020-07-12 22:29 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2020-07-12 22:29 UTC (permalink / raw
  To: michael.chan; +Cc: netdev, kuba

From: Michael Chan <michael.chan@broadcom.com>
Date: Sat, 11 Jul 2020 20:48:22 -0400

> 2 Fixes related to PHY/link settings.  The last one fixes the sizing of
> the completion ring.
> 
> Please also queue for -stable.  Thanks.

Series applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-12 22:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-12  0:48 [PATCH net 0/3] bnxt_en: 3 bug fixes Michael Chan
2020-07-12  0:48 ` [PATCH net 1/3] bnxt_en: Fix race when modifying pause settings Michael Chan
2020-07-12  0:48 ` [PATCH net 2/3] bnxt_en: Init ethtool link settings after reading updated PHY configuration Michael Chan
2020-07-12  0:48 ` [PATCH net 3/3] bnxt_en: Fix completion ring sizing with TPA enabled Michael Chan
2020-07-12 22:29 ` [PATCH net 0/3] bnxt_en: 3 bug fixes David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.