[dpdk-dev] [PATCH] net/bnxt: fix dynamic VNIC count
Ensure that the current count of in-use VNICs is decremented
when a VNIC is freed. Don't attempt VNIC allocation when the
maximum supported number of VNICs is currently allocated.
Fixes: 49d0709b257f ("net/bnxt: delete and flush L2 filters cleanly")
Fixes: d24610f7bfda ("net/bnxt: allow flow creation when RSS is enabled")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt.h | 2 +-
drivers/net/bnxt/bnxt_flow.c | 5 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index bd2dec4d63..a385710846 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -719,7 +719,7 @@ struct bnxt {
uint32_tmax_ring_grps;
struct bnxt_ring_grp_info *grp_info;
- unsigned intnr_vnics;
+ uint16_tnr_vnics;
#define BNXT_GET_DEFAULT_VNIC(bp) (&(bp)->vnic_info[0])
struct bnxt_vnic_info *vnic_info;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 844bf1520f..73fd24cd9a 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -925,6 +925,9 @@ static int bnxt_vnic_prep(struct bnxt *bp, struct
bnxt_vnic_info *vnic)
uint64_t rx_offloads = dev_conf->rxmode.offloads;
int rc;
+ if (bp->nr_vnics > bp->max_vnics - 1)
+ return -ENOMEM;
+
rc = bnxt_vnic_grp_alloc(bp, vnic);
if (rc)
goto ret;
@@ -1550,6 +1553,7 @@ bnxt_flow_validate(struct rte_eth_dev *dev,
bnxt_hwrm_vnic_ctx_free(bp, vnic);
bnxt_hwrm_vnic_free(bp, vnic);
vnic->rx_queue_cnt = 0;
+ bp->nr_vnics--;
PMD_DRV_LOG(DEBUG, "Free VNIC\n");
}
}
@@ -2011,6 +2015,7 @@ _bnxt_flow_destroy(struct bnxt *bp,
bnxt_hwrm_vnic_free(bp, vnic);
vnic->rx_queue_cnt = 0;
+ bp->nr_vnics--;
}
} else {
rte_flow_error_set(error, -ret,
--
2.25.1
[dpdk-dev] [PATCH] net/bnxt: fix initialization of filter struct
Initialize filter structure to all zeroes on allocation.
Fixes: 5ef3b79fdfe6 ("net/bnxt: support flow filter ops")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_filter.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
index 1d08e03b2f..6aadfa9526 100644
--- a/drivers/net/bnxt/bnxt_filter.c
+++ b/drivers/net/bnxt/bnxt_filter.c
@@ -188,6 +188,7 @@ struct bnxt_filter_info *bnxt_get_unused_filter(struct bnxt
*bp)
return NULL;
}
STAILQ_REMOVE_HEAD(&bp->free_filter_list, next);
+ memset(filter, 0, sizeof(*filter));
return filter;
}
--
2.25.1
Re: [dpdk-dev] [PATCH v2 1/4] mbuf: mark old offload flag as deprecated
On Thu, Apr 29, 2021 at 4:05 AM David Marchand
wrote:
>
> PKT_RX_EIP_CKSUM_BAD has been declared deprecated but there was no
> warning to applications still using it.
> Fix this by marking as deprecated with the newly introduced
> RTE_DEPRECATED.
>
> Fixes: e8a419d6de4b ("mbuf: rename outer IP checksum macro")
>
> Signed-off-by: David Marchand
> Reviewed-by: Flavio Leitner
> Acked-by: Olivier Matz
> ---
> Changes since v1:
> - updated commitlog following Olivier comment,
>
> ---
> lib/mbuf/rte_mbuf_core.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index c17dc95c51..bb38d7f581 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -83,7 +83,8 @@ extern "C" {
> * Deprecated.
> * This flag has been renamed, use PKT_RX_OUTER_IP_CKSUM_BAD instead.
> */
> -#define PKT_RX_EIP_CKSUM_BAD PKT_RX_OUTER_IP_CKSUM_BAD
> +#define PKT_RX_EIP_CKSUM_BAD \
> + RTE_DEPRECATED(PKT_RX_EIP_CKSUM_BAD) PKT_RX_OUTER_IP_CKSUM_BAD
>
> /**
> * A vlan has been stripped by the hardware and its tci is saved in
> --
> 2.23.0
>
Acked-by: Lance Richardson
Re: [dpdk-dev] [PATCH] net/bnxt: fix initialization of filter struct
On Wed, Apr 28, 2021 at 6:04 PM Lance Richardson
wrote:
>
> Initialize filter structure to all zeroes on allocation.
>
> Fixes: 5ef3b79fdfe6 ("net/bnxt: support flow filter ops")
> Cc: sta...@dpdk.org
> Signed-off-by: Lance Richardson
> Reviewed-by: Ajit Kumar Khaparde
> ---
> drivers/net/bnxt/bnxt_filter.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
> index 1d08e03b2f..6aadfa9526 100644
> --- a/drivers/net/bnxt/bnxt_filter.c
> +++ b/drivers/net/bnxt/bnxt_filter.c
> @@ -188,6 +188,7 @@ struct bnxt_filter_info *bnxt_get_unused_filter(struct
> bnxt *bp)
> return NULL;
> }
> STAILQ_REMOVE_HEAD(&bp->free_filter_list, next);
> + memset(filter, 0, sizeof(*filter));
>
> return filter;
> }
> --
> 2.25.1
>
Self-NAK, will send a v2 shortly.
Re: [dpdk-dev] [PATCH] net/bnxt: fix initialization of filter struct
On Thu, Apr 29, 2021 at 9:43 AM Lance Richardson
wrote:
>
> On Wed, Apr 28, 2021 at 6:04 PM Lance Richardson
> wrote:
> >
> > Initialize filter structure to all zeroes on allocation.
> >
> > Fixes: 5ef3b79fdfe6 ("net/bnxt: support flow filter ops")
> > Cc: sta...@dpdk.org
> > Signed-off-by: Lance Richardson
> > Reviewed-by: Ajit Kumar Khaparde
> > ---
> > drivers/net/bnxt/bnxt_filter.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/bnxt/bnxt_filter.c b/drivers/net/bnxt/bnxt_filter.c
> > index 1d08e03b2f..6aadfa9526 100644
> > --- a/drivers/net/bnxt/bnxt_filter.c
> > +++ b/drivers/net/bnxt/bnxt_filter.c
> > @@ -188,6 +188,7 @@ struct bnxt_filter_info *bnxt_get_unused_filter(struct
> > bnxt *bp)
> > return NULL;
> > }
> > STAILQ_REMOVE_HEAD(&bp->free_filter_list, next);
> > + memset(filter, 0, sizeof(*filter));
> >
> > return filter;
> > }
> > --
> > 2.25.1
> >
> Self-NAK, will send a v2 shortly.
Let's drop this patch, this is already fixed in upstream via
commit 7105de7085b0 ("net/bnxt: reset filter indices on free"),
but that commit should be applied to -stable branches.
[dpdk-dev] [PATCH 1/4] net/bnxt: fix ring and context memory allocation
Use requested socket ID when allocating memory for transmit rings,
receive rings, and completion queues. Use device NUMA ID when
allocating context memory, notification queue rings, async
completion queue rings, and VNIC attributes.
Fixes: 6eb3cc2294fd ("net/bnxt: add initial Tx code")
Fixes: 9738793f28ec ("net/bnxt: add VNIC functions and structs")
Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
Fixes: bd0a14c99f65 ("net/bnxt: use dedicated CPR for async events")
Fixes: 683e5cf79249 ("net/bnxt: use common NQ ring")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Somnath Kotur
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 17 +
drivers/net/bnxt/bnxt_ring.c | 30 ++
drivers/net/bnxt/bnxt_ring.h | 2 +-
drivers/net/bnxt/bnxt_rxq.c| 4 ++--
drivers/net/bnxt/bnxt_txq.c| 4 ++--
drivers/net/bnxt/bnxt_vnic.c | 3 ++-
6 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index d859ef503..d4b8762d5 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -580,13 +580,14 @@ static int bnxt_register_fc_ctx_mem(struct bnxt *bp)
return rc;
}
-static int bnxt_alloc_ctx_mem_buf(char *type, size_t size,
+static int bnxt_alloc_ctx_mem_buf(struct bnxt *bp, char *type, size_t size,
struct bnxt_ctx_mem_buf_info *ctx)
{
if (!ctx)
return -EINVAL;
- ctx->va = rte_zmalloc(type, size, 0);
+ ctx->va = rte_zmalloc_socket(type, size, 0,
+bp->eth_dev->device->numa_node);
if (ctx->va == NULL)
return -ENOMEM;
rte_mem_lock_page(ctx->va);
@@ -610,7 +611,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
sprintf(type, "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
/* 4 bytes for each counter-id */
- rc = bnxt_alloc_ctx_mem_buf(type,
+ rc = bnxt_alloc_ctx_mem_buf(bp, type,
max_fc * 4,
&bp->flow_stat->rx_fc_in_tbl);
if (rc)
@@ -619,7 +620,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
sprintf(type, "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
- rc = bnxt_alloc_ctx_mem_buf(type,
+ rc = bnxt_alloc_ctx_mem_buf(bp, type,
max_fc * 16,
&bp->flow_stat->rx_fc_out_tbl);
if (rc)
@@ -628,7 +629,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
sprintf(type, "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
/* 4 bytes for each counter-id */
- rc = bnxt_alloc_ctx_mem_buf(type,
+ rc = bnxt_alloc_ctx_mem_buf(bp, type,
max_fc * 4,
&bp->flow_stat->tx_fc_in_tbl);
if (rc)
@@ -637,7 +638,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
sprintf(type, "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
- rc = bnxt_alloc_ctx_mem_buf(type,
+ rc = bnxt_alloc_ctx_mem_buf(bp, type,
max_fc * 16,
&bp->flow_stat->tx_fc_out_tbl);
if (rc)
@@ -4518,7 +4519,7 @@ static int bnxt_alloc_ctx_mem_blk(struct bnxt *bp,
if (!mz) {
mz = rte_memzone_reserve_aligned(mz_name,
rmem->nr_pages * 8,
- SOCKET_ID_ANY,
+ bp->eth_dev->device->numa_node,
RTE_MEMZONE_2MB |
RTE_MEMZONE_SIZE_HINT_ONLY |
RTE_MEMZONE_IOVA_CONTIG,
@@ -4541,7 +4542,7 @@ static int bnxt_alloc_ctx_mem_blk(struct bnxt *bp,
if (!mz) {
mz = rte_memzone_reserve_aligned(mz_name,
mem_size,
-SOCKET_ID_ANY,
+bp->eth_dev->device->numa_node,
RTE_MEMZON
[dpdk-dev] [PATCH 0/4] net/bnxt: various fixes
Several fixes for the bnxt PMD: - Fix NUMA-aware memory allocations. - Fix transmit descriptor status implementation. - Fix handling of transmit completions in non-vector path. - Remove dead code. Lance Richardson (4): net/bnxt: fix ring and context memory allocation net/bnxt: fix tx desc status implementation net/bnxt: fix scalar Tx completion handling net/bnxt: remove dead code drivers/net/bnxt/bnxt_cpr.h | 17 --- drivers/net/bnxt/bnxt_ethdev.c| 71 +++ drivers/net/bnxt/bnxt_hwrm.c | 2 - drivers/net/bnxt/bnxt_ring.c | 31 ++-- drivers/net/bnxt/bnxt_ring.h | 2 +- drivers/net/bnxt/bnxt_rxq.c | 4 +- drivers/net/bnxt/bnxt_rxr.c | 11 - drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 3 -- drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 3 -- drivers/net/bnxt/bnxt_txq.c | 4 +- drivers/net/bnxt/bnxt_txr.c | 22 - drivers/net/bnxt/bnxt_vnic.c | 3 +- 12 files changed, 70 insertions(+), 103 deletions(-) -- 2.25.1
[dpdk-dev] [PATCH 3/4] net/bnxt: fix scalar Tx completion handling
Preserve the raw (unmasked) transmit completion ring
consumer index.
Remove cache prefetches that have no measurable performance
benefit.
Fixes: c7de4195cc4c ("net/bnxt: modify ring index logic")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_txr.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_txr.c b/drivers/net/bnxt/bnxt_txr.c
index 27459960d..54eaab34a 100644
--- a/drivers/net/bnxt/bnxt_txr.c
+++ b/drivers/net/bnxt/bnxt_txr.c
@@ -444,30 +444,26 @@ static void bnxt_tx_cmp(struct bnxt_tx_queue *txq, int
nr_pkts)
static int bnxt_handle_tx_cp(struct bnxt_tx_queue *txq)
{
+ uint32_t nb_tx_pkts = 0, cons, ring_mask, opaque;
struct bnxt_cp_ring_info *cpr = txq->cp_ring;
uint32_t raw_cons = cpr->cp_raw_cons;
- uint32_t cons;
- uint32_t nb_tx_pkts = 0;
+ struct bnxt_ring *cp_ring_struct;
struct tx_cmpl *txcmp;
- struct cmpl_base *cp_desc_ring = cpr->cp_desc_ring;
- struct bnxt_ring *cp_ring_struct = cpr->cp_ring_struct;
- uint32_t ring_mask = cp_ring_struct->ring_mask;
- uint32_t opaque = 0;
if (bnxt_tx_bds_in_hw(txq) < txq->tx_free_thresh)
return 0;
+ cp_ring_struct = cpr->cp_ring_struct;
+ ring_mask = cp_ring_struct->ring_mask;
+
do {
cons = RING_CMPL(ring_mask, raw_cons);
txcmp = (struct tx_cmpl *)&cpr->cp_desc_ring[cons];
- rte_prefetch_non_temporal(&cp_desc_ring[(cons + 2) &
- ring_mask]);
- if (!CMPL_VALID(txcmp, cpr->valid))
+ if (!CMP_VALID(txcmp, raw_cons, cp_ring_struct))
break;
- opaque = rte_cpu_to_le_32(txcmp->opaque);
- NEXT_CMPL(cpr, cons, cpr->valid, 1);
- rte_prefetch0(&cp_desc_ring[cons]);
+
+ opaque = rte_le_to_cpu_32(txcmp->opaque);
if (CMP_TYPE(txcmp) == TX_CMPL_TYPE_TX_L2)
nb_tx_pkts += opaque;
@@ -475,9 +471,11 @@ static int bnxt_handle_tx_cp(struct bnxt_tx_queue *txq)
RTE_LOG_DP(ERR, PMD,
"Unhandled CMP type %02x\n",
CMP_TYPE(txcmp));
- raw_cons = cons;
+ raw_cons = NEXT_RAW_CMP(raw_cons);
} while (nb_tx_pkts < ring_mask);
+ cpr->valid = !!(raw_cons & cp_ring_struct->ring_size);
+
if (nb_tx_pkts) {
if (txq->offloads & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
bnxt_tx_cmp_fast(txq, nb_tx_pkts);
--
2.25.1
[dpdk-dev] [PATCH 2/4] net/bnxt: fix tx desc status implementation
With tx completion batching, a single transmit completion
can correspond to one or more transmit descriptors, adjust
implementation to account for this.
RTE_ETH_TX_DESC_DONE should be returned for descriptors that
are available for use instead of RTE_ETH_TX_DESC_UNAVAIL.
Fixes: 5735eb241947 ("net/bnxt: support Tx batching")
Fixes: 478ed3bb7b9d "(net/bnxt: support Tx descriptor status")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 54 +++---
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index d4b8762d5..a4a0142e8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3296,41 +3296,49 @@ static int
bnxt_tx_descriptor_status_op(void *tx_queue, uint16_t offset)
{
struct bnxt_tx_queue *txq = (struct bnxt_tx_queue *)tx_queue;
- struct bnxt_tx_ring_info *txr;
- struct bnxt_cp_ring_info *cpr;
- struct rte_mbuf **tx_buf;
- struct tx_pkt_cmpl *txcmp;
- uint32_t cons, cp_cons;
+ struct bnxt_cp_ring_info *cpr = txq->cp_ring;
+ uint32_t ring_mask, raw_cons, nb_tx_pkts = 0;
+ struct bnxt_ring *cp_ring_struct;
+ struct cmpl_base *cp_desc_ring;
int rc;
- if (!txq)
- return -EINVAL;
-
rc = is_bnxt_in_error(txq->bp);
if (rc)
return rc;
- cpr = txq->cp_ring;
- txr = txq->tx_ring;
-
if (offset >= txq->nb_tx_desc)
return -EINVAL;
- cons = RING_CMP(cpr->cp_ring_struct, offset);
- txcmp = (struct tx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
- cp_cons = cpr->cp_raw_cons;
+ /* Return "desc done" if descriptor is available for use. */
+ if (bnxt_tx_bds_in_hw(txq) <= offset)
+ return RTE_ETH_TX_DESC_DONE;
- if (cons > cp_cons) {
- if (CMPL_VALID(txcmp, cpr->valid))
- return RTE_ETH_TX_DESC_UNAVAIL;
- } else {
- if (CMPL_VALID(txcmp, !cpr->valid))
- return RTE_ETH_TX_DESC_UNAVAIL;
+ raw_cons = cpr->cp_raw_cons;
+ cp_desc_ring = cpr->cp_desc_ring;
+ cp_ring_struct = cpr->cp_ring_struct;
+ ring_mask = cpr->cp_ring_struct->ring_mask;
+
+ /* Check to see if hw has posted a completion for the descriptor. */
+ while (1) {
+ struct tx_cmpl *txcmp;
+ uint32_t cons;
+
+ cons = RING_CMPL(ring_mask, raw_cons);
+ txcmp = (struct tx_cmpl *)&cp_desc_ring[cons];
+
+ if (!CMP_VALID(txcmp, raw_cons, cp_ring_struct))
+ break;
+
+ if (CMP_TYPE(txcmp) == TX_CMPL_TYPE_TX_L2)
+ nb_tx_pkts += rte_le_to_cpu_32(txcmp->opaque);
+
+ if (nb_tx_pkts > offset)
+ return RTE_ETH_TX_DESC_DONE;
+
+ raw_cons = NEXT_RAW_CMP(raw_cons);
}
- tx_buf = &txr->tx_buf_ring[cons];
- if (*tx_buf == NULL)
- return RTE_ETH_TX_DESC_DONE;
+ /* Descriptor is pending transmit, not yet completed by hardware. */
return RTE_ETH_TX_DESC_FULL;
}
--
2.25.1
[dpdk-dev] [PATCH 4/4] net/bnxt: remove dead code
Code related to maintaining completion ring "valid" state is
no longer needed, remove it.
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
Reviewed-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_cpr.h | 17 -
drivers/net/bnxt/bnxt_hwrm.c | 2 --
drivers/net/bnxt/bnxt_ring.c | 1 -
drivers/net/bnxt/bnxt_rxr.c | 11 ---
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 3 ---
drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 3 ---
drivers/net/bnxt/bnxt_txr.c | 2 --
7 files changed, 39 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
index 28c0a9049..2a56ec52c 100644
--- a/drivers/net/bnxt/bnxt_cpr.h
+++ b/drivers/net/bnxt/bnxt_cpr.h
@@ -15,14 +15,6 @@ struct bnxt_db_info;
(!!(rte_le_to_cpu_32(((struct cmpl_base *)(cmp))->info3_v) &\
CMPL_BASE_V) == !((raw_cons) & ((ring)->ring_size)))
-#define CMPL_VALID(cmp, v) \
- (!!(rte_le_to_cpu_32(((struct cmpl_base *)(cmp))->info3_v) &\
- CMPL_BASE_V) == !(v))
-
-#define NQ_CMP_VALID(nqcmp, raw_cons, ring)\
- (!!((nqcmp)->v & rte_cpu_to_le_32(NQ_CN_V)) == \
-!((raw_cons) & ((ring)->ring_size)))
-
#define CMP_TYPE(cmp) \
(((struct cmpl_base *)cmp)->type & CMPL_BASE_TYPE_MASK)
@@ -35,18 +27,10 @@ struct bnxt_db_info;
#define RING_CMP(ring, idx)((idx) & (ring)->ring_mask)
#define RING_CMPL(ring_mask, idx) ((idx) & (ring_mask))
#define NEXT_CMP(idx) RING_CMP(ADV_RAW_CMP(idx, 1))
-#define FLIP_VALID(cons, mask, val)((cons) >= (mask) ? !(val) : (val))
#define DB_CP_REARM_FLAGS (DB_KEY_CP | DB_IDX_VALID)
#define DB_CP_FLAGS(DB_KEY_CP | DB_IDX_VALID | DB_IRQ_DIS)
-#define NEXT_CMPL(cpr, idx, v, inc)do { \
- (idx) += (inc); \
- if (unlikely((idx) >= (cpr)->cp_ring_struct->ring_size)) { \
- (v) = !(v); \
- (idx) = 0; \
- } \
-} while (0)
#define B_CP_DB_REARM(cpr, raw_cons) \
rte_write32((DB_CP_REARM_FLAGS |\
DB_RING_IDX(&((cpr)->cp_db), raw_cons)),\
@@ -107,7 +91,6 @@ struct bnxt_cp_ring_info {
uint32_thw_stats_ctx_id;
struct bnxt_ring*cp_ring_struct;
- boolvalid;
};
#define RX_CMP_L2_ERRORS \
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 6eab2342f..9ec65ad22 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2670,7 +2670,6 @@ void bnxt_free_nq_ring(struct bnxt *bp, struct
bnxt_cp_ring_info *cpr)
memset(cpr->cp_desc_ring, 0, cpr->cp_ring_struct->ring_size *
sizeof(*cpr->cp_desc_ring));
cpr->cp_raw_cons = 0;
- cpr->valid = 0;
}
void bnxt_free_cp_ring(struct bnxt *bp, struct bnxt_cp_ring_info *cpr)
@@ -2684,7 +2683,6 @@ void bnxt_free_cp_ring(struct bnxt *bp, struct
bnxt_cp_ring_info *cpr)
memset(cpr->cp_desc_ring, 0, cpr->cp_ring_struct->ring_size *
sizeof(*cpr->cp_desc_ring));
cpr->cp_raw_cons = 0;
- cpr->valid = 0;
}
void bnxt_free_hwrm_rx_ring(struct bnxt *bp, int queue_index)
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 4a90ac264..cb18dfba7 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -769,7 +769,6 @@ int bnxt_alloc_async_cp_ring(struct bnxt *bp)
return rc;
cpr->cp_raw_cons = 0;
- cpr->valid = 0;
bnxt_set_db(bp, &cpr->cp_db, ring_type, 0,
cp_ring->fw_ring_id, cp_ring->ring_mask);
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 756a45ba9..0dee73af8 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -297,9 +297,6 @@ static int bnxt_agg_bufs_valid(struct bnxt_cp_ring_info
*cpr,
raw_cp_cons = ADV_RAW_CMP(raw_cp_cons, agg_bufs);
last_cp_cons = RING_CMP(cpr->cp_ring_struct, raw_cp_cons);
agg_cmpl = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[last_cp_cons];
- cpr->valid = FLIP_VALID(raw_cp_cons,
- cpr->cp_ring_struct->ring_mask,
- cpr->valid);
return CMP_VALID(agg_cmpl, raw_cp_cons, cpr->cp_ring_struct);
}
@@ -898,10 +895,6 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
if (!CMP_VALID(rxcmp1, tmp_raw_cons, cpr->cp_ring_struct))
return -EBUSY;
- cpr->valid = FLIP_VALID(cp_cons,
-
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/test: fix IPv6 header initialization
On Tue, May 11, 2021 at 10:42 AM Lance Richardson wrote: > > On Tue, May 11, 2021 at 10:31 AM David Marchand > wrote: > > > > On Fri, Mar 26, 2021 at 5:37 PM Lance Richardson > > wrote: > > > > > > Fix two issues found when writing PMD unit tests for HW ptype and > > > L4 checksum offload: > > > > Would those unit tests be interesting to other pmd driver writers? > > > I think so, although some adjustments would be needed to account > for differences in hardware capabilities. The tests I've written so far > are still very much a work in progress, but I hope to have something > ready for RFC in the near future. What is the current status of this patch? Thanks, Lance
[dpdk-dev] [PATCH 1/1] app/test: fix TCP header initialization
Initialize TCP data offset field with TCP header length, this
field is used to derive L4 header length and by hardware to
validate a TCP header.
Fixes: 41f72ec94074 ("test: add packet burst generator functions")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
app/test/packet_burst_generator.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/app/test/packet_burst_generator.c
b/app/test/packet_burst_generator.c
index 8b390853a2..0fd7290b0e 100644
--- a/app/test/packet_burst_generator.c
+++ b/app/test/packet_burst_generator.c
@@ -117,6 +117,7 @@ initialize_tcp_header(struct rte_tcp_hdr *tcp_hdr, uint16_t
src_port,
memset(tcp_hdr, 0, sizeof(struct rte_tcp_hdr));
tcp_hdr->src_port = rte_cpu_to_be_16(src_port);
tcp_hdr->dst_port = rte_cpu_to_be_16(dst_port);
+ tcp_hdr->data_off = (sizeof(struct rte_tcp_hdr) << 2) & 0xF0;
return pkt_len;
}
--
2.25.1
Re: [dpdk-dev] Build failures in 21.11-rc2
Do the machines used for the CentOS 8 CI builds support AVX512? The failing build is on an Ice Lake server with: meson -Dmachine=native build ninja -C build On Tue, Nov 9, 2021 at 11:08 AM Thomas Monjalon wrote: > > 09/11/2021 16:57, David Marchand: > > On Tue, Nov 9, 2021 at 4:53 PM Lance Richardson > > wrote: > > > > > > 21.11-rc2 builds are failing on CentOS 8.4, apparently because > > > the intrinsic _mm512_set_epi8() is not defined in the GCC > > > headers (see example below). > > > > Thanks for reporting Lance. > > It rings a bell, think it was mentionned in the past. > > CI compilation was passing: > http://mails.dpdk.org/archives/test-report/2021-November/238534.html > I would like to better understand. > > [...] > > > Here is a GCC patch to add this intrinsic: > > > https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg188664.html > > > > > > BTW, GCC 10.2.1 (via "scl enable gcc-toolset-10") is able to build > > > 21.11-rc2 successfully on CentOS 8.4. > > > > > > Lance > > > > > > # gcc --version > > > gcc (GCC) 8.4.1 20200928 (Red Hat 8.4.1-1) > [...] > > > ../lib/hash/rte_thash_x86_gfni.h:59:24: error: implicit declaration of > > > function ‘_mm512_set_epi8’; did you mean ‘_mm512_set1_epi8’? > > What is different in your setup compared to the CI? > >
Re: [dpdk-dev] Build failures in 21.11-rc2
Based on the CI build logs, the target architecture was Skylake, which supports AVX512 but doesn't support GFNI. On Tue, Nov 9, 2021 at 11:15 AM Thomas Monjalon wrote: > > 09/11/2021 17:12, Lance Richardson: > > Do the machines used for the CentOS 8 CI builds support AVX512? > > Probably not. OK thank you. > > > The failing build is on an Ice Lake server with: > > meson -Dmachine=native build > > ninja -C build > > > > On Tue, Nov 9, 2021 at 11:08 AM Thomas Monjalon wrote: > > > 09/11/2021 16:57, David Marchand: > > > > On Tue, Nov 9, 2021 at 4:53 PM Lance Richardson > > > > wrote: > > > > > > > > > > 21.11-rc2 builds are failing on CentOS 8.4, apparently because > > > > > the intrinsic _mm512_set_epi8() is not defined in the GCC > > > > > headers (see example below). > > > > > > > > Thanks for reporting Lance. > > > > It rings a bell, think it was mentionned in the past. > > > > > > CI compilation was passing: > > > http://mails.dpdk.org/archives/test-report/2021-November/238534.html > > > I would like to better understand. > > > > > > [...] > > > > > Here is a GCC patch to add this intrinsic: > > > > > https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg188664.html > > > > > > > > > > BTW, GCC 10.2.1 (via "scl enable gcc-toolset-10") is able to build > > > > > 21.11-rc2 successfully on CentOS 8.4. > > > > > > > > > > Lance > > > > > > > > > > # gcc --version > > > > > gcc (GCC) 8.4.1 20200928 (Red Hat 8.4.1-1) > > > [...] > > > > > ../lib/hash/rte_thash_x86_gfni.h:59:24: error: implicit declaration of > > > > > function ‘_mm512_set_epi8’; did you mean ‘_mm512_set1_epi8’? > > > > > > What is different in your setup compared to the CI? > > >
Re: [dpdk-dev] DPDK Max Mbuf Allocation
On Mon, Sep 13, 2021 at 11:51 AM Stephen Hemminger wrote: > > On Mon, 13 Sep 2021 16:43:18 +0100 > Ferruh Yigit wrote: > > > On 9/13/2021 5:56 AM, Kamaraj P wrote: > > > Hello All, > > > > > > Would like to understand or if there are any guidelines to allocate the > > > max > > > no of mbuf per NIC ? > > > For example, if i have defined as below: > > > #define RX_RING_SIZE 1024 > > > #define TX_RING_SIZE 1024 > > > > > > The Maximum RX/TX queues can be defined as 8 per NIC, What would be the > > > max > > > no of mbuf can be allocated per NIC ? > > > Please share if there is any guildliness or any limitation to increase the > > > mbuf ? > > > > > > > Hi Kamaraj, > > > > Max number of the queues and max number of the descriptors per queue > > depends to > > HW and changes form HW to HW. > > This information is shared by the PMDs that application needs to take into > > account. For example the descriptor limitations are provided by > > 'rx_desc_lim'/'tx_desc_lim' etc. > > > > After descriptor number is defined, testpmd uses the mbuf count as > > following, > > which can be taken as sample: > > > > nb_mbuf_per_pool = RTE_TEST_RX_DESC_MAX + RTE_TEST_TX_DESC_MAX + > > MAX_PKT_BURST + > >(nb_lcores * mb_mempool_cache); > > > > It is a a little more complicated since some devices (like bnxt) allocate > multiple mbuf's per packet. Something like +1, and it's worth noting that this makes it difficult to run many sample applications on the bnxt PMD. > > nb_mbuf_per_pool = MAX_RX_QUEUES * (RTE_TEST_RX_DESC_MAX * MBUF_PER_RX + > MBUF_PER_Q) > + MAX_TX_QUEUE * RTE_TEST_TX_DESC_MAX * MBUF_PER_TX > + nb_lcores * MAX_PKT_BURST > + nb_lcores * mb_mempool_cache > + nb_lcores * PKTMBUF_POOL_RESERVED; > > Ended up with >MBUF_PER_RX = 3 For releases up to around 20.11, 3 is the correct value (one mbuf per RX ring entry, two mbufs in each aggregation ring per RX ring entry). Currently the value for MBUF_PER_RX would be 5 (four mbufs in each aggregation ring for each RX ring entry). BTW, a future version will avoid populating aggregation rings with mbufs when LRO or scattered receive are not enabled. >MBUF_PER_Q = 6 Hmm, it's not clear where these would be allocated in the bnxt PMD. It seems to me that MBUF_PER_Q is zero for the bnxt PMD. > and when using jumbo >MBUF_PER_TX = MAX_MTU / MBUF_DATA_SIZE = 2 I don't think this is correct... the bnxt PMD allocates TX descriptor rings with the requested number of descriptors from tx_queue_setup(), this is the maximum number of mbufs that can be present in a TX ring. > > > > >
[dpdk-dev] [PATCH] net/bnxt: fix Rx queue startup state
Since the addition of support for runtime queue setup,
receive queues that are started by default no longer
have the correct state. Fix this by setting the state
when a port is started.
Fixes: 0105ea1296c9 ("net/bnxt: support runtime queue setup")
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
Reviewed-by: Somnath Kotur
Reviewed-by: Kalesh Anakkur Purayil
---
drivers/net/bnxt/bnxt_ethdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index d6e3847963..097dd10de9 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -482,6 +482,12 @@ static int bnxt_setup_one_vnic(struct bnxt *bp, uint16_t
vnic_id)
rxq->vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
else
vnic->rx_queue_cnt++;
+
+ if (!rxq->rx_deferred_start) {
+ bp->eth_dev->data->rx_queue_state[j] =
+ RTE_ETH_QUEUE_STATE_STARTED;
+ rxq->rx_started = true;
+ }
}
PMD_DRV_LOG(DEBUG, "vnic->rx_queue_cnt = %d\n", vnic->rx_queue_cnt);
--
2.25.1
[dpdk-dev] [PATCH] net: fix checksum API documentation
Minor corrections and improvements to documentation
for checksum APIs.
Fixes: 6006818cfb26 ("net: new checksum functions")
Fixes: 45a08ef55e44 ("net: introduce functions to verify L4 checksums")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
lib/net/rte_ip.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index 05948b69b7..fd08ea31b2 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -488,7 +488,7 @@ rte_ipv6_phdr_cksum(const struct rte_ipv6_hdr *ipv6_hdr,
uint64_t ol_flags)
}
/**
- * @internal Calculate the non-complemented IPv4 L4 checksum
+ * @internal Calculate the non-complemented IPv6 L4 checksum
*/
static inline uint16_t
__rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void
*l4_hdr)
@@ -509,15 +509,15 @@ __rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr
*ipv6_hdr, const void *l4_hdr)
/**
* Process the IPv6 UDP or TCP checksum.
*
- * The IPv4 header should not contains options. The layer 4 checksum
- * must be set to 0 in the packet by the caller.
+ * The IPv6 header must not be followed by extension headers. The layer 4
+ * checksum must be set to 0 in the L4 header by the caller.
*
* @param ipv6_hdr
* The pointer to the contiguous IPv6 header.
* @param l4_hdr
* The pointer to the beginning of the L4 header.
* @return
- * The complemented checksum to set in the IP packet.
+ * The complemented checksum to set in the L4 header.
*/
static inline uint16_t
rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
--
2.25.1
Re: [dpdk-dev] [dpdk-stable] [PATCH] net: fix checksum API documentation
On Tue, Sep 21, 2021 at 6:11 AM Morten Brørup wrote: > Ferruh, the description of the return value is technically correct; it > mentions the IP packet, not the IP header. > So this change is a clarification only. > > However, I agree that the same clarification would also benefit > 'rte_ipv4_udptcp_cksum()'. > Sure, will fix in v2. Also for rte_ipv4_udptcp_cksum(), a comment says "The IP and layer 4 checksum must be set to zero", but only the layer 4 checksum actually needs to be set to zero. I'll fix that at the same time. Thanks, Lance
[dpdk-dev] [PATCH v2] net: fix checksum API documentation
Minor corrections and improvements to documentation
for checksum APIs.
Fixes: 6006818cfb26 ("net: new checksum functions")
Fixes: 45a08ef55e44 ("net: introduce functions to verify L4 checksums")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
v2:
- Updates for rte_ipv4_udptcp_cksum()
lib/net/rte_ip.h | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index 05948b69b7..bc3dd44a4c 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -371,15 +371,14 @@ __rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr
*ipv4_hdr, const void *l4_hdr)
/**
* Process the IPv4 UDP or TCP checksum.
*
- * The IP and layer 4 checksum must be set to 0 in the packet by
- * the caller.
+ * The layer 4 checksum must be set to 0 in the L4 header by the caller.
*
* @param ipv4_hdr
* The pointer to the contiguous IPv4 header.
* @param l4_hdr
* The pointer to the beginning of the L4 header.
* @return
- * The complemented checksum to set in the IP packet.
+ * The complemented checksum to set in the L4 header.
*/
static inline uint16_t
rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
@@ -488,7 +487,7 @@ rte_ipv6_phdr_cksum(const struct rte_ipv6_hdr *ipv6_hdr,
uint64_t ol_flags)
}
/**
- * @internal Calculate the non-complemented IPv4 L4 checksum
+ * @internal Calculate the non-complemented IPv6 L4 checksum
*/
static inline uint16_t
__rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void
*l4_hdr)
@@ -509,15 +508,15 @@ __rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr
*ipv6_hdr, const void *l4_hdr)
/**
* Process the IPv6 UDP or TCP checksum.
*
- * The IPv4 header should not contains options. The layer 4 checksum
- * must be set to 0 in the packet by the caller.
+ * The IPv6 header must not be followed by extension headers. The layer 4
+ * checksum must be set to 0 in the L4 header by the caller.
*
* @param ipv6_hdr
* The pointer to the contiguous IPv6 header.
* @param l4_hdr
* The pointer to the beginning of the L4 header.
* @return
- * The complemented checksum to set in the IP packet.
+ * The complemented checksum to set in the L4 header.
*/
static inline uint16_t
rte_ipv6_udptcp_cksum(const struct rte_ipv6_hdr *ipv6_hdr, const void *l4_hdr)
--
2.25.1
Re: [dpdk-dev] [PATCH v3 2/2] net/i40e: replace SMP barrier with thread fence
On Thu, Jul 8, 2021 at 8:09 AM Zhang, Qi Z wrote: > > > > > -Original Message- > > From: Joyce Kong > > Sent: Tuesday, July 6, 2021 2:54 PM > > To: Xing, Beilei ; Zhang, Qi Z > > ; > > ruifeng.w...@arm.com; honnappa.nagaraha...@arm.com; Richardson, Bruce > > ; Zhang, Helin > > Cc: dev@dpdk.org; sta...@dpdk.org; n...@arm.com > > Subject: [PATCH v3 2/2] net/i40e: replace SMP barrier with thread fence > > > > Simply replace the SMP barrier with atomic thread fence for i40e hw ring > > sacn, > > if there is no synchronization point. > > > > Signed-off-by: Joyce Kong > > Reviewed-by: Ruifeng Wang > > --- > > drivers/net/i40e/i40e_rxtx.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c > > index > > 9aaabfd92..86e2f083e 100644 > > --- a/drivers/net/i40e/i40e_rxtx.c > > +++ b/drivers/net/i40e/i40e_rxtx.c > > @@ -482,7 +482,8 @@ i40e_rx_scan_hw_ring(struct i40e_rx_queue *rxq) > > I40E_RXD_QW1_STATUS_SHIFT; > > } > > > > - rte_smp_rmb(); > > + /* This barrier is to order loads of different words in the > > descriptor */ > > + rte_atomic_thread_fence(__ATOMIC_ACQUIRE); > > Now for x86, you actually replace a compiler barrier with a memory fence, > this may have potential performance impact which need additional resource to > investigate No memory fence instruction is generated for __ATOMIC_ACQUIRE on x86 for any version of gcc or clang that I've tried, based on experiments here: https://godbolt.org/z/Yxr1vGhKP
[dpdk-dev] [PATCH] net/bnxt: fix missing barriers in completion handling
Ensure that Rx/Tx/Async completion entry fields are accessed
only after the completion's valid flag has been loaded and
verified. This is needed for correct operation on systems that
use relaxed memory consistency models.
Fixes: 2eb53b134aae ("net/bnxt: add initial Rx code")
Fixes: 6eb3cc2294fd ("net/bnxt: add initial Tx code")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_cpr.h | 36 ---
drivers/net/bnxt/bnxt_ethdev.c| 16 ++--
drivers/net/bnxt/bnxt_irq.c | 7 +++---
drivers/net/bnxt/bnxt_rxr.c | 9 ---
drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 2 +-
drivers/net/bnxt/bnxt_txr.c | 2 +-
8 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
index 2a56ec52c..3ee6b74bc 100644
--- a/drivers/net/bnxt/bnxt_cpr.h
+++ b/drivers/net/bnxt/bnxt_cpr.h
@@ -8,13 +8,10 @@
#include
#include
+#include "hsi_struct_def_dpdk.h"
struct bnxt_db_info;
-#define CMP_VALID(cmp, raw_cons, ring) \
- (!!(rte_le_to_cpu_32(((struct cmpl_base *)(cmp))->info3_v) &\
- CMPL_BASE_V) == !((raw_cons) & ((ring)->ring_size)))
-
#define CMP_TYPE(cmp) \
(((struct cmpl_base *)cmp)->type & CMPL_BASE_TYPE_MASK)
@@ -121,4 +118,35 @@ bool bnxt_is_recovery_enabled(struct bnxt *bp);
bool bnxt_is_master_func(struct bnxt *bp);
void bnxt_stop_rxtx(struct bnxt *bp);
+
+/**
+ * Check validity of a completion ring entry. If the entry is valid, include a
+ * C11 __ATOMIC_ACQUIRE fence to ensure that subsequent loads of fields in the
+ * completion are not hoisted by the compiler or by the CPU to come before the
+ * loading of the "valid" field.
+ *
+ * Note: the caller must not access any fields in the specified completion
+ * entry prior to calling this function.
+ *
+ * @param cmp
+ * Pointer to an entry in the completion ring.
+ * @param raw_cons
+ * Raw consumer index of entry in completion ring.
+ * @param ring_size
+ * Size of completion ring.
+ */
+static __rte_always_inline bool
+bnxt_cpr_cmp_valid(const void *cmpl, uint32_t raw_cons, uint32_t ring_size)
+{
+ const struct cmpl_base *c = cmpl;
+ bool expected, valid;
+
+ expected = !(raw_cons & ring_size);
+ valid = !!(rte_le_to_cpu_32(c->info3_v) & CMPL_BASE_V);
+ if (valid == expected) {
+ rte_atomic_thread_fence(__ATOMIC_ACQUIRE);
+ return true;
+ }
+ return false;
+}
#endif
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ed09f1bf5..ee6929692 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3126,7 +3126,7 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
{
struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
struct bnxt_cp_ring_info *cpr;
- uint32_t desc = 0, raw_cons;
+ uint32_t desc = 0, raw_cons, cp_ring_size;
struct bnxt_rx_queue *rxq;
struct rx_pkt_cmpl *rxcmp;
int rc;
@@ -3138,6 +3138,7 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
rxq = dev->data->rx_queues[rx_queue_id];
cpr = rxq->cp_ring;
raw_cons = cpr->cp_raw_cons;
+ cp_ring_size = cpr->cp_ring_struct->ring_size;
while (1) {
uint32_t agg_cnt, cons, cmpl_type;
@@ -3145,7 +3146,7 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
- if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
+ if (!bnxt_cpr_cmp_valid(rxcmp, raw_cons, cp_ring_size))
break;
cmpl_type = CMP_TYPE(rxcmp);
@@ -3189,7 +3190,7 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t
offset)
struct bnxt_rx_queue *rxq = rx_queue;
struct bnxt_cp_ring_info *cpr;
struct bnxt_rx_ring_info *rxr;
- uint32_t desc, raw_cons;
+ uint32_t desc, raw_cons, cp_ring_size;
struct bnxt *bp = rxq->bp;
struct rx_pkt_cmpl *rxcmp;
int rc;
@@ -3203,6 +3204,7 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t
offset)
rxr = rxq->rx_ring;
cpr = rxq->cp_ring;
+ cp_ring_size = cpr->cp_ring_struct->ring_size;
/*
* For the vector receive case, the completion at the requested
@@ -3219,7 +3221,7 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t
offset)
cons = RING_CMP(cpr->cp_ring_struct
Re: [dpdk-dev] [PATCH] net/bnxt: fix missing barriers in completion handling
On Fri, Jul 9, 2021 at 2:00 AM Ruifeng Wang wrote: > > > +/** > > + * Check validity of a completion ring entry. If the entry is valid, > > include a > > + * C11 __ATOMIC_ACQUIRE fence to ensure that subsequent loads of fields > > in the > > + * completion are not hoisted by the compiler or by the CPU to come before > > the > > + * loading of the "valid" field. > > + * > > + * Note: the caller must not access any fields in the specified completion > > + * entry prior to calling this function. > > + * > > + * @param cmp > Nit, cmpl Thanks, good catch. I'll fix this in v2. > > > > > /* Check to see if hw has posted a completion for the descriptor. */ > > @@ -3327,7 +3327,7 @@ bnxt_tx_descriptor_status_op(void *tx_queue, > > uint16_t offset) > > cons = RING_CMPL(ring_mask, raw_cons); > > txcmp = (struct tx_cmpl *)&cp_desc_ring[cons]; > > > > - if (!CMP_VALID(txcmp, raw_cons, cp_ring_struct)) > > + if (!bnxt_cpr_cmp_valid(txcmp, raw_cons, ring_mask + 1)) > cpr->cp_ring_struct->ring_size can be used instead of 'ring_mask + 1'? > > > break; > > > > if (CMP_TYPE(txcmp) == TX_CMPL_TYPE_TX_L2) > > > > > diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_neon.c > > b/drivers/net/bnxt/bnxt_rxtx_vec_neon.c > > index 263e6ec3c..13211060c 100644 > > --- a/drivers/net/bnxt/bnxt_rxtx_vec_neon.c > > +++ b/drivers/net/bnxt/bnxt_rxtx_vec_neon.c > > @@ -339,7 +339,7 @@ bnxt_handle_tx_cp_vec(struct bnxt_tx_queue *txq) > > cons = RING_CMPL(ring_mask, raw_cons); > > txcmp = (struct tx_cmpl *)&cp_desc_ring[cons]; > > > > - if (!CMP_VALID(txcmp, raw_cons, cp_ring_struct)) > > + if (!bnxt_cpr_cmp_valid(txcmp, raw_cons, ring_mask + 1)) > Same here. I think cpr->cp_ring_struct->ring_size can be used and it avoids > calculation. > Also some places in other vector files. It's true that cpr->cp_ring_struct->ring_size and ring_mask + 1 are equivalent, but there doesn't seem to be a meaningful difference between the two in the generated code. Based on disassembly of x86 and Arm code for this function, the compiler correctly determines that the value of ring_mask + 1 doesn't change within the loop, so it is only computed once. The only difference would be in whether an add instruction or a load instruction is used to put the value in the register.
Re: [dpdk-dev] [PATCH] net/bnxt: fix build failure
On Fri, Jul 9, 2021 at 4:48 AM Thomas Monjalon wrote: > > 09/07/2021 00:49, Ajit Khaparde: > > Fix build failures because of uninitialized variable usage. > > You should add the error log here. > You don't mention the condition of failure. > Nobody reproduced a failure so far. Hi Thomas, This addresses FC34 (which I think is using gcc11) build failures, there is an example here: http://mails.dpdk.org/archives/test-report/2021-July/203186.html Regards, Lance
[dpdk-dev] [PATCH v2] net/bnxt: fix missing barriers in completion handling
Ensure that Rx/Tx/Async completion entry fields are accessed
only after the completion's valid flag has been loaded and
verified. This is needed for correct operation on systems that
use relaxed memory consistency models.
Fixes: 2eb53b134aae ("net/bnxt: add initial Rx code")
Fixes: 6eb3cc2294fd ("net/bnxt: add initial Tx code")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
---
v2:
* Corrected name of the first parameter to bnxt_cpr_cmp_valid() in
comments ('cmp' to 'cmpl').
drivers/net/bnxt/bnxt_cpr.h | 36 ---
drivers/net/bnxt/bnxt_ethdev.c| 16 ++--
drivers/net/bnxt/bnxt_irq.c | 7 +++---
drivers/net/bnxt/bnxt_rxr.c | 9 ---
drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 2 +-
drivers/net/bnxt/bnxt_txr.c | 2 +-
8 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
index 2a56ec52c..4095c8c40 100644
--- a/drivers/net/bnxt/bnxt_cpr.h
+++ b/drivers/net/bnxt/bnxt_cpr.h
@@ -8,13 +8,10 @@
#include
#include
+#include "hsi_struct_def_dpdk.h"
struct bnxt_db_info;
-#define CMP_VALID(cmp, raw_cons, ring) \
- (!!(rte_le_to_cpu_32(((struct cmpl_base *)(cmp))->info3_v) &\
- CMPL_BASE_V) == !((raw_cons) & ((ring)->ring_size)))
-
#define CMP_TYPE(cmp) \
(((struct cmpl_base *)cmp)->type & CMPL_BASE_TYPE_MASK)
@@ -121,4 +118,35 @@ bool bnxt_is_recovery_enabled(struct bnxt *bp);
bool bnxt_is_master_func(struct bnxt *bp);
void bnxt_stop_rxtx(struct bnxt *bp);
+
+/**
+ * Check validity of a completion ring entry. If the entry is valid, include a
+ * C11 __ATOMIC_ACQUIRE fence to ensure that subsequent loads of fields in the
+ * completion are not hoisted by the compiler or by the CPU to come before the
+ * loading of the "valid" field.
+ *
+ * Note: the caller must not access any fields in the specified completion
+ * entry prior to calling this function.
+ *
+ * @param cmpl
+ * Pointer to an entry in the completion ring.
+ * @param raw_cons
+ * Raw consumer index of entry in completion ring.
+ * @param ring_size
+ * Size of completion ring.
+ */
+static __rte_always_inline bool
+bnxt_cpr_cmp_valid(const void *cmpl, uint32_t raw_cons, uint32_t ring_size)
+{
+ const struct cmpl_base *c = cmpl;
+ bool expected, valid;
+
+ expected = !(raw_cons & ring_size);
+ valid = !!(rte_le_to_cpu_32(c->info3_v) & CMPL_BASE_V);
+ if (valid == expected) {
+ rte_atomic_thread_fence(__ATOMIC_ACQUIRE);
+ return true;
+ }
+ return false;
+}
#endif
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ed09f1bf5..ee6929692 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3126,7 +3126,7 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
{
struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
struct bnxt_cp_ring_info *cpr;
- uint32_t desc = 0, raw_cons;
+ uint32_t desc = 0, raw_cons, cp_ring_size;
struct bnxt_rx_queue *rxq;
struct rx_pkt_cmpl *rxcmp;
int rc;
@@ -3138,6 +3138,7 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
rxq = dev->data->rx_queues[rx_queue_id];
cpr = rxq->cp_ring;
raw_cons = cpr->cp_raw_cons;
+ cp_ring_size = cpr->cp_ring_struct->ring_size;
while (1) {
uint32_t agg_cnt, cons, cmpl_type;
@@ -3145,7 +3146,7 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t
rx_queue_id)
cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
- if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
+ if (!bnxt_cpr_cmp_valid(rxcmp, raw_cons, cp_ring_size))
break;
cmpl_type = CMP_TYPE(rxcmp);
@@ -3189,7 +3190,7 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t
offset)
struct bnxt_rx_queue *rxq = rx_queue;
struct bnxt_cp_ring_info *cpr;
struct bnxt_rx_ring_info *rxr;
- uint32_t desc, raw_cons;
+ uint32_t desc, raw_cons, cp_ring_size;
struct bnxt *bp = rxq->bp;
struct rx_pkt_cmpl *rxcmp;
int rc;
@@ -3203,6 +3204,7 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t
offset)
rxr = rxq->rx_ring;
cpr = rxq->cp_ring;
+ cp_ring_size = cpr->cp_ring_struct->ring_size;
/*
* For the vector receive case, the completion at the request
Re: [dpdk-dev] RHEL 7 support
On Tue, Jul 13, 2021 at 5:48 AM Kevin Traynor wrote:
>
> On 12/07/2021 18:34, Honnappa Nagarahalli wrote:
> >
> >
> >>
> >> Hi,
> >>
> >> I would like to open a discussion about RHEL 7 support in DPDK.
> >> How long do we want to support it in new DPDK versions?
> >> Can we drop RHEL 7 support starting DPDK 21.11?
> > I think the concerns were from Redhat and Intel.
> >
>
> I think it's fine to drop from a DPDK distro package view. The DPDK
> package in RHEL7 won't be updated to new main releases anymore.
>
> It's not quite as clear for a user compiling their own DPDK. CentOS7 is
> in maintenance mode until mid 2024 and will only get security or
> critical fixes.
>
> You could still have a user on CentOS7 compiling their own DPDK from the
> latest tarballs. It feels like an unlikely combination but maybe someone
> knows of a user like that.
>
> As the distro version won't get latest software packages, it seems
> reasonable that latest DPDK is not supported either, but there would
> need to be some advance warning.
>
> 20.11 LTS will provide bugfix/security that should be ok on CentOS7
> until Jan 2023.
>
> >>
> >> If we decide to drop RHEL 7 support, does it mean we can generally use
> >> standard C11 atomics?
> > What is the next RHEL version we support officially?
>
> RHEL8 is supported.
>
> > If we adopt C11 atomic APIs, we need to think about windows support too.
> >
> >>
> >> What other benefits or impacts can you think about?
> >>
> >
>
Are the issues with continuing to support RHEL 7 all related to the default
gcc version in RHEL 7 (the structure initialization to {0} issue and lack of
etc. apply to 4.8 but not 4.9+), or are there also lib/kernel issues?
The minimum supported gcc version is already documented to be 4.9+, which
seems to rule out RHEL 7 unless a newer version of gcc is used (which is
easy to do by installing one of the "software collection" toolset packages,
not sure if that is a valid option to suggest).
[PATCH 0/2] net/bnxt: minor performance fixes for AVX2 Rx
This series contains two minor performance fixes for the bnxt AVX2 vecgtorized burst receive function. Lance Richardson (2): net/bnxt: avoid unnecessary work in AVX2 Rx path net/bnxt: remove software prefetches from AVX2 Rx path drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) -- 2.25.1 smime.p7s Description: S/MIME Cryptographic Signature
[PATCH 1/2] net/bnxt: avoid unnecessary work in AVX2 Rx path
Each call to the AVX2 vector burst receive function makes at
least one pass through the function's inner loop, loading
256 bytes of completion descriptors and copying 8 rte_mbuf
pointers regardless of whether there are any packets to be
received.
Unidirectional forwarding performance is improved by about
3-4% if we ensure that at least one packet can be received
before entering the inner loop.
Fixes: c4e4c18963b0 ("net/bnxt: add AVX2 RX/Tx")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
b/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
index e4905b4fd1..54e3af22ac 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
@@ -98,6 +98,10 @@ recv_burst_vec_avx2(void *rx_queue, struct rte_mbuf
**rx_pkts, uint16_t nb_pkts)
rte_prefetch0(&cp_desc_ring[cons + 8]);
rte_prefetch0(&cp_desc_ring[cons + 12]);
+ /* Return immediately if there is not at least one completed packet. */
+ if (!bnxt_cpr_cmp_valid(&cp_desc_ring[cons], raw_cons, cp_ring_size))
+ return 0;
+
/* Ensure that we do not go past the ends of the rings. */
nb_pkts = RTE_MIN(nb_pkts, RTE_MIN(rx_ring_size - mbcons,
(cp_ring_size - cons) / 2));
--
2.25.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH 2/2] net/bnxt: remove software prefetches from AVX2 Rx path
Testing has shown no performance benefit from software prefetching
of receive completion descriptors in the AVX2 burst receive path,
and slightly better performance without them on some CPU families,
so this patch removes them.
Fixes: c4e4c18963b0 ("net/bnxt: add AVX2 RX/Tx")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
b/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
index 54e3af22ac..34bd22edf0 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
@@ -92,12 +92,6 @@ recv_burst_vec_avx2(void *rx_queue, struct rte_mbuf
**rx_pkts, uint16_t nb_pkts)
cons = raw_cons & (cp_ring_size - 1);
mbcons = (raw_cons / 2) & (rx_ring_size - 1);
- /* Prefetch first four descriptor pairs. */
- rte_prefetch0(&cp_desc_ring[cons + 0]);
- rte_prefetch0(&cp_desc_ring[cons + 4]);
- rte_prefetch0(&cp_desc_ring[cons + 8]);
- rte_prefetch0(&cp_desc_ring[cons + 12]);
-
/* Return immediately if there is not at least one completed packet. */
if (!bnxt_cpr_cmp_valid(&cp_desc_ring[cons], raw_cons, cp_ring_size))
return 0;
@@ -136,14 +130,6 @@ recv_burst_vec_avx2(void *rx_queue, struct rte_mbuf
**rx_pkts, uint16_t nb_pkts)
_mm256_storeu_si256((void *)&rx_pkts[i + 4], t0);
#endif
- /* Prefetch eight descriptor pairs for next iteration. */
- if (i + BNXT_RX_DESCS_PER_LOOP_VEC256 < nb_pkts) {
- rte_prefetch0(&cp_desc_ring[cons + 16]);
- rte_prefetch0(&cp_desc_ring[cons + 20]);
- rte_prefetch0(&cp_desc_ring[cons + 24]);
- rte_prefetch0(&cp_desc_ring[cons + 28]);
- }
-
/*
* Load eight receive completion descriptors into 256-bit
* registers. Loads are issued in reverse order in order to
--
2.25.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH] net/bnxt: fix xstats get names implementation
When the xstats_names parameter to rte_eth_xstats_get_names()
is non-NULL and the size parameter is less than the required
number of entries, the driver must return the required size
without modifying (and over-running) the caller's xstats_names
array.
Update bnxt_dev_xstats_get_names_op() in accordance with this
requirement.
Fixes: bfb9c2260be2 ("net/bnxt: support xstats get/reset")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
drivers/net/bnxt/bnxt_stats.c | 93 +--
1 file changed, 46 insertions(+), 47 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index 991eafc644..197fd7c02b 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -846,7 +846,7 @@ int bnxt_flow_stats_cnt(struct bnxt *bp)
int bnxt_dev_xstats_get_names_op(struct rte_eth_dev *eth_dev,
struct rte_eth_xstat_name *xstats_names,
- __rte_unused unsigned int limit)
+ unsigned int size)
{
struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
const unsigned int stat_cnt = RTE_DIM(bnxt_rx_stats_strings) +
@@ -862,63 +862,62 @@ int bnxt_dev_xstats_get_names_op(struct rte_eth_dev
*eth_dev,
if (rc)
return rc;
- if (xstats_names != NULL) {
- count = 0;
+ if (xstats_names == NULL || size < stat_cnt)
+ return stat_cnt;
- for (i = 0; i < RTE_DIM(bnxt_rx_stats_strings); i++) {
- strlcpy(xstats_names[count].name,
- bnxt_rx_stats_strings[i].name,
- sizeof(xstats_names[count].name));
- count++;
- }
+ for (i = 0; i < RTE_DIM(bnxt_rx_stats_strings); i++) {
+ strlcpy(xstats_names[count].name,
+ bnxt_rx_stats_strings[i].name,
+ sizeof(xstats_names[count].name));
+ count++;
+ }
- for (i = 0; i < RTE_DIM(bnxt_tx_stats_strings); i++) {
- strlcpy(xstats_names[count].name,
- bnxt_tx_stats_strings[i].name,
- sizeof(xstats_names[count].name));
- count++;
- }
+ for (i = 0; i < RTE_DIM(bnxt_tx_stats_strings); i++) {
+ strlcpy(xstats_names[count].name,
+ bnxt_tx_stats_strings[i].name,
+ sizeof(xstats_names[count].name));
+ count++;
+ }
- for (i = 0; i < RTE_DIM(bnxt_func_stats_strings); i++) {
- strlcpy(xstats_names[count].name,
- bnxt_func_stats_strings[i].name,
- sizeof(xstats_names[count].name));
- count++;
- }
+ for (i = 0; i < RTE_DIM(bnxt_func_stats_strings); i++) {
+ strlcpy(xstats_names[count].name,
+ bnxt_func_stats_strings[i].name,
+ sizeof(xstats_names[count].name));
+ count++;
+ }
- for (i = 0; i < RTE_DIM(bnxt_rx_ext_stats_strings); i++) {
- strlcpy(xstats_names[count].name,
- bnxt_rx_ext_stats_strings[i].name,
- sizeof(xstats_names[count].name));
+ for (i = 0; i < RTE_DIM(bnxt_rx_ext_stats_strings); i++) {
+ strlcpy(xstats_names[count].name,
+ bnxt_rx_ext_stats_strings[i].name,
+ sizeof(xstats_names[count].name));
- count++;
- }
+ count++;
+ }
- for (i = 0; i < RTE_DIM(bnxt_tx_ext_stats_strings); i++) {
- strlcpy(xstats_names[count].name,
- bnxt_tx_ext_stats_strings[i].name,
- sizeof(xstats_names[count].name));
+ for (i = 0; i < RTE_DIM(bnxt_tx_ext_stats_strings); i++) {
+ strlcpy(xstats_names[count].name,
+ bnxt_tx_ext_stats_strings[i].name,
+ sizeof(xstats_names[count].name));
- count++;
- }
+ count++;
+ }
- if (bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_COUNTERS &&
- bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_MGMT &&
- BNXT_FLOW_XSTATS_EN(bp)) {
- for (i = 0; i < bp->max_l2_ctx; i++) {
- char buf[RTE_ETH_XSTATS_NAME_SIZE];
+ if (bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_COUNTERS &&
+ bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_MGMT &&
+ BNXT_FLOW_XSTATS_EN(bp)) {
+ for (i = 0; i < bp->max_l2_ctx; i++) {
Using IOAT PMD
Hi Bruce, I've been looking into using the IOAT PMD, initially with dma_autotest and the dpdk-dma example application. These seem to work fine on SKX with the current main branch, but when I try the same procedure on ICX (binding all 8 devices to vfio-pci in both cases), I get the following output for each device when probed. Is something different needed when using IOAT on ICX vs. SKX? Thanks, Lance EAL: Probe PCI driver: dmadev_ioat (8086:b00) device: :80:01.0 (socket 2) IOAT: ioat_dmadev_probe(): Init :80:01.0 on NUMA node 2 IOAT: ioat_dmadev_create(): ioat_dmadev_create: Channel count == 255 IOAT: ioat_dmadev_create(): ioat_dmadev_create: Channel appears locked IOAT: ioat_dmadev_create(): ioat_dmadev_create: cannot reset device. CHANCMD=0xff, CHANSTS=0x, CHANERR=0x EAL: Releasing PCI mapped resource for :80:01.0 EAL: Calling pci_unmap_resource for :80:01.0 at 0x410243 EAL: Requested device :80:01.0 cannot be used
Re: Using IOAT PMD
On Thu, Dec 16, 2021 at 11:20 AM Bruce Richardson wrote: > > On Thu, Dec 16, 2021 at 11:04:54AM -0500, Lance Richardson wrote: > > Hi Bruce, > > > > I've been looking into using the IOAT PMD, initially with dma_autotest > > and the dpdk-dma example application. These seem to work fine on > > SKX with the current main branch, but when I try the same procedure > > on ICX (binding all 8 devices to vfio-pci in both cases), I get the > > following > > output for each device when probed. Is something different needed when > > using IOAT on ICX vs. SKX? > > > > Thanks, > > Lance > > > > EAL: Probe PCI driver: dmadev_ioat (8086:b00) device: :80:01.0 (socket > > 2) > > IOAT: ioat_dmadev_probe(): Init :80:01.0 on NUMA node 2 > > IOAT: ioat_dmadev_create(): ioat_dmadev_create: Channel count == 255 > > > > IOAT: ioat_dmadev_create(): ioat_dmadev_create: Channel appears locked > > > > IOAT: ioat_dmadev_create(): ioat_dmadev_create: cannot reset device. > > CHANCMD=0xff, CHANSTS=0x, CHANERR=0x > > > > EAL: Releasing PCI mapped resource for :80:01.0 > > EAL: Calling pci_unmap_resource for :80:01.0 at 0x410243 > > EAL: Requested device :80:01.0 cannot be used > > That is strange, the same PMD should work ok on both platforms. This is all > on latest branch, right? Let me attempt to reproduce and get back to you. Hi Bruce, That's correct, I'm using the current tip of the main branch, which seems to be identical to 21.11.0. > > /Bruce > > PS: Is this a 4-socket system you are running on, since I see "socket 2" > being described as the socket number for device 80:01.0? > It is a two-socket system with sub-NUMA enabled, so it appears as four NUMA nodes. I'm only binding the devices on the second socket. Thanks, Lance
Re: Using IOAT PMD
On Thu, Dec 16, 2021 at 12:33 PM Bruce Richardson wrote: > > On Thu, Dec 16, 2021 at 11:34:25AM -0500, Lance Richardson wrote: > > On Thu, Dec 16, 2021 at 11:20 AM Bruce Richardson > > wrote: > > > > > > On Thu, Dec 16, 2021 at 11:04:54AM -0500, Lance Richardson wrote: > > > > Hi Bruce, > > > > > > > > I've been looking into using the IOAT PMD, initially with dma_autotest > > > > and the dpdk-dma example application. These seem to work fine on > > > > SKX with the current main branch, but when I try the same procedure > > > > on ICX (binding all 8 devices to vfio-pci in both cases), I get the > > > > following > > > > output for each device when probed. Is something different needed when > > > > using IOAT on ICX vs. SKX? > > > > > > > > Thanks, > > > > Lance > > > > > > > > EAL: Probe PCI driver: dmadev_ioat (8086:b00) device: :80:01.0 > > > > (socket 2) > > > > IOAT: ioat_dmadev_probe(): Init :80:01.0 on NUMA node 2 > > > > IOAT: ioat_dmadev_create(): ioat_dmadev_create: Channel count == 255 > > > > > > > > IOAT: ioat_dmadev_create(): ioat_dmadev_create: Channel appears locked > > > > > > > > IOAT: ioat_dmadev_create(): ioat_dmadev_create: cannot reset device. > > > > CHANCMD=0xff, CHANSTS=0x, CHANERR=0x > > > > > > > > EAL: Releasing PCI mapped resource for :80:01.0 > > > > EAL: Calling pci_unmap_resource for :80:01.0 at 0x410243 > > > > EAL: Requested device :80:01.0 cannot be used > > > > > > That is strange, the same PMD should work ok on both platforms. This is > > > all > > > on latest branch, right? Let me attempt to reproduce and get back to you. > > > > Hi Bruce, > > > > That's correct, I'm using the current tip of the main branch, which > > seems to be identical to 21.11.0. > > > > > > /Bruce > > > > > > PS: Is this a 4-socket system you are running on, since I see "socket 2" > > > being described as the socket number for device 80:01.0? > > > > > It is a two-socket system with sub-NUMA enabled, so it appears as four > > NUMA nodes. I'm only binding the devices on the second socket. > > > > Ok, [not that that should affect anything to do with ioat, AFAIK] > > Tried quickly reproducing the issue on some of our systems and failed to do > so. Does this error appear consistently, especially after a reboot? > > Thanks, > /Bruce It fails consistently after every warm reboot or power cycle. The kernel ioatdma driver always loads successfully at boot time for both sockets, but it also fails once I have bound the devices to vfio-pci and attempted to run examples/dpdk-dma. The kernel log messages are similar, both seem to read all-ones values. However, I have found that it works when binding to igb_uio instead of vfio, so maybe that's some kind of clue (vfio does work for the NIC ports). I'll continue to experiment with igb_uio, but I'm happy to gather any debug info for the vfio case if that would help. Thanks, Lance
[PATCH] buildtools: fix avx512 check for Python 3.5
Python 3.5 subprocess.run() does not have a capture_output
parameter (it is present only in 3.7 and up). Capture output
by using subprocess.PIPE for stdout instead.
Fixes: bb9cd91095b3 ("buildtools: make AVX512 check portable")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
buildtools/binutils-avx512-check.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/buildtools/binutils-avx512-check.py
b/buildtools/binutils-avx512-check.py
index a4e14f3593..9d7d360d95 100644
--- a/buildtools/binutils-avx512-check.py
+++ b/buildtools/binutils-avx512-check.py
@@ -15,7 +15,7 @@
src = '__asm__("vpgatherqq {}");'.format(gather_params).encode('utf-8')
subprocess.run(cc + ['-c', '-xc', '-o', obj.name, '-'], input=src,
check=True)
asm = subprocess.run([objdump, '-d', '--no-show-raw-insn', obj.name],
- capture_output=True,
check=True).stdout.decode('utf-8')
+ stdout=subprocess.PIPE,
check=True).stdout.decode('utf-8')
if gather_params not in asm:
print('vpgatherqq displacement error with as')
sys.exit(1)
--
2.25.1
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [dpdk-dev] [PATCH v4 02/22] app/testpmd: fix max rx packet length for VLAN packets
On Mon, Jan 18, 2021 at 2:08 AM Steve Yang wrote:
>
> When the max rx packet length is smaller than the sum of mtu size and
> ether overhead size, it should be enlarged, otherwise the VLAN packets
> will be dropped.
>
> Removed the rx_offloads assignment for jumbo frame during command line
> parsing, and set the correct jumbo frame flag if MTU size is larger than
> the default value 'RTE_ETHER_MTU' within 'init_config()'.
>
> Fixes: 384161e00627 ("app/testpmd: adjust on the fly VLAN configuration")
> Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")
> Fixes: ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
>
> Cc: Wenzhuo Lu
> Cc: Beilei Xing
> Cc: Bernard Iremonger
>
> Signed-off-by: Steve Yang
> ---
> app/test-pmd/cmdline.c| 6 --
> app/test-pmd/config.c | 2 +-
> app/test-pmd/parameters.c | 7 ++-
> app/test-pmd/testpmd.c| 18 ++
> 4 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 2ccbaa039e..65042fcff5 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1886,7 +1886,6 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
>
> RTE_ETH_FOREACH_DEV(pid) {
> struct rte_port *port = &ports[pid];
> - uint64_t rx_offloads = port->dev_conf.rxmode.offloads;
>
> if (!strcmp(res->name, "max-pkt-len")) {
> if (res->value < RTE_ETHER_MIN_LEN) {
> @@ -1898,11 +1897,6 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
> return;
>
> port->dev_conf.rxmode.max_rx_pkt_len = res->value;
> - if (res->value > RTE_ETHER_MAX_LEN)
> - rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> - else
> - rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> - port->dev_conf.rxmode.offloads = rx_offloads;
> } else {
> printf("Unknown parameter\n");
> return;
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 3f6c8642b1..1195f054f3 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1434,7 +1434,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
> * device supports jumbo frame.
> */
> eth_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;
> - if (mtu > RTE_ETHER_MAX_LEN - eth_overhead) {
> + if (mtu > RTE_ETHER_MTU) {
> rte_port->dev_conf.rxmode.offloads |=
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> rte_port->dev_conf.rxmode.max_rx_pkt_len =
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 414a0068fb..df5eb10d84 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -834,12 +834,9 @@ launch_args_parse(int argc, char** argv)
> }
> if (!strcmp(lgopts[opt_idx].name, "max-pkt-len")) {
> n = atoi(optarg);
> - if (n >= RTE_ETHER_MIN_LEN) {
> + if (n >= RTE_ETHER_MIN_LEN)
> rx_mode.max_rx_pkt_len = (uint32_t) n;
> - if (n > RTE_ETHER_MAX_LEN)
> - rx_offloads |=
> -
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> - } else
> + else
> rte_exit(EXIT_FAILURE,
> "Invalid max-pkt-len=%d -
> should be > %d\n",
> n, RTE_ETHER_MIN_LEN);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 2b60f6c5d3..c256e719ae 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1410,6 +1410,7 @@ init_config(void)
> struct rte_gro_param gro_param;
> uint32_t gso_types;
> uint16_t data_size;
> + uint16_t eth_overhead;
> bool warning = 0;
> int k;
> int ret;
> @@ -1446,6 +1447,23 @@ init_config(void)
> rte_exit(EXIT_FAILURE,
> "rte_eth_dev_info_get() failed\n");
>
> + /* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
> + if (port->dev_info.max_mtu != UINT16_MAX &&
> + port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
> + eth_overhead = port->dev_info.max_rx_pktlen -
> + port->dev_info.max_mtu;
> + else
> +
Re: [dpdk-dev] [PATCH v4 02/22] app/testpmd: fix max rx packet length for VLAN packets
On Thu, Jan 21, 2021 at 10:27 AM Lance Richardson
wrote:
>
> On Mon, Jan 18, 2021 at 2:08 AM Steve Yang wrote:
> >
> > When the max rx packet length is smaller than the sum of mtu size and
> > ether overhead size, it should be enlarged, otherwise the VLAN packets
> > will be dropped.
> >
> > Removed the rx_offloads assignment for jumbo frame during command line
> > parsing, and set the correct jumbo frame flag if MTU size is larger than
> > the default value 'RTE_ETHER_MTU' within 'init_config()'.
> >
> > Fixes: 384161e00627 ("app/testpmd: adjust on the fly VLAN configuration")
> > Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")
> > Fixes: ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> > Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
> >
> > Cc: Wenzhuo Lu
> > Cc: Beilei Xing
> > Cc: Bernard Iremonger
> >
> > Signed-off-by: Steve Yang
> > ---
> > app/test-pmd/cmdline.c| 6 --
> > app/test-pmd/config.c | 2 +-
> > app/test-pmd/parameters.c | 7 ++-
> > app/test-pmd/testpmd.c| 18 ++
> > 4 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index 2ccbaa039e..65042fcff5 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -1886,7 +1886,6 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
> >
> > RTE_ETH_FOREACH_DEV(pid) {
> > struct rte_port *port = &ports[pid];
> > - uint64_t rx_offloads = port->dev_conf.rxmode.offloads;
> >
> > if (!strcmp(res->name, "max-pkt-len")) {
> > if (res->value < RTE_ETHER_MIN_LEN) {
> > @@ -1898,11 +1897,6 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
> > return;
> >
> > port->dev_conf.rxmode.max_rx_pkt_len = res->value;
> > - if (res->value > RTE_ETHER_MAX_LEN)
> > - rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> > - else
> > - rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> > - port->dev_conf.rxmode.offloads = rx_offloads;
> > } else {
> > printf("Unknown parameter\n");
> > return;
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 3f6c8642b1..1195f054f3 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1434,7 +1434,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
> > * device supports jumbo frame.
> > */
> > eth_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;
> > - if (mtu > RTE_ETHER_MAX_LEN - eth_overhead) {
> > + if (mtu > RTE_ETHER_MTU) {
> > rte_port->dev_conf.rxmode.offloads |=
> > DEV_RX_OFFLOAD_JUMBO_FRAME;
> > rte_port->dev_conf.rxmode.max_rx_pkt_len =
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 414a0068fb..df5eb10d84 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -834,12 +834,9 @@ launch_args_parse(int argc, char** argv)
> > }
> > if (!strcmp(lgopts[opt_idx].name, "max-pkt-len")) {
> > n = atoi(optarg);
> > - if (n >= RTE_ETHER_MIN_LEN) {
> > + if (n >= RTE_ETHER_MIN_LEN)
> > rx_mode.max_rx_pkt_len = (uint32_t)
> > n;
> > - if (n > RTE_ETHER_MAX_LEN)
> > - rx_offloads |=
> > -
> > DEV_RX_OFFLOAD_JUMBO_FRAME;
> > - } else
> > + else
> > rte_exit(EXIT_FAILURE,
> > "Invalid max-pkt-len=%d -
> > should be > %d\n",
> > n, RTE_ETHER_MIN_LEN);
> > diff --git a/app/test-pmd
[dpdk-dev] [PATCH] net/bnxt: fix null termination of receive mbuf chain
The last mbuf in a multi-segment packet needs to be
NULL-terminated.
Fixes: 0958d8b6435d ("net/bnxt: support LRO")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Somnath Kotur
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_rxr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 969cae19fc..c34a8905e7 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -325,6 +325,7 @@ static int bnxt_rx_pages(struct bnxt_rx_queue *rxq,
*/
rte_bitmap_set(rxr->ag_bitmap, ag_cons);
}
+ last->next = NULL;
bnxt_prod_ag_mbuf(rxq);
return 0;
}
--
2.25.1
Re: [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix max-pkt-len option invalid
On Mon, Jan 25, 2021 at 3:35 AM Steve Yang wrote:
>
> Moved the setting of 'DEV_RX_OFFLOAD_JUMBO_FRAME' from
> 'cmd_config_max_pkt_len_parsed()' to 'init_config()' caused fail the case
> where 'max_rx_pkt_len' is changed from the command line via
> "port config all max-pkt-len".
>
> The 'init_config()' function is only called when testpmd is started,
> but the DEV_RX_OFFLOAD_JUMBO_FRAME setting needs to be recomputed whenever
> 'max_rx_pkt_len' changes.
>
> Define the 'update_jumbo_frame_offload()' function for both 'init_config()'
> and 'cmd_config_max_pkt_len_parsed()', and detect if 'max_rx_pkt_len'
> should be update to 1500 + overhead as default configuration. At the same
> time, the offloads of rx queue also should update the value once port
> offloads changed (e.g.: disabled JUMBO_FRAME offload via changed
> max-pkt-len, reproduce steps [1]), otherwise, it would cause unexpected
> result.
>
> [1]
> --
> ./x86_64-native-linuxapp-gcc/app/dpdk-testpmd -c 0xf -n 4 -- -i
> --max-pkt-len=9000 --tx-offloads=0x8000 --rxq=4 --txq=4 --disable-rss
> testpmd> set verbose 3
> testpmd> port stop all
> testpmd> port config all max-pkt-len 1518 port start all
>
> // Got fail error info without this patch
> Configuring Port 0 (socket 1)
> Ethdev port_id=0 rx_queue_id=0, new added offloads 0x800 must be
> within per-queue offload capabilities 0x0 in rte_eth_rx_queue_setup()
> Fail to configure port 0 rx queues //<-- Fail error info;
> --
>
> Fixes: 761c4d6690 ("app/testpmd: fix max Rx packet length for VLAN packets")
>
> Signed-off-by: Steve Yang
> ---
> app/test-pmd/cmdline.c| 1 +
> app/test-pmd/parameters.c | 1 +
> app/test-pmd/testpmd.c| 63 ---
> app/test-pmd/testpmd.h| 2 ++
> 4 files changed, 50 insertions(+), 17 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 89034c8b72..8b6b7b6206 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1901,6 +1901,7 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
> printf("Unknown parameter\n");
> return;
> }
> + update_jumbo_frame_offload(pid, false);
I'm probably missing something, but why isn't this a matter of simply calling
port_mtu_set() here (with mtu computed from max pkt len) and keeping
init_config() as currently implemented?
> }
>
> init_port_config();
Re: [dpdk-dev] [PATCH v5] app/testpmd: fix setting maximum packet length
On Mon, Jan 25, 2021 at 1:15 PM Ferruh Yigit wrote:
>
> From: Steve Yang
>
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
>
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the
> flag status needs to be calculated whenever 'max_rx_pkt_len' changes.
>
> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> didn't.
>
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and
> 'cmd_config_max_pkt_len_parsed()'.
>
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
> is zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
>
> [1]
> +/*
> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
> + * MTU is also aligned if JUMBO_FRAME offload is not set.
> + *
> + * port->dev_info should be get before calling this function.
Should this be "port->dev_info should be set ..." instead?
> + if (rx_offloads != port->dev_conf.rxmode.offloads) {
> + uint16_t qid;
> +
> + port->dev_conf.rxmode.offloads = rx_offloads;
> +
> + /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
> + for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> + if (on)
> + port->rx_conf[qid].offloads |=
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> + else
> + port->rx_conf[qid].offloads &=
> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> + }
Is it correct to set per-queue offloads that aren't advertised by the PMD
as supported in rx_queue_offload_capa?
> + }
> +
> + /* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
> +* if unset do it here
> +*/
> + if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> + ret = rte_eth_dev_set_mtu(portid,
> + port->dev_conf.rxmode.max_rx_pkt_len -
> eth_overhead);
> + if (ret)
> + printf("Failed to set MTU to %u for port %u\n",
> + port->dev_conf.rxmode.max_rx_pkt_len -
> eth_overhead,
> + portid);
> + }
> +
> + return 0;
> +}
> +
Applied and tested with a few iterations of configuring max packet size
back and forth between jumbo and non-jumbo sizes, also tried setting
max packet size using the command-line option, all seems good based
on rx offloads and packet forwarding.
Two minor questions above, otherwise LGTM.
Re: [dpdk-dev] [PATCH v5] app/testpmd: fix setting maximum packet length
Acked-by: Lance Richardson
Thanks,
Lance
On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit wrote:
>
> On 1/25/2021 7:41 PM, Lance Richardson wrote:
> > On Mon, Jan 25, 2021 at 1:15 PM Ferruh Yigit wrote:
> >>
> >> From: Steve Yang
> >>
> >> "port config all max-pkt-len" command fails because it doesn't set the
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> >>
> >> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> >> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> >> 'init_config()' function is only called during testpmd startup, but the
> >> flag status needs to be calculated whenever 'max_rx_pkt_len' changes.
> >>
> >> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> >> didn't.
> >>
> >> Adding the 'update_jumbo_frame_offload()' helper function to update
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> >> function is called both by 'init_config()' and
> >> 'cmd_config_max_pkt_len_parsed()'.
> >>
> >> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> >> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
> >> is zero.
> >> If '--max-pkt-len=N' argument provided, it will be used instead.
> >> And with each "port config all max-pkt-len" command, the
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> >> updated.
> >>
> >> [1]
> >
> >
> >
> >> +/*
> >> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
> >> + * MTU is also aligned if JUMBO_FRAME offload is not set.
> >> + *
> >> + * port->dev_info should be get before calling this function.
> >
> > Should this be "port->dev_info should be set ..." instead?
> >
>
> Ack.
>
> >
> >
> >
> >> + if (rx_offloads != port->dev_conf.rxmode.offloads) {
> >> + uint16_t qid;
> >> +
> >> + port->dev_conf.rxmode.offloads = rx_offloads;
> >> +
> >> + /* Apply JUMBO_FRAME offload configuration to Rx queue(s)
> >> */
> >> + for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> >> + if (on)
> >> + port->rx_conf[qid].offloads |=
> >> DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> + else
> >> + port->rx_conf[qid].offloads &=
> >> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> + }
> >
> > Is it correct to set per-queue offloads that aren't advertised by the PMD
> > as supported in rx_queue_offload_capa?
> >
>
> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads'
> values
> are reflected to 'port->rx_conf[].offloads' for all queues.
>
> We should set the offload in 'port->rx_conf[].offloads' if it is set in
> 'port->dev_conf.rxmode.offloads'.
>
> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can
> have
> it. And the port level capability is already checked above.
>
> >> + }
> >> +
> >> + /* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
> >> +* if unset do it here
> >> +*/
> >> + if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> >> + ret = rte_eth_dev_set_mtu(portid,
> >> + port->dev_conf.rxmode.max_rx_pkt_len -
> >> eth_overhead);
> >> + if (ret)
> >> + printf("Failed to set MTU to %u for port %u\n",
> >> + port->dev_conf.rxmode.max_rx_pkt_len -
> >> eth_overhead,
> >> + portid);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >
> > Applied and tested with a few iterations of configuring max packet size
> > back and forth between jumbo and non-jumbo sizes, also tried setting
> > max packet size using the command-line option, all seems good based
> > on rx offloads and packet forwarding.
> >
> > Two minor questions above, otherwise LGTM.
> >
>
> Thanks for testing. I will wait for more comments before new version.
Re: [dpdk-dev] [PATCH v5] app/testpmd: fix setting maximum packet length
On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit wrote:
>
> >> + if (rx_offloads != port->dev_conf.rxmode.offloads) {
> >> + uint16_t qid;
> >> +
> >> + port->dev_conf.rxmode.offloads = rx_offloads;
> >> +
> >> + /* Apply JUMBO_FRAME offload configuration to Rx queue(s)
> >> */
> >> + for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> >> + if (on)
> >> + port->rx_conf[qid].offloads |=
> >> DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> + else
> >> + port->rx_conf[qid].offloads &=
> >> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> + }
> >
> > Is it correct to set per-queue offloads that aren't advertised by the PMD
> > as supported in rx_queue_offload_capa?
> >
>
> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads'
> values
> are reflected to 'port->rx_conf[].offloads' for all queues.
>
> We should set the offload in 'port->rx_conf[].offloads' if it is set in
> 'port->dev_conf.rxmode.offloads'.
>
> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can
> have
> it. And the port level capability is already checked above.
>
I'm still not 100% clear about the per-queue offload question.
With this patch, and jumbo max packet size configured (on the command
line in this case), I see:
testpmd> show port 0 rx_offload configuration
Rx Offloading Configuration of port 0 :
Port : JUMBO_FRAME
Queue[ 0] : JUMBO_FRAME
testpmd> show port 0 rx_offload capabilities
Rx Offloading Capabilities of port 0 :
Per Queue :
Per Port : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER TIMESTAMP
KEEP_CRC OUTER_UDP_CKSUM RSS_HASH
Yet if I configure a jumbo MTU starting with standard max packet size,
jumbo is only enabled at the port level:
testpmd> port config mtu 0 9000
testpmd> port start all
testpmd> show port 0 rx_offload configuration
Rx Offloading Configuration of port 0 :
Port : JUMBO_FRAME
Queue[ 0] :
It still seems odd for a per-queue offload to be enabled on a PMD that
doesn't support per-queue receive offloads.
Re: [dpdk-dev] [PATCH v5] app/testpmd: fix setting maximum packet length
On Tue, Jan 26, 2021 at 6:01 AM Ferruh Yigit wrote:
>
> On 1/26/2021 3:45 AM, Lance Richardson wrote:
> > On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit wrote:
> >>
> >>>> + if (rx_offloads != port->dev_conf.rxmode.offloads) {
> >>>> + uint16_t qid;
> >>>> +
> >>>> + port->dev_conf.rxmode.offloads = rx_offloads;
> >>>> +
> >>>> + /* Apply JUMBO_FRAME offload configuration to Rx
> >>>> queue(s) */
> >>>> + for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> >>>> + if (on)
> >>>> + port->rx_conf[qid].offloads |=
> >>>> DEV_RX_OFFLOAD_JUMBO_FRAME;
> >>>> + else
> >>>> + port->rx_conf[qid].offloads &=
> >>>> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >>>> + }
> >>>
> >>> Is it correct to set per-queue offloads that aren't advertised by the PMD
> >>> as supported in rx_queue_offload_capa?
> >>>
> >>
> >> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads'
> >> values
> >> are reflected to 'port->rx_conf[].offloads' for all queues.
> >>
> >> We should set the offload in 'port->rx_conf[].offloads' if it is set in
> >> 'port->dev_conf.rxmode.offloads'.
> >>
> >> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can
> >> have
> >> it. And the port level capability is already checked above.
> >>
> >
> > I'm still not 100% clear about the per-queue offload question.
> >
> > With this patch, and jumbo max packet size configured (on the command
> > line in this case), I see:
> >
> > testpmd> show port 0 rx_offload configuration
> > Rx Offloading Configuration of port 0 :
> >Port : JUMBO_FRAME
> >Queue[ 0] : JUMBO_FRAME
> >
> > testpmd> show port 0 rx_offload capabilities
> > Rx Offloading Capabilities of port 0 :
> >Per Queue :
> >Per Port : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
> > OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER TIMESTAMP
> > KEEP_CRC OUTER_UDP_CKSUM RSS_HASH
> >
>
> The port level offload is applied to all queues on the port, testpmd config
> structure reflects this logic in implementation.
> If Rx offload X is set for a port, it is set for all Rx queue offloads, this
> is
> not new behavior and not related to this patch.
>
OK, is this purely for display purposes within testpmd? I ask because
it appears that all PMDs supporting per-queue offload configuration already
take care of combining port-level and per-queue offloads within their
tx_queue_setup()/rx_queue_setup() functions and then track the combined
set of offloads within a per-queue field, e.g. this line is common to
e1000/i40e/ionic/ixgbe/octeontx2/thunderx/txgbe rx_queue_setup()
implementations:
offloads = rx_conf->offloads | dev->data->dev_conf.rxmode.offloads;
And rte_ethdev.h says:
No need to repeat any bit in rx_conf->offloads which has already been
enabled in rte_eth_dev_configure() at port level. An offloading enabled
at port level can't be disabled at queue level.
Which I suppose confirms that if testpmd is combining per-port and per-
queue offloads, it's just for the purposes of testpmd.
Apologies for worrying at this even more, I just wanted to be sure that
I understand what the PMD is expected to do.
Regards,
Lance
[dpdk-dev] [PATCH] net/bnxt: fix cp ring size calculation
The size of the receive completion ring should be recalculated
when MTU is increased to a size that requires scattered receive
or when LRO is enabled. Move logic for this calculation from
the ring configuration path to the device start path.
- Made size calculation dependent only on scattered_rx
status.
- Moved calculation of scattered_rx up in the initialization
sequence.
- Made LRO offload status part of scattered_rx calculation.
When the completion ring size is too small, completion overflows
can occur causing the ring to be disabled in hardware.
Fixes: 04067844a3e9 ("net/bnxt: reduce CQ queue size without aggregation ring")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 6 +-
drivers/net/bnxt/bnxt_rxr.c| 15 +--
2 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ec88cb0046..ca779726ec 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1143,6 +1143,9 @@ static int bnxt_scattered_rx(struct rte_eth_dev *eth_dev)
if (eth_dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_SCATTER)
return 1;
+ if (eth_dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO)
+ return 1;
+
for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
struct bnxt_rx_queue *rxq = eth_dev->data->rx_queues[i];
@@ -1418,11 +1421,12 @@ static int bnxt_dev_start_op(struct rte_eth_dev
*eth_dev)
bnxt_enable_int(bp);
+ eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
+
rc = bnxt_start_nic(bp);
if (rc)
goto error;
- eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
eth_dev->data->dev_started = 1;
bnxt_link_update_op(eth_dev, 1);
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 8c2781c968..4674f7cea2 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1116,12 +1116,9 @@ void bnxt_free_rx_rings(struct bnxt *bp)
int bnxt_init_rx_ring_struct(struct bnxt_rx_queue *rxq, unsigned int socket_id)
{
- struct rte_eth_dev *eth_dev = rxq->bp->eth_dev;
- struct rte_eth_rxmode *rxmode;
struct bnxt_cp_ring_info *cpr;
struct bnxt_rx_ring_info *rxr;
struct bnxt_ring *ring;
- bool use_agg_ring;
rxq->rx_buf_size = BNXT_MAX_PKT_LEN + sizeof(struct rte_mbuf);
@@ -1164,19 +1161,9 @@ int bnxt_init_rx_ring_struct(struct bnxt_rx_queue *rxq,
unsigned int socket_id)
return -ENOMEM;
cpr->cp_ring_struct = ring;
- rxmode = ð_dev->data->dev_conf.rxmode;
- use_agg_ring = (rxmode->offloads & DEV_RX_OFFLOAD_SCATTER) ||
- (rxmode->offloads & DEV_RX_OFFLOAD_TCP_LRO) ||
- (rxmode->max_rx_pkt_len >
-(uint32_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) -
- RTE_PKTMBUF_HEADROOM));
-
/* Allocate two completion slots per entry in desc ring. */
ring->ring_size = rxr->rx_ring_struct->ring_size * 2;
-
- /* Allocate additional slots if aggregation ring is in use. */
- if (use_agg_ring)
- ring->ring_size *= AGG_RING_SIZE_FACTOR;
+ ring->ring_size *= AGG_RING_SIZE_FACTOR;
ring->ring_size = rte_align32pow2(ring->ring_size);
ring->ring_mask = ring->ring_size - 1;
--
2.25.1
Re: [dpdk-dev] [PATCH] net/bnxt: fix cp ring size calculation
On Fri, Jan 29, 2021 at 12:57 PM Lance Richardson
wrote:
>
> The size of the receive completion ring should be recalculated
> when MTU is increased to a size that requires scattered receive
> or when LRO is enabled. Move logic for this calculation from
> the ring configuration path to the device start path.
>- Made size calculation dependent only on scattered_rx
> status.
>- Moved calculation of scattered_rx up in the initialization
> sequence.
>- Made LRO offload status part of scattered_rx calculation.
>
> When the completion ring size is too small, completion overflows
> can occur causing the ring to be disabled in hardware.
>
> Fixes: 04067844a3e9 ("net/bnxt: reduce CQ queue size without aggregation
> ring")
> Cc: sta...@dpdk.org
> Signed-off-by: Lance Richardson
> Reviewed-by: Ajit Kumar Khaparde
> ---
Please disregard, v2 coming.
[dpdk-dev] [PATCH v2] net/bnxt: fix cp ring size calculation
The size of the receive completion ring should be recalculated
when MTU is increased to a size that requires scattered receive
or when LRO is enabled. Move logic for this calculation from
the ring configuration path to the device start path.
- Made size calculation dependent only on scattered_rx
status.
- Moved calculation of scattered_rx up in the initialization
sequence.
- Made LRO offload status part of scattered_rx calculation.
When the completion ring size is too small, completion overflows
can occur causing the ring to be disabled in hardware.
Fixes: 04067844a3e9 ("net/bnxt: reduce CQ queue size without aggregation ring")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
v2: Fixed conflict resolution mistake (two missing hunks).
drivers/net/bnxt/bnxt_ethdev.c | 6 +-
drivers/net/bnxt/bnxt_ring.c | 22 ++
drivers/net/bnxt/bnxt_rxr.c| 15 +--
3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ec88cb0046..ca779726ec 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1143,6 +1143,9 @@ static int bnxt_scattered_rx(struct rte_eth_dev *eth_dev)
if (eth_dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_SCATTER)
return 1;
+ if (eth_dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO)
+ return 1;
+
for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
struct bnxt_rx_queue *rxq = eth_dev->data->rx_queues[i];
@@ -1418,11 +1421,12 @@ static int bnxt_dev_start_op(struct rte_eth_dev
*eth_dev)
bnxt_enable_int(bp);
+ eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
+
rc = bnxt_start_nic(bp);
if (rc)
goto error;
- eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
eth_dev->data->dev_started = 1;
bnxt_link_update_op(eth_dev, 1);
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 4e513244a6..ba23c1fa03 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -583,6 +583,17 @@ int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int
queue_index)
struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
int rc;
+ /*
+* Storage for the cp ring is allocated based on worst-case
+* usage, the actual size to be used by hw is computed here.
+*/
+ cp_ring->ring_size = rxr->rx_ring_struct->ring_size * 2;
+
+ if (bp->eth_dev->data->scattered_rx)
+ cp_ring->ring_size *= AGG_RING_SIZE_FACTOR;
+
+ cp_ring->ring_mask = cp_ring->ring_size - 1;
+
rc = bnxt_alloc_cmpl_ring(bp, queue_index, cpr);
if (rc)
goto err_out;
@@ -693,6 +704,17 @@ int bnxt_alloc_hwrm_rings(struct bnxt *bp)
struct bnxt_ring *cp_ring = cpr->cp_ring_struct;
struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
+ /*
+* Storage for the cp ring is allocated based on worst-case
+* usage, the actual size to be used by hw is computed here.
+*/
+ cp_ring->ring_size = rxr->rx_ring_struct->ring_size * 2;
+
+ if (bp->eth_dev->data->scattered_rx)
+ cp_ring->ring_size *= AGG_RING_SIZE_FACTOR;
+
+ cp_ring->ring_mask = cp_ring->ring_size - 1;
+
if (bnxt_alloc_cmpl_ring(bp, i, cpr))
goto err_out;
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 8c2781c968..4674f7cea2 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1116,12 +1116,9 @@ void bnxt_free_rx_rings(struct bnxt *bp)
int bnxt_init_rx_ring_struct(struct bnxt_rx_queue *rxq, unsigned int socket_id)
{
- struct rte_eth_dev *eth_dev = rxq->bp->eth_dev;
- struct rte_eth_rxmode *rxmode;
struct bnxt_cp_ring_info *cpr;
struct bnxt_rx_ring_info *rxr;
struct bnxt_ring *ring;
- bool use_agg_ring;
rxq->rx_buf_size = BNXT_MAX_PKT_LEN + sizeof(struct rte_mbuf);
@@ -1164,19 +1161,9 @@ int bnxt_init_rx_ring_struct(struct bnxt_rx_queue *rxq,
unsigned int socket_id)
return -ENOMEM;
cpr->cp_ring_struct = ring;
- rxmode = ð_dev->data->dev_conf.rxmode;
- use_agg_ring = (rxmode->offloads & DEV_RX_OFFLOAD_SCATTER) ||
- (rxmode->offloads & DEV_RX_OFFLOAD_TCP_LRO) ||
- (rxmode->max_rx_pkt_len >
-(uint32_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) -
- RTE_PKTMBUF_HEADROOM));
-
/* Allocate two complet
[dpdk-dev] [PATCH 21.05] app/testpmd: count outer IP checksum errors
Count and display outer IP checksum errors in the checksum
forwarder.
Example forwarder stats output:
RX-packets: 158RX-dropped: 0 RX-total: 158
Bad-ipcsum: 48 Bad-l4csum: 48Bad-outer-l4csum: 6
Bad-outer-ipcsum: 40
TX-packets: 0 TX-dropped: 0 TX-total: 0
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
Reviewed-by: Kalesh Anakkur Purayil
---
app/test-pmd/csumonly.c | 5 +
app/test-pmd/testpmd.c | 11 ++-
app/test-pmd/testpmd.h | 2 ++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index d813d4fae0..37ed415cee 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -814,6 +814,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
uint32_t rx_bad_ip_csum;
uint32_t rx_bad_l4_csum;
uint32_t rx_bad_outer_l4_csum;
+ uint32_t rx_bad_outer_ip_csum;
struct testpmd_offload_info info;
uint16_t nb_segments = 0;
int ret;
@@ -833,6 +834,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
rx_bad_ip_csum = 0;
rx_bad_l4_csum = 0;
rx_bad_outer_l4_csum = 0;
+ rx_bad_outer_ip_csum = 0;
gro_enable = gro_ports[fs->rx_port].enable;
txp = &ports[fs->tx_port];
@@ -862,6 +864,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
rx_bad_l4_csum += 1;
if (rx_ol_flags & PKT_RX_OUTER_L4_CKSUM_BAD)
rx_bad_outer_l4_csum += 1;
+ if (rx_ol_flags & PKT_RX_EIP_CKSUM_BAD)
+ rx_bad_outer_ip_csum += 1;
/* step 1: dissect packet, parsing optional vlan, ip4/ip6, vxlan
* and inner headers */
@@ -1124,6 +1128,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
fs->rx_bad_ip_csum += rx_bad_ip_csum;
fs->rx_bad_l4_csum += rx_bad_l4_csum;
fs->rx_bad_outer_l4_csum += rx_bad_outer_l4_csum;
+ fs->rx_bad_outer_ip_csum += rx_bad_outer_ip_csum;
inc_tx_burst_stats(fs, nb_tx);
if (unlikely(nb_tx < nb_rx)) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c256e719ae..67f60745a0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1805,6 +1805,8 @@ fwd_stream_stats_display(streamid_t stream_id)
" Rx- bad outer L4 checksum: %-14"PRIu64"\n",
fs->rx_bad_ip_csum, fs->rx_bad_l4_csum,
fs->rx_bad_outer_l4_csum);
+ printf(" RX- bad outer IP checksum: %-14"PRIu64"\n",
+ fs->rx_bad_outer_ip_csum);
} else {
printf("\n");
}
@@ -1827,6 +1829,7 @@ fwd_stats_display(void)
uint64_t rx_bad_ip_csum;
uint64_t rx_bad_l4_csum;
uint64_t rx_bad_outer_l4_csum;
+ uint64_t rx_bad_outer_ip_csum;
} ports_stats[RTE_MAX_ETHPORTS];
uint64_t total_rx_dropped = 0;
uint64_t total_tx_dropped = 0;
@@ -1859,6 +1862,8 @@ fwd_stats_display(void)
ports_stats[fs->rx_port].rx_bad_l4_csum += fs->rx_bad_l4_csum;
ports_stats[fs->rx_port].rx_bad_outer_l4_csum +=
fs->rx_bad_outer_l4_csum;
+ ports_stats[fs->rx_port].rx_bad_outer_ip_csum +=
+ fs->rx_bad_outer_ip_csum;
if (record_core_cycles)
fwd_cycles += fs->core_cycles;
@@ -1890,13 +1895,16 @@ fwd_stats_display(void)
"RX-total: %-"PRIu64"\n", stats.ipackets, stats.imissed,
stats.ipackets + stats.imissed);
- if (cur_fwd_eng == &csum_fwd_engine)
+ if (cur_fwd_eng == &csum_fwd_engine) {
printf(" Bad-ipcsum: %-14"PRIu64
" Bad-l4csum: %-14"PRIu64
"Bad-outer-l4csum: %-14"PRIu64"\n",
ports_stats[pt_id].rx_bad_ip_csum,
ports_stats[pt_id].rx_bad_l4_csum,
ports_stats[pt_id].rx_bad_outer_l4_csum);
+ printf(" Bad-outer-ipcsum: %-14"PRIu64"\n",
+ ports_stats[pt_id].rx_bad_outer_ip_csum);
+ }
if (stats.ierrors + stats.rx_nombuf > 0) {
printf(" RX-error: %-"PRIu64"\n", stats.ierrors);
printf(" RX-nombufs: %-14"PRIu64"\n", stats.rx_nombuf);
@@ -1974,6 +1982,7 @@ fwd_stats_reset(void)
fs->rx_bad_ip_csum = 0;
fs->rx_bad_l4_csum =
Re: [dpdk-dev] [PATCH] app/testpmd: remove duplicated offload display
On Fri, Feb 5, 2021 at 9:27 AM Ferruh Yigit wrote: > > "show port cap all|" was to display offload configuration of > port(s). > > But later two other commands added to show same information in more > accurate way: > show port (port_id) rx_offload configuration > show port (port_id) tx_offload configuration > > These new commands can both show port and queue level configuration, > also with their capabilities counterparts easier to see offload > capability and configuration of the port in similar syntax. > > So the functionality is duplicated and removing this version, to favor > the new commands. > > Another problem with this command is it requires each new offload to be > added into the function to display them, and there were missing offloads > that are not displayed, this requirement for sure will create gaps by > time as new offloads added. > > Signed-off-by: Ferruh Yigit > --- Acked-by: Lance Richardson
[dpdk-dev] [PATCH 21.05] app/testpmd: support show Rx queue count
Add support for querying receive queue count in order to allow
the rte_eth_dev rx_queue_count() API to be exercised and tested.
Signed-off-by: Lance Richardson
---
app/test-pmd/cmdline.c | 65 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 6 ++
2 files changed, 71 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 59722d268..6e2fe57a6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -16699,6 +16699,70 @@ cmdline_parse_inst_t cmd_show_rx_tx_desc_status = {
},
};
+/* *** display rx queue count *** */
+struct cmd_show_rx_queue_count_result {
+ cmdline_fixed_string_t cmd_show;
+ cmdline_fixed_string_t cmd_port;
+ cmdline_fixed_string_t cmd_rxq;
+ cmdline_fixed_string_t cmd_count;
+ portid_t cmd_pid;
+ portid_t cmd_qid;
+};
+
+static void
+cmd_show_rx_queue_count_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_show_rx_queue_count_result *res = parsed_result;
+ int rc;
+
+ if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+ printf("invalid port id %u\n", res->cmd_pid);
+ return;
+ }
+
+ rc = rte_eth_rx_queue_count(res->cmd_pid, res->cmd_qid);
+ if (rc < 0) {
+ printf("Invalid queueid = %d\n", res->cmd_qid);
+ return;
+ }
+ printf("Used desc count = %d\n", rc);
+}
+
+cmdline_parse_token_string_t cmd_show_rx_queue_count_show =
+ TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+ cmd_show, "show");
+cmdline_parse_token_string_t cmd_show_rx_queue_count_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+ cmd_port, "port");
+cmdline_parse_token_num_t cmd_show_rx_queue_count_pid =
+ TOKEN_NUM_INITIALIZER(struct cmd_show_rx_queue_count_result,
+ cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_count_rxq =
+ TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+ cmd_rxq, "rxq");
+cmdline_parse_token_num_t cmd_show_rx_queue_count_qid =
+ TOKEN_NUM_INITIALIZER(struct cmd_show_rx_queue_count_result,
+ cmd_qid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_count_count =
+ TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+ cmd_count, "count");
+cmdline_parse_inst_t cmd_show_rx_queue_count = {
+ .f = cmd_show_rx_queue_count_parsed,
+ .data = NULL,
+ .help_str = "show port rxq count",
+ .tokens = {
+ (void *)&cmd_show_rx_queue_count_show,
+ (void *)&cmd_show_rx_queue_count_port,
+ (void *)&cmd_show_rx_queue_count_pid,
+ (void *)&cmd_show_rx_queue_count_rxq,
+ (void *)&cmd_show_rx_queue_count_qid,
+ (void *)&cmd_show_rx_queue_count_count,
+ NULL,
+ },
+};
+
/* Common result structure for set port ptypes */
struct cmd_set_port_ptypes_result {
cmdline_fixed_string_t set;
@@ -17098,6 +17162,7 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)&cmd_config_tx_metadata_specific,
(cmdline_parse_inst_t *)&cmd_show_tx_metadata,
(cmdline_parse_inst_t *)&cmd_show_rx_tx_desc_status,
+ (cmdline_parse_inst_t *)&cmd_show_rx_queue_count,
(cmdline_parse_inst_t *)&cmd_set_raw,
(cmdline_parse_inst_t *)&cmd_show_set_raw,
(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a45910b81..789ee7d27 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -266,6 +266,12 @@ Display information for a given port's RX/TX descriptor
status::
testpmd> show port (port_id) (rxq|txq) (queue_id) desc (desc_id) status
+show rxq count
+~
+
+Display the number of ready descriptors on a given RX queue::
+
+ testpmd> show port (port_id) rxq (queue_id) count
show config
~~~
--
2.25.1
[dpdk-dev] [PATCH 21.05 v2] app/testpmd: support show Rx queue desc count
Add support for querying the count of ready descriptors on a
receive queue in order to allow the rte_eth_dev rx_queue_count()
API to be exercised and tested.
Signed-off-by: Lance Richardson
---
app/test-pmd/cmdline.c | 83 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 ++
2 files changed, 90 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 59722d268..821dd3d77 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -16699,6 +16699,88 @@ cmdline_parse_inst_t cmd_show_rx_tx_desc_status = {
},
};
+/* *** display rx queue desc used count *** */
+struct cmd_show_rx_queue_desc_used_count_result {
+ cmdline_fixed_string_t cmd_show;
+ cmdline_fixed_string_t cmd_port;
+ cmdline_fixed_string_t cmd_rxq;
+ cmdline_fixed_string_t cmd_desc;
+ cmdline_fixed_string_t cmd_used;
+ cmdline_fixed_string_t cmd_count;
+ portid_t cmd_pid;
+ portid_t cmd_qid;
+};
+
+static void
+cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
+ int rc;
+
+ if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+ printf("invalid port id %u\n", res->cmd_pid);
+ return;
+ }
+
+ rc = rte_eth_rx_queue_count(res->cmd_pid, res->cmd_qid);
+ if (rc < 0) {
+ printf("Invalid queueid = %d\n", res->cmd_qid);
+ return;
+ }
+ printf("Used desc count = %d\n", rc);
+}
+
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_show =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_show, "show");
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_port =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_port, "port");
+cmdline_parse_token_num_t cmd_show_rx_queue_desc_used_count_pid =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_rxq =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_rxq, "rxq");
+cmdline_parse_token_num_t cmd_show_rx_queue_desc_used_count_qid =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_qid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_desc =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_count, "desc");
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_used =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_count, "used");
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_count =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_count, "count");
+cmdline_parse_inst_t cmd_show_rx_queue_count = {
+ .f = cmd_show_rx_queue_desc_used_count_parsed,
+ .data = NULL,
+ .help_str = "show port rxq desc used count",
+ .tokens = {
+ (void *)&cmd_show_rx_queue_desc_used_count_show,
+ (void *)&cmd_show_rx_queue_desc_used_count_port,
+ (void *)&cmd_show_rx_queue_desc_used_count_pid,
+ (void *)&cmd_show_rx_queue_desc_used_count_rxq,
+ (void *)&cmd_show_rx_queue_desc_used_count_qid,
+ (void *)&cmd_show_rx_queue_desc_used_count_desc,
+ (void *)&cmd_show_rx_queue_desc_used_count_used,
+ (void *)&cmd_show_rx_queue_desc_used_count_count,
+ NULL,
+ },
+};
+
/* Common result structure for set port ptypes */
struct cmd_set_port_ptypes_result {
cmdline_fixed_string_t set;
@@ -17098,6 +17180,7 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)&cmd_config_tx_metadata_specific,
(cmdline_parse_inst_t *)&cmd_show_tx_metadata,
(cmdline_parse_inst_t *)&cmd_show_rx_tx_desc_status,
+ (cmdline_parse_inst_t *)&cmd_show_rx_queue_count,
(cmdline_parse_inst_t *)&cmd_set_raw,
(cmdline_parse_inst_t *)&cmd_show_set_raw,
(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a45910b81..703dead0c 100644
--- a/doc/guides/testpmd_app_ug/testpmd_fu
Re: [dpdk-dev] [PATCH 21.05] app/testpmd: support show Rx queue count
On Fri, Feb 12, 2021 at 6:51 AM Ferruh Yigit wrote: > > On 2/11/2021 7:44 PM, Lance Richardson wrote: > > Add support for querying receive queue count in order to allow > > the rte_eth_dev rx_queue_count() API to be exercised and tested. > > > > +1 to adding this feature, but the naming is a little misleading, "Rx queue > count", it looks like it will print the number of Rx queues, and the API has > the > same problem indeed. > > Can you please clarify it that it is to get number of used descriptor in a Rx > queue? > And "used descriptor" part also needs some explanation I think. > That makes sense, fixed in v2. > > There is already an existing command: > "show port rxq|txq desc status" > > What do you think adding the new one as something like: > "show port rxq desc used count" Sounds good, v2 is updated to use that form. > > +show rxq count > > +~ > > The '~' line length should be same as header length > Fixed in v2. > > + > > +Display the number of ready descriptors on a given RX queue:: > > Can you please describe more, what is "ready descriptor"? > > The 'rte_eth_rx_queue_count()' API should be returning number of descriptors > filled by HW. > I took a stab at this in v2, but maybe it could be expanded more. As I understand it, the returned descriptor count should correspond to the number of packets that could be received in the next call to the burst receive function... not necessarily the hardware-specific notion of a descriptor, which might include descriptors used for chained mbufs, LRO metadata, async status messages from firmware, etc. Thanks, Lance
[dpdk-dev] [PATCH 21.05 v3] app/testpmd: display rxq desc used count
Add support for displaying the count of used (filled by hardware
but not yet processed by the driver) descriptors on a receive
queue in order to allow the rte_eth_dev rx_queue_count() API to
be exercised and tested.
Signed-off-by: Lance Richardson
---
v3:
- Made terminology more consistent betwen commit log, title, and
documentation text.
- Added long help text for new command.
v2:
- Changed command syntax from "show port <> rxq <> count" to
"show port <> rxq <> desc used count".
- Expanded description to clarify the meaning of the displayed
descriptor count.
- Fixed header line length.
app/test-pmd/cmdline.c | 87 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 ++
2 files changed, 94 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 59722d268..8eb2a48ef 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -246,6 +246,10 @@ static void cmd_help_long_parsed(void *parsed_result,
"show port (port_id) rxq|txq (queue_id) desc (desc_id)
status"
" Show status of rx|tx descriptor.\n\n"
+ "show port (port_id) rxq (queue_id) desc used count\n"
+ "Show current number of filled receive"
+ " packet descriptors.\n\n"
+
"show port (port_id) macs|mcast_macs"
" Display list of mac addresses added to
port.\n\n"
@@ -16699,6 +16703,88 @@ cmdline_parse_inst_t cmd_show_rx_tx_desc_status = {
},
};
+/* *** display rx queue desc used count *** */
+struct cmd_show_rx_queue_desc_used_count_result {
+ cmdline_fixed_string_t cmd_show;
+ cmdline_fixed_string_t cmd_port;
+ cmdline_fixed_string_t cmd_rxq;
+ cmdline_fixed_string_t cmd_desc;
+ cmdline_fixed_string_t cmd_used;
+ cmdline_fixed_string_t cmd_count;
+ portid_t cmd_pid;
+ portid_t cmd_qid;
+};
+
+static void
+cmd_show_rx_queue_desc_used_count_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_show_rx_queue_desc_used_count_result *res = parsed_result;
+ int rc;
+
+ if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+ printf("invalid port id %u\n", res->cmd_pid);
+ return;
+ }
+
+ rc = rte_eth_rx_queue_count(res->cmd_pid, res->cmd_qid);
+ if (rc < 0) {
+ printf("Invalid queueid = %d\n", res->cmd_qid);
+ return;
+ }
+ printf("Used desc count = %d\n", rc);
+}
+
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_show =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_show, "show");
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_port =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_port, "port");
+cmdline_parse_token_num_t cmd_show_rx_queue_desc_used_count_pid =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_rxq =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_rxq, "rxq");
+cmdline_parse_token_num_t cmd_show_rx_queue_desc_used_count_qid =
+ TOKEN_NUM_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_qid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_desc =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_count, "desc");
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_used =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_count, "used");
+cmdline_parse_token_string_t cmd_show_rx_queue_desc_used_count_count =
+ TOKEN_STRING_INITIALIZER
+ (struct cmd_show_rx_queue_desc_used_count_result,
+cmd_count, "count");
+cmdline_parse_inst_t cmd_show_rx_queue_desc_used_count = {
+ .f = cmd_show_rx_queue_desc_used_count_parsed,
+ .data = NULL,
+ .help_str = "show port rxq desc used count",
+ .tokens = {
+ (void *)&cmd_show_rx_queue_desc_used_count_show,
+ (void *)&cmd_show_rx_queue_desc_used_count_port,
+ (void *)&cmd_show_rx_queue_desc_used_count_pid,
+ (void *)&cmd_sh
[dpdk-dev] [PATCH] net/bnxt: fix Rx queue count implementation
bnxt_rx_queue_count_op() incorrectly returns the number of
filled but unprocessed completion queue entries instead of
the number of filled but unprocessed received packet
completions. Fix by properly accounting for the number of
completion ring entries used by the various received packet
completion types.
Fixes: 34c0ba839bae ("net/bnxt: fix Rx queue count")
Signed-off-by: Lance Richardson
Reviewed-by: Somnath Kotur
Cc: sta...@dpdk.org
---
drivers/net/bnxt/bnxt_cpr.h| 4
drivers/net/bnxt/bnxt_ethdev.c | 38 +-
drivers/net/bnxt/bnxt_rxr.c| 3 +--
drivers/net/bnxt/bnxt_rxr.h| 4
4 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
index d7e0d4621a..28c0a9049c 100644
--- a/drivers/net/bnxt/bnxt_cpr.h
+++ b/drivers/net/bnxt/bnxt_cpr.h
@@ -26,6 +26,10 @@ struct bnxt_db_info;
#define CMP_TYPE(cmp) \
(((struct cmpl_base *)cmp)->type & CMPL_BASE_TYPE_MASK)
+/* Get completion length from completion type, in 16-byte units. */
+#define CMP_LEN(cmp_type) (((cmp_type) & 1) + 1)
+
+
#define ADV_RAW_CMP(idx, n)((idx) + (n))
#define NEXT_RAW_CMP(idx) ADV_RAW_CMP(idx, 1)
#define RING_CMP(ring, idx)((idx) & (ring)->ring_mask)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 22c880c5c3..9824cdb6d8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2942,8 +2942,8 @@ static uint32_t
bnxt_rx_queue_count_op(struct rte_eth_dev *dev, uint16_t rx_queue_id)
{
struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
- uint32_t desc = 0, raw_cons = 0, cons;
struct bnxt_cp_ring_info *cpr;
+ uint32_t desc = 0, raw_cons;
struct bnxt_rx_queue *rxq;
struct rx_pkt_cmpl *rxcmp;
int rc;
@@ -2957,15 +2957,43 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
raw_cons = cpr->cp_raw_cons;
while (1) {
+ uint32_t agg_cnt, cons, cmpl_type;
+
cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
- rte_prefetch0(&cpr->cp_desc_ring[cons]);
rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
- if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct)) {
+ if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
break;
- } else {
- raw_cons++;
+
+ cmpl_type = CMP_TYPE(rxcmp);
+
+ switch (cmpl_type) {
+ case CMPL_BASE_TYPE_RX_L2:
+ case CMPL_BASE_TYPE_RX_L2_V2:
+ agg_cnt = BNXT_RX_L2_AGG_BUFS(rxcmp);
+ raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
+ desc++;
+ break;
+
+ case CMPL_BASE_TYPE_RX_TPA_END:
+ if (BNXT_CHIP_P5(rxq->bp)) {
+ struct rx_tpa_v2_end_cmpl_hi *p5_tpa_end;
+
+ p5_tpa_end = (void *)rxcmp;
+ agg_cnt = BNXT_TPA_END_AGG_BUFS_TH(p5_tpa_end);
+ } else {
+ struct rx_tpa_end_cmpl *tpa_end;
+
+ tpa_end = (void *)rxcmp;
+ agg_cnt = BNXT_TPA_END_AGG_BUFS(tpa_end);
+ }
+
+ raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
desc++;
+ break;
+
+ default:
+ raw_cons += CMP_LEN(cmpl_type);
}
}
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 4674f7cea2..e8f174fc57 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -827,8 +827,7 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
goto next_rx;
}
- agg_buf = (rxcmp->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK)
- >> RX_PKT_CMPL_AGG_BUFS_SFT;
+ agg_buf = BNXT_RX_L2_AGG_BUFS(rxcmp);
if (agg_buf && !bnxt_agg_bufs_valid(cpr, agg_buf, tmp_raw_cons))
return -EBUSY;
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index 0e31b37cad..06d10848da 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -37,6 +37,10 @@ static inline uint16_t bnxt_tpa_start_agg_id(struct bnxt *bp,
#define BNXT_TPA_END_AGG_ID_TH(cmp) \
rte_le_to_cpu_16((cmp)->agg_id)
+#define BNXT_RX_L2_AGG_BUFS(cmp) \
+ (((cmp)->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK) >> \
+ RX_PKT_CMPL_AGG_BUFS_SFT)
+
#define BNXT_RX_POST_THRESH32
/* Number of descriptors to process per inner loop in vector mode. */
--
2.25.1
Re: [dpdk-dev] [PATCH 21.05] app/testpmd: count outer IP checksum errors
> Lance, can you spare some time to make the above mentioned patch? Sure. Are you looking for something like the changes below, or also including a search and replace of the deprecated macro name with the new one? - /** External IP header checksum error. */ +/** + * Deprecated. + * This flag has been renamed, use PKT_RX_OUTER_IP_CKSUM_BAD instead. + */ +#define PKT_RX_EIP_CKSUM_BAD (1ULL << 5) + +/** + * This flag is set when the outermost IP header checksum is detected as + * wrong by the hardware. + */ #define PKT_RX_OUTER_IP_CKSUM_BAD (1ULL << 5) /**
[dpdk-dev] [PATCH] mbuf: rename outer ip checksum macro
Rename PKT_RX_EIP_CKSUM_BAD to PKT_RX_OUTER_IP_CKSUM_BAD and
deprecate the original name. The new name is better aligned
with existing PKT_RX_OUTER_* flags, which should help reduce
confusion about its purpose.
Suggested-by: Ferruh Yigit
Signed-off-by: Lance Richardson
---
app/test-pmd/csumonly.c | 2 +-
app/test/test_mbuf.c | 2 +-
doc/guides/nics/features.rst | 2 +-
drivers/event/octeontx/ssovf_worker.c| 2 +-
drivers/net/bnxt/bnxt_rxr.c | 2 +-
drivers/net/i40e/i40e_rxtx.c | 2 +-
drivers/net/i40e/i40e_rxtx_vec_altivec.c | 8 +++---
drivers/net/i40e/i40e_rxtx_vec_avx2.c| 18 ++--
drivers/net/i40e/i40e_rxtx_vec_avx512.c | 18 ++--
drivers/net/i40e/i40e_rxtx_vec_neon.c| 16 +--
drivers/net/i40e/i40e_rxtx_vec_sse.c | 16 +--
drivers/net/iavf/iavf_rxtx.c | 2 +-
drivers/net/iavf/iavf_rxtx_vec_avx2.c| 36
drivers/net/iavf/iavf_rxtx_vec_avx512.c | 36
drivers/net/iavf/iavf_rxtx_vec_sse.c | 32 ++---
drivers/net/ice/ice_rxtx.c | 2 +-
drivers/net/ice/ice_rxtx_vec_avx2.c | 34 +++---
drivers/net/ice/ice_rxtx_vec_avx512.c| 34 +++---
drivers/net/ice/ice_rxtx_vec_sse.c | 24
drivers/net/ixgbe/ixgbe_rxtx.c | 2 +-
drivers/net/octeontx2/otx2_lookup.c | 2 +-
drivers/net/qede/qede_rxtx.c | 4 +--
drivers/net/sfc/sfc_ef100_rx.c | 2 +-
drivers/net/sfc/sfc_ef10_rx_ev.h | 2 +-
drivers/net/txgbe/txgbe_rxtx.c | 2 +-
lib/librte_mbuf/rte_mbuf.c | 4 +--
lib/librte_mbuf/rte_mbuf_core.h | 11 +++-
27 files changed, 163 insertions(+), 154 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 37ed415ce..6b4df335f 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -864,7 +864,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
rx_bad_l4_csum += 1;
if (rx_ol_flags & PKT_RX_OUTER_L4_CKSUM_BAD)
rx_bad_outer_l4_csum += 1;
- if (rx_ol_flags & PKT_RX_EIP_CKSUM_BAD)
+ if (rx_ol_flags & PKT_RX_OUTER_IP_CKSUM_BAD)
rx_bad_outer_ip_csum += 1;
/* step 1: dissect packet, parsing optional vlan, ip4/ip6, vxlan
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index a40f7d488..7ed8b81d9 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -1613,7 +1613,7 @@ test_get_rx_ol_flag_name(void)
VAL_NAME(PKT_RX_IP_CKSUM_BAD),
VAL_NAME(PKT_RX_IP_CKSUM_GOOD),
VAL_NAME(PKT_RX_IP_CKSUM_NONE),
- VAL_NAME(PKT_RX_EIP_CKSUM_BAD),
+ VAL_NAME(PKT_RX_OUTER_IP_CKSUM_BAD),
VAL_NAME(PKT_RX_VLAN_STRIPPED),
VAL_NAME(PKT_RX_IEEE1588_PTP),
VAL_NAME(PKT_RX_IEEE1588_TMST),
diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index e9bb55376..dbca9a85b 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -612,7 +612,7 @@ Supports inner packet L3 checksum.
``mbuf.ol_flags:PKT_TX_OUTER_IP_CKSUM``,
``mbuf.ol_flags:PKT_TX_OUTER_IPV4`` | ``PKT_TX_OUTER_IPV6``.
* **[uses] mbuf**: ``mbuf.outer_l2_len``, ``mbuf.outer_l3_len``.
-* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_EIP_CKSUM_BAD``.
+* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_OUTER_IP_CKSUM_BAD``.
* **[provides] rte_eth_dev_info**:
``rx_offload_capa,rx_queue_offload_capa:DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM``,
``tx_offload_capa,tx_queue_offload_capa:DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM``.
diff --git a/drivers/event/octeontx/ssovf_worker.c
b/drivers/event/octeontx/ssovf_worker.c
index 3dfe665a2..a9149fb93 100644
--- a/drivers/event/octeontx/ssovf_worker.c
+++ b/drivers/event/octeontx/ssovf_worker.c
@@ -434,7 +434,7 @@ octeontx_create_rx_ol_flags_array(void *mem)
case OCCTX_ERRLEV_LC:
if (errcode == OCCTX_EC_IP4_CSUM) {
val |= PKT_RX_IP_CKSUM_BAD;
- val |= PKT_RX_EIP_CKSUM_BAD;
+ val |= PKT_RX_OUTER_IP_CKSUM_BAD;
} else {
val |= PKT_RX_IP_CKSUM_GOOD;
}
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 4674f7cea..91a00c7cf 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -529,7 +529,7 @@ bnxt_init_ol_flags_tables(struct bnxt_rx_queue *rxq)
pt[i] |= PKT_RX_IP_CKSUM_BAD;
if (i & (RX_PKT_CMPL_ERRORS_T_IP_CS_ERROR >> 4))
- pt[i] |
[dpdk-dev] [PATCH v2] mbuf: rename outer ip checksum macro
Rename PKT_RX_EIP_CKSUM_BAD to PKT_RX_OUTER_IP_CKSUM_BAD and deprecate the original name. The new name is better aligned with existing PKT_RX_OUTER_* flags, which should help reduce confusion about its use. Suggested-by: Ferruh Yigit Signed-off-by: Lance Richardson --- v2: - Added deprecation note. - Changed old macro definition to use the new one. app/test-pmd/csumonly.c | 2 +- app/test/test_mbuf.c | 2 +- doc/guides/nics/features.rst | 2 +- doc/guides/rel_notes/deprecation.rst | 5 drivers/event/octeontx/ssovf_worker.c| 2 +- drivers/net/bnxt/bnxt_rxr.c | 2 +- drivers/net/i40e/i40e_rxtx.c | 2 +- drivers/net/i40e/i40e_rxtx_vec_altivec.c | 8 +++--- drivers/net/i40e/i40e_rxtx_vec_avx2.c| 18 ++-- drivers/net/i40e/i40e_rxtx_vec_avx512.c | 18 ++-- drivers/net/i40e/i40e_rxtx_vec_neon.c| 16 +-- drivers/net/i40e/i40e_rxtx_vec_sse.c | 16 +-- drivers/net/iavf/iavf_rxtx.c | 2 +- drivers/net/iavf/iavf_rxtx_vec_avx2.c| 36 drivers/net/iavf/iavf_rxtx_vec_avx512.c | 36 drivers/net/iavf/iavf_rxtx_vec_sse.c | 32 ++--- drivers/net/ice/ice_rxtx.c | 2 +- drivers/net/ice/ice_rxtx_vec_avx2.c | 34 +++--- drivers/net/ice/ice_rxtx_vec_avx512.c| 34 +++--- drivers/net/ice/ice_rxtx_vec_sse.c | 24 drivers/net/ixgbe/ixgbe_rxtx.c | 2 +- drivers/net/octeontx2/otx2_lookup.c | 2 +- drivers/net/qede/qede_rxtx.c | 4 +-- drivers/net/sfc/sfc_ef100_rx.c | 2 +- drivers/net/sfc/sfc_ef10_rx_ev.h | 2 +- drivers/net/txgbe/txgbe_rxtx.c | 2 +- lib/librte_mbuf/rte_mbuf.c | 4 +-- lib/librte_mbuf/rte_mbuf_core.h | 13 +++-- 28 files changed, 169 insertions(+), 155 deletions(-) diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index 37ed415ce..6b4df335f 100644 --- a/app/test-pmd/csumonly.c +++ b/app/test-pmd/csumonly.c @@ -864,7 +864,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) rx_bad_l4_csum += 1; if (rx_ol_flags & PKT_RX_OUTER_L4_CKSUM_BAD) rx_bad_outer_l4_csum += 1; - if (rx_ol_flags & PKT_RX_EIP_CKSUM_BAD) + if (rx_ol_flags & PKT_RX_OUTER_IP_CKSUM_BAD) rx_bad_outer_ip_csum += 1; /* step 1: dissect packet, parsing optional vlan, ip4/ip6, vxlan diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index a40f7d488..7ed8b81d9 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -1613,7 +1613,7 @@ test_get_rx_ol_flag_name(void) VAL_NAME(PKT_RX_IP_CKSUM_BAD), VAL_NAME(PKT_RX_IP_CKSUM_GOOD), VAL_NAME(PKT_RX_IP_CKSUM_NONE), - VAL_NAME(PKT_RX_EIP_CKSUM_BAD), + VAL_NAME(PKT_RX_OUTER_IP_CKSUM_BAD), VAL_NAME(PKT_RX_VLAN_STRIPPED), VAL_NAME(PKT_RX_IEEE1588_PTP), VAL_NAME(PKT_RX_IEEE1588_TMST), diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst index e9bb55376..dbca9a85b 100644 --- a/doc/guides/nics/features.rst +++ b/doc/guides/nics/features.rst @@ -612,7 +612,7 @@ Supports inner packet L3 checksum. ``mbuf.ol_flags:PKT_TX_OUTER_IP_CKSUM``, ``mbuf.ol_flags:PKT_TX_OUTER_IPV4`` | ``PKT_TX_OUTER_IPV6``. * **[uses] mbuf**: ``mbuf.outer_l2_len``, ``mbuf.outer_l3_len``. -* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_EIP_CKSUM_BAD``. +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_OUTER_IP_CKSUM_BAD``. * **[provides] rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_capa:DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM``, ``tx_offload_capa,tx_queue_offload_capa:DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM``. diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index 82c1a90a3..86271024b 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -125,3 +125,8 @@ Deprecation Notices * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific content. On Linux and FreeBSD, supported prior to DPDK 20.11, original structure will be kept until DPDK 21.11. + +* ethdev: The offload flag ``PKT_RX_EIP_CKSUM_BAD`` will be removed and + replaced by the new flag ``PKT_RX_OUTER_IP_CKSUM_BAD``. The new name is more + consistent with existing outer header checksum status flag naming, which + should help in reducing confusion about its usage. diff --git a/drivers/event/octeontx/ssovf_worker.c b/drivers/event/octeontx/ssovf_worker.c index 3dfe665a2..a9149fb93 100644 --- a/drivers/event/octeontx/ssovf_worker.c +++ b/drivers/event/octeontx/ssovf_worker.c @@ -434,7 +434,7 @@ octeontx_create_rx_ol_flags_array(void *mem) case OCC
[dpdk-dev] [PATCH] net/bnxt: fix Rx descriptor status
Fix a number of issues in the bnxt receive descriptor status
function, including:
- Provide status of receive descriptor instead of completion
descriptor.
- Remove invalid comparison of raw ring index with masked ring
index.
- Correct misinterpretation of offset parameter as ring index.
- Correct misuse of completion ring index for mbuf ring (the
two rings have different sizes).
Fixes: 0fe613bb87b2 ("net/bnxt: support Rx descriptor status")
Signed-off-by: Lance Richardson
Cc: sta...@dpdk.org
Reviewed-by: Andy Gospodarek
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 108 ++---
1 file changed, 86 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 9824cdb6d8..477e04ef5a 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3003,42 +3003,106 @@ bnxt_rx_queue_count_op(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
static int
bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t offset)
{
- struct bnxt_rx_queue *rxq = (struct bnxt_rx_queue *)rx_queue;
- struct bnxt_rx_ring_info *rxr;
+ struct bnxt_rx_queue *rxq = rx_queue;
struct bnxt_cp_ring_info *cpr;
- struct rte_mbuf *rx_buf;
+ struct bnxt_rx_ring_info *rxr;
+ uint32_t desc, cons, raw_cons;
+ struct bnxt *bp = rxq->bp;
struct rx_pkt_cmpl *rxcmp;
- uint32_t cons, cp_cons;
int rc;
- if (!rxq)
- return -EINVAL;
-
- rc = is_bnxt_in_error(rxq->bp);
+ rc = is_bnxt_in_error(bp);
if (rc)
return rc;
- cpr = rxq->cp_ring;
- rxr = rxq->rx_ring;
-
if (offset >= rxq->nb_rx_desc)
return -EINVAL;
- cons = RING_CMP(cpr->cp_ring_struct, offset);
- cp_cons = cpr->cp_raw_cons;
- rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
+ rxr = rxq->rx_ring;
+ cpr = rxq->cp_ring;
- if (cons > cp_cons) {
- if (CMPL_VALID(rxcmp, cpr->valid))
- return RTE_ETH_RX_DESC_DONE;
- } else {
- if (CMPL_VALID(rxcmp, !cpr->valid))
+ /*
+* For the vector receive case, the completion at the requested
+* offset can be indexed directly.
+*/
+ if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
+ struct rx_pkt_cmpl *rxcmp;
+
+ /* Check status of completion descriptor. */
+ raw_cons = cpr->cp_raw_cons +
+ offset * CMP_LEN(CMPL_BASE_TYPE_RX_L2);
+ cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
+ rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
+
+ if (CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
return RTE_ETH_RX_DESC_DONE;
+
+ /* Check whether rx desc has an mbuf attached. */
+ cons = RING_CMP(rxr->rx_ring_struct, raw_cons / 2);
+ if (cons >= rxq->rxrearm_start &&
+ cons < rxq->rxrearm_start + rxq->rxrearm_nb) {
+ return RTE_ETH_RX_DESC_UNAVAIL;
+ }
+
+ return RTE_ETH_RX_DESC_AVAIL;
}
- rx_buf = rxr->rx_buf_ring[cons];
- if (rx_buf == NULL || rx_buf == &rxq->fake_mbuf)
- return RTE_ETH_RX_DESC_UNAVAIL;
+ /*
+* For the non-vector receive case, scan the completion ring to
+* locate the completion descriptor for the requested offset.
+*/
+ raw_cons = cpr->cp_raw_cons;
+ desc = 0;
+ while (1) {
+ uint32_t agg_cnt, cons, cmpl_type;
+
+ cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
+ rxcmp = (struct rx_pkt_cmpl *)&cpr->cp_desc_ring[cons];
+
+ if (!CMP_VALID(rxcmp, raw_cons, cpr->cp_ring_struct))
+ break;
+
+ cmpl_type = CMP_TYPE(rxcmp);
+
+ switch (cmpl_type) {
+ case CMPL_BASE_TYPE_RX_L2:
+ case CMPL_BASE_TYPE_RX_L2_V2:
+ if (desc == offset) {
+ cons = rxcmp->opaque;
+ if (rxr->rx_buf_ring[cons])
+ return RTE_ETH_RX_DESC_DONE;
+ else
+ return RTE_ETH_RX_DESC_UNAVAIL;
+ }
+ agg_cnt = BNXT_RX_L2_AGG_BUFS(rxcmp);
+ raw_cons = raw_cons + CMP_LEN(cmpl_type) + agg_cnt;
+ desc++;
+ break;
+
+ case CMPL_BASE_TYPE_RX_TPA_END:
+ if (desc == offset)
+ return RTE_ETH_RX_D
[dpdk-dev] [PATCH] net/bnxt: don't set mbuf pointers to null in vector PMD
Remove code for setting consumed mbuf pointers to NULL from the
vector receive functions as a minor performance optimization.
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
Reviewed-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_rxq.c | 10 ++
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 17 +
drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 17 +
3 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index f46b10c1c5..d345c37c8d 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -212,6 +212,16 @@ void bnxt_rx_queue_release_mbufs(struct bnxt_rx_queue *rxq)
sw_ring = rxq->rx_ring->rx_buf_ring;
if (sw_ring) {
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
+ /*
+* The vector receive burst function does not set used
+* mbuf pointers to NULL, do that here to simplify
+* cleanup logic.
+*/
+ for (i = 0; i < rxq->rxrearm_nb; i++)
+ sw_ring[rxq->rxrearm_start + i] = NULL;
+ rxq->rxrearm_nb = 0;
+#endif
for (i = 0;
i < rxq->rx_ring->rx_ring_struct->ring_size; i++) {
if (sw_ring[i]) {
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
b/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
index b0981da0f4..3d54d9d59d 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
@@ -274,22 +274,8 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
num_valid = (sizeof(uint64_t) / sizeof(uint16_t)) -
(__builtin_clzl(valid & desc_valid_mask) / 16);
- switch (num_valid) {
- case 4:
- rxr->rx_buf_ring[mbcons + 3] = NULL;
- /* FALLTHROUGH */
- case 3:
- rxr->rx_buf_ring[mbcons + 2] = NULL;
- /* FALLTHROUGH */
- case 2:
- rxr->rx_buf_ring[mbcons + 1] = NULL;
- /* FALLTHROUGH */
- case 1:
- rxr->rx_buf_ring[mbcons + 0] = NULL;
+ if (num_valid == 0)
break;
- case 0:
- goto out;
- }
descs_to_mbufs(rxcmp, rxcmp1, mb_init, &rx_pkts[nb_rx_pkts],
rxr);
@@ -299,7 +285,6 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
break;
}
-out:
if (nb_rx_pkts) {
rxr->rx_raw_prod = RING_ADV(rxr->rx_raw_prod, nb_rx_pkts);
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index 7ae6f5c098..7a58434ce9 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -256,22 +256,8 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
valid = _mm_cvtsi128_si64(_mm_packs_epi32(info3_v, info3_v));
num_valid = __builtin_popcountll(valid & desc_valid_mask);
- switch (num_valid) {
- case 4:
- rxr->rx_buf_ring[mbcons + 3] = NULL;
- /* FALLTHROUGH */
- case 3:
- rxr->rx_buf_ring[mbcons + 2] = NULL;
- /* FALLTHROUGH */
- case 2:
- rxr->rx_buf_ring[mbcons + 1] = NULL;
- /* FALLTHROUGH */
- case 1:
- rxr->rx_buf_ring[mbcons + 0] = NULL;
+ if (num_valid == 0)
break;
- case 0:
- goto out;
- }
descs_to_mbufs(rxcmp, rxcmp1, mbuf_init, &rx_pkts[nb_rx_pkts],
rxr);
@@ -281,7 +267,6 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
break;
}
-out:
if (nb_rx_pkts) {
rxr->rx_raw_prod = RING_ADV(rxr->rx_raw_prod, nb_rx_pkts);
--
2.25.1
Re: [dpdk-dev] [PATCH] net/bnxt: fix Rx descriptor status
On Tue, Mar 2, 2021 at 9:29 AM Lance Richardson
wrote:
>
> Fix a number of issues in the bnxt receive descriptor status
> function, including:
>- Provide status of receive descriptor instead of completion
> descriptor.
>- Remove invalid comparison of raw ring index with masked ring
> index.
>- Correct misinterpretation of offset parameter as ring index.
>- Correct misuse of completion ring index for mbuf ring (the
> two rings have different sizes).
>
> Fixes: 0fe613bb87b2 ("net/bnxt: support Rx descriptor status")
> Signed-off-by: Lance Richardson
> Cc: sta...@dpdk.org
> Reviewed-by: Andy Gospodarek
> Reviewed-by: Ajit Kumar Khaparde
> ---
This patch has a dependency on:
http://patchwork.dpdk.org/project/dpdk/list/?series=15289&archive=both&state=*
[dpdk-dev] [PATCH] net/bnxt: optimizations for Tx completion handling
Avoid copying mbuf pointers to separate array for bulk
mbuf free when handling transmit completions for vector
mode transmit.
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 4 +-
drivers/net/bnxt/bnxt_ring.c| 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_common.h | 89 +++--
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 5 +-
drivers/net/bnxt/bnxt_rxtx_vec_sse.c| 7 +-
drivers/net/bnxt/bnxt_txq.c | 8 +--
drivers/net/bnxt/bnxt_txr.c | 68 ++-
drivers/net/bnxt/bnxt_txr.h | 7 +-
8 files changed, 106 insertions(+), 84 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 88da345034..d4028e2bb2 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3186,7 +3186,7 @@ bnxt_tx_descriptor_status_op(void *tx_queue, uint16_t
offset)
struct bnxt_tx_queue *txq = (struct bnxt_tx_queue *)tx_queue;
struct bnxt_tx_ring_info *txr;
struct bnxt_cp_ring_info *cpr;
- struct bnxt_sw_tx_bd *tx_buf;
+ struct rte_mbuf **tx_buf;
struct tx_pkt_cmpl *txcmp;
uint32_t cons, cp_cons;
int rc;
@@ -3216,7 +3216,7 @@ bnxt_tx_descriptor_status_op(void *tx_queue, uint16_t
offset)
return RTE_ETH_TX_DESC_UNAVAIL;
}
tx_buf = &txr->tx_buf_ring[cons];
- if (tx_buf->mbuf == NULL)
+ if (*tx_buf == NULL)
return RTE_ETH_TX_DESC_DONE;
return RTE_ETH_TX_DESC_FULL;
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 997dcdc28b..e4055fa49b 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -230,7 +230,7 @@ int bnxt_alloc_rings(struct bnxt *bp, uint16_t qidx,
tx_ring->vmem =
(void **)((char *)mz->addr + tx_vmem_start);
tx_ring_info->tx_buf_ring =
- (struct bnxt_sw_tx_bd *)tx_ring->vmem;
+ (struct rte_mbuf **)tx_ring->vmem;
}
}
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_common.h
b/drivers/net/bnxt/bnxt_rxtx_vec_common.h
index 91ff6736b1..9b9489a695 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_common.h
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_common.h
@@ -100,57 +100,78 @@ bnxt_rxq_rearm(struct bnxt_rx_queue *rxq, struct
bnxt_rx_ring_info *rxr)
* is enabled.
*/
static inline void
-bnxt_tx_cmp_vec_fast(struct bnxt_tx_queue *txq, int nr_pkts)
+bnxt_tx_cmp_vec_fast(struct bnxt_tx_queue *txq, uint32_t nr_pkts)
{
struct bnxt_tx_ring_info *txr = txq->tx_ring;
- struct rte_mbuf **free = txq->free;
uint16_t cons, raw_cons = txr->tx_raw_cons;
- unsigned int blk = 0;
- uint32_t ring_mask = txr->tx_ring_struct->ring_mask;
-
- while (nr_pkts--) {
- struct bnxt_sw_tx_bd *tx_buf;
-
- cons = raw_cons++ & ring_mask;
- tx_buf = &txr->tx_buf_ring[cons];
- free[blk++] = tx_buf->mbuf;
- tx_buf->mbuf = NULL;
+ uint32_t ring_mask, ring_size, num;
+ struct rte_mempool *pool;
+
+ ring_mask = txr->tx_ring_struct->ring_mask;
+ ring_size = txr->tx_ring_struct->ring_size;
+
+ cons = raw_cons & ring_mask;
+ num = RTE_MIN(nr_pkts, ring_size - cons);
+ pool = txr->tx_buf_ring[cons]->pool;
+
+ rte_mempool_put_bulk(pool, (void **)&txr->tx_buf_ring[cons], num);
+ memset(&txr->tx_buf_ring[cons], 0, num * sizeof(struct rte_mbuf *));
+ raw_cons += num;
+ num = nr_pkts - num;
+ if (num) {
+ cons = raw_cons & ring_mask;
+ rte_mempool_put_bulk(pool, (void **)&txr->tx_buf_ring[cons],
+num);
+ memset(&txr->tx_buf_ring[cons], 0,
+ num * sizeof(struct rte_mbuf *));
+ raw_cons += num;
}
- if (blk)
- rte_mempool_put_bulk(free[0]->pool, (void **)free, blk);
txr->tx_raw_cons = raw_cons;
}
static inline void
-bnxt_tx_cmp_vec(struct bnxt_tx_queue *txq, int nr_pkts)
+bnxt_tx_cmp_vec(struct bnxt_tx_queue *txq, uint32_t nr_pkts)
{
struct bnxt_tx_ring_info *txr = txq->tx_ring;
- struct rte_mbuf **free = txq->free;
uint16_t cons, raw_cons = txr->tx_raw_cons;
- unsigned int blk = 0;
- uint32_t ring_mask = txr->tx_ring_struct->ring_mask;
+ uint32_t ring_mask, ring_size, num, blk;
+ struct rte_mempool *pool;
- while (nr_pkts--) {
- struct bnxt_sw_tx_bd *tx_buf;
- struct rte_mbuf *mbuf;
+ ring_mask = txr->tx_ring_struct->ring_mask;
+ ring_size = txr->tx_ring_struct->ring_size;
Re: [dpdk-dev] [PATCH v4 2/4] eal: add asprintf() internal wrapper
On Fri, Mar 5, 2021 at 7:05 PM Dmitry Kozlyuk wrote: > > POSIX asprintf() is unavailable on Windows. AFAIK asprintf() is not a POSIX API, it is a GNU extension that has also been implemented in some BSDs. > Add eal_asprintf() wrapper for EAL internal use. > On Windows it's a function, on Unix it's a macro for asprintf(). > > Signed-off-by: Dmitry Kozlyuk > Acked-by: Khoa To > ---
[dpdk-dev] [PATCH 1/1] net/bnxt: fix transmit length hint threshold
Use correct threshold when selecting "greater than or equal to
2K" length hint.
Fixes: 6eb3cc2294fd ("net/bnxt: add initial Tx code")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
Reviewed-by: Somnath Kotur
---
drivers/net/bnxt/bnxt_txr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/bnxt_txr.c b/drivers/net/bnxt/bnxt_txr.c
index 65355fb040..27459960de 100644
--- a/drivers/net/bnxt/bnxt_txr.c
+++ b/drivers/net/bnxt/bnxt_txr.c
@@ -187,7 +187,7 @@ static uint16_t bnxt_start_xmit(struct rte_mbuf *tx_pkt,
txbd->flags_type |= TX_BD_SHORT_FLAGS_COAL_NOW;
txbd->flags_type |= TX_BD_LONG_FLAGS_NO_CMPL;
txbd->len = tx_pkt->data_len;
- if (tx_pkt->pkt_len >= 2014)
+ if (tx_pkt->pkt_len >= 2048)
txbd->flags_type |= TX_BD_LONG_FLAGS_LHINT_GTE2K;
else
txbd->flags_type |= lhint_arr[tx_pkt->pkt_len >> 9];
--
2.25.1
[dpdk-dev] [PATCH 1/1] net/bnxt: fix Rx buffer posting
Remove early buffer posting logic from burst receive loop to address
several issues:
- Posting receive descriptors without first posting completion
entries risks overflowing the completion queue.
- Posting receive descriptors without updating rx_raw_prod
creates the possibility that the receive descriptor doorbell
can be written twice with the same value.
- Having this logic in the inner descriptor processing loop
can impact performance.
Fixes: 637e34befd9c ("net/bnxt: optimize Rx processing")
Fixes: 04067844a3e9 ("net/bnxt: reduce CQ queue size without aggregation ring")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_rxr.c | 3 ---
drivers/net/bnxt/bnxt_rxr.h | 2 --
2 files changed, 5 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index c72545ada7..7179c6cb30 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1018,9 +1018,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts,
raw_cons = NEXT_RAW_CMP(raw_cons);
if (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt)
break;
- /* Post some Rx buf early in case of larger burst processing */
- if (nb_rx_pkts == BNXT_RX_POST_THRESH)
- bnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);
}
cpr->cp_raw_cons = raw_cons;
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index a6fdd7767a..b43256e03e 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -41,8 +41,6 @@ static inline uint16_t bnxt_tpa_start_agg_id(struct bnxt *bp,
(((cmp)->agg_bufs_v1 & RX_PKT_CMPL_AGG_BUFS_MASK) >> \
RX_PKT_CMPL_AGG_BUFS_SFT)
-#define BNXT_RX_POST_THRESH32
-
/* Number of descriptors to process per inner loop in vector mode. */
#define RTE_BNXT_DESCS_PER_LOOP4U
--
2.25.1
[dpdk-dev] [PATCH 1/1] net/bnxt: fix handling of null flow mask
When the mask field of an rte_flow pattern item is NULL,
the default mask for that item type should be used.
Fixes: 5ef3b79fdfe6 ("net/bnxt: support flow filter ops")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
drivers/net/bnxt/bnxt_flow.c | 47 +++-
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index a8f5d91fc4..e3906b4779 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -188,11 +188,15 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
PMD_DRV_LOG(DEBUG, "Parse inner header\n");
break;
case RTE_FLOW_ITEM_TYPE_ETH:
- if (!item->spec || !item->mask)
+ if (!item->spec)
break;
eth_spec = item->spec;
- eth_mask = item->mask;
+
+ if (item->mask)
+ eth_mask = item->mask;
+ else
+ eth_mask = &rte_flow_item_eth_mask;
/* Source MAC address mask cannot be partially set.
* Should be All 0's or all 1's.
@@ -281,7 +285,12 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
break;
case RTE_FLOW_ITEM_TYPE_VLAN:
vlan_spec = item->spec;
- vlan_mask = item->mask;
+
+ if (item->mask)
+ vlan_mask = item->mask;
+ else
+ vlan_mask = &rte_flow_item_vlan_mask;
+
if (en & en_ethertype) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ITEM,
@@ -324,11 +333,15 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
case RTE_FLOW_ITEM_TYPE_IPV4:
/* If mask is not involved, we could use EM filters. */
ipv4_spec = item->spec;
- ipv4_mask = item->mask;
- if (!item->spec || !item->mask)
+ if (!item->spec)
break;
+ if (item->mask)
+ ipv4_mask = item->mask;
+ else
+ ipv4_mask = &rte_flow_item_ipv4_mask;
+
/* Only IP DST and SRC fields are maskable. */
if (ipv4_mask->hdr.version_ihl ||
ipv4_mask->hdr.type_of_service ||
@@ -385,11 +398,15 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
break;
case RTE_FLOW_ITEM_TYPE_IPV6:
ipv6_spec = item->spec;
- ipv6_mask = item->mask;
- if (!item->spec || !item->mask)
+ if (!item->spec)
break;
+ if (item->mask)
+ ipv6_mask = item->mask;
+ else
+ ipv6_mask = &rte_flow_item_ipv6_mask;
+
/* Only IP DST and SRC fields are maskable. */
if (ipv6_mask->hdr.vtc_flow ||
ipv6_mask->hdr.payload_len ||
@@ -437,11 +454,15 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
break;
case RTE_FLOW_ITEM_TYPE_TCP:
tcp_spec = item->spec;
- tcp_mask = item->mask;
- if (!item->spec || !item->mask)
+ if (!item->spec)
break;
+ if (item->mask)
+ tcp_mask = item->mask;
+ else
+ tcp_mask = &rte_flow_item_tcp_mask;
+
/* Check TCP mask. Only DST & SRC ports are maskable */
if (tcp_mask->hdr.sent_seq ||
tcp_mask->hdr.recv_ack ||
@@ -482,11 +503,15 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
break;
case RTE_FLOW_ITEM_TYPE_UDP:
udp_spec = item->spec;
- udp_mask = item->mask;
- if (!item->spec || !item->mask)
+ if (!item->spec)
break;
+ if (item->mask)
+ udp_mask = item->mask;
+
Re: [dpdk-dev] Questions about keeping CRC
On Fri, Mar 19, 2021 at 12:07 PM Stephen Hemminger wrote: > > On Fri, 19 Mar 2021 20:13:20 +0800 > "Min Hu (Connor)" wrote: > > > Hi, all, > > DPDK has introduced one offload: DEV_RX_OFFLOAD_KEEP_CRC. It means > > that > > the device has the ablility of keeping CRC（four bytes at the end of > > packet）of packet in RX. > > In common scenarios, When one packet enter into NIC device, NIC > > will check the CRC and then strip the CRC，at last send the packet into > > the buffer. > > So my question is: > >why the DEV_RX_OFFLOAD_KEEP_CRC is introduced into DPDK? I think > > that > > when the packet enter into the NIC, the CRC will has no significance to > > APP. Or is there any scenarios that CRC is useful for APP？ > > Thanks for your reply. > > Your right it doesn't make sense for almost all applications. Maybe an > application > testing for bad NIC hardware might use it. > > It is one of those features introduced in DPDK because "our hardware can do > it, > therefore it ought to be exposed in DPDK API"... The only use case I have seen was in L2 forwarding applications which would receive packets with CRC preserved and then transmit them with an indication to the NIC that the CRC should not be regenerated. The idea was that if the packet was corrupted anywhere in the system (e.g. by a memory error), it could be detected at the receiver. Of course DPDK doesn't have the notion of transmitting a packet without regenerating the CRC, so that use case doesn't seem to apply here. I think that DEV_RX_OFFLOAD_KEEP_CRC is not likely to be useful, but I would be interested in hearing otherwise. I happen to know of at least one PMD that advertises this ability but doesn't actually behave any differently when it is enabled.
[dpdk-dev] [PATCH] app/test: fix IPv6 header initialization
Fix two issues found when writing PMD unit tests for HW ptype and
L4 checksum offload:
- The version field in the IPv6 header was being set to zero,
which prevented hardware from recognizing it as IPv6. The
IP version field is now set to six.
- The payload_len field was being initialized using host byte
order, which (among other things) resulted in incorrect L4
checksum computation. The payload_len field is now set using
network (big-endian) byte order.
Fixes: 92073ef961ee ("bond: unit tests")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
app/test/packet_burst_generator.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/test/packet_burst_generator.c
b/app/test/packet_burst_generator.c
index f203f9d09e..8b390853a2 100644
--- a/app/test/packet_burst_generator.c
+++ b/app/test/packet_burst_generator.c
@@ -141,8 +141,8 @@ uint16_t
initialize_ipv6_header(struct rte_ipv6_hdr *ip_hdr, uint8_t *src_addr,
uint8_t *dst_addr, uint16_t pkt_data_len)
{
- ip_hdr->vtc_flow = 0;
- ip_hdr->payload_len = pkt_data_len;
+ ip_hdr->vtc_flow = rte_cpu_to_be_32(0x6000); /* Set version to 6. */
+ ip_hdr->payload_len = rte_cpu_to_be_16(pkt_data_len);
ip_hdr->proto = IPPROTO_UDP;
ip_hdr->hop_limits = IP_DEFTTL;
--
2.25.1
[dpdk-dev] [PATCH] eal: fix memory mapping for 32-bit targets
For 32-bit targets, size_t is normally a 32-bit type and
does not have sufficient range to represent 64-bit offsets
that can are needed when mapping PCI addresses. Use off_t
instead, which is usually a 64-bit type when compiled with
_D_FILE_OFFSET_BITS=64 as is the case for DPDK.
Found when attempting to run 32-bit Linux dpdk-testpmd
using VFIO driver:
EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
0x20, 0x200): Invalid argument ((nil))
Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
lib/eal/include/rte_eal_paging.h | 2 +-
lib/eal/unix/eal_unix_memory.c | 10 +-
lib/eal/windows/eal_memory.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/eal/include/rte_eal_paging.h b/lib/eal/include/rte_eal_paging.h
index ed98e70e9e..2c05025ffc 100644
--- a/lib/eal/include/rte_eal_paging.h
+++ b/lib/eal/include/rte_eal_paging.h
@@ -61,7 +61,7 @@ enum rte_map_flags {
__rte_internal
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset);
+ int fd, off_t offset);
/**
* OS-independent implementation of POSIX munmap(3).
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index ec7156df96..51a42e1a43 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -24,14 +24,14 @@
static void *
mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, off_t offset)
{
void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
if (virt == MAP_FAILED) {
RTE_LOG(DEBUG, EAL,
- "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
- requested_addr, size, prot, flags, fd, offset,
- strerror(errno));
+ "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%llx): %s\n",
+ requested_addr, size, prot, flags, fd,
+ (unsigned long long)offset, strerror(errno));
rte_errno = errno;
return NULL;
}
@@ -106,7 +106,7 @@ mem_rte_to_sys_prot(int prot)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, off_t offset)
{
int sys_flags = 0;
int sys_prot;
diff --git a/lib/eal/windows/eal_memory.c b/lib/eal/windows/eal_memory.c
index 2cf5a5e649..f1c4b03e96 100644
--- a/lib/eal/windows/eal_memory.c
+++ b/lib/eal/windows/eal_memory.c
@@ -508,7 +508,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, off_t offset)
{
HANDLE file_handle = INVALID_HANDLE_VALUE;
HANDLE mapping_handle = INVALID_HANDLE_VALUE;
--
2.25.1
Re: [dpdk-dev] [PATCH] eal: fix memory mapping for 32-bit targets
On Fri, May 7, 2021 at 2:10 PM Lance Richardson
wrote:
>
> For 32-bit targets, size_t is normally a 32-bit type and
> does not have sufficient range to represent 64-bit offsets
> that can are needed when mapping PCI addresses. Use off_t
> instead, which is usually a 64-bit type when compiled with
> _D_FILE_OFFSET_BITS=64 as is the case for DPDK.
>
> Found when attempting to run 32-bit Linux dpdk-testpmd
> using VFIO driver:
>
> EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
> 0x20, 0x200): Invalid argument ((nil))
>
> Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
> Cc: sta...@dpdk.org
> Signed-off-by: Lance Richardson
> ---
> lib/eal/include/rte_eal_paging.h | 2 +-
> lib/eal/unix/eal_unix_memory.c | 10 +-
> lib/eal/windows/eal_memory.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/lib/eal/include/rte_eal_paging.h
> b/lib/eal/include/rte_eal_paging.h
> index ed98e70e9e..2c05025ffc 100644
> --- a/lib/eal/include/rte_eal_paging.h
> +++ b/lib/eal/include/rte_eal_paging.h
> @@ -61,7 +61,7 @@ enum rte_map_flags {
> __rte_internal
> void *
> rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
> - int fd, size_t offset);
> + int fd, off_t offset);
>
> /**
> * OS-independent implementation of POSIX munmap(3).
> diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
> index ec7156df96..51a42e1a43 100644
> --- a/lib/eal/unix/eal_unix_memory.c
> +++ b/lib/eal/unix/eal_unix_memory.c
> @@ -24,14 +24,14 @@
>
> static void *
> mem_map(void *requested_addr, size_t size, int prot, int flags,
> - int fd, size_t offset)
> + int fd, off_t offset)
> {
> void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
> if (virt == MAP_FAILED) {
> RTE_LOG(DEBUG, EAL,
> - "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
> - requested_addr, size, prot, flags, fd, offset,
> - strerror(errno));
> + "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%llx):
> %s\n",
> + requested_addr, size, prot, flags, fd,
> + (unsigned long long)offset, strerror(errno));
> rte_errno = errno;
> return NULL;
> }
> @@ -106,7 +106,7 @@ mem_rte_to_sys_prot(int prot)
>
> void *
> rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
> - int fd, size_t offset)
> + int fd, off_t offset)
> {
> int sys_flags = 0;
> int sys_prot;
> diff --git a/lib/eal/windows/eal_memory.c b/lib/eal/windows/eal_memory.c
> index 2cf5a5e649..f1c4b03e96 100644
> --- a/lib/eal/windows/eal_memory.c
> +++ b/lib/eal/windows/eal_memory.c
> @@ -508,7 +508,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
>
> void *
> rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
> - int fd, size_t offset)
> + int fd, off_t offset)
> {
> HANDLE file_handle = INVALID_HANDLE_VALUE;
> HANDLE mapping_handle = INVALID_HANDLE_VALUE;
> --
> 2.25.1
>
Windows compilation is failing in CI, apparently Windows doesn't
understand "off_t".
Should we add an "rte_off_t" definition for non-POSIX portability?
Re: [dpdk-dev] [PATCH] eal: fix memory mapping for 32-bit targets
> Windows compilation is failing in CI, apparently Windows doesn't > understand "off_t". > > Should we add an "rte_off_t" definition for non-POSIX portability? On second thought, it's probably better to simply use uint64_t.
[dpdk-dev] [PATCH v2] eal: fix memory mapping for 32-bit targets
For 32-bit targets, size_t is normally a 32-bit type and
does not have sufficient range to represent 64-bit offsets
that can are needed when mapping PCI addresses. Use uint64_t
instead.
Found when attempting to run 32-bit Linux dpdk-testpmd
using VFIO driver:
EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
0x20, 0x200): Invalid argument ((nil))
Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
v2: Use uint64_t instead of off_t (off_t is unknown to Windows).
lib/eal/include/rte_eal_paging.h | 2 +-
lib/eal/unix/eal_unix_memory.c | 10 +-
lib/eal/windows/eal_memory.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/eal/include/rte_eal_paging.h b/lib/eal/include/rte_eal_paging.h
index ed98e70e9e..c60317d0f5 100644
--- a/lib/eal/include/rte_eal_paging.h
+++ b/lib/eal/include/rte_eal_paging.h
@@ -61,7 +61,7 @@ enum rte_map_flags {
__rte_internal
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset);
+ int fd, uint64_t offset);
/**
* OS-independent implementation of POSIX munmap(3).
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index ec7156df96..41a94a7511 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -24,14 +24,14 @@
static void *
mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
if (virt == MAP_FAILED) {
RTE_LOG(DEBUG, EAL,
- "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
- requested_addr, size, prot, flags, fd, offset,
- strerror(errno));
+ "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%llx): %s\n",
+ requested_addr, size, prot, flags, fd,
+ (unsigned long long)offset, strerror(errno));
rte_errno = errno;
return NULL;
}
@@ -106,7 +106,7 @@ mem_rte_to_sys_prot(int prot)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
int sys_flags = 0;
int sys_prot;
diff --git a/lib/eal/windows/eal_memory.c b/lib/eal/windows/eal_memory.c
index 2cf5a5e649..4db048ccb5 100644
--- a/lib/eal/windows/eal_memory.c
+++ b/lib/eal/windows/eal_memory.c
@@ -508,7 +508,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
HANDLE file_handle = INVALID_HANDLE_VALUE;
HANDLE mapping_handle = INVALID_HANDLE_VALUE;
--
2.25.1
Re: [dpdk-dev] [PATCH v2] eal: fix memory mapping for 32-bit targets
On Fri, May 7, 2021 at 4:51 PM Dmitry Kozlyuk wrote:
>
> 2021-05-07 15:06 (UTC-0400), Lance Richardson:
> [...]
> > diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
> > index ec7156df96..41a94a7511 100644
> > --- a/lib/eal/unix/eal_unix_memory.c
> > +++ b/lib/eal/unix/eal_unix_memory.c
> > @@ -24,14 +24,14 @@
> >
> > static void *
> > mem_map(void *requested_addr, size_t size, int prot, int flags,
> > - int fd, size_t offset)
> > + int fd, uint64_t offset)
> > {
> > void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
> > if (virt == MAP_FAILED) {
> > RTE_LOG(DEBUG, EAL,
> > - "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
> > - requested_addr, size, prot, flags, fd, offset,
> > - strerror(errno));
> > + "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%llx):
> > %s\n",
> > + requested_addr, size, prot, flags, fd,
> > + (unsigned long long)offset, strerror(errno));
>
> Why not PRIx64?
Good point. %llx made sense for off_t.
Thanks,
Lance
[dpdk-dev] [PATCH] eal: fix memory mapping for 32-bit targets
For 32-bit targets, size_t is normally a 32-bit type and
does not have sufficient range to represent 64-bit offsets
that can are needed when mapping PCI addresses. Use uint64_t
instead.
Found when attempting to run 32-bit Linux dpdk-testpmd
using VFIO driver:
EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
0x20, 0x200): Invalid argument ((nil))
Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
v3: Use PRIx64 instead of llx to format offset in log message.
v2: Use uint64_t instead of off_t (off_t is unknown to Windows).
lib/eal/include/rte_eal_paging.h | 2 +-
lib/eal/unix/eal_unix_memory.c | 11 ++-
lib/eal/windows/eal_memory.c | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/eal/include/rte_eal_paging.h b/lib/eal/include/rte_eal_paging.h
index ed98e70e9e..c60317d0f5 100644
--- a/lib/eal/include/rte_eal_paging.h
+++ b/lib/eal/include/rte_eal_paging.h
@@ -61,7 +61,7 @@ enum rte_map_flags {
__rte_internal
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset);
+ int fd, uint64_t offset);
/**
* OS-independent implementation of POSIX munmap(3).
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index ec7156df96..68ae93bd6e 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
#include
@@ -24,14 +25,14 @@
static void *
mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
if (virt == MAP_FAILED) {
RTE_LOG(DEBUG, EAL,
- "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
- requested_addr, size, prot, flags, fd, offset,
- strerror(errno));
+ "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%"PRIx64"): %s\n",
+ requested_addr, size, prot, flags, fd, offset,
+ strerror(errno));
rte_errno = errno;
return NULL;
}
@@ -106,7 +107,7 @@ mem_rte_to_sys_prot(int prot)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
int sys_flags = 0;
int sys_prot;
diff --git a/lib/eal/windows/eal_memory.c b/lib/eal/windows/eal_memory.c
index 2cf5a5e649..4db048ccb5 100644
--- a/lib/eal/windows/eal_memory.c
+++ b/lib/eal/windows/eal_memory.c
@@ -508,7 +508,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
HANDLE file_handle = INVALID_HANDLE_VALUE;
HANDLE mapping_handle = INVALID_HANDLE_VALUE;
--
2.25.1
[dpdk-dev] [PATCH v3] eal: fix memory mapping for 32-bit targets
For 32-bit targets, size_t is normally a 32-bit type and
does not have sufficient range to represent 64-bit offsets
that can are needed when mapping PCI addresses. Use uint64_t
instead.
Found when attempting to run 32-bit Linux dpdk-testpmd
using VFIO driver:
EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
0x20, 0x200): Invalid argument ((nil))
Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
v3: Use PRIx64 instead of llx to format offset in log message.
v2: Use uint64_t instead of off_t (off_t is unknown to Windows).
lib/eal/include/rte_eal_paging.h | 2 +-
lib/eal/unix/eal_unix_memory.c | 11 ++-
lib/eal/windows/eal_memory.c | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/eal/include/rte_eal_paging.h b/lib/eal/include/rte_eal_paging.h
index ed98e70e9e..c60317d0f5 100644
--- a/lib/eal/include/rte_eal_paging.h
+++ b/lib/eal/include/rte_eal_paging.h
@@ -61,7 +61,7 @@ enum rte_map_flags {
__rte_internal
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset);
+ int fd, uint64_t offset);
/**
* OS-independent implementation of POSIX munmap(3).
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index ec7156df96..68ae93bd6e 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
#include
@@ -24,14 +25,14 @@
static void *
mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
if (virt == MAP_FAILED) {
RTE_LOG(DEBUG, EAL,
- "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
- requested_addr, size, prot, flags, fd, offset,
- strerror(errno));
+ "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%"PRIx64"): %s\n",
+ requested_addr, size, prot, flags, fd, offset,
+ strerror(errno));
rte_errno = errno;
return NULL;
}
@@ -106,7 +107,7 @@ mem_rte_to_sys_prot(int prot)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
int sys_flags = 0;
int sys_prot;
diff --git a/lib/eal/windows/eal_memory.c b/lib/eal/windows/eal_memory.c
index 2cf5a5e649..4db048ccb5 100644
--- a/lib/eal/windows/eal_memory.c
+++ b/lib/eal/windows/eal_memory.c
@@ -508,7 +508,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, uint64_t offset)
{
HANDLE file_handle = INVALID_HANDLE_VALUE;
HANDLE mapping_handle = INVALID_HANDLE_VALUE;
--
2.25.1
Re: [dpdk-dev] [PATCH v3] eal: fix memory mapping for 32-bit targets
On Mon, May 10, 2021 at 11:55 AM Thomas Monjalon
wrote:
> 10/05/2021 17:31, Burakov, Anatoly:
> > On 10-May-21 1:42 PM, Thomas Monjalon wrote:
> > > +Cc Anatoly
> > >
> > > 08/05/2021 16:27, Lance Richardson:
> > >> For 32-bit targets, size_t is normally a 32-bit type and
> > >> does not have sufficient range to represent 64-bit offsets
> > >> that can are needed when mapping PCI addresses. Use uint64_t
> > >> instead.
> > >>
> > >> Found when attempting to run 32-bit Linux dpdk-testpmd
> > >> using VFIO driver:
> > >>
> > >> EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
> > >> 0x20, 0x200): Invalid argument ((nil))
> > >>
> > >> Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
> > >> Cc: sta...@dpdk.org
> > >> Signed-off-by: Lance Richardson
> > >> ---
> >
> > LGTM, although the Fixes: tag is probably wrong because IIRC previous
> > versions did have the same issue.
>
> What should be the right Fixes tag Anatoly?
>
I think this would be more appropriate (prior to this commit,
pci_map_resource()
called mmap() directly with off_t offset, with this commit offset was
narrowed
to size_t before calling mmap()):
Fixes: 2fd3567e5425 ("pci: use OS generic memory mapping functions")
Re: [dpdk-dev] [PATCH v3] eal: fix memory mapping for 32-bit targets
On Tue, May 11, 2021 at 4:18 AM Thomas Monjalon wrote:
>
> 08/05/2021 16:27, Lance Richardson:
> > For 32-bit targets, size_t is normally a 32-bit type and
> > does not have sufficient range to represent 64-bit offsets
> > that can are needed when mapping PCI addresses. Use uint64_t
> > instead.
> >
> > Found when attempting to run 32-bit Linux dpdk-testpmd
> > using VFIO driver:
> >
> > EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
> > 0x20, 0x200): Invalid argument ((nil))
> >
> > Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
> > Cc: sta...@dpdk.org
> > Signed-off-by: Lance Richardson
> > ---
> > v3: Use PRIx64 instead of llx to format offset in log message.
> > v2: Use uint64_t instead of off_t (off_t is unknown to Windows).
>
> off_t is referenced in drivers/bus/pci/windows/pci.c
> (for unused parameter) so it should be fine?
> If not, we could add it?
v1 of the patch had the following build error in CI:
*Build Failed #1:
OS: WIN10-64
Target: x86_64-windows-clang
FAILED: lib/librte_eal.a.p/eal_common_eal_common_fbarray.c.obj
In file included from ../lib/eal/common/eal_common_fbarray.c:14:
..\lib\eal\include\rte_eal_paging.h:64:10: error: unknown type name 'off_t'
int fd, off_t offset);
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/test: fix IPv6 header initialization
On Tue, May 11, 2021 at 10:31 AM David Marchand wrote: > > On Fri, Mar 26, 2021 at 5:37 PM Lance Richardson > wrote: > > > > Fix two issues found when writing PMD unit tests for HW ptype and > > L4 checksum offload: > > Would those unit tests be interesting to other pmd driver writers? > I think so, although some adjustments would be needed to account for differences in hardware capabilities. The tests I've written so far are still very much a work in progress, but I hope to have something ready for RFC in the near future.
[dpdk-dev] [PATCH v4] eal: fix memory mapping for 32-bit targets
For 32-bit targets, size_t is normally a 32-bit type and
does not have sufficient range to represent 64-bit offsets
that can are needed when mapping PCI addresses. Use off_t
instead, which is usually a 64-bit type when compiled with
_D_FILE_OFFSET_BITS=64 as is the case for DPDK.
Found when attempting to run 32-bit Linux dpdk-testpmd
using VFIO driver:
EAL: pci_map_resource(): cannot map resource(63, 0xc001, \
0x20, 0x200): Invalid argument ((nil))
Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
---
v4: Identical to v1, with now included in rte_eal_paging.h
to (hopefully) make off_t available for Windows builds.
v3: Use PRIx64 instead of llx ot format offset in log message.
v2: Use uint64_t instead of off_t (off_t is unknown to Windows).
lib/eal/include/rte_eal_paging.h | 3 ++-
lib/eal/unix/eal_unix_memory.c | 10 +-
lib/eal/windows/eal_memory.c | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/eal/include/rte_eal_paging.h b/lib/eal/include/rte_eal_paging.h
index ed98e70e9e..974bf5ee64 100644
--- a/lib/eal/include/rte_eal_paging.h
+++ b/lib/eal/include/rte_eal_paging.h
@@ -3,6 +3,7 @@
*/
#include
+#include
#include
@@ -61,7 +62,7 @@ enum rte_map_flags {
__rte_internal
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset);
+ int fd, off_t offset);
/**
* OS-independent implementation of POSIX munmap(3).
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index ec7156df96..51a42e1a43 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -24,14 +24,14 @@
static void *
mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, off_t offset)
{
void *virt = mmap(requested_addr, size, prot, flags, fd, offset);
if (virt == MAP_FAILED) {
RTE_LOG(DEBUG, EAL,
- "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%zx): %s\n",
- requested_addr, size, prot, flags, fd, offset,
- strerror(errno));
+ "Cannot mmap(%p, 0x%zx, 0x%x, 0x%x, %d, 0x%llx): %s\n",
+ requested_addr, size, prot, flags, fd,
+ (unsigned long long)offset, strerror(errno));
rte_errno = errno;
return NULL;
}
@@ -106,7 +106,7 @@ mem_rte_to_sys_prot(int prot)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, off_t offset)
{
int sys_flags = 0;
int sys_prot;
diff --git a/lib/eal/windows/eal_memory.c b/lib/eal/windows/eal_memory.c
index 2cf5a5e649..f1c4b03e96 100644
--- a/lib/eal/windows/eal_memory.c
+++ b/lib/eal/windows/eal_memory.c
@@ -508,7 +508,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
void *
rte_mem_map(void *requested_addr, size_t size, int prot, int flags,
- int fd, size_t offset)
+ int fd, off_t offset)
{
HANDLE file_handle = INVALID_HANDLE_VALUE;
HANDLE mapping_handle = INVALID_HANDLE_VALUE;
--
2.25.1
Re: [dpdk-dev] [PATCH v4] eal: fix memory mapping for 32-bit targets
> v4: Identical to v1, with now included in rte_eal_paging.h > to (hopefully) make off_t available for Windows builds. With this version, using off_t is no longer a problem, however based on the new compilation error it appears that for Windows, off_t is a 32-bit type while size_t is a 64-bit type: ../lib/eal/windows/eal_memory.c:519:37: error: shift count >= width of type [-Werror,-Wshift-count-overflow] DWORD offset_high = (DWORD)(offset >> 32); ^ ~~ So the options seem to be to either use uint64_t for the offset parameter as in patch v3, or else introduce something like: typedef uint64_t rte_off_t; Thoughts/opinions?
[dpdk-dev] [PATCH 0/3] net/bnxt: vector mode patches
Vector mode updates for the bnxt PMD. Lance Richardson (3): net/bnxt: refactor HW ptype mapping table net/bnxt: fix Rx burst size constraint net/bnxt: add AVX2 vector PMD doc/guides/nics/bnxt.rst | 57 ++- drivers/net/bnxt/bnxt_ethdev.c| 119 +++-- drivers/net/bnxt/bnxt_rxr.c | 38 +- drivers/net/bnxt/bnxt_rxr.h | 54 ++- drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 597 ++ drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 73 +++- drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 78 ++-- drivers/net/bnxt/bnxt_txr.h | 7 + drivers/net/bnxt/meson.build | 17 + 9 files changed, 911 insertions(+), 129 deletions(-) create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_avx2.c -- 2.25.1
[dpdk-dev] [PATCH 3/3] net/bnxt: add AVX2 vector PMD
Implement AVX2 vector PMD. Signed-off-by: Lance Richardson Reviewed-by: Ajit Khaparde --- doc/guides/nics/bnxt.rst | 57 ++- drivers/net/bnxt/bnxt_ethdev.c| 119 +++-- drivers/net/bnxt/bnxt_rxr.c | 4 +- drivers/net/bnxt/bnxt_rxr.h | 11 +- drivers/net/bnxt/bnxt_rxtx_vec_avx2.c | 597 ++ drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 25 +- drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 31 +- drivers/net/bnxt/bnxt_txr.h | 7 + drivers/net/bnxt/meson.build | 17 + 9 files changed, 780 insertions(+), 88 deletions(-) create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_avx2.c diff --git a/doc/guides/nics/bnxt.rst b/doc/guides/nics/bnxt.rst index 0fb2032447..feb0c6a765 100644 --- a/doc/guides/nics/bnxt.rst +++ b/doc/guides/nics/bnxt.rst @@ -853,23 +853,36 @@ DPDK implements a light-weight library to allow PMDs to be bonded together and p Vector Processing - +The BNXT PMD provides vectorized burst transmit/receive function implementations +on x86-based platforms using SSE (Streaming SIMD Extensions) and AVX2 (Advanced +Vector Extensions 2) instructions, and on Arm-based platforms using Arm Neon +Advanced SIMD instructions. Vector processing support is currently implemented +only for Intel/AMD and Arm CPU architectures. + Vector processing provides significantly improved performance over scalar -processing (see Vector Processor, here). +processing. This improved performance is derived from a number of optimizations: + +* Using SIMD instructions to operate on multiple packets in parallel. +* Using SIMD instructions to do more work per instruction than is possible + with scalar instructions, for example by leveraging 128-bit and 256-bi + load/store instructions or by using SIMD shuffle and permute operations. +* Batching -The BNXT PMD supports the vector processing using SSE (Streaming SIMD -Extensions) instructions on x86 platforms. It also supports NEON intrinsics for -vector processing on ARM CPUs. The BNXT vPMD (vector mode PMD) is available for -Intel/AMD and ARM CPU architectures. + * TX: transmit completions are processed in bulk. + * RX: bulk allocation of mbufs is used when allocating rxq buffers. -This improved performance comes from several optimizations: +* Simplifications enabled by not supporting chained mbufs in vector mode. +* Simplifications enabled by not supporting some stateless offloads in vector + mode: -* Batching - * TX: processing completions in bulk - * RX: allocating mbufs in bulk -* Chained mbufs are *not* supported, i.e. a packet should fit a single mbuf -* Some stateless offloads are *not* supported with vector processing - * TX: no offloads will be supported - * RX: reduced RX offloads (listed below) will be supported:: + * TX: only the following reduced set of transmit offloads is supported in + vector mode:: + + DEV_TX_OFFLOAD_MBUF_FAST_FREE + + * RX: only the following reduced set of receive offloads is supported in + vector mode (note that jumbo MTU is allowed only when the MTU setting + does not require `DEV_RX_OFFLOAD_SCATTER` to be enabled):: DEV_RX_OFFLOAD_VLAN_STRIP DEV_RX_OFFLOAD_KEEP_CRC @@ -878,23 +891,21 @@ This improved performance comes from several optimizations: DEV_RX_OFFLOAD_UDP_CKSUM DEV_RX_OFFLOAD_TCP_CKSUM DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM + DEV_RX_OFFLOAD_OUTER_UDP_CKSUM DEV_RX_OFFLOAD_RSS_HASH DEV_RX_OFFLOAD_VLAN_FILTER -The BNXT Vector PMD is enabled in DPDK builds by default. - -However, a decision to enable vector mode will be made when the port transitions -from stopped to started. Any TX offloads or some RX offloads (other than listed -above) will disable the vector mode. -Offload configuration changes that impact vector mode must be made when the port -is stopped. +The BNXT Vector PMD is enabled in DPDK builds by default. The decision to enable +vector processing is made at run-time when the port is started; if no transmit +offloads outside the set supported for vector mode are enabled then vector mode +transmit will be enabled, and if no receive offloads outside the set supported +for vector mode are enabled then vector mode receive will be enabled. Offload +configuration changes that impact the decision to enable vector mode are allowed +only when the port is stopped. Note that TX (or RX) vector mode can be enabled independently from RX (or TX) vector mode. -Also vector mode is allowed when jumbo is enabled -as long as the MTU setting does not require scattered Rx. - Appendix diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 0208795fd2..a7d056a34f 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -1178,32 +1178,57 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev) return bnxt_recv_pkts; } -#if defined
[dpdk-dev] [PATCH 2/3] net/bnxt: fix Rx burst size constraint
The burst receive function should return all packets currently
present in the receive ring up to the requested burst size,
update vector mode receive functions accordingly.
Fixes: 398358341419 ("net/bnxt: support NEON")
Fixes: bc4a000f2f53 ("net/bnxt: implement SSE vector mode")
Cc: sta...@dpdk.org
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 29 +--
drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 29 +--
2 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
b/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
index a6fbc0b0bf..a6e630ea5e 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_neon.c
@@ -158,9 +158,8 @@ descs_to_mbufs(uint32x4_t mm_rxcmp[4], uint32x4_t
mm_rxcmp1[4],
vst1q_u32((uint32_t *)&mbuf[3]->rx_descriptor_fields1, tmp);
}
-uint16_t
-bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
- uint16_t nb_pkts)
+static uint16_t
+recv_burst_vec_neon(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t
nb_pkts)
{
struct bnxt_rx_queue *rxq = rx_queue;
struct bnxt_cp_ring_info *cpr = rxq->cp_ring;
@@ -185,9 +184,6 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
if (rxq->rxrearm_nb >= rxq->rx_free_thresh)
bnxt_rxq_rearm(rxq, rxr);
- /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
- nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
-
cons = raw_cons & (cp_ring_size - 1);
mbcons = (raw_cons / 2) & (rx_ring_size - 1);
@@ -305,6 +301,27 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
return nb_rx_pkts;
}
+uint16_t
+bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
+{
+ uint16_t cnt = 0;
+
+ while (nb_pkts > RTE_BNXT_MAX_RX_BURST) {
+ uint16_t burst;
+
+ burst = recv_burst_vec_neon(rx_queue, rx_pkts + cnt,
+ RTE_BNXT_MAX_RX_BURST);
+
+ cnt += burst;
+ nb_pkts -= burst;
+
+ if (burst < RTE_BNXT_MAX_RX_BURST)
+ return cnt;
+ }
+
+ return cnt + recv_burst_vec_neon(rx_queue, rx_pkts + cnt, nb_pkts);
+}
+
static void
bnxt_handle_tx_cp_vec(struct bnxt_tx_queue *txq)
{
diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index 6dd18a0077..fe074f82cf 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -149,9 +149,8 @@ descs_to_mbufs(__m128i mm_rxcmp[4], __m128i mm_rxcmp1[4],
_mm_store_si128((void *)&mbuf[3]->rx_descriptor_fields1, t0);
}
-uint16_t
-bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
- uint16_t nb_pkts)
+static uint16_t
+recv_burst_vec_sse(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
{
struct bnxt_rx_queue *rxq = rx_queue;
const __m128i mbuf_init = _mm_set_epi64x(0, rxq->mbuf_initializer);
@@ -176,9 +175,6 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
if (rxq->rxrearm_nb >= rxq->rx_free_thresh)
bnxt_rxq_rearm(rxq, rxr);
- /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
- nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
-
cons = raw_cons & (cp_ring_size - 1);
mbcons = (raw_cons / 2) & (rx_ring_size - 1);
@@ -286,6 +282,27 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
return nb_rx_pkts;
}
+uint16_t
+bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
+{
+ uint16_t cnt = 0;
+
+ while (nb_pkts > RTE_BNXT_MAX_RX_BURST) {
+ uint16_t burst;
+
+ burst = recv_burst_vec_sse(rx_queue, rx_pkts + cnt,
+ RTE_BNXT_MAX_RX_BURST);
+
+ cnt += burst;
+ nb_pkts -= burst;
+
+ if (burst < RTE_BNXT_MAX_RX_BURST)
+ return cnt;
+ }
+
+ return cnt + recv_burst_vec_sse(rx_queue, rx_pkts + cnt, nb_pkts);
+}
+
static void
bnxt_handle_tx_cp_vec(struct bnxt_tx_queue *txq)
{
--
2.25.1
[dpdk-dev] [PATCH 1/3] net/bnxt: refactor HW ptype mapping table
Make the definition of the table used to map hardware packet type
information to DPDK packet type more generic.
Add macro definitions for constants used in creating table
indices, use these to eliminate raw constants in code.
Add build-time assertions to validate ptype mapping constants.
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_rxr.c | 34 +++--
drivers/net/bnxt/bnxt_rxr.h | 43 ++-
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 19
drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 18 +++
4 files changed, 85 insertions(+), 29 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 2ef4115ef9..a6a8fb213b 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -396,14 +396,14 @@ bnxt_init_ptype_table(void)
return;
for (i = 0; i < BNXT_PTYPE_TBL_DIM; i++) {
- if (i & (RX_PKT_CMPL_FLAGS2_META_FORMAT_VLAN >> 2))
+ if (i & BNXT_PTYPE_TBL_VLAN_MSK)
pt[i] = RTE_PTYPE_L2_ETHER_VLAN;
else
pt[i] = RTE_PTYPE_L2_ETHER;
- ip6 = i & (RX_PKT_CMPL_FLAGS2_IP_TYPE >> 7);
- tun = i & (RX_PKT_CMPL_FLAGS2_T_IP_CS_CALC >> 2);
- type = (i & 0x78) << 9;
+ ip6 = !!(i & BNXT_PTYPE_TBL_IP_VER_MSK);
+ tun = !!(i & BNXT_PTYPE_TBL_TUN_MSK);
+ type = (i & BNXT_PTYPE_TBL_TYPE_MSK) >> BNXT_PTYPE_TBL_TYPE_SFT;
if (!tun && !ip6)
l3 = RTE_PTYPE_L3_IPV4_EXT_UNKNOWN;
@@ -415,25 +415,25 @@ bnxt_init_ptype_table(void)
l3 = RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN;
switch (type) {
- case RX_PKT_CMPL_FLAGS_ITYPE_ICMP:
+ case BNXT_PTYPE_TBL_TYPE_ICMP:
if (tun)
pt[i] |= l3 | RTE_PTYPE_INNER_L4_ICMP;
else
pt[i] |= l3 | RTE_PTYPE_L4_ICMP;
break;
- case RX_PKT_CMPL_FLAGS_ITYPE_TCP:
+ case BNXT_PTYPE_TBL_TYPE_TCP:
if (tun)
pt[i] |= l3 | RTE_PTYPE_INNER_L4_TCP;
else
pt[i] |= l3 | RTE_PTYPE_L4_TCP;
break;
- case RX_PKT_CMPL_FLAGS_ITYPE_UDP:
+ case BNXT_PTYPE_TBL_TYPE_UDP:
if (tun)
pt[i] |= l3 | RTE_PTYPE_INNER_L4_UDP;
else
pt[i] |= l3 | RTE_PTYPE_L4_UDP;
break;
- case RX_PKT_CMPL_FLAGS_ITYPE_IP:
+ case BNXT_PTYPE_TBL_TYPE_IP:
pt[i] |= l3;
break;
}
@@ -450,17 +450,19 @@ bnxt_parse_pkt_type(struct rx_pkt_cmpl *rxcmp, struct
rx_pkt_cmpl_hi *rxcmp1)
flags_type = rte_le_to_cpu_16(rxcmp->flags_type);
flags2 = rte_le_to_cpu_32(rxcmp1->flags2);
+ /* Validate ptype table indexing at build time. */
+ bnxt_check_ptype_constants();
+
/*
* Index format:
-* bit 0: RX_PKT_CMPL_FLAGS2_T_IP_CS_CALC
-* bit 1: RX_CMPL_FLAGS2_IP_TYPE
-* bit 2: RX_PKT_CMPL_FLAGS2_META_FORMAT_VLAN
-* bits 3-6: RX_PKT_CMPL_FLAGS_ITYPE
+* bit 0: Set if IP tunnel encapsulated packet.
+* bit 1: Set if IPv6 packet, clear if IPv4.
+* bit 2: Set if VLAN tag present.
+* bits 3-6: Four-bit hardware packet type field.
*/
- index = ((flags_type & RX_PKT_CMPL_FLAGS_ITYPE_MASK) >> 9) |
- ((flags2 & (RX_PKT_CMPL_FLAGS2_META_FORMAT_VLAN |
- RX_PKT_CMPL_FLAGS2_T_IP_CS_CALC)) >> 2) |
- ((flags2 & RX_PKT_CMPL_FLAGS2_IP_TYPE) >> 7);
+ index = BNXT_CMPL_ITYPE_TO_IDX(flags_type) |
+ BNXT_CMPL_VLAN_TUN_TO_IDX(flags2) |
+ BNXT_CMPL_IP_VER_TO_IDX(flags2);
return bnxt_ptype_table[index];
}
diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
index b43256e03e..79f1458698 100644
--- a/drivers/net/bnxt/bnxt_rxr.h
+++ b/drivers/net/bnxt/bnxt_rxr.h
@@ -131,7 +131,48 @@ bnxt_cfa_code_dynfield(struct rte_mbuf *mbuf)
#define BNXT_CFA_META_EEM_TCAM_SHIFT 31
#define BNXT_CFA_META_EM_TEST(x) ((x) >> BNXT_CFA_META_EEM_TCAM_SHIFT)
-#define BNXT_PTYPE_TBL_DIM 128
+/* Definitions for translation of hardware packet type to mbuf ptype. */
+#define BNXT_PTYPE_TBL_DIM 128
+#define BNXT_PTYPE_TBL_TUN_SFT 0 /* Set if tunneled packet. */
+#define BNXT_PTYPE_TBL_TUN_MSK
Re: [dpdk-dev] [PATCH 3/3] net/bnxt: add AVX2 vector PMD
On Mon, May 24, 2021 at 3:00 PM Lance Richardson wrote: > > Implement AVX2 vector PMD. > There are CI test failures for this patch series that appear be unrelated, are these known/expected failures? >From http://mails.dpdk.org/archives/test-report/2021-May/196470.html Ubuntu 18.04 ARM Kernel: 4.15.0-132-generic Compiler: gcc 7.5 NIC: Arm Intel Corporation Ethernet Converged Network Adapter XL710-QDA2 4 Mbps Target: x86_64-native-linuxapp-gcc Fail/Total: 4/5 Failed Tests: - dynamic_config - mtu_update - scatter - stats_checks >From http://mails.dpdk.org/archives/test-report/2021-May/196343.html 20 line log output for Ubuntu 18.04 ARM (dpdk_unit_test): Summary of Failures: 3/96 DPDK:fast-tests / atomic_autotestFAIL 22.96s (killed by signal 9 SIGKILL) 29/96 DPDK:fast-tests / func_reentrancy_autotest FAIL 2.82s (exit status 255 or signal 127 SIGinvalid) 38/96 DPDK:fast-tests / malloc_autotestFAIL 33.25s (killed by signal 9 SIGKILL) 48/96 DPDK:fast-tests / pflock_autotestFAIL 6.14s (killed by signal 9 SIGKILL)
Re: [dpdk-dev] [PATCH] net/bnxt: add support for flow mark action
On Fri, Dec 13, 2019 at 5:42 PM Ajit Khaparde
wrote:
>
> Add support for RTE_FLOW_ACTION_TYPE_MARK
> Signed-off-by: Ajit Khaparde
> ---
> diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> index 22d9f9e84..ce00da633 100644
> --- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> +++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> @@ -280,8 +280,14 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>
> rte_compiler_barrier();
>
> - if (rxcmp->flags_type & RX_PKT_CMPL_FLAGS_RSS_VALID)
> + if (rxcmp->flags_type & RX_PKT_CMPL_FLAGS_RSS_VALID) {
> 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;
> + }
>
> if (rxcmp1->flags2 &
> RX_PKT_CMPL_FLAGS2_META_FORMAT_VLAN) {
> --
> 2.21.0 (Apple Git-122.2)
>
Hi Ajit,
IMO this functionality should not be supported in vector mode.
Regards,
Lance
[dpdk-dev] coding style question - c99 comments
The DPDK Coding Style document doesn't mention the use of c99 comments using '//' as a comment delimiter, but it does say that traditional '/* */' comment delimiters should be used and therefore seems to imply that c99-style comments are not allowed. Is this correct? Lance
Re: [dpdk-dev] coding style question - c99 comments
On Thu, Jan 23, 2020 at 11:15 AM Stephen Hemminger wrote: > > On Thu, 23 Jan 2020 11:05:58 -0500 > Lance Richardson wrote: > > > The DPDK Coding Style document doesn't mention the use of c99 comments > > using '//' as a comment delimiter, but it does say that traditional > > '/* */' comment > > delimiters should be used and therefore seems to imply that c99-style > > comments > > are not allowed. > > > > Is this correct? > > > >Lance > > Don't use c99 comments. > These will get flagged by checkpatch Hi Stephen, Actually, the reason I was asking was they aren't being flagged by checkpatch. It seems checkpatch.pl now has this: my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE If c99 comments are should not be allowed, it looks like C99_COMMENT_TOLERANCE should be added to the list of checkers to ignore in checkpatches.sh. I was thinking of rolling a patch, but wanted to first confirm that they are actually not allowed. Thanks, Lance
[dpdk-dev] [PATCH] devtools: check c99 comment use in checkpatches.sh
C99-style comments are not permitted according to DPDk coding
style guidelines, enable checking for these by checkpatch.pl.
Signed-off-by: Lance Richardson
---
devtools/checkpatches.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index b16bace92..c996a8780 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
SPLIT_STRING,LONG_LINE_STRING,\
LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
-NEW_TYPEDEFS,COMPARISON_TO_NULL"
+NEW_TYPEDEFS,COMPARISON_TO_NULL,C99_COMMENT_TOLERANCE"
options="$options $DPDK_CHECKPATCH_OPTIONS"
print_usage () {
--
2.17.1
Re: [dpdk-dev] coding style question - c99 comments
Thanks, a patch adding C99_COMMENT_TOLERANCE has been posted. Lance On Fri, Jan 24, 2020 at 12:10 PM Ferruh Yigit wrote: > On 1/23/2020 4:43 PM, Lance Richardson wrote: > > On Thu, Jan 23, 2020 at 11:15 AM Stephen Hemminger > > wrote: > >> > >> On Thu, 23 Jan 2020 11:05:58 -0500 > >> Lance Richardson wrote: > >> > >>> The DPDK Coding Style document doesn't mention the use of c99 comments > >>> using '//' as a comment delimiter, but it does say that traditional > >>> '/* */' comment > >>> delimiters should be used and therefore seems to imply that c99-style > comments > >>> are not allowed. > >>> > >>> Is this correct? > >>> > >>>Lance > >> > >> Don't use c99 comments. > >> These will get flagged by checkpatch > > > > Hi Stephen, > > > > Actually, the reason I was asking was they aren't being flagged by > checkpatch. > > > > It seems checkpatch.pl now has this: > > my $allow_c99_comments = 1; # Can be overridden by --ignore > > C99_COMMENT_TOLERANCE > > > > If c99 comments are should not be allowed, it looks like > > C99_COMMENT_TOLERANCE should > > be added to the list of checkers to ignore in checkpatches.sh. I was > > thinking of rolling a patch, > > but wanted to first confirm that they are actually not allowed. > > > > Hi Lance, > > That is what I understand from coding style document that c99 comments are > not > allowed and agree to not use them. > > +1 to add C99_COMMENT_TOLERANCE > >
Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
On Thu, Apr 30, 2020 at 9:37 AM Yuan Linsi wrote:
>
> From: yuanlinsi01
>
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
>
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
>
> Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> Cc: sta...@dpdk.org
>
> Signed-off-by: yuanlinsi01
> Signed-off-by: rongdongsheng
> ---
> drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> index d0e7910e7..8f73add9b 100644
> --- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> +++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> @@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
> **rx_pkts,
> /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
> nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
>
> - /* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
> + /*
> +* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP.
> +* nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
> +*/
> nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
> + if (!nb_pkts)
> + return 0;
>
> /* Handle RX burst request */
> while (1) {
> --
> 2.11.0
>
Thanks for the fix!
Acked-by: Lance Richardson
[dpdk-dev] [PATCH v2] devtools: check c99 comment use in checkpatches.sh
C99-style comments are not permitted according to DPDK coding
style guidelines, enable checking for these by checkpatch.pl.
Signed-off-by: Lance Richardson
---
v2:
* Fixed a typo in commit message.
devtools/checkpatches.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index b16bace92..c996a8780 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\
SPLIT_STRING,LONG_LINE_STRING,\
LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
-NEW_TYPEDEFS,COMPARISON_TO_NULL"
+NEW_TYPEDEFS,COMPARISON_TO_NULL,C99_COMMENT_TOLERANCE"
options="$options $DPDK_CHECKPATCH_OPTIONS"
print_usage () {
--
2.17.1
[dpdk-dev] [PATCH 0/3] net/bnxt: vector mode enhancements
- Switch to non-vector receive function when "mark" action is requested. - Add support for rx_burst_mode_get()/tx_burst_mode_get() to return currently active receive/transmit mode. - Add vector mode support for ARM64. Lance Richardson (3): net/bnxt: disable vector receive for mark action net/bnxt: support Rx/Tx burst mode info net/bnxt: add ARM64 vector support doc/guides/nics/features/bnxt.ini | 1 + drivers/net/bnxt/Makefile | 3 + drivers/net/bnxt/bnxt_ethdev.c | 52 ++- drivers/net/bnxt/bnxt_flow.c| 20 +- drivers/net/bnxt/bnxt_ring.c| 4 +- drivers/net/bnxt/bnxt_rxq.h | 2 +- drivers/net/bnxt/bnxt_rxr.c | 18 + drivers/net/bnxt/bnxt_rxr.h | 2 +- drivers/net/bnxt/bnxt_rxtx_vec_common.h | 57 +++ drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 473 drivers/net/bnxt/bnxt_rxtx_vec_sse.c| 46 +-- drivers/net/bnxt/bnxt_txr.h | 2 +- drivers/net/bnxt/meson.build| 2 + 13 files changed, 618 insertions(+), 64 deletions(-) create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_common.h create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_neon.c -- 2.25.1
[dpdk-dev] [PATCH 2/3] net/bnxt: support Rx/Tx burst mode info
Retrieve burst mode options according to the selected Rx/Tx burst
function name.
Signed-off-by: Lance Richardson
Reviewed-by: Somnath Kotur
Reviewed-by: Ajit Kumar Khaparde
---
doc/guides/nics/features/bnxt.ini | 1 +
drivers/net/bnxt/bnxt_ethdev.c| 46 +++
2 files changed, 47 insertions(+)
diff --git a/doc/guides/nics/features/bnxt.ini
b/doc/guides/nics/features/bnxt.ini
index 37a99e336..f1f300c8b 100644
--- a/doc/guides/nics/features/bnxt.ini
+++ b/doc/guides/nics/features/bnxt.ini
@@ -9,6 +9,7 @@ Link status = Y
Link status event= Y
Rx interrupt = Y
Queue start/stop = Y
+Burst mode info = Y
MTU update = Y
Jumbo frame = Y
Scattered Rx = Y
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index e8b4c058a..edb650bbc 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2582,6 +2582,50 @@ bnxt_txq_info_get_op(struct rte_eth_dev *dev, uint16_t
queue_id,
qinfo->conf.tx_deferred_start = txq->tx_deferred_start;
}
+static int
+bnxt_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
+ struct rte_eth_burst_mode *mode)
+{
+ eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
+
+ if (pkt_burst == bnxt_recv_pkts) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Scalar");
+ return 0;
+ }
+#ifdef RTE_ARCH_X86
+ if (pkt_burst == bnxt_recv_pkts_vec) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Vector SSE");
+ return 0;
+ }
+#endif
+
+ return -EINVAL;
+}
+
+static int
+bnxt_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
+ struct rte_eth_burst_mode *mode)
+{
+ eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
+
+ if (pkt_burst == bnxt_xmit_pkts) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Scalar");
+ return 0;
+ }
+#ifdef RTE_ARCH_X86
+ if (pkt_burst == bnxt_xmit_pkts_vec) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Vector SSE");
+ return 0;
+ }
+#endif
+
+ return -EINVAL;
+}
+
int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -4158,6 +4202,8 @@ static const struct eth_dev_ops bnxt_dev_ops = {
.set_mc_addr_list = bnxt_dev_set_mc_addr_list_op,
.rxq_info_get = bnxt_rxq_info_get_op,
.txq_info_get = bnxt_txq_info_get_op,
+ .rx_burst_mode_get = bnxt_rx_burst_mode_get,
+ .tx_burst_mode_get = bnxt_tx_burst_mode_get,
.dev_led_on = bnxt_dev_led_on_op,
.dev_led_off = bnxt_dev_led_off_op,
.xstats_get_by_id = bnxt_dev_xstats_get_by_id_op,
--
2.25.1
[dpdk-dev] [PATCH 1/3] net/bnxt: disable vector receive for mark action
The bnxt vector mode receive handler does not support the rte_flow
'mark' action. Since we cannot know in advance whether this action
will be required, add support for dynamically switching from vector
to non-vector receive when the first flow create request with a
mark action is processed.
Fixes: 94eb699bc82e ("net/bnxt: support flow mark action")
Suggested-by: Thomas Monjalon
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_flow.c | 20
drivers/net/bnxt/bnxt_rxr.c | 18 ++
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 84a21dba9..19bc66a5a 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -18,6 +18,7 @@
#include "bnxt_hwrm.h"
#include "bnxt_ring.h"
#include "bnxt_rxq.h"
+#include "bnxt_rxr.h"
#include "bnxt_vnic.h"
#include "hsi_struct_def_dpdk.h"
@@ -1403,18 +1404,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
break;
case RTE_FLOW_ACTION_TYPE_MARK:
- if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
- PMD_DRV_LOG(DEBUG,
- "Disable vector processing for mark\n");
- rte_flow_error_set(error,
- ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act,
- "Disable vector processing for
mark");
- rc = -rte_errno;
- goto ret;
- }
-
if (bp->mark_table == NULL) {
rte_flow_error_set(error,
ENOMEM,
@@ -1425,6 +1414,13 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
goto ret;
}
+ if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
+ PMD_DRV_LOG(DEBUG,
+ "Disabling vector processing for mark\n");
+ bp->eth_dev->rx_pkt_burst = bnxt_recv_pkts;
+ bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
+ }
+
filter->valid_flags |= BNXT_FLOW_MARK_FLAG;
filter->mark = ((const struct rte_flow_action_mark *)
act->conf)->id;
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 91ff72904..eeb0aeefc 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -747,6 +747,24 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts,
!rte_spinlock_trylock(&rxq->lock)))
return 0;
+#if defined(RTE_ARCH_X86)
+ /*
+* Replenish buffers if needed when a transition has been made from
+* vector- to non-vector- receive processing.
+*/
+ while (unlikely(rxq->rxrearm_nb)) {
+ if (!bnxt_alloc_rx_data(rxq, rxr, rxq->rxrearm_start)) {
+ rxr->rx_prod = rxq->rxrearm_start;
+ bnxt_db_write(&rxr->rx_db, rxr->rx_prod);
+ rxq->rxrearm_start++;
+ rxq->rxrearm_nb--;
+ } else {
+ /* Retry allocation on next call. */
+ break;
+ }
+ }
+#endif
+
/* Handle RX burst request */
while (1) {
cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
--
2.25.1
[dpdk-dev] [PATCH 3/3] net/bnxt: add ARM64 vector support
Add bnxt vector PMD support using NEON SIMD instructions.
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/Makefile | 3 +
drivers/net/bnxt/bnxt_ethdev.c | 10 +-
drivers/net/bnxt/bnxt_ring.c| 4 +-
drivers/net/bnxt/bnxt_rxq.h | 2 +-
drivers/net/bnxt/bnxt_rxr.c | 2 +-
drivers/net/bnxt/bnxt_rxr.h | 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_common.h | 57 +++
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 473
drivers/net/bnxt/bnxt_rxtx_vec_sse.c| 46 +--
drivers/net/bnxt/bnxt_txr.h | 2 +-
drivers/net/bnxt/meson.build| 2 +
11 files changed, 548 insertions(+), 55 deletions(-)
create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_common.h
create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_neon.c
diff --git a/drivers/net/bnxt/Makefile b/drivers/net/bnxt/Makefile
index 2a39ed139..98be7b28e 100644
--- a/drivers/net/bnxt/Makefile
+++ b/drivers/net/bnxt/Makefile
@@ -42,6 +42,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += rte_pmd_bnxt.c
ifeq ($(CONFIG_RTE_ARCH_X86), y)
SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_rxtx_vec_sse.c
endif
+ifeq ($(CONFIG_RTE_ARCH_ARM64), y)
+SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_rxtx_vec_neon.c
+endif
ifeq ($(CONFIG_RTE_LIBRTE_BNXT_PMD), y)
CFLAGS += -I$(SRCDIR) -I$(SRCDIR)/tf_core -I$(SRCDIR)/tf_ulp
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index edb650bbc..10d70ab0b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1075,7 +1075,7 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
#ifndef RTE_LIBRTE_IEEE1588
/*
* Vector mode receive can be enabled only if scatter rx is not
@@ -1115,7 +1115,7 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev)
static eth_tx_burst_t
bnxt_transmit_function(__rte_unused struct rte_eth_dev *eth_dev)
{
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
#ifndef RTE_LIBRTE_IEEE1588
/*
* Vector mode transmit can be enabled only if not using scatter rx
@@ -2593,7 +2593,7 @@ bnxt_rx_burst_mode_get(struct rte_eth_dev *dev,
__rte_unused uint16_t queue_id,
"Scalar");
return 0;
}
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
if (pkt_burst == bnxt_recv_pkts_vec) {
snprintf(mode->info, sizeof(mode->info), "%s",
"Vector SSE");
@@ -2615,7 +2615,7 @@ bnxt_tx_burst_mode_get(struct rte_eth_dev *dev,
__rte_unused uint16_t queue_id,
"Scalar");
return 0;
}
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
if (pkt_burst == bnxt_xmit_pkts_vec) {
snprintf(mode->info, sizeof(mode->info), "%s",
"Vector SSE");
@@ -2644,7 +2644,7 @@ int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t
new_mtu)
new_pkt_size = new_mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN +
VLAN_TAG_SIZE * BNXT_NUM_VLANS;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
/*
* If vector-mode tx/rx is active, disallow any MTU change that would
* require scattered receive support.
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 24a947f27..54f654744 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -608,7 +608,7 @@ int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int
queue_index)
bnxt_db_write(&rxr->ag_db, rxr->ag_prod);
}
rxq->index = queue_index;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
bnxt_rxq_vec_setup(rxq);
#endif
@@ -713,7 +713,7 @@ int bnxt_alloc_hwrm_rings(struct bnxt *bp)
bnxt_db_write(&rxr->rx_db, rxr->rx_prod);
bnxt_db_write(&rxr->ag_db, rxr->ag_prod);
rxq->index = i;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
bnxt_rxq_vec_setup(rxq);
#endif
}
diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h
index 4f5182d9e..d5ce3b6d5 100644
--- a/drivers/net/bnxt/bnxt_rxq.h
+++ b/drivers/net/bnxt/bnxt_rxq.h
@@ -22,7 +22,7 @@ struct bnxt_rx_queue {
uint16_tnb_rx_hold; /* num held free RX desc */
uint16_trx_free_thresh; /* max free RX desc to hold */
uint16_tqueue_id; /* RX queue index */
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
uint16_trxrear
Re: [dpdk-dev] [PATCH 3/3] net/bnxt: add ARM64 vector support
On Tue, Jun 16, 2020 at 1:18 AM Ruifeng Wang wrote: > > > +#if defined(RTE_ARCH_ARM64) > > +#include > > rte_vect.h includes compatibility improvement. It can replace arm_neon.h here. > > > +#else > > +#error "bnxt vector pmd: unsupported target." > > +#endif > > + Thanks, Ruifeng, I've addressed this in v2. Lance
[dpdk-dev] [PATCH v2 0/3] net/bnxt: vector mode enhancements
- Switch to non-vector receive function when "mark" action is requested. - Add support for rx_burst_mode_get()/tx_burst_mode_get() to return currently active receive/transmit mode. - Add vector mode support for ARM64. Lance Richardson (3): net/bnxt: disable vector receive for mark action net/bnxt: support Rx/Tx burst mode info net/bnxt: add ARM64 vector support doc/guides/nics/features/bnxt.ini | 1 + drivers/net/bnxt/Makefile | 3 + drivers/net/bnxt/bnxt_ethdev.c | 52 ++- drivers/net/bnxt/bnxt_flow.c| 20 +- drivers/net/bnxt/bnxt_ring.c| 4 +- drivers/net/bnxt/bnxt_rxq.h | 2 +- drivers/net/bnxt/bnxt_rxr.c | 18 + drivers/net/bnxt/bnxt_rxr.h | 2 +- drivers/net/bnxt/bnxt_rxtx_vec_common.h | 57 +++ drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 469 drivers/net/bnxt/bnxt_rxtx_vec_sse.c| 46 +-- drivers/net/bnxt/bnxt_txr.h | 2 +- drivers/net/bnxt/meson.build| 2 + 13 files changed, 614 insertions(+), 64 deletions(-) create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_common.h create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_neon.c -- 2.25.1
[dpdk-dev] [PATCH 1/3] net/bnxt: disable vector receive for mark action
The bnxt vector mode receive handler does not support the rte_flow
'mark' action. Since we cannot know in advance whether this action
will be required, add support for dynamically switching from vector
to non-vector receive when the first flow create request with a
mark action is processed.
Fixes: 94eb699bc82e ("net/bnxt: support flow mark action")
Suggested-by: Thomas Monjalon
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
v2: no change
drivers/net/bnxt/bnxt_flow.c | 20
drivers/net/bnxt/bnxt_rxr.c | 18 ++
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 84a21dba9..19bc66a5a 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -18,6 +18,7 @@
#include "bnxt_hwrm.h"
#include "bnxt_ring.h"
#include "bnxt_rxq.h"
+#include "bnxt_rxr.h"
#include "bnxt_vnic.h"
#include "hsi_struct_def_dpdk.h"
@@ -1403,18 +1404,6 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
break;
case RTE_FLOW_ACTION_TYPE_MARK:
- if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
- PMD_DRV_LOG(DEBUG,
- "Disable vector processing for mark\n");
- rte_flow_error_set(error,
- ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ACTION,
- act,
- "Disable vector processing for
mark");
- rc = -rte_errno;
- goto ret;
- }
-
if (bp->mark_table == NULL) {
rte_flow_error_set(error,
ENOMEM,
@@ -1425,6 +1414,13 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
goto ret;
}
+ if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
+ PMD_DRV_LOG(DEBUG,
+ "Disabling vector processing for mark\n");
+ bp->eth_dev->rx_pkt_burst = bnxt_recv_pkts;
+ bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
+ }
+
filter->valid_flags |= BNXT_FLOW_MARK_FLAG;
filter->mark = ((const struct rte_flow_action_mark *)
act->conf)->id;
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 91ff72904..eeb0aeefc 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -747,6 +747,24 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts,
!rte_spinlock_trylock(&rxq->lock)))
return 0;
+#if defined(RTE_ARCH_X86)
+ /*
+* Replenish buffers if needed when a transition has been made from
+* vector- to non-vector- receive processing.
+*/
+ while (unlikely(rxq->rxrearm_nb)) {
+ if (!bnxt_alloc_rx_data(rxq, rxr, rxq->rxrearm_start)) {
+ rxr->rx_prod = rxq->rxrearm_start;
+ bnxt_db_write(&rxr->rx_db, rxr->rx_prod);
+ rxq->rxrearm_start++;
+ rxq->rxrearm_nb--;
+ } else {
+ /* Retry allocation on next call. */
+ break;
+ }
+ }
+#endif
+
/* Handle RX burst request */
while (1) {
cons = RING_CMP(cpr->cp_ring_struct, raw_cons);
--
2.25.1
[dpdk-dev] [PATCH 2/3] net/bnxt: support Rx/Tx burst mode info
Retrieve burst mode options according to the selected Rx/Tx burst
function name.
Signed-off-by: Lance Richardson
Reviewed-by: Somnath Kotur
Reviewed-by: Ajit Kumar Khaparde
---
v2: no change
doc/guides/nics/features/bnxt.ini | 1 +
drivers/net/bnxt/bnxt_ethdev.c| 46 +++
2 files changed, 47 insertions(+)
diff --git a/doc/guides/nics/features/bnxt.ini
b/doc/guides/nics/features/bnxt.ini
index 37a99e336..f1f300c8b 100644
--- a/doc/guides/nics/features/bnxt.ini
+++ b/doc/guides/nics/features/bnxt.ini
@@ -9,6 +9,7 @@ Link status = Y
Link status event= Y
Rx interrupt = Y
Queue start/stop = Y
+Burst mode info = Y
MTU update = Y
Jumbo frame = Y
Scattered Rx = Y
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index e8b4c058a..edb650bbc 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2582,6 +2582,50 @@ bnxt_txq_info_get_op(struct rte_eth_dev *dev, uint16_t
queue_id,
qinfo->conf.tx_deferred_start = txq->tx_deferred_start;
}
+static int
+bnxt_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
+ struct rte_eth_burst_mode *mode)
+{
+ eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
+
+ if (pkt_burst == bnxt_recv_pkts) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Scalar");
+ return 0;
+ }
+#ifdef RTE_ARCH_X86
+ if (pkt_burst == bnxt_recv_pkts_vec) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Vector SSE");
+ return 0;
+ }
+#endif
+
+ return -EINVAL;
+}
+
+static int
+bnxt_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
+ struct rte_eth_burst_mode *mode)
+{
+ eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
+
+ if (pkt_burst == bnxt_xmit_pkts) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Scalar");
+ return 0;
+ }
+#ifdef RTE_ARCH_X86
+ if (pkt_burst == bnxt_xmit_pkts_vec) {
+ snprintf(mode->info, sizeof(mode->info), "%s",
+"Vector SSE");
+ return 0;
+ }
+#endif
+
+ return -EINVAL;
+}
+
int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu)
{
struct bnxt *bp = eth_dev->data->dev_private;
@@ -4158,6 +4202,8 @@ static const struct eth_dev_ops bnxt_dev_ops = {
.set_mc_addr_list = bnxt_dev_set_mc_addr_list_op,
.rxq_info_get = bnxt_rxq_info_get_op,
.txq_info_get = bnxt_txq_info_get_op,
+ .rx_burst_mode_get = bnxt_rx_burst_mode_get,
+ .tx_burst_mode_get = bnxt_tx_burst_mode_get,
.dev_led_on = bnxt_dev_led_on_op,
.dev_led_off = bnxt_dev_led_off_op,
.xstats_get_by_id = bnxt_dev_xstats_get_by_id_op,
--
2.25.1
[dpdk-dev] [PATCH 3/3] net/bnxt: add ARM64 vector support
Add bnxt vector PMD support using NEON SIMD instructions.
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
v2:
- Use rte_vect.h instead of including arm_neon.h directly in
bnxt_rxtx_vec_neon.c.
drivers/net/bnxt/Makefile | 3 +
drivers/net/bnxt/bnxt_ethdev.c | 10 +-
drivers/net/bnxt/bnxt_ring.c| 4 +-
drivers/net/bnxt/bnxt_rxq.h | 2 +-
drivers/net/bnxt/bnxt_rxr.c | 2 +-
drivers/net/bnxt/bnxt_rxr.h | 2 +-
drivers/net/bnxt/bnxt_rxtx_vec_common.h | 57 +++
drivers/net/bnxt/bnxt_rxtx_vec_neon.c | 469
drivers/net/bnxt/bnxt_rxtx_vec_sse.c| 46 +--
drivers/net/bnxt/bnxt_txr.h | 2 +-
drivers/net/bnxt/meson.build| 2 +
11 files changed, 544 insertions(+), 55 deletions(-)
create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_common.h
create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_neon.c
diff --git a/drivers/net/bnxt/Makefile b/drivers/net/bnxt/Makefile
index 2a39ed139..98be7b28e 100644
--- a/drivers/net/bnxt/Makefile
+++ b/drivers/net/bnxt/Makefile
@@ -42,6 +42,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += rte_pmd_bnxt.c
ifeq ($(CONFIG_RTE_ARCH_X86), y)
SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_rxtx_vec_sse.c
endif
+ifeq ($(CONFIG_RTE_ARCH_ARM64), y)
+SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_rxtx_vec_neon.c
+endif
ifeq ($(CONFIG_RTE_LIBRTE_BNXT_PMD), y)
CFLAGS += -I$(SRCDIR) -I$(SRCDIR)/tf_core -I$(SRCDIR)/tf_ulp
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index edb650bbc..10d70ab0b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1075,7 +1075,7 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev)
{
struct bnxt *bp = eth_dev->data->dev_private;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
#ifndef RTE_LIBRTE_IEEE1588
/*
* Vector mode receive can be enabled only if scatter rx is not
@@ -1115,7 +1115,7 @@ bnxt_receive_function(struct rte_eth_dev *eth_dev)
static eth_tx_burst_t
bnxt_transmit_function(__rte_unused struct rte_eth_dev *eth_dev)
{
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
#ifndef RTE_LIBRTE_IEEE1588
/*
* Vector mode transmit can be enabled only if not using scatter rx
@@ -2593,7 +2593,7 @@ bnxt_rx_burst_mode_get(struct rte_eth_dev *dev,
__rte_unused uint16_t queue_id,
"Scalar");
return 0;
}
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
if (pkt_burst == bnxt_recv_pkts_vec) {
snprintf(mode->info, sizeof(mode->info), "%s",
"Vector SSE");
@@ -2615,7 +2615,7 @@ bnxt_tx_burst_mode_get(struct rte_eth_dev *dev,
__rte_unused uint16_t queue_id,
"Scalar");
return 0;
}
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
if (pkt_burst == bnxt_xmit_pkts_vec) {
snprintf(mode->info, sizeof(mode->info), "%s",
"Vector SSE");
@@ -2644,7 +2644,7 @@ int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t
new_mtu)
new_pkt_size = new_mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN +
VLAN_TAG_SIZE * BNXT_NUM_VLANS;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
/*
* If vector-mode tx/rx is active, disallow any MTU change that would
* require scattered receive support.
diff --git a/drivers/net/bnxt/bnxt_ring.c b/drivers/net/bnxt/bnxt_ring.c
index 24a947f27..54f654744 100644
--- a/drivers/net/bnxt/bnxt_ring.c
+++ b/drivers/net/bnxt/bnxt_ring.c
@@ -608,7 +608,7 @@ int bnxt_alloc_hwrm_rx_ring(struct bnxt *bp, int
queue_index)
bnxt_db_write(&rxr->ag_db, rxr->ag_prod);
}
rxq->index = queue_index;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
bnxt_rxq_vec_setup(rxq);
#endif
@@ -713,7 +713,7 @@ int bnxt_alloc_hwrm_rings(struct bnxt *bp)
bnxt_db_write(&rxr->rx_db, rxr->rx_prod);
bnxt_db_write(&rxr->ag_db, rxr->ag_prod);
rxq->index = i;
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64)
bnxt_rxq_vec_setup(rxq);
#endif
}
diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h
index 4f5182d9e..d5ce3b6d5 100644
--- a/drivers/net/bnxt/bnxt_rxq.h
+++ b/drivers/net/bnxt/bnxt_rxq.h
@@ -22,7 +22,7 @@ struct bnxt_rx_queue {
uint16_tnb_rx_hold; /* num held free RX desc */
uint16_trx_free_thresh; /* max free RX desc to hold */
uint16_tqueue_id; /* RX queue index */
-#ifdef RTE_ARCH_X86
+#if
Re: [dpdk-dev] [PATCH] net/bnxt: allow configuring vector mode
Hi Stephen,
On Thu, Mar 5, 2020 at 1:45 AM Stephen Hemminger
wrote:
>
>
> Make the configuration use the same as other drivers with
> vector mode: ixge, i40e, ...
s/ixge/ixgbe/?
>
> This will also make future support of vector mode on other
> architectures possible.
>
> Fixes: bc4a000f2f53 ("net/bnxt: implement SSE vector mode")
> +#error "bnxt: IEEE1588 is incompatiable with vector mode"
> +#endif
s/incompatiable/incompatible/
This was this approach taken in the initial patch set to add bnxt
vector mode support,
however based on feedback the current approach was used instead. That discussion
can be found here:
http://patches.dpdk.org/patch/53683/
If mark support could be treated as a receive offload, it would be
possible to choose
the non-vector receive handler based on whether mark is enabled. Adding a kvargs
option to disable vector mode might be another possibility. Otherwise,
a build-time
configuration option does seem to be useful.
LGTM... with the above:
Acked-by: Lance Richardson
Regards,
Lance
[dpdk-dev] [PATCH] net/bnxt: fix vector receive check
Take DEV_RX_OFFLOAD_RSS_HASH into account when deciding whether vector
mode receive can be enabled.
Fixes: 8b945a7f7dcb ("drivers/net: update Rx RSS hash offload capabilities")
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Khaparde
---
drivers/net/bnxt/bnxt_ethdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index b1e831b87..41848f36f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -758,6 +758,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
*eth_dev)
DEV_RX_OFFLOAD_UDP_CKSUM |
DEV_RX_OFFLOAD_TCP_CKSUM |
DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_RSS_HASH |
DEV_RX_OFFLOAD_VLAN_FILTER))) {
PMD_DRV_LOG(INFO, "Using vector mode receive for port %d\n",
eth_dev->data->port_id);
--
2.17.1
[dpdk-dev] [PATCH] net/bnxt: fix logic error when freeing RSS context
The conditional used to determine whether freeing RSS
contexts for thor vs. non-thor controller was reversed.
Fix this, also reset number of active RSS contexts to
zero after release in the thor case.
Fixes: 38412304b50a ("net/bnxt: enable RSS for thor-based controllers")
Signed-off-by: Lance Richardson
Reviewed-by: Ajit Kumar Khaparde
---
drivers/net/bnxt/bnxt_hwrm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 696974c83..71f03775a 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2312,12 +2312,13 @@ void bnxt_free_all_hwrm_resources(struct bnxt *bp)
bnxt_clear_hwrm_vnic_filters(bp, vnic);
- if (!BNXT_CHIP_THOR(bp)) {
+ if (BNXT_CHIP_THOR(bp)) {
for (j = 0; j < vnic->num_lb_ctxts; j++) {
bnxt_hwrm_vnic_ctx_free(bp, vnic,
vnic->fw_grp_ids[j]);
vnic->fw_grp_ids[j] = INVALID_HW_RING_ID;
}
+ vnic->num_lb_ctxts = 0;
} else {
bnxt_hwrm_vnic_ctx_free(bp, vnic, vnic->rss_rule);
vnic->rss_rule = INVALID_HW_RING_ID;
--
2.17.1
