Hi Erik,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Carrillo, Erik G
> Sent: Monday, June 10, 2019 8:02 PM
> To: Anoob Joseph <ano...@marvell.com>; Jerin Jacob Kollanukkaran
> <jer...@marvell.com>; Rao, Nikhil <nikhil....@intel.com>; Gujjar,
> Abhinandan S <abhinandan.guj...@intel.com>; Richardson, Bruce
> <bruce.richard...@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.gua...@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathr...@marvell.com>; dev@dpdk.org;
> Lukas Bartosik <lbarto...@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavat...@marvell.com>; Hemant Agrawal
> <hemant.agra...@nxp.com>; Nipun Gupta <nipun.gu...@nxp.com>; Van
> Haaren, Harry <harry.van.haa...@intel.com>; Mattias Rönnblom
> <mattias.ronnb...@ericsson.com>; Ma, Liang J <liang.j...@intel.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH 32/39] eventdev: add routine to launch
> eventmode workers
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Anoob,
>
> I've listed a few notes in-line.
>
> > -----Original Message-----
> > From: Anoob Joseph <ano...@marvell.com>
> > Sent: Monday, June 3, 2019 12:33 PM
> > To: Jerin Jacob <jer...@marvell.com>; Rao, Nikhil
> > <nikhil....@intel.com>; Carrillo, Erik G <erik.g.carri...@intel.com>;
> > Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; Richardson, Bruce
> > <bruce.richard...@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.gua...@intel.com>
> > Cc: Anoob Joseph <ano...@marvell.com>; Narayana Prasad
> > <pathr...@marvell.com>; dev@dpdk.org; Lukasz Bartosik
> > <lbarto...@marvell.com>; Pavan Nikhilesh <pbhagavat...@marvell.com>;
> > Hemant Agrawal <hemant.agra...@nxp.com>; Nipun Gupta
> > <nipun.gu...@nxp.com>; Van Haaren, Harry
> <harry.van.haa...@intel.com>;
> > Mattias Rönnblom <mattias.ronnb...@ericsson.com>; Ma, Liang J
> > <liang.j...@intel.com>
> > Subject: [PATCH 32/39] eventdev: add routine to launch eventmode
> > workers
> >
> > With eventmode, workers could be drafted differently according to the
> > capabilities of the underlying event device. The added function would
> > receive an array of such workers and probes the eventmode properties
> > to choose the worker.
> >
> > Signed-off-by: Anoob Joseph <ano...@marvell.com>
> > Signed-off-by: Lukasz Bartosik <lbarto...@marvell.com>
> > ---
>
> <...snipped...>
>
> > +
> > +/* Event mode packet tx types */
> > +enum rte_eventmode_helper_tx_types {
> > + RTE_EVETNMODE_HELPER_TX_TYPE_INVALID = 0,
>
> A couple of characters are transposed in the above.
[Anoob] Will fix it in v2.
>
> > + RTE_EVENTMODE_HELPER_TX_TYPE_MAX = 16 };
> > +
> > struct rte_eventmode_helper_conf {
> > enum rte_eventmode_helper_pkt_transfer_mode mode;
> > /**< Packet transfer mode of the application */ @@ -41,6
> > +55,20 @@ struct rte_eventmode_helper_event_link_info {
> > /**< Lcore to be polling on this port */ };
> >
>
> I believe anonymous unions and structures should be annotated with
> RTE_STD_C11 below and in other places throughout the series.
[Anoob] Will fix in v2.
>
> > +/* Workers registered by the application */ struct
> > +rte_eventmode_helper_app_worker_params {
> > + union {
> > + struct {
> > + uint64_t burst : 4;
> > + /**< Specify status of rx type burst */
> > + };
> > + uint64_t u64;
> > + } cap;
> > + /**< Capabilities of this worker */
> > + void (*worker_thread)(void *mode_conf);
> > + /**< Worker thread */
> > +};
> > +
> > /* Common helper functions for command line parsing */
> >
> > /**
> > @@ -157,6 +185,27 @@ uint8_t __rte_experimental
> > rte_eventmode_helper_get_tx_queue(struct
> rte_eventmode_helper_conf
> > *mode_conf,
> > uint8_t eventdev_id);
> >
>
> The doxygen documentation for __rte_experimental functions in general
> should have:
>
> * @warning
> * @b EXPERIMENTAL: this API may change without prior notice
>
> as well.
>
[Anoob] Will add in v2.
> > +/**
> > + * Launch eventmode worker
> > + *
> > + * The application can request the eventmode helper subsystem to
> > +launch the
> > + * worker based on the capabilities of event device and the options
> > +selected
> > + * while initializing the eventmode.
> > + *
> > + * @param mode_conf
> > + * Configuration of the mode in which app is doing packet handling
> > + * @param app_wrkr
> > + * List of all the workers registered by application, along with it's
> > + * capabilities
> > + * @param nb_wrkr_param
> > + * Number of workers passed by the application
> > + *
> > + */
> > +void __rte_experimental
> > +rte_eventmode_helper_launch_worker(struct
> > rte_eventmode_helper_conf *mode_conf,
> > + struct rte_eventmode_helper_app_worker_params
> > *app_wrkr,
> > + uint8_t nb_wrkr_param);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
>
> <...snipped...>
>
> Regards,
> Erik
