DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev]  [PATCH V2 00/10] bnxt patchset with bug fixes
@ 2020-01-13  5:01 Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 01/10] net/bnxt: handle flow create failure Kalesh A P
                   ` (10 more replies)
  0 siblings, 11 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

Please apply.

V1->V2: fixed the correct the commit ids in Fixes tag.

Kalesh AP (8):
  net/bnxt: handle flow create failure
  net/bnxt: fix probe failure in FreeBSD
  net/bnxt: fix to use correct IOVA mapping
  net/bnxt: fix enable/disable VLAN filtering
  net/bnxt: fix enable/disable vlan strip
  net/bnxt: handle hw filter setting when port is stopped
  net/bnxt: fix a memory leak in port stop
  net/bnxt: use macro for PCI log format

Somnath Kotur (2):
  net/bnxt: release port upon close
  net/bnxt: fix to cap max rings to minimum of compl rings and stat
    contexts

 drivers/net/bnxt/bnxt.h        |  21 +-
 drivers/net/bnxt/bnxt_ethdev.c | 422 ++++++++++++++++++++++-------------------
 drivers/net/bnxt/bnxt_filter.c |   5 +-
 drivers/net/bnxt/bnxt_flow.c   |   4 +-
 drivers/net/bnxt/bnxt_hwrm.c   |  28 ++-
 drivers/net/bnxt/bnxt_irq.c    |   8 +
 drivers/net/bnxt/bnxt_ring.c   |  17 --
 drivers/net/bnxt/bnxt_vnic.c   |  11 --
 8 files changed, 273 insertions(+), 243 deletions(-)

-- 
2.10.1


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

* [dpdk-dev]  [PATCH V2 01/10] net/bnxt: handle flow create failure
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 02/10] net/bnxt: fix probe failure in FreeBSD Kalesh A P
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

If flow create fails due to not enough filter resources,
driver does not populate the rte_flow_error using
rte_flow_error_set().

Since "rte_errno" could have garbage value and is not relaiable,
it could cause a segfault in the stack in port_flow_complain().

Fix it to set rte_flow_error using rte_flow_error_set()
when flow create fails due to not enough filter resources.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index cde1fa4..5564c53 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1702,7 +1702,9 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 
 	filter = bnxt_get_unused_filter(bp);
 	if (filter == NULL) {
-		PMD_DRV_LOG(ERR, "Not enough resources for a new flow.\n");
+		rte_flow_error_set(error, ENOSPC,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Not enough resources for a new flow");
 		goto free_flow;
 	}
 
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 02/10] net/bnxt: fix probe failure in FreeBSD
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 01/10] net/bnxt: handle flow create failure Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 03/10] net/bnxt: fix to use correct IOVA mapping Kalesh A P
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

In FreeBSD environment, nic_uio driver does not support interrupts
and rte_intr_callback_register() will fail to register interrupts
which in turn causes bnxt driver probe failure.

Fixed driver to ignore interrupt callback failures in FreeBSD.
Also fixed to not use a dedicated completion ring for async events
from FW and process these events on RXQ0 in FreeBSD.

Fixes: 6de4c538b393 ("net/bnxt: fix error handling in port start")
Fixes: 43f78b380f89 ("net/bnxt: retry IRQ callback deregistration")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        | 8 ++++++++
 drivers/net/bnxt/bnxt_ethdev.c | 3 +++
 drivers/net/bnxt/bnxt_irq.c    | 8 ++++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 21ca059..3487b91 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -117,6 +117,14 @@
 #define BNXT_NUM_ASYNC_CPR(bp) 1
 #endif
 
+/* In FreeBSD OS, nic_uio driver does not support interrupts */
+#ifdef RTE_EXEC_ENV_FREEBSD
+#ifdef BNXT_NUM_ASYNC_CPR
+#undef BNXT_NUM_ASYNC_CPR
+#endif
+#define BNXT_NUM_ASYNC_CPR(bp)	0
+#endif
+
 #define BNXT_MISC_VEC_ID               RTE_INTR_VEC_ZERO_OFFSET
 #define BNXT_RX_VEC_START              RTE_INTR_VEC_RXTX_OFFSET
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7b5df9a..879ea58 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -439,8 +439,11 @@ static int bnxt_init_chip(struct bnxt *bp)
 
 	/* enable uio/vfio intr/eventfd mapping */
 	rc = rte_intr_enable(intr_handle);
+#ifndef RTE_EXEC_ENV_FREEBSD
+	/* In FreeBSD OS, nic_uio driver does not support interrupts */
 	if (rc)
 		goto err_free;
+#endif
 
 	rc = bnxt_get_hwrm_link_config(bp, &new);
 	if (rc) {
diff --git a/drivers/net/bnxt/bnxt_irq.c b/drivers/net/bnxt/bnxt_irq.c
index 846325e..40e1b0c 100644
--- a/drivers/net/bnxt/bnxt_irq.c
+++ b/drivers/net/bnxt/bnxt_irq.c
@@ -181,5 +181,13 @@ int bnxt_request_int(struct bnxt *bp)
 			irq->requested = 1;
 	}
 
+#ifdef RTE_EXEC_ENV_FREEBSD
+	/**
+	 * In FreeBSD OS, nic_uio does not support interrupts and
+	 * interrupt register callback will fail.
+	 */
+	rc = 0;
+#endif
+
 	return rc;
 }
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 03/10] net/bnxt: fix to use correct IOVA mapping
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 01/10] net/bnxt: handle flow create failure Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 02/10] net/bnxt: fix probe failure in FreeBSD Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 04/10] net/bnxt: fix enable/disable VLAN filtering Kalesh A P
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

Use rte_malloc_virt2iova() to obtain the IO address of a
virtual address obtained through rte_malloc().

Fixed to use the iova address returned by rte_memzone_reserve_aligned()
as the call always returns with populating "mz->iova" with
rte_malloc_virt2iova(mz->addr).

Removed redundant rte_mem_lock_page() call to lock the pages.

Fixes: 96b0931d51e7 ("fix extended port counter statistics")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 49 ------------------------------------------
 drivers/net/bnxt/bnxt_hwrm.c   | 28 ++++++++++--------------
 drivers/net/bnxt/bnxt_ring.c   | 17 ---------------
 drivers/net/bnxt/bnxt_vnic.c   | 11 ----------
 4 files changed, 11 insertions(+), 94 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 879ea58..a948c78 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4233,18 +4233,6 @@ static int bnxt_alloc_ctx_mem_blk(struct bnxt *bp,
 
 		memset(mz->addr, 0, mz->len);
 		mz_phys_addr = mz->iova;
-		if ((unsigned long)mz->addr == mz_phys_addr) {
-			PMD_DRV_LOG(DEBUG,
-				    "physical address same as virtual\n");
-			PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-			mz_phys_addr = rte_mem_virt2iova(mz->addr);
-			if (mz_phys_addr == RTE_BAD_IOVA) {
-				PMD_DRV_LOG(ERR,
-					"unable to map addr to phys memory\n");
-				return -ENOMEM;
-			}
-		}
-		rte_mem_lock_page(((char *)mz->addr));
 
 		rmem->pg_tbl = mz->addr;
 		rmem->pg_tbl_map = mz_phys_addr;
@@ -4268,22 +4256,8 @@ static int bnxt_alloc_ctx_mem_blk(struct bnxt *bp,
 
 	memset(mz->addr, 0, mz->len);
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		for (sz = 0; sz < mem_size; sz += BNXT_PAGE_SIZE)
-			rte_mem_lock_page(((char *)mz->addr) + sz);
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "unable to map addr to phys memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	for (sz = 0, i = 0; sz < mem_size; sz += BNXT_PAGE_SIZE, i++) {
-		rte_mem_lock_page(((char *)mz->addr) + sz);
 		rmem->pg_arr[i] = ((char *)mz->addr) + sz;
 		rmem->dma_arr[i] = mz_phys_addr + sz;
 
@@ -4460,18 +4434,6 @@ static int bnxt_alloc_stats_mem(struct bnxt *bp)
 	}
 	memset(mz->addr, 0, mz->len);
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG,
-			    "Using rte_mem_virt2iova()\n");
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "Can't map address to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	bp->rx_mem_zone = (const void *)mz;
 	bp->hw_rx_port_stats = mz->addr;
@@ -4498,17 +4460,6 @@ static int bnxt_alloc_stats_mem(struct bnxt *bp)
 	}
 	memset(mz->addr, 0, mz->len);
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "Can't map address to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	bp->tx_mem_zone = (const void *)mz;
 	bp->hw_tx_port_stats = mz->addr;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 50272dc..3b01339 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -309,8 +309,8 @@ int bnxt_hwrm_cfa_l2_set_rx_mask(struct bnxt *bp,
 	if (vlan_table) {
 		if (!(mask & HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_VLAN_NONVLAN))
 			mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_VLANONLY;
-		req.vlan_tag_tbl_addr = rte_cpu_to_le_64(
-			 rte_mem_virt2iova(vlan_table));
+		req.vlan_tag_tbl_addr =
+			rte_cpu_to_le_64(rte_malloc_virt2iova(vlan_table));
 		req.num_vlan_tags = rte_cpu_to_le_32((uint32_t)vlan_count);
 	}
 	req.mask = rte_cpu_to_le_32(mask);
@@ -351,7 +351,7 @@ int bnxt_hwrm_cfa_vlan_antispoof_cfg(struct bnxt *bp, uint16_t fid,
 	req.fid = rte_cpu_to_le_16(fid);
 
 	req.vlan_tag_mask_tbl_addr =
-		rte_cpu_to_le_64(rte_mem_virt2iova(vlan_table));
+		rte_cpu_to_le_64(rte_malloc_virt2iova(vlan_table));
 	req.num_vlan_entries = rte_cpu_to_le_32((uint32_t)vlan_count);
 
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
@@ -1024,9 +1024,8 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto error;
 		}
-		rte_mem_lock_page(bp->hwrm_cmd_resp_addr);
 		bp->hwrm_cmd_resp_dma_addr =
-			rte_mem_virt2iova(bp->hwrm_cmd_resp_addr);
+			rte_malloc_virt2iova(bp->hwrm_cmd_resp_addr);
 		if (bp->hwrm_cmd_resp_dma_addr == RTE_BAD_IOVA) {
 			PMD_DRV_LOG(ERR,
 			"Unable to map response buffer to physical memory.\n");
@@ -1061,9 +1060,8 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto error;
 		}
-		rte_mem_lock_page(bp->hwrm_short_cmd_req_addr);
 		bp->hwrm_short_cmd_req_dma_addr =
-			rte_mem_virt2iova(bp->hwrm_short_cmd_req_addr);
+			rte_malloc_virt2iova(bp->hwrm_short_cmd_req_addr);
 		if (bp->hwrm_short_cmd_req_dma_addr == RTE_BAD_IOVA) {
 			rte_free(bp->hwrm_short_cmd_req_addr);
 			PMD_DRV_LOG(ERR,
@@ -2471,11 +2469,10 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	bp->max_resp_len = HWRM_MAX_RESP_LEN;
 	bp->hwrm_cmd_resp_addr = rte_malloc(type, bp->max_resp_len, 0);
-	rte_mem_lock_page(bp->hwrm_cmd_resp_addr);
 	if (bp->hwrm_cmd_resp_addr == NULL)
 		return -ENOMEM;
 	bp->hwrm_cmd_resp_dma_addr =
-		rte_mem_virt2iova(bp->hwrm_cmd_resp_addr);
+		rte_malloc_virt2iova(bp->hwrm_cmd_resp_addr);
 	if (bp->hwrm_cmd_resp_dma_addr == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -3421,7 +3418,7 @@ int bnxt_hwrm_func_buf_rgtr(struct bnxt *bp)
 			 page_getenum(bp->pf.active_vfs * HWRM_MAX_REQ_LEN));
 	req.req_buf_len = rte_cpu_to_le_16(HWRM_MAX_REQ_LEN);
 	req.req_buf_page_addr0 =
-		rte_cpu_to_le_64(rte_mem_virt2iova(bp->pf.vf_req_buf));
+		rte_cpu_to_le_64(rte_malloc_virt2iova(bp->pf.vf_req_buf));
 	if (req.req_buf_page_addr0 == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map buffer address to physical memory\n");
@@ -3851,10 +3848,9 @@ int bnxt_get_nvram_directory(struct bnxt *bp, uint32_t len, uint8_t *data)
 
 	buflen = dir_entries * entry_length;
 	buf = rte_malloc("nvm_dir", buflen, 0);
-	rte_mem_lock_page(buf);
 	if (buf == NULL)
 		return -ENOMEM;
-	dma_handle = rte_mem_virt2iova(buf);
+	dma_handle = rte_malloc_virt2iova(buf);
 	if (dma_handle == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -3885,11 +3881,10 @@ int bnxt_hwrm_get_nvram_item(struct bnxt *bp, uint32_t index,
 	struct hwrm_nvm_read_output *resp = bp->hwrm_cmd_resp_addr;
 
 	buf = rte_malloc("nvm_item", length, 0);
-	rte_mem_lock_page(buf);
 	if (!buf)
 		return -ENOMEM;
 
-	dma_handle = rte_mem_virt2iova(buf);
+	dma_handle = rte_malloc_virt2iova(buf);
 	if (dma_handle == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -3939,11 +3934,10 @@ int bnxt_hwrm_flash_nvram(struct bnxt *bp, uint16_t dir_type,
 	uint8_t *buf;
 
 	buf = rte_malloc("nvm_write", data_len, 0);
-	rte_mem_lock_page(buf);
 	if (!buf)
 		return -ENOMEM;
 
-	dma_handle = rte_mem_virt2iova(buf);
+	dma_handle = rte_malloc_virt2iova(buf);
 	if (dma_handle == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -4006,7 +4000,7 @@ static int bnxt_hwrm_func_vf_vnic_query(struct bnxt *bp, uint16_t vf,
 
 	req.vf_id = rte_cpu_to_le_16(bp->pf.first_vf_id + vf);
 	req.max_vnic_id_cnt = rte_cpu_to_le_32(bp->pf.total_vnics);
-	req.vnic_id_tbl_addr = rte_cpu_to_le_64(rte_mem_virt2iova(vnic_ids));
+	req.vnic_id_tbl_addr = rte_cpu_to_le_64(rte_malloc_virt2iova(vnic_ids));
 
 	if (req.vnic_id_tbl_addr == RTE_BAD_IOVA) {
 		HWRM_UNLOCK();
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index ea46fa9..d6e4e8a 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -110,9 +110,7 @@ int bnxt_alloc_rings(struct bnxt *bp, uint16_t qidx,
 	uint64_t rx_offloads = bp->eth_dev->data->dev_conf.rxmode.offloads;
 	const struct rte_memzone *mz = NULL;
 	char mz_name[RTE_MEMZONE_NAMESIZE];
-	rte_iova_t mz_phys_addr_base;
 	rte_iova_t mz_phys_addr;
-	int sz;
 
 	int stats_len = (tx_ring_info || rx_ring_info) ?
 	    RTE_CACHE_LINE_ROUNDUP(sizeof(struct hwrm_stat_ctx_query_output) -
@@ -214,22 +212,7 @@ int bnxt_alloc_rings(struct bnxt *bp, uint16_t qidx,
 			return -ENOMEM;
 	}
 	memset(mz->addr, 0, mz->len);
-	mz_phys_addr_base = mz->iova;
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr_base) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		for (sz = 0; sz < total_alloc_len; sz += getpagesize())
-			rte_mem_lock_page(((char *)mz->addr) + sz);
-		mz_phys_addr_base = rte_mem_virt2iova(mz->addr);
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-			"unable to map ring address to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	if (tx_ring_info) {
 		txq->mz = mz;
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index 104342e..bc054a8 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -150,17 +150,6 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
 			return -ENOMEM;
 	}
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "unable to map to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	for (i = 0; i < max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 04/10] net/bnxt: fix enable/disable VLAN filtering
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (2 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 03/10] net/bnxt: fix to use correct IOVA mapping Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 05/10] net/bnxt: fix enable/disable vlan strip Kalesh A P
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

There is no condition check for the user requested operation
for VLAN filtering. As a result, VLAN filtering is getting disabled
when the user enables/disables VLAN stripping on same port.

The function bnxt_hwrm_clear_l2_filter() didn't actually free
L2 filter in HW if the reference count of filter is zero.

Fixed it by incrementing the reference count of filter in
bnxt_alloc_filter() routine.

Because of the recent changes in bnxt_hwrm_clear_l2_filter(),
change was needed in the routine bnxt_set_default_mac_addr_op()
to destroy and re-create the default filter when the user
changes the default MAC of the port.

Fixes: 51fda51e7b70 ("net/bnxt: fix to keep the L2 filter intact so it can be reused")
Fixes: 6118503d8071 ("net/bnxt: fix VLAN filtering")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 66 +++++++++++++++++++++++-------------------
 drivers/net/bnxt/bnxt_filter.c |  5 ++--
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index a948c78..83f475d 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1860,18 +1860,12 @@ static int bnxt_del_dflt_mac_filter(struct bnxt *bp,
 }
 
 static int
-bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
+bnxt_config_vlan_hw_filter(struct bnxt *bp, uint64_t rx_offloads)
 {
-	struct bnxt *bp = dev->data->dev_private;
-	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
 	struct bnxt_vnic_info *vnic;
 	unsigned int i;
 	int rc;
 
-	rc = is_bnxt_in_error(bp);
-	if (rc)
-		return rc;
-
 	vnic = BNXT_GET_DEFAULT_VNIC(bp);
 	if (!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
 		/* Remove any VLAN filters programmed */
@@ -1895,6 +1889,28 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	PMD_DRV_LOG(DEBUG, "VLAN Filtering: %d\n",
 		    !!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER));
 
+	return 0;
+}
+
+static int
+bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
+{
+	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	struct bnxt *bp = dev->data->dev_private;
+	unsigned int i;
+	int rc;
+
+	rc = is_bnxt_in_error(bp);
+	if (rc)
+		return rc;
+
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		/* Enable or disable VLAN filtering */
+		rc = bnxt_config_vlan_hw_filter(bp, rx_offloads);
+		if (rc)
+			return rc;
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		for (i = 0; i < bp->nr_vnics; i++) {
@@ -1984,7 +2000,6 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 	struct bnxt *bp = dev->data->dev_private;
 	/* Default Filter is tied to VNIC 0 */
 	struct bnxt_vnic_info *vnic = BNXT_GET_DEFAULT_VNIC(bp);
-	struct bnxt_filter_info *filter;
 	int rc;
 
 	rc = is_bnxt_in_error(bp);
@@ -1997,32 +2012,23 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 	if (rte_is_zero_ether_addr(addr))
 		return -EINVAL;
 
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
-		/* Default Filter is at Index 0 */
-		if (filter->mac_index != 0)
-			continue;
-
-		memcpy(filter->l2_addr, addr, RTE_ETHER_ADDR_LEN);
-		memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
-		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX |
-			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
-		filter->enables |=
-			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
-			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
+	/* Check if the requested MAC is already added */
+	if (memcmp(addr, bp->mac_addr, RTE_ETHER_ADDR_LEN) == 0)
+		return 0;
 
-		rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
-		if (rc) {
-			memcpy(filter->l2_addr, bp->mac_addr,
-			       RTE_ETHER_ADDR_LEN);
-			return rc;
-		}
+	/* Destroy filter and re-create it */
+	bnxt_del_dflt_mac_filter(bp, vnic);
 
-		memcpy(bp->mac_addr, addr, RTE_ETHER_ADDR_LEN);
-		PMD_DRV_LOG(DEBUG, "Set MAC addr\n");
-		return 0;
+	memcpy(bp->mac_addr, addr, RTE_ETHER_ADDR_LEN);
+	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_VLAN_FILTER) {
+		/* This filter will allow only untagged packets */
+		rc = bnxt_add_vlan_filter(bp, 0);
+	} else {
+		rc = bnxt_add_mac_filter(bp, vnic, addr, 0, 0);
 	}
 
-	return 0;
+	PMD_DRV_LOG(DEBUG, "Set MAC addr\n");
+	return rc;
 }
 
 static int
diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 0c410f8..b31f104 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -39,9 +39,10 @@ struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp)
 	filter->flags = HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
 	filter->enables = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
-	memcpy(filter->l2_addr, bp->eth_dev->data->mac_addrs->addr_bytes,
-	       RTE_ETHER_ADDR_LEN);
+	memcpy(filter->l2_addr, bp->mac_addr, RTE_ETHER_ADDR_LEN);
 	memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
+	/* bump up the reference count of filter */
+	filter->l2_ref_cnt++;
 	return filter;
 }
 
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 05/10] net/bnxt: fix enable/disable vlan strip
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (3 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 04/10] net/bnxt: fix enable/disable VLAN filtering Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 06/10] net/bnxt: handle hw filter setting when port is stopped Kalesh A P
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

HWRM_VNIC_CFG command to configure vnic dynamically with
traffic running is not working. Driver has to free and
recreate the vnic and then reconfigure the vnic filters.

Fixes: 7fe5668d2ea3 ("net/bnxt: support VLAN filter and strip")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 248 +++++++++++++++++++++++++----------------
 1 file changed, 150 insertions(+), 98 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 83f475d..0b2c29b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -132,6 +132,7 @@ static int bnxt_dev_uninit(struct rte_eth_dev *eth_dev);
 static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev);
 static int bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev);
 static void bnxt_cancel_fw_health_check(struct bnxt *bp);
+static int bnxt_restore_vlan_filters(struct bnxt *bp);
 
 int is_bnxt_in_error(struct bnxt *bp)
 {
@@ -228,14 +229,97 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool reconfig)
 	return rc;
 }
 
-static int bnxt_init_chip(struct bnxt *bp)
+static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id)
 {
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+	struct bnxt_vnic_info *vnic = &bp->vnic_info[vnic_id];
+	uint64_t rx_offloads = dev_conf->rxmode.offloads;
 	struct bnxt_rx_queue *rxq;
+	unsigned int j;
+	int rc;
+
+	rc = bnxt_vnic_grp_alloc(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	PMD_DRV_LOG(DEBUG, "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
+		    vnic_id, vnic, vnic->fw_grp_ids);
+
+	rc = bnxt_hwrm_vnic_alloc(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	/* Alloc RSS context only if RSS mode is enabled */
+	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) {
+		int j, nr_ctxs = bnxt_rss_ctxts(bp);
+
+		rc = 0;
+		for (j = 0; j < nr_ctxs; j++) {
+			rc = bnxt_hwrm_vnic_ctx_alloc(bp, vnic, j);
+			if (rc)
+				break;
+		}
+		if (rc) {
+			PMD_DRV_LOG(ERR,
+				    "HWRM vnic %d ctx %d alloc failure rc: %x\n",
+				    vnic_id, j, rc);
+			goto err_out;
+		}
+		vnic->num_lb_ctxts = nr_ctxs;
+	}
+
+	/*
+	 * Firmware sets pf pair in default vnic cfg. If the VLAN strip
+	 * setting is not available at this time, it will not be
+	 * configured correctly in the CFA.
+	 */
+	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+		vnic->vlan_strip = true;
+	else
+		vnic->vlan_strip = false;
+
+	rc = bnxt_hwrm_vnic_cfg(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	rc = bnxt_set_hwrm_vnic_filters(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	for (j = 0; j < bp->rx_num_qs_per_vnic; j++) {
+		rxq = bp->eth_dev->data->rx_queues[j];
+
+		PMD_DRV_LOG(DEBUG,
+			    "rxq[%d]->vnic=%p vnic->fw_grp_ids=%p\n",
+			    j, rxq->vnic, rxq->vnic->fw_grp_ids);
+
+		if (BNXT_HAS_RING_GRPS(bp) && rxq->rx_deferred_start)
+			rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
+	}
+
+	rc = bnxt_vnic_rss_configure(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);
+
+	if (rx_offloads & DEV_RX_OFFLOAD_TCP_LRO)
+		bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 1);
+	else
+		bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 0);
+
+	return 0;
+err_out:
+	PMD_DRV_LOG(ERR, "HWRM vnic %d cfg failure rc: %x\n",
+		    vnic_id, rc);
+	return rc;
+}
+
+static int bnxt_init_chip(struct bnxt *bp)
+{
 	struct rte_eth_link new;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
-	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
-	uint64_t rx_offloads = dev_conf->rxmode.offloads;
 	uint32_t intr_vector = 0;
 	uint32_t queue_id, base = BNXT_MISC_VEC_ID;
 	uint32_t vec = BNXT_MISC_VEC_ID;
@@ -303,93 +387,11 @@ static int bnxt_init_chip(struct bnxt *bp)
 
 	/* VNIC configuration */
 	for (i = 0; i < bp->nr_vnics; i++) {
-		struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
-		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
-
-		rc = bnxt_vnic_grp_alloc(bp, vnic);
+		rc = bnxt_setup_one_vnic(bp, i);
 		if (rc)
 			goto err_out;
-
-		PMD_DRV_LOG(DEBUG, "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
-			    i, vnic, vnic->fw_grp_ids);
-
-		rc = bnxt_hwrm_vnic_alloc(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR, "HWRM vnic %d alloc failure rc: %x\n",
-				i, rc);
-			goto err_out;
-		}
-
-		/* Alloc RSS context only if RSS mode is enabled */
-		if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) {
-			int j, nr_ctxs = bnxt_rss_ctxts(bp);
-
-			rc = 0;
-			for (j = 0; j < nr_ctxs; j++) {
-				rc = bnxt_hwrm_vnic_ctx_alloc(bp, vnic, j);
-				if (rc)
-					break;
-			}
-			if (rc) {
-				PMD_DRV_LOG(ERR,
-				  "HWRM vnic %d ctx %d alloc failure rc: %x\n",
-				  i, j, rc);
-				goto err_out;
-			}
-			vnic->num_lb_ctxts = nr_ctxs;
-		}
-
-		/*
-		 * Firmware sets pf pair in default vnic cfg. If the VLAN strip
-		 * setting is not available at this time, it will not be
-		 * configured correctly in the CFA.
-		 */
-		if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
-			vnic->vlan_strip = true;
-		else
-			vnic->vlan_strip = false;
-
-		rc = bnxt_hwrm_vnic_cfg(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR, "HWRM vnic %d cfg failure rc: %x\n",
-				i, rc);
-			goto err_out;
-		}
-
-		rc = bnxt_set_hwrm_vnic_filters(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR,
-				"HWRM vnic %d filter failure rc: %x\n",
-				i, rc);
-			goto err_out;
-		}
-
-		for (j = 0; j < bp->rx_num_qs_per_vnic; j++) {
-			rxq = bp->eth_dev->data->rx_queues[j];
-
-			PMD_DRV_LOG(DEBUG,
-				    "rxq[%d]->vnic=%p vnic->fw_grp_ids=%p\n",
-				    j, rxq->vnic, rxq->vnic->fw_grp_ids);
-
-			if (BNXT_HAS_RING_GRPS(bp) && rxq->rx_deferred_start)
-				rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
-		}
-
-		rc = bnxt_vnic_rss_configure(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR,
-				    "HWRM vnic set RSS failure rc: %x\n", rc);
-			goto err_out;
-		}
-
-		bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);
-
-		if (bp->eth_dev->data->dev_conf.rxmode.offloads &
-		    DEV_RX_OFFLOAD_TCP_LRO)
-			bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 1);
-		else
-			bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 0);
 	}
+
 	rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, &bp->vnic_info[0], 0, NULL);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
@@ -1892,12 +1894,69 @@ bnxt_config_vlan_hw_filter(struct bnxt *bp, uint64_t rx_offloads)
 	return 0;
 }
 
+static int bnxt_free_one_vnic(struct bnxt *bp, uint16_t vnic_id)
+{
+	struct bnxt_vnic_info *vnic = &bp->vnic_info[vnic_id];
+	unsigned int i;
+	int rc;
+
+	/* Destroy vnic filters and vnic */
+	if (bp->eth_dev->data->dev_conf.rxmode.offloads &
+	    DEV_RX_OFFLOAD_VLAN_FILTER) {
+		for (i = 0; i < RTE_ETHER_MAX_VLAN_ID; i++)
+			bnxt_del_vlan_filter(bp, i);
+	}
+	bnxt_del_dflt_mac_filter(bp, vnic);
+
+	rc = bnxt_hwrm_vnic_free(bp, vnic);
+	if (rc)
+		return rc;
+
+	rte_free(vnic->fw_grp_ids);
+	vnic->fw_grp_ids = NULL;
+
+	return 0;
+}
+
+static int
+bnxt_config_vlan_hw_stripping(struct bnxt *bp, uint64_t rx_offloads)
+{
+	struct bnxt_vnic_info *vnic = BNXT_GET_DEFAULT_VNIC(bp);
+	int rc;
+
+	/* Destroy, recreate and reconfigure the default vnic */
+	rc = bnxt_free_one_vnic(bp, 0);
+	if (rc)
+		return rc;
+
+	/* default vnic 0 */
+	rc = bnxt_setup_one_vnic(bp, 0);
+	if (rc)
+		return rc;
+
+	if (bp->eth_dev->data->dev_conf.rxmode.offloads &
+	    DEV_RX_OFFLOAD_VLAN_FILTER) {
+		rc = bnxt_add_vlan_filter(bp, 0);
+		bnxt_restore_vlan_filters(bp);
+	} else {
+		rc = bnxt_add_mac_filter(bp, vnic, NULL, 0, 0);
+	}
+
+	rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL);
+	if (rc)
+		return rc;
+
+	PMD_DRV_LOG(DEBUG, "VLAN Strip Offload: %d\n",
+		    !!(rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP));
+
+	return rc;
+}
+
 static int
 bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 {
 	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
 	struct bnxt *bp = dev->data->dev_private;
-	unsigned int i;
 	int rc;
 
 	rc = is_bnxt_in_error(bp);
@@ -1913,16 +1972,9 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
-		for (i = 0; i < bp->nr_vnics; i++) {
-			struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
-			if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
-				vnic->vlan_strip = true;
-			else
-				vnic->vlan_strip = false;
-			bnxt_hwrm_vnic_cfg(bp, vnic);
-		}
-		PMD_DRV_LOG(DEBUG, "VLAN Strip Offload: %d\n",
-			!!(rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP));
+		rc = bnxt_config_vlan_hw_stripping(bp, rx_offloads);
+		if (rc)
+			return rc;
 	}
 
 	if (mask & ETH_VLAN_EXTEND_MASK) {
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 06/10] net/bnxt: handle hw filter setting when port is stopped
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (4 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 05/10] net/bnxt: fix enable/disable vlan strip Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 07/10] net/bnxt: fix a memory leak in port stop Kalesh A P
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

Driver destroy the vnic when the port is brought down.
Port hw filter setting such as promiscuos, allmulti and
vlan filtering will be applied when port is started.

Fixed to return success silently for these callbacks
when port is stopped. Also fixed to clear "bp->dev_stopped"
before invoking bnxt_vlan_offload_set_op() in bnxt_dev_start_op().

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 0b2c29b..436ecbb 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -870,6 +870,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 	eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
 
 	bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
+	bp->dev_stopped = 0;
 
 	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		vlan_mask |= ETH_VLAN_FILTER_MASK;
@@ -884,7 +885,6 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	bp->flags |= BNXT_FLAG_INIT_DONE;
 	eth_dev->data->dev_started = 1;
-	bp->dev_stopped = 0;
 	pthread_mutex_lock(&bp->def_cp_lock);
 	bnxt_schedule_fw_health_check(bp);
 	pthread_mutex_unlock(&bp->def_cp_lock);
@@ -895,6 +895,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 	bnxt_shutdown_nic(bp);
 	bnxt_free_tx_mbufs(bp);
 	bnxt_free_rx_mbufs(bp);
+	bp->dev_stopped = 1;
 	return rc;
 }
 
@@ -1168,6 +1169,10 @@ static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1193,6 +1198,10 @@ static int bnxt_promiscuous_disable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1218,6 +1227,10 @@ static int bnxt_allmulticast_enable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1243,6 +1256,10 @@ static int bnxt_allmulticast_disable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1963,6 +1980,10 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (mask & ETH_VLAN_FILTER_MASK) {
 		/* Enable or disable VLAN filtering */
 		rc = bnxt_config_vlan_hw_filter(bp, rx_offloads);
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 07/10] net/bnxt: fix a memory leak in port stop
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (5 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 06/10] net/bnxt: handle hw filter setting when port is stopped Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 08/10] net/bnxt: use macro for PCI log format Kalesh A P
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

The memory for mark table is allocated during port start.
But the allocated memory is freed only during port close
or driver unload which in turn causes a memory leakage
on each port start/stop.

Fixed it by moving the memory free to port stop.

Fixes: a968a9f5456d ("net/bnxt: add support for flow mark action")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 436ecbb..2661418 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -970,7 +970,10 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 	bnxt_int_handler(eth_dev);
 	bnxt_shutdown_nic(bp);
 	bnxt_hwrm_if_change(bp, 0);
-	memset(bp->mark_table, 0, BNXT_MARK_TABLE_SZ);
+
+	rte_free(bp->mark_table);
+	bp->mark_table = NULL;
+
 	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 	bp->dev_stopped = 1;
 	bp->rx_cosq_cnt = 0;
@@ -992,9 +995,6 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 		bp->grp_info = NULL;
 	}
 
-	rte_free(bp->mark_table);
-	bp->mark_table = NULL;
-
 	bnxt_dev_uninit(eth_dev);
 }
 
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 08/10] net/bnxt: use macro for PCI log format
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (6 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 07/10] net/bnxt: fix a memory leak in port stop Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 09/10] net/bnxt: release port upon close Kalesh A P
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

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

Fixes: 19e6af01bb36 ("net/bnxt: support get/set EEPROM")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 2661418..ba3f0a7 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3593,9 +3593,9 @@ bnxt_get_eeprom_length_op(struct rte_eth_dev *dev)
 	if (rc)
 		return rc;
 
-	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x\n",
-		bp->pdev->addr.domain, bp->pdev->addr.bus,
-		bp->pdev->addr.devid, bp->pdev->addr.function);
+	PMD_DRV_LOG(INFO, PCI_PRI_FMT "\n",
+		    bp->pdev->addr.domain, bp->pdev->addr.bus,
+		    bp->pdev->addr.devid, bp->pdev->addr.function);
 
 	rc = bnxt_hwrm_nvm_get_dir_info(bp, &dir_entries, &entry_length);
 	if (rc != 0)
@@ -3617,10 +3617,10 @@ bnxt_get_eeprom_op(struct rte_eth_dev *dev,
 	if (rc)
 		return rc;
 
-	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x in_eeprom->offset = %d "
-		"len = %d\n", bp->pdev->addr.domain,
-		bp->pdev->addr.bus, bp->pdev->addr.devid,
-		bp->pdev->addr.function, in_eeprom->offset, in_eeprom->length);
+	PMD_DRV_LOG(INFO, PCI_PRI_FMT " in_eeprom->offset = %d len = %d\n",
+		    bp->pdev->addr.domain, bp->pdev->addr.bus,
+		    bp->pdev->addr.devid, bp->pdev->addr.function,
+		    in_eeprom->offset, in_eeprom->length);
 
 	if (in_eeprom->offset == 0) /* special offset value to get directory */
 		return bnxt_get_nvram_directory(bp, in_eeprom->length,
@@ -3693,10 +3693,10 @@ bnxt_set_eeprom_op(struct rte_eth_dev *dev,
 	if (rc)
 		return rc;
 
-	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x in_eeprom->offset = %d "
-		"len = %d\n", bp->pdev->addr.domain,
-		bp->pdev->addr.bus, bp->pdev->addr.devid,
-		bp->pdev->addr.function, in_eeprom->offset, in_eeprom->length);
+	PMD_DRV_LOG(INFO, PCI_PRI_FMT " in_eeprom->offset = %d len = %d\n",
+		    bp->pdev->addr.domain, bp->pdev->addr.bus,
+		    bp->pdev->addr.devid, bp->pdev->addr.function,
+		    in_eeprom->offset, in_eeprom->length);
 
 	if (!BNXT_PF(bp)) {
 		PMD_DRV_LOG(ERR, "NVM write not supported from a VF\n");
-- 
2.10.1


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

* [dpdk-dev]  [PATCH V2 09/10] net/bnxt: release port upon close
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (7 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 08/10] net/bnxt: use macro for PCI log format Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 10/10] net/bnxt: fix to cap max rings to minimum of compl rings and stat contexts Kalesh A P
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

From: Somnath Kotur <somnath.kotur@broadcom.com>

Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
for the port can be freed by rte_eth_dev_close().

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ba3f0a7..8acfade 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
 	if (rc)
 		goto error_free;
 
+	/* Pass the information to the rte_eth_dev_close() that it should also
+	 * release the private port resources.
+	 */
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
+
 	PMD_DRV_LOG(INFO,
 		    DRV_MODULE_NAME "found at mem %" PRIX64 ", node addr %pM\n",
 		    pci_dev->mem_resource[0].phys_addr,
-- 
2.10.1


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

* [dpdk-dev] [PATCH V2 10/10] net/bnxt: fix to cap max rings to minimum of compl rings and stat contexts
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (8 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 09/10] net/bnxt: release port upon close Kalesh A P
@ 2020-01-13  5:01 ` Kalesh A P
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
  10 siblings, 0 replies; 29+ messages in thread
