[PATCH 1/3] eventdev: add element offset to event vector

[PATCH 1/3] eventdev: add element offset to event vector

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add ``elem_offset:12`` bit field event vector structure
the bits are taken from ``rsvd:15``.
The element offset defines the offset into the vector array
at which valid elements start.
The valid elements count will be equal to nb_elem - elem_offset.

Update Rx/Tx adapter SW implementation to use elem_offset.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
 lib/eventdev/rte_event_eth_tx_adapter.c | 7 ++++---
 lib/eventdev/rte_eventdev.h             | 8 ++++++--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index bf8741d2ea..bd72f9b845 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter *rx_adapter,
 	vec->vector_ev->port = vec->port;
 	vec->vector_ev->queue = vec->queue;
 	vec->vector_ev->attr_valid = true;
+	vec->vector_ev->elem_offset = 0;
 	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
 }

diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
index b4b37f1cae..da70883e0d 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c
@@ -524,16 +524,17 @@ txa_process_event_vector(struct txa_service_data *txa,
 		queue = vec->queue;
 		tqi = txa_service_queue(txa, port, queue);
 		if (unlikely(tqi == NULL || !tqi->added)) {
-			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
+			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
+					      vec->nb_elem - vec->elem_offset);
 			rte_mempool_put(rte_mempool_from_obj(vec), vec);
 			return 0;
 		}
