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

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: Anoob Joseph
> Sent: Friday, June 7, 2019 4:15 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: 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
> 
> Hi Jerin,

Hi Anoob,

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

OK

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

I think, we can avoid global variable to communicate between
Workers. I think, we don't need worry about diff between l2fwd and l2fwd-event.
If worried about maintaining the patches splitting then I can think we can
squash a few l2fwd app patches if required to add the context structure.
It will be scalable approach.