| Message ID | 1467302516-106285-1-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 263F2592F; Thu, 30 Jun 2016 18:02:49 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 97A365922 for <dev@dpdk.org>; Thu, 30 Jun 2016 18:02:47 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 30 Jun 2016 09:01:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,552,1459839600"; d="scan'208";a="838108160" Received: from sie-lab-212-209.ir.intel.com (HELO silpixa00377983.ir.intel.com) ([10.237.212.209]) by orsmga003.jf.intel.com with ESMTP; 30 Jun 2016 09:01:57 -0700 From: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> To: dev@dpdk.org Cc: thomas.monjalon@6wind.com, konstantin.ananyev@intel.com Date: Thu, 30 Jun 2016 17:01:56 +0100 Message-Id: <1467302516-106285-1-git-send-email-sergio.gonzalez.monroy@intel.com> X-Mailer: git-send-email 2.4.11 In-Reply-To: <1467285021-103920-1-git-send-email-sergio.gonzalez.monroy@intel.com> References: <1467285021-103920-1-git-send-email-sergio.gonzalez.monroy@intel.com> Subject: [dpdk-dev] [PATCH v2] mk: fix acl library static linking X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Sergio Gonzalez Monroy
June 30, 2016, 4:01 p.m. UTC
Since below commit, ACL library is outside the scope of --whole-archive
and ACL autotest fails.
RTE>>acl_autotest
ACL: allocation of 25166728 bytes on socket 9 for ACL_acl_ctx failed
ACL: rte_acl_add_rules(acl_ctx): rule #1 is invalid
Line 1584: SSE classify with zero categories failed!
Test Failed
This is the result of the linker picking weak over non-weak functions.
Fixes: 95dc3c3cf31c ("mk: reduce scope of whole-archive static linking")
Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
mk/rte.app.mk | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
2016-06-30 17:01, Sergio Gonzalez Monroy: > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag > _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter > _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm > -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats > _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power > > _LDLIBS-y += --whole-archive > > +# librte_acl needs --whole-archive because of weak functions > +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer > _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash > _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost I was suggesting to keep -lrte_acl at the same place in the group of algorithms libraries, in order to keep an order satisfying this comment: # Order is important: from higher level to lower level But I have not tested if --whole-archive -lrte_acl --no-whole-archive works.
On 30/06/2016 17:10, Thomas Monjalon wrote: > 2016-06-30 17:01, Sergio Gonzalez Monroy: >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >> _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >> _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >> _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >> _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >> _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >> >> _LDLIBS-y += --whole-archive >> >> +# librte_acl needs --whole-archive because of weak functions >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >> _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost > I was suggesting to keep -lrte_acl at the same place in the group of > algorithms libraries, in order to keep an order satisfying this comment: > # Order is important: from higher level to lower level > > But I have not tested if --whole-archive -lrte_acl --no-whole-archive works. > Sorry, I missed that. Why is important being before jobstats and power?
2016-06-30 17:14, Sergio Gonzalez Monroy: > On 30/06/2016 17:10, Thomas Monjalon wrote: > > 2016-06-30 17:01, Sergio Gonzalez Monroy: > >> --- a/mk/rte.app.mk > >> +++ b/mk/rte.app.mk > >> @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm > >> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power > >> > >> _LDLIBS-y += --whole-archive > >> > >> +# librte_acl needs --whole-archive because of weak functions > >> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost > > I was suggesting to keep -lrte_acl at the same place in the group of > > algorithms libraries, in order to keep an order satisfying this comment: > > # Order is important: from higher level to lower level > > > > But I have not tested if --whole-archive -lrte_acl --no-whole-archive works. > > > > Sorry, I missed that. > > Why is important being before jobstats and power? It is not. But I think we need to have some groups. And ACL is probably at the same layer level as lpm, sched, etc.
On 30/06/2016 17:22, Thomas Monjalon wrote: > 2016-06-30 17:14, Sergio Gonzalez Monroy: >> On 30/06/2016 17:10, Thomas Monjalon wrote: >>> 2016-06-30 17:01, Sergio Gonzalez Monroy: >>>> --- a/mk/rte.app.mk >>>> +++ b/mk/rte.app.mk >>>> @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >>>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >>>> >>>> _LDLIBS-y += --whole-archive >>>> >>>> +# librte_acl needs --whole-archive because of weak functions >>>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost >>> I was suggesting to keep -lrte_acl at the same place in the group of >>> algorithms libraries, in order to keep an order satisfying this comment: >>> # Order is important: from higher level to lower level >>> >>> But I have not tested if --whole-archive -lrte_acl --no-whole-archive works. >>> >> Sorry, I missed that. >> >> Why is important being before jobstats and power? > It is not. > But I think we need to have some groups. > And ACL is probably at the same layer level as lpm, sched, etc. I guess I just don't see the groups you are mentioning :) How are timer, hash and vhost in the same group? Wouldn't hash be in the same group as acl and lpm?
2016-07-01 09:05, Sergio Gonzalez Monroy: > On 30/06/2016 17:22, Thomas Monjalon wrote: > > 2016-06-30 17:14, Sergio Gonzalez Monroy: > >> On 30/06/2016 17:10, Thomas Monjalon wrote: > >>> 2016-06-30 17:01, Sergio Gonzalez Monroy: > >>>> --- a/mk/rte.app.mk > >>>> +++ b/mk/rte.app.mk > >>>> @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm > >>>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power > >>>> > >>>> _LDLIBS-y += --whole-archive > >>>> > >>>> +# librte_acl needs --whole-archive because of weak functions > >>>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash > >>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost > >>> I was suggesting to keep -lrte_acl at the same place in the group of > >>> algorithms libraries, in order to keep an order satisfying this comment: > >>> # Order is important: from higher level to lower level > >>> > >>> But I have not tested if --whole-archive -lrte_acl --no-whole-archive works. > >>> > >> Sorry, I missed that. > >> > >> Why is important being before jobstats and power? > > It is not. > > But I think we need to have some groups. > > And ACL is probably at the same layer level as lpm, sched, etc. > > I guess I just don't see the groups you are mentioning :) I define groups as separated by blank line :) > How are timer, hash and vhost in the same group? It is far from perfect and subject to improvements :) > Wouldn't hash be in the same group as acl and lpm? It makes sense to use hash in drivers (example: enic).
On 01/07/2016 11:05, Thomas Monjalon wrote: > 2016-07-01 09:05, Sergio Gonzalez Monroy: >> On 30/06/2016 17:22, Thomas Monjalon wrote: >>> 2016-06-30 17:14, Sergio Gonzalez Monroy: >>>> On 30/06/2016 17:10, Thomas Monjalon wrote: >>>>> 2016-06-30 17:01, Sergio Gonzalez Monroy: >>>>>> --- a/mk/rte.app.mk >>>>>> +++ b/mk/rte.app.mk >>>>>> @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm >>>>>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power >>>>>> >>>>>> _LDLIBS-y += --whole-archive >>>>>> >>>>>> +# librte_acl needs --whole-archive because of weak functions >>>>>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost >>>>> I was suggesting to keep -lrte_acl at the same place in the group of >>>>> algorithms libraries, in order to keep an order satisfying this comment: >>>>> # Order is important: from higher level to lower level >>>>> >>>>> But I have not tested if --whole-archive -lrte_acl --no-whole-archive works. >>>>> >>>> Sorry, I missed that. >>>> >>>> Why is important being before jobstats and power? >>> It is not. >>> But I think we need to have some groups. >>> And ACL is probably at the same layer level as lpm, sched, etc. >> I guess I just don't see the groups you are mentioning :) > I define groups as separated by blank line :) > >> How are timer, hash and vhost in the same group? > It is far from perfect and subject to improvements :) > >> Wouldn't hash be in the same group as acl and lpm? > It makes sense to use hash in drivers (example: enic). You have not convinced me, but I'm not going to argue more over this. You would just prefer to do the following, right? +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += --whole-archive _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += --no-whole-archive
2016-07-01 11:27, Sergio Gonzalez Monroy: > On 01/07/2016 11:05, Thomas Monjalon wrote: > > 2016-07-01 09:05, Sergio Gonzalez Monroy: > >> On 30/06/2016 17:22, Thomas Monjalon wrote: > >>> 2016-06-30 17:14, Sergio Gonzalez Monroy: > >>>> On 30/06/2016 17:10, Thomas Monjalon wrote: > >>>>> 2016-06-30 17:01, Sergio Gonzalez Monroy: > >>>>>> --- a/mk/rte.app.mk > >>>>>> +++ b/mk/rte.app.mk > >>>>>> @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm > >>>>>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power > >>>>>> > >>>>>> _LDLIBS-y += --whole-archive > >>>>>> > >>>>>> +# librte_acl needs --whole-archive because of weak functions > >>>>>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash > >>>>>> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost > >>>>> I was suggesting to keep -lrte_acl at the same place in the group of > >>>>> algorithms libraries, in order to keep an order satisfying this comment: > >>>>> # Order is important: from higher level to lower level > >>>>> > >>>>> But I have not tested if --whole-archive -lrte_acl --no-whole-archive works. > >>>>> > >>>> Sorry, I missed that. > >>>> > >>>> Why is important being before jobstats and power? > >>> It is not. > >>> But I think we need to have some groups. > >>> And ACL is probably at the same layer level as lpm, sched, etc. > >> I guess I just don't see the groups you are mentioning :) > > I define groups as separated by blank line :) > > > >> How are timer, hash and vhost in the same group? > > It is far from perfect and subject to improvements :) > > > >> Wouldn't hash be in the same group as acl and lpm? > > It makes sense to use hash in drivers (example: enic). > > You have not convinced me, but I'm not going to argue more over this. But you have convinced me (I was already convinced) that more cleanups and explanations are needed in this area :) > You would just prefer to do the following, right? > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += --whole-archive > _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl > +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += --no-whole-archive Yes for this fix. Later we can improve few things, thanks.
diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 83314ca..48d5e73 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -76,12 +76,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += -lrte_ip_frag _LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS) += -lrte_jobstats _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power _LDLIBS-y += --whole-archive +# librte_acl needs --whole-archive because of weak functions +_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH) += -lrte_hash _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost