DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mk: fix acl library static linking
@ 2016-06-30 11:10 Sergio Gonzalez Monroy
  2016-06-30 11:38 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 11:10 UTC (permalink / raw)
  To: dev

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 83314ca..7f89fd4 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -76,12 +76,12 @@ _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
 
+_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
-- 
2.4.11

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 11:10 [dpdk-dev] [PATCH] mk: fix acl library static linking Sergio Gonzalez Monroy
@ 2016-06-30 11:38 ` Thomas Monjalon
  2016-06-30 12:04   ` Sergio Gonzalez Monroy
  2016-06-30 12:14 ` Ananyev, Konstantin
  2016-06-30 16:01 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy
  2 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-30 11:38 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev

2016-06-30 12:10, Sergio Gonzalez Monroy:
> 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>

Please could you detail which symbol is missing?

Does it need to be commented in rte.app.mk?
The other libs are in whole-archive to support dlopen of drivers.
But the problem here is not because of a driver use.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 11:38 ` Thomas Monjalon
@ 2016-06-30 12:04   ` Sergio Gonzalez Monroy
  2016-06-30 12:44     ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 12:04 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 30/06/2016 12:38, Thomas Monjalon wrote:
> 2016-06-30 12:10, Sergio Gonzalez Monroy:
>> 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>
> Please could you detail which symbol is missing?

It is not missing a symbol, it is picking weak over non-weak symbol.
It just happens that the only other using weak symbols are PMDs and
they are under the scope of --whole-archive already.

> Does it need to be commented in rte.app.mk?
> The other libs are in whole-archive to support dlopen of drivers.
> But the problem here is not because of a driver use.

There seem to be a bunch of libraries under --whole-archive scope that 
are not
PMDs, ie. cfgfile, cmdline...

What is the criteria?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 11:10 [dpdk-dev] [PATCH] mk: fix acl library static linking Sergio Gonzalez Monroy
  2016-06-30 11:38 ` Thomas Monjalon
@ 2016-06-30 12:14 ` Ananyev, Konstantin
  2016-06-30 16:01 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy
  2 siblings, 0 replies; 22+ messages in thread
From: Ananyev, Konstantin @ 2016-06-30 12:14 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez Monroy
> Sent: Thursday, June 30, 2016 12:10 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] mk: fix acl library static linking
> 
> 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 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 83314ca..7f89fd4 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -76,12 +76,12 @@ _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
> 
> +_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
> --
> 2.4.11

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 12:04   ` Sergio Gonzalez Monroy
@ 2016-06-30 12:44     ` Thomas Monjalon
  2016-06-30 14:02       ` Sergio Gonzalez Monroy
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-30 12:44 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev

2016-06-30 13:04, Sergio Gonzalez Monroy:
> On 30/06/2016 12:38, Thomas Monjalon wrote:
> > Does it need to be commented in rte.app.mk?
> > The other libs are in whole-archive to support dlopen of drivers.
> > But the problem here is not because of a driver use.
> 
> There seem to be a bunch of libraries under --whole-archive scope that 
> are not
> PMDs, ie. cfgfile, cmdline...
> 
> What is the criteria?

The criteria is a bit vague. We must try to include only libs which can
be used by a driver.
cmdline should probably not be there.
Does it make sense to use cfgfile in a driver? maybe yes.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 12:44     ` Thomas Monjalon
@ 2016-06-30 14:02       ` Sergio Gonzalez Monroy
  2016-06-30 15:24         ` Ananyev, Konstantin
  2016-06-30 15:28         ` Thomas Monjalon
  0 siblings, 2 replies; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 14:02 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 30/06/2016 13:44, Thomas Monjalon wrote:
