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
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
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
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
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
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
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
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
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
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
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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) > >
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