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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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.