I wrote  “—dpdk argument” in the first place because other programs that make 
use of rte_eal_init() do not have the “—dpdk” argument (see examples/ dir in 
dpdk release).

As a matter of facts, you’re right, we’re skipping the program name. This has 
some implications I didn’t think about:
    - rte_eal_init() gets called with argv[0] = “—dpdk”. It believes that the 
program name is “—dpdk” (the program name is used only for logging purposes)
    - in the following OVS code argv[0]==“—“ (before this patch argv[0] would 
have been the last dpdk sub argument).
      Luckily, we call set_program_name() before rte_eal_init().
      Unluckily, we call proctitle_init() after rte_eal_init().

These implications have (almost) nothing to do with my patch. My patch just 
fixes the argument parsing. It doesn’t fix the wrong program naming problem for 
dpdk log and for proctitle_init().

IMHO, we should push this (my patch fixes something, at least) and worry about 
the wrong naming problem later

What do you think?

Daniele

On Jun 19, 2014, at 3:02 PM, Ryan Wilson 76511 <wr...@vmware.com> wrote:

> I did a bit of research and I found out what's happening here.
> 
> Here's an OVS command for DPDK:
> 
> /home/ryan/ovs/_build-gcc/vswitchd/ovs-vswitchd --dpdk -c 0x1 -n 4 --
> unix:/home/alex/root/run/db.sock --pidfile --log-file --enable-dummy
> -vconsole:off --detach
> 
> What will happen when running with DPDK is rte_eal_init will count the
> following arguments "--dpdk -c 0x1 -n 4". Hence, we'll be left to parse
> the remaining command "-- unix:/home/alex/root/run/db.sock --pidfile
> --log-file --enable-dummy -vconsole:off --detach".
> 
> Without DPDK, we'll have to parse
> "/home/ryan/ovs/_build-gcc/vswitchd/ovs-vswitchd
> unix:/home/alex/root/run/db.sock --pidfile --log-file --enable-dummy
> -vconsole:off --detach".
> 
> Thus, the only difference is the first argument here "--" vs.
> "/home/ryan/ovs/_build-gcc/vswitchd/ovs-vswitchd". Basically, the code
> always skips by default the first of the remaining arguments. With DPDK,
> its "--" but without DPDK, its the program name. So dpdk_init() actually
> does skip the program name, I believe.
> 
> Anyways it doesn't really matter as long as this works.
> 
> Ryan
> 
> On 6/19/14 2:14 PM, "Pravin Shelar" <pshe...@nicira.com> wrote:
> 
>> On Wed, Jun 18, 2014 at 4:54 PM, Ryan Wilson 76511 <wr...@vmware.com>
>> wrote:
>>> Well we're really not 'skipping' the '--dpdk' argument since that is
>>> passed to rte_eal_init() as well. We're skipping the program name which
>>> is
>>> the path to ovs-vswitchd. I'd change the comment in the patch to
>>> something
>>> like:
>>> 
>> Well it depends on individual perspective, but vswitchd program name
>> is present with or without dpdk parameter, so I think we are skipping
>> --dpdk.
>> 
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index fbdb6b3..5cd4a07 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -1193,7 +1193,8 @@ dpdk_init(int argc, char **argv)
>>>     rte_memzone_dump();
>>>     rte_eal_init_ret = 0;
>>> 
>>> -    return result;
>>> +    /* We need to skip 'result' arguments plus the program name itself
>>> */
>>> +    return result + 1;
>>> }
>>> 
>>> void
>>> 
>>> 
>>> Otherwise, LGTM.
>>> 
>>> Acked-by: Ryan Wilson <wr...@nicira.com>
>>> 
>>> Can someone else review this and if they have no qualms, push it? This
>>> is
>>> necessary for DPDK to work properly.
>>> 
>>> 
>>> On 6/16/14 9:46 AM, "Daniele Di Proietto" <ddiproie...@vmware.com>
>>> wrote:
>>> 
>>>> rte_eal_init() returns the number of parsed dpdk arguments to skip.
>>>> dpdk_init() should add 1 to that number, because it has already skipped
>>>> the
>>>> "--dpdk" argument itself
>>>> 
>>>> Signed-off-by: Daniele Di Proietto <ddiproie...@vmware.com>
>>>> ---
>>>> lib/netdev-dpdk.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>>> index fbdb6b3..1ae5217 100644
>>>> --- a/lib/netdev-dpdk.c
>>>> +++ b/lib/netdev-dpdk.c
>>>> @@ -1193,7 +1193,8 @@ dpdk_init(int argc, char **argv)
>>>>    rte_memzone_dump();
>>>>    rte_eal_init_ret = 0;
>>>> 
>>>> -    return result;
>>>> +    /* We need to skip 'result' arguments plus the "--dpdk" argument
>>>> itself */
>>>> +    return result + 1;
>>>> }
>>>> 
>>>> void
>>>> --
>>>> 2.0.0
>>>> 
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev@openvswitch.org
>>>> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailma
>>>> n/
>>>> listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbff
>>>> g%
>>>> 3D%3D%0A&m=YHHbETWmp5Oa3XgP%2BcIg9XyAXHfV73IFEmQJ8uMHZcs%3D%0A&s=4306bd1
>>>> 6f
>>>> cbb62b8d4255b12d84dfc16ba8fde09d0d0bba9bf0e110877852ea2
>>> 
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> 
>>> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman
>>> /listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbff
>>> g%3D%3D%0A&m=z3zvkqljqI%2BZ1%2B5CXFQJqhn%2BJeuJJy0OURBm8ICxMzc%3D%0A&s=97
>>> 339566f48e933a25c3fc3abff1f0bb66f1bfaca89edd76a88006c5929d9efe

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to