> On 2022-08-16 17:49, pbhagavat...@marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavat...@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 <pbhagavat...@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
> >