> 2016-06-30 13:04, Sergio Gonzalez Monroy:
>> On 30/06/2016 12:38, Thomas Monjalon wrote:
>>> Does it need to be commented in rte.app.mk?
>>> The other libs are in whole-archive to support dlopen of drivers.
>>> But the problem here is not because of a driver use.
>> There seem to be a bunch of libraries under --whole-archive scope that
>> are not
>> PMDs, ie. cfgfile, cmdline...
>>
>> What is the criteria?
> The criteria is a bit vague. We must try to include only libs which can
> be used by a driver.
> cmdline should probably not be there.
> Does it make sense to use cfgfile in a driver? maybe yes.

So as it is, ACL autotest is broken when building static libs 
(non-combined).
For combined libs we usually wrap libdpdk.a with --whole-archive, thus it is
not an issue.

Just thinking a bit more about the 'dlopen of drivers' case you 
mentioned before,
shouldn't the driver have proper dependencies and therefore need shared 
DPDK libraries?
What does happen if binary/app and driver are built against different 
library versions?
Where does it say that we do support this use case?

Sergio

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 14:02       ` Sergio Gonzalez Monroy
@ 2016-06-30 15:24         ` Ananyev, Konstantin
  2016-06-30 15:47           ` Thomas Monjalon
  2016-06-30 15:28         ` Thomas Monjalon
  1 sibling, 1 reply; 22+ messages in thread
From: Ananyev, Konstantin @ 2016-06-30 15:24 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio, Thomas Monjalon; +Cc: dev


Hi Thomas,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez Monroy
> Sent: Thursday, June 30, 2016 3:02 PM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
> 
> On 30/06/2016 13:44, Thomas Monjalon wrote:
> > 2016-06-30 13:04, Sergio Gonzalez Monroy:
> >> On 30/06/2016 12:38, Thomas Monjalon wrote:
> >>> Does it need to be commented in rte.app.mk?
> >>> The other libs are in whole-archive to support dlopen of drivers.
> >>> But the problem here is not because of a driver use.
> >> There seem to be a bunch of libraries under --whole-archive scope that
> >> are not
> >> PMDs, ie. cfgfile, cmdline...
> >>
> >> What is the criteria?
> > The criteria is a bit vague. We must try to include only libs which can
> > be used by a driver.
> > cmdline should probably not be there.
> > Does it make sense to use cfgfile in a driver? maybe yes.
> 
> So as it is, ACL autotest is broken when building static libs
> (non-combined).
> For combined libs we usually wrap libdpdk.a with --whole-archive, thus it is
> not an issue.
> 
> Just thinking a bit more about the 'dlopen of drivers' case you
> mentioned before,
> shouldn't the driver have proper dependencies and therefore need shared
> DPDK libraries?
> What does happen if binary/app and driver are built against different
> library versions?
> Where does it say that we do support this use case?
> 
> Sergio
> 

So are you going to apply this patch?
Right now acl just can't be used properly in case of static library build.
Thanks
Konstantin

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 14:02       ` Sergio Gonzalez Monroy
  2016-06-30 15:24         ` Ananyev, Konstantin
@ 2016-06-30 15:28         ` Thomas Monjalon
  2016-06-30 15:58           ` Sergio Gonzalez Monroy
  1 sibling, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-30 15:28 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev

2016-06-30 15:02, Sergio Gonzalez Monroy:
> On 30/06/2016 13:44, Thomas Monjalon wrote:
> > 2016-06-30 13:04, Sergio Gonzalez Monroy:
> >> On 30/06/2016 12:38, Thomas Monjalon wrote:
> >>> Does it need to be commented in rte.app.mk?
> >>> The other libs are in whole-archive to support dlopen of drivers.
> >>> But the problem here is not because of a driver use.
> >> There seem to be a bunch of libraries under --whole-archive scope that
> >> are not
> >> PMDs, ie. cfgfile, cmdline...
> >>
> >> What is the criteria?
> > The criteria is a bit vague. We must try to include only libs which can
> > be used by a driver.
> > cmdline should probably not be there.
> > Does it make sense to use cfgfile in a driver? maybe yes.
> 
> So as it is, ACL autotest is broken when building static libs 
> (non-combined).

