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

Reply via email to