Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix dropping of initial IPsec pkts

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Lukasz,
 
> Hi Konstantin,
> 
> In this patch I moved the sa_init() before rte_eth_dev_start() in order to avoid dropping
> of IPsec pkts when a traffic flows and the ipsec-secgw application is started.
> 
> However I remember that during review of event mode patches you mentioned that
> moving sa_init() before rte_eth_dev_start() is an issue for one of
> the Intel drivers.

Yes, I think so.
http://mails.dpdk.org/archives/dev/2019-December/153908.html
Moving that piece of code (dev_start) after sa_init() breaks ixgbe inline-crypto support.
As I understand, because configured ipsec flows don't persist dev_start().
At least for ixgbe PMD.
Any reason why to move that code at all?


 > Is this still the case ?

AFAIK, yes. 
Thanks for bringing it to attention.
Konstantin


> 
> Thanks,
> Lukasz
> 
> On 08.04.2020 13:32, Lukasz Bartosik wrote:
> > In inline event mode when traffic flows and the ipsec-secgw
> > app is started then for short period of time IPsec packets
> > arrive at application without being decrypted and are dropped
> > by the application. This happens because eth ports are started
> > before creation of inline sessions and IPsec flows. This fix
> > rearranges the code in such a way that eth ports are always
> > started after creation of inline sessions and IPsec flows.
> >
> > Change-Id: Ifddc446082fb2897f81559517f90e1ee603e13f3
> > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > ---
> >  examples/ipsec-secgw/event_helper.c | 26 --------------------------
> >  examples/ipsec-secgw/ipsec-secgw.c  | 26 +++++++++++++-------------
> >  2 files changed, 13 insertions(+), 39 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/event_helper.c b/examples/ipsec-secgw/event_helper.c
> > index 076f1f2..da861e4 100644
> > --- a/examples/ipsec-secgw/event_helper.c
> > +++ b/examples/ipsec-secgw/event_helper.c
> > @@ -1526,7 +1526,6 @@ int32_t
> >  eh_devs_init(struct eh_conf *conf)
> >  {
> >  	struct eventmode_conf *em_conf;
> > -	uint16_t port_id;
> >  	int ret;
> >
> >  	if (conf == NULL) {
> > @@ -1558,16 +1557,6 @@ eh_devs_init(struct eh_conf *conf)
> >  	/* Display the current configuration */
> >  	eh_display_conf(conf);
> >
> > -	/* Stop eth devices before setting up adapter */
> > -	RTE_ETH_FOREACH_DEV(port_id) {
> > -
> > -		/* Use only the ports enabled */
> > -		if ((conf->eth_portmask & (1 << port_id)) == 0)
> > -			continue;
> > -
> > -		rte_eth_dev_stop(port_id);
> > -	}
> > -
> >  	/* Setup eventdev */
> >  	ret = eh_initialize_eventdev(em_conf);
> >  	if (ret < 0) {
> > @@ -1589,21 +1578,6 @@ eh_devs_init(struct eh_conf *conf)
> >  		return ret;
> >  	}
> >
> > -	/* Start eth devices after setting up adapter */
> > -	RTE_ETH_FOREACH_DEV(port_id) {
> > -
> > -		/* Use only the ports enabled */
> > -		if ((conf->eth_portmask & (1 << port_id)) == 0)
> > -			continue;
> > -
> > -		ret = rte_eth_dev_start(port_id);
> > -		if (ret < 0) {
> > -			EH_LOG_ERR("Failed to start eth dev %d, %d",
> > -				   port_id, ret);
> > -			return ret;
> > -		}
> > -	}
> > -
> >  	return 0;
> >  }
> >
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> > index 5fde4f7..e03bd89 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > @@ -2829,6 +2829,19 @@ main(int32_t argc, char **argv)
> >  	if (ret < 0)
> >  		rte_exit(EXIT_FAILURE, "eh_devs_init failed, err=%d\n", ret);
> >
> > +	/* Replicate each context per socket */
> > +	for (i = 0; i < NB_SOCKETS && i < rte_socket_count(); i++) {
> > +		socket_id = rte_socket_id_by_idx(i);
> > +		if ((socket_ctx[socket_id].mbuf_pool != NULL) &&
> > +			(socket_ctx[socket_id].sa_in == NULL) &&
> > +			(socket_ctx[socket_id].sa_out == NULL)) {
> > +			sa_init(&socket_ctx[socket_id], socket_id);
> > +			sp4_init(&socket_ctx[socket_id], socket_id);
> > +			sp6_init(&socket_ctx[socket_id], socket_id);
> > +			rt_init(&socket_ctx[socket_id], socket_id);
> > +		}
> > +	}
> > +
> >  	/* start ports */
> >  	RTE_ETH_FOREACH_DEV(portid) {
> >  		if ((enabled_port_mask & (1 << portid)) == 0)
> > @@ -2866,19 +2879,6 @@ main(int32_t argc, char **argv)
> >  			rte_exit(EXIT_FAILURE, "failed at reassemble init");
> >  	}
> >
> > -	/* Replicate each context per socket */
> > -	for (i = 0; i < NB_SOCKETS && i < rte_socket_count(); i++) {
> > -		socket_id = rte_socket_id_by_idx(i);
> > -		if ((socket_ctx[socket_id].mbuf_pool != NULL) &&
> > -			(socket_ctx[socket_id].sa_in == NULL) &&
> > -			(socket_ctx[socket_id].sa_out == NULL)) {
> > -			sa_init(&socket_ctx[socket_id], socket_id);
> > -			sp4_init(&socket_ctx[socket_id], socket_id);
> > -			sp6_init(&socket_ctx[socket_id], socket_id);
> > -			rt_init(&socket_ctx[socket_id], socket_id);
> > -		}
> > -	}
> > -
> >  	check_all_ports_link_status(enabled_port_mask);
> >
> >  	/* launch per-lcore init on every lcore */
> >