[dpdk-dev] [PATCH 14/14] examples/ipsec-secgw: add cmd line option for bufs

Anoob Joseph anoobj at marvell.com
Fri Jan 3 06:42:23 CET 2020


Hi Konstantin,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: dev <dev-bounces at dpdk.org> On Behalf Of Ananyev, Konstantin
> Sent: Monday, December 23, 2019 9:47 PM
> To: Anoob Joseph <anoobj at marvell.com>; Akhil Goyal
> <akhil.goyal at nxp.com>; Nicolau, Radu <radu.nicolau at intel.com>; Thomas
> Monjalon <thomas at monjalon.net>
> Cc: Lukas Bartosik <lbartosik at marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj at marvell.com>; Narayana Prasad Raju Athreya
> <pathreya at marvell.com>; Ankur Dwivedi <adwivedi at marvell.com>;
> Archana Muniganti <marchana at marvell.com>; Tejasree Kondoj
> <ktejasree at marvell.com>; Vamsi Krishna Attunuru
> <vattunuru at marvell.com>; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 14/14] examples/ipsec-secgw: add cmd line
> option for bufs
> 
> 
> 
> > > Add command line option -s which can be used to configure number of
> > > buffers in a pool. Default number of buffers is 8192.
> > >
> > > Signed-off-by: Anoob Joseph <anoobj at marvell.com>
> > > Signed-off-by: Lukasz Bartosik <lbartosik at marvell.com>
> > > ---
> > >  examples/ipsec-secgw/ipsec-secgw.c | 23 +++++++++++++++++++----
> > >  1 file changed, 19 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> > > b/examples/ipsec-secgw/ipsec-secgw.c
> > > index 76719f2..f8e28d6 100644
> > > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > > @@ -59,8 +59,6 @@ volatile bool force_quit;
> > >
> > >  #define MEMPOOL_CACHE_SIZE 256
> > >
> > > -#define NB_MBUF	(32000)
> > > -
> > >  #define CDEV_QUEUE_DESC 2048
> > >  #define CDEV_MAP_ENTRIES 16384
> > >  #define CDEV_MP_NB_OBJS 1024
> > > @@ -167,6 +165,7 @@ static int32_t numa_on = 1; /**< NUMA is
> enabled
> > > by default. */  static uint32_t nb_lcores;  static uint32_t
> > > single_sa;  static uint32_t single_sa_idx;
> > > +static uint32_t nb_bufs_in_pool = 8192;
> >
> > Why to change the default number (behavior) here?
> > Why not to keep existing one as default?
> 
> Or, at least try to guess required number of mbufs (like l3fwd, etc., do)?

[Anoob] Existing code sets the default number of mbufs to 32k, which is leading to higher cache misses on our platform. Also, other example applications have 8192 as the minimum. Hence the change.

Do you see any perf issues with lowering the default value? Also, I'm fine with making the default one same as the ones in l2fwd & l3fwd.

>From l3fwd:

/*
 * This expression is used to calculate the number of mbufs needed
 * depending on user input, taking  into account memory for rx and
 * tx hardware rings, cache per lcore and mtable per port per lcore.
 * RTE_MAX is used to ensure that NB_MBUF never goes below a minimum
 * value of 8192
 */
#define NB_MBUF(nports) RTE_MAX(	\
	(nports*nb_rx_queue*nb_rxd +		\
	nports*nb_lcores*MAX_PKT_BURST +	\
	nports*n_tx_queue*nb_txd +		\
	nb_lcores*MEMPOOL_CACHE_SIZE),		\
	(unsigned)8192)

I do understand that we will have to rework the above logic a bit more to handle the in-flight packets in cryptodev. What's your suggestion?
  
> 
> >
> > >
> > >  /*
> > >   * RX/TX HW offload capabilities to enable/use on ethernet ports.
> > > @@ -1261,6 +1260,7 @@ print_usage(const char *prgname)
> > >  		" [-w REPLAY_WINDOW_SIZE]"
> > >  		" [-e]"
> > >  		" [-a]"
> > > +		" [-s NUMBER_OF_MBUFS_IN_PKT_POOL]"
> > >  		" -f CONFIG_FILE"
> > >  		" --config (port,queue,lcore)[,(port,queue,lcore)]"
> > >  		" [--single-sa SAIDX]"
> > > @@ -1284,6 +1284,7 @@ print_usage(const char *prgname)
> > >  		"     size for each SA\n"
> > >  		"  -e enables ESN\n"
> > >  		"  -a enables SA SQN atomic behaviour\n"
> > > +		"  -s number of mbufs in packet pool (default 8192)\n"
> > >  		"  -f CONFIG_FILE: Configuration file\n"
> > >  		"  --config (port,queue,lcore): Rx queue configuration\n"
> > >  		"  --single-sa SAIDX: Use single SA index for outbound
> traffic,\n"
> > > @@ -1534,7 +1535,7 @@ parse_args(int32_t argc, char **argv, struct
> > > eh_conf *eh_conf)
> > >
> > >  	argvopt = argv;
> > >
> > > -	while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:",
> > > +	while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:s:",
> > >  				lgopts, &option_index)) != EOF) {
> > >
> > >  		switch (opt) {
> > > @@ -1568,6 +1569,19 @@ parse_args(int32_t argc, char **argv, struct
> eh_conf *eh_conf)
> > >  			cfgfile = optarg;
> > >  			f_present = 1;
> > >  			break;
> > > +
> > > +		case 's':
> > > +			ret = parse_decimal(optarg);
> > > +			if (ret < 0) {
> > > +				printf("Invalid number of buffers in a pool: "
> > > +					"%s\n", optarg);
> > > +				print_usage(prgname);
> > > +				return -1;
> > > +			}
> > > +
> > > +			nb_bufs_in_pool = ret;
> > > +			break;
> > > +
> > >  		case 'j':
> > >  			ret = parse_decimal(optarg);
> > >  			if (ret < RTE_MBUF_DEFAULT_BUF_SIZE || @@ -
> 2792,11 +2806,12 @@
> > > main(int32_t argc, char **argv)
> > >  		if (socket_ctx[socket_id].mbuf_pool)
> > >  			continue;
> > >
> > > -		pool_init(&socket_ctx[socket_id], socket_id, NB_MBUF);
> > > +		pool_init(&socket_ctx[socket_id], socket_id,
> nb_bufs_in_pool);
> > >  		session_pool_init(&socket_ctx[socket_id], socket_id,
> sess_sz);
> > >  		session_priv_pool_init(&socket_ctx[socket_id], socket_id,
> > >  			sess_sz);
> > >  	}
> > > +	printf("Number of mbufs in packet pool %d\n", nb_bufs_in_pool);
> > >
> > >  	RTE_ETH_FOREACH_DEV(portid) {
> > >  		if ((enabled_port_mask & (1 << portid)) == 0)
> > > --
> > > 2.7.4



More information about the dev mailing list