From 9db66782bd06b23c47782aa4c7ce8adb1027101e Mon Sep 17 00:00:00 2001 From: Somnath Kotur Date: Tue, 28 Jan 2020 12:59:22 +0530 Subject: [PATCH] net/bnxt: fix supporting zero mark ID with RSS action 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 Reviewed-by: Ajit Khaparde --- 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 1a5d542b45..68786a89bf 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 bd6c726232..9fb6dbdd9e 100644 --- a/drivers/net/bnxt/bnxt_flow.c +++ b/drivers/net/bnxt/bnxt_flow.c @@ -1263,7 +1263,6 @@ use_vnic: 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 @@ done: /* 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 1ae0c3c38f..1960b05151 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 bf860020cb..d10dae25a8 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.20.1