I think the --whole-archive option must be set specifically for ACL
with a comment explaining it is required because of weak functions:

# librte_acl needs --whole-archive because of weak functions
_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += --whole-archive -lrte_acl --no-whole-archive

> For combined libs we usually wrap libdpdk.a with --whole-archive, thus it is
> not an issue.
> 
> Just thinking a bit more about the 'dlopen of drivers' case you 
> mentioned before,
> shouldn't the driver have proper dependencies and therefore need shared 
> DPDK libraries?

It is possible to build a .so, without any DT_NEEDED entries, which will
find the required symbols in the static linked binary.

> What does happen if binary/app and driver are built against different 
> library versions?

Bad things :)

> Where does it say that we do support this use case?

It is maybe not written. But I know it is used by people wanting to load
some PMD.so on demand while having the rest statically compiled.
I agree it needs to be documented and probably better managed and tested.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 15:24         ` Ananyev, Konstantin
@ 2016-06-30 15:47           ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-30 15:47 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: Gonzalez Monroy, Sergio, dev

2016-06-30 15:24, Ananyev, Konstantin:
> Hi Thomas,
[...]
> 
> So are you going to apply this patch?
> Right now acl just can't be used properly in case of static library build.

Got it.
I have suggested a small rework in another mail.
Then I'll apply it promptly.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix acl library static linking
  2016-06-30 15:28         ` Thomas Monjalon
@ 2016-06-30 15:58           ` Sergio Gonzalez Monroy
  0 siblings, 0 replies; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 15:58 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 30/06/2016 16:28, Thomas Monjalon wrote:
> 2016-06-30 15:02, Sergio Gonzalez Monroy:
>> On 30/06/2016 13:44, Thomas Monjalon wrote:
>>> 2016-06-30 13:04, Sergio Gonzalez Monroy:
>>>> On 30/06/2016 12:38, Thomas Monjalon wrote:
>>>>> Does it need to be commented in rte.app.mk?
>>>>> The other libs are in whole-archive to support dlopen of drivers.
>>>>> But the problem here is not because of a driver use.
>>>> There seem to be a bunch of libraries under --whole-archive scope that
>>>> are not
>>>> PMDs, ie. cfgfile, cmdline...
>>>>
>>>> What is the criteria?
>>> The criteria is a bit vague. We must try to include only libs which can
>>> be used by a driver.
>>> cmdline should probably not be there.
>>> Does it make sense to use cfgfile in a driver? maybe yes.
>> So as it is, ACL autotest is broken when building static libs
>> (non-combined).
> I think the --whole-archive option must be set specifically for ACL
> with a comment explaining it is required because of weak functions:
>
> # librte_acl needs --whole-archive because of weak functions
> _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += --whole-archive -lrte_acl --no-whole-archive

Will do.

>> For combined libs we usually wrap libdpdk.a with --whole-archive, thus it is
>> not an issue.
>>
>> Just thinking a bit more about the 'dlopen of drivers' case you
>> mentioned before,
>> shouldn't the driver have proper dependencies and therefore need shared
>> DPDK libraries?
> It is possible to build a .so, without any DT_NEEDED entries, which will
> find the required symbols in the static linked binary.

Of course! All DPDK libraries were like that until recently.
That doesn't mean it was right though.

>> What does happen if binary/app and driver are built against different
>> library versions?
> Bad things :)
>
>> Where does it say that we do support this use case?
> It is maybe not written. But I know it is used by people wanting to load
> some PMD.so on demand while having the rest statically compiled.
> I agree it needs to be documented and probably better managed and tested.
>

Note that this only applies to apps built with DPDK build system.

In my opinion, I don't think we should be supporting such case.
But if we were to, we are probably just better of whole-archiving all 
libraries into the
application. For example, what if there was a driver wanting to use ACL 
or any
other DPDK lib not currently in the set of libs we "consider" should be 
use by drivers?

Also, from what I have seen in the list, most folks do end up using 
combined lib and
wrapping it with --whole-archive.

Sergio

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-06-30 11:10 [dpdk-dev] [PATCH] mk: fix acl library static linking Sergio Gonzalez Monroy
  2016-06-30 11:38 ` Thomas Monjalon
  2016-06-30 12:14 ` Ananyev, Konstantin
@ 2016-06-30 16:01 ` Sergio Gonzalez Monroy
  2016-06-30 16:10   ` Thomas Monjalon
  2016-06-30 16:11   ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy
  2 siblings, 2 replies; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 16:01 UTC (permalink / raw)
  To: dev; +Cc: thomas.monjalon, konstantin.ananyev

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(-)

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
-- 
2.4.11

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-06-30 16:01 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy
@ 2016-06-30 16:10   ` Thomas Monjalon
  2016-06-30 16:14     ` Sergio Gonzalez Monroy
  2016-06-30 16:11   ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy
  1 sibling, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-30 16:10 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev, konstantin.ananyev

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v3] mk: fix acl library static linking
  2016-06-30 16:01 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy
  2016-06-30 16:10   ` Thomas Monjalon
