From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id B5FE293D8 for ; Wed, 21 Oct 2015 12:54:24 +0200 (CEST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 00F7AC0BEA92; Wed, 21 Oct 2015 10:54:23 +0000 (UTC) Received: from dhcp195.koti.laiskiainen.org (vpn1-6-228.ams2.redhat.com [10.36.6.228]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t9LAsM7T008989; Wed, 21 Oct 2015 06:54:22 -0400 To: David Marchand References: From: Panu Matilainen Message-ID: <56276EDD.7060002@redhat.com> Date: Wed, 21 Oct 2015 13:54:21 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 1/2] eal: move plugin loading to eal/common X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Oct 2015 10:54:25 -0000 On 10/21/2015 01:15 PM, David Marchand wrote: > On Wed, Oct 21, 2015 at 10:29 AM, Panu Matilainen > wrote: > >> There's no good reason to limit plugins to Linux, make it available >> on FreeBSD too. Refactor the plugin code from Linux EAL to common >> helper functions, also check for potential errors during initialization. >> > > Not sure about this "potential errors". > > >> >> Signed-off-by: Panu Matilainen >> --- >> lib/librte_eal/bsdapp/eal/eal.c | 3 ++ >> lib/librte_eal/common/eal_common_options.c | 53 >> ++++++++++++++++++++++++++++++ >> lib/librte_eal/common/eal_options.h | 1 + >> lib/librte_eal/linuxapp/eal/eal.c | 39 ++-------------------- >> 4 files changed, 59 insertions(+), 37 deletions(-) >> >> diff --git a/lib/librte_eal/bsdapp/eal/eal.c >> b/lib/librte_eal/bsdapp/eal/eal.c >> index 1b6f705..f07a3c3 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal.c >> +++ b/lib/librte_eal/bsdapp/eal/eal.c >> @@ -543,6 +543,9 @@ rte_eal_init(int argc, char **argv) >> >> rte_eal_mcfg_complete(); >> >> + if (eal_plugins_init() < 0) >> + rte_panic("Cannot init plugins\n"); >> + >> > > Why testing for error when this cannot happen (see below) ? This is just a preparation for the next patch which will add a possibility of failure. Its done here to get the new "infrastructure" in place in one commit, which seemed to be the preferred option by others. >> eal_thread_init_master(rte_config.master_lcore); >> >> ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN); >> diff --git a/lib/librte_eal/common/eal_common_options.c >> b/lib/librte_eal/common/eal_common_options.c >> index 1f459ac..b542868 100644 >> --- a/lib/librte_eal/common/eal_common_options.c >> +++ b/lib/librte_eal/common/eal_common_options.c >> > > [snip] > > + >> +int >> +eal_plugins_init(void) >> +{ >> + struct shared_driver *solib = NULL; >> + >> + TAILQ_FOREACH(solib, &solib_list, next) { >> + RTE_LOG(DEBUG, EAL, "open shared lib %s\n", solib->name); >> + solib->lib_handle = dlopen(solib->name, RTLD_NOW); >> + if (solib->lib_handle == NULL) >> + RTE_LOG(WARNING, EAL, "%s\n", dlerror()); >> + } >> + return 0; >> +} > > > I can't see a non 0 return here. Yes, there is none, see above. > Btw, returning an error here would change current behavior of dpdk loading > drivers. > Not sure we want this as people may rely on this loading not failing. Yeah, dpdk currently doesn't fail if you pass garbage to -d, which is actually fairly questionable behavior. Why would you load drivers with -d if you dont care about them getting loaded? Well, maybe to handle an "everything" case but that's much better handled with the driver directory thing. So actually the current patches make things a bit inconsistent, why should driver directories cause a failure if individual drivers do not? The question is, which behavior is the one people want: I personally would rather make -dgiddy.goo fail rather than just warn and chug away but its not exactly a deal-breaker for me. - Panu - >