[ovs-dev] [PATCH v3 3/3] netdev-dpdk: Autofill lcore coremask if absent

[Qiu, Michael]

On 1/14/2016 5:18 AM, Aaron Conole 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.

It's not a good idea to use the lowest set bit in the current task CPU
affinity,

I think numa info should be considered as the NIC could belongs to
different Socket.

If remote socket, it will lead bad performance.

Thanks,
Michael
> Signed-off-by: Aaron Conole <aconole at redhat.com>
> Cc: Kevin Traynor <kevin.traynor at intel.com>
> ---
> 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
>
>  lib/netdev-dpdk.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index b7bb3eb..29b3db2 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -65,6 +65,8 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>  #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE
>  #define OVS_VPORT_DPDK "ovs_dpdk"
>  
> +#define MAX_BUFSIZ 256
> +
>  /*
>   * need to reserve tons of extra space in the mbufs so we can align the
>   * DMA addresses to 4KB.
> @@ -2168,7 +2170,8 @@ grow_argv(char ***argv, size_t cur_siz, size_t grow_by)
>  }
>  
>  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)
>  {
>      struct dpdk_options_map {
>          const char *ovs_configuration;
> @@ -2176,14 +2179,14 @@ get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv)
>          bool default_enabled;
>          const char *default_value;
>      } opts[] = {
> -        {"dpdk-lcore-mask", "-c", true, "0x1"},
> +        {"dpdk-lcore-mask", "-c", false, NULL},
>          /* XXX: DPDK 2.2.0 support, the true should become false for -n */
>          {"dpdk-mem-channels", "-n", true, "4"},
>          {"dpdk-alloc-mem", "-m", false, NULL},
>          {"dpdk-socket-mem", "--socket-mem", true, "1024,0"},
>          {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
>      };
> -    int i, ret = 1;
> +    int i, ret = argc;
>  
>      for(i = 0; i < (sizeof(opts) / sizeof(opts[0])); ++i) {
>          const char *lookup = smap_get(&ovs_cfg->other_config,
> @@ -2226,7 +2229,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;
>  
> @@ -2260,12 +2264,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);
>      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));
> +                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) {
> @@ -2281,10 +2314,13 @@ __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);
> +        }
>      }
>  
>      dpdk_argv = argv;