On Thu, Sep 16, 2021 at 2:05 PM Kundapura, Ganapati
<ganapati.kundap...@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjac...@gmail.com>
> > Sent: 16 September 2021 10:13
> > To: Kundapura, Ganapati <ganapati.kundap...@intel.com>
> > Cc: Jayatheerthan, Jay <jay.jayatheert...@intel.com>; dpdk-dev
> > <dev@dpdk.org>; Pavan Nikhilesh <pbhagavat...@marvell.com>
> > Subject: Re: [PATCH v3 1/3] eventdev: add rx queue info get api
> >
> > ()
> >
> > On Wed, Sep 8, 2021 at 1:51 PM Kundapura, Ganapati
> > <ganapati.kundap...@intel.com> wrote:
> > >
> > > Hi Jerin,
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjac...@gmail.com>
> > > > Sent: 07 September 2021 15:07
> > > > To: Kundapura, Ganapati <ganapati.kundap...@intel.com>
> > > > Cc: Jayatheerthan, Jay <jay.jayatheert...@intel.com>; dpdk-dev
> > > > <dev@dpdk.org>; Pavan Nikhilesh <pbhagavat...@marvell.com>
> > > > Subject: Re: [PATCH v3 1/3] eventdev: add rx queue info get api
> > > >
> > > > On Tue, Sep 7, 2021 at 2:20 PM Kundapura, Ganapati
> > > > <ganapati.kundap...@intel.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjac...@gmail.com>
> > > > > > Sent: 07 September 2021 13:42
> > > > > > To: Kundapura, Ganapati <ganapati.kundap...@intel.com>
> > > > > > Cc: Jayatheerthan, Jay <jay.jayatheert...@intel.com>; dpdk-dev
> > > > > > <dev@dpdk.org>; Pavan Nikhilesh <pbhagavat...@marvell.com>
> > > > > > Subject: Re: [PATCH v3 1/3] eventdev: add rx queue info get api
> > > > > >
> > > > > > in
> > > > > >
> > > > > > On Tue, Sep 7, 2021 at 12:15 PM Ganapati Kundapura
> > > > > > <ganapati.kundap...@intel.com> wrote:
> > > > > > >
> > > > > > > Added rte_event_eth_rx_adapter_queue_info_get() API to get rx
> > > > > > > queue information - event queue identifier, flags for handling
> > > > > > > received packets, schedular type, event priority, polling
> > > > > > > frequency of the receive queue and flow identifier in
> > > > > > > rte_event_eth_rx_adapter_queue_info structure
> > > > > > >
> > > > > > > Signed-off-by: Ganapati Kundapura
> > > > > > > <ganapati.kundap...@intel.com>
> > > > > > >
> > > > > > > ---
> > > > > > > v3:
> > > > > > > * Split single patch into implementaion, test and document
> > updation
> > > > > > > patches separately
> > > > > >
> > > > > > > +struct rte_event_eth_rx_adapter_queue_info;
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * Retrieve information about Rx queue. This callback is
> > > > > > > +invoked if
> > > > > > > + * the caps returned from the
> > > > > > > +eventdev_eth_rx_adapter_caps_get(,
> > > > > > > +eth_port_id)
> > > > > > > + * has RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT set.
> > > > > >
> > > > > > It will useful for
> > !RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT
> > > > case
> > > > > > too.
> > > > > >
> > > >
> > > >
> > > >
> > > > Missed this comment in v4
> > > Sorry I missed these comments
> > >
> > > rte_event_eth_rx_adapter_queue_info_get() calls PMD callback if
> > > internal port cap is set, otherwise it implements to return the
> > > queue_info.
> > > PMD callback is for internal port and queue_info_get() api implements
> > > for non internal port
> >
> > This API will be useful for
> > !RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT case too. In order to
> > use this in a generic way, I suggest in following way,
> >
> > In the rte_event_eth_rx_adapter_queue_add() stores this config in
> > common code(i.e
> > file: ./lib/eventdev/rte_event_eth_rx_adapter.c and
> > rte_event_eth_rx_adapter_queue_add() function) as named memzone or
> > so.
> >
> > Later when rte_event_eth_rx_adapter_queue_conf_get() called, the config
> > can be retrived through named memzone lookup and have PMD specific
> > callback like your patch for overriding any parameter as neeed. i.e If PMD
> > callback is NULL, Application should get the config struct as provided in
> > rte_event_eth_rx_adapter_queue_add() with
> > rte_event_eth_rx_adapter_queue_conf_get().
> >
> See below updated implementation
>
> que_conf_get() {
> memzone lookup
> return error on failure
>
> Fill queue config info in queue conf structure
> if CAP_INTERNAL_PORT set and PMD call back is non NULL
IMO, We don't need to check CAP_INTERNAL_PORT. Rest looks good to me.
Also update the test case check this path(case where PMD callback is NULL)
> call the PMD callback which overrides the info in queue conf
> structure
>
> return 0; // queue config info is returned in passed queue_conf structure
> }
>
> rx adapter structures are allocated in memzone, rx_queue_add() stores
> the queue config info in named memzone and queue_conf_get() returns queue
> config
> info from the named memzone.
>
> Please let me know your inputs on this
> >
> >
> > >
> > > > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > > b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > > index 182dd2e..75c0010 100644
> > > > > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > > @@ -33,6 +33,7 @@
> > > > > > > * - rte_event_eth_rx_adapter_stop()
> > > > > > > * - rte_event_eth_rx_adapter_stats_get()
> > > > > > > * - rte_event_eth_rx_adapter_stats_reset()
> > > > > > > + * - rte_event_eth_rx_adapter_queue_info_get()
> > > > > > > *
> > > > > > > * The application creates an ethernet to event adapter using
> > > > > > > * rte_event_eth_rx_adapter_create_ext() or
> > > > > > > rte_event_eth_rx_adapter_create() @@ -140,6 +141,56 @@
> > typedef
> > > > int
> > > > > > (*rte_event_eth_rx_adapter_conf_cb) (uint8_t id, uint8_t dev_id,
> > > > > > > void *arg);
> > > > > > >
> > > > > > > /**
> > > > > > > + * Rx queue info
> > > > > > > + */
> > > > > > > +struct rte_event_eth_rx_adapter_queue_info {
> > > > > >
> > > > > > Can we avoid the duplication of this structure and use
> > > > > > rte_event_eth_rx_adapter_queue_conf instead.
> > > > > >
> > > Agree
> > > > > > API can be rte_event_eth_rx_adapter_queue_conf_get() to align
> > > > > > the structure.
> > > Agree
> > > > > >
> > > > > > Also instead of every driver duplicating this code, How about
> > > > > > - common code stores the config in
> > > > > > rte_event_eth_rx_adapter_queue_add()
> > > > > > - common code stores the config in
> > > > > > rte_event_eth_rx_adapter_queue_conf_get()
> > >
> > > queue_add() stores the config in dev_info and queue_conf_get() retrieves
> > the config from dev_info.
> > > Please clarify on common code to store and retrieve queue conf?
> >
> >
> > See above.
> >
> >
> > > > > > - Addtional PMD level API can be given incase, something needs
> > > > > > to overridden by Adapter.
> > > >
> > > Existing PMD callbacks like queue_add, queue_del, adapter_start,
> > > adapter_stop etc doesn't have any additional PMD level api.
> > > queue_info_get PMD callback is also similar.
> >
> >
> > See above.
> >
> >
> >
> > >
> > > >
> > > > Missed addressing this comment in v4.
