[ovs-dev] [RFC 1/2] dpdk: Convert initialization from cmdline to db

Aaron Conole aconole at redhat.com
Fri Dec 4 15:19:27 UTC 2015


Mauricio Vásquez <mauricio.vasquezbernal at studenti.polito.it> writes:
> Dear Aaron,
>
> The general idea looks good to me.
>
> Just  some comments inline
>
> On 3 December 2015 at 05:23, Aaron Conole <aconole at redhat.com> wrote:
>
<< snipped >>
>> +        if(lookup) {
>> +            char **newargv = grow_argv(argv, ret, 2);
>> +
>> +            if (!newargv)
>> +                return ret;
>> +
>> +            *argv = newargv;
>> +            (*argv)[ret++] = strdup(opts[i].dpdk_option);
>> +            (*argv)[ret++] = strdup(lookup);
>> +        }
>> +    }
>>
>> -    /* Remove the --dpdk argument from arg list.*/
>> -    argc--;
>> -    argv++;
>> +    return ret;
>> +}
>>
>>
> argv shall be NULL terminated, i.e. argv[argc] shall be NULL.

Bah, not sure how I forgot the argv/argc conventions.
I'll fix when I submit the formal patch, thanks for catching this.

>
> -    /* Reject --user option */
>> -    int i;
>> -    for (i = 0; i < argc; i++) {
>> -        if (!strcmp(argv[i], "--user")) {
>> -            VLOG_ERR("Can not mix --dpdk and --user options, aborting.");
>> -        }
>> +void
>> +dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg)
>> +{
>> +    static int initialized = 0;
>> +
>> +    char **argv;
>> +    int result;
>> +    int argc;
>> +
<< snipped >>
>>
>>      /* Make sure things are initialized ... */
>>      result = rte_eal_init(argc, argv);
>>      if (result < 0) {
>>          ovs_abort(result, "Cannot init EAL");
>>      }
>> +
>> +    for(result = 0; result < argc-1; ++result)
>> +        free(argv[result]);
>> +
>> +    free(argv);
>>
> I am not sure if freeing argv is valid in this case, C99 standard states
> that it shall remain valid until the program terminates.

I hope dpdk doesn't rely on that behavior, but you're correct. I'll
switch to using an atexit() function that delays releasing all the memory.

Thanks so much for the review!

-Aaron



More information about the dev mailing list