netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/7] bnxt_en: Updates for net-next
@ 2024-04-09 21:54 Michael Chan
  2024-04-09 21:54 ` [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state Michael Chan
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

The first patch prevents a driver crash when RSS contexts are
configred in ifdown state.  Patches 2 to 6 are improvements for
managing MSIX for the aux device (for RoCE).  The existing
scheme statically carves out the MSIX vectors for RoCE even if
the RoCE driver is not loaded.  The new scheme adds flexibility
and allows the L2 driver to use the RoCE MSIX vectors if needed
when they are unused by the RoCE driver.  The last patch updates
the MODULE_DESCRIPTION().

Kalesh AP (1):
  bnxt_en: Remove a redundant NULL check in bnxt_register_dev()

Michael Chan (1):
  bnxt_en: Update MODULE_DESCRIPTION

Pavan Chebbi (1):
  bnxt_en: Skip ethtool RSS context configuration in ifdown state

Vikas Gupta (4):
  bnxt_en: Remove unneeded MSIX base structure fields and code
  bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions
  bnxt_en: Change MSIX/NQs allocation policy
  bnxt_en: Utilize ulp client resources if RoCE is not registered

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 103 +++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   1 +
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   5 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 147 ++++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  14 +-
 5 files changed, 186 insertions(+), 84 deletions(-)

-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:26   ` Jacob Keller
  2024-04-09 21:54 ` [PATCH net-next 2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev() Michael Chan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Kalesh AP, Somnath Kotur

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

From: Pavan Chebbi <pavan.chebbi@broadcom.com>

The current implementation requires the ifstate to be up when
configuring the RSS contexts.  It will try to fetch the RX ring
IDs and will crash if it is in ifdown state.  Return error if
!netif_running() to prevent the crash.

An improved implementation is in the works to allow RSS contexts
to be changed while in ifdown state.

Fixes: b3d0083caf9a ("bnxt_en: Support RSS contexts in ethtool .{get|set}_rxfh()")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 9c49f629d565..68444234b268 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1876,6 +1876,11 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
 		return -EOPNOTSUPP;
 	}
 
+	if (!netif_running(bp->dev)) {
+		NL_SET_ERR_MSG_MOD(extack, "Unable to set RSS contexts when interface is down");
+		return -EAGAIN;
+	}
+
 	if (*rss_context != ETH_RXFH_CONTEXT_ALLOC) {
 		rss_ctx = bnxt_get_rss_ctx_from_index(bp, *rss_context);
 		if (!rss_ctx) {
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev()
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
  2024-04-09 21:54 ` [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:35   ` Jacob Keller
  2024-04-09 21:54 ` [PATCH net-next 3/7] bnxt_en: Remove unneeded MSIX base structure fields and code Michael Chan
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Kalesh AP, Vikas Gupta, Somnath Kotur

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

The memory for "edev->ulp_tbl" is allocated inside the
bnxt_rdma_aux_device_init() function. If it fails, the driver
will not create the auxiliary device for RoCE. Hence the NULL
check inside bnxt_register_dev() is unnecessary.

Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 86dcd2c76587..fd890819d4bc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -64,9 +64,6 @@ int bnxt_register_dev(struct bnxt_en_dev *edev,
 		return -ENOMEM;
 
 	ulp = edev->ulp_tbl;
-	if (!ulp)
-		return -ENOMEM;
-
 	ulp->handle = handle;
 	rcu_assign_pointer(ulp->ulp_ops, ulp_ops);
 
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 3/7] bnxt_en: Remove unneeded MSIX base structure fields and code
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
  2024-04-09 21:54 ` [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state Michael Chan
  2024-04-09 21:54 ` [PATCH net-next 2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev() Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:36   ` Jacob Keller
  2024-04-09 21:54 ` [PATCH net-next 4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions Michael Chan
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta, Somnath Kotur

[-- Attachment #1: Type: text/plain, Size: 5010 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

Ever since commit:

303432211324 ("bnxt_en: Remove runtime interrupt vector allocation")

The MSIX base vector is effectively always 0.  Remove all unneeded
structure fields and code referencing the MSIX base.

Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 31 +++----------------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 20 +++---------
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  2 --
 3 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 795f3f957eb5..8213a37cba3e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3905,13 +3905,12 @@ static int bnxt_alloc_cp_sub_ring(struct bnxt *bp,
 static int bnxt_alloc_cp_rings(struct bnxt *bp)
 {
 	bool sh = !!(bp->flags & BNXT_FLAG_SHARED_RINGS);
-	int i, j, rc, ulp_base_vec, ulp_msix;
+	int i, j, rc, ulp_msix;
 	int tcs = bp->num_tc;
 
 	if (!tcs)
 		tcs = 1;
 	ulp_msix = bnxt_get_ulp_msix_num(bp);
-	ulp_base_vec = bnxt_get_ulp_msix_base(bp);
 	for (i = 0, j = 0; i < bp->cp_nr_rings; i++) {
 		struct bnxt_napi *bnapi = bp->bnapi[i];
 		struct bnxt_cp_ring_info *cpr, *cpr2;
@@ -3930,10 +3929,7 @@ static int bnxt_alloc_cp_rings(struct bnxt *bp)
 		if (rc)
 			return rc;
 
-		if (ulp_msix && i >= ulp_base_vec)
-			ring->map_idx = i + ulp_msix;
-		else
-			ring->map_idx = i;
+		ring->map_idx = ulp_msix + i;
 
 		if (!(bp->flags & BNXT_FLAG_CHIP_P5_PLUS))
 			continue;
@@ -7347,17 +7343,7 @@ static int bnxt_hwrm_reserve_rings(struct bnxt *bp, struct bnxt_hw_rings *hwr)
 
 int bnxt_nq_rings_in_use(struct bnxt *bp)
 {
-	int cp = bp->cp_nr_rings;
-	int ulp_msix, ulp_base;
-
-	ulp_msix = bnxt_get_ulp_msix_num(bp);
-	if (ulp_msix) {
-		ulp_base = bnxt_get_ulp_msix_base(bp);
-		cp += ulp_msix;
-		if ((ulp_base + ulp_msix) > cp)
-			cp = ulp_base + ulp_msix;
-	}
-	return cp;
+	return bp->cp_nr_rings + bnxt_get_ulp_msix_num(bp);
 }
 
 static int bnxt_cp_rings_in_use(struct bnxt *bp)
@@ -7373,16 +7359,7 @@ static int bnxt_cp_rings_in_use(struct bnxt *bp)
 
 static int bnxt_get_func_stat_ctxs(struct bnxt *bp)
 {
-	int ulp_stat = bnxt_get_ulp_stat_ctxs(bp);
-	int cp = bp->cp_nr_rings;
-
-	if (!ulp_stat)
-		return cp;
-
-	if (bnxt_nq_rings_in_use(bp) > cp + bnxt_get_ulp_msix_num(bp))
-		return bnxt_get_ulp_msix_base(bp) + ulp_stat;
-
-	return cp + ulp_stat;
+	return bp->cp_nr_rings + bnxt_get_ulp_stat_ctxs(bp);
 }
 
 static int bnxt_get_total_rss_ctxs(struct bnxt *bp, struct bnxt_hw_rings *hwr)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index fd890819d4bc..cae88747b110 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -31,21 +31,20 @@ static DEFINE_IDA(bnxt_aux_dev_ids);
 static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 {
 	struct bnxt_en_dev *edev = bp->edev;
-	int num_msix, idx, i;
+	int num_msix, i;
 
 	if (!edev->ulp_tbl->msix_requested) {
 		netdev_warn(bp->dev, "Requested MSI-X vectors insufficient\n");
 		return;
 	}
 	num_msix = edev->ulp_tbl->msix_requested;
-	idx = edev->ulp_tbl->msix_base;
 	for (i = 0; i < num_msix; i++) {
-		ent[i].vector = bp->irq_tbl[idx + i].vector;
-		ent[i].ring_idx = idx + i;
+		ent[i].vector = bp->irq_tbl[i].vector;
+		ent[i].ring_idx = i;
 		if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS)
 			ent[i].db_offset = bp->db_offset;
 		else
-			ent[i].db_offset = (idx + i) * 0x80;
+			ent[i].db_offset = i * 0x80;
 	}
 }
 
@@ -111,17 +110,6 @@ int bnxt_get_ulp_msix_num(struct bnxt *bp)
 		min_t(u32, roce_msix, num_online_cpus()) : 0);
 }
 
-int bnxt_get_ulp_msix_base(struct bnxt *bp)
-{
-	if (bnxt_ulp_registered(bp->edev)) {
-		struct bnxt_en_dev *edev = bp->edev;
-
-		if (edev->ulp_tbl->msix_requested)
-			return edev->ulp_tbl->msix_base;
-	}
-	return 0;
-}
-
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
 {
 	if (bnxt_ulp_registered(bp->edev)) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index b9e73de14b57..f8df4095399f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -46,7 +46,6 @@ struct bnxt_ulp {
 	unsigned long	*async_events_bmap;
 	u16		max_async_event_id;
 	u16		msix_requested;
-	u16		msix_base;
 	atomic_t	ref_count;
 };
 
@@ -96,7 +95,6 @@ static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
 }
 
 int bnxt_get_ulp_msix_num(struct bnxt *bp);
-int bnxt_get_ulp_msix_base(struct bnxt *bp);
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp);
 void bnxt_ulp_stop(struct bnxt *bp);
 void bnxt_ulp_start(struct bnxt *bp, int err);
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
                   ` (2 preceding siblings ...)
  2024-04-09 21:54 ` [PATCH net-next 3/7] bnxt_en: Remove unneeded MSIX base structure fields and code Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:37   ` Jacob Keller
  2024-04-09 21:54 ` [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy Michael Chan
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

In its current form, bnxt_rdma_aux_device_init() not only initializes
the necessary data structures of the newly created aux device but also
adds the aux device into the aux bus subsytem. Refactor the logic into
separate functions, first function to initialize the aux device along
with the required resources and second, to actually add the device to
the aux bus subsytem.
This separation helps to create bnxt_en_dev much earlier and save its
resources separately.

Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 10 ++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 33 ++++++++++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  2 ++
 3 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8213a37cba3e..8c24e83ba050 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -14786,10 +14786,13 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 	if (BNXT_PF(bp))
 		bnxt_sriov_disable(bp);
 
-	bnxt_rdma_aux_device_uninit(bp);
+	bnxt_rdma_aux_device_del(bp);
 
 	bnxt_ptp_clear(bp);
 	unregister_netdev(dev);
+
+	bnxt_rdma_aux_device_uninit(bp);
+
 	bnxt_free_l2_filters(bp, true);
 	bnxt_free_ntp_fltrs(bp, true);
 	if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp))
@@ -15408,13 +15411,15 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (BNXT_SUPPORTS_NTUPLE_VNIC(bp))
 		bnxt_init_multi_rss_ctx(bp);
 
+	bnxt_rdma_aux_device_init(bp);
+
 	rc = register_netdev(dev);
 	if (rc)
 		goto init_err_cleanup;
 
 	bnxt_dl_fw_reporters_create(bp);
 
-	bnxt_rdma_aux_device_init(bp);
+	bnxt_rdma_aux_device_add(bp);
 
 	bnxt_print_device_info(bp);
 
@@ -15422,6 +15427,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 init_err_cleanup:
+	bnxt_rdma_aux_device_uninit(bp);
 	bnxt_dl_unregister(bp);
 init_err_dl:
 	bnxt_shutdown_tc(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index cae88747b110..ae2b367b1226 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -291,7 +291,6 @@ void bnxt_rdma_aux_device_uninit(struct bnxt *bp)
 
 	aux_priv = bp->aux_priv;
 	adev = &aux_priv->aux_dev;
-	auxiliary_device_delete(adev);
 	auxiliary_device_uninit(adev);
 }
 
@@ -309,6 +308,14 @@ static void bnxt_aux_dev_release(struct device *dev)
 	bp->aux_priv = NULL;
 }
 
+void bnxt_rdma_aux_device_del(struct bnxt *bp)
+{
+	if (!bp->edev)
+		return;
+
+	auxiliary_device_delete(&bp->aux_priv->aux_dev);
+}
+
 static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 {
 	edev->net = bp->dev;
@@ -332,6 +339,23 @@ static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 	edev->ulp_tbl->msix_requested = bnxt_get_ulp_msix_num(bp);
 }
 
+void bnxt_rdma_aux_device_add(struct bnxt *bp)
+{
+	struct auxiliary_device *aux_dev;
+	int rc;
+
+	if (!bp->edev)
+		return;
+
+	aux_dev = &bp->aux_priv->aux_dev;
+	rc = auxiliary_device_add(aux_dev);
+	if (rc) {
+		netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
+		auxiliary_device_uninit(aux_dev);
+		bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+	}
+}
+
 void bnxt_rdma_aux_device_init(struct bnxt *bp)
 {
 	struct auxiliary_device *aux_dev;
@@ -386,13 +410,6 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 	bp->edev = edev;
 	bnxt_set_edev_info(edev, bp);
 
-	rc = auxiliary_device_add(aux_dev);
-	if (rc) {
-		netdev_warn(bp->dev,
-			    "Failed to add auxiliary device for ROCE\n");
-		goto aux_dev_uninit;
-	}
-
 	return;
 
 aux_dev_uninit:
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index f8df4095399f..ae7266ddf167 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -103,6 +103,8 @@ void bnxt_ulp_irq_stop(struct bnxt *bp);
 void bnxt_ulp_irq_restart(struct bnxt *bp, int err);
 void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl);
 void bnxt_rdma_aux_device_uninit(struct bnxt *bp);
+void bnxt_rdma_aux_device_del(struct bnxt *bp);
+void bnxt_rdma_aux_device_add(struct bnxt *bp);
 void bnxt_rdma_aux_device_init(struct bnxt *bp);
 int bnxt_register_dev(struct bnxt_en_dev *edev, struct bnxt_ulp_ops *ulp_ops,
 		      void *handle);
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
                   ` (3 preceding siblings ...)
  2024-04-09 21:54 ` [PATCH net-next 4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:40   ` Jacob Keller
  2024-04-09 21:54 ` [PATCH net-next 6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered Michael Chan
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta

[-- Attachment #1: Type: text/plain, Size: 8939 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

The existing scheme sets aside a number of MSIX/NQs for the RoCE
driver whether the RoCE driver is registered or not.  This scheme
is not flexible and limits the resources available for the L2 rings
if RoCE is never used.

Modify the scheme so that the RoCE MSIX/NQs can be used by the L2
driver if they are not used for RoCE.  The MSIX/NQs are now
represented by 3 fields.  bp->ulp_num_msix_want contains the
desired default value, edev->ulp_num_msix_vec contains the
available value (but not necessarily in use), and
ulp_tbl->msix_requested contains the actual value in use by RoCE.

The L2 driver can dip into edev->ulp_num_msix_vec if necessary.

We need to add rtnl_lock() back in bnxt_register_dev() and
bnxt_unregister_dev() to synchronize the MSIX usage between L2 and
RoCE.

Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 25 +++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 77 +++++++++++++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  8 +-
 4 files changed, 92 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8c24e83ba050..88cf8f47e071 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7549,6 +7549,20 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 	if (!netif_is_rxfh_configured(bp->dev))
 		bnxt_set_dflt_rss_indir_tbl(bp, NULL);
 
+	if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
+		int resv_msix, resv_ctx, ulp_msix, ulp_ctxs;
+		struct bnxt_hw_resc *hw_resc;
+
+		hw_resc = &bp->hw_resc;
+		resv_msix = hw_resc->resv_irqs - bp->cp_nr_rings;
+		ulp_msix = bnxt_get_ulp_msix_num(bp);
+		ulp_msix = min_t(int, resv_msix, ulp_msix);
+		bnxt_set_ulp_msix_num(bp, ulp_msix);
+		resv_ctx = hw_resc->resv_stat_ctxs  - bp->cp_nr_rings;
+		ulp_ctxs = min(resv_ctx, bnxt_get_ulp_stat_ctxs(bp));
+		bnxt_set_ulp_stat_ctxs(bp, ulp_ctxs);
+	}
+
 	return rc;
 }
 
@@ -14982,6 +14996,7 @@ static void bnxt_trim_dflt_sh_rings(struct bnxt *bp)
 static int bnxt_set_dflt_rings(struct bnxt *bp, bool sh)
 {
 	int dflt_rings, max_rx_rings, max_tx_rings, rc;
+	int avail_msix;
 
 	if (!bnxt_can_reserve_rings(bp))
 		return 0;
@@ -15009,6 +15024,14 @@ static int bnxt_set_dflt_rings(struct bnxt *bp, bool sh)
 		bp->cp_nr_rings = bp->tx_nr_rings_per_tc + bp->rx_nr_rings;
 	bp->tx_nr_rings = bp->tx_nr_rings_per_tc;
 
+	avail_msix = bnxt_get_max_func_irqs(bp) - bp->cp_nr_rings;
+	if (avail_msix >= BNXT_MIN_ROCE_CP_RINGS) {
+		int ulp_num_msix = min(avail_msix, bp->ulp_num_msix_want);
+
+		bnxt_set_ulp_msix_num(bp, ulp_num_msix);
+		bnxt_set_dflt_ulp_stat_ctxs(bp);
+	}
+
 	rc = __bnxt_reserve_rings(bp);
 	if (rc && rc != -ENODEV)
 		netdev_warn(bp->dev, "Unable to reserve tx rings\n");
@@ -15358,6 +15381,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	bnxt_set_rx_skb_mode(bp, false);
 	bnxt_set_tpa_flags(bp);
 	bnxt_set_ring_params(bp);
+	bnxt_rdma_aux_device_init(bp);
 	rc = bnxt_set_dflt_rings(bp, true);
 	if (rc) {
 		if (BNXT_VF(bp) && rc == -ENODEV) {
@@ -15411,7 +15435,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (BNXT_SUPPORTS_NTUPLE_VNIC(bp))
 		bnxt_init_multi_rss_ctx(bp);
 
-	bnxt_rdma_aux_device_init(bp);
 
 	rc = register_netdev(dev);
 	if (rc)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 0640fcb57ef8..ad57ef051798 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2303,6 +2303,7 @@ struct bnxt {
 
 	struct bnxt_irq	*irq_tbl;
 	int			total_irqs;
+	int			ulp_num_msix_want;
 	u8			mac_addr[ETH_ALEN];
 
 #ifdef CONFIG_BNXT_DCB
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index ae2b367b1226..de2cb1d4cd98 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -48,6 +48,46 @@ static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 	}
 }
 
+int bnxt_get_ulp_msix_num(struct bnxt *bp)
+{
+	if (bp->edev)
+		return bp->edev->ulp_num_msix_vec;
+	return 0;
+}
+
+void bnxt_set_ulp_msix_num(struct bnxt *bp, int num)
+{
+	if (bp->edev)
+		bp->edev->ulp_num_msix_vec = num;
+}
+
+int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
+{
+	if (bp->edev)
+		return bp->edev->ulp_num_ctxs;
+	return 0;
+}
+
+void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ulp_ctx)
+{
+	if (bp->edev)
+		bp->edev->ulp_num_ctxs = num_ulp_ctx;
+}
+
+void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp)
+{
+	if (bp->edev) {
+		bp->edev->ulp_num_ctxs = BNXT_MIN_ROCE_STAT_CTXS;
+		/* Reserve one additional stat_ctx for PF0 (except
+		 * on 1-port NICs) as it also creates one stat_ctx
+		 * for PF1 in case of RoCE bonding.
+		 */
+		if (BNXT_PF(bp) && !bp->pf.port_id &&
+		    bp->port_count > 1)
+			bp->edev->ulp_num_ctxs++;
+	}
+}
+
 int bnxt_register_dev(struct bnxt_en_dev *edev,
 		      struct bnxt_ulp_ops *ulp_ops,
 		      void *handle)
@@ -56,11 +96,19 @@ int bnxt_register_dev(struct bnxt_en_dev *edev,
 	struct bnxt *bp = netdev_priv(dev);
 	unsigned int max_stat_ctxs;
 	struct bnxt_ulp *ulp;
+	int rc = 0;
 
+	rtnl_lock();
+	if (!bp->irq_tbl) {
+		rc = -ENODEV;
+		goto exit;
+	}
 	max_stat_ctxs = bnxt_get_max_func_stat_ctxs(bp);
 	if (max_stat_ctxs <= BNXT_MIN_ROCE_STAT_CTXS ||
-	    bp->cp_nr_rings == max_stat_ctxs)
-		return -ENOMEM;
+	    bp->cp_nr_rings == max_stat_ctxs) {
+		rc = -ENOMEM;
+		goto exit;
+	}
 
 	ulp = edev->ulp_tbl;
 	ulp->handle = handle;
@@ -69,9 +117,13 @@ int bnxt_register_dev(struct bnxt_en_dev *edev,
 	if (test_bit(BNXT_STATE_OPEN, &bp->state))
 		bnxt_hwrm_vnic_cfg(bp, &bp->vnic_info[BNXT_VNIC_DEFAULT]);
 
+	edev->ulp_tbl->msix_requested = bnxt_get_ulp_msix_num(bp);
+
 	bnxt_fill_msix_vecs(bp, bp->edev->msix_entries);
 	edev->flags |= BNXT_EN_FLAG_MSIX_REQUESTED;
-	return 0;
+exit:
+	rtnl_unlock();
+	return rc;
 }
 EXPORT_SYMBOL(bnxt_register_dev);
 
@@ -83,8 +135,10 @@ void bnxt_unregister_dev(struct bnxt_en_dev *edev)
 	int i = 0;
 
 	ulp = edev->ulp_tbl;
+	rtnl_lock();
 	if (ulp->msix_requested)
 		edev->flags &= ~BNXT_EN_FLAG_MSIX_REQUESTED;
+	edev->ulp_tbl->msix_requested = 0;
 
 	if (ulp->max_async_event_id)
 		bnxt_hwrm_func_drv_rgtr(bp, NULL, 0, true);
@@ -97,11 +151,12 @@ void bnxt_unregister_dev(struct bnxt_en_dev *edev)
 		msleep(100);
 		i++;
 	}
+	rtnl_unlock();
 	return;
 }
 EXPORT_SYMBOL(bnxt_unregister_dev);
 