From: Kalesh A P @ 2020-01-13  5:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, ajit.khaparde

From: Somnath Kotur <somnath.kotur@broadcom.com>

Max Tx rings count could be lesser than max Rx rings in some
cases, so take this into account as well.

Account for stat contexts available(one for each ring) along with
no: of completion rings(one for each ring) to cap the max no: of
Tx /Rx rings that can be possibly created.

Fixes: f03e66cb ("net/bnxt: limit queue count for NS3/Stingray devices")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 3487b91..ddb2681 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -622,12 +622,19 @@ struct bnxt {
 	uint16_t		max_tx_rings;
 	uint16_t		max_rx_rings;
 #define MAX_STINGRAY_RINGS		128U
-#define BNXT_MAX_RINGS(bp) \
+/* For sake of symmetry, max Tx rings == max Rx rings, one stat ctx for each */
+#define BNXT_MAX_RX_RINGS(bp) \
 	(BNXT_STINGRAY(bp) ? RTE_MIN(RTE_MIN(bp->max_rx_rings, \
 					     MAX_STINGRAY_RINGS), \
-				     bp->max_stat_ctx) : \
-				RTE_MIN(bp->max_rx_rings, bp->max_stat_ctx))
+				     bp->max_stat_ctx / 2U) : \
+				RTE_MIN(bp->max_rx_rings, \
+					bp->max_stat_ctx / 2U))
+#define BNXT_MAX_TX_RINGS(bp) \
+	(RTE_MIN((bp)->max_tx_rings, BNXT_MAX_RX_RINGS(bp)))
 
