Re: [dpdk-dev] [PATCH v1] eventdev: update rx timestamp in mbuf using mbuf dynamic field

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Sep 16, 2021 at 2:03 PM Kundapura, Ganapati
<ganapati.kundapura@intel.com> wrote:
>
> Hi Jerrin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: 16 September 2021 10:20
> > To: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>
> > Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; dpdk-dev
> > <dev@dpdk.org>
> > Subject: Re: [PATCH v1] eventdev: update rx timestamp in mbuf using mbuf
> > dynamic field
> >
> > On Tue, Sep 14, 2021 at 12:44 PM Ganapati Kundapura
> > <ganapati.kundapura@intel.com> wrote:
> > >
> > > Add support to register timestamp dynamic field in mbuf.
> > >
> > > Update the timestamp in mbuf for each packet before enqueuing to event
> > > device if the timestamp is not already set.
> > >
> > > Adding the timestamp in Rx adapter avoids additional latency due to
> > > the event device.
> > >
> > > Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> > > ---
> > >  lib/eventdev/rte_event_eth_rx_adapter.c | 35
> > > +++++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > >
> > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > index de8ab05..9cb2550 100644
> > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > @@ -17,6 +17,7 @@
> > >  #include <rte_service_component.h>
> > >  #include <rte_thash.h>
> > >  #include <rte_interrupts.h>
> > > +#include <rte_mbuf_dyn.h>
> > >
> > >  #include "rte_eventdev.h"
> > >  #include "eventdev_pmd.h"
> > > @@ -240,6 +241,17 @@ struct eth_rx_queue_info {
> > >
> > >  static struct rte_event_eth_rx_adapter **event_eth_rx_adapter;
> > >
> > > +/* Enable dynamic timestamp field in mbuf */ uint64_t
> > > +event_eth_rx_timestamp_dynflag; int
> > > +event_eth_rx_timestamp_dynfield_offset = -1;
> > > +
> > > +static inline rte_mbuf_timestamp_t *
> > > +rte_event_eth_rx_timestamp_dynfield(struct rte_mbuf *mbuf)
> >
> > Internal functions, please avoid using rte_
> >
> > Rest looks good to me. Please send v2 as there are some patchwork failures
> > too.
> > I have rebased the next-eventdev tree. So v2 should be pass and we can
> > merge it.
> >
> > Cc: @Ferruh Yigit
> Looks like rebased next-eventdev tree is not having the latest commits in the tree.
> Patch V2 also failed to apply cleanly. V2 changes are after circular buffer changes but
> in rebased next-eventdev tree is having commit before circular buffer patch

Could you share the circular buffer patch? Also, delegate to me the
eventdev patches.
Also, Please add dependencies like the following in the patch to track.

Depends-on: series-18612 ("net/cnxk: support for inline ipsec")

> >
> >
> > > +{
> > > +       return RTE_MBUF_DYNFIELD(mbuf,
> > > +               event_eth_rx_timestamp_dynfield_offset,
> > > +rte_mbuf_timestamp_t *); }
> > > +
> > >  static inline int
> > >  rxa_validate_id(uint8_t id)
> > >  {
> > > @@ -890,8 +902,18 @@ rxa_buffer_mbufs(struct
> > rte_event_eth_rx_adapter *rx_adapter,
> > >         int do_rss;
> > >         uint16_t nb_cb;
> > >         uint16_t dropped;
> > > +       uint64_t ts, ts_mask;
> > >
> > >         if (!eth_rx_queue_info->ena_vector) {
> > > +               ts = m->ol_flags & event_eth_rx_timestamp_dynflag ?
> > > +                                               0 :
> > > + rte_get_tsc_cycles();
> > > +
> > > +               /* 0xffff ffff ffff ffff if PKT_RX_TIMESTAMP is set,
> > > +                * otherwise 0
> > > +                */
> > > +               ts_mask = (uint64_t)(!(m->ol_flags &
> > > +
> > > + event_eth_rx_timestamp_dynflag)) - 1ULL;
> > > +
> > >                 /* 0xffff ffff if PKT_RX_RSS_HASH is set, otherwise 0 */
> > >                 rss_mask = ~(((m->ol_flags & PKT_RX_RSS_HASH) != 0) - 1);
> > >                 do_rss = !rss_mask &&
> > > !eth_rx_queue_info->flow_id_mask; @@ -899,6 +921,11 @@
> > rxa_buffer_mbufs(struct rte_event_eth_rx_adapter *rx_adapter,
> > >                         struct rte_event *ev;
> > >
> > >                         m = mbufs[i];
> > > +                       *rte_event_eth_rx_timestamp_dynfield(m) =
> > > +                               ts |
> > > +                               (*rte_event_eth_rx_timestamp_dynfield(m) &
> > > +                               ts_mask);
> > > +
> > >                         ev = &buf->events[new_tail];
> > >
> > >                         rss = do_rss ? rxa_do_softrss(m,
> > > rx_adapter->rss_key_be) @@ -2256,6 +2283,14 @@
> > rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
> > >         event_eth_rx_adapter[id] = rx_adapter;
> > >         if (conf_cb == rxa_default_conf_cb)
> > >                 rx_adapter->default_cb_arg = 1;
> > > +
> > > +       if (rte_mbuf_dyn_rx_timestamp_register(
> > > +                       &event_eth_rx_timestamp_dynfield_offset,
> > > +                       &event_eth_rx_timestamp_dynflag) != 0) {
> > > +               RTE_EDEV_LOG_ERR("Error registering timestamp field in
> > mbuf\n");
> > > +               return -rte_errno;
> > > +       }
> > > +
> > >         rte_eventdev_trace_eth_rx_adapter_create(id, dev_id, conf_cb,
> > >                 conf_arg);
> > >         return 0;
> > > --
> > > 2.6.4
> > >