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

["Lukas Bartosik [C]" <lbartosik@marvell.com>]

Hi Konstantin,

Please see my answer below.

Thanks,
Lukasz

On 16.04.2020 01:47, Ananyev, Konstantin wrote:
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> 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.
> https://urldefense.proofpoint.com/v2/url?u=http-3A__mails.dpdk.org_archives_dev_2019-2DDecember_153908.html&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=SchRHhE7GLCjEY4i2a1byjC_FpWgRLtq4-kLvKp3_84&m=w3xh94Ox4xIhabfE-nD2VbEWbh2JTmiscVMb6pJZcYo&s=9rDtRPGK2QBDAcY8VQf0HQzXINtQzucwIxU7DB2ND5s&e= 
> 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?
> 

[Lukasz] We're observing issue in inline mode. When traffic flows and ipsec-secgw application is started then
for short period of time inline is not applied by HW and IPsec packets reach the application. This is because
sa_init() (which creates security associations SAs for HW) is executed after rte_eth_dev_start().
That's the reason I moved the code. And that movement fixes the issue because now SAs are already
created when eth ports are started.

Would it be possible to fix the ixgbe so that SAs would survive rte_eth_dev_start() ?
Do you have any other idea how we could cover both cases ?

>  > 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 */
>>>