+#define BNXT_MAX_RINGS(bp) \
+	(RTE_MIN((((bp)->max_cp_rings - BNXT_NUM_ASYNC_CPR(bp)) / 2U), \
+		 BNXT_MAX_TX_RINGS(bp)))
 	uint16_t		max_nq_rings;
 	uint16_t		max_l2_ctx;
 	uint16_t		max_rx_em_flows;
-- 
2.10.1


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

* [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes
  2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
                   ` (9 preceding siblings ...)
  2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 10/10] net/bnxt: fix to cap max rings to minimum of compl rings and stat contexts Kalesh A P
@ 2020-01-14  5:14 ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure Ajit Khaparde
                     ` (10 more replies)
  10 siblings, 11 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

v1->v2: fixed the correct the commit ids in Fixes tag.
v2->v3: shorten commit log and update commit ids for Fixes tag.

Note that Fixes tag in patch 07/10 is correct.

Kalesh AP (8):
  net/bnxt: handle flow create failure
  net/bnxt: fix probe failure in FreeBSD
  net/bnxt: fix to use correct IOVA mapping
  net/bnxt: fix enable/disable VLAN filtering
  net/bnxt: fix VLAN strip support
  net/bnxt: handle HW filter setting when port is stopped
  net/bnxt: fix a memory leak in port stop
  net/bnxt: use macro for PCI log format

Somnath Kotur (2):
  net/bnxt: release port upon close
  net/bnxt: fix calculation of max rings

 drivers/net/bnxt/bnxt.h        |  21 +-
 drivers/net/bnxt/bnxt_ethdev.c | 422 ++++++++++++++++++---------------
 drivers/net/bnxt/bnxt_filter.c |   5 +-
 drivers/net/bnxt/bnxt_flow.c   |   4 +-
 drivers/net/bnxt/bnxt_hwrm.c   |  28 +--
 drivers/net/bnxt/bnxt_irq.c    |   8 +
 drivers/net/bnxt/bnxt_ring.c   |  17 --
 drivers/net/bnxt/bnxt_vnic.c   |  11 -
 8 files changed, 273 insertions(+), 243 deletions(-)

-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14 12:59     ` Ferruh Yigit
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 02/10] net/bnxt: fix probe failure in FreeBSD Ajit Khaparde
                     ` (9 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP

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

If flow create fails due to not enough filter resources,
driver does not populate the rte_flow_error using
rte_flow_error_set().

Since "rte_errno" could have garbage value and is not relaiable,
it could cause a segfault in the stack in port_flow_complain().

Fix it to set rte_flow_error using rte_flow_error_set()
when flow create fails due to not enough filter resources.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index cde1fa41c..5564c5363 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1702,7 +1702,9 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 
 	filter = bnxt_get_unused_filter(bp);
 	if (filter == NULL) {
-		PMD_DRV_LOG(ERR, "Not enough resources for a new flow.\n");
+		rte_flow_error_set(error, ENOSPC,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Not enough resources for a new flow");
 		goto free_flow;
 	}
 
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 02/10] net/bnxt: fix probe failure in FreeBSD
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 03/10] net/bnxt: fix to use correct IOVA mapping Ajit Khaparde
                     ` (8 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, Kalesh AP, Somnath Kotur, Santoshkumar Karanappa Rastapur

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

In FreeBSD environment, nic_uio driver does not support interrupts
and rte_intr_callback_register() will fail to register interrupts
which in turn causes bnxt driver probe failure.

Fixed driver to ignore interrupt callback failures in FreeBSD.
Also fixed to not use a dedicated completion ring for async events
from FW and process these events on RXQ0 in FreeBSD.

Fixes: 6de4c538b393 ("net/bnxt: fix error handling in port start")
Fixes: 43f78b380f89 ("net/bnxt: retry IRQ callback deregistration")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        | 8 ++++++++
 drivers/net/bnxt/bnxt_ethdev.c | 3 +++
 drivers/net/bnxt/bnxt_irq.c    | 8 ++++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 21ca059b8..3487b917e 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -117,6 +117,14 @@
 #define BNXT_NUM_ASYNC_CPR(bp) 1
 #endif
 
+/* In FreeBSD OS, nic_uio driver does not support interrupts */
+#ifdef RTE_EXEC_ENV_FREEBSD
+#ifdef BNXT_NUM_ASYNC_CPR
+#undef BNXT_NUM_ASYNC_CPR
+#endif
+#define BNXT_NUM_ASYNC_CPR(bp)	0
+#endif
+
 #define BNXT_MISC_VEC_ID               RTE_INTR_VEC_ZERO_OFFSET
 #define BNXT_RX_VEC_START              RTE_INTR_VEC_RXTX_OFFSET
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7b5df9ac1..879ea580f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -439,8 +439,11 @@ static int bnxt_init_chip(struct bnxt *bp)
 
 	/* enable uio/vfio intr/eventfd mapping */
 	rc = rte_intr_enable(intr_handle);
+#ifndef RTE_EXEC_ENV_FREEBSD
+	/* In FreeBSD OS, nic_uio driver does not support interrupts */
 	if (rc)
 		goto err_free;
+#endif
 
 	rc = bnxt_get_hwrm_link_config(bp, &new);
 	if (rc) {
diff --git a/drivers/net/bnxt/bnxt_irq.c b/drivers/net/bnxt/bnxt_irq.c
index 846325ea9..40e1b0c98 100644
--- a/drivers/net/bnxt/bnxt_irq.c
+++ b/drivers/net/bnxt/bnxt_irq.c
@@ -181,5 +181,13 @@ int bnxt_request_int(struct bnxt *bp)
 			irq->requested = 1;
 	}
 
+#ifdef RTE_EXEC_ENV_FREEBSD
+	/**
+	 * In FreeBSD OS, nic_uio does not support interrupts and
+	 * interrupt register callback will fail.
+	 */
+	rc = 0;
+#endif
+
 	return rc;
 }
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 03/10] net/bnxt: fix to use correct IOVA mapping
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 02/10] net/bnxt: fix probe failure in FreeBSD Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 04/10] net/bnxt: fix enable/disable VLAN filtering Ajit Khaparde
                     ` (7 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, stable, Somnath Kotur

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

Use rte_malloc_virt2iova() to obtain the IO address of a
virtual address obtained through rte_malloc().

Fixed to use the iova address returned by rte_memzone_reserve_aligned()
as the call always returns with populating "mz->iova" with
rte_malloc_virt2iova(mz->addr).

Removed redundant rte_mem_lock_page() call to lock the pages.

Fixes: f55e12f33416 ("net/bnxt: support extended port counters")
Cc: stable@dpdk.org

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 49 ----------------------------------
 drivers/net/bnxt/bnxt_hwrm.c   | 28 ++++++++-----------
 drivers/net/bnxt/bnxt_ring.c   | 17 ------------
 drivers/net/bnxt/bnxt_vnic.c   | 11 --------
 4 files changed, 11 insertions(+), 94 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 879ea580f..a948c78cb 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4233,18 +4233,6 @@ static int bnxt_alloc_ctx_mem_blk(struct bnxt *bp,
 
 		memset(mz->addr, 0, mz->len);
 		mz_phys_addr = mz->iova;
-		if ((unsigned long)mz->addr == mz_phys_addr) {
-			PMD_DRV_LOG(DEBUG,
-				    "physical address same as virtual\n");
-			PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-			mz_phys_addr = rte_mem_virt2iova(mz->addr);
-			if (mz_phys_addr == RTE_BAD_IOVA) {
-				PMD_DRV_LOG(ERR,
-					"unable to map addr to phys memory\n");
-				return -ENOMEM;
-			}
-		}
-		rte_mem_lock_page(((char *)mz->addr));
 
 		rmem->pg_tbl = mz->addr;
 		rmem->pg_tbl_map = mz_phys_addr;
@@ -4268,22 +4256,8 @@ static int bnxt_alloc_ctx_mem_blk(struct bnxt *bp,
 
 	memset(mz->addr, 0, mz->len);
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		for (sz = 0; sz < mem_size; sz += BNXT_PAGE_SIZE)
-			rte_mem_lock_page(((char *)mz->addr) + sz);
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "unable to map addr to phys memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	for (sz = 0, i = 0; sz < mem_size; sz += BNXT_PAGE_SIZE, i++) {
-		rte_mem_lock_page(((char *)mz->addr) + sz);
 		rmem->pg_arr[i] = ((char *)mz->addr) + sz;
 		rmem->dma_arr[i] = mz_phys_addr + sz;
 
@@ -4460,18 +4434,6 @@ static int bnxt_alloc_stats_mem(struct bnxt *bp)
 	}
 	memset(mz->addr, 0, mz->len);
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG,
-			    "Using rte_mem_virt2iova()\n");
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "Can't map address to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	bp->rx_mem_zone = (const void *)mz;
 	bp->hw_rx_port_stats = mz->addr;
@@ -4498,17 +4460,6 @@ static int bnxt_alloc_stats_mem(struct bnxt *bp)
 	}
 	memset(mz->addr, 0, mz->len);
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "Can't map address to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	bp->tx_mem_zone = (const void *)mz;
 	bp->hw_tx_port_stats = mz->addr;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 50272dcf7..3b013396b 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -309,8 +309,8 @@ int bnxt_hwrm_cfa_l2_set_rx_mask(struct bnxt *bp,
 	if (vlan_table) {
 		if (!(mask & HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_VLAN_NONVLAN))
 			mask |= HWRM_CFA_L2_SET_RX_MASK_INPUT_MASK_VLANONLY;
-		req.vlan_tag_tbl_addr = rte_cpu_to_le_64(
-			 rte_mem_virt2iova(vlan_table));
+		req.vlan_tag_tbl_addr =
+			rte_cpu_to_le_64(rte_malloc_virt2iova(vlan_table));
 		req.num_vlan_tags = rte_cpu_to_le_32((uint32_t)vlan_count);
 	}
 	req.mask = rte_cpu_to_le_32(mask);
@@ -351,7 +351,7 @@ int bnxt_hwrm_cfa_vlan_antispoof_cfg(struct bnxt *bp, uint16_t fid,
 	req.fid = rte_cpu_to_le_16(fid);
 
 	req.vlan_tag_mask_tbl_addr =
-		rte_cpu_to_le_64(rte_mem_virt2iova(vlan_table));
+		rte_cpu_to_le_64(rte_malloc_virt2iova(vlan_table));
 	req.num_vlan_entries = rte_cpu_to_le_32((uint32_t)vlan_count);
 
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
@@ -1024,9 +1024,8 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto error;
 		}
-		rte_mem_lock_page(bp->hwrm_cmd_resp_addr);
 		bp->hwrm_cmd_resp_dma_addr =
-			rte_mem_virt2iova(bp->hwrm_cmd_resp_addr);
+			rte_malloc_virt2iova(bp->hwrm_cmd_resp_addr);
 		if (bp->hwrm_cmd_resp_dma_addr == RTE_BAD_IOVA) {
 			PMD_DRV_LOG(ERR,
 			"Unable to map response buffer to physical memory.\n");
@@ -1061,9 +1060,8 @@ int bnxt_hwrm_ver_get(struct bnxt *bp)
 			rc = -ENOMEM;
 			goto error;
 		}
-		rte_mem_lock_page(bp->hwrm_short_cmd_req_addr);
 		bp->hwrm_short_cmd_req_dma_addr =
-			rte_mem_virt2iova(bp->hwrm_short_cmd_req_addr);
+			rte_malloc_virt2iova(bp->hwrm_short_cmd_req_addr);
 		if (bp->hwrm_short_cmd_req_dma_addr == RTE_BAD_IOVA) {
 			rte_free(bp->hwrm_short_cmd_req_addr);
 			PMD_DRV_LOG(ERR,
@@ -2471,11 +2469,10 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	bp->max_resp_len = HWRM_MAX_RESP_LEN;
 	bp->hwrm_cmd_resp_addr = rte_malloc(type, bp->max_resp_len, 0);
-	rte_mem_lock_page(bp->hwrm_cmd_resp_addr);
 	if (bp->hwrm_cmd_resp_addr == NULL)
 		return -ENOMEM;
 	bp->hwrm_cmd_resp_dma_addr =
-		rte_mem_virt2iova(bp->hwrm_cmd_resp_addr);
+		rte_malloc_virt2iova(bp->hwrm_cmd_resp_addr);
 	if (bp->hwrm_cmd_resp_dma_addr == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -3421,7 +3418,7 @@ int bnxt_hwrm_func_buf_rgtr(struct bnxt *bp)
 			 page_getenum(bp->pf.active_vfs * HWRM_MAX_REQ_LEN));
 	req.req_buf_len = rte_cpu_to_le_16(HWRM_MAX_REQ_LEN);
 	req.req_buf_page_addr0 =
-		rte_cpu_to_le_64(rte_mem_virt2iova(bp->pf.vf_req_buf));
+		rte_cpu_to_le_64(rte_malloc_virt2iova(bp->pf.vf_req_buf));
 	if (req.req_buf_page_addr0 == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map buffer address to physical memory\n");
@@ -3851,10 +3848,9 @@ int bnxt_get_nvram_directory(struct bnxt *bp, uint32_t len, uint8_t *data)
 
 	buflen = dir_entries * entry_length;
 	buf = rte_malloc("nvm_dir", buflen, 0);
-	rte_mem_lock_page(buf);
 	if (buf == NULL)
 		return -ENOMEM;
-	dma_handle = rte_mem_virt2iova(buf);
+	dma_handle = rte_malloc_virt2iova(buf);
 	if (dma_handle == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -3885,11 +3881,10 @@ int bnxt_hwrm_get_nvram_item(struct bnxt *bp, uint32_t index,
 	struct hwrm_nvm_read_output *resp = bp->hwrm_cmd_resp_addr;
 
 	buf = rte_malloc("nvm_item", length, 0);
-	rte_mem_lock_page(buf);
 	if (!buf)
 		return -ENOMEM;
 
-	dma_handle = rte_mem_virt2iova(buf);
+	dma_handle = rte_malloc_virt2iova(buf);
 	if (dma_handle == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -3939,11 +3934,10 @@ int bnxt_hwrm_flash_nvram(struct bnxt *bp, uint16_t dir_type,
 	uint8_t *buf;
 
 	buf = rte_malloc("nvm_write", data_len, 0);
-	rte_mem_lock_page(buf);
 	if (!buf)
 		return -ENOMEM;
 
-	dma_handle = rte_mem_virt2iova(buf);
+	dma_handle = rte_malloc_virt2iova(buf);
 	if (dma_handle == RTE_BAD_IOVA) {
 		PMD_DRV_LOG(ERR,
 			"unable to map response address to physical memory\n");
@@ -4006,7 +4000,7 @@ static int bnxt_hwrm_func_vf_vnic_query(struct bnxt *bp, uint16_t vf,
 
 	req.vf_id = rte_cpu_to_le_16(bp->pf.first_vf_id + vf);
 	req.max_vnic_id_cnt = rte_cpu_to_le_32(bp->pf.total_vnics);
-	req.vnic_id_tbl_addr = rte_cpu_to_le_64(rte_mem_virt2iova(vnic_ids));
+	req.vnic_id_tbl_addr = rte_cpu_to_le_64(rte_malloc_virt2iova(vnic_ids));
 
 	if (req.vnic_id_tbl_addr == RTE_BAD_IOVA) {
 		HWRM_UNLOCK();
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index ea46fa9bc..d6e4e8a28 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -110,9 +110,7 @@ int bnxt_alloc_rings(struct bnxt *bp, uint16_t qidx,
 	uint64_t rx_offloads = bp->eth_dev->data->dev_conf.rxmode.offloads;
 	const struct rte_memzone *mz = NULL;
 	char mz_name[RTE_MEMZONE_NAMESIZE];
-	rte_iova_t mz_phys_addr_base;
 	rte_iova_t mz_phys_addr;
-	int sz;
 
 	int stats_len = (tx_ring_info || rx_ring_info) ?
 	    RTE_CACHE_LINE_ROUNDUP(sizeof(struct hwrm_stat_ctx_query_output) -
@@ -214,22 +212,7 @@ int bnxt_alloc_rings(struct bnxt *bp, uint16_t qidx,
 			return -ENOMEM;
 	}
 	memset(mz->addr, 0, mz->len);
-	mz_phys_addr_base = mz->iova;
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr_base) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		for (sz = 0; sz < total_alloc_len; sz += getpagesize())
-			rte_mem_lock_page(((char *)mz->addr) + sz);
-		mz_phys_addr_base = rte_mem_virt2iova(mz->addr);
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-			"unable to map ring address to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	if (tx_ring_info) {
 		txq->mz = mz;
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index 104342e13..bc054a8e0 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -150,17 +150,6 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
 			return -ENOMEM;
 	}
 	mz_phys_addr = mz->iova;
-	if ((unsigned long)mz->addr == mz_phys_addr) {
-		PMD_DRV_LOG(DEBUG,
-			    "Memzone physical address same as virtual.\n");
-		PMD_DRV_LOG(DEBUG, "Using rte_mem_virt2iova()\n");
-		mz_phys_addr = rte_mem_virt2iova(mz->addr);
-		if (mz_phys_addr == RTE_BAD_IOVA) {
-			PMD_DRV_LOG(ERR,
-				    "unable to map to physical memory\n");
-			return -ENOMEM;
-		}
-	}
 
 	for (i = 0; i < max_vnics; i++) {
 		vnic = &bp->vnic_info[i];
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 04/10] net/bnxt: fix enable/disable VLAN filtering
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (2 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 03/10] net/bnxt: fix to use correct IOVA mapping Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 05/10] net/bnxt: fix VLAN strip support Ajit Khaparde
                     ` (6 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, stable, Somnath Kotur

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

There is no condition check for the user requested operation
for VLAN filtering. As a result, VLAN filtering is getting disabled
when the user enables/disables VLAN stripping on same port.

The function bnxt_hwrm_clear_l2_filter() didn't actually free
L2 filter in HW if the reference count of filter is zero.

Fixed it by incrementing the reference count of filter in
bnxt_alloc_filter() routine.

Because of the recent changes in bnxt_hwrm_clear_l2_filter(),
change was needed in the routine bnxt_set_default_mac_addr_op()
to destroy and re-create the default filter when the user
changes the default MAC of the port.

Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Fixes: 6118503d8071 ("net/bnxt: fix VLAN filtering")
Cc: stable@dpdk.org

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 66 ++++++++++++++++++----------------
 drivers/net/bnxt/bnxt_filter.c |  5 +--
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index a948c78cb..83f475d49 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1860,18 +1860,12 @@ static int bnxt_del_dflt_mac_filter(struct bnxt *bp,
 }
 
 static int
-bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
+bnxt_config_vlan_hw_filter(struct bnxt *bp, uint64_t rx_offloads)
 {
-	struct bnxt *bp = dev->data->dev_private;
-	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
 	struct bnxt_vnic_info *vnic;
 	unsigned int i;
 	int rc;
 
-	rc = is_bnxt_in_error(bp);
-	if (rc)
-		return rc;
-
 	vnic = BNXT_GET_DEFAULT_VNIC(bp);
 	if (!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
 		/* Remove any VLAN filters programmed */
@@ -1895,6 +1889,28 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	PMD_DRV_LOG(DEBUG, "VLAN Filtering: %d\n",
 		    !!(rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER));
 
+	return 0;
+}
+
+static int
+bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
+{
+	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	struct bnxt *bp = dev->data->dev_private;
+	unsigned int i;
+	int rc;
+
+	rc = is_bnxt_in_error(bp);
+	if (rc)
+		return rc;
+
+	if (mask & ETH_VLAN_FILTER_MASK) {
+		/* Enable or disable VLAN filtering */
+		rc = bnxt_config_vlan_hw_filter(bp, rx_offloads);
+		if (rc)
+			return rc;
+	}
+
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
 		for (i = 0; i < bp->nr_vnics; i++) {
@@ -1984,7 +2000,6 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 	struct bnxt *bp = dev->data->dev_private;
 	/* Default Filter is tied to VNIC 0 */
 	struct bnxt_vnic_info *vnic = BNXT_GET_DEFAULT_VNIC(bp);
-	struct bnxt_filter_info *filter;
 	int rc;
 
 	rc = is_bnxt_in_error(bp);
@@ -1997,32 +2012,23 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev,
 	if (rte_is_zero_ether_addr(addr))
 		return -EINVAL;
 
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
-		/* Default Filter is at Index 0 */
-		if (filter->mac_index != 0)
-			continue;
-
-		memcpy(filter->l2_addr, addr, RTE_ETHER_ADDR_LEN);
-		memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
-		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX |
-			HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_OUTERMOST;
-		filter->enables |=
-			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
-			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
+	/* Check if the requested MAC is already added */
+	if (memcmp(addr, bp->mac_addr, RTE_ETHER_ADDR_LEN) == 0)
+		return 0;
 
-		rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
-		if (rc) {
-			memcpy(filter->l2_addr, bp->mac_addr,
-			       RTE_ETHER_ADDR_LEN);
-			return rc;
-		}
+	/* Destroy filter and re-create it */
+	bnxt_del_dflt_mac_filter(bp, vnic);
 
-		memcpy(bp->mac_addr, addr, RTE_ETHER_ADDR_LEN);
-		PMD_DRV_LOG(DEBUG, "Set MAC addr\n");
-		return 0;
+	memcpy(bp->mac_addr, addr, RTE_ETHER_ADDR_LEN);
+	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_VLAN_FILTER) {
+		/* This filter will allow only untagged packets */
+		rc = bnxt_add_vlan_filter(bp, 0);
+	} else {
+		rc = bnxt_add_mac_filter(bp, vnic, addr, 0, 0);
 	}
 
-	return 0;
+	PMD_DRV_LOG(DEBUG, "Set MAC addr\n");
+	return rc;
 }
 
 static int
diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 0c410f8ba..b31f10479 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -39,9 +39,10 @@ struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp)
 	filter->flags = HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
 	filter->enables = HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR |
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
-	memcpy(filter->l2_addr, bp->eth_dev->data->mac_addrs->addr_bytes,
-	       RTE_ETHER_ADDR_LEN);
+	memcpy(filter->l2_addr, bp->mac_addr, RTE_ETHER_ADDR_LEN);
 	memset(filter->l2_addr_mask, 0xff, RTE_ETHER_ADDR_LEN);
+	/* bump up the reference count of filter */
+	filter->l2_ref_cnt++;
 	return filter;
 }
 
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 05/10] net/bnxt: fix VLAN strip support
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (3 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 04/10] net/bnxt: fix enable/disable VLAN filtering Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 06/10] net/bnxt: handle HW filter setting when port is stopped Ajit Khaparde
                     ` (5 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, Somnath Kotur

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

HWRM_VNIC_CFG command to configure vnic dynamically with
traffic running is not working. Driver has to free and
recreate the vnic and then reconfigure the vnic filters.

Fixes: 7fe5668d2ea3 ("net/bnxt: support VLAN filter and strip")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 248 ++++++++++++++++++++-------------
 1 file changed, 150 insertions(+), 98 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 83f475d49..0b2c29bc8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -132,6 +132,7 @@ static int bnxt_dev_uninit(struct rte_eth_dev *eth_dev);
 static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev);
 static int bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev);
 static void bnxt_cancel_fw_health_check(struct bnxt *bp);
+static int bnxt_restore_vlan_filters(struct bnxt *bp);
 
 int is_bnxt_in_error(struct bnxt *bp)
 {
@@ -228,14 +229,97 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool reconfig)
 	return rc;
 }
 
-static int bnxt_init_chip(struct bnxt *bp)
+static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id)
 {
+	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
+	struct bnxt_vnic_info *vnic = &bp->vnic_info[vnic_id];
+	uint64_t rx_offloads = dev_conf->rxmode.offloads;
 	struct bnxt_rx_queue *rxq;
+	unsigned int j;
+	int rc;
+
+	rc = bnxt_vnic_grp_alloc(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	PMD_DRV_LOG(DEBUG, "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
+		    vnic_id, vnic, vnic->fw_grp_ids);
+
+	rc = bnxt_hwrm_vnic_alloc(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	/* Alloc RSS context only if RSS mode is enabled */
+	if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) {
+		int j, nr_ctxs = bnxt_rss_ctxts(bp);
+
+		rc = 0;
+		for (j = 0; j < nr_ctxs; j++) {
+			rc = bnxt_hwrm_vnic_ctx_alloc(bp, vnic, j);
+			if (rc)
+				break;
+		}
+		if (rc) {
+			PMD_DRV_LOG(ERR,
+				    "HWRM vnic %d ctx %d alloc failure rc: %x\n",
+				    vnic_id, j, rc);
+			goto err_out;
+		}
+		vnic->num_lb_ctxts = nr_ctxs;
+	}
+
+	/*
+	 * Firmware sets pf pair in default vnic cfg. If the VLAN strip
+	 * setting is not available at this time, it will not be
+	 * configured correctly in the CFA.
+	 */
+	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
+		vnic->vlan_strip = true;
+	else
+		vnic->vlan_strip = false;
+
+	rc = bnxt_hwrm_vnic_cfg(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	rc = bnxt_set_hwrm_vnic_filters(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	for (j = 0; j < bp->rx_num_qs_per_vnic; j++) {
+		rxq = bp->eth_dev->data->rx_queues[j];
+
+		PMD_DRV_LOG(DEBUG,
+			    "rxq[%d]->vnic=%p vnic->fw_grp_ids=%p\n",
+			    j, rxq->vnic, rxq->vnic->fw_grp_ids);
+
+		if (BNXT_HAS_RING_GRPS(bp) && rxq->rx_deferred_start)
+			rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
+	}
+
+	rc = bnxt_vnic_rss_configure(bp, vnic);
+	if (rc)
+		goto err_out;
+
+	bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);
+
+	if (rx_offloads & DEV_RX_OFFLOAD_TCP_LRO)
+		bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 1);
+	else
+		bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 0);
+
+	return 0;
+err_out:
+	PMD_DRV_LOG(ERR, "HWRM vnic %d cfg failure rc: %x\n",
+		    vnic_id, rc);
+	return rc;
+}
+
+static int bnxt_init_chip(struct bnxt *bp)
+{
 	struct rte_eth_link new;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(bp->eth_dev);
-	struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
-	uint64_t rx_offloads = dev_conf->rxmode.offloads;
 	uint32_t intr_vector = 0;
 	uint32_t queue_id, base = BNXT_MISC_VEC_ID;
 	uint32_t vec = BNXT_MISC_VEC_ID;
@@ -303,93 +387,11 @@ static int bnxt_init_chip(struct bnxt *bp)
 
 	/* VNIC configuration */
 	for (i = 0; i < bp->nr_vnics; i++) {
-		struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
-		struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
-
-		rc = bnxt_vnic_grp_alloc(bp, vnic);
+		rc = bnxt_setup_one_vnic(bp, i);
 		if (rc)
 			goto err_out;
-
-		PMD_DRV_LOG(DEBUG, "vnic[%d] = %p vnic->fw_grp_ids = %p\n",
-			    i, vnic, vnic->fw_grp_ids);
-
-		rc = bnxt_hwrm_vnic_alloc(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR, "HWRM vnic %d alloc failure rc: %x\n",
-				i, rc);
-			goto err_out;
-		}
-
-		/* Alloc RSS context only if RSS mode is enabled */
-		if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) {
-			int j, nr_ctxs = bnxt_rss_ctxts(bp);
-
-			rc = 0;
-			for (j = 0; j < nr_ctxs; j++) {
-				rc = bnxt_hwrm_vnic_ctx_alloc(bp, vnic, j);
-				if (rc)
-					break;
-			}
-			if (rc) {
-				PMD_DRV_LOG(ERR,
-				  "HWRM vnic %d ctx %d alloc failure rc: %x\n",
-				  i, j, rc);
-				goto err_out;
-			}
-			vnic->num_lb_ctxts = nr_ctxs;
-		}
-
-		/*
-		 * Firmware sets pf pair in default vnic cfg. If the VLAN strip
-		 * setting is not available at this time, it will not be
-		 * configured correctly in the CFA.
-		 */
-		if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
-			vnic->vlan_strip = true;
-		else
-			vnic->vlan_strip = false;
-
-		rc = bnxt_hwrm_vnic_cfg(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR, "HWRM vnic %d cfg failure rc: %x\n",
-				i, rc);
-			goto err_out;
-		}
-
-		rc = bnxt_set_hwrm_vnic_filters(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR,
-				"HWRM vnic %d filter failure rc: %x\n",
-				i, rc);
-			goto err_out;
-		}
-
-		for (j = 0; j < bp->rx_num_qs_per_vnic; j++) {
-			rxq = bp->eth_dev->data->rx_queues[j];
-
-			PMD_DRV_LOG(DEBUG,
-				    "rxq[%d]->vnic=%p vnic->fw_grp_ids=%p\n",
-				    j, rxq->vnic, rxq->vnic->fw_grp_ids);
-
-			if (BNXT_HAS_RING_GRPS(bp) && rxq->rx_deferred_start)
-				rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
-		}
-
-		rc = bnxt_vnic_rss_configure(bp, vnic);
-		if (rc) {
-			PMD_DRV_LOG(ERR,
-				    "HWRM vnic set RSS failure rc: %x\n", rc);
-			goto err_out;
-		}
-
-		bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);
-
-		if (bp->eth_dev->data->dev_conf.rxmode.offloads &
-		    DEV_RX_OFFLOAD_TCP_LRO)
-			bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 1);
-		else
-			bnxt_hwrm_vnic_tpa_cfg(bp, vnic, 0);
 	}
+
 	rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, &bp->vnic_info[0], 0, NULL);
 	if (rc) {
 		PMD_DRV_LOG(ERR,
@@ -1892,12 +1894,69 @@ bnxt_config_vlan_hw_filter(struct bnxt *bp, uint64_t rx_offloads)
 	return 0;
 }
 
+static int bnxt_free_one_vnic(struct bnxt *bp, uint16_t vnic_id)
+{
+	struct bnxt_vnic_info *vnic = &bp->vnic_info[vnic_id];
+	unsigned int i;
+	int rc;
+
+	/* Destroy vnic filters and vnic */
+	if (bp->eth_dev->data->dev_conf.rxmode.offloads &
+	    DEV_RX_OFFLOAD_VLAN_FILTER) {
+		for (i = 0; i < RTE_ETHER_MAX_VLAN_ID; i++)
+			bnxt_del_vlan_filter(bp, i);
+	}
+	bnxt_del_dflt_mac_filter(bp, vnic);
+
+	rc = bnxt_hwrm_vnic_free(bp, vnic);
+	if (rc)
+		return rc;
+
+	rte_free(vnic->fw_grp_ids);
+	vnic->fw_grp_ids = NULL;
+
+	return 0;
+}
+
+static int
+bnxt_config_vlan_hw_stripping(struct bnxt *bp, uint64_t rx_offloads)
+{
+	struct bnxt_vnic_info *vnic = BNXT_GET_DEFAULT_VNIC(bp);
+	int rc;
+
+	/* Destroy, recreate and reconfigure the default vnic */
+	rc = bnxt_free_one_vnic(bp, 0);
+	if (rc)
+		return rc;
+
+	/* default vnic 0 */
+	rc = bnxt_setup_one_vnic(bp, 0);
+	if (rc)
+		return rc;
+
+	if (bp->eth_dev->data->dev_conf.rxmode.offloads &
+	    DEV_RX_OFFLOAD_VLAN_FILTER) {
+		rc = bnxt_add_vlan_filter(bp, 0);
+		bnxt_restore_vlan_filters(bp);
+	} else {
+		rc = bnxt_add_mac_filter(bp, vnic, NULL, 0, 0);
+	}
+
+	rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL);
+	if (rc)
+		return rc;
+
+	PMD_DRV_LOG(DEBUG, "VLAN Strip Offload: %d\n",
+		    !!(rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP));
+
+	return rc;
+}
+
 static int
 bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 {
 	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
 	struct bnxt *bp = dev->data->dev_private;
-	unsigned int i;
 	int rc;
 
 	rc = is_bnxt_in_error(bp);
@@ -1913,16 +1972,9 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		/* Enable or disable VLAN stripping */
-		for (i = 0; i < bp->nr_vnics; i++) {
-			struct bnxt_vnic_info *vnic = &bp->vnic_info[i];
-			if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
-				vnic->vlan_strip = true;
-			else
-				vnic->vlan_strip = false;
-			bnxt_hwrm_vnic_cfg(bp, vnic);
-		}
-		PMD_DRV_LOG(DEBUG, "VLAN Strip Offload: %d\n",
-			!!(rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP));
+		rc = bnxt_config_vlan_hw_stripping(bp, rx_offloads);
+		if (rc)
+			return rc;
 	}
 
 	if (mask & ETH_VLAN_EXTEND_MASK) {
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 06/10] net/bnxt: handle HW filter setting when port is stopped
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (4 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 05/10] net/bnxt: fix VLAN strip support Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 07/10] net/bnxt: fix a memory leak in port stop Ajit Khaparde
                     ` (4 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, Venkat Duvvuru

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

Driver destroy the vnic when the port is brought down.
Port hw filter setting such as promiscuos, allmulti and
vlan filtering will be applied when port is started.

Fixed to return success silently for these callbacks
when port is stopped. Also fixed to clear "bp->dev_stopped"
before invoking bnxt_vlan_offload_set_op() in bnxt_dev_start_op().

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 0b2c29bc8..436ecbb84 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -870,6 +870,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 	eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
 
 	bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
+	bp->dev_stopped = 0;
 
 	if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		vlan_mask |= ETH_VLAN_FILTER_MASK;
@@ -884,7 +885,6 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 
 	bp->flags |= BNXT_FLAG_INIT_DONE;
 	eth_dev->data->dev_started = 1;
-	bp->dev_stopped = 0;
 	pthread_mutex_lock(&bp->def_cp_lock);
 	bnxt_schedule_fw_health_check(bp);
 	pthread_mutex_unlock(&bp->def_cp_lock);
@@ -895,6 +895,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
 	bnxt_shutdown_nic(bp);
 	bnxt_free_tx_mbufs(bp);
 	bnxt_free_rx_mbufs(bp);
+	bp->dev_stopped = 1;
 	return rc;
 }
 
@@ -1168,6 +1169,10 @@ static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1193,6 +1198,10 @@ static int bnxt_promiscuous_disable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1218,6 +1227,10 @@ static int bnxt_allmulticast_enable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1243,6 +1256,10 @@ static int bnxt_allmulticast_disable_op(struct rte_eth_dev *eth_dev)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (bp->vnic_info == NULL)
 		return 0;
 
@@ -1963,6 +1980,10 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	if (rc)
 		return rc;
 
+	/* Filter settings will get applied when port is started */
+	if (bp->dev_stopped == 1)
+		return 0;
+
 	if (mask & ETH_VLAN_FILTER_MASK) {
 		/* Enable or disable VLAN filtering */
 		rc = bnxt_config_vlan_hw_filter(bp, rx_offloads);
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 07/10] net/bnxt: fix a memory leak in port stop
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (5 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 06/10] net/bnxt: handle HW filter setting when port is stopped Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 08/10] net/bnxt: use macro for PCI log format Ajit Khaparde
                     ` (3 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP

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

The memory for mark table is allocated during port start.
But the allocated memory is freed only during port close
or driver unload which in turn causes a memory leakage
on each port start/stop.

Fixed it by moving the memory free to port stop.

Fixes: a968a9f5456d ("net/bnxt: add support for flow mark action")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 436ecbb84..26614186f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -970,7 +970,10 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 	bnxt_int_handler(eth_dev);
 	bnxt_shutdown_nic(bp);
 	bnxt_hwrm_if_change(bp, 0);
-	memset(bp->mark_table, 0, BNXT_MARK_TABLE_SZ);
+
+	rte_free(bp->mark_table);
+	bp->mark_table = NULL;
+
 	bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
 	bp->dev_stopped = 1;
 	bp->rx_cosq_cnt = 0;
@@ -992,9 +995,6 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
 		bp->grp_info = NULL;
 	}
 
-	rte_free(bp->mark_table);
-	bp->mark_table = NULL;
-
 	bnxt_dev_uninit(eth_dev);
 }
 
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 08/10] net/bnxt: use macro for PCI log format
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (6 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 07/10] net/bnxt: fix a memory leak in port stop Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close Ajit Khaparde
                     ` (2 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Kalesh AP, Somnath Kotur

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

Fixes: 19e6af01bb36 ("net/bnxt: support get/set EEPROM")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 26614186f..ba3f0a7d9 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3593,9 +3593,9 @@ bnxt_get_eeprom_length_op(struct rte_eth_dev *dev)
 	if (rc)
 		return rc;
 
-	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x\n",
-		bp->pdev->addr.domain, bp->pdev->addr.bus,
-		bp->pdev->addr.devid, bp->pdev->addr.function);
+	PMD_DRV_LOG(INFO, PCI_PRI_FMT "\n",
+		    bp->pdev->addr.domain, bp->pdev->addr.bus,
+		    bp->pdev->addr.devid, bp->pdev->addr.function);
 
 	rc = bnxt_hwrm_nvm_get_dir_info(bp, &dir_entries, &entry_length);
 	if (rc != 0)
@@ -3617,10 +3617,10 @@ bnxt_get_eeprom_op(struct rte_eth_dev *dev,
 	if (rc)
 		return rc;
 
-	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x in_eeprom->offset = %d "
-		"len = %d\n", bp->pdev->addr.domain,
-		bp->pdev->addr.bus, bp->pdev->addr.devid,
-		bp->pdev->addr.function, in_eeprom->offset, in_eeprom->length);
+	PMD_DRV_LOG(INFO, PCI_PRI_FMT " in_eeprom->offset = %d len = %d\n",
+		    bp->pdev->addr.domain, bp->pdev->addr.bus,
+		    bp->pdev->addr.devid, bp->pdev->addr.function,
+		    in_eeprom->offset, in_eeprom->length);
 
 	if (in_eeprom->offset == 0) /* special offset value to get directory */
 		return bnxt_get_nvram_directory(bp, in_eeprom->length,
@@ -3693,10 +3693,10 @@ bnxt_set_eeprom_op(struct rte_eth_dev *dev,
 	if (rc)
 		return rc;
 
-	PMD_DRV_LOG(INFO, "%04x:%02x:%02x:%02x in_eeprom->offset = %d "
-		"len = %d\n", bp->pdev->addr.domain,
-		bp->pdev->addr.bus, bp->pdev->addr.devid,
-		bp->pdev->addr.function, in_eeprom->offset, in_eeprom->length);
+	PMD_DRV_LOG(INFO, PCI_PRI_FMT " in_eeprom->offset = %d len = %d\n",
+		    bp->pdev->addr.domain, bp->pdev->addr.bus,
+		    bp->pdev->addr.devid, bp->pdev->addr.function,
+		    in_eeprom->offset, in_eeprom->length);
 
 	if (!BNXT_PF(bp)) {
 		PMD_DRV_LOG(ERR, "NVM write not supported from a VF\n");
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (7 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 08/10] net/bnxt: use macro for PCI log format Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14 12:56     ` Ferruh Yigit
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 10/10] net/bnxt: fix calculation of max rings Ajit Khaparde
  2020-01-14  5:27   ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
  10 siblings, 1 reply; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, Kalesh Anakkur Purayil