-		for (i = 0; i < vec->nb_elem; i++) {
+		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
 			nb_tx += rte_eth_tx_buffer(port, queue, tqi->tx_buf,
 						   mbufs[i]);
 		}
 	} else {
-		for (i = 0; i < vec->nb_elem; i++) {
+		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
 			port = mbufs[i]->port;
 			queue = rte_event_eth_tx_adapter_txq_get(mbufs[i]);
 			tqi = txa_service_queue(txa, port, queue);
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 6a6f6ea4c1..b0698fe748 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1060,8 +1060,12 @@ rte_event_dev_close(uint8_t dev_id);
  */
 struct rte_event_vector {
 	uint16_t nb_elem;
-	/**< Number of elements in this event vector. */
-	uint16_t rsvd : 15;
+	/**< Total number of elements in this event vector. */
+	uint16_t elem_offset : 12;
+	/**< Offset into the vector array where valid elements start from.
+	 * The valid elements count would be nb_elem - elem_offset.
+	 */
+	uint16_t rsvd : 3;
 	/**< Reserved for future use */
 	uint16_t attr_valid : 1;
 	/**< Indicates that the below union attributes have valid information.
--
2.25.1

[PATCH 2/3] examples: update event vector free routine

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Update event vector free routine to account for element
offset while freeing elements.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 app/test-eventdev/test_pipeline_common.c | 5 +++--
 examples/l2fwd-event/l2fwd_common.c      | 5 +++--
 examples/l3fwd/l3fwd_event.c             | 5 +++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-eventdev/test_pipeline_common.c
index 4f40d37659..4404c665f5 100644
--- a/app/test-eventdev/test_pipeline_common.c
+++ b/app/test-eventdev/test_pipeline_common.c
@@ -673,8 +673,9 @@ pipeline_vector_array_free(struct rte_event events[], uint16_t num)
 	uint16_t i;

 	for (i = 0; i < num; i++) {
-		rte_pktmbuf_free_bulk(events[i].vec->mbufs,
-				      events[i].vec->nb_elem);
+		rte_pktmbuf_free_bulk(
+			&events[i].vec->mbufs[events[i].vec->elem_offset],
+			events[i].vec->nb_elem - events[i].vec->elem_offset);
 		rte_mempool_put(rte_mempool_from_obj(events[i].vec),
 				events[i].vec);
 	}
diff --git a/examples/l2fwd-event/l2fwd_common.c b/examples/l2fwd-event/l2fwd_common.c
index 41a0d3f22f..5af7fa6127 100644
--- a/examples/l2fwd-event/l2fwd_common.c
+++ b/examples/l2fwd-event/l2fwd_common.c
@@ -121,8 +121,9 @@ l2fwd_event_vector_array_free(struct rte_event events[], uint16_t num)
 	uint16_t i;

 	for (i = 0; i < num; i++) {
-		rte_pktmbuf_free_bulk(events[i].vec->mbufs,
-				      events[i].vec->nb_elem);
+		rte_pktmbuf_free_bulk(
+			&events[i].vec->mbufs[events[i].vec->elem_offset],
+			events[i].vec->nb_elem - events[i].vec->elem_offset);
 		rte_mempool_put(rte_mempool_from_obj(events[i].vec),
 				events[i].vec);
 	}
diff --git a/examples/l3fwd/l3fwd_event.c b/examples/l3fwd/l3fwd_event.c
index 0b58475c85..984dad1ece 100644
--- a/examples/l3fwd/l3fwd_event.c
+++ b/examples/l3fwd/l3fwd_event.c
@@ -294,8 +294,9 @@ l3fwd_event_vector_array_free(struct rte_event events[], uint16_t num)
 	uint16_t i;

 	for (i = 0; i < num; i++) {
-		rte_pktmbuf_free_bulk(events[i].vec->mbufs,
-				      events[i].vec->nb_elem);
+		rte_pktmbuf_free_bulk(
+			&events[i].vec->mbufs[events[i].vec->elem_offset],
+			events[i].vec->nb_elem - events[i].vec->elem_offset);
 		rte_mempool_put(rte_mempool_from_obj(events[i].vec),
 				events[i].vec);
 	}
--
2.25.1

[PATCH 3/3] event/cnxk: update event vector Tx routine

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Update event vector transmit routine to honor elem_offset.
Use ``rte_event_vector::elem_offset`` to report partial
vector transmission to the application when there is not
enough space in the SQ.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 drivers/event/cnxk/cn10k_worker.h | 100 +++++++++++++++++++++---------
 1 file changed, 71 insertions(+), 29 deletions(-)

diff --git a/drivers/event/cnxk/cn10k_worker.h b/drivers/event/cnxk/cn10k_worker.h
index a71e076ff8..7d1fbe9c1b 100644
--- a/drivers/event/cnxk/cn10k_worker.h
+++ b/drivers/event/cnxk/cn10k_worker.h
@@ -516,7 +516,15 @@ cn10k_sso_txq_fc_wait(const struct cn10k_eth_txq *txq)
 		;
 }

-static __rte_always_inline void
+static __rte_always_inline int32_t
+cn10k_sso_sq_depth(const struct cn10k_eth_txq *txq)
+{
+	return (txq->nb_sqb_bufs_adj -
+		__atomic_load_n((int16_t *)txq->fc_mem, __ATOMIC_RELAXED))
+	       << txq->sqes_per_sqb_log2;
+}
+
+static __rte_always_inline uint16_t
 cn10k_sso_tx_one(struct cn10k_sso_hws *ws, struct rte_mbuf *m, uint64_t *cmd,
 		 uint16_t lmt_id, uintptr_t lmt_addr, uint8_t sched_type,
 		 const uint64_t *txq_data, const uint32_t flags)
@@ -529,6 +537,9 @@ cn10k_sso_tx_one(struct cn10k_sso_hws *ws, struct rte_mbuf *m, uint64_t *cmd,
 	bool sec;

 	txq = cn10k_sso_hws_xtract_meta(m, txq_data);
+	if (cn10k_sso_sq_depth(txq) <= 0)
+		return 0;
+
 	cn10k_nix_tx_skeleton(txq, cmd, flags, 0);
 	/* Perform header writes before barrier
 	 * for TSO
@@ -566,21 +577,29 @@ cn10k_sso_tx_one(struct cn10k_sso_hws *ws, struct rte_mbuf *m, uint64_t *cmd,

 	cn10k_sso_txq_fc_wait(txq);
 	roc_lmt_submit_steorl(lmt_id, pa);
+
+	if (flags & NIX_TX_OFFLOAD_MBUF_NOFF_F) {
+		if (ref_cnt > 1)
+			rte_io_wmb();
+	}
+	return 1;
 }

-static __rte_always_inline void
+static __rte_always_inline uint16_t
 cn10k_sso_vwqe_split_tx(struct cn10k_sso_hws *ws, struct rte_mbuf **mbufs,
 			uint16_t nb_mbufs, uint64_t *cmd, uint16_t lmt_id,
 			uintptr_t lmt_addr, uint8_t sched_type,
 			const uint64_t *txq_data, const uint32_t flags)
 {
-	uint16_t port[4], queue[4];
 	uint16_t i, j, pkts, scalar;
+	uint16_t port[4], queue[4];
 	struct cn10k_eth_txq *txq;
+	uint16_t done, cnt;
+	int32_t space;

 	scalar = nb_mbufs & (NIX_DESCS_PER_LOOP - 1);
 	pkts = RTE_ALIGN_FLOOR(nb_mbufs, NIX_DESCS_PER_LOOP);
-
+	cnt = 0;
 	for (i = 0; i < pkts; i += NIX_DESCS_PER_LOOP) {
 		port[0] = mbufs[i]->port;
 		port[1] = mbufs[i + 1]->port;
@@ -594,27 +613,42 @@ cn10k_sso_vwqe_split_tx(struct cn10k_sso_hws *ws, struct rte_mbuf **mbufs,

 		if (((port[0] ^ port[1]) & (port[2] ^ port[3])) ||
 		    ((queue[0] ^ queue[1]) & (queue[2] ^ queue[3]))) {
-			for (j = 0; j < 4; j++)
-				cn10k_sso_tx_one(ws, mbufs[i + j], cmd, lmt_id,
-						 lmt_addr, sched_type, txq_data,
-						 flags);
+			for (j = 0; j < 4; j++) {
+				done = cn10k_sso_tx_one(
+					ws, mbufs[i + j], cmd, lmt_id, lmt_addr,
+					sched_type, txq_data, flags);
+				if (!done)
+					goto fail;
+				rte_io_wmb();
+				cnt++;
+			}
 		} else {
 			txq = (struct cn10k_eth_txq
 				       *)(txq_data[(txq_data[port[0]] >> 48) +
 						   queue[0]] &
 					  (BIT_ULL(48) - 1));
-			cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws,
-						   &mbufs[i], 4, cmd,
-						   flags | NIX_TX_VWQE_F);
+			space = cn10k_sso_sq_depth(txq);
+			if (space < NIX_DESCS_PER_LOOP)
+				goto fail;
+			cn10k_nix_xmit_pkts_vector(
+				txq, (uint64_t *)ws, &mbufs[i],
+				NIX_DESCS_PER_LOOP, cmd, flags | NIX_TX_VWQE_F);
+			cnt += NIX_DESCS_PER_LOOP;
 		}
 	}

 	mbufs += i;

 	for (i = 0; i < scalar; i++) {
-		cn10k_sso_tx_one(ws, mbufs[i], cmd, lmt_id, lmt_addr,
-				 sched_type, txq_data, flags);
+		done = cn10k_sso_tx_one(ws, mbufs[i], cmd, lmt_id, lmt_addr,
+					sched_type, txq_data, flags);
+		if (!done)
+			break;
+		rte_io_wmb();
+		cnt++;
 	}
+fail:
+	return cnt;
 }

 static __rte_always_inline uint16_t
@@ -633,7 +667,12 @@ cn10k_sso_hws_event_tx(struct cn10k_sso_hws *ws, struct rte_event *ev,
 	if (ev->event_type & RTE_EVENT_TYPE_VECTOR) {
 		struct rte_mbuf **mbufs = ev->vec->mbufs;
 		uint64_t meta = *(uint64_t *)ev->vec;
+		uint16_t offset, nb_pkts;
+		int32_t space;

+		nb_pkts = meta & 0xFFFF;
+		offset = (meta >> 16) & 0xFFF;
+		nb_pkts -= offset;
 		if (meta & BIT(31)) {
 			txq = (struct cn10k_eth_txq
 				       *)(txq_data[(txq_data[meta >> 32] >>
@@ -641,29 +680,32 @@ cn10k_sso_hws_event_tx(struct cn10k_sso_hws *ws, struct rte_event *ev,
 						   (meta >> 48)] &
 					  (BIT_ULL(48) - 1));

-			cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws, mbufs,
-						   meta & 0xFFFF, cmd,
+			/* Transmit based on queue depth */
+			space = cn10k_sso_sq_depth(txq);
+			if (space <= 0)
+				return 0;
+			nb_pkts = nb_pkts < space ? nb_pkts : (uint16_t)space;
+			cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws,
+						   mbufs + offset, nb_pkts, cmd,
 						   flags | NIX_TX_VWQE_F);
 		} else {
-			cn10k_sso_vwqe_split_tx(
-				ws, mbufs, meta & 0xFFFF, cmd, lmt_id, lmt_addr,
-				ev->sched_type, txq_data, flags);
+			nb_pkts = cn10k_sso_vwqe_split_tx(
+				ws, mbufs + offset, nb_pkts, cmd, lmt_id,
+				lmt_addr, ev->sched_type, txq_data, flags);
 		}
-		rte_mempool_put(rte_mempool_from_obj(ev->vec), ev->vec);
+		if (!((meta & 0xFFFF) - nb_pkts - offset))
+			rte_mempool_put(rte_mempool_from_obj(ev->vec), ev->vec);
+		else
+			*(uint64_t *)ev->vec = (meta & ~0xFFF0000UL) |
+					       ((uint32_t)nb_pkts + offset)
+						       << 16;
 		rte_prefetch0(ws);
-		return 1;
+		return !((meta & 0xFFFF) - nb_pkts - offset);
 	}

 	m = ev->mbuf;
-	txq = cn10k_sso_hws_xtract_meta(m, txq_data);
-	if (((txq->nb_sqb_bufs_adj -
-	      __atomic_load_n((int16_t *)txq->fc_mem, __ATOMIC_RELAXED))
-	     << txq->sqes_per_sqb_log2) <= 0)
-		return 0;
-	cn10k_sso_tx_one(ws, m, cmd, lmt_id, lmt_addr, ev->sched_type, txq_data,
-			 flags);
-
-	return 1;
+	return cn10k_sso_tx_one(ws, m, cmd, lmt_id, lmt_addr, ev->sched_type,
+				txq_data, flags);
 }

 #define T(name, sz, flags)                                                     \