-int bnxt_get_ulp_msix_num(struct bnxt *bp)
+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;
@@ -110,18 +165,6 @@ int bnxt_get_ulp_msix_num(struct bnxt *bp)
 		min_t(u32, roce_msix, num_online_cpus()) : 0);
 }
 
-int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
-{
-	if (bnxt_ulp_registered(bp->edev)) {
-		struct bnxt_en_dev *edev = bp->edev;
-
-		if (edev->ulp_tbl->msix_requested)
-			return BNXT_MIN_ROCE_STAT_CTXS;
-	}
-
-	return 0;
-}
-
 int bnxt_send_msg(struct bnxt_en_dev *edev,
 			 struct bnxt_fw_msg *fw_msg)
 {
@@ -336,7 +379,6 @@ static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 	edev->pf_port_id = bp->pf.port_id;
 	edev->en_state = bp->state;
 	edev->bar0 = bp->bar0;
-	edev->ulp_tbl->msix_requested = bnxt_get_ulp_msix_num(bp);
 }
 
 void bnxt_rdma_aux_device_add(struct bnxt *bp)
@@ -409,6 +451,7 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp)
 	aux_priv->edev = edev;
 	bp->edev = edev;
 	bnxt_set_edev_info(edev, bp);
+	bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
 
 	return;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index ae7266ddf167..04ce3328e66f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -85,17 +85,23 @@ struct bnxt_en_dev {
 							 * updated in resume.
 							 */
 	void __iomem                    *bar0;
+
+	u16				ulp_num_msix_vec;
+	u16				ulp_num_ctxs;
 };
 
 static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
 {
-	if (edev && edev->ulp_tbl)
+	if (edev && rcu_access_pointer(edev->ulp_tbl->ulp_ops))
 		return true;
 	return false;
 }
 
 int bnxt_get_ulp_msix_num(struct bnxt *bp);
