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

[Jerin Jacob <jerinjacobk@gmail.com>]

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.

>
> > 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