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