@ 2016-06-30 16:11   ` Sergio Gonzalez Monroy
  2016-07-01 14:38     ` [dpdk-dev] [PATCH v4 1/2] mk: allow duplicate linker flags in libraries list Sergio Gonzalez Monroy
  1 sibling, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 16:11 UTC (permalink / raw)
  To: dev; +Cc: thomas.monjalon, konstantin.ananyev

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>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
v3:
- patch version history
- carry Acked-by

v2:
- add comment in makefile

 mk/rte.app.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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
-- 
2.4.11

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-06-30 16:10   ` Thomas Monjalon
@ 2016-06-30 16:14     ` Sergio Gonzalez Monroy
  2016-06-30 16:22       ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-06-30 16:14 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, konstantin.ananyev

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?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-06-30 16:14     ` Sergio Gonzalez Monroy
@ 2016-06-30 16:22       ` Thomas Monjalon
  2016-07-01  8:05         ` Sergio Gonzalez Monroy
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-06-30 16:22 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev, konstantin.ananyev

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-06-30 16:22       ` Thomas Monjalon
@ 2016-07-01  8:05         ` Sergio Gonzalez Monroy
  2016-07-01 10:05           ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-07-01  8:05 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, konstantin.ananyev

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?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-07-01  8:05         ` Sergio Gonzalez Monroy
@ 2016-07-01 10:05           ` Thomas Monjalon
  2016-07-01 10:27             ` Sergio Gonzalez Monroy
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2016-07-01 10:05 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev, konstantin.ananyev

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).

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-07-01 10:05           ` Thomas Monjalon
@ 2016-07-01 10:27             ` Sergio Gonzalez Monroy
  2016-07-01 10:39               ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-07-01 10:27 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, konstantin.ananyev

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: fix acl library static linking
  2016-07-01 10:27             ` Sergio Gonzalez Monroy
@ 2016-07-01 10:39               ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2016-07-01 10:39 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev, konstantin.ananyev

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4 1/2] mk: allow duplicate linker flags in libraries list
  2016-06-30 16:11   ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy
@ 2016-07-01 14:38     ` Sergio Gonzalez Monroy
  2016-07-01 14:38       ` [dpdk-dev] [PATCH v4 2/2] mk: fix acl library static linking Sergio Gonzalez Monroy
  0 siblings, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-07-01 14:38 UTC (permalink / raw)
  To: dev; +Cc: thomas.monjalon, konstantin.ananyev

Since [1] duplicates in LDLIBS are removed. The side effect is that it
does not distinguish between libraries or linker flags.

This patch allows multiple linker flags in LDLIBS, such as
--whole-archive.

[1] Commit: edf4d331dcdb ("mk: eliminate duplicates from libraries list")

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.app.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 83314ca..bf8bcf9 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -166,7 +166,8 @@ LDLIBS += $(_LDLIBS-y) $(CPU_LDLIBS) $(EXTRA_LDLIBS)
 
 # Eliminate duplicates without sorting
 LDLIBS := $(shell echo $(LDLIBS) | \
-	awk '{for (i = 1; i <= NF; i++) { if (!seen[$$i]++) print $$i }}')
+	awk '{for (i = 1; i <= NF; i++) { \
+		if ($$i !~ /^-l.*/ || !seen[$$i]++) print $$i }}')
 
 ifeq ($(RTE_DEVEL_BUILD)$(CONFIG_RTE_BUILD_SHARED_LIB),yy)
 LDFLAGS += -rpath=$(RTE_SDK_BIN)/lib
-- 
2.4.11

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v4 2/2] mk: fix acl library static linking
  2016-07-01 14:38     ` [dpdk-dev] [PATCH v4 1/2] mk: allow duplicate linker flags in libraries list Sergio Gonzalez Monroy