--
2.25.1

Re: [PATCH 1/3] eventdev: add element offset to event vector

[Mattias Rönnblom]

On 2022-08-16 17:49, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add ``elem_offset:12`` bit field event vector structure
> the bits are taken from ``rsvd:15``.
> The element offset defines the offset into the vector array
> at which valid elements start.
> The valid elements count will be equal to nb_elem - elem_offset.
> 

I'm missing a rationale why this change is a good idea. (I can guess, 
but I think it's better to spell it out.)

> Update Rx/Tx adapter SW implementation to use elem_offset.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>   lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
>   lib/eventdev/rte_event_eth_tx_adapter.c | 7 ++++---
>   lib/eventdev/rte_eventdev.h             | 8 ++++++--
>   3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index bf8741d2ea..bd72f9b845 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter *rx_adapter,
>   	vec->vector_ev->port = vec->port;
>   	vec->vector_ev->queue = vec->queue;
>   	vec->vector_ev->attr_valid = true;
> +	vec->vector_ev->elem_offset = 0;
>   	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
>   }
> 
> diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
> index b4b37f1cae..da70883e0d 100644
> --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> @@ -524,16 +524,17 @@ txa_process_event_vector(struct txa_service_data *txa,
>   		queue = vec->queue;
>   		tqi = txa_service_queue(txa, port, queue);
>   		if (unlikely(tqi == NULL || !tqi->added)) {
> -			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
> +			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
> +					      vec->nb_elem - vec->elem_offset);
>   			rte_mempool_put(rte_mempool_from_obj(vec), vec);
>   			return 0;
>   		}
> -		for (i = 0; i < vec->nb_elem; i++) {
> +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
>   			nb_tx += rte_eth_tx_buffer(port, queue, tqi->tx_buf,
>   						   mbufs[i]);
>   		}
>   	} else {
> -		for (i = 0; i < vec->nb_elem; i++) {
> +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
>   			port = mbufs[i]->port;
>   			queue = rte_event_eth_tx_adapter_txq_get(mbufs[i]);
>   			tqi = txa_service_queue(txa, port, queue);
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index 6a6f6ea4c1..b0698fe748 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1060,8 +1060,12 @@ rte_event_dev_close(uint8_t dev_id);
>    */
>   struct rte_event_vector {
>   	uint16_t nb_elem;
> -	/**< Number of elements in this event vector. */
> -	uint16_t rsvd : 15;
> +	/**< Total number of elements in this event vector. */

I'm not sure "total" adds anything here. Didn't the old nb_elem also 
include the total number of elements?

nb_elem doesn't represent the number of elements in the vector any more, 
does it?

Why not just keep the old semantics, and let it represent the number of 
used slots in the vector array? As opposed to being the <last used 
index> + 1.

> +	uint16_t elem_offset : 12;
> +	/**< Offset into the vector array where valid elements start from.
> +	 * The valid elements count would be nb_elem - elem_offset.
> +	 */
> +	uint16_t rsvd : 3;
>   	/**< Reserved for future use */
>   	uint16_t attr_valid : 1;
>   	/**< Indicates that the below union attributes have valid information.
> --
> 2.25.1
> 

RE: [EXT] Re: [PATCH 1/3] eventdev: add element offset to event vector

[Pavan Nikhilesh Bhagavatula]

> On 2022-08-16 17:49, pbhagavatula@marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Add ``elem_offset:12`` bit field event vector structure
> > the bits are taken from ``rsvd:15``.
> > The element offset defines the offset into the vector array
> > at which valid elements start.
> > The valid elements count will be equal to nb_elem - elem_offset.
> >
> 
> I'm missing a rationale why this change is a good idea. (I can guess,
> but I think it's better to spell it out.)
> 

Sure, I will add it in the next version.

> > Update Rx/Tx adapter SW implementation to use elem_offset.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> >   lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
> >   lib/eventdev/rte_event_eth_tx_adapter.c | 7 ++++---
> >   lib/eventdev/rte_eventdev.h             | 8 ++++++--
> >   3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index bf8741d2ea..bd72f9b845 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter
> *rx_adapter,
> >   	vec->vector_ev->port = vec->port;
> >   	vec->vector_ev->queue = vec->queue;
> >   	vec->vector_ev->attr_valid = true;
> > +	vec->vector_ev->elem_offset = 0;
> >   	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
> >   }
> >
> > diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
> b/lib/eventdev/rte_event_eth_tx_adapter.c
> > index b4b37f1cae..da70883e0d 100644
> > --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> > @@ -524,16 +524,17 @@ txa_process_event_vector(struct
> txa_service_data *txa,
> >   		queue = vec->queue;
> >   		tqi = txa_service_queue(txa, port, queue);
> >   		if (unlikely(tqi == NULL || !tqi->added)) {
> > -			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
> > +			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
> > +					      vec->nb_elem - vec-
> >elem_offset);
> >   			rte_mempool_put(rte_mempool_from_obj(vec),
> vec);
> >   			return 0;
> >   		}
> > -		for (i = 0; i < vec->nb_elem; i++) {
> > +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
> >   			nb_tx += rte_eth_tx_buffer(port, queue, tqi-
> >tx_buf,
> >   						   mbufs[i]);
> >   		}
> >   	} else {
> > -		for (i = 0; i < vec->nb_elem; i++) {
> > +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
> >   			port = mbufs[i]->port;
> >   			queue =
> rte_event_eth_tx_adapter_txq_get(mbufs[i]);
> >   			tqi = txa_service_queue(txa, port, queue);
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index 6a6f6ea4c1..b0698fe748 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -1060,8 +1060,12 @@ rte_event_dev_close(uint8_t dev_id);
> >    */
> >   struct rte_event_vector {
> >   	uint16_t nb_elem;
> > -	/**< Number of elements in this event vector. */
> > -	uint16_t rsvd : 15;
> > +	/**< Total number of elements in this event vector. */
> 
> I'm not sure "total" adds anything here. Didn't the old nb_elem also
> include the total number of elements?
> 

Yes, I added it to clarify that it includes slots that don’t have valid elements.
I will update the comment to convey that it includes elements before offset.

> nb_elem doesn't represent the number of elements in the vector any more,
> does it?
> 
> Why not just keep the old semantics, and let it represent the number of
> used slots in the vector array? As opposed to being the <last used
> index> + 1.

I think its simpler to just manage updates to the vector by updating elem_offset and keeping 
nb_elem as a constant, valid elements count can simply be calculated via nb_elem - elem_offset.
Vector is empty when nb_elem = elem_offset and can be reused simply by setting elem_offset to 0.

Having to update both nb_elem and elem_offset might be a tad bit error prone.

> 
> > +	uint16_t elem_offset : 12;
> > +	/**< Offset into the vector array where valid elements start from.
> > +	 * The valid elements count would be nb_elem - elem_offset.
> > +	 */
> > +	uint16_t rsvd : 3;
> >   	/**< Reserved for future use */
> >   	uint16_t attr_valid : 1;
> >   	/**< Indicates that the below union attributes have valid
> information.
> > --
> > 2.25.1
> >

Re: [EXT] Re: [PATCH 1/3] eventdev: add element offset to event vector

[Mattias Rönnblom]

On 2022-08-23 22:39, Pavan Nikhilesh Bhagavatula wrote:
>> On 2022-08-16 17:49, pbhagavatula@marvell.com wrote:
>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>
>>> Add ``elem_offset:12`` bit field event vector structure
>>> the bits are taken from ``rsvd:15``.
>>> The element offset defines the offset into the vector array
>>> at which valid elements start.
>>> The valid elements count will be equal to nb_elem - elem_offset.
>>>
>>
>> I'm missing a rationale why this change is a good idea. (I can guess,
>> but I think it's better to spell it out.)
>>
> 
> Sure, I will add it in the next version.
> 
>>> Update Rx/Tx adapter SW implementation to use elem_offset.
>>>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>> ---
>>>    lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
>>>    lib/eventdev/rte_event_eth_tx_adapter.c | 7 ++++---
>>>    lib/eventdev/rte_eventdev.h             | 8 ++++++--
>>>    3 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
>> b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> index bf8741d2ea..bd72f9b845 100644
>>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
>>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> @@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter
>> *rx_adapter,
>>>    	vec->vector_ev->port = vec->port;
>>>    	vec->vector_ev->queue = vec->queue;
>>>    	vec->vector_ev->attr_valid = true;
>>> +	vec->vector_ev->elem_offset = 0;
>>>    	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
>>>    }
>>>
>>> diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
>> b/lib/eventdev/rte_event_eth_tx_adapter.c
>>> index b4b37f1cae..da70883e0d 100644
>>> --- a/lib/eventdev/rte_event_eth_tx_adapter.c
>>> +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
>>> @@ -524,16 +524,17 @@ txa_process_event_vector(struct
>> txa_service_data *txa,
>>>    		queue = vec->queue;
>>>    		tqi = txa_service_queue(txa, port, queue);
>>>    		if (unlikely(tqi == NULL || !tqi->added)) {
>>> -			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
>>> +			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
>>> +					      vec->nb_elem - vec-
>>> elem_offset);
>>>    			rte_mempool_put(rte_mempool_from_obj(vec),
>> vec);
>>>    			return 0;
>>>    		}
>>> -		for (i = 0; i < vec->nb_elem; i++) {
>>> +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
>>>    			nb_tx += rte_eth_tx_buffer(port, queue, tqi-
>>> tx_buf,
>>>    						   mbufs[i]);
>>>    		}
>>>    	} else {
>>> -		for (i = 0; i < vec->nb_elem; i++) {
>>> +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
>>>    			port = mbufs[i]->port;
>>>    			queue =
>> rte_event_eth_tx_adapter_txq_get(mbufs[i]);
>>>    			tqi = txa_service_queue(txa, port, queue);
>>> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
>>> index 6a6f6ea4c1..b0698fe748 100644
>>> --- a/lib/eventdev/rte_eventdev.h
>>> +++ b/lib/eventdev/rte_eventdev.h
>>> @@ -1060,8 +1060,12 @@ rte_event_dev_close(uint8_t dev_id);
>>>     */
>>>    struct rte_event_vector {
>>>    	uint16_t nb_elem;
>>> -	/**< Number of elements in this event vector. */
>>> -	uint16_t rsvd : 15;
>>> +	/**< Total number of elements in this event vector. */
>>
>> I'm not sure "total" adds anything here. Didn't the old nb_elem also
>> include the total number of elements?
>>
> 
> Yes, I added it to clarify that it includes slots that don’t have valid elements.
> I will update the comment to convey that it includes elements before offset.
> 

The issue is that it doesn't clarify anything. Change the name, or 
change the semantics to fit the name, instead of explaining a poor name 
in a comment.

>> nb_elem doesn't represent the number of elements in the vector any more,
>> does it?
>>
>> Why not just keep the old semantics, and let it represent the number of
>> used slots in the vector array? As opposed to being the <last used
>> index> + 1.
> 
> I think its simpler to just manage updates to the vector by updating elem_offset and keeping
> nb_elem as a constant, valid elements count can simply be calculated via nb_elem - elem_offset.
> Vector is empty when nb_elem = elem_offset and can be reused simply by setting elem_offset to 0.
> 
> Having to update both nb_elem and elem_offset might be a tad bit error prone.
> 

I think you should focus more on the end result, rather how easily you 
can get there. In my experience, in the long run, that's what pays off 
is to keep the design clean and reduce the overall complexity.

You don't think having a field called "nb_elem" which value doesn't 
represent the number of elements, but rather something else, is error prone?

>>
>>> +	uint16_t elem_offset : 12;
>>> +	/**< Offset into the vector array where valid elements start from.
>>> +	 * The valid elements count would be nb_elem - elem_offset.
>>> +	 */
>>> +	uint16_t rsvd : 3;
>>>    	/**< Reserved for future use */
>>>    	uint16_t attr_valid : 1;
>>>    	/**< Indicates that the below union attributes have valid
>> information.
>>> --
>>> 2.25.1
>>>

RE: [EXT] Re: [PATCH 1/3] eventdev: add element offset to event vector

[Pavan Nikhilesh Bhagavatula]

> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Wednesday, August 24, 2022 2:12 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Mattias
> Rönnblom <hofors@lysator.liu.se>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Jay Jayatheerthan <jay.jayatheerthan@intel.com>
> Cc: dev@dpdk.org; erik.g.carrillo@intel.com; abhinandan.gujjar@intel.com;
> timothy.mcdaniel@intel.com; Shijith Thotton <sthotton@marvell.com>;
> hemant.agrawal@nxp.com; nipun.gupta@nxp.com;
> harry.van.haaren@intel.com; liangma@liangbit.com;
> peter.mccarthy@intel.com
> Subject: Re: [EXT] Re: [PATCH 1/3] eventdev: add element offset to event
> vector
> 
> On 2022-08-23 22:39, Pavan Nikhilesh Bhagavatula wrote:
> >> On 2022-08-16 17:49, pbhagavatula@marvell.com wrote:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>
> >>> Add ``elem_offset:12`` bit field event vector structure
> >>> the bits are taken from ``rsvd:15``.
> >>> The element offset defines the offset into the vector array
> >>> at which valid elements start.
> >>> The valid elements count will be equal to nb_elem - elem_offset.
> >>>
> >>
> >> I'm missing a rationale why this change is a good idea. (I can guess,
> >> but I think it's better to spell it out.)
> >>
> >
> > Sure, I will add it in the next version.
> >
> >>> Update Rx/Tx adapter SW implementation to use elem_offset.
> >>>
> >>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>> ---
> >>>    lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
> >>>    lib/eventdev/rte_event_eth_tx_adapter.c | 7 ++++---
> >>>    lib/eventdev/rte_eventdev.h             | 8 ++++++--
> >>>    3 files changed, 11 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> >> b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> index bf8741d2ea..bd72f9b845 100644
> >>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> @@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter
> >> *rx_adapter,
> >>>    	vec->vector_ev->port = vec->port;
> >>>    	vec->vector_ev->queue = vec->queue;
> >>>    	vec->vector_ev->attr_valid = true;
> >>> +	vec->vector_ev->elem_offset = 0;
> >>>    	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
> >>>    }
> >>>
> >>> diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
> >> b/lib/eventdev/rte_event_eth_tx_adapter.c
> >>> index b4b37f1cae..da70883e0d 100644
> >>> --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> >>> +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> >>> @@ -524,16 +524,17 @@ txa_process_event_vector(struct
> >> txa_service_data *txa,
> >>>    		queue = vec->queue;
> >>>    		tqi = txa_service_queue(txa, port, queue);
> >>>    		if (unlikely(tqi == NULL || !tqi->added)) {
> >>> -			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
> >>> +			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
> >>> +					      vec->nb_elem - vec-
> >>> elem_offset);
> >>>    			rte_mempool_put(rte_mempool_from_obj(vec),
> >> vec);
> >>>    			return 0;
> >>>    		}
> >>> -		for (i = 0; i < vec->nb_elem; i++) {
> >>> +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
> >>>    			nb_tx += rte_eth_tx_buffer(port, queue, tqi-
> >>> tx_buf,
> >>>    						   mbufs[i]);
> >>>    		}
> >>>    	} else {
> >>> -		for (i = 0; i < vec->nb_elem; i++) {
> >>> +		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
> >>>    			port = mbufs[i]->port;
> >>>    			queue =
> >> rte_event_eth_tx_adapter_txq_get(mbufs[i]);
> >>>    			tqi = txa_service_queue(txa, port, queue);
> >>> diff --git a/lib/eventdev/rte_eventdev.h
> b/lib/eventdev/rte_eventdev.h
> >>> index 6a6f6ea4c1..b0698fe748 100644
> >>> --- a/lib/eventdev/rte_eventdev.h
> >>> +++ b/lib/eventdev/rte_eventdev.h
> >>> @@ -1060,8 +1060,12 @@ rte_event_dev_close(uint8_t dev_id);
> >>>     */
> >>>    struct rte_event_vector {
> >>>    	uint16_t nb_elem;
> >>> -	/**< Number of elements in this event vector. */
> >>> -	uint16_t rsvd : 15;
> >>> +	/**< Total number of elements in this event vector. */
> >>
> >> I'm not sure "total" adds anything here. Didn't the old nb_elem also
> >> include the total number of elements?
> >>
> >
> > Yes, I added it to clarify that it includes slots that don’t have valid elements.
> > I will update the comment to convey that it includes elements before
> offset.
> >
> 
> The issue is that it doesn't clarify anything. Change the name, or
> change the semantics to fit the name, instead of explaining a poor name
> in a comment.
>

Names are always subjective and will confuse someone or the other.
But we can do our best to communicate the semantics, how about 
total_(elements|slots|lanes) and valid_(element|slot|lane)_offset.

I will send the next version once we agree upon the naming.

> >> nb_elem doesn't represent the number of elements in the vector any
> more,
> >> does it?
> >>
> >> Why not just keep the old semantics, and let it represent the number of
> >> used slots in the vector array? As opposed to being the <last used
> >> index> + 1.
> >
> > I think its simpler to just manage updates to the vector by updating
> elem_offset and keeping
> > nb_elem as a constant, valid elements count can simply be calculated via
> nb_elem - elem_offset.
> > Vector is empty when nb_elem = elem_offset and can be reused simply by
> setting elem_offset to 0.
> >
> > Having to update both nb_elem and elem_offset might be a tad bit error
> prone.
> >
> 
> I think you should focus more on the end result, rather how easily you
> can get there. In my experience, in the long run, that's what pays off
> is to keep the design clean and reduce the overall complexity.
> 
> You don't think having a field called "nb_elem" which value doesn't
> represent the number of elements, but rather something else, is error
> prone?
> 
> >>
> >>> +	uint16_t elem_offset : 12;
> >>> +	/**< Offset into the vector array where valid elements start from.
> >>> +	 * The valid elements count would be nb_elem - elem_offset.
> >>> +	 */
> >>> +	uint16_t rsvd : 3;
> >>>    	/**< Reserved for future use */
> >>>    	uint16_t attr_valid : 1;
> >>>    	/**< Indicates that the below union attributes have valid
> >> information.
> >>> --
> >>> 2.25.1
> >>>

Re: [EXT] Re: [PATCH 1/3] eventdev: add element offset to event vector

[Jerin Jacob]

> > >>>    struct rte_event_vector {
> > >>>           uint16_t nb_elem;
> > >>> - /**< Number of elements in this event vector. */
> > >>> - uint16_t rsvd : 15;
> > >>> + /**< Total number of elements in this event vector. */
> > >>
> > >> I'm not sure "total" adds anything here. Didn't the old nb_elem also
> > >> include the total number of elements?
> > >>
> > >
> > > Yes, I added it to clarify that it includes slots that don’t have valid elements.
> > > I will update the comment to convey that it includes elements before
> > offset.
> > >
> >
> > The issue is that it doesn't clarify anything. Change the name, or
> > change the semantics to fit the name, instead of explaining a poor name
> > in a comment.
> >
>
> Names are always subjective and will confuse someone or the other.
> But we can do our best to communicate the semantics, how about
> total_(elements|slots|lanes) and valid_(element|slot|lane)_offset.
>
> I will send the next version once we agree upon the naming.

In order to make forward progress, @Mattias Rönnblom Do you have
specific name suggestions for the next version?
If not, I suggest to pick total_elements

Re: [EXT] Re: [PATCH 1/3] eventdev: add element offset to event vector

[Mattias Rönnblom]

On 2022-09-14 15:02, Jerin Jacob wrote:
>>>>>>     struct rte_event_vector {
>>>>>>            uint16_t nb_elem;
>>>>>> - /**< Number of elements in this event vector. */
>>>>>> - uint16_t rsvd : 15;
>>>>>> + /**< Total number of elements in this event vector. */
>>>>>
>>>>> I'm not sure "total" adds anything here. Didn't the old nb_elem also
>>>>> include the total number of elements?
>>>>>
>>>>
>>>> Yes, I added it to clarify that it includes slots that don’t have valid elements.
>>>> I will update the comment to convey that it includes elements before
>>> offset.
>>>>
>>>
>>> The issue is that it doesn't clarify anything. Change the name, or
>>> change the semantics to fit the name, instead of explaining a poor name
>>> in a comment.
>>>
>>
>> Names are always subjective and will confuse someone or the other.
>> But we can do our best to communicate the semantics, how about
>> total_(elements|slots|lanes) and valid_(element|slot|lane)_offset.
>>
>> I will send the next version once we agree upon the naming.
> 
> In order to make forward progress, @Mattias Rönnblom Do you have
> specific name suggestions for the next version?
> If not, I suggest to pick total_elements

I may be missing something here, but I would suggest keeping the old 
name and old semantics. I.e, nb_elem is the number of elements actually 
used. New is only that they may start at index elem_offset, as opposed 
to the old always-at-index-0.

Instead of changes like:
-		for (i = 0; i < vec->nb_elem; i++) {
+		for (i = vec->elem_offset; i < vec->nb_elem; i++) {
    			port = mbufs[i]->port;
    			queue =

You would have:
		for (i = 0; i < vec->nb_elem; i++) {
-   			port = mbufs[i]->port;
+   			port = mbufs[i + vec->elem_offset]->port;
    			queue =


If you for some reason want to have the start index and the end index 
(like the patch suggested), you could replace the original nb_elem with 
two fields, elem_start (what patch calls elem_offset) and elem_end (what 
patch call nb_elem). I think having only an offset and a length is more 
straightforward though. In the elem_end case, you will have people 
asking themselves if it is the last index used, or the first index not 
used (i.e., last index + 1).

[PATCH v2 1/3] eventdev: add element offset to event vector

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add `rte_event_vector:elem_offset:12` bit field event vector
structure the bits are taken from `rte_event_vector::rsvd:15`.
The element offset defines the offset into the vector array
at which valid elements start.
The valid elements count will be equal to
`rte_event_vector::nb_elem`.

Update Rx/Tx adapter SW implementation to use elem_offset.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 v2 Changes:
 - Revert changes to definition of `rte_event_vector::nb_elem`

 lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
 lib/eventdev/rte_event_eth_tx_adapter.c | 8 +++++---
 lib/eventdev/rte_eventdev.h             | 6 ++++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index bf8741d2ea..bd72f9b845 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter *rx_adapter,
 	vec->vector_ev->port = vec->port;
 	vec->vector_ev->queue = vec->queue;
 	vec->vector_ev->attr_valid = true;
+	vec->vector_ev->elem_offset = 0;
 	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
 }

diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
index b4b37f1cae..ccc7fffcce 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c
@@ -524,16 +524,18 @@ txa_process_event_vector(struct txa_service_data *txa,
 		queue = vec->queue;
 		tqi = txa_service_queue(txa, port, queue);
 		if (unlikely(tqi == NULL || !tqi->added)) {
-			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
+			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
+					      vec->nb_elem);
 			rte_mempool_put(rte_mempool_from_obj(vec), vec);
 			return 0;
 		}
 		for (i = 0; i < vec->nb_elem; i++) {
 			nb_tx += rte_eth_tx_buffer(port, queue, tqi->tx_buf,
-						   mbufs[i]);
+						   mbufs[i + vec->elem_offset]);
 		}
 	} else {
-		for (i = 0; i < vec->nb_elem; i++) {
+		for (i = vec->elem_offset; i < vec->elem_offset + vec->nb_elem;
+		     i++) {
 			port = mbufs[i]->port;
 			queue = rte_event_eth_tx_adapter_txq_get(mbufs[i]);
 			tqi = txa_service_queue(txa, port, queue);
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 6a6f6ea4c1..f9fd44604e 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1060,8 +1060,10 @@ rte_event_dev_close(uint8_t dev_id);
  */
 struct rte_event_vector {
 	uint16_t nb_elem;
-	/**< Number of elements in this event vector. */
-	uint16_t rsvd : 15;
+	/**< Number of elements valid in this event vector. */
+	uint16_t elem_offset : 12;
+	/**< Offset into the vector array where valid elements start from. */
+	uint16_t rsvd : 3;
 	/**< Reserved for future use */
 	uint16_t attr_valid : 1;
 	/**< Indicates that the below union attributes have valid information.
--
2.25.1

[PATCH v2 2/3] examples: update event vector free routine

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Update event vector free routine to account for element
offset while freeing elements.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 app/test-eventdev/test_pipeline_common.c | 5 +++--
 examples/l2fwd-event/l2fwd_common.c      | 5 +++--
 examples/l3fwd/l3fwd_event.c             | 5 +++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-eventdev/test_pipeline_common.c
index 4f40d37659..f9a67b2a14 100644
--- a/app/test-eventdev/test_pipeline_common.c
+++ b/app/test-eventdev/test_pipeline_common.c
@@ -673,8 +673,9 @@ pipeline_vector_array_free(struct rte_event events[], uint16_t num)
 	uint16_t i;
 
 	for (i = 0; i < num; i++) {
-		rte_pktmbuf_free_bulk(events[i].vec->mbufs,
-				      events[i].vec->nb_elem);
+		rte_pktmbuf_free_bulk(
+			&events[i].vec->mbufs[events[i].vec->elem_offset],
+			events[i].vec->nb_elem);
 		rte_mempool_put(rte_mempool_from_obj(events[i].vec),
 				events[i].vec);
 	}
diff --git a/examples/l2fwd-event/l2fwd_common.c b/examples/l2fwd-event/l2fwd_common.c
index 41a0d3f22f..03983b3bd7 100644
--- a/examples/l2fwd-event/l2fwd_common.c
+++ b/examples/l2fwd-event/l2fwd_common.c
@@ -121,8 +121,9 @@ l2fwd_event_vector_array_free(struct rte_event events[], uint16_t num)
 	uint16_t i;
 
 	for (i = 0; i < num; i++) {
-		rte_pktmbuf_free_bulk(events[i].vec->mbufs,
-				      events[i].vec->nb_elem);
+		rte_pktmbuf_free_bulk(
+			&events[i].vec->mbufs[events[i].vec->elem_offset],
+			events[i].vec->nb_elem);
 		rte_mempool_put(rte_mempool_from_obj(events[i].vec),
 				events[i].vec);
 	}
diff --git a/examples/l3fwd/l3fwd_event.c b/examples/l3fwd/l3fwd_event.c
index 0b58475c85..3d60fd7d91 100644
--- a/examples/l3fwd/l3fwd_event.c
+++ b/examples/l3fwd/l3fwd_event.c
@@ -294,8 +294,9 @@ l3fwd_event_vector_array_free(struct rte_event events[], uint16_t num)
 	uint16_t i;
 
 	for (i = 0; i < num; i++) {
-		rte_pktmbuf_free_bulk(events[i].vec->mbufs,
-				      events[i].vec->nb_elem);
+		rte_pktmbuf_free_bulk(
+			&events[i].vec->mbufs[events[i].vec->elem_offset],
+			events[i].vec->nb_elem);
 		rte_mempool_put(rte_mempool_from_obj(events[i].vec),
 				events[i].vec);
 	}
-- 
2.25.1

[PATCH v2 3/3] event/cnxk: update event vector Tx routine

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Update event vector transmit routine to honor elem_offset.
Use ``rte_event_vector::elem_offset`` to report partial
vector transmission to the application when there is not
enough space in the SQ.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 drivers/event/cnxk/cn10k_worker.h | 145 ++++++++++++++++++------------
 1 file changed, 89 insertions(+), 56 deletions(-)

diff --git a/drivers/event/cnxk/cn10k_worker.h b/drivers/event/cnxk/cn10k_worker.h
index 55d9e56766..23b5d7ca7d 100644
--- a/drivers/event/cnxk/cn10k_worker.h
+++ b/drivers/event/cnxk/cn10k_worker.h
@@ -534,12 +534,21 @@ cn10k_sso_txq_fc_wait(const struct cn10k_eth_txq *txq)
 		;
 }
 
-static __rte_always_inline void
+static __rte_always_inline int32_t
+cn10k_sso_sq_depth(const struct cn10k_eth_txq *txq)
+{
+	return (txq->nb_sqb_bufs_adj -
+		__atomic_load_n((int16_t *)txq->fc_mem, __ATOMIC_RELAXED))
+	       << txq->sqes_per_sqb_log2;
+}
+
+static __rte_always_inline uint16_t
 cn10k_sso_tx_one(struct cn10k_sso_hws *ws, struct rte_mbuf *m, uint64_t *cmd,
 		 uint16_t lmt_id, uintptr_t lmt_addr, uint8_t sched_type,
 		 const uint64_t *txq_data, const uint32_t flags)
 {
 	uint8_t lnum = 0, loff = 0, shft = 0;
+	uint16_t ref_cnt = m->refcnt;
 	struct cn10k_eth_txq *txq;
 	uintptr_t laddr;
 	uint16_t segdw;
@@ -547,6 +556,9 @@ cn10k_sso_tx_one(struct cn10k_sso_hws *ws, struct rte_mbuf *m, uint64_t *cmd,
 	bool sec;
 
 	txq = cn10k_sso_hws_xtract_meta(m, txq_data);
+	if (cn10k_sso_sq_depth(txq) <= 0)
+		return 0;
+
 	cn10k_nix_tx_skeleton(txq, cmd, flags, 0);
 	/* Perform header writes before barrier
 	 * for TSO
@@ -584,55 +596,66 @@ cn10k_sso_tx_one(struct cn10k_sso_hws *ws, struct rte_mbuf *m, uint64_t *cmd,
 
 	cn10k_sso_txq_fc_wait(txq);
 	roc_lmt_submit_steorl(lmt_id, pa);
+
+	if (flags & NIX_TX_OFFLOAD_MBUF_NOFF_F) {
+		if (ref_cnt > 1)
+			rte_io_wmb();
+	}
+	return 1;
 }
 
-static __rte_always_inline void
+static __rte_always_inline uint16_t
 cn10k_sso_vwqe_split_tx(struct cn10k_sso_hws *ws, struct rte_mbuf **mbufs,
-			uint16_t nb_mbufs, uint64_t *cmd, uint16_t lmt_id,
-			uintptr_t lmt_addr, uint8_t sched_type,
+			uint16_t nb_mbufs, uint64_t *cmd,
 			const uint64_t *txq_data, const uint32_t flags)
 {
-	uint16_t port[4], queue[4];
-	uint16_t i, j, pkts, scalar;
+	uint16_t count = 0, port, queue, ret = 0, last_idx = 0;
 	struct cn10k_eth_txq *txq;
+	int32_t space;
+	int i;
 
-	scalar = nb_mbufs & (NIX_DESCS_PER_LOOP - 1);
-	pkts = RTE_ALIGN_FLOOR(nb_mbufs, NIX_DESCS_PER_LOOP);
-
-	for (i = 0; i < pkts; i += NIX_DESCS_PER_LOOP) {
-		port[0] = mbufs[i]->port;
-		port[1] = mbufs[i + 1]->port;
-		port[2] = mbufs[i + 2]->port;
-		port[3] = mbufs[i + 3]->port;
-
-		queue[0] = rte_event_eth_tx_adapter_txq_get(mbufs[i]);
-		queue[1] = rte_event_eth_tx_adapter_txq_get(mbufs[i + 1]);
-		queue[2] = rte_event_eth_tx_adapter_txq_get(mbufs[i + 2]);
-		queue[3] = rte_event_eth_tx_adapter_txq_get(mbufs[i + 3]);
-
-		if (((port[0] ^ port[1]) & (port[2] ^ port[3])) ||
-		    ((queue[0] ^ queue[1]) & (queue[2] ^ queue[3]))) {
-			for (j = 0; j < 4; j++)
-				cn10k_sso_tx_one(ws, mbufs[i + j], cmd, lmt_id,
-						 lmt_addr, sched_type, txq_data,
-						 flags);
-		} else {
-			txq = (struct cn10k_eth_txq
-				       *)(txq_data[(txq_data[port[0]] >> 48) +
-						   queue[0]] &
-					  (BIT_ULL(48) - 1));
-			cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws,
-						   &mbufs[i], 4, cmd,
-						   flags | NIX_TX_VWQE_F);
+	port = mbufs[0]->port;
+	queue = rte_event_eth_tx_adapter_txq_get(mbufs[0]);
+	for (i = 0; i < nb_mbufs; i++) {
+		if (port != mbufs[i]->port ||
+		    queue != rte_event_eth_tx_adapter_txq_get(mbufs[i])) {
+			if (count) {
+				txq = (struct cn10k_eth_txq
+					       *)(txq_data[(txq_data[port] >>
+							    48) +
+							   queue] &
+						  (BIT_ULL(48) - 1));
+				/* Transmit based on queue depth */
+				space = cn10k_sso_sq_depth(txq);
+				if (space < count)
+					goto done;
+				cn10k_nix_xmit_pkts_vector(
+					txq, (uint64_t *)ws, &mbufs[last_idx],
+					count, cmd, flags | NIX_TX_VWQE_F);
+				ret += count;
+				count = 0;
+			}
+			port = mbufs[i]->port;
+			queue = rte_event_eth_tx_adapter_txq_get(mbufs[i]);
+			last_idx = i;
 		}
+		count++;
 	}
-
-	mbufs += i;
-
-	for (i = 0; i < scalar; i++) {
-		cn10k_sso_tx_one(ws, mbufs[i], cmd, lmt_id, lmt_addr,
-				 sched_type, txq_data, flags);
+	if (count) {
+		txq = (struct cn10k_eth_txq
+			       *)(txq_data[(txq_data[port] >> 48) + queue] &
+				  (BIT_ULL(48) - 1));
+		/* Transmit based on queue depth */
+		space = cn10k_sso_sq_depth(txq);
+		if (space < count)
+			goto done;
+		cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws,
+					   &mbufs[last_idx], count, cmd,
+					   flags | NIX_TX_VWQE_F);
+		ret += count;
 	}
+done:
+	return ret;
 }
 
 static __rte_always_inline uint16_t
@@ -651,7 +674,11 @@ cn10k_sso_hws_event_tx(struct cn10k_sso_hws *ws, struct rte_event *ev,
 	if (ev->event_type & RTE_EVENT_TYPE_VECTOR) {
 		struct rte_mbuf **mbufs = ev->vec->mbufs;
 		uint64_t meta = *(uint64_t *)ev->vec;
+		uint16_t offset, nb_pkts, left;
+		int32_t space;
 
+		nb_pkts = meta & 0xFFFF;
+		offset = (meta >> 16) & 0xFFF;
 		if (meta & BIT(31)) {
 			txq = (struct cn10k_eth_txq
 				       *)(txq_data[(txq_data[meta >> 32] >>
@@ -659,29 +686,35 @@ cn10k_sso_hws_event_tx(struct cn10k_sso_hws *ws, struct rte_event *ev,
 						   (meta >> 48)] &
 					  (BIT_ULL(48) - 1));
 
-			cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws, mbufs,
-						   meta & 0xFFFF, cmd,
+			/* Transmit based on queue depth */
+			space = cn10k_sso_sq_depth(txq);
+			if (space <= 0)
+				return 0;
+			nb_pkts = nb_pkts < space ? nb_pkts : (uint16_t)space;
+			cn10k_nix_xmit_pkts_vector(txq, (uint64_t *)ws,
+						   mbufs + offset, nb_pkts, cmd,
 						   flags | NIX_TX_VWQE_F);
 		} else {
-			cn10k_sso_vwqe_split_tx(
-				ws, mbufs, meta & 0xFFFF, cmd, lmt_id, lmt_addr,
-				ev->sched_type, txq_data, flags);
+			nb_pkts = cn10k_sso_vwqe_split_tx(ws, mbufs + offset,
+							  nb_pkts, cmd,
+							  txq_data, flags);
+		}
+		left = (meta & 0xFFFF) - nb_pkts;
+
+		if (!left) {
+			rte_mempool_put(rte_mempool_from_obj(ev->vec), ev->vec);
+		} else {
+			*(uint64_t *)ev->vec =
+				(meta & ~0xFFFFFFFUL) |
+				(((uint32_t)nb_pkts + offset) << 16) | left;
 		}
-		rte_mempool_put(rte_mempool_from_obj(ev->vec), ev->vec);
 		rte_prefetch0(ws);
-		return 1;
+		return !left;
 	}
 
 	m = ev->mbuf;
-	txq = cn10k_sso_hws_xtract_meta(m, txq_data);
-	if (((txq->nb_sqb_bufs_adj -
-	      __atomic_load_n((int16_t *)txq->fc_mem, __ATOMIC_RELAXED))
-	     << txq->sqes_per_sqb_log2) <= 0)
-		return 0;
-	cn10k_sso_tx_one(ws, m, cmd, lmt_id, lmt_addr, ev->sched_type, txq_data,
-			 flags);
-
-	return 1;
+	return cn10k_sso_tx_one(ws, m, cmd, lmt_id, lmt_addr, ev->sched_type,
+				txq_data, flags);
 }
 
 #define T(name, sz, flags)                                                     \
-- 
2.25.1

Re: [PATCH v2 1/3] eventdev: add element offset to event vector

[Mattias Rönnblom]

On 2022-09-21 18:43, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add `rte_event_vector:elem_offset:12` bit field event vector
> structure the bits are taken from `rte_event_vector::rsvd:15`.
> The element offset defines the offset into the vector array
> at which valid elements start.
> The valid elements count will be equal to
> `rte_event_vector::nb_elem`.
> 
> Update Rx/Tx adapter SW implementation to use elem_offset.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>   v2 Changes:
>   - Revert changes to definition of `rte_event_vector::nb_elem`
> 
>   lib/eventdev/rte_event_eth_rx_adapter.c | 1 +
>   lib/eventdev/rte_event_eth_tx_adapter.c | 8 +++++---
>   lib/eventdev/rte_eventdev.h             | 6 ++++--
>   3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index bf8741d2ea..bd72f9b845 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -855,6 +855,7 @@ rxa_init_vector(struct event_eth_rx_adapter *rx_adapter,
>   	vec->vector_ev->port = vec->port;
>   	vec->vector_ev->queue = vec->queue;
>   	vec->vector_ev->attr_valid = true;
> +	vec->vector_ev->elem_offset = 0;
>   	TAILQ_INSERT_TAIL(&rx_adapter->vector_list, vec, next);
>   }
> 
> diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
> index b4b37f1cae..ccc7fffcce 100644
> --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> @@ -524,16 +524,18 @@ txa_process_event_vector(struct txa_service_data *txa,
>   		queue = vec->queue;
>   		tqi = txa_service_queue(txa, port, queue);
>   		if (unlikely(tqi == NULL || !tqi->added)) {
> -			rte_pktmbuf_free_bulk(mbufs, vec->nb_elem);
> +			rte_pktmbuf_free_bulk(&mbufs[vec->elem_offset],
> +					      vec->nb_elem);
>   			rte_mempool_put(rte_mempool_from_obj(vec), vec);
>   			return 0;
>   		}
>   		for (i = 0; i < vec->nb_elem; i++) {
>   			nb_tx += rte_eth_tx_buffer(port, queue, tqi->tx_buf,
> -						   mbufs[i]);
> +						   mbufs[i + vec->elem_offset]);
>   		}
>   	} else {
> -		for (i = 0; i < vec->nb_elem; i++) {
> +		for (i = vec->elem_offset; i < vec->elem_offset + vec->nb_elem;
> +		     i++) {
>   			port = mbufs[i]->port;
>   			queue = rte_event_eth_tx_adapter_txq_get(mbufs[i]);
>   			tqi = txa_service_queue(txa, port, queue);
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index 6a6f6ea4c1..f9fd44604e 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1060,8 +1060,10 @@ rte_event_dev_close(uint8_t dev_id);
>    */
>   struct rte_event_vector {
>   	uint16_t nb_elem;
> -	/**< Number of elements in this event vector. */
> -	uint16_t rsvd : 15;
> +	/**< Number of elements valid in this event vector. */
> +	uint16_t elem_offset : 12;
> +	/**< Offset into the vector array where valid elements start from. */
> +	uint16_t rsvd : 3;
>   	/**< Reserved for future use */
>   	uint16_t attr_valid : 1;
>   	/**< Indicates that the below union attributes have valid information.
> --
> 2.25.1

Reviewed-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>

Re: [PATCH v2 1/3] eventdev: add element offset to event vector

[Jerin Jacob]

On Thu, Sep 22, 2022 at 11:10 AM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2022-09-21 18:43, pbhagavatula@marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Add `rte_event_vector:elem_offset:12` bit field event vector
> > structure the bits are taken from `rte_event_vector::rsvd:15`.
> > The element offset defines the offset into the vector array
> > at which valid elements start.
> > The valid elements count will be equal to
> > `rte_event_vector::nb_elem`.
> >
> > Update Rx/Tx adapter SW implementation to use elem_offset.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> >   v2 Changes:
> >   - Revert changes to definition of `rte_event_vector::nb_elem`
> >

> >    */
> >   struct rte_event_vector {
> >       uint16_t nb_elem;
> > -     /**< Number of elements in this event vector. */
> > -     uint16_t rsvd : 15;
> > +     /**< Number of elements valid in this event vector. */
> > +     uint16_t elem_offset : 12;
> > +     /**< Offset into the vector array where valid elements start from. */
> > +     uint16_t rsvd : 3;
> >       /**< Reserved for future use */
> >       uint16_t attr_valid : 1;
> >       /**< Indicates that the below union attributes have valid information.
> > --
> > 2.25.1
>
> Reviewed-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>

Series applied to dpdk-next-net-eventdev/for-main. Thanks