* [dpdk-dev] [PATCH v2 0/9] bnxt patchset
@ 2020-01-28 7:29 Somnath Kotur
2020-01-28 7:29 ` [dpdk-dev] [PATCH 1/9] net/bnxt: fix bnxt_alloc_filter() to use a common routine Somnath Kotur
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit
Please apply.
Kalesh AP (5):
net/bnxt: remove redundant if statement
net/bnxt: remove redundant macro
net/bnxt: remove unnecessary structure variable
net/bnxt: remove a redundant variable
net/bnxt: fix coverity warnings
Somnath Kotur (4):
net/bnxt: fix bnxt_alloc_filter() to use a common routine
net/bnxt: fix bumping of L2 filter reference count
net/bnxt: fix to allow group ID 0 for RSS action
net/bnxt: fix to support zero mark id along with RSS action
drivers/net/bnxt/bnxt.h | 29 +++++++++++++++++------
drivers/net/bnxt/bnxt_ethdev.c | 49 ++++++++++++++++++++++-----------------
drivers/net/bnxt/bnxt_filter.c | 7 ++----
drivers/net/bnxt/bnxt_flow.c | 52 +++++++++++++++---------------------------
drivers/net/bnxt/bnxt_hwrm.c | 6 +++--
drivers/net/bnxt/bnxt_rxr.c | 25 ++++++++++++--------
drivers/net/bnxt/bnxt_rxr.h | 6 +++--
drivers/net/bnxt/bnxt_stats.c | 4 ++--
8 files changed, 95 insertions(+), 83 deletions(-)
--
v1->v2: Added one more patch to the set ("net/bnxt: fix coverity warnings")
1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread* [dpdk-dev] [PATCH 1/9] net/bnxt: fix bnxt_alloc_filter() to use a common routine 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 2/9] net/bnxt: fix bumping of L2 filter reference count Somnath Kotur ` (8 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit Invoke bnxt_get_unused_filter() inside bnxt_alloc_filter() so that all filters are allocated from one common routine. Fixes: f92735db1e4c ("net/bnxt: add L2 filter alloc/init/free") Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt_filter.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c index b31f104..e218433 100644 --- a/drivers/net/bnxt/bnxt_filter.c +++ b/drivers/net/bnxt/bnxt_filter.c @@ -26,13 +26,11 @@ struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp) { struct bnxt_filter_info *filter; - /* Find the 1st unused filter from the free_filter_list pool*/ - filter = STAILQ_FIRST(&bp->free_filter_list); + filter = bnxt_get_unused_filter(bp); if (!filter) { PMD_DRV_LOG(ERR, "No more free filter resources\n"); return NULL; } - STAILQ_REMOVE_HEAD(&bp->free_filter_list, next); filter->mac_index = INVALID_MAC_INDEX; /* Default to L2 MAC Addr filter */ -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 2/9] net/bnxt: fix bumping of L2 filter reference count 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 1/9] net/bnxt: fix bnxt_alloc_filter() to use a common routine Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 3/9] net/bnxt: fix to allow group ID 0 for RSS action Somnath Kotur ` (7 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit Now that the L2 filter reference count is bumped up in all cases including bnxt_alloc_filter() which is issued in init, just move this ref count bump inside the routine issuing the HWRM cmd so that it is bumped up only if the cmd is successful. Fixes: f0f6b5e6cf94 ("fix reusing L2 filter") Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt_filter.c | 3 +-- drivers/net/bnxt/bnxt_flow.c | 1 - drivers/net/bnxt/bnxt_hwrm.c | 2 ++ 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c index e218433..a1463a0 100644 --- a/drivers/net/bnxt/bnxt_filter.c +++ b/drivers/net/bnxt/bnxt_filter.c @@ -39,8 +39,7 @@ struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp) HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK; 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; } diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c index 5564c53..4b3b597 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -867,7 +867,6 @@ bnxt_free_filter(bp, filter1); return NULL; } - filter1->l2_ref_cnt++; return filter1; } diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 3b01339..460cc48 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -488,6 +488,8 @@ int bnxt_hwrm_set_l2_filter(struct bnxt *bp, filter->flow_id = rte_le_to_cpu_32(resp->flow_id); HWRM_UNLOCK(); + filter->l2_ref_cnt++; + return rc; } -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 3/9] net/bnxt: fix to allow group ID 0 for RSS action 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 1/9] net/bnxt: fix bnxt_alloc_filter() to use a common routine Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 2/9] net/bnxt: fix bumping of L2 filter reference count Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 4/9] net/bnxt: remove redundant if statement Somnath Kotur ` (6 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit Allow RSS action with group ID 0. The RSS match check will ensure if requested RSS action configuration parameters should be allowed as per the VNIC's RSS configuration or not and if it does match as it is by design in OVS-DPDK use case, the default vnic can be used to create the filter. As part of this ensure that rx_queue_cnt for the default vnic is setup during init as this field was being set only for non-zero vnics while handling RSS action for flow create. Check for outofboundds erorr while accessing the vnic array. Fixes: adc0f81c6552d ("net/bnxt: support RSS action") Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt.h | 14 +++++++++++++- drivers/net/bnxt/bnxt_ethdev.c | 4 ++++ drivers/net/bnxt/bnxt_flow.c | 33 ++++----------------------------- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index ddb2681..bca9ad4 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -696,11 +696,23 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete, #define bnxt_release_flow_lock(bp) \ pthread_mutex_unlock(&(bp)->flow_lock) +#define BNXT_VALID_VNIC_OR_RET(bp, vnic_id) do { \ + if ((vnic_id) >= (bp)->max_vnics) { \ + rte_flow_error_set(error, \ + EINVAL, \ + RTE_FLOW_ERROR_TYPE_ATTR_GROUP, \ + NULL, \ + "Group id is invalid!"); \ + rc = -rte_errno; \ + goto ret; \ + } \ +} while (0) + extern int bnxt_logtype_driver; #define PMD_DRV_LOG_RAW(level, fmt, args...) \ rte_log(RTE_LOG_ ## level, bnxt_logtype_driver, "%s(): " fmt, \ __func__, ## args) #define PMD_DRV_LOG(level, fmt, args...) \ - PMD_DRV_LOG_RAW(level, fmt, ## args) + PMD_DRV_LOG_RAW(level, fmt, ## args) #endif diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 2ef1169..fc3f1a8 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -295,8 +295,12 @@ static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id) if (BNXT_HAS_RING_GRPS(bp) && rxq->rx_deferred_start) rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID; + else + vnic->rx_queue_cnt++; } + PMD_DRV_LOG(DEBUG, "vnic->rx_queue_cnt = %d\n", vnic->rx_queue_cnt); + rc = bnxt_vnic_rss_configure(bp, vnic); if (rc) goto err_out; diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c index 4b3b597..bd6c726 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -1059,16 +1059,9 @@ static int match_vnic_rss_cfg(struct bnxt *bp, vnic_id = act_q->index; } + BNXT_VALID_VNIC_OR_RET(bp, vnic_id); + vnic = &bp->vnic_info[vnic_id]; - if (vnic == NULL) { - rte_flow_error_set(error, - EINVAL, - RTE_FLOW_ERROR_TYPE_ACTION, - act, - "No matching VNIC found."); - rc = -rte_errno; - goto ret; - } if (vnic->rx_queue_cnt) { if (vnic->start_grp_id != act_q->index) { PMD_DRV_LOG(ERR, @@ -1268,28 +1261,10 @@ static int match_vnic_rss_cfg(struct bnxt *bp, rss = (const struct rte_flow_action_rss *)act->conf; vnic_id = attr->group; - if (!vnic_id) { - PMD_DRV_LOG(ERR, "Group id cannot be 0\n"); - rte_flow_error_set(error, - EINVAL, - RTE_FLOW_ERROR_TYPE_ATTR, - NULL, - "Group id cannot be 0"); - rc = -rte_errno; - goto ret; - } + + BNXT_VALID_VNIC_OR_RET(bp, vnic_id); vnic = &bp->vnic_info[vnic_id]; - if (vnic == NULL) { - rte_flow_error_set(error, - EINVAL, - RTE_FLOW_ERROR_TYPE_ACTION, - act, - "No matching VNIC for RSS group."); - rc = -rte_errno; - goto ret; - } - PMD_DRV_LOG(DEBUG, "VNIC found\n"); /* Check if requested RSS config matches RSS config of VNIC * only if it is not a fresh VNIC configuration. -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 4/9] net/bnxt: remove redundant if statement 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (2 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 3/9] net/bnxt: fix to allow group ID 0 for RSS action Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 5/9] net/bnxt: remove redundant macro Somnath Kotur ` (5 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Since "eth_dev->data->dev_started" has been assigned to 0 at the beginning of bnxt_dev_stop_op() routine, the code inside the if() condition is redundant. Remove it. Anyways "eth_dev->data->dev_link.link_status" will be set to 0 in bnxt_dev_set_link_down_op() later in the routine. Fixes: 316e412299fd ("net/bnxt: fix crash when closing") Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt_ethdev.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index fc3f1a8..267a361 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -948,10 +948,6 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bnxt_cancel_fw_health_check(bp); bp->flags &= ~BNXT_FLAG_INIT_DONE; - if (bp->eth_dev->data->dev_started) { - /* TBD: STOP HW queues DMA */ - eth_dev->data->dev_link.link_status = 0; - } bnxt_dev_set_link_down_op(eth_dev); /* Wait for link to be reset and the async notification to process. -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 5/9] net/bnxt: remove redundant macro 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (3 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 4/9] net/bnxt: remove redundant if statement Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 6/9] net/bnxt: remove unnecessary structure variable Somnath Kotur ` (4 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Use "dev->data->dev_started" state, instead of local BNXT_FLAG_INIT_DONE to check whether device has been initialised or not. Fixes: ed2ced6fe927 ("net/bnxt: check initialization before accessing stats") Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com> Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt.h | 1 - drivers/net/bnxt/bnxt_ethdev.c | 2 -- drivers/net/bnxt/bnxt_stats.c | 4 ++-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index bca9ad4..95f1f1a 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -520,7 +520,6 @@ struct bnxt { #define BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED BIT(18) #define BNXT_FLAG_EXT_STATS_SUPPORTED BIT(19) #define BNXT_FLAG_NEW_RM BIT(20) -#define BNXT_FLAG_INIT_DONE BIT(21) #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS BIT(22) #define BNXT_FLAG_ADV_FLOW_MGMT BIT(23) #define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 267a361..b04685c 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -887,7 +887,6 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev); eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev); - bp->flags |= BNXT_FLAG_INIT_DONE; eth_dev->data->dev_started = 1; pthread_mutex_lock(&bp->def_cp_lock); bnxt_schedule_fw_health_check(bp); @@ -947,7 +946,6 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bnxt_cancel_fw_health_check(bp); - bp->flags &= ~BNXT_FLAG_INIT_DONE; bnxt_dev_set_link_down_op(eth_dev); /* Wait for link to be reset and the async notification to process. diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c index 32f112b..4668629 100644 --- a/drivers/net/bnxt/bnxt_stats.c +++ b/drivers/net/bnxt/bnxt_stats.c @@ -389,7 +389,7 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev, if (rc) return rc; - if (!(bp->flags & BNXT_FLAG_INIT_DONE)) + if (!eth_dev->data->dev_started) return -EIO; num_q_stats = RTE_MIN(bp->rx_cp_nr_rings, @@ -434,7 +434,7 @@ int bnxt_stats_reset_op(struct rte_eth_dev *eth_dev) if (ret) return ret; - if (!(bp->flags & BNXT_FLAG_INIT_DONE)) { + if (!eth_dev->data->dev_started) { PMD_DRV_LOG(ERR, "Device Initialization not complete!\n"); return -EINVAL; } -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 6/9] net/bnxt: remove unnecessary structure variable 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (4 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 5/9] net/bnxt: remove redundant macro Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 7/9] net/bnxt: remove a redundant variable Somnath Kotur ` (3 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> This change could help in reducing the size of bnxt PMD private data structure by converting a uint8_t variable to use bit map flag. Fixes: 5cd0e2889c432 ("net/bnxt: support NIC Partitioning") Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt.h | 4 ++-- drivers/net/bnxt/bnxt_hwrm.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 95f1f1a..434ce28 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -520,12 +520,13 @@ struct bnxt { #define BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED BIT(18) #define BNXT_FLAG_EXT_STATS_SUPPORTED BIT(19) #define BNXT_FLAG_NEW_RM BIT(20) +#define BNXT_FLAG_NPAR_PF BIT(21) #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS BIT(22) #define BNXT_FLAG_ADV_FLOW_MGMT BIT(23) #define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24) #define BNXT_PF(bp) (!((bp)->flags & BNXT_FLAG_VF)) #define BNXT_VF(bp) ((bp)->flags & BNXT_FLAG_VF) -#define BNXT_NPAR(bp) ((bp)->port_partition_type) +#define BNXT_NPAR(bp) ((bp)->flags & BNXT_FLAG_NPAR_PF) #define BNXT_MH(bp) ((bp)->flags & BNXT_FLAG_MULTI_HOST) #define BNXT_SINGLE_PF(bp) (BNXT_PF(bp) && !BNXT_NPAR(bp) && !BNXT_MH(bp)) #define BNXT_USE_CHIMP_MB 0 //For non-CFA commands, everything uses Chimp. @@ -647,7 +648,6 @@ struct bnxt { #define BNXT_OUTER_TPID_BD_SHFT 16 uint32_t outer_tpid_bd; struct bnxt_pf_info pf; - uint8_t port_partition_type; uint8_t dev_stopped; uint8_t vxlan_port_cnt; uint8_t geneve_port_cnt; diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 460cc48..f325aff 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -2952,10 +2952,10 @@ int bnxt_hwrm_func_qcfg(struct bnxt *bp, uint16_t *mtu) case HWRM_FUNC_QCFG_OUTPUT_PORT_PARTITION_TYPE_NPAR1_5: case HWRM_FUNC_QCFG_OUTPUT_PORT_PARTITION_TYPE_NPAR2_0: /* FALLTHROUGH */ - bp->port_partition_type = resp->port_partition_type; + bp->flags |= BNXT_FLAG_NPAR_PF; break; default: - bp->port_partition_type = 0; + bp->flags &= ~BNXT_FLAG_NPAR_PF; break; } -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 7/9] net/bnxt: remove a redundant variable 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (5 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 6/9] net/bnxt: remove unnecessary structure variable Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 8/9] net/bnxt: fix to support zero mark id along with RSS action Somnath Kotur ` (2 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Use "dev->data->dev_started" state, instead of local "dev_stopped" to check whether port has been started or not. Fixes: 316e412299fd ("net/bnxt: fix crash when closing") Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt.h | 1 - drivers/net/bnxt/bnxt_ethdev.c | 21 +++++++++------------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 434ce28..1a5d542 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -648,7 +648,6 @@ struct bnxt { #define BNXT_OUTER_TPID_BD_SHFT 16 uint32_t outer_tpid_bd; struct bnxt_pf_info pf; - uint8_t dev_stopped; uint8_t vxlan_port_cnt; uint8_t geneve_port_cnt; uint16_t vxlan_port; diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index b04685c..72e5441 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -872,9 +872,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) goto error; eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev); + eth_dev->data->dev_started = 1; 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; @@ -887,7 +887,6 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev); eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev); - eth_dev->data->dev_started = 1; pthread_mutex_lock(&bp->def_cp_lock); bnxt_schedule_fw_health_check(bp); pthread_mutex_unlock(&bp->def_cp_lock); @@ -898,7 +897,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; + eth_dev->data->dev_started = 0; return rc; } @@ -973,7 +972,6 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bp->mark_table = NULL; bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE; - bp->dev_stopped = 1; bp->rx_cosq_cnt = 0; } @@ -981,7 +979,7 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev) { struct bnxt *bp = eth_dev->data->dev_private; - if (bp->dev_stopped == 0) + if (eth_dev->data->dev_started) bnxt_dev_stop_op(eth_dev); bnxt_uninit_resources(bp, false); @@ -1174,7 +1172,7 @@ static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1203,7 +1201,7 @@ static int bnxt_promiscuous_disable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1232,7 +1230,7 @@ static int bnxt_allmulticast_enable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1261,7 +1259,7 @@ static int bnxt_allmulticast_disable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1985,7 +1983,7 @@ static int bnxt_free_one_vnic(struct bnxt *bp, uint16_t vnic_id) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!dev->data->dev_started) return 0; if (mask & ETH_VLAN_FILTER_MASK) { @@ -3887,7 +3885,7 @@ static void bnxt_dev_cleanup(struct bnxt *bp) { bnxt_set_hwrm_link_config(bp, false); bp->link_info.link_up = 0; - if (bp->dev_stopped == 0) + if (bp->eth_dev->data->dev_started) bnxt_dev_stop_op(bp->eth_dev); bnxt_uninit_resources(bp, true); @@ -4824,7 +4822,6 @@ static int bnxt_init_resources(struct bnxt *bp, bool reconfig_dev) bp = eth_dev->data->dev_private; - bp->dev_stopped = 1; bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE; if (bnxt_vf_pciid(pci_dev->id.device_id)) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 8/9] net/bnxt: fix to support zero mark id along with RSS action 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (6 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 7/9] net/bnxt: remove a redundant variable Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 9/9] net/bnxt: fix coverity warnings Somnath Kotur 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit Certain applications(Ex: OVS-DPDK) can issue rte_flow_create with mark id set to 0. The mark table entry creation in the driver was assuming non-zero mark ids. Fix it to have a valid flag for each entry. Also, it is possible that both RSS flags and cfa_code/ mark id are set as part of the Rx packet completion descriptor if the 'action' for a flow is MARK + RSS. Fix Rx completion processing to look for both fields as opposed to only either. Fixes: 94eb699bc82e ("net/bnxt: support flow mark action") Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt.h | 9 +++++++-- drivers/net/bnxt/bnxt_flow.c | 20 +++++++++++++++----- drivers/net/bnxt/bnxt_rxr.c | 25 +++++++++++++++---------- drivers/net/bnxt/bnxt_rxr.h | 6 ++++-- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 1a5d542..68786a8 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -470,6 +470,11 @@ struct bnxt_error_recovery_info { uint32_t last_reset_counter; }; +struct bnxt_mark_info { + uint32_t mark_id; + bool valid; +}; + /* address space location of register */ #define BNXT_FW_STATUS_REG_TYPE_MASK 3 /* register is located in PCIe config space */ @@ -668,10 +673,10 @@ struct bnxt { /* Struct to hold adapter error recovery related info */ struct bnxt_error_recovery_info *recovery_info; -#define BNXT_MARK_TABLE_SZ (sizeof(uint32_t) * 64 * 1024) +#define BNXT_MARK_TABLE_SZ (sizeof(struct bnxt_mark_info) * 64 * 1024) /* TCAM and EM should be 16-bit only. Other modes not supported. */ #define BNXT_FLOW_ID_MASK 0x0000ffff - uint32_t *mark_table; + struct bnxt_mark_info *mark_table; }; int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu); diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c index bd6c726..9fb6dbd 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -1263,7 +1263,6 @@ static int match_vnic_rss_cfg(struct bnxt *bp, vnic_id = attr->group; BNXT_VALID_VNIC_OR_RET(bp, vnic_id); - vnic = &bp->vnic_info[vnic_id]; /* Check if requested RSS config matches RSS config of VNIC @@ -1641,7 +1640,7 @@ struct bnxt_vnic_info *find_matching_vnic(struct bnxt *bp, bool update_flow = false; struct rte_flow *flow; int ret = 0; - uint32_t tun_type; + uint32_t tun_type, flow_id; if (BNXT_VF(bp) && !BNXT_VF_IS_TRUSTED(bp)) { rte_flow_error_set(error, EINVAL, @@ -1773,8 +1772,16 @@ struct bnxt_vnic_info *find_matching_vnic(struct bnxt *bp, /* TCAM and EM should be 16-bit only. * Other modes not supported. */ - bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] = - filter->mark; + flow_id = filter->flow_id & BNXT_FLOW_ID_MASK; + if (bp->mark_table[flow_id].valid) { + PMD_DRV_LOG(ERR, + "Entry for Mark id 0x%x occupied" + " flow id 0x%x\n", + filter->mark, filter->flow_id); + goto free_filter; + } + bp->mark_table[flow_id].valid = true; + bp->mark_table[flow_id].mark_id = filter->mark; } bnxt_release_flow_lock(bp); return flow; @@ -1850,6 +1857,7 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp, struct bnxt_filter_info *filter; struct bnxt_vnic_info *vnic; int ret = 0; + uint32_t flow_id; filter = flow->filter; vnic = flow->vnic; @@ -1868,7 +1876,9 @@ static int bnxt_handle_tunnel_redirect_destroy(struct bnxt *bp, PMD_DRV_LOG(ERR, "Could not find matching flow\n"); if (filter->valid_flags & BNXT_FLOW_MARK_FLAG) { - bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] = 0; + flow_id = filter->flow_id & BNXT_FLOW_ID_MASK; + memset(&bp->mark_table[flow_id], 0, + sizeof(bp->mark_table[flow_id])); filter->flow_id = 0; } diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c index 1ae0c3c..1960b05 100644 --- a/drivers/net/bnxt/bnxt_rxr.c +++ b/drivers/net/bnxt/bnxt_rxr.c @@ -488,11 +488,10 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt, if (flags_type & RX_PKT_CMPL_FLAGS_RSS_VALID) { mbuf->hash.rss = rxcmp->rss_hash; mbuf->ol_flags |= PKT_RX_RSS_HASH; - } else { - mbuf->hash.fdir.id = bnxt_get_cfa_code_or_mark_id(rxq->bp, - rxcmp1); - mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID; } + + bnxt_set_mark_in_mbuf(rxq->bp, rxcmp1, mbuf); + #ifdef RTE_LIBRTE_IEEE1588 if (unlikely((flags_type & RX_PKT_CMPL_FLAGS_MASK) == RX_PKT_CMPL_FLAGS_ITYPE_PTP_W_TIMESTAMP)) { @@ -897,8 +896,9 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq) return 0; } -uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, - struct rx_pkt_cmpl_hi *rxcmp1) +void bnxt_set_mark_in_mbuf(struct bnxt *bp, + struct rx_pkt_cmpl_hi *rxcmp1, + struct rte_mbuf *mbuf) { uint32_t cfa_code = 0; uint8_t meta_fmt = 0; @@ -907,10 +907,13 @@ uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, cfa_code = rte_le_to_cpu_16(rxcmp1->cfa_code); if (!cfa_code) - return 0; + return; - if (cfa_code && !bp->mark_table[cfa_code]) - return cfa_code; + if (cfa_code && !bp->mark_table[cfa_code].valid) { + PMD_DRV_LOG(WARNING, "Invalid mark_tbl entry! cfa_code: 0x%x\n", + cfa_code); + return; + } flags2 = rte_le_to_cpu_16(rxcmp1->flags2); meta = rte_le_to_cpu_32(rxcmp1->metadata); @@ -932,5 +935,7 @@ uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, */ meta_fmt >>= BNXT_CFA_META_FMT_EM_EEM_SHFT; } - return bp->mark_table[cfa_code]; + + mbuf->hash.fdir.hi = bp->mark_table[cfa_code].mark_id; + mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID; } diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h index bf86002..d10dae2 100644 --- a/drivers/net/bnxt/bnxt_rxr.h +++ b/drivers/net/bnxt/bnxt_rxr.h @@ -226,8 +226,10 @@ uint16_t bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts, int bnxt_rxq_vec_setup(struct bnxt_rx_queue *rxq); #endif -uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, - struct rx_pkt_cmpl_hi *rxcmp1); +void bnxt_set_mark_in_mbuf(struct bnxt *bp, + struct rx_pkt_cmpl_hi *rxcmp1, + struct rte_mbuf *mbuf); + #define BNXT_RX_META_CFA_CODE_SHIFT 19 #define BNXT_CFA_CODE_META_SHIFT 16 #define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT 0x8000000 -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH 9/9] net/bnxt: fix coverity warnings 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (7 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 8/9] net/bnxt: fix to support zero mark id along with RSS action Somnath Kotur @ 2020-01-28 7:29 ` Somnath Kotur 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 9 siblings, 0 replies; 22+ messages in thread From: Somnath Kotur @ 2020-01-28 7:29 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> return value stored in "ret" but it has been overwritten before use. CID 353621: Code maintainability issues (UNUSED_VALUE) Fixes: 57813868 ("net/bnxt: fix VLAN strip") Fixes: df6cd7c1 ("net/bnxt: handle reset notify async event from FW") Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt_ethdev.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 72e5441..c1cb401 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -1956,9 +1956,15 @@ static int bnxt_free_one_vnic(struct bnxt *bp, uint16_t vnic_id) 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); + if (rc) + return rc; + rc = bnxt_restore_vlan_filters(bp); + if (rc) + return rc; } else { rc = bnxt_add_mac_filter(bp, vnic, NULL, 0, 0); + if (rc) + return rc; } rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL); @@ -3961,10 +3967,16 @@ static int bnxt_restore_filters(struct bnxt *bp) struct rte_eth_dev *dev = bp->eth_dev; int ret = 0; - if (dev->data->all_multicast) + if (dev->data->all_multicast) { ret = bnxt_allmulticast_enable_op(dev); - if (dev->data->promiscuous) + if (ret) + return ret; + } + if (dev->data->promiscuous) { ret = bnxt_promiscuous_enable_op(dev); + if (ret) + return ret; + } ret = bnxt_restore_mac_filters(bp); if (ret) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 0/9] bnxt patch set 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur ` (8 preceding siblings ...) 2020-01-28 7:29 ` [dpdk-dev] [PATCH 9/9] net/bnxt: fix coverity warnings Somnath Kotur @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 1/9] net/bnxt: fix alloc filter to use a common routine Ajit Khaparde ` (9 more replies) 9 siblings, 10 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit Please apply. v1->v2: Added one more patch to the set ("net/bnxt: fix coverity warnings") v2->v3: Updated Fixes tag and added stable@dpdk.org where necessary. Kalesh AP (5): net/bnxt: remove redundant if statement net/bnxt: remove redundant macro net/bnxt: remove unnecessary structure variable net/bnxt: remove a redundant variable net/bnxt: fix coverity warnings Somnath Kotur (4): net/bnxt: fix alloc filter to use a common routine net/bnxt: fix bumping of L2 filter reference count net/bnxt: fix to allow group ID 0 for RSS action net/bnxt: fix to support zero mark id along with RSS action drivers/net/bnxt/bnxt.h | 29 ++++++++++++++----- drivers/net/bnxt/bnxt_ethdev.c | 49 ++++++++++++++++++-------------- drivers/net/bnxt/bnxt_filter.c | 7 ++--- drivers/net/bnxt/bnxt_flow.c | 52 ++++++++++++---------------------- drivers/net/bnxt/bnxt_hwrm.c | 6 ++-- drivers/net/bnxt/bnxt_rxr.c | 25 +++++++++------- drivers/net/bnxt/bnxt_rxr.h | 6 ++-- drivers/net/bnxt/bnxt_stats.c | 4 +-- 8 files changed, 95 insertions(+), 83 deletions(-) -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 1/9] net/bnxt: fix alloc filter to use a common routine 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 2/9] net/bnxt: fix bumping of L2 filter reference count Ajit Khaparde ` (8 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable, Kalesh Anakkur Purayil From: Somnath Kotur <somnath.kotur@broadcom.com> Invoke bnxt_get_unused_filter() inside bnxt_alloc_filter() so that all filters are allocated from one common routine. Fixes: f92735db1e4c ("net/bnxt: add L2 filter alloc/init/free") Cc: stable@dpdk.org Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com> --- drivers/net/bnxt/bnxt_filter.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c index b31f10479..e2184334f 100644 --- a/drivers/net/bnxt/bnxt_filter.c +++ b/drivers/net/bnxt/bnxt_filter.c @@ -26,13 +26,11 @@ struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp) { struct bnxt_filter_info *filter; - /* Find the 1st unused filter from the free_filter_list pool*/ - filter = STAILQ_FIRST(&bp->free_filter_list); + filter = bnxt_get_unused_filter(bp); if (!filter) { PMD_DRV_LOG(ERR, "No more free filter resources\n"); return NULL; } - STAILQ_REMOVE_HEAD(&bp->free_filter_list, next); filter->mac_index = INVALID_MAC_INDEX; /* Default to L2 MAC Addr filter */ -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 2/9] net/bnxt: fix bumping of L2 filter reference count 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 1/9] net/bnxt: fix alloc filter to use a common routine Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 3/9] net/bnxt: fix to allow group ID 0 for RSS action Ajit Khaparde ` (7 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable, Kalesh Anakkur Purayil From: Somnath Kotur <somnath.kotur@broadcom.com> Now that the L2 filter reference count is bumped up in all cases including bnxt_alloc_filter() which is issued in init, just move this ref count bump inside the routine issuing the HWRM cmd so that it is bumped up only if the cmd is successful. Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow") Cc: stable@dpdk.org Reviewed-by: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com> --- drivers/net/bnxt/bnxt_filter.c | 3 +-- drivers/net/bnxt/bnxt_flow.c | 1 - drivers/net/bnxt/bnxt_hwrm.c | 2 ++ 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c index e2184334f..a1463a0e2 100644 --- a/drivers/net/bnxt/bnxt_filter.c +++ b/drivers/net/bnxt/bnxt_filter.c @@ -39,8 +39,7 @@ struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp) HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK; 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; } diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c index 5564c5363..4b3b59795 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -867,7 +867,6 @@ bnxt_create_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf, bnxt_free_filter(bp, filter1); return NULL; } - filter1->l2_ref_cnt++; return filter1; } diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 3b013396b..460cc4894 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -488,6 +488,8 @@ int bnxt_hwrm_set_l2_filter(struct bnxt *bp, filter->flow_id = rte_le_to_cpu_32(resp->flow_id); HWRM_UNLOCK(); + filter->l2_ref_cnt++; + return rc; } -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 3/9] net/bnxt: fix to allow group ID 0 for RSS action 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 1/9] net/bnxt: fix alloc filter to use a common routine Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 2/9] net/bnxt: fix bumping of L2 filter reference count Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 4/9] net/bnxt: remove redundant if statement Ajit Khaparde ` (6 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, Somnath Kotur, stable From: Somnath Kotur <somnath.kotur@broadcom.com> Allow RSS action with group ID 0. The RSS match check will ensure if requested RSS action configuration parameters should be allowed as per the VNIC's RSS configuration or not and if it does match as it is by design in OVS-DPDK use case, the default vnic can be used to create the filter. As part of this ensure that rx_queue_cnt for the default vnic is setup during init as this field was being set only for non-zero vnics while handling RSS action for flow create. Check for outofboundds erorr while accessing the vnic array. Fixes: adc0f81c6552d ("net/bnxt: support RSS action") Cc: stable@dpdk.org Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt.h | 14 +++++++++++++- drivers/net/bnxt/bnxt_ethdev.c | 4 ++++ drivers/net/bnxt/bnxt_flow.c | 33 ++++----------------------------- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index ddb26814c..bca9ad418 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -696,11 +696,23 @@ extern const struct rte_flow_ops bnxt_flow_ops; #define bnxt_release_flow_lock(bp) \ pthread_mutex_unlock(&(bp)->flow_lock) +#define BNXT_VALID_VNIC_OR_RET(bp, vnic_id) do { \ + if ((vnic_id) >= (bp)->max_vnics) { \ + rte_flow_error_set(error, \ + EINVAL, \ + RTE_FLOW_ERROR_TYPE_ATTR_GROUP, \ + NULL, \ + "Group id is invalid!"); \ + rc = -rte_errno; \ + goto ret; \ + } \ +} while (0) + extern int bnxt_logtype_driver; #define PMD_DRV_LOG_RAW(level, fmt, args...) \ rte_log(RTE_LOG_ ## level, bnxt_logtype_driver, "%s(): " fmt, \ __func__, ## args) #define PMD_DRV_LOG(level, fmt, args...) \ - PMD_DRV_LOG_RAW(level, fmt, ## args) + PMD_DRV_LOG_RAW(level, fmt, ## args) #endif diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 2ef1169b8..fc3f1a8db 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -295,8 +295,12 @@ static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t vnic_id) if (BNXT_HAS_RING_GRPS(bp) && rxq->rx_deferred_start) rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID; + else + vnic->rx_queue_cnt++; } + PMD_DRV_LOG(DEBUG, "vnic->rx_queue_cnt = %d\n", vnic->rx_queue_cnt); + rc = bnxt_vnic_rss_configure(bp, vnic); if (rc) goto err_out; diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c index 4b3b59795..bd6c72623 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -1059,16 +1059,9 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev, vnic_id = act_q->index; } + BNXT_VALID_VNIC_OR_RET(bp, vnic_id); + vnic = &bp->vnic_info[vnic_id]; - if (vnic == NULL) { - rte_flow_error_set(error, - EINVAL, - RTE_FLOW_ERROR_TYPE_ACTION, - act, - "No matching VNIC found."); - rc = -rte_errno; - goto ret; - } if (vnic->rx_queue_cnt) { if (vnic->start_grp_id != act_q->index) { PMD_DRV_LOG(ERR, @@ -1268,28 +1261,10 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev, rss = (const struct rte_flow_action_rss *)act->conf; vnic_id = attr->group; - if (!vnic_id) { - PMD_DRV_LOG(ERR, "Group id cannot be 0\n"); - rte_flow_error_set(error, - EINVAL, - RTE_FLOW_ERROR_TYPE_ATTR, - NULL, - "Group id cannot be 0"); - rc = -rte_errno; - goto ret; - } + + BNXT_VALID_VNIC_OR_RET(bp, vnic_id); vnic = &bp->vnic_info[vnic_id]; - if (vnic == NULL) { - rte_flow_error_set(error, - EINVAL, - RTE_FLOW_ERROR_TYPE_ACTION, - act, - "No matching VNIC for RSS group."); - rc = -rte_errno; - goto ret; - } - PMD_DRV_LOG(DEBUG, "VNIC found\n"); /* Check if requested RSS config matches RSS config of VNIC * only if it is not a fresh VNIC configuration. -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 4/9] net/bnxt: remove redundant if statement 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (2 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 3/9] net/bnxt: fix to allow group ID 0 for RSS action Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 5/9] net/bnxt: remove redundant macro Ajit Khaparde ` (5 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, Kalesh AP, stable, Somnath Kotur From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Since "eth_dev->data->dev_started" has been assigned to 0 at the beginning of bnxt_dev_stop_op() routine, the code inside the if() condition is redundant. Remove it. Anyways "eth_dev->data->dev_link.link_status" will be set to 0 in bnxt_dev_set_link_down_op() later in the routine. Fixes: 316e412299fd ("net/bnxt: fix crash when closing") Cc: stable@dpdk.org Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> --- drivers/net/bnxt/bnxt_ethdev.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index fc3f1a8db..267a36148 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -948,10 +948,6 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bnxt_cancel_fw_health_check(bp); bp->flags &= ~BNXT_FLAG_INIT_DONE; - if (bp->eth_dev->data->dev_started) { - /* TBD: STOP HW queues DMA */ - eth_dev->data->dev_link.link_status = 0; - } bnxt_dev_set_link_down_op(eth_dev); /* Wait for link to be reset and the async notification to process. -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 5/9] net/bnxt: remove redundant macro 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (3 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 4/9] net/bnxt: remove redundant if statement Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 6/9] net/bnxt: remove unnecessary structure variable Ajit Khaparde ` (4 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev Cc: ferruh.yigit, Kalesh AP, stable, Santoshkumar Karanappa Rastapur, Somnath Kotur From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Use "dev->data->dev_started" state, instead of local BNXT_FLAG_INIT_DONE to check whether device has been initialised or not. Fixes: ed2ced6fe927 ("net/bnxt: check initialization before accessing stats") Cc: stable@dpdk.org Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> --- drivers/net/bnxt/bnxt.h | 1 - drivers/net/bnxt/bnxt_ethdev.c | 2 -- drivers/net/bnxt/bnxt_stats.c | 4 ++-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index bca9ad418..95f1f1a20 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -520,7 +520,6 @@ struct bnxt { #define BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED BIT(18) #define BNXT_FLAG_EXT_STATS_SUPPORTED BIT(19) #define BNXT_FLAG_NEW_RM BIT(20) -#define BNXT_FLAG_INIT_DONE BIT(21) #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS BIT(22) #define BNXT_FLAG_ADV_FLOW_MGMT BIT(23) #define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 267a36148..b04685cea 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -887,7 +887,6 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev); eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev); - bp->flags |= BNXT_FLAG_INIT_DONE; eth_dev->data->dev_started = 1; pthread_mutex_lock(&bp->def_cp_lock); bnxt_schedule_fw_health_check(bp); @@ -947,7 +946,6 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bnxt_cancel_fw_health_check(bp); - bp->flags &= ~BNXT_FLAG_INIT_DONE; bnxt_dev_set_link_down_op(eth_dev); /* Wait for link to be reset and the async notification to process. diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c index 32f112b5b..466862995 100644 --- a/drivers/net/bnxt/bnxt_stats.c +++ b/drivers/net/bnxt/bnxt_stats.c @@ -389,7 +389,7 @@ int bnxt_stats_get_op(struct rte_eth_dev *eth_dev, if (rc) return rc; - if (!(bp->flags & BNXT_FLAG_INIT_DONE)) + if (!eth_dev->data->dev_started) return -EIO; num_q_stats = RTE_MIN(bp->rx_cp_nr_rings, @@ -434,7 +434,7 @@ int bnxt_stats_reset_op(struct rte_eth_dev *eth_dev) if (ret) return ret; - if (!(bp->flags & BNXT_FLAG_INIT_DONE)) { + if (!eth_dev->data->dev_started) { PMD_DRV_LOG(ERR, "Device Initialization not complete!\n"); return -EINVAL; } -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 6/9] net/bnxt: remove unnecessary structure variable 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (4 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 5/9] net/bnxt: remove redundant macro Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 7/9] net/bnxt: remove a redundant variable Ajit Khaparde ` (3 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev Cc: ferruh.yigit, Kalesh AP, stable, Santoshkumar Karanappa Rastapur, Somnath Kotur From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> This change could help in reducing the size of bnxt PMD private data structure by converting a uint8_t variable to use bit map flag. Fixes: 5cd0e2889c432 ("net/bnxt: support NIC Partitioning") Cc: stable@dpdk.org Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> --- drivers/net/bnxt/bnxt.h | 4 ++-- drivers/net/bnxt/bnxt_hwrm.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 95f1f1a20..434ce2829 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -520,12 +520,13 @@ struct bnxt { #define BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED BIT(18) #define BNXT_FLAG_EXT_STATS_SUPPORTED BIT(19) #define BNXT_FLAG_NEW_RM BIT(20) +#define BNXT_FLAG_NPAR_PF BIT(21) #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS BIT(22) #define BNXT_FLAG_ADV_FLOW_MGMT BIT(23) #define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24) #define BNXT_PF(bp) (!((bp)->flags & BNXT_FLAG_VF)) #define BNXT_VF(bp) ((bp)->flags & BNXT_FLAG_VF) -#define BNXT_NPAR(bp) ((bp)->port_partition_type) +#define BNXT_NPAR(bp) ((bp)->flags & BNXT_FLAG_NPAR_PF) #define BNXT_MH(bp) ((bp)->flags & BNXT_FLAG_MULTI_HOST) #define BNXT_SINGLE_PF(bp) (BNXT_PF(bp) && !BNXT_NPAR(bp) && !BNXT_MH(bp)) #define BNXT_USE_CHIMP_MB 0 //For non-CFA commands, everything uses Chimp. @@ -647,7 +648,6 @@ struct bnxt { #define BNXT_OUTER_TPID_BD_SHFT 16 uint32_t outer_tpid_bd; struct bnxt_pf_info pf; - uint8_t port_partition_type; uint8_t dev_stopped; uint8_t vxlan_port_cnt; uint8_t geneve_port_cnt; diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c index 460cc4894..f325aff82 100644 --- a/drivers/net/bnxt/bnxt_hwrm.c +++ b/drivers/net/bnxt/bnxt_hwrm.c @@ -2952,10 +2952,10 @@ int bnxt_hwrm_func_qcfg(struct bnxt *bp, uint16_t *mtu) case HWRM_FUNC_QCFG_OUTPUT_PORT_PARTITION_TYPE_NPAR1_5: case HWRM_FUNC_QCFG_OUTPUT_PORT_PARTITION_TYPE_NPAR2_0: /* FALLTHROUGH */ - bp->port_partition_type = resp->port_partition_type; + bp->flags |= BNXT_FLAG_NPAR_PF; break; default: - bp->port_partition_type = 0; + bp->flags &= ~BNXT_FLAG_NPAR_PF; break; } -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 7/9] net/bnxt: remove a redundant variable 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (5 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 6/9] net/bnxt: remove unnecessary structure variable Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 8/9] net/bnxt: fix to support zero mark id along with RSS action Ajit Khaparde ` (2 subsequent siblings) 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev Cc: ferruh.yigit, Kalesh AP, stable, Santoshkumar Karanappa Rastapur, Somnath Kotur From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Use "dev->data->dev_started" state, instead of local "dev_stopped" to check whether port has been started or not. Fixes: 316e412299fd ("net/bnxt: fix crash when closing") Cc: stable@dpdk.org Reviewed-by: Santoshkumar Karanappa Rastapur <santosh.rastapur@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> --- drivers/net/bnxt/bnxt.h | 1 - drivers/net/bnxt/bnxt_ethdev.c | 21 +++++++++------------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 434ce2829..1a5d542b4 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -648,7 +648,6 @@ struct bnxt { #define BNXT_OUTER_TPID_BD_SHFT 16 uint32_t outer_tpid_bd; struct bnxt_pf_info pf; - uint8_t dev_stopped; uint8_t vxlan_port_cnt; uint8_t geneve_port_cnt; uint16_t vxlan_port; diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index b04685cea..72e544170 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -872,9 +872,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) goto error; eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev); + eth_dev->data->dev_started = 1; 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; @@ -887,7 +887,6 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev); eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev); - eth_dev->data->dev_started = 1; pthread_mutex_lock(&bp->def_cp_lock); bnxt_schedule_fw_health_check(bp); pthread_mutex_unlock(&bp->def_cp_lock); @@ -898,7 +897,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; + eth_dev->data->dev_started = 0; return rc; } @@ -973,7 +972,6 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev) bp->mark_table = NULL; bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE; - bp->dev_stopped = 1; bp->rx_cosq_cnt = 0; } @@ -981,7 +979,7 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev) { struct bnxt *bp = eth_dev->data->dev_private; - if (bp->dev_stopped == 0) + if (eth_dev->data->dev_started) bnxt_dev_stop_op(eth_dev); bnxt_uninit_resources(bp, false); @@ -1174,7 +1172,7 @@ static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1203,7 +1201,7 @@ static int bnxt_promiscuous_disable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1232,7 +1230,7 @@ static int bnxt_allmulticast_enable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1261,7 +1259,7 @@ static int bnxt_allmulticast_disable_op(struct rte_eth_dev *eth_dev) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!eth_dev->data->dev_started) return 0; if (bp->vnic_info == NULL) @@ -1985,7 +1983,7 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) return rc; /* Filter settings will get applied when port is started */ - if (bp->dev_stopped == 1) + if (!dev->data->dev_started) return 0; if (mask & ETH_VLAN_FILTER_MASK) { @@ -3887,7 +3885,7 @@ static void bnxt_dev_cleanup(struct bnxt *bp) { bnxt_set_hwrm_link_config(bp, false); bp->link_info.link_up = 0; - if (bp->dev_stopped == 0) + if (bp->eth_dev->data->dev_started) bnxt_dev_stop_op(bp->eth_dev); bnxt_uninit_resources(bp, true); @@ -4824,7 +4822,6 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev) bp = eth_dev->data->dev_private; - bp->dev_stopped = 1; bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE; if (bnxt_vf_pciid(pci_dev->id.device_id)) -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 8/9] net/bnxt: fix to support zero mark id along with RSS action 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (6 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 7/9] net/bnxt: remove a redundant variable Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 9/9] net/bnxt: fix coverity warnings Ajit Khaparde 2020-01-28 21:07 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, Somnath Kotur From: Somnath Kotur <somnath.kotur@broadcom.com> Certain applications(Ex: OVS-DPDK) can issue rte_flow_create with mark id set to 0. The mark table entry creation in the driver was assuming non-zero mark ids. Fix it to have a valid flag for each entry. Also, it is possible that both RSS flags and cfa_code/ mark id are set as part of the Rx packet completion descriptor if the 'action' for a flow is MARK + RSS. Fix Rx completion processing to look for both fields as opposed to only either. Fixes: 94eb699bc82e ("net/bnxt: support flow mark action") Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> --- drivers/net/bnxt/bnxt.h | 9 +++++++-- drivers/net/bnxt/bnxt_flow.c | 20 +++++++++++++++----- drivers/net/bnxt/bnxt_rxr.c | 25 +++++++++++++++---------- drivers/net/bnxt/bnxt_rxr.h | 6 ++++-- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h index 1a5d542b4..68786a89b 100644 --- a/drivers/net/bnxt/bnxt.h +++ b/drivers/net/bnxt/bnxt.h @@ -470,6 +470,11 @@ struct bnxt_error_recovery_info { uint32_t last_reset_counter; }; +struct bnxt_mark_info { + uint32_t mark_id; + bool valid; +}; + /* address space location of register */ #define BNXT_FW_STATUS_REG_TYPE_MASK 3 /* register is located in PCIe config space */ @@ -668,10 +673,10 @@ struct bnxt { /* Struct to hold adapter error recovery related info */ struct bnxt_error_recovery_info *recovery_info; -#define BNXT_MARK_TABLE_SZ (sizeof(uint32_t) * 64 * 1024) +#define BNXT_MARK_TABLE_SZ (sizeof(struct bnxt_mark_info) * 64 * 1024) /* TCAM and EM should be 16-bit only. Other modes not supported. */ #define BNXT_FLOW_ID_MASK 0x0000ffff - uint32_t *mark_table; + struct bnxt_mark_info *mark_table; }; int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu); diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c index bd6c72623..9fb6dbdd9 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -1263,7 +1263,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev, vnic_id = attr->group; BNXT_VALID_VNIC_OR_RET(bp, vnic_id); - vnic = &bp->vnic_info[vnic_id]; /* Check if requested RSS config matches RSS config of VNIC @@ -1641,7 +1640,7 @@ bnxt_flow_create(struct rte_eth_dev *dev, bool update_flow = false; struct rte_flow *flow; int ret = 0; - uint32_t tun_type; + uint32_t tun_type, flow_id; if (BNXT_VF(bp) && !BNXT_VF_IS_TRUSTED(bp)) { rte_flow_error_set(error, EINVAL, @@ -1773,8 +1772,16 @@ bnxt_flow_create(struct rte_eth_dev *dev, /* TCAM and EM should be 16-bit only. * Other modes not supported. */ - bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] = - filter->mark; + flow_id = filter->flow_id & BNXT_FLOW_ID_MASK; + if (bp->mark_table[flow_id].valid) { + PMD_DRV_LOG(ERR, + "Entry for Mark id 0x%x occupied" + " flow id 0x%x\n", + filter->mark, filter->flow_id); + goto free_filter; + } + bp->mark_table[flow_id].valid = true; + bp->mark_table[flow_id].mark_id = filter->mark; } bnxt_release_flow_lock(bp); return flow; @@ -1850,6 +1857,7 @@ _bnxt_flow_destroy(struct bnxt *bp, struct bnxt_filter_info *filter; struct bnxt_vnic_info *vnic; int ret = 0; + uint32_t flow_id; filter = flow->filter; vnic = flow->vnic; @@ -1868,7 +1876,9 @@ _bnxt_flow_destroy(struct bnxt *bp, PMD_DRV_LOG(ERR, "Could not find matching flow\n"); if (filter->valid_flags & BNXT_FLOW_MARK_FLAG) { - bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] = 0; + flow_id = filter->flow_id & BNXT_FLOW_ID_MASK; + memset(&bp->mark_table[flow_id], 0, + sizeof(bp->mark_table[flow_id])); filter->flow_id = 0; } diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c index 1ae0c3c38..1960b0515 100644 --- a/drivers/net/bnxt/bnxt_rxr.c +++ b/drivers/net/bnxt/bnxt_rxr.c @@ -488,11 +488,10 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt, if (flags_type & RX_PKT_CMPL_FLAGS_RSS_VALID) { mbuf->hash.rss = rxcmp->rss_hash; mbuf->ol_flags |= PKT_RX_RSS_HASH; - } else { - mbuf->hash.fdir.id = bnxt_get_cfa_code_or_mark_id(rxq->bp, - rxcmp1); - mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID; } + + bnxt_set_mark_in_mbuf(rxq->bp, rxcmp1, mbuf); + #ifdef RTE_LIBRTE_IEEE1588 if (unlikely((flags_type & RX_PKT_CMPL_FLAGS_MASK) == RX_PKT_CMPL_FLAGS_ITYPE_PTP_W_TIMESTAMP)) { @@ -897,8 +896,9 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq) return 0; } -uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, - struct rx_pkt_cmpl_hi *rxcmp1) +void bnxt_set_mark_in_mbuf(struct bnxt *bp, + struct rx_pkt_cmpl_hi *rxcmp1, + struct rte_mbuf *mbuf) { uint32_t cfa_code = 0; uint8_t meta_fmt = 0; @@ -907,10 +907,13 @@ uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, cfa_code = rte_le_to_cpu_16(rxcmp1->cfa_code); if (!cfa_code) - return 0; + return; - if (cfa_code && !bp->mark_table[cfa_code]) - return cfa_code; + if (cfa_code && !bp->mark_table[cfa_code].valid) { + PMD_DRV_LOG(WARNING, "Invalid mark_tbl entry! cfa_code: 0x%x\n", + cfa_code); + return; + } flags2 = rte_le_to_cpu_16(rxcmp1->flags2); meta = rte_le_to_cpu_32(rxcmp1->metadata); @@ -932,5 +935,7 @@ uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, */ meta_fmt >>= BNXT_CFA_META_FMT_EM_EEM_SHFT; } - return bp->mark_table[cfa_code]; + + mbuf->hash.fdir.hi = bp->mark_table[cfa_code].mark_id; + mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID; } diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h index bf860020c..d10dae25a 100644 --- a/drivers/net/bnxt/bnxt_rxr.h +++ b/drivers/net/bnxt/bnxt_rxr.h @@ -226,8 +226,10 @@ uint16_t bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts, int bnxt_rxq_vec_setup(struct bnxt_rx_queue *rxq); #endif -uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp, - struct rx_pkt_cmpl_hi *rxcmp1); +void bnxt_set_mark_in_mbuf(struct bnxt *bp, + struct rx_pkt_cmpl_hi *rxcmp1, + struct rte_mbuf *mbuf); + #define BNXT_RX_META_CFA_CODE_SHIFT 19 #define BNXT_CFA_CODE_META_SHIFT 16 #define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT 0x8000000 -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] [PATCH v3 9/9] net/bnxt: fix coverity warnings 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (7 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 8/9] net/bnxt: fix to support zero mark id along with RSS action Ajit Khaparde @ 2020-01-28 21:01 ` Ajit Khaparde 2020-01-29 12:30 ` Ferruh Yigit 2020-01-28 21:07 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 9 siblings, 1 reply; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:01 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, Kalesh AP, stable, Somnath Kotur From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> return value stored in "ret" but it has been overwritten before use. CID 353621: Code maintainability issues (UNUSED_VALUE) Fixes: 7fe5668d2ea3 ("net/bnxt: support VLAN filter and strip") Fixes: df6cd7c1f73a ("net/bnxt: handle reset notify async event from FW") Cc: stable@dpdk.org Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> --- drivers/net/bnxt/bnxt_ethdev.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 72e544170..c1cb40160 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -1956,9 +1956,15 @@ bnxt_config_vlan_hw_stripping(struct bnxt *bp, uint64_t rx_offloads) 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); + if (rc) + return rc; + rc = bnxt_restore_vlan_filters(bp); + if (rc) + return rc; } else { rc = bnxt_add_mac_filter(bp, vnic, NULL, 0, 0); + if (rc) + return rc; } rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL); @@ -3961,10 +3967,16 @@ static int bnxt_restore_filters(struct bnxt *bp) struct rte_eth_dev *dev = bp->eth_dev; int ret = 0; - if (dev->data->all_multicast) + if (dev->data->all_multicast) { ret = bnxt_allmulticast_enable_op(dev); - if (dev->data->promiscuous) + if (ret) + return ret; + } + if (dev->data->promiscuous) { ret = bnxt_promiscuous_enable_op(dev); + if (ret) + return ret; + } ret = bnxt_restore_mac_filters(bp); if (ret) -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] [PATCH v3 9/9] net/bnxt: fix coverity warnings 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 9/9] net/bnxt: fix coverity warnings Ajit Khaparde @ 2020-01-29 12:30 ` Ferruh Yigit 0 siblings, 0 replies; 22+ messages in thread From: Ferruh Yigit @ 2020-01-29 12:30 UTC (permalink / raw) To: Ajit Khaparde, dev; +Cc: Kalesh AP, stable, Somnath Kotur On 1/28/2020 9:01 PM, Ajit Khaparde wrote: > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > return value stored in "ret" but it has been overwritten before use. > > CID 353621: Code maintainability issues (UNUSED_VALUE) > Fixes: 7fe5668d2ea3 ("net/bnxt: support VLAN filter and strip") > Fixes: df6cd7c1f73a ("net/bnxt: handle reset notify async event from FW") > Cc: stable@dpdk.org > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com> Hi Kalesh, The patch title doesn't really say much, it should say the actual reason not just fixing a tools warning, I am updating it as: "net/bnxt: fix return value check" Also the defined syntax for Coverity issues is: Coverity issue: #### For this case it should be: Coverity issue: 353621 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] [PATCH v3 0/9] bnxt patch set 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde ` (8 preceding siblings ...) 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 9/9] net/bnxt: fix coverity warnings Ajit Khaparde @ 2020-01-28 21:07 ` Ajit Khaparde 9 siblings, 0 replies; 22+ messages in thread From: Ajit Khaparde @ 2020-01-28 21:07 UTC (permalink / raw) To: dpdk-dev; +Cc: Ferruh Yigit On Tue, Jan 28, 2020 at 1:02 PM Ajit Khaparde <ajit.khaparde@broadcom.com> wrote: > Please apply. > > v1->v2: Added one more patch to the set ("net/bnxt: fix coverity warnings") > v2->v3: Updated Fixes tag and added stable@dpdk.org where necessary. > Patchset applied to dpdk-next-net-brcm. Thanks > > Kalesh AP (5): > net/bnxt: remove redundant if statement > net/bnxt: remove redundant macro > net/bnxt: remove unnecessary structure variable > net/bnxt: remove a redundant variable > net/bnxt: fix coverity warnings > > Somnath Kotur (4): > net/bnxt: fix alloc filter to use a common routine > net/bnxt: fix bumping of L2 filter reference count > net/bnxt: fix to allow group ID 0 for RSS action > net/bnxt: fix to support zero mark id along with RSS action > > drivers/net/bnxt/bnxt.h | 29 ++++++++++++++----- > drivers/net/bnxt/bnxt_ethdev.c | 49 ++++++++++++++++++-------------- > drivers/net/bnxt/bnxt_filter.c | 7 ++--- > drivers/net/bnxt/bnxt_flow.c | 52 ++++++++++++---------------------- > drivers/net/bnxt/bnxt_hwrm.c | 6 ++-- > drivers/net/bnxt/bnxt_rxr.c | 25 +++++++++------- > drivers/net/bnxt/bnxt_rxr.h | 6 ++-- > drivers/net/bnxt/bnxt_stats.c | 4 +-- > 8 files changed, 95 insertions(+), 83 deletions(-) > > -- > 2.21.0 (Apple Git-122.2) > > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-01-29 12:31 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-28 7:29 [dpdk-dev] [PATCH v2 0/9] bnxt patchset Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 1/9] net/bnxt: fix bnxt_alloc_filter() to use a common routine Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 2/9] net/bnxt: fix bumping of L2 filter reference count Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 3/9] net/bnxt: fix to allow group ID 0 for RSS action Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 4/9] net/bnxt: remove redundant if statement Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 5/9] net/bnxt: remove redundant macro Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 6/9] net/bnxt: remove unnecessary structure variable Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 7/9] net/bnxt: remove a redundant variable Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 8/9] net/bnxt: fix to support zero mark id along with RSS action Somnath Kotur 2020-01-28 7:29 ` [dpdk-dev] [PATCH 9/9] net/bnxt: fix coverity warnings Somnath Kotur 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 1/9] net/bnxt: fix alloc filter to use a common routine Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 2/9] net/bnxt: fix bumping of L2 filter reference count Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 3/9] net/bnxt: fix to allow group ID 0 for RSS action Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 4/9] net/bnxt: remove redundant if statement Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 5/9] net/bnxt: remove redundant macro Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 6/9] net/bnxt: remove unnecessary structure variable Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 7/9] net/bnxt: remove a redundant variable Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 8/9] net/bnxt: fix to support zero mark id along with RSS action Ajit Khaparde 2020-01-28 21:01 ` [dpdk-dev] [PATCH v3 9/9] net/bnxt: fix coverity warnings Ajit Khaparde 2020-01-29 12:30 ` Ferruh Yigit 2020-01-28 21:07 ` [dpdk-dev] [PATCH v3 0/9] bnxt patch set Ajit Khaparde
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).