From: Somnath Kotur <somnath.kotur@broadcom.com>

Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
for the port can be freed by rte_eth_dev_close().

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ba3f0a7d9..8acfade5f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
 	if (rc)
 		goto error_free;
 
+	/* Pass the information to the rte_eth_dev_close() that it should also
+	 * release the private port resources.
+	 */
+	eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
+
 	PMD_DRV_LOG(INFO,
 		    DRV_MODULE_NAME "found at mem %" PRIX64 ", node addr %pM\n",
 		    pci_dev->mem_resource[0].phys_addr,
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH v3 10/10] net/bnxt: fix calculation of max rings
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (8 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close Ajit Khaparde
@ 2020-01-14  5:14   ` Ajit Khaparde
  2020-01-14  5:27   ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
  10 siblings, 0 replies; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:14 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, Somnath Kotur, Kalesh Anakkur Purayil

From: Somnath Kotur <somnath.kotur@broadcom.com>

Max Tx rings count could be lesser than max Rx rings in some
cases, so take this into account as well.

Account for stat contexts available(one for each ring) along with
no: of completion rings(one for each ring) to cap the max no: of
Tx /Rx rings that can be possibly created.

Fixes: f03e66cb ("net/bnxt: limit queue count for NS3/Stingray devices")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 3487b917e..ddb26814c 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -622,12 +622,19 @@ struct bnxt {
 	uint16_t		max_tx_rings;
 	uint16_t		max_rx_rings;
 #define MAX_STINGRAY_RINGS		128U
-#define BNXT_MAX_RINGS(bp) \
+/* For sake of symmetry, max Tx rings == max Rx rings, one stat ctx for each */
+#define BNXT_MAX_RX_RINGS(bp) \
 	(BNXT_STINGRAY(bp) ? RTE_MIN(RTE_MIN(bp->max_rx_rings, \
 					     MAX_STINGRAY_RINGS), \
-				     bp->max_stat_ctx) : \
-				RTE_MIN(bp->max_rx_rings, bp->max_stat_ctx))
+				     bp->max_stat_ctx / 2U) : \
+				RTE_MIN(bp->max_rx_rings, \
+					bp->max_stat_ctx / 2U))
+#define BNXT_MAX_TX_RINGS(bp) \
+	(RTE_MIN((bp)->max_tx_rings, BNXT_MAX_RX_RINGS(bp)))
 
+#define BNXT_MAX_RINGS(bp) \
+	(RTE_MIN((((bp)->max_cp_rings - BNXT_NUM_ASYNC_CPR(bp)) / 2U), \
+		 BNXT_MAX_TX_RINGS(bp)))
 	uint16_t		max_nq_rings;
 	uint16_t		max_l2_ctx;
 	uint16_t		max_rx_em_flows;
-- 
2.21.0 (Apple Git-122.2)


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

* Re: [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes
  2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
                     ` (9 preceding siblings ...)
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 10/10] net/bnxt: fix calculation of max rings Ajit Khaparde
@ 2020-01-14  5:27   ` Ajit Khaparde
  2020-01-14 13:02     ` Ferruh Yigit
  10 siblings, 1 reply; 29+ messages in thread
