From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 7E249333 for ; Mon, 22 Sep 2014 21:11:57 +0200 (CEST) Received: from [2001:470:8:a08:18c5:c64e:4bf:67a] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1XW97S-0006PD-Uu; Mon, 22 Sep 2014 15:18:00 -0400 Date: Mon, 22 Sep 2014 15:17:53 -0400 From: Neil Horman To: Alan Carew Message-ID: <20140922191753.GH25406@hmsreliant.think-freely.org> References: <1411410879-28872-1-git-send-email-alan.carew@intel.com> <1411410879-28872-7-git-send-email-alan.carew@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1411410879-28872-7-git-send-email-alan.carew@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 06/10] Alternate implementation of librte_power for VM Power Management(Guest). 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: Mon, 22 Sep 2014 19:11:57 -0000 On Mon, Sep 22, 2014 at 07:34:35PM +0100, Alan Carew wrote: > Re-using the host based librte_power API the alternate implementation uses > the guest channel API to forward request for frequency changes to the host > monitor. > A subset of the librte_power API is supported: > rte_power_init(unsigned lcore_id) > rte_power_exit(unsigned lcore_id) > rte_power_freq_up(unsigned lcore_id) > rte_power_freq_down(unsigned lcore_id) > rte_power_freq_min(unsigned lcore_id) > rte_power_freq_max(unsigned lcore_id) > > The other unsupported APIs from librte_power return -ENOTSUP. > > Signed-off-by: Alan Carew > --- > lib/librte_power_vm/Makefile | 49 ++++++++++++++ > lib/librte_power_vm/rte_power.c | 146 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 195 insertions(+) > create mode 100644 lib/librte_power_vm/Makefile > create mode 100644 lib/librte_power_vm/rte_power.c > NAK. This is a bad design choice. Creating an alternate library with all the same symbols in place prevents an application from compiling in support for both host and guest power management in parallel (i.e. if an app wants to be able to do power management in either environment, and only gets built once, it won't work). In fact, linking a statically built library with both CONFIG_RTE_LIBRTE_POWER=y and CONFIG_RTE_LIBRTE_POWER_VM=y yields the following link-time build break: LD test /home/nhorman/git/dpdk/build/lib/librte_power.a(guest_channel.o): In function `guest_channel_host_connect': guest_channel.c:(.text+0x0): multiple definition of `guest_channel_host_connect' /home/nhorman/git/dpdk/build/lib/librte_power.a(guest_channel.o):guest_channel.c:(.text+0x0): first defined here /home/nhorman/git/dpdk/build/lib/librte_power.a(guest_channel.o): In function `guest_channel_send_msg': guest_channel.c:(.text+0x370): multiple definition of `guest_channel_send_msg' .... Ad nauseum. What you should do is merge this functionality in with the existing librte power library, and make the choice of implementation a run time decision, so theres only a single public facing API symbol set, and both implementations can coexist, getting chosen at run time (via initialization config option, environment detection, etc). Konstantin and I had a simmilar discussion regarding the ACL library and the use of the match function. I think we came up with some reasonably performant solutions. Neil