Thread (22 messages) 22 messages, 3 authors, 2016-07-01

Re: [PATCH v2] mk: fix acl library static linking

From: Thomas Monjalon <hidden>
Date: 2016-07-01 10:39:06

2016-07-01 11:27, Sergio Gonzalez Monroy:
On 01/07/2016 11:05, Thomas Monjalon wrote:
quoted
2016-07-01 09:05, Sergio Gonzalez Monroy:
quoted
On 30/06/2016 17:22, Thomas Monjalon wrote:
quoted
2016-06-30 17:14, Sergio Gonzalez Monroy:
quoted
On 30/06/2016 17:10, Thomas Monjalon wrote:
quoted
2016-06-30 17:01, Sergio Gonzalez Monroy:
quoted
--- 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 :)
quoted
How are timer, hash and vhost in the same group?
It is far from perfect and subject to improvements :)
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help