+void bnxt_set_ulp_msix_num(struct bnxt *bp, int num);
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp);
+void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ctxs);
+void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp);
 void bnxt_ulp_stop(struct bnxt *bp);
 void bnxt_ulp_start(struct bnxt *bp, int err);
 void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs);
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
                   ` (4 preceding siblings ...)
  2024-04-09 21:54 ` [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:41   ` Jacob Keller
  2024-04-09 21:54 ` [PATCH net-next 7/7] bnxt_en: Update MODULE_DESCRIPTION Michael Chan
  2024-04-11  3:10 ` [PATCH net-next 0/7] bnxt_en: Updates for net-next patchwork-bot+netdevbpf
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta

[-- Attachment #1: Type: text/plain, Size: 5696 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

If the RoCE driver is not registered for a RoCE capable device, add
flexibility to use the RoCE resources (MSIX/NQs) for L2 purposes,
such as additional rings configured by the user or for XDP.

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

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 88cf8f47e071..a2e21fe64ab9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7470,14 +7470,27 @@ static bool bnxt_rings_ok(struct bnxt *bp, struct bnxt_hw_rings *hwr)
 static int __bnxt_reserve_rings(struct bnxt *bp)
 {
 	struct bnxt_hw_rings hwr = {0};
+	int cp = bp->cp_nr_rings;
 	int rx_rings, rc;
+	int ulp_msix = 0;
 	bool sh = false;
 	int tx_cp;
 
 	if (!bnxt_need_reserve_rings(bp))
 		return 0;
 
-	hwr.cp = bnxt_nq_rings_in_use(bp);
+	if (!bnxt_ulp_registered(bp->edev)) {
+		ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
+		if (!ulp_msix)
+			bnxt_set_ulp_stat_ctxs(bp, 0);
+
+		if (ulp_msix > bp->ulp_num_msix_want)
+			ulp_msix = bp->ulp_num_msix_want;
+		hwr.cp = cp + ulp_msix;
+	} else {
+		hwr.cp = bnxt_nq_rings_in_use(bp);
+	}
+
 	hwr.tx = bp->tx_nr_rings;
 	hwr.rx = bp->rx_nr_rings;
 	if (bp->flags & BNXT_FLAG_SHARED_RINGS)
@@ -7550,12 +7563,11 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 		bnxt_set_dflt_rss_indir_tbl(bp, NULL);
 
 	if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
-		int resv_msix, resv_ctx, ulp_msix, ulp_ctxs;
+		int resv_msix, resv_ctx, ulp_ctxs;
 		struct bnxt_hw_resc *hw_resc;
 
 		hw_resc = &bp->hw_resc;
 		resv_msix = hw_resc->resv_irqs - bp->cp_nr_rings;
-		ulp_msix = bnxt_get_ulp_msix_num(bp);
 		ulp_msix = min_t(int, resv_msix, ulp_msix);
 		bnxt_set_ulp_msix_num(bp, ulp_msix);
 		resv_ctx = hw_resc->resv_stat_ctxs  - bp->cp_nr_rings;
@@ -10609,13 +10621,23 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
 {
 	bool irq_cleared = false;
 	int tcs = bp->num_tc;
+	int irqs_required;
 	int rc;
 
 	if (!bnxt_need_reserve_rings(bp))
 		return 0;
 
-	if (irq_re_init && BNXT_NEW_RM(bp) &&
-	    bnxt_get_num_msix(bp) != bp->total_irqs) {
+	if (!bnxt_ulp_registered(bp->edev)) {
+		int ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
+
+		if (ulp_msix > bp->ulp_num_msix_want)
+			ulp_msix = bp->ulp_num_msix_want;
+		irqs_required = ulp_msix + bp->cp_nr_rings;
+	} else {
+		irqs_required = bnxt_get_num_msix(bp);
+	}
+
+	if (irq_re_init && BNXT_NEW_RM(bp) && irqs_required != bp->total_irqs) {
 		bnxt_ulp_irq_stop(bp);
 		bnxt_clear_int_mode(bp);
 		irq_cleared = true;
@@ -13625,8 +13647,8 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
 		return -ENOMEM;
 	hwr.stat = hwr.cp;
 	if (BNXT_NEW_RM(bp)) {
-		hwr.cp += bnxt_get_ulp_msix_num(bp);
-		hwr.stat += bnxt_get_ulp_stat_ctxs(bp);
+		hwr.cp += bnxt_get_ulp_msix_num_in_use(bp);
+		hwr.stat += bnxt_get_ulp_stat_ctxs_in_use(bp);
 		hwr.grp = rx;
 		hwr.rss_ctx = bnxt_get_total_rss_ctxs(bp, &hwr);
 	}
@@ -14899,8 +14921,9 @@ static void _bnxt_get_max_rings(struct bnxt *bp, int *max_rx, int *max_tx,
 	*max_rx = hw_resc->max_rx_rings;
 	*max_cp = bnxt_get_max_func_cp_rings_for_en(bp);
 	max_irq = min_t(int, bnxt_get_max_func_irqs(bp) -
-			bnxt_get_ulp_msix_num(bp),
-			hw_resc->max_stat_ctxs - bnxt_get_ulp_stat_ctxs(bp));
+			bnxt_get_ulp_msix_num_in_use(bp),
+			hw_resc->max_stat_ctxs -
+			bnxt_get_ulp_stat_ctxs_in_use(bp));
 	if (!(bp->flags & BNXT_FLAG_CHIP_P5_PLUS))
 		*max_cp = min_t(int, *max_cp, max_irq);
 	max_ring_grps = hw_resc->max_hw_ring_grps;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index de2cb1d4cd98..edb10aebd095 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -61,6 +61,13 @@ void bnxt_set_ulp_msix_num(struct bnxt *bp, int num)
 		bp->edev->ulp_num_msix_vec = num;
 }
 
+int bnxt_get_ulp_msix_num_in_use(struct bnxt *bp)
+{
+	if (bnxt_ulp_registered(bp->edev))
+		return bp->edev->ulp_num_msix_vec;
+	return 0;
+}
+
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
 {
 	if (bp->edev)
@@ -74,6 +81,13 @@ void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ulp_ctx)
 		bp->edev->ulp_num_ctxs = num_ulp_ctx;
 }
 
+int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp)
+{
+	if (bnxt_ulp_registered(bp->edev))
+		return bp->edev->ulp_num_ctxs;
+	return 0;
+}
+
 void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp)
 {
 	if (bp->edev) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
index 04ce3328e66f..b86baf901a5d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -98,9 +98,11 @@ static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
 }
 
 int bnxt_get_ulp_msix_num(struct bnxt *bp);
+int bnxt_get_ulp_msix_num_in_use(struct bnxt *bp);
 void bnxt_set_ulp_msix_num(struct bnxt *bp, int num);
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp);
 void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ctxs);
+int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp);
 void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp);
 void bnxt_ulp_stop(struct bnxt *bp);
 void bnxt_ulp_start(struct bnxt *bp, int err);
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* [PATCH net-next 7/7] bnxt_en: Update MODULE_DESCRIPTION
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
                   ` (5 preceding siblings ...)
  2024-04-09 21:54 ` [PATCH net-next 6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered Michael Chan
@ 2024-04-09 21:54 ` Michael Chan
  2024-04-09 23:42   ` Jacob Keller
  2024-04-11  3:10 ` [PATCH net-next 0/7] bnxt_en: Updates for net-next patchwork-bot+netdevbpf
  7 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 21:54 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

Update MODULE_DESCRIPTION to the more generic adapter family name.
The old name only includes the first generation of supported
adapters.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index a2e21fe64ab9..d728df139c9c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -76,7 +76,7 @@
 				 NETIF_MSG_TX_ERR)
 
 MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Broadcom BCM573xx network driver");