From: Ajit Khaparde @ 2020-01-14  5:27 UTC (permalink / raw)
  To: dpdk-dev; +Cc: Ferruh Yigit

On Mon, Jan 13, 2020 at 9:16 PM Ajit Khaparde <ajit.khaparde@broadcom.com>
wrote:

> v1->v2: fixed the correct the commit ids in Fixes tag.
> v2->v3: shorten commit log and update commit ids for Fixes tag.
>
> Note that Fixes tag in patch 07/10 is correct.
>
Patches applied to dpdk-next-net-brcm. Thanks


>
> Kalesh AP (8):
>   net/bnxt: handle flow create failure
>   net/bnxt: fix probe failure in FreeBSD
>   net/bnxt: fix to use correct IOVA mapping
>   net/bnxt: fix enable/disable VLAN filtering
>   net/bnxt: fix VLAN strip support
>   net/bnxt: handle HW filter setting when port is stopped
>   net/bnxt: fix a memory leak in port stop
>   net/bnxt: use macro for PCI log format
>
> Somnath Kotur (2):
>   net/bnxt: release port upon close
>   net/bnxt: fix calculation of max rings
>
>  drivers/net/bnxt/bnxt.h        |  21 +-
>  drivers/net/bnxt/bnxt_ethdev.c | 422 ++++++++++++++++++---------------
>  drivers/net/bnxt/bnxt_filter.c |   5 +-
>  drivers/net/bnxt/bnxt_flow.c   |   4 +-
>  drivers/net/bnxt/bnxt_hwrm.c   |  28 +--
>  drivers/net/bnxt/bnxt_irq.c    |   8 +
>  drivers/net/bnxt/bnxt_ring.c   |  17 --
>  drivers/net/bnxt/bnxt_vnic.c   |  11 -
>  8 files changed, 273 insertions(+), 243 deletions(-)
>
> --
> 2.21.0 (Apple Git-122.2)
>
>

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

