[04/39] examples/l2fwd-event: move global vars to common header
Checks
Commit Message
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(-)
Comments
> -----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];
> +
> +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.
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.
> -----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.
@@ -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];
+
+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;
+
#endif /* _L2FWD_COMMON_H_ */
@@ -42,29 +42,11 @@
#include "l2fwd_common.h"
-static volatile bool force_quit;
-
-/* MAC updating enabled by default */
-static int mac_updating = 1;
-
static uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT;
static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
-/* 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];
-
static unsigned int l2fwd_rx_queue_per_lcore = 1;
-struct lcore_queue_conf lcore_queue_conf[RTE_MAX_LCORE];
-
-static struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
-
static struct rte_eth_conf port_conf = {
.rxmode = {
.split_hdr_size = 0,
@@ -76,11 +58,6 @@ static struct rte_eth_conf port_conf = {
struct rte_mempool *l2fwd_pktmbuf_pool;
-struct l2fwd_port_statistics port_statistics[RTE_MAX_ETHPORTS];
-
-/* A tsc-based timer responsible for triggering statistics printout */
-static uint64_t timer_period = 10; /* default period is 10 seconds */
-
/* Print out statistics on packets dropped */
static void
print_stats(void)
@@ -492,6 +469,20 @@ signal_handler(int signum)
}
}
+static void
+l2fwd_init_global_vars(void)
+{
+ force_quit = false;
+
+ /* MAC updating enabled by default */
+ mac_updating = 1;
+
+ /* Default period is 10 seconds */
+ timer_period = 10;
+
+ l2fwd_enabled_port_mask = 0;
+}
+
int
main(int argc, char **argv)
{
@@ -505,6 +496,9 @@ main(int argc, char **argv)
unsigned int nb_lcores = 0;
unsigned int nb_mbufs;
+ /* Set default values for global vars */
+ l2fwd_init_global_vars();
+
/* init EAL */
ret = rte_eal_init(argc, argv);
if (ret < 0)
@@ -512,7 +506,6 @@ main(int argc, char **argv)
argc -= ret;
argv += ret;
- force_quit = false;
signal(SIGINT, signal_handler);
signal(SIGTERM, signal_handler);