Hi Konstantin,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.anan...@intel.com>
> Sent: Tuesday, December 24, 2019 12:18 AM
> To: Anoob Joseph <ano...@marvell.com>; Akhil Goyal <akhil.go...@nxp.com>;
> Nicolau, Radu <radu.nico...@intel.com>; Thomas Monjalon
> <tho...@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Narayana Prasad Raju
> Athreya <pathr...@marvell.com>; Ankur Dwivedi <adwiv...@marvell.com>;
> Archana Muniganti <march...@marvell.com>; Tejasree Kondoj
> <ktejas...@marvell.com>; Vamsi Krishna Attunuru <vattun...@marvell.com>;
> Lukas Bartosik <lbarto...@marvell.com>; dev@dpdk.org
> Subject: [EXT] RE: [PATCH 04/14] examples/ipsec-secgw: add Rx adapter
> support
> 
> External Email
> 
> ----------------------------------------------------------------------
> > Add Rx adapter support. The event helper init routine will initialize
> > the Rx adapter according to the configuration. If Rx adapter config is
> > not present it will generate a default config. It will check the
> > available eth ports and event queues and map them 1:1. So one eth port
> > will be connected to one event queue. This way event queue ID could be
> > used to figure out the port on which a packet came in.
> >
> > Signed-off-by: Anoob Joseph <ano...@marvell.com>
> > Signed-off-by: Lukasz Bartosik <lbarto...@marvell.com>
> > ---
> >  examples/ipsec-secgw/event_helper.c | 289
> > +++++++++++++++++++++++++++++++++++-
> >  examples/ipsec-secgw/event_helper.h |  29 ++++
> >  2 files changed, 317 insertions(+), 1 deletion(-)
> >
> > diff --git a/examples/ipsec-secgw/event_helper.c
> > b/examples/ipsec-secgw/event_helper.c
> > index d0157f4..f0eca01 100644
> > --- a/examples/ipsec-secgw/event_helper.c
> > +++ b/examples/ipsec-secgw/event_helper.c
> > @@ -4,10 +4,60 @@
> >  #include <rte_bitmap.h>
> >  #include <rte_ethdev.h>
> >  #include <rte_eventdev.h>
> > +#include <rte_event_eth_rx_adapter.h>
> >  #include <rte_malloc.h>
> >
> >  #include "event_helper.h"
> >
> > +static int
> > +eh_get_enabled_cores(struct rte_bitmap *eth_core_mask) {
> > +   int i;
> > +   int count = 0;
> > +
> > +   RTE_LCORE_FOREACH(i) {
> > +           /* Check if this core is enabled in core mask*/
> > +           if (rte_bitmap_get(eth_core_mask, i)) {
> > +                   /* We have found enabled core */
> > +                   count++;
> > +           }
> > +   }
> > +   return count;
> > +}
> > +
> > +static inline unsigned int
> > +eh_get_next_eth_core(struct eventmode_conf *em_conf) {
> > +   static unsigned int prev_core = -1;
> > +   unsigned int next_core;
> > +
> > +   /*
> > +    * Make sure we have at least one eth core running, else the following
> > +    * logic would lead to an infinite loop.
> > +    */
> > +   if (eh_get_enabled_cores(em_conf->eth_core_mask) == 0) {
> > +           EH_LOG_ERR("No enabled eth core found");
> > +           return RTE_MAX_LCORE;
> > +   }
> > +
> > +get_next_core:
> > +   /* Get the next core */
> > +   next_core = rte_get_next_lcore(prev_core, 0, 1);
> > +
> > +   /* Check if we have reached max lcores */
> > +   if (next_core == RTE_MAX_LCORE)
> > +           return next_core;
> > +
> > +   /* Update prev_core */
> > +   prev_core = next_core;
> > +
> > +   /* Only some cores are marked as eth cores. Skip others */
> > +   if (!(rte_bitmap_get(em_conf->eth_core_mask, next_core)))
> > +           goto get_next_core;
> 
> Are loops statements forbidden in C now? 😉
> As a generic comment - too many (unnecessary) gotos in this patch series.
> It is not uncommon to see 2-3 labels inside the function and bunch gotos to
> them.
> Would be good to rework the code a bit to get rid of them.

[Anoob] Sure. Will rework the code and see if the gotos can be minimized. In 
this case, it seemed more straightforward to have goto instead of the loop. 
Will recheck anyway.
 
> 
> > +
> > +   return next_core;
> > +}
> > +
> >  static inline unsigned int
> >  eh_get_next_active_core(struct eventmode_conf *em_conf, unsigned int
> > prev_core)  { @@ -154,6 +204,87 @@ eh_set_default_conf_link(struct
> > eventmode_conf *em_conf)  }
> >

Reply via email to