* Re: [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close Ajit Khaparde
@ 2020-01-14 12:56     ` Ferruh Yigit
  2020-01-14 14:49       ` Somnath Kotur
  0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2020-01-14 12:56 UTC (permalink / raw)
  To: Ajit Khaparde, dev; +Cc: Somnath Kotur, Kalesh Anakkur Purayil

On 1/14/2020 5:14 AM, Ajit Khaparde wrote:
> From: Somnath Kotur <somnath.kotur@broadcom.com>
> 
> Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
> for the port can be freed by rte_eth_dev_close().
> 
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index ba3f0a7d9..8acfade5f 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
>  	if (rc)
>  		goto error_free;
>  
> +	/* Pass the information to the rte_eth_dev_close() that it should also
> +	 * release the private port resources.
> +	 */
> +	eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
> +

When you give this flag, 'rte_eth_dev_close()' will release port resources, it
will memset the 'eth_dev->data'.

In case user first closed the port, later removed it, remove path should be safe
for it. I can see 'bnxt_dev_uninit()' operates on the values of 'eth_dev->data'
which should crash in this case.

Can you please double check the return path, all following should be safe:
- close the port
- remove the port
- close and remove the port

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

* Re: [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure
  2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure Ajit Khaparde
@ 2020-01-14 12:59     ` Ferruh Yigit
  0 siblings, 0 replies; 29+ messages in thread
From: Ferruh Yigit @ 2020-01-14 12:59 UTC (permalink / raw)
  To: Ajit Khaparde, Kalesh AP; +Cc: dev

On 1/14/2020 5:14 AM, Ajit Khaparde wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> If flow create fails due to not enough filter resources,
> driver does not populate the rte_flow_error using
> rte_flow_error_set().
> 
> Since "rte_errno" could have garbage value and is not relaiable,
> it could cause a segfault in the stack in port_flow_complain().
> 
> Fix it to set rte_flow_error using rte_flow_error_set()
> when flow create fails due to not enough filter resources.

Hi Ajit, Kalesh,

In patch title, 'handle' seems means fix, can you please prefer the 'fix' since
it become kind of keyword to define the patch content. This also helps us
missing the "Fixes: " tag as it has been here.

Thanks,
ferruh

> 
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_flow.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
> index cde1fa41c..5564c5363 100644
> --- a/drivers/net/bnxt/bnxt_flow.c
> +++ b/drivers/net/bnxt/bnxt_flow.c
> @@ -1702,7 +1702,9 @@ bnxt_flow_create(struct rte_eth_dev *dev,
>  
>  	filter = bnxt_get_unused_filter(bp);
>  	if (filter == NULL) {
> -		PMD_DRV_LOG(ERR, "Not enough resources for a new flow.\n");
> +		rte_flow_error_set(error, ENOSPC,
> +				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> +				   "Not enough resources for a new flow");
>  		goto free_flow;
>  	}
>  
> 


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

* Re: [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes
  2020-01-14  5:27   ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
@ 2020-01-14 13:02     ` Ferruh Yigit
  0 siblings, 0 replies; 29+ messages in thread
From: Ferruh Yigit @ 2020-01-14 13:02 UTC (permalink / raw)
  To: Ajit Khaparde, dpdk-dev

On 1/14/2020 5:27 AM, Ajit Khaparde wrote:
> 
> 
> On Mon, Jan 13, 2020 at 9:16 PM Ajit Khaparde <ajit.khaparde@broadcom.com
> <mailto:ajit.khaparde@broadcom.com>> wrote:
> 
>     v1->v2: fixed the correct the commit ids in Fixes tag.
>     v2->v3: shorten commit log and update commit ids for Fixes tag.
> 
>     Note that Fixes tag in patch 07/10 is correct.
> 
> Patches applied to dpdk-next-net-brcm. Thanks

Hi Ajit,

There are some patches with fixes lines but not requested the backport [1], are
they explicitly not wanted to be backported or missed the tag?
To prevent this questions, for the cases the commit explicitly not wanted to be
backported can you please put the reasoning for it into the commit log as comment.

Thanks,
ferruh


[1]
Is it candidate for Cc: stable@dpdk.org backport?
        net/bnxt: fix probe failure in FreeBSD
        net/bnxt: fix VLAN strip support
        net/bnxt: use macro for PCI log format
        net/bnxt: fix calculation of max rings


>  
> 
> 
>     Kalesh AP (8):
>       net/bnxt: handle flow create failure
>       net/bnxt: fix probe failure in FreeBSD
>       net/bnxt: fix to use correct IOVA mapping
>       net/bnxt: fix enable/disable VLAN filtering
>       net/bnxt: fix VLAN strip support
>       net/bnxt: handle HW filter setting when port is stopped
>       net/bnxt: fix a memory leak in port stop
>       net/bnxt: use macro for PCI log format
> 
>     Somnath Kotur (2):
>       net/bnxt: release port upon close
>       net/bnxt: fix calculation of max rings
> 
>      drivers/net/bnxt/bnxt.h        |  21 +-
>      drivers/net/bnxt/bnxt_ethdev.c | 422 ++++++++++++++++++---------------
>      drivers/net/bnxt/bnxt_filter.c |   5 +-
>      drivers/net/bnxt/bnxt_flow.c   |   4 +-
>      drivers/net/bnxt/bnxt_hwrm.c   |  28 +--
>      drivers/net/bnxt/bnxt_irq.c    |   8 +
>      drivers/net/bnxt/bnxt_ring.c   |  17 --
>      drivers/net/bnxt/bnxt_vnic.c   |  11 -
>      8 files changed, 273 insertions(+), 243 deletions(-)
> 
>     -- 
>     2.21.0 (Apple Git-122.2)
> 


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

* Re: [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close
  2020-01-14 12:56     ` Ferruh Yigit
@ 2020-01-14 14:49       ` Somnath Kotur
  2020-01-16 11:20         ` Ferruh Yigit
  0 siblings, 1 reply; 29+ messages in thread
From: Somnath Kotur @ 2020-01-14 14:49 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Ajit Khaparde, dev, Kalesh Anakkur Purayil

Ferruh,
Will recheck and revert on

Thanks
Som

On Tue, 14 Jan 2020, 18:26 Ferruh Yigit, <ferruh.yigit@intel.com> wrote:

> On 1/14/2020 5:14 AM, Ajit Khaparde wrote:
> > From: Somnath Kotur <somnath.kotur@broadcom.com>
> >
> > Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
> > for the port can be freed by rte_eth_dev_close().
> >
> > Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> > Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com
> >
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >  drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> > index ba3f0a7d9..8acfade5f 100644
> > --- a/drivers/net/bnxt/bnxt_ethdev.c
> > +++ b/drivers/net/bnxt/bnxt_ethdev.c
> > @@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
> >       if (rc)
> >               goto error_free;
> >
> > +     /* Pass the information to the rte_eth_dev_close() that it should
> also
> > +      * release the private port resources.
> > +      */
> > +     eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
> > +
>
> When you give this flag, 'rte_eth_dev_close()' will release port
> resources, it
> will memset the 'eth_dev->data'.
>
> In case user first closed the port, later removed it, remove path should
> be safe
> for it. I can see 'bnxt_dev_uninit()' operates on the values of
> 'eth_dev->data'
> which should crash in this case.
>
> Can you please double check the return path, all following should be safe:
> - close the port
> - remove the port
> - close and remove the port
>

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

* Re: [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close
  2020-01-14 14:49       ` Somnath Kotur
@ 2020-01-16 11:20         ` Ferruh Yigit
  2020-01-16 11:30           ` Somnath Kotur
  0 siblings, 1 reply; 29+ messages in thread
From: Ferruh Yigit @ 2020-01-16 11:20 UTC (permalink / raw)
  To: Somnath Kotur; +Cc: Ajit Khaparde, dev, Kalesh Anakkur Purayil

On 1/14/2020 2:49 PM, Somnath Kotur wrote:
> Ferruh,
> Will recheck and revert on

Hi Ajit,

this is blocking the next-net-brcm tree to be pulled. Would you want to drop
this patch from your tree and continue with merge and deal with this patch later?
Or if this patch is the critical for the series I can keep waiting.

Thanks,
ferruh

> 
> Thanks
> Som
> 
> On Tue, 14 Jan 2020, 18:26 Ferruh Yigit, <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 1/14/2020 5:14 AM, Ajit Khaparde wrote:
>     > From: Somnath Kotur <somnath.kotur@broadcom.com
>     <mailto:somnath.kotur@broadcom.com>>
>     >
>     > Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
>     > for the port can be freed by rte_eth_dev_close().
>     >
>     > Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com
>     <mailto:somnath.kotur@broadcom.com>>
>     > Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com
>     <mailto:kalesh-anakkur.purayil@broadcom.com>>
>     > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com
>     <mailto:ajit.khaparde@broadcom.com>>
>     > ---
>     >  drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
>     >  1 file changed, 5 insertions(+)
>     >
>     > diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
>     > index ba3f0a7d9..8acfade5f 100644
>     > --- a/drivers/net/bnxt/bnxt_ethdev.c
>     > +++ b/drivers/net/bnxt/bnxt_ethdev.c
>     > @@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
>     >       if (rc)
>     >               goto error_free;
>     > 
>     > +     /* Pass the information to the rte_eth_dev_close() that it should also
>     > +      * release the private port resources.
>     > +      */
>     > +     eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
>     > +
> 
>     When you give this flag, 'rte_eth_dev_close()' will release port resources, it
>     will memset the 'eth_dev->data'.
> 
>     In case user first closed the port, later removed it, remove path should be safe
>     for it. I can see 'bnxt_dev_uninit()' operates on the values of 'eth_dev->data'
>     which should crash in this case.
> 
>     Can you please double check the return path, all following should be safe:
>     - close the port
>     - remove the port
>     - close and remove the port
> 


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

* Re: [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close
  2020-01-16 11:20         ` Ferruh Yigit
@ 2020-01-16 11:30           ` Somnath Kotur
  0 siblings, 0 replies; 29+ messages in thread
From: Somnath Kotur @ 2020-01-16 11:30 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Ajit Khaparde, dev, Kalesh Anakkur Purayil

Ferruh,



On Thu, 16 Jan 2020, 16:50 Ferruh Yigit, <ferruh.yigit@intel.com> wrote:

> On 1/14/2020 2:49 PM, Somnath Kotur wrote:
> > Ferruh,
> > Will recheck and revert on
>
> Hi Ajit,
>
> this is blocking the next-net-brcm tree to be pulled. Would you want to
> drop
> this patch from your tree and continue with merge and deal with this patch
> later?
> Or if this patch is the critical for the series I can keep waiting.
>
Kalesh will be sending v4 of the this patch set addressing this patch as
well shortly ..few hours full now

Thanks
Som

>
> Thanks,
> ferruh
>
> >
> > Thanks
> > Som
> >
> > On Tue, 14 Jan 2020, 18:26 Ferruh Yigit, <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> >     On 1/14/2020 5:14 AM, Ajit Khaparde wrote:
> >     > From: Somnath Kotur <somnath.kotur@broadcom.com
> >     <mailto:somnath.kotur@broadcom.com>>
> >     >
> >     > Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private
> resources
> >     > for the port can be freed by rte_eth_dev_close().
> >     >
> >     > Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com
> >     <mailto:somnath.kotur@broadcom.com>>
> >     > Reviewed-by: Kalesh Anakkur Purayil <
> kalesh-anakkur.purayil@broadcom.com
> >     <mailto:kalesh-anakkur.purayil@broadcom.com>>
> >     > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com
> >     <mailto:ajit.khaparde@broadcom.com>>
> >     > ---
> >     >  drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
> >     >  1 file changed, 5 insertions(+)
> >     >
> >     > diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> >     > index ba3f0a7d9..8acfade5f 100644
> >     > --- a/drivers/net/bnxt/bnxt_ethdev.c
> >     > +++ b/drivers/net/bnxt/bnxt_ethdev.c
> >     > @@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
> >     >       if (rc)
> >     >               goto error_free;
> >     >
> >     > +     /* Pass the information to the rte_eth_dev_close() that it
> should also
> >     > +      * release the private port resources.
> >     > +      */
> >     > +     eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
> >     > +
> >
> >     When you give this flag, 'rte_eth_dev_close()' will release port
> resources, it
> >     will memset the 'eth_dev->data'.
> >
> >     In case user first closed the port, later removed it, remove path
> should be safe
> >     for it. I can see 'bnxt_dev_uninit()' operates on the values of
> 'eth_dev->data'
> >     which should crash in this case.
> >
> >     Can you please double check the return path, all following should be
> safe:
> >     - close the port
> >     - remove the port
> >     - close and remove the port
> >
>
>

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

end of thread, back to index

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13  5:01 [dpdk-dev] [PATCH V2 00/10] bnxt patchset with bug fixes Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 01/10] net/bnxt: handle flow create failure Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 02/10] net/bnxt: fix probe failure in FreeBSD Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 03/10] net/bnxt: fix to use correct IOVA mapping Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 04/10] net/bnxt: fix enable/disable VLAN filtering Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 05/10] net/bnxt: fix enable/disable vlan strip Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 06/10] net/bnxt: handle hw filter setting when port is stopped Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 07/10] net/bnxt: fix a memory leak in port stop Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 08/10] net/bnxt: use macro for PCI log format Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 09/10] net/bnxt: release port upon close Kalesh A P
2020-01-13  5:01 ` [dpdk-dev] [PATCH V2 10/10] net/bnxt: fix to cap max rings to minimum of compl rings and stat contexts Kalesh A P
2020-01-14  5:14 ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 01/10] net/bnxt: handle flow create failure Ajit Khaparde
2020-01-14 12:59     ` Ferruh Yigit
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 02/10] net/bnxt: fix probe failure in FreeBSD Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 03/10] net/bnxt: fix to use correct IOVA mapping Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 04/10] net/bnxt: fix enable/disable VLAN filtering Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 05/10] net/bnxt: fix VLAN strip support Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 06/10] net/bnxt: handle HW filter setting when port is stopped Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 07/10] net/bnxt: fix a memory leak in port stop Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 08/10] net/bnxt: use macro for PCI log format Ajit Khaparde
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close Ajit Khaparde
2020-01-14 12:56     ` Ferruh Yigit
2020-01-14 14:49       ` Somnath Kotur
2020-01-16 11:20         ` Ferruh Yigit
2020-01-16 11:30           ` Somnath Kotur
2020-01-14  5:14   ` [dpdk-dev] [PATCH v3 10/10] net/bnxt: fix calculation of max rings Ajit Khaparde
2020-01-14  5:27   ` [dpdk-dev] [PATCH v3 00/10] bnxt patchset with bug fixes Ajit Khaparde
2020-01-14 13:02     ` Ferruh Yigit

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox