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