On 28.01.2019 22:21, Ian Stokes wrote:
> On 1/22/2019 1:22 PM, Ilya Maximets wrote:
>> No need to implement dynamic vector to store arguments.
>> 'svec' perfectly covers all the needed functionality.
>>
>
> Thanks for this Ilya, testing this this the last few days and seems in
> working order, I see Aaron has also acked this, I just have one comment below:
>
>> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com>
>> ---
>> lib/dpdk.c | 217 ++++++++++++++++-------------------------------------
>> 1 file changed, 66 insertions(+), 151 deletions(-)
>>
>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>> index 0ee3e19c6..d70884aad 100644
>> --- a/lib/dpdk.c
>> +++ b/lib/dpdk.c
>> @@ -38,6 +38,7 @@
>> #include "openvswitch/vlog.h"
>> #include "ovs-numa.h"
>> #include "smap.h"
>> +#include "svec.h"
>> #include "vswitch-idl.h"
>> VLOG_DEFINE_THIS_MODULE(dpdk);
>> @@ -75,65 +76,23 @@ process_vhost_flags(char *flag, const char *default_val,
>> int size,
>> return changed;
>> }
>> -static char **
>> -grow_argv(char ***argv, size_t cur_siz, size_t grow_by)
>> -{
>> - return xrealloc(*argv, sizeof(char *) * (cur_siz + grow_by));
>> -}
>> -
>> -static void
>> -dpdk_option_extend(char ***argv, int argc, const char *option,
>> - const char *value)
>> -{
>> - char **newargv = grow_argv(argv, argc, 2);
>> - *argv = newargv;
>> - newargv[argc] = xstrdup(option);
>> - newargv[argc+1] = xstrdup(value);
>> -}
>> -
>> -static char **
>> -move_argv(char ***argv, size_t cur_size, char **src_argv, size_t src_argc)
>> -{
>> - char **newargv = grow_argv(argv, cur_size, src_argc);
>> - while (src_argc--) {
>> - newargv[cur_size+src_argc] = src_argv[src_argc];
>> - src_argv[src_argc] = NULL;
>> - }
>> - return newargv;
>> -}
>> -
>> -static int
>> -extra_dpdk_args(const char *ovs_extra_config, char ***argv, int argc)
>> -{
>> - int ret = argc;
>> - char *release_tok = xstrdup(ovs_extra_config);
>> - char *tok, *endptr = NULL;
>> -
>> - for (tok = strtok_r(release_tok, " ", &endptr); tok != NULL;
>> - tok = strtok_r(NULL, " ", &endptr)) {
>> - char **newarg = grow_argv(argv, ret, 1);
>> - *argv = newarg;
>> - newarg[ret++] = xstrdup(tok);
>> - }
>> - free(release_tok);
>> - return ret;
>> -}
>> -
>> static bool
>> -argv_contains(char **argv_haystack, const size_t argc_haystack,
>> - const char *needle)
>> +args_contains(const struct svec *args, const char *needle)
>
> I think 'needle' only makes sense as an argument name when haystack was also
> passed :), but without haystack it seems arbitrary, I suggest it be changed
> to something like like value.
Sure.
I'd like to also replace both "dpdk_extras" with "dpdk-extra" and fix the too
deep
indentation near "auto_determine = false;". I'll send the v2 with these changes.
Hope I could keep the Ack with these changes.
I'll also add the "--socket-limit" patch to the set as it will need some
trivial rebase.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