+MODULE_DESCRIPTION("Broadcom NetXtreme network driver");
 
 #define BNXT_RX_OFFSET (NET_SKB_PAD + NET_IP_ALIGN)
 #define BNXT_RX_DMA_OFFSET NET_SKB_PAD
-- 
2.30.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state
  2024-04-09 21:54 ` [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state Michael Chan
@ 2024-04-09 23:26   ` Jacob Keller
  2024-04-09 23:51     ` Michael Chan
  0 siblings, 1 reply; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:26 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Kalesh AP, Somnath Kotur



On 4/9/2024 2:54 PM, Michael Chan wrote:
> From: Pavan Chebbi <pavan.chebbi@broadcom.com>
> 
> The current implementation requires the ifstate to be up when
> configuring the RSS contexts.  It will try to fetch the RX ring
> IDs and will crash if it is in ifdown state.  Return error if
> !netif_running() to prevent the crash.
> 
> An improved implementation is in the works to allow RSS contexts
> to be changed while in ifdown state.
> 
> Fixes: b3d0083caf9a ("bnxt_en: Support RSS contexts in ethtool .{get|set}_rxfh()")
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---

Makes sense, though I think you could send this fix directly to net as
its clearly a bug fix and preventing a crash is good.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks,
Jake

>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 9c49f629d565..68444234b268 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -1876,6 +1876,11 @@ static int bnxt_set_rxfh_context(struct bnxt *bp,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	if (!netif_running(bp->dev)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Unable to set RSS contexts when interface is down");
> +		return -EAGAIN;
> +	}
> +
>  	if (*rss_context != ETH_RXFH_CONTEXT_ALLOC) {
>  		rss_ctx = bnxt_get_rss_ctx_from_index(bp, *rss_context);
>  		if (!rss_ctx) {

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

* Re: [PATCH net-next 2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev()
  2024-04-09 21:54 ` [PATCH net-next 2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev() Michael Chan
@ 2024-04-09 23:35   ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:35 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Kalesh AP, Vikas Gupta, Somnath Kotur



On 4/9/2024 2:54 PM, Michael Chan wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> The memory for "edev->ulp_tbl" is allocated inside the
> bnxt_rdma_aux_device_init() function. If it fails, the driver
> will not create the auxiliary device for RoCE. Hence the NULL
> check inside bnxt_register_dev() is unnecessary.
> 
> Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> index 86dcd2c76587..fd890819d4bc 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> @@ -64,9 +64,6 @@ int bnxt_register_dev(struct bnxt_en_dev *edev,
>  		return -ENOMEM;
>  
>  	ulp = edev->ulp_tbl;
> -	if (!ulp)
> -		return -ENOMEM;
> -
>  	ulp->handle = handle;
>  	rcu_assign_pointer(ulp->ulp_ops, ulp_ops);
>  

Ok, so ulp_tbl is setup by bnxt_rdma_aux_device_init, and you're
claiming that the only way bnxt_register_dev gets called is if the
auxiliary driver loads?

But bnxt_register_dev is a simple direct-exported function, so the bnxt
driver itself doesn't enforce this. However, if we go look at the
infiniband driver that does call bnxt_register_dev, it only calls this
as part of its auxiliary device probe.

This does make the NULL check here seem redundant, though I think its a
lot more layered than a normal removal of a NULL check since this
crosses module boundary and its a bit subtle to follow the full flow of
the auxiliary drivers.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

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

* Re: [PATCH net-next 3/7] bnxt_en: Remove unneeded MSIX base structure fields and code
  2024-04-09 21:54 ` [PATCH net-next 3/7] bnxt_en: Remove unneeded MSIX base structure fields and code Michael Chan
@ 2024-04-09 23:36   ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:36 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta, Somnath Kotur



On 4/9/2024 2:54 PM, Michael Chan wrote:
> From: Vikas Gupta <vikas.gupta@broadcom.com>
> 
> Ever since commit:
> 
> 303432211324 ("bnxt_en: Remove runtime interrupt vector allocation")
> 
> The MSIX base vector is effectively always 0.  Remove all unneeded
> structure fields and code referencing the MSIX base.
> 
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---

Nice simplification.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

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

* Re: [PATCH net-next 4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions
  2024-04-09 21:54 ` [PATCH net-next 4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions Michael Chan
@ 2024-04-09 23:37   ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:37 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta



On 4/9/2024 2:54 PM, Michael Chan wrote:
> From: Vikas Gupta <vikas.gupta@broadcom.com>
> 
> In its current form, bnxt_rdma_aux_device_init() not only initializes
> the necessary data structures of the newly created aux device but also
> adds the aux device into the aux bus subsytem. Refactor the logic into
> separate functions, first function to initialize the aux device along
> with the required resources and second, to actually add the device to
> the aux bus subsytem.
> This separation helps to create bnxt_en_dev much earlier and save its
> resources separately.
> 
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

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

* Re: [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy
  2024-04-09 21:54 ` [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy Michael Chan
@ 2024-04-09 23:40   ` Jacob Keller
  2024-04-09 23:48     ` Michael Chan
  0 siblings, 1 reply; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:40 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta



On 4/9/2024 2:54 PM, Michael Chan wrote:
> From: Vikas Gupta <vikas.gupta@broadcom.com>
> 
> The existing scheme sets aside a number of MSIX/NQs for the RoCE
> driver whether the RoCE driver is registered or not.  This scheme
> is not flexible and limits the resources available for the L2 rings
> if RoCE is never used.
> 
> Modify the scheme so that the RoCE MSIX/NQs can be used by the L2
> driver if they are not used for RoCE.  The MSIX/NQs are now
> represented by 3 fields.  bp->ulp_num_msix_want contains the
> desired default value, edev->ulp_num_msix_vec contains the
> available value (but not necessarily in use), and
> ulp_tbl->msix_requested contains the actual value in use by RoCE.
> 
> The L2 driver can dip into edev->ulp_num_msix_vec if necessary.
> 
> We need to add rtnl_lock() back in bnxt_register_dev() and
> bnxt_unregister_dev() to synchronize the MSIX usage between L2 and
> RoCE.
> 
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---

Whats the behavior if the L2 driver dips into this pool and then RoCE is
enabled later?

I guess RoCE would fail to get the resources it needs, but then system
administrator could re-configure the L2 device to use fewer resources?

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks,
Jake

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

* Re: [PATCH net-next 6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered
  2024-04-09 21:54 ` [PATCH net-next 6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered Michael Chan
@ 2024-04-09 23:41   ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:41 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek,
	Vikas Gupta



On 4/9/2024 2:54 PM, Michael Chan wrote:
> From: Vikas Gupta <vikas.gupta@broadcom.com>
> 
> If the RoCE driver is not registered for a RoCE capable device, add
> flexibility to use the RoCE resources (MSIX/NQs) for L2 purposes,
> such as additional rings configured by the user or for XDP.
> 
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 41 +++++++++++++++----
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 14 +++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  2 +
>  3 files changed, 48 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 88cf8f47e071..a2e21fe64ab9 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -7470,14 +7470,27 @@ static bool bnxt_rings_ok(struct bnxt *bp, struct bnxt_hw_rings *hwr)
>  static int __bnxt_reserve_rings(struct bnxt *bp)
>  {
>  	struct bnxt_hw_rings hwr = {0};
> +	int cp = bp->cp_nr_rings;
>  	int rx_rings, rc;
> +	int ulp_msix = 0;
>  	bool sh = false;
>  	int tx_cp;
>  
>  	if (!bnxt_need_reserve_rings(bp))
>  		return 0;
>  
> -	hwr.cp = bnxt_nq_rings_in_use(bp);
> +	if (!bnxt_ulp_registered(bp->edev)) {
> +		ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
> +		if (!ulp_msix)
> +			bnxt_set_ulp_stat_ctxs(bp, 0);
> +
> +		if (ulp_msix > bp->ulp_num_msix_want)
> +			ulp_msix = bp->ulp_num_msix_want;
> +		hwr.cp = cp + ulp_msix;
> +	} else {
> +		hwr.cp = bnxt_nq_rings_in_use(bp);
> +	}
> +
>  	hwr.tx = bp->tx_nr_rings;
>  	hwr.rx = bp->rx_nr_rings;
>  	if (bp->flags & BNXT_FLAG_SHARED_RINGS)
> @@ -7550,12 +7563,11 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
>  		bnxt_set_dflt_rss_indir_tbl(bp, NULL);
>  
>  	if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
> -		int resv_msix, resv_ctx, ulp_msix, ulp_ctxs;
> +		int resv_msix, resv_ctx, ulp_ctxs;
>  		struct bnxt_hw_resc *hw_resc;
>  
>  		hw_resc = &bp->hw_resc;
>  		resv_msix = hw_resc->resv_irqs - bp->cp_nr_rings;
> -		ulp_msix = bnxt_get_ulp_msix_num(bp);
>  		ulp_msix = min_t(int, resv_msix, ulp_msix);
>  		bnxt_set_ulp_msix_num(bp, ulp_msix);
>  		resv_ctx = hw_resc->resv_stat_ctxs  - bp->cp_nr_rings;
> @@ -10609,13 +10621,23 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
>  {
>  	bool irq_cleared = false;
>  	int tcs = bp->num_tc;
> +	int irqs_required;
>  	int rc;
>  
>  	if (!bnxt_need_reserve_rings(bp))
>  		return 0;
>  
> -	if (irq_re_init && BNXT_NEW_RM(bp) &&
> -	    bnxt_get_num_msix(bp) != bp->total_irqs) {
> +	if (!bnxt_ulp_registered(bp->edev)) {
> +		int ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
> +
> +		if (ulp_msix > bp->ulp_num_msix_want)
> +			ulp_msix = bp->ulp_num_msix_want;
> +		irqs_required = ulp_msix + bp->cp_nr_rings;
> +	} else {
> +		irqs_required = bnxt_get_num_msix(bp);
> +	}
> +
> +	if (irq_re_init && BNXT_NEW_RM(bp) && irqs_required != bp->total_irqs) {
>  		bnxt_ulp_irq_stop(bp);
>  		bnxt_clear_int_mode(bp);
>  		irq_cleared = true;
> @@ -13625,8 +13647,8 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
>  		return -ENOMEM;
>  	hwr.stat = hwr.cp;
>  	if (BNXT_NEW_RM(bp)) {
> -		hwr.cp += bnxt_get_ulp_msix_num(bp);
> -		hwr.stat += bnxt_get_ulp_stat_ctxs(bp);
> +		hwr.cp += bnxt_get_ulp_msix_num_in_use(bp);
> +		hwr.stat += bnxt_get_ulp_stat_ctxs_in_use(bp);
>  		hwr.grp = rx;
>  		hwr.rss_ctx = bnxt_get_total_rss_ctxs(bp, &hwr);
>  	}
> @@ -14899,8 +14921,9 @@ static void _bnxt_get_max_rings(struct bnxt *bp, int *max_rx, int *max_tx,
>  	*max_rx = hw_resc->max_rx_rings;
>  	*max_cp = bnxt_get_max_func_cp_rings_for_en(bp);
>  	max_irq = min_t(int, bnxt_get_max_func_irqs(bp) -
> -			bnxt_get_ulp_msix_num(bp),
> -			hw_resc->max_stat_ctxs - bnxt_get_ulp_stat_ctxs(bp));
> +			bnxt_get_ulp_msix_num_in_use(bp),
> +			hw_resc->max_stat_ctxs -
> +			bnxt_get_ulp_stat_ctxs_in_use(bp));
>  	if (!(bp->flags & BNXT_FLAG_CHIP_P5_PLUS))
>  		*max_cp = min_t(int, *max_cp, max_irq);
>  	max_ring_grps = hw_resc->max_hw_ring_grps;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> index de2cb1d4cd98..edb10aebd095 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> @@ -61,6 +61,13 @@ void bnxt_set_ulp_msix_num(struct bnxt *bp, int num)
>  		bp->edev->ulp_num_msix_vec = num;
>  }
>  
> +int bnxt_get_ulp_msix_num_in_use(struct bnxt *bp)
> +{
> +	if (bnxt_ulp_registered(bp->edev))
> +		return bp->edev->ulp_num_msix_vec;
> +	return 0;
> +}
> +
>  int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
>  {
>  	if (bp->edev)
> @@ -74,6 +81,13 @@ void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ulp_ctx)
>  		bp->edev->ulp_num_ctxs = num_ulp_ctx;
>  }
>  
> +int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp)
> +{
> +	if (bnxt_ulp_registered(bp->edev))
> +		return bp->edev->ulp_num_ctxs;
> +	return 0;
> +}
> +
>  void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp)
>  {
>  	if (bp->edev) {
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
> index 04ce3328e66f..b86baf901a5d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
> @@ -98,9 +98,11 @@ static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev)
>  }
>  
>  int bnxt_get_ulp_msix_num(struct bnxt *bp);
> +int bnxt_get_ulp_msix_num_in_use(struct bnxt *bp);
>  void bnxt_set_ulp_msix_num(struct bnxt *bp, int num);
>  int bnxt_get_ulp_stat_ctxs(struct bnxt *bp);
>  void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ctxs);
> +int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp);
>  void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp);
>  void bnxt_ulp_stop(struct bnxt *bp);
>  void bnxt_ulp_start(struct bnxt *bp, int err);

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

* Re: [PATCH net-next 7/7] bnxt_en: Update MODULE_DESCRIPTION
  2024-04-09 21:54 ` [PATCH net-next 7/7] bnxt_en: Update MODULE_DESCRIPTION Michael Chan
@ 2024-04-09 23:42   ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-09 23:42 UTC (permalink / raw)
  To: Michael Chan, davem
  Cc: netdev, edumazet, kuba, pabeni, pavan.chebbi, andrew.gospodarek



On 4/9/2024 2:54 PM, Michael Chan wrote:
> Update MODULE_DESCRIPTION to the more generic adapter family name.
> The old name only includes the first generation of supported
> adapters.
> 
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index a2e21fe64ab9..d728df139c9c 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -76,7 +76,7 @@
>  				 NETIF_MSG_TX_ERR)
>  
>  MODULE_LICENSE("GPL");
> -MODULE_DESCRIPTION("Broadcom BCM573xx network driver");
> +MODULE_DESCRIPTION("Broadcom NetXtreme network driver");
>  
>  #define BNXT_RX_OFFSET (NET_SKB_PAD + NET_IP_ALIGN)
>  #define BNXT_RX_DMA_OFFSET NET_SKB_PAD

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

* Re: [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy
  2024-04-09 23:40   ` Jacob Keller
@ 2024-04-09 23:48     ` Michael Chan
  2024-04-10 19:28       ` Jacob Keller
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 23:48 UTC (permalink / raw)
  To: Jacob Keller
  Cc: davem, netdev, edumazet, kuba, pabeni, pavan.chebbi,
	andrew.gospodarek, Vikas Gupta

[-- Attachment #1: Type: text/plain, Size: 2002 bytes --]

On Tue, Apr 9, 2024 at 4:40 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>
>
>
> On 4/9/2024 2:54 PM, Michael Chan wrote:
> > From: Vikas Gupta <vikas.gupta@broadcom.com>
> >
> > The existing scheme sets aside a number of MSIX/NQs for the RoCE
> > driver whether the RoCE driver is registered or not.  This scheme
> > is not flexible and limits the resources available for the L2 rings
> > if RoCE is never used.
> >
> > Modify the scheme so that the RoCE MSIX/NQs can be used by the L2
> > driver if they are not used for RoCE.  The MSIX/NQs are now
> > represented by 3 fields.  bp->ulp_num_msix_want contains the
> > desired default value, edev->ulp_num_msix_vec contains the
> > available value (but not necessarily in use), and
> > ulp_tbl->msix_requested contains the actual value in use by RoCE.
> >
> > The L2 driver can dip into edev->ulp_num_msix_vec if necessary.
> >
> > We need to add rtnl_lock() back in bnxt_register_dev() and
> > bnxt_unregister_dev() to synchronize the MSIX usage between L2 and
> > RoCE.
> >
> > Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> > Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> > Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> > ---
>
> Whats the behavior if the L2 driver dips into this pool and then RoCE is
> enabled later?

Thanks for the review.  If the user increases the L2 rings or enables
XDP which will cause the driver to allocate a new set of XDP TX rings,
it can now use the RoCE MSIX if needed and if they are not in use.

>
> I guess RoCE would fail to get the resources it needs, but then system
> administrator could re-configure the L2 device to use fewer resources?

If the above simply reduces the RoCE MSIX, the RoCE driver can still
operate with fewer MSIX.  If the above has reduced the MSIX below the
minimum required for RoCE, then RoCE will fail to initialize.  At that
point, the user can reduce the L2 rings and reload the RoCE driver.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state
  2024-04-09 23:26   ` Jacob Keller
@ 2024-04-09 23:51     ` Michael Chan
  2024-04-10 19:29       ` Jacob Keller
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Chan @ 2024-04-09 23:51 UTC (permalink / raw)
  To: Jacob Keller
  Cc: davem, netdev, edumazet, kuba, pabeni, pavan.chebbi,
	andrew.gospodarek, Kalesh AP, Somnath Kotur

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Tue, Apr 9, 2024 at 4:26 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>
>
>
> On 4/9/2024 2:54 PM, Michael Chan wrote:
> > From: Pavan Chebbi <pavan.chebbi@broadcom.com>
> >
> > The current implementation requires the ifstate to be up when
> > configuring the RSS contexts.  It will try to fetch the RX ring
> > IDs and will crash if it is in ifdown state.  Return error if
> > !netif_running() to prevent the crash.
> >
> > An improved implementation is in the works to allow RSS contexts
> > to be changed while in ifdown state.
> >
> > Fixes: b3d0083caf9a ("bnxt_en: Support RSS contexts in ethtool .{get|set}_rxfh()")
> > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> > Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> > Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> > ---
>
> Makes sense, though I think you could send this fix directly to net as
> its clearly a bug fix and preventing a crash is good.

This RSS context feature in the driver is new and is only in net-next,
so the patch only applies to net-next.  Thanks.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy
  2024-04-09 23:48     ` Michael Chan
@ 2024-04-10 19:28       ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-10 19:28 UTC (permalink / raw)
  To: Michael Chan
  Cc: davem, netdev, edumazet, kuba, pabeni, pavan.chebbi,
	andrew.gospodarek, Vikas Gupta



On 4/9/2024 4:48 PM, Michael Chan wrote:
> On Tue, Apr 9, 2024 at 4:40 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>>
>>
>>
>> On 4/9/2024 2:54 PM, Michael Chan wrote:
>>> From: Vikas Gupta <vikas.gupta@broadcom.com>
>>>
>>> The existing scheme sets aside a number of MSIX/NQs for the RoCE
>>> driver whether the RoCE driver is registered or not.  This scheme
>>> is not flexible and limits the resources available for the L2 rings
>>> if RoCE is never used.
>>>
>>> Modify the scheme so that the RoCE MSIX/NQs can be used by the L2
>>> driver if they are not used for RoCE.  The MSIX/NQs are now
>>> represented by 3 fields.  bp->ulp_num_msix_want contains the
>>> desired default value, edev->ulp_num_msix_vec contains the
>>> available value (but not necessarily in use), and
>>> ulp_tbl->msix_requested contains the actual value in use by RoCE.
>>>
>>> The L2 driver can dip into edev->ulp_num_msix_vec if necessary.
>>>
>>> We need to add rtnl_lock() back in bnxt_register_dev() and
>>> bnxt_unregister_dev() to synchronize the MSIX usage between L2 and
>>> RoCE.
>>>
>>> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
>>> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
>>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>>> ---
>>
>> Whats the behavior if the L2 driver dips into this pool and then RoCE is
>> enabled later?
> 
> Thanks for the review.  If the user increases the L2 rings or enables
> XDP which will cause the driver to allocate a new set of XDP TX rings,
> it can now use the RoCE MSIX if needed and if they are not in use.
> 
>>
>> I guess RoCE would fail to get the resources it needs, but then system
>> administrator could re-configure the L2 device to use fewer resources?
> 
> If the above simply reduces the RoCE MSIX, the RoCE driver can still
> operate with fewer MSIX.  If the above has reduced the MSIX below the
> minimum required for RoCE, then RoCE will fail to initialize.  At that
> point, the user can reduce the L2 rings and reload the RoCE driver.

Sensible behavior. Thanks for the detailed explanation

-Jake

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

* Re: [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state
  2024-04-09 23:51     ` Michael Chan
@ 2024-04-10 19:29       ` Jacob Keller
  0 siblings, 0 replies; 20+ messages in thread
From: Jacob Keller @ 2024-04-10 19:29 UTC (permalink / raw)
  To: Michael Chan
  Cc: davem, netdev, edumazet, kuba, pabeni, pavan.chebbi,
	andrew.gospodarek, Kalesh AP, Somnath Kotur



On 4/9/2024 4:51 PM, Michael Chan wrote:
> On Tue, Apr 9, 2024 at 4:26 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>>
>>
>>
>> On 4/9/2024 2:54 PM, Michael Chan wrote:
>>> From: Pavan Chebbi <pavan.chebbi@broadcom.com>
>>>
>>> The current implementation requires the ifstate to be up when
>>> configuring the RSS contexts.  It will try to fetch the RX ring
>>> IDs and will crash if it is in ifdown state.  Return error if
>>> !netif_running() to prevent the crash.
>>>
>>> An improved implementation is in the works to allow RSS contexts
>>> to be changed while in ifdown state.
>>>
>>> Fixes: b3d0083caf9a ("bnxt_en: Support RSS contexts in ethtool .{get|set}_rxfh()")
>>> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>>> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
>>> Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
>>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>>> ---
>>
>> Makes sense, though I think you could send this fix directly to net as
>> its clearly a bug fix and preventing a crash is good.
> 
> This RSS context feature in the driver is new and is only in net-next,
> so the patch only applies to net-next.  Thanks.

Ah, even better fixing it before it hits a stable.

Thanks,
Jake

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

* Re: [PATCH net-next 0/7] bnxt_en: Updates for net-next
  2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
                   ` (6 preceding siblings ...)
  2024-04-09 21:54 ` [PATCH net-next 7/7] bnxt_en: Update MODULE_DESCRIPTION Michael Chan
@ 2024-04-11  3:10 ` patchwork-bot+netdevbpf
  7 siblings, 0 replies; 20+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-04-11  3:10 UTC (permalink / raw)
  To: Michael Chan
  Cc: davem, netdev, edumazet, kuba, pabeni, pavan.chebbi,
	andrew.gospodarek

Hello:

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

On Tue,  9 Apr 2024 14:54:24 -0700 you wrote:
> The first patch prevents a driver crash when RSS contexts are
> configred in ifdown state.  Patches 2 to 6 are improvements for
> managing MSIX for the aux device (for RoCE).  The existing
> scheme statically carves out the MSIX vectors for RoCE even if
> the RoCE driver is not loaded.  The new scheme adds flexibility
> and allows the L2 driver to use the RoCE MSIX vectors if needed
> when they are unused by the RoCE driver.  The last patch updates
> the MODULE_DESCRIPTION().
> 
> [...]

Here is the summary with links:
  - [net-next,1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state
    https://git.kernel.org/netdev/net-next/c/17b0dfa1f35b
  - [net-next,2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev()
    https://git.kernel.org/netdev/net-next/c/43226dccd1bd
  - [net-next,3/7] bnxt_en: Remove unneeded MSIX base structure fields and code
    https://git.kernel.org/netdev/net-next/c/b58f5a9c7034
  - [net-next,4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions
    https://git.kernel.org/netdev/net-next/c/194fad5b2781
  - [net-next,5/7] bnxt_en: Change MSIX/NQs allocation policy
    https://git.kernel.org/netdev/net-next/c/2e4592dc9bee
  - [net-next,6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered
    https://git.kernel.org/netdev/net-next/c/d630624ebd70
  - [net-next,7/7] bnxt_en: Update MODULE_DESCRIPTION
    https://git.kernel.org/netdev/net-next/c/008ce0fd3903

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



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

end of thread, other threads:[~2024-04-11  3:10 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-09 21:54 [PATCH net-next 0/7] bnxt_en: Updates for net-next Michael Chan
2024-04-09 21:54 ` [PATCH net-next 1/7] bnxt_en: Skip ethtool RSS context configuration in ifdown state Michael Chan
2024-04-09 23:26   ` Jacob Keller
2024-04-09 23:51     ` Michael Chan
2024-04-10 19:29       ` Jacob Keller
2024-04-09 21:54 ` [PATCH net-next 2/7] bnxt_en: Remove a redundant NULL check in bnxt_register_dev() Michael Chan
2024-04-09 23:35   ` Jacob Keller
2024-04-09 21:54 ` [PATCH net-next 3/7] bnxt_en: Remove unneeded MSIX base structure fields and code Michael Chan
2024-04-09 23:36   ` Jacob Keller
2024-04-09 21:54 ` [PATCH net-next 4/7] bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions Michael Chan
2024-04-09 23:37   ` Jacob Keller
2024-04-09 21:54 ` [PATCH net-next 5/7] bnxt_en: Change MSIX/NQs allocation policy Michael Chan
2024-04-09 23:40   ` Jacob Keller
2024-04-09 23:48     ` Michael Chan
2024-04-10 19:28       ` Jacob Keller
2024-04-09 21:54 ` [PATCH net-next 6/7] bnxt_en: Utilize ulp client resources if RoCE is not registered Michael Chan
2024-04-09 23:41   ` Jacob Keller
2024-04-09 21:54 ` [PATCH net-next 7/7] bnxt_en: Update MODULE_DESCRIPTION Michael Chan
2024-04-09 23:42   ` Jacob Keller
2024-04-11  3:10 ` [PATCH net-next 0/7] bnxt_en: Updates for net-next patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).