[PATCH net-next 0/3] bnxt_en: MSIX improvements

[PATCH net-next 0/3] bnxt_en: MSIX improvements

[Michael Chan]

This patchset makes some improvements related to MSIX.  The first
patch adjusts the default MSIX vectors assigned for RoCE.  On the
PF, the number of MSIX is increased to 64 from the current 9.  The
second patch allocates additional MSIX vectors ahead of time when
changing ethtool channels if dynamic MSIX is supported.  The 3rd
patch makes sure that the IRQ name is not truncated.

Edwin Peer (1):
  bnxt_en: resize bnxt_irq name field to fit format string

Michael Chan (2):
  bnxt_en: Increase the number of MSIX vectors for RoCE device
  bnxt_en: Add MSIX check in bnxt_check_rings()

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 19 ++++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  5 ++++-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 11 ++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 14 ++++++++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  6 ++++--
 5 files changed, 42 insertions(+), 13 deletions(-)

-- 
2.30.1

[PATCH net-next 1/3] bnxt_en: Increase the number of MSIX vectors for RoCE device

[Michael Chan]

If RocE is supported on the device, set the number of RoCE MSIX vectors
to the number of online CPUs + 1 and capped at these maximums:

VF: 2
NPAR: 5
PF: 64

For the PF, the maximum is now increased from the previous value
of 9 to get better performance for kernel applications.

Remove the unnecessary check for BNXT_FLAG_ROCE_CAP.
bnxt_set_dflt_ulp_msix() will only be called if the flag is set.

Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 14 ++++++++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  6 ++++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index b9e7d3e7b15d..fdd6356f21ef 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -176,11 +176,17 @@ EXPORT_SYMBOL(bnxt_unregister_dev);
 
 static int bnxt_set_dflt_ulp_msix(struct bnxt *bp)
 {
-	u32 roce_msix = BNXT_VF(bp) ?
-			BNXT_MAX_VF_ROCE_MSIX : BNXT_MAX_ROCE_MSIX;
+	int roce_msix = BNXT_MAX_ROCE_MSIX;
 
-	return ((bp->flags & BNXT_FLAG_ROCE_CAP) ?
-		min_t(u32, roce_msix, num_online_cpus()) : 0);
+	if (BNXT_VF(bp))
+		roce_msix = BNXT_MAX_ROCE_MSIX_VF;
+	else if (bp->port_partition_type)
+		roce_msix = BNXT_MAX_ROCE_MSIX_NPAR_PF;
+
+	/* NQ MSIX vectors should match the number of CPUs plus 1 more for
+	 * the CREQ MSIX, up to the default.
+	 */
+	return min_t(int, roce_msix, num_online_cpus() + 1);
 }
 
 int bnxt_send_msg(struct bnxt_en_dev *edev,
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 4eafe6ec0abf..4f4914f5c84c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -15,8 +15,10 @@
 
 #define BNXT_MIN_ROCE_CP_RINGS	2
 #define BNXT_MIN_ROCE_STAT_CTXS	1
-#define BNXT_MAX_ROCE_MSIX	9
-#define BNXT_MAX_VF_ROCE_MSIX	2
+
+#define BNXT_MAX_ROCE_MSIX_VF		2
+#define BNXT_MAX_ROCE_MSIX_NPAR_PF	5
+#define BNXT_MAX_ROCE_MSIX		64
 
 struct hwrm_async_event_cmpl;
 struct bnxt;
-- 
2.30.1

[PATCH net-next 2/3] bnxt_en: Add MSIX check in bnxt_check_rings()

[Michael Chan]

bnxt_check_rings() is called to ensure that we have the hardware ring
resources before committing to reinitialize with the new number of
rings.  MSIX vectors are never checked at this point, because up
until recently we must first disable MSIX before we can allocate the
new set of MSIX vectors.

Now that we support dynamic MSIX allocation, check to make sure we
can dynamically allocate the new MSIX vectors as the last step in
bnxt_check_rings() if dynamic MSIX is supported.

For example, the IOMMU group may limit the number of MSIX vectors
for the device.  With this patch, the ring change will fail more
gracefully when there is not enough MSIX vectors.

It is also better to move bnxt_check_rings() to be called as the last
step when changing ethtool rings.

Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 19 ++++++++++++++++++-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 11 ++++++-----
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c9248ed9330c..6e422e24750a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -13803,6 +13803,7 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
 	int max_rx, max_tx, max_cp, tx_sets = 1, tx_cp;
 	struct bnxt_hw_rings hwr = {0};
 	int rx_rings = rx;
+	int rc;
 
 	if (tcs)
 		tx_sets = tcs;
@@ -13835,7 +13836,23 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
 	}
 	if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS)
 		hwr.cp_p5 = hwr.tx + rx;
