On 09/03/2016 09:38, "Aaron Conole" <acon...@redhat.com> wrote:
>The user has control over the DPDK internal lcore coremask, but this
>parameter can be autofilled with a bit more intelligence. If the user
>does not fill this parameter in, we use the lowest set bit in the
>current task CPU affinity. Otherwise, we will reassign the current
>thread to the specified lcore mask, in addition to the dpdk lcore
>threads.
>
>Signed-off-by: Aaron Conole <acon...@redhat.com>
>Tested-by: Sean K Mooney <sean.k.moo...@intel.com>
>Tested-by: RobertX Wojciechowicz <robertx.wojciechow...@intel.com>
>Tested-by: Kevin Traynor <kevin.tray...@intel.com>
>Acked-by: Panu Matilainen <pmati...@redhat.com>
>Acked-by: Kevin Traynor <kevin.tray...@intel.com>
>Acked-by: Flavio Leitner <f...@sysclose.org>
>---
>v2:
>* Fix a conditional branch coding standard issue
>* When lcore coremask is set, do not reset the affinities as
> suggested by Kevin Traynor
>
>v3:
>* Fixed grow_argv calls
>* Fixed an error in argc assignment after 'break;' introduced
> in v2
>* Switched to using xstrdup
>
>v4->v7:
>* No change
>
>v8:
>* Assign the lcore only when resetting the affinity.
>
>v9,v10:
>* No change
>
> lib/netdev-dpdk.c | 62
>++++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 48 insertions(+), 14 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 7e8e72e..289f916 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -68,6 +68,7 @@ static struct vlog_rate_limit rl =
>VLOG_RATE_LIMIT_INIT(5, 20);
> #define OVS_VPORT_DPDK "ovs_dpdk"
>
> #define MAX_DPDK_EXCL_OPTS 10
>+#define MAX_BUFSIZ 256
I would remove this, please see below
>
> /*
> * need to reserve tons of extra space in the mbufs so we can align the
>@@ -2770,7 +2771,6 @@ construct_dpdk_options(const struct
>ovsrec_open_vswitch *ovs_cfg,
> {"dpdk-mem-channels", "-n", false, "4"},
> {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
> };
>-
> int i, ret = initial_size;
>
> /*First, construct from the flat-options (non-mutex)*/
>@@ -2848,9 +2848,10 @@ construct_dpdk_mutex_options(const struct
>ovsrec_open_vswitch *ovs_cfg,
> }
>
> static int
>-get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv)
>+get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv,
>+ int argc)
> {
>- int i = construct_dpdk_options(ovs_cfg, argv, 1);
>+ int i = construct_dpdk_options(ovs_cfg, argv, argc);
> i = construct_dpdk_mutex_options(ovs_cfg, argv, i);
> return i;
> }
>@@ -2874,7 +2875,8 @@ __dpdk_init(const struct ovsrec_open_vswitch
>*ovs_cfg)
> {
> char **argv = NULL;
> int result;
>- int argc;
>+ int argc = 0, argc_tmp;
>+ bool auto_determine = true;
> int err;
> cpu_set_t cpuset;
>
>@@ -2908,12 +2910,41 @@ __dpdk_init(const struct ovsrec_open_vswitch
>*ovs_cfg)
> ovs_abort(0, "Thread getaffinity error %d.", err);
> }
>
>- argv = grow_argv(&argv, 0, 1);
>+ argv = grow_argv(&argv, argc, argc+1);
I think "1" is better than "argc+1"
> if (!argv) {
> ovs_abort(0, "Unable to allocate an initial argv.");
> }
>- argv[0] = xstrdup("ovs"); /* TODO use prctl to get process name */
>- argc = get_dpdk_args(ovs_cfg, &argv);
>+ argv[argc++] = xstrdup("ovs"); /* TODO use prctl to get process name
>*/
>+ argc_tmp = get_dpdk_args(ovs_cfg, &argv, argc);
>+
>+ while(argc_tmp != argc) {
>+ if (!strcmp("-c", argv[argc++])) {
>+ auto_determine = false;
>+ break;
>+ }
>+ }
>+ argc = argc_tmp;
>+
>+ /**
>+ * NOTE: This is an unsophisticated mechanism for determining the
>DPDK
>+ * lcore for the DPDK Master.
>+ */
>+ if (auto_determine) {
>+ int i;
>+ for (i = 0; i < CPU_SETSIZE; i++) {
>+ if (CPU_ISSET(i, &cpuset)) {
>+ char buf[MAX_BUFSIZ];
>+ snprintf(buf, MAX_BUFSIZ, "0x%08llX", (1ULL<<i));
please use xasprintf()
>+ argv = grow_argv(&argv, argc, 2);
>+ if (!argv) {
>+ ovs_abort(0, "Unable to grow argv for coremask");
>+ }
>+ argv[argc++] = xstrdup("-c");
>+ argv[argc++] = xstrdup(buf);
>+ i = CPU_SETSIZE;
>+ }
>+ }
>+ }
>
> argv = grow_argv(&argv, argc, 1);
> if (!argv) {
>@@ -2929,10 +2960,16 @@ __dpdk_init(const struct ovsrec_open_vswitch
>*ovs_cfg)
> ovs_abort(result, "Cannot init EAL");
> }
>
>- /* Set the main thread affinity back to pre rte_eal_init() value */
>- err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t),
>&cpuset);
>- if (err) {
>- ovs_abort(0, "Thread getaffinity error %d.", err);
>+ if (auto_determine) {
>+ /* Set the main thread affinity back to pre rte_eal_init() value
>*/
>+ err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t),
>+ &cpuset);
>+ if (err) {
>+ ovs_abort(0, "Thread getaffinity error %d.", err);
>+ }
>+
>+ /* We are called from the main thread here */
>+ RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
This needs to be done in any case. Every thread in OVS must have
lcore_id == NON_PMD_CORE_ID, except for the pmd threads.
> }
>
> dpdk_argv = argv;
>@@ -2943,9 +2980,6 @@ __dpdk_init(const struct ovsrec_open_vswitch
>*ovs_cfg)
> rte_memzone_dump(stdout);
> rte_eal_init_ret = 0;
>
>- /* We are called from the main thread here */
>- RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
>-
> ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
>
> #ifdef VHOST_CUSE
>--
>2.5.0
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
