[ovs-dev] [PATCH] vswitchd: skip right number of arguments in dpdk_init()

Ryan Wilson 76511 wryan at vmware.com
Fri Jun 20 00:27:08 UTC 2014


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 <ddiproietto at vmware.com<mailto:ddiproietto at vmware.com>>
Date: Thursday, June 19, 2014 3:42 PM
To: Ryan Wilson <wryan at vmware.com<mailto:wryan at vmware.com>>
Cc: Pravin Shelar <pshelar at nicira.com<mailto:pshelar at nicira.com>>, "dev at openvswitch.org<mailto:dev at openvswitch.org>" <dev at openvswitch.org<mailto:dev at 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 <wryan at vmware.com<mailto:wryan at 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" <pshelar at nicira.com<mailto:pshelar at nicira.com>> wrote:

On Wed, Jun 18, 2014 at 4:54 PM, Ryan Wilson 76511 <wryan at vmware.com<mailto:wryan at 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 <wryan at nicira.com<mailto:wryan at 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" <ddiproietto at vmware.com<mailto:ddiproietto at 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 <ddiproietto at vmware.com<mailto:ddiproietto at 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 at openvswitch.org<mailto:dev at 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 at openvswitch.org<mailto:dev at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140620/0a333382/attachment-0005.html>


More information about the dev mailing list