RE: [PATCH 1/3] eventdev/eth_rx: add params set/get APIs

["Naga Harish K, S V" <s.v.naga.harish.k@intel.com>]

Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, January 20, 2023 3:02 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH 1/3] eventdev/eth_rx: add params set/get APIs
> 
> On Fri, Jan 20, 2023 at 2:28 PM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > Hi Jerin,
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, January 18, 2023 3:52 PM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH 1/3] eventdev/eth_rx: add params set/get APIs
> > >
> > > On Sat, Jan 7, 2023 at 9:49 PM Naga Harish K S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > > The adapter configuration parameters defined in the ``struct
> > > > rte_event_eth_rx_adapter_config_params`` can be configured and
> > > > retrieved using ``rte_event_eth_rx_adapter_set_params`` and
> > > > ``rte_event_eth_tx_adapter_get_params`` respectively.
> > > >
> > > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > > > ---
> > > >
> > > > +/**
> > > > + * Adapter configuration parameters  */ struct
> > > > +rte_event_eth_rx_adapter_runtime_params {
> > > > +       uint32_t max_nb_rx;
> > > > +       /**< The adapter can return early if it has processed at least
> > > > +        * max_nb_rx mbufs. This isn't treated as a requirement; batching
> may
> > > > +        * cause the adapter to process more than max_nb_rx mbufs.
> > >
> > > This parameter as specific to SW only driver. So future something
> > > added from HW only driver item then it won't work for SW driver. So
> > > we need capability per adapter.
> > >
> > > So,  I would suggest following theme across _all_ adapters.
> > >
> > > 1) Introduce RTE_EVENT_ETH_RX_ADAPTER_CAP_RUNTIME_XYZ and
> associate
> > > each parameter(the one we think, it is not common for all
> > > adapters)
> >
> > The parameters that are exposed in the patch are all existing
> > parameters and they are made runtime configurable for SW
> > implementation. I think, there are no such parameters existing today
> > for HW driver implementation. Hence it may be better to introduce these
> flags when the HW driver Implementation requires runtime configurable
> parameters.
> 
> Since current values are not applicable to HW. So we any way need the
> capability now to tell this is not applicable for HW.
> 

Depending on the existing adapter capability flag "RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT",
the current values can be applied to only SW implementation. In this way, there is no need for
creating new capability flags.

> >
> > > 2) Add some reserved fields in
> > > rte_event_eth_rx_adapter_runtime_params
> > > so that we don't break ABI in future
> >
> > Agreed.
> >
> > > 3) Add rte_event_eth_rx_adapter_runtime_params_init() function just
> > > make structure fill with default to avoid ABI break
> > > 4) Add rte_event_eth_rx_adapter_runtime_params_info_get(). Lets
> > > capability flags and other items can be return via this
> >
> > These two items(3,4) can be taken as and when item "1" above is
> implemented.
> 
> See above.
> 
> >
> > > 5) Change rte_event_eth_rx_adapter_set_params as
> > > rte_event_eth_rx_adapter_runtime_set()  or
> > > rte_event_eth_rx_adapter_runtime_params_set() to make it runtime
> > > explicit
> >
> > Agreed
> >
> > > 6) Change rte_event_eth_rx_adapter_get_params as
> > > rte_event_eth_rx_adapter_runtime_get() or
> > > rte_event_eth_rx_adapter_runtime_params_get()  to make it runtime
> > > explicit
> >
> > Agreed