Re: [dpdk-dev] [PATCH 04/39] examples/l2fwd-event: move global vars to common header

[Anoob Joseph <anoobj@marvell.com>]

Hi Jerin,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran
> Sent: Friday, June 7, 2019 3:33 PM
> To: Anoob Joseph <anoobj@marvell.com>; Nikhil Rao <nikhil.rao@intel.com>;
> Erik Gabriel Carrillo <erik.g.carrillo@intel.com>; Abhinandan Gujjar
> <abhinandan.gujjar@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; dev@dpdk.org; Lukas Bartosik
> <lbartosik@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Nipun Gupta <nipun.gupta@nxp.com>; Harry van Haaren
> <harry.van.haaren@intel.com>; Mattias Rönnblom
> <mattias.ronnblom@ericsson.com>; Liang Ma <liang.j.ma@intel.com>
> Subject: RE: [PATCH 04/39] examples/l2fwd-event: move global vars to common
> header
> 
> > -----Original Message-----
> > From: Anoob Joseph <anoobj@marvell.com>
> > Sent: Monday, June 3, 2019 11:02 PM
> > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nikhil Rao
> > <nikhil.rao@intel.com>; Erik Gabriel Carrillo
> > <erik.g.carrillo@intel.com>; Abhinandan Gujjar
> > <abhinandan.gujjar@intel.com>; Bruce Richardson
> > <bruce.richardson@intel.com>; Pablo de Lara
> > <pablo.de.lara.guarch@intel.com>
> > Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad Raju Athreya
> > <pathreya@marvell.com>; dev@dpdk.org; Lukas Bartosik
> > <lbartosik@marvell.com>; Pavan Nikhilesh Bhagavatula
> > <pbhagavatula@marvell.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>;
> > Nipun Gupta <nipun.gupta@nxp.com>; Harry van Haaren
> > <harry.van.haaren@intel.com>; Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com>; Liang Ma <liang.j.ma@intel.com>
> > Subject: [PATCH 04/39] examples/l2fwd-event: move global vars to
> > common header
> >
> > Moving global variables to common header for access from control plane
> > and data plane code.
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > ---
> >  examples/l2fwd-event/l2fwd_common.h | 26
> > +++++++++++++++++++++++
> >  examples/l2fwd-event/main.c         | 41 +++++++++++++++---------------------
> > -
> >  2 files changed, 43 insertions(+), 24 deletions(-)
> >
> > diff --git a/examples/l2fwd-event/l2fwd_common.h b/examples/l2fwd-
> > event/l2fwd_common.h index a7bb5af..55226f7 100644
> > --- a/examples/l2fwd-event/l2fwd_common.h
> > +++ b/examples/l2fwd-event/l2fwd_common.h
> > @@ -5,6 +5,10 @@
> >  #ifndef _L2FWD_COMMON_H_
> >  #define _L2FWD_COMMON_H_
> >
> > +#include <stdbool.h>
> > +
> > +#include <rte_common.h>
> > +
> >  #define RTE_LOGTYPE_L2FWD RTE_LOGTYPE_USER1
> >
> >  #define MAX_PKT_BURST 32
> > @@ -34,4 +38,26 @@ struct l2fwd_port_statistics {
> >  	uint64_t dropped;
> >  } __rte_cache_aligned;
> >
> > +volatile bool force_quit;
> > +
> > +int mac_updating;
> > +
> > +/* ethernet addresses of ports */
> > +static struct rte_ether_addr l2fwd_ports_eth_addr[RTE_MAX_ETHPORTS];
> > +
> > +/* mask of enabled ports */
> > +static uint32_t l2fwd_enabled_port_mask;
> > +
> > +/* list of enabled ports */
> > +static uint32_t l2fwd_dst_ports[RTE_MAX_ETHPORTS];
> > +

[Anoob] Static has to be removed from all the above vars. Will fix in the next version.

> > +struct lcore_queue_conf lcore_queue_conf[RTE_MAX_LCORE];
> > +
> > +struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
> > +
> > +struct l2fwd_port_statistics port_statistics[RTE_MAX_ETHPORTS];
> > +
> > +/* A tsc-based timer responsible for triggering statistics printout
> > +*/ uint64_t timer_period;
> 
> Instead of moving global variables to other header file, IMO, it is better to
> create a structure with context and share with workers with
> rte_eal_mp_remote_launch() or so.

[Anoob] That would make the design a bit different from regular l2fwd. Even in l2fwd, all these variable are present. Be it global or static.

Another option is to have the vars declared as extern in the l2fwd_worker.c and remove the additions in the header. If that approach is fine, we can keep the changes between l2fwd & l2fwd-event minimal. Please share your thoughts on which approach would be better.