Hi Konstantin, Do you have any thoughts how both cases could be covered: 1. Inline not applied to inbound IPsec pkts for short duration of time after rte_eth_dev_start() but before sa_init() is executed (which creates SAs). 2. SAs not surviving rte_eth_dev_start() on ixgbe driver.
Thanks, Lukasz On 16.04.2020 14:28, Lukas Bartosik [C] wrote: > 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 <lbarto...@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 */ >>> >