@ 2016-07-01 14:38       ` Sergio Gonzalez Monroy
  2016-07-01 14:45         ` Thomas Monjalon
  0 siblings, 1 reply; 22+ messages in thread
From: Sergio Gonzalez Monroy @ 2016-07-01 14:38 UTC (permalink / raw)
  To: dev; +Cc: thomas.monjalon, konstantin.ananyev

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>
---
v4:
- keep order of acl in _LDLIBS and wrap it with --whole-archive

v3:
- patch version history
- carry Acked-by

v2:
- add comment in makefile

 mk/rte.app.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index bf8bcf9..ea22961 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -76,7 +76,10 @@ _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
+# librte_acl needs --whole-archive because of weak functions
+_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
+_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS)       += -lrte_jobstats
 _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)          += -lrte_power
 
-- 
2.4.11

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v4 2/2] mk: fix acl library static linking
  2016-07-01 14:38       ` [dpdk-dev] [PATCH v4 2/2] mk: fix acl library static linking Sergio Gonzalez Monroy
@ 2016-07-01 14:45         ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2016-07-01 14:45 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev, konstantin.ananyev

2016-07-01 15:38, Sergio Gonzalez Monroy:
> 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>

Series applied, thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-07-01 14:45 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-30 11:10 [dpdk-dev] [PATCH] mk: fix acl library static linking Sergio Gonzalez Monroy
2016-06-30 11:38 ` Thomas Monjalon
2016-06-30 12:04   ` Sergio Gonzalez Monroy
2016-06-30 12:44     ` Thomas Monjalon
2016-06-30 14:02       ` Sergio Gonzalez Monroy
2016-06-30 15:24         ` Ananyev, Konstantin
2016-06-30 15:47           ` Thomas Monjalon
2016-06-30 15:28         ` Thomas Monjalon
2016-06-30 15:58           ` Sergio Gonzalez Monroy
2016-06-30 12:14 ` Ananyev, Konstantin
2016-06-30 16:01 ` [dpdk-dev] [PATCH v2] " Sergio Gonzalez Monroy
2016-06-30 16:10   ` Thomas Monjalon
2016-06-30 16:14     ` Sergio Gonzalez Monroy
2016-06-30 16:22       ` Thomas Monjalon
2016-07-01  8:05         ` Sergio Gonzalez Monroy
2016-07-01 10:05           ` Thomas Monjalon
2016-07-01 10:27             ` Sergio Gonzalez Monroy
2016-07-01 10:39               ` Thomas Monjalon
2016-06-30 16:11   ` [dpdk-dev] [PATCH v3] " Sergio Gonzalez Monroy
2016-07-01 14:38     ` [dpdk-dev] [PATCH v4 1/2] mk: allow duplicate linker flags in libraries list Sergio Gonzalez Monroy
2016-07-01 14:38       ` [dpdk-dev] [PATCH v4 2/2] mk: fix acl library static linking Sergio Gonzalez Monroy
2016-07-01 14:45         ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).