On Wed, Feb 4, 2015 at 4:58 PM, Daniele Di Proietto <
daniele.di.proie...@gmail.com> wrote:
> 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.
>
Good to know,
> 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.
>
I set CONFIG_RTE_LIBRTE_MBUF_DEBUG,
CONFIG_RTE_LIBRTE_ETHDEV_DEBUG and
CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG.
make install,
and try build dpdk (i have to disable -Werror, due to:
/home/alex/dpdk-1.7.1/x86_64-native-linuxapp-gcc//include/rte_mempool.h:383:13:
warning: unknown pragma ignored [-Wunknown-pragmas]
#pragma GCC pop_options)
I set NON_PMD_CORE_ID =10. Everything seems working, also tried reconfig
the cpu mask + dpdk queues...
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 for the quick reponse!~~
Alex Wang,
> Thanks,
>
> Daniele
>
> 2015-02-05 0:14 GMT+01:00 Alex Wang <al...@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 <pshe...@nicira.com>
> wrote:
> >>
> >> On Tue, Feb 3, 2015 at 5:54 PM, Alex Wang <al...@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 <al...@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@openvswitch.org
> >> > http://openvswitch.org/mailman/listinfo/dev
> >
> >
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
