Hi Jerin,
Please see the replies inline.
> -----Original Message-----
> From: Jerin Jacob <jerinjac...@gmail.com>
> Sent: Monday, September 20, 2021 11:50 AM
> To: Naga Harish K, S V <s.v.naga.haris...@intel.com>
> Cc: Jerin Jacob <jer...@marvell.com>; Jayatheerthan, Jay
> <jay.jayatheert...@intel.com>; dpdk-dev <dev@dpdk.org>; Kundapura,
> Ganapati <ganapati.kundap...@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/5] eventdev: rx_adapter: add support
> to configure event buffer size
>
> On Sat, Sep 18, 2021 at 6:41 PM Naga Harish K S V
> <s.v.naga.haris...@intel.com> wrote:
> >
> > Currently Rx event buffer is static array with a default size of
> > 192(6*BATCH_SIZE).
> >
> > ``rte_event_eth_rx_adapter_create2`` api is added which takes ``struct
> > rte_event_eth_rx_adapter_params`` to configure event buffer size in
> > addition other params . The event buffer is allocated dynamically at
> > run time aligned to BATCH_SIZE + 2*BATCH_SIZE.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.haris...@intel.com>
> > Signed-off-by: Ganapati Kundapura <ganapati.kundap...@intel.com>
> > ---
> >
> > +/**
> > + * A structure to hold adapter config params */ struct
> > +rte_event_eth_rx_adapter_params {
> > + uint16_t event_buf_size;
> > + /**< size of event buffer for the adapter */
>
> See below.
>
> > +};
> > +
> > /**
> > *
> > * Callback function invoked by the SW adapter before it continues @@
> > -330,6 +339,40 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id,
> uint8_t dev_id,
> > rte_event_eth_rx_adapter_conf_cb conf_cb,
> > void *conf_arg);
> >
> > +/**
> > + * Create a new ethernet Rx event adapter with the specified identifier.
> > + * This function allocates Rx adapter event buffer with the size
> > +specified
> > + * in rxa_params aligned to BATCH_SIZE plus (BATCH_SIZE+BATCH_SIZE)
> > +and
> > + * uses an internal configuration function that creates an event port.
>
> This function may use for adding another
> rte_event_eth_rx_adapter_params:: value.
> So semantics of rte_event_eth_rx_adapter_params::event_buf_size you
> can document at in that structure. This function, you can tell it adapter
> creation varint with parameters or so See below.
>
The documentation is updated as as per the review comments.
> > + * This default function reconfigures the event device with an
> > + * additional event port and setups up the event port using the port
> > + config
> > + * parameter passed into this function. In case the application needs
> > + more
> > + * control in configuration of the service, it should use the
> > + * rte_event_eth_rx_adapter_create_ext() version.
> > + *
> > + * @param id
> > + * The identifier of the ethernet Rx event adapter.
> > + *
> > + * @param dev_id
> > + * The identifier of the event device to configure.
> > + *
> > + * @param rxa_params
> > + * Pointer to struct rte_event_eth_rx_adapter_params containing
> > + * size to allocate rx event buffer.
>
> Value NULL is allowed to represent the default values or so.
>
The api is updated to treat NULL pointer for adapter params with default values.
> > + *
> > + * @param port_config
> > + * Argument of type *rte_event_port_conf* that is passed to the
> > +conf_cb
> > + * function.
> > + *
> > + * @return
> > + * - 0: Success
> > + * - <0: Error code on failure
> > + */
> > +__rte_experimental
> > +int rte_event_eth_rx_adapter_create2(uint8_t id, uint8_t dev_id,
> > + struct rte_event_eth_rx_adapter_params *rxa_params,
> > + struct rte_event_port_conf *port_config);
>
> Couple of suggestion on API name and prototype:
> - I think, we can remove 2 version and give more meaningful,name like
> rte_event_eth_rx_adapter_create_with_param() or so
>
> - Keep new parameter as last to have better compatibility i.e
> rte_event_eth_rx_adapter_create_with_param(uint8_t id, uint8_t dev_id,
> struct rte_event_port_conf *port_config, struct
> rte_event_eth_rx_adapter_params *rxa_params)
The function name and parameters are adjusted as suggested.
Regards
Harish
