[ovs-dev] [PATCH] netdev-dpdk: Allow changing NON_PMD_CORE_ID for testing purpose.
Traynor, Kevin
kevin.traynor at intel.com
Thu Feb 5 18:29:44 UTC 2015
Hi,
I've done some quick testing on this patch for different values of NON_PMD_CORE_ID and it looks to be working fine.
The only issue I've seen is that I'm isolcpus for all cores except core 0, so when I change the NON_PMD_CORE_ID to non-zero, the pmd is being scheduled on core 0 also and throughput suffers depending on system load.
I'd suggest adding a comment beside the #define to warn that this value affects where the pmd is scheduled, and it may be on a non-isolated core.
Thanks,
Kevin.
> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Daniele Di Proietto
> Sent: Thursday, February 5, 2015 12:58 AM
> To: Alex Wang
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Allow changing NON_PMD_CORE_ID for testing purpose.
>
> Hey Alex,
>
> I remember that calling certain DPDK functions (that we use to
> initialize devices) with '_lcore_id !=0' resulted in an error. If this
> is not the case anymore with DPDK 1.7.1 (i.e. if you tested this patch
> with NON_PMD_CORE_ID!=0 and it worked), then I have no objection.
>
> I would test this myself, but I don't have access to a DPDK capable
> system right now. If you want to be more sure about this you can build
> DPDK with CONFIG_RTE_LIBRTE_MBUF_DEBUG=y and watch for failed
> assertions.
>
> Last thing: we could also avoid using the _lcore_id to set the
> affinity of a thread (e.g. a thread with '_lcore_id == 1' doesn't
> necessarily need to be pinned to cpu 1). This would be another way to
> achieve the same goal, but I prefer your approach, because it is more
> consistent with DPDK internals.
>
> Hope this helps. Let me know if there's anything else about this
>
> Thanks,
>
> Daniele
>
> 2015-02-05 0:14 GMT+01:00 Alex Wang <alexw at nicira.com>:
> > Hey Daniele,
> >
> > Do you still remember why you mentioned:
> >
> > "/* We have to use 0 to allow non pmd threads to perform certain DPDK
> > * operations, like rte_eth_dev_configure(). */
> > "
> > in your commit: db73f716 (netdev-dpdk: Fix race condition with DPDK
> > mempools in non pmd threads)
> >
> > This posted commit works during my manual test. And the
> > rte_eth_dev_configure() in dpdk-1.7.1 does not require the caller lcore id
> > to
> > be 0.
> >
> > But Pravin mentioned that you may know more about why use 0 for non-pmd
> > threads (to prevent crash?).
> >
> > Could you share some thoughts?
> >
> > Thanks,
> > Alex Wang,
> >
> > On Wed, Feb 4, 2015 at 2:14 PM, Pravin Shelar <pshelar at nicira.com> wrote:
> >>
> >> On Tue, Feb 3, 2015 at 5:54 PM, Alex Wang <alexw at nicira.com> wrote:
> >> > For testing purpose, developers may want to change the NON_PMD_CORE_ID
> >> > and use a different core for non-pmd threads. Since the netdev-dpdk
> >> > module is hard-coded to assert the non-pmd threads using core 0, such
> >> > change will cause abortion of OVS.
> >> >
> >> > This commit fixes the assertion and allows changing NON_PMD_CORE_ID.
> >> >
> >> > Signed-off-by: Alex Wang <alexw at nicira.com>
> >> > ---
> >> > lib/dpctl.c | 2 +-
> >> > lib/dpif-netdev.h | 1 -
> >> > lib/netdev-dpdk.c | 12 ++++++------
> >> > lib/netdev-dpdk.h | 2 ++
> >> > 4 files changed, 9 insertions(+), 8 deletions(-)
> >> >
> >> > diff --git a/lib/dpctl.c b/lib/dpctl.c
> >> > index 4c2614b..125023c 100644
> >> > --- a/lib/dpctl.c
> >> > +++ b/lib/dpctl.c
> >> > @@ -31,11 +31,11 @@
> >> > #include "dirs.h"
> >> > #include "dpctl.h"
> >> > #include "dpif.h"
> >> > -#include "dpif-netdev.h"
> >> > #include "dynamic-string.h"
> >> > #include "flow.h"
> >> > #include "match.h"
> >> > #include "netdev.h"
> >> > +#include "netdev-dpdk.h"
> >> > #include "netlink.h"
> >> > #include "odp-util.h"
> >> > #include "ofp-parse.h"
> >> > diff --git a/lib/dpif-netdev.h b/lib/dpif-netdev.h
> >> > index d811507..410fcfa 100644
> >> > --- a/lib/dpif-netdev.h
> >> > +++ b/lib/dpif-netdev.h
> >> > @@ -42,7 +42,6 @@ static inline void dp_packet_pad(struct ofpbuf *b)
> >> >
> >> > #define NR_QUEUE 1
> >> > #define NR_PMD_THREADS 1
> >> > -#define NON_PMD_CORE_ID 0
> >> >
> >> > #ifdef __cplusplus
> >> > }
> >> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >> > index 0ede200..391695f 100644
> >> > --- a/lib/netdev-dpdk.c
> >> > +++ b/lib/netdev-dpdk.c
> >> > @@ -1553,8 +1553,8 @@ pmd_thread_setaffinity_cpu(int cpu)
> >> > VLOG_ERR("Thread affinity error %d",err);
> >> > return err;
> >> > }
> >> > - /* lcore_id 0 is reseved for use by non pmd threads. */
> >> > - ovs_assert(cpu);
> >> > + /* NON_PMD_CORE_ID is reserved for use by non pmd threads. */
> >> > + ovs_assert(cpu != NON_PMD_CORE_ID);
> >>
> >> > RTE_PER_LCORE(_lcore_id) = cpu;
> >> >
> >> > return 0;
> >> > @@ -1563,13 +1563,13 @@ pmd_thread_setaffinity_cpu(int cpu)
> >> > void
> >> > thread_set_nonpmd(void)
> >> > {
> >> > - /* We have to use 0 to allow non pmd threads to perform certain
> >> > DPDK
> >> > - * operations, like rte_eth_dev_configure(). */
> >> > - RTE_PER_LCORE(_lcore_id) = 0;
> >> > + /* We have to use NON_PMD_CORE_ID to allow non-pmd threads to
> >> > perform
> >> > + * certain DPDK operations, like rte_eth_dev_configure(). */
> >>
> >>
> >> The is not equivalent comment. have you confirmed that
> >> rte_eth_dev_configure() works on any core?
> >>
> >> > + RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
> >> > }
> >> >
> >> > static bool
> >> > thread_is_pmd(void)
> >> > {
> >> > - return rte_lcore_id() != 0;
> >> > + return rte_lcore_id() != NON_PMD_CORE_ID;
> >> > }
> >> > diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
> >> > index c24d6da..9a47165 100644
> >> > --- a/lib/netdev-dpdk.h
> >> > +++ b/lib/netdev-dpdk.h
> >> > @@ -5,6 +5,8 @@
> >> >
> >> > struct dpif_packet;
> >> >
> >> > +#define NON_PMD_CORE_ID 0
> >> > +
> >> > #ifdef DPDK_NETDEV
> >> >
> >> > #include <rte_config.h>
> >> > --
> >> > 1.7.9.5
> >> >
> >> > _______________________________________________
> >> > dev mailing list
> >> > dev at openvswitch.org
> >> > http://openvswitch.org/mailman/listinfo/dev
> >
> >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list