Sent out a new version of the patch with the correct commit message and
signed-off-bys.
Ryan
On 6/23/14 1:57 PM, "Pravin Shelar" <pshe...@nicira.com> wrote:
>I like this better, If you send me signed off line I will merge it.
>
>
>On Thu, Jun 19, 2014 at 5:27 PM, Ryan Wilson 76511 <wr...@vmware.com>
>wrote:
>> This patch will fix the naming issue, too. But yea ideally I'd like to
>>merge
>> something soon so we can have this fixed.
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index fbdb6b3..0f068e9 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1179,9 +1179,12 @@ dpdk_init(int argc, char **argv)
>> {
>> int result;
>>
>> - if (strcmp(argv[1], "--dpdk"))
>> + if (argc < 2 || strcmp(argv[1], "--dpdk"))
>> return 0;
>>
>> + /* Make sure program name passed to rte_eal_init() is vswitchd. */
>> + argv[1] = argv[0];
>> +
>> argc--;
>> argv++;
>>
>> @@ -1193,7 +1196,10 @@ dpdk_init(int argc, char **argv)
>> rte_memzone_dump();
>> rte_eal_init_ret = 0;
>>
>> - return result;
>> + if (argc > result)
>> + argv[result] = argv[0];
>> +
>> + return result + 1;
>> }
>>
>> void
>>
>> From: Daniele Di Proietto <ddiproie...@vmware.com>
>> Date: Thursday, June 19, 2014 3:42 PM
>> To: Ryan Wilson <wr...@vmware.com>
>> Cc: Pravin Shelar <pshe...@nicira.com>, "dev@openvswitch.org"
>> <dev@openvswitch.org>
>> Subject: Re: [ovs-dev] [PATCH] vswitchd: skip right number of arguments
>>in
>> dpdk_init()
>>
>> 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