-	return bnxt_hwrm_check_rings(bp, &hwr);
+	rc = bnxt_hwrm_check_rings(bp, &hwr);
+	if (!rc && pci_msix_can_alloc_dyn(bp->pdev)) {
+		if (!bnxt_ulp_registered(bp->edev)) {
+			hwr.cp += bnxt_get_ulp_msix_num(bp);
+			hwr.cp = min_t(int, hwr.cp, bnxt_get_max_func_irqs(bp));
+		}
+		if (hwr.cp > bp->total_irqs) {
+			int total_msix = bnxt_change_msix(bp, hwr.cp);
+
+			if (total_msix < hwr.cp) {
+				netdev_warn(bp->dev, "Unable to allocate %d MSIX vectors, maximum available %d\n",
+					    hwr.cp, total_msix);
+				rc = -ENOSPC;
+			}
+		}
+	}
+	return rc;
 }
 
 static void bnxt_unmap_bars(struct bnxt *bp, struct pci_dev *pdev)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 7392a716f28d..4aecc40be6eb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -955,11 +955,6 @@ static int bnxt_set_channels(struct net_device *dev,
 		}
 		tx_xdp = req_rx_rings;
 	}
-	rc = bnxt_check_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;
-	}
 
 	if (bnxt_get_nr_rss_ctxs(bp, req_rx_rings) !=
 	    bnxt_get_nr_rss_ctxs(bp, bp->rx_nr_rings) &&
@@ -968,6 +963,12 @@ static int bnxt_set_channels(struct net_device *dev,
 		return -EINVAL;
 	}
 
+	rc = bnxt_check_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;
+	}
+
 	if (netif_running(dev)) {
 		if (BNXT_PF(bp)) {
 			/* TODO CHIMP_FW: Send message to all VF's
-- 
2.30.1

[PATCH net-next 3/3] bnxt_en: resize bnxt_irq name field to fit format string

[Michael Chan]

From: Edwin Peer <edwin.peer@broadcom.com>

The name field of struct bnxt_irq is written using snprintf in
bnxt_setup_msix(). Make the field large enough to fit the maximal
formatted string to prevent truncation.  Truncated IRQ names are
less meaningful to the user.  For example, "enp4s0f0np0-TxRx-0"
gets truncated to "enp4s0f0np0-TxRx-" with the existing code.

Make sure we have space for the extra characters added to the IRQ
names:

  - the characters introduced by the static format string: hyphens
  - the maximal static substituted ring type string: "TxRx"
  - the maximum length of an integer formatted as a string, even
    though reasonable ring numbers would never be as long as this.

Signed-off-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 3b805ed433ed..69231e85140b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1217,12 +1217,15 @@ struct bnxt_napi {
 	bool			in_reset;
 };
 
+/* "TxRx", 2 hypens, plus maximum integer */
+#define BNXT_IRQ_NAME_EXTRA	17
+
 struct bnxt_irq {
 	irq_handler_t	handler;
 	unsigned int	vector;
 	u8		requested:1;
 	u8		have_cpumask:1;
-	char		name[IFNAMSIZ + 2];
+	char		name[IFNAMSIZ + BNXT_IRQ_NAME_EXTRA];
 	cpumask_var_t	cpu_mask;
 };
 
-- 
2.30.1

Re: [PATCH net-next 1/3] bnxt_en: Increase the number of MSIX vectors for RoCE device

[Simon Horman]

On Mon, Sep 09, 2024 at 01:27:35PM -0700, Michael Chan wrote:
> If RocE is supported on the device, set the number of RoCE MSIX vectors
> to the number of online CPUs + 1 and capped at these maximums:
> 
> VF: 2
> NPAR: 5
> PF: 64
> 
> For the PF, the maximum is now increased from the previous value
> of 9 to get better performance for kernel applications.
> 
> Remove the unnecessary check for BNXT_FLAG_ROCE_CAP.
> bnxt_set_dflt_ulp_msix() will only be called if the flag is set.
> 
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next 2/3] bnxt_en: Add MSIX check in bnxt_check_rings()

[Simon Horman]

On Mon, Sep 09, 2024 at 01:27:36PM -0700, Michael Chan wrote:
> bnxt_check_rings() is called to ensure that we have the hardware ring
> resources before committing to reinitialize with the new number of
> rings.  MSIX vectors are never checked at this point, because up
> until recently we must first disable MSIX before we can allocate the
> new set of MSIX vectors.
> 
> Now that we support dynamic MSIX allocation, check to make sure we
> can dynamically allocate the new MSIX vectors as the last step in
> bnxt_check_rings() if dynamic MSIX is supported.
> 
> For example, the IOMMU group may limit the number of MSIX vectors
> for the device.  With this patch, the ring change will fail more
> gracefully when there is not enough MSIX vectors.
> 
> It is also better to move bnxt_check_rings() to be called as the last
> step when changing ethtool rings.
> 
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next 3/3] bnxt_en: resize bnxt_irq name field to fit format string

[Simon Horman]

On Mon, Sep 09, 2024 at 01:27:37PM -0700, Michael Chan wrote:
> From: Edwin Peer <edwin.peer@broadcom.com>
> 
> The name field of struct bnxt_irq is written using snprintf in
> bnxt_setup_msix(). Make the field large enough to fit the maximal
> formatted string to prevent truncation.  Truncated IRQ names are
> less meaningful to the user.  For example, "enp4s0f0np0-TxRx-0"
> gets truncated to "enp4s0f0np0-TxRx-" with the existing code.
> 
> Make sure we have space for the extra characters added to the IRQ
> names:
> 
>   - the characters introduced by the static format string: hyphens
>   - the maximal static substituted ring type string: "TxRx"
>   - the maximum length of an integer formatted as a string, even
>     though reasonable ring numbers would never be as long as this.
> 
> Signed-off-by: Edwin Peer <edwin.peer@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next 0/3] bnxt_en: MSIX improvements

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  9 Sep 2024 13:27:34 -0700 you wrote:
> This patchset makes some improvements related to MSIX.  The first
> patch adjusts the default MSIX vectors assigned for RoCE.  On the
> PF, the number of MSIX is increased to 64 from the current 9.  The
> second patch allocates additional MSIX vectors ahead of time when
> changing ethtool channels if dynamic MSIX is supported.  The 3rd
> patch makes sure that the IRQ name is not truncated.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] bnxt_en: Increase the number of MSIX vectors for RoCE device
    https://git.kernel.org/netdev/net-next/c/f775cb1bbfd5
  - [net-next,2/3] bnxt_en: Add MSIX check in bnxt_check_rings()
    https://git.kernel.org/netdev/net-next/c/2d51eb0bd81c
  - [net-next,3/3] bnxt_en: resize bnxt_irq name field to fit format string
    https://git.kernel.org/netdev/net-next/c/f77cdee5db06

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html