On Tue, Sep 05, 2017 at 02:34:44PM +0000, Van Haaren, Harry wrote:
> > From: Guduri Prathyusha [mailto:gprathyu...@caviumnetworks.com]
> > Sent: Tuesday, September 5, 2017 3:11 PM
> > To: Van Haaren, Harry <harry.van.haa...@intel.com>
> > Cc: dev@dpdk.org; Guduri Prathyusha <gprathyu...@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 1/2] service: fix service lcore stop function
> >
> > lcore_states store the state of the lcore. Fixing the invalid
> > dereference of lcore_states with service number
> >
> > Fixes: 21698354c832 ("service: introduce service cores concept")
> >
> > Signed-off-by: Guduri Prathyusha <gprathyu...@caviumnetworks.com>
>
> Hi Guduri,
Hi Harry
>
> This looks like a genuine bug-fix - thanks for looking into it and providing
> a patch. Typically, DPDK tries to fix the unit-tests and a bug at the same
> time - this ensures that the unit tests continue to pass at every test. So,
> patch 2/2 of this series can be merged into this patch, and include both
> Fixes: lines.
Sure, thanks for a quick review
>
> Secondly, does this patchset apply to the current dpdk (17.08), or to the
> future service-cores patches? If it doesn't apply cleanly on the future
> patches, it is probably better to A) include it in that patchset, or B)
> rebase this fix patch onto the service cores patches.
>
> Hope that makes sense, and let me know if you'd prefer A) or B) above.
> Thanks, -Harry
>
>
This series is based on the current 17.08 dpdk master branch. I will rebase the
patches to the future service core patches and send a v2 mentioning the patch
is dependent on the future service core patches. Is that fine?
Thanks,
Prathyusha
> > ---
> > lib/librte_eal/common/rte_service.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/common/rte_service.c
> > b/lib/librte_eal/common/rte_service.c
> > index 7efb76dc8..2ac77cc2a 100644
> > --- a/lib/librte_eal/common/rte_service.c
> > +++ b/lib/librte_eal/common/rte_service.c
> > @@ -609,7 +609,7 @@ rte_service_lcore_stop(uint32_t lcore)
> > uint32_t i;
> > for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) {
> > int32_t enabled =
> > - lcore_states[i].service_mask & (UINT64_C(1) << i);
> > + lcore_states[lcore].service_mask & (UINT64_C(1) << i);
> > int32_t service_running = rte_services[i].runstate !=
> > RUNSTATE_STOPPED;
> > int32_t only_core = rte_services[i].num_mapped_cores == 1;
> > --
> > 2.14.1
>
