On Fri, Jan 20, 2023 at 4:03 PM Naga Harish K, S V
<s.v.naga.haris...@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjac...@gmail.com>
> > Sent: Friday, January 20, 2023 3:02 PM
> > To: Naga Harish K, S V <s.v.naga.haris...@intel.com>
> > Cc: jer...@marvell.com; Gujjar, Abhinandan S
> > <abhinandan.guj...@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > <jay.jayatheert...@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.haris...@intel.com> wrote:
> > >
> > > Hi Jerin,
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjac...@gmail.com>
> > > > Sent: Wednesday, January 18, 2023 3:52 PM
> > > > To: Naga Harish K, S V <s.v.naga.haris...@intel.com>
> > > > Cc: jer...@marvell.com; Gujjar, Abhinandan S
> > > > <abhinandan.guj...@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > <jay.jayatheert...@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.haris...@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.haris...@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.
OK. Makes sense. Please send next version with remaining suggestions.
>
> > >
> > > > 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
