* [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled @ 2016-06-22 11:34 Panu Matilainen 2016-06-22 11:49 ` Thomas Monjalon 2016-06-23 17:19 ` Dumitrescu, Cristian 0 siblings, 2 replies; 9+ messages in thread From: Panu Matilainen @ 2016-06-22 11:34 UTC (permalink / raw) To: dev; +Cc: cristian.dumitrescu, zhuangwj Commit 9fc37d1c071c is missing a conditional in the dependencies, causing builds to fail when KNI is not enabled: == Build lib/librte_port LD librte_port.so.3 /usr/bin/ld: cannot find -lrte_kni collect2: error: ld returned 1 exit status Fixes: 9fc37d1c071c ("port: support KNI") Signed-off-by: Panu Matilainen <pmatilai@redhat.com> --- lib/librte_port/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile index 0fc929b..3d84a0e 100644 --- a/lib/librte_port/Makefile +++ b/lib/librte_port/Makefile @@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched +ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni +endif include $(RTE_SDK)/mk/rte.lib.mk -- 2.5.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-22 11:34 [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled Panu Matilainen @ 2016-06-22 11:49 ` Thomas Monjalon 2016-06-22 11:57 ` Olivier Matz 2016-06-23 17:19 ` Dumitrescu, Cristian 1 sibling, 1 reply; 9+ messages in thread From: Thomas Monjalon @ 2016-06-22 11:49 UTC (permalink / raw) To: Panu Matilainen; +Cc: dev, cristian.dumitrescu, zhuangwj, olivier.matz 2016-06-22 14:34, Panu Matilainen: > --- a/lib/librte_port/Makefile > +++ b/lib/librte_port/Makefile > @@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool > DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether > DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag > DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched > +ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni > +endif I do not remember why $(CONFIG_RTE_LIBRTE_PORT) is needed in its Makefile. I think we can do DEPDIRS-$(CONFIG_RTE_LIBRTE_KNI) += lib/librte_kni and set DEPDIRS-y everywhere else. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-22 11:49 ` Thomas Monjalon @ 2016-06-22 11:57 ` Olivier Matz 2016-06-22 12:20 ` Thomas Monjalon 0 siblings, 1 reply; 9+ messages in thread From: Olivier Matz @ 2016-06-22 11:57 UTC (permalink / raw) To: Thomas Monjalon, Panu Matilainen; +Cc: dev, cristian.dumitrescu, zhuangwj Hi Thomas, On 06/22/2016 01:49 PM, Thomas Monjalon wrote: > 2016-06-22 14:34, Panu Matilainen: >> --- a/lib/librte_port/Makefile >> +++ b/lib/librte_port/Makefile >> @@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched >> +ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni >> +endif > > I do not remember why $(CONFIG_RTE_LIBRTE_PORT) is needed in its Makefile. > I think we can do > DEPDIRS-$(CONFIG_RTE_LIBRTE_KNI) += lib/librte_kni > and set DEPDIRS-y everywhere else. > It's probably not much used, but the build framework allows to do the following to build only one directory: make lib/librte_port_sub This directly jumps to the librte_port Makefile, bypassing parent directories. I think that's why the config check is duplicated in the Makefile. Olivier ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-22 11:57 ` Olivier Matz @ 2016-06-22 12:20 ` Thomas Monjalon 2016-06-22 15:32 ` Olivier Matz 0 siblings, 1 reply; 9+ messages in thread From: Thomas Monjalon @ 2016-06-22 12:20 UTC (permalink / raw) To: Olivier Matz; +Cc: Panu Matilainen, dev, cristian.dumitrescu, zhuangwj 2016-06-22 13:57, Olivier Matz: > Hi Thomas, > > On 06/22/2016 01:49 PM, Thomas Monjalon wrote: > > 2016-06-22 14:34, Panu Matilainen: > >> --- a/lib/librte_port/Makefile > >> +++ b/lib/librte_port/Makefile > >> @@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool > >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether > >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag > >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched > >> +ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni > >> +endif > > > > I do not remember why $(CONFIG_RTE_LIBRTE_PORT) is needed in its Makefile. > > I think we can do > > DEPDIRS-$(CONFIG_RTE_LIBRTE_KNI) += lib/librte_kni > > and set DEPDIRS-y everywhere else. > > > > It's probably not much used, but the build framework allows to do > the following to build only one directory: > > make lib/librte_port_sub > > This directly jumps to the librte_port Makefile, bypassing parent > directories. I think that's why the config check is duplicated in the > Makefile. If we want to specifically build this directory, why preventing us to do so with CONFIG_RTE_LIBRTE_PORT? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-22 12:20 ` Thomas Monjalon @ 2016-06-22 15:32 ` Olivier Matz 2016-06-23 8:53 ` Bruce Richardson 0 siblings, 1 reply; 9+ messages in thread From: Olivier Matz @ 2016-06-22 15:32 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Panu Matilainen, dev, cristian.dumitrescu, zhuangwj On 06/22/2016 02:20 PM, Thomas Monjalon wrote: > 2016-06-22 13:57, Olivier Matz: >> Hi Thomas, >> >> On 06/22/2016 01:49 PM, Thomas Monjalon wrote: >>> 2016-06-22 14:34, Panu Matilainen: >>>> --- a/lib/librte_port/Makefile >>>> +++ b/lib/librte_port/Makefile >>>> @@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched >>>> +ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni >>>> +endif >>> >>> I do not remember why $(CONFIG_RTE_LIBRTE_PORT) is needed in its Makefile. >>> I think we can do >>> DEPDIRS-$(CONFIG_RTE_LIBRTE_KNI) += lib/librte_kni >>> and set DEPDIRS-y everywhere else. >>> >> >> It's probably not much used, but the build framework allows to do >> the following to build only one directory: >> >> make lib/librte_port_sub >> >> This directly jumps to the librte_port Makefile, bypassing parent >> directories. I think that's why the config check is duplicated in the >> Makefile. > > If we want to specifically build this directory, why preventing us to do > so with CONFIG_RTE_LIBRTE_PORT? If we call foo_sub with CONFIG_FOO=n, it will generate a library and install headers in the build directory, however the config is unset. Some propositions if we want to replace DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) by DEPDIRS-y: 1/ say that "make foo_sub" should be used with care, only if CONFIG_FOO is set (else it is not supported) -> nothing to do 2/ fix the make %_sub feature to browse parent directories, checking the SUBDIRS-${CONFIG_FOO} 3/ remove the make %_sub feature, maybe nobody cares... I think 1/ is acceptable. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-22 15:32 ` Olivier Matz @ 2016-06-23 8:53 ` Bruce Richardson 2016-06-23 8:59 ` Olivier Matz 0 siblings, 1 reply; 9+ messages in thread From: Bruce Richardson @ 2016-06-23 8:53 UTC (permalink / raw) To: Olivier Matz Cc: Thomas Monjalon, Panu Matilainen, dev, cristian.dumitrescu, zhuangwj On Wed, Jun 22, 2016 at 05:32:37PM +0200, Olivier Matz wrote: > On 06/22/2016 02:20 PM, Thomas Monjalon wrote: > > 2016-06-22 13:57, Olivier Matz: > >> Hi Thomas, > >> > >> On 06/22/2016 01:49 PM, Thomas Monjalon wrote: > >>> 2016-06-22 14:34, Panu Matilainen: > >>>> --- a/lib/librte_port/Makefile > >>>> +++ b/lib/librte_port/Makefile > >>>> @@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool > >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether > >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag > >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched > >>>> +ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > >>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni > >>>> +endif > >>> > >>> I do not remember why $(CONFIG_RTE_LIBRTE_PORT) is needed in its Makefile. > >>> I think we can do > >>> DEPDIRS-$(CONFIG_RTE_LIBRTE_KNI) += lib/librte_kni > >>> and set DEPDIRS-y everywhere else. > >>> > >> > >> It's probably not much used, but the build framework allows to do > >> the following to build only one directory: > >> > >> make lib/librte_port_sub > >> > >> This directly jumps to the librte_port Makefile, bypassing parent > >> directories. I think that's why the config check is duplicated in the > >> Makefile. > > > > If we want to specifically build this directory, why preventing us to do > > so with CONFIG_RTE_LIBRTE_PORT? > > If we call foo_sub with CONFIG_FOO=n, it will generate a library and > install headers in the build directory, however the config is unset. > Some propositions if we want to replace > DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) by DEPDIRS-y: > > 1/ say that "make foo_sub" should be used with care, only if CONFIG_FOO > is set (else it is not supported) -> nothing to do > 2/ fix the make %_sub feature to browse parent directories, checking > the SUBDIRS-${CONFIG_FOO} > 3/ remove the make %_sub feature, maybe nobody cares... > > I think 1/ is acceptable. +1 Especially given the fact I wasn't even aware that you could do that (building just one subdir). Do we have a usecase where people might want to build just one DPDK subdirectory? /Bruce ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-23 8:53 ` Bruce Richardson @ 2016-06-23 8:59 ` Olivier Matz 0 siblings, 0 replies; 9+ messages in thread From: Olivier Matz @ 2016-06-23 8:59 UTC (permalink / raw) To: Bruce Richardson Cc: Thomas Monjalon, Panu Matilainen, dev, cristian.dumitrescu, zhuangwj Hi Bruce, On 06/23/2016 10:53 AM, Bruce Richardson wrote: >>> If we want to specifically build this directory, why preventing us to do >>> so with CONFIG_RTE_LIBRTE_PORT? >> >> If we call foo_sub with CONFIG_FOO=n, it will generate a library and >> install headers in the build directory, however the config is unset. >> Some propositions if we want to replace >> DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) by DEPDIRS-y: >> >> 1/ say that "make foo_sub" should be used with care, only if CONFIG_FOO >> is set (else it is not supported) -> nothing to do >> 2/ fix the make %_sub feature to browse parent directories, checking >> the SUBDIRS-${CONFIG_FOO} >> 3/ remove the make %_sub feature, maybe nobody cares... >> >> I think 1/ is acceptable. > > +1 > > Especially given the fact I wasn't even aware that you could do that (building > just one subdir). Do we have a usecase where people might want to build just one > DPDK subdirectory? The only use-case I see is to gain few seconds when recompiling: it can avoid to check deps in all directories if you know that your modifications only take place in a specific directory. Well, nothing really essential ;) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-22 11:34 [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled Panu Matilainen 2016-06-22 11:49 ` Thomas Monjalon @ 2016-06-23 17:19 ` Dumitrescu, Cristian 2016-06-27 10:25 ` Thomas Monjalon 1 sibling, 1 reply; 9+ messages in thread From: Dumitrescu, Cristian @ 2016-06-23 17:19 UTC (permalink / raw) To: Panu Matilainen, dev; +Cc: zhuangwj > -----Original Message----- > From: Panu Matilainen [mailto:pmatilai@redhat.com] > Sent: Wednesday, June 22, 2016 12:34 PM > To: dev@dpdk.org > Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; > zhuangwj@gmail.com > Subject: [PATCH] port: fix build when KNI support is not enabled > > Commit 9fc37d1c071c is missing a conditional in the dependencies, > causing builds to fail when KNI is not enabled: > == Build lib/librte_port > LD librte_port.so.3 > /usr/bin/ld: cannot find -lrte_kni > collect2: error: ld returned 1 exit status > > Fixes: 9fc37d1c071c ("port: support KNI") > > Signed-off-by: Panu Matilainen <pmatilai@redhat.com> > --- Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled 2016-06-23 17:19 ` Dumitrescu, Cristian @ 2016-06-27 10:25 ` Thomas Monjalon 0 siblings, 0 replies; 9+ messages in thread From: Thomas Monjalon @ 2016-06-27 10:25 UTC (permalink / raw) To: Panu Matilainen; +Cc: dev, Dumitrescu, Cristian, zhuangwj > > Commit 9fc37d1c071c is missing a conditional in the dependencies, > > causing builds to fail when KNI is not enabled: > > == Build lib/librte_port > > LD librte_port.so.3 > > /usr/bin/ld: cannot find -lrte_kni > > collect2: error: ld returned 1 exit status > > > > Fixes: 9fc37d1c071c ("port: support KNI") > > > > Signed-off-by: Panu Matilainen <pmatilai@redhat.com> > > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com> Applied, thanks A cleanup of the makefiles is welcome, as discussed in this thread, to remove useless reference to their own config variable. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-06-27 10:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-22 11:34 [dpdk-dev] [PATCH] port: fix build when KNI support is not enabled Panu Matilainen 2016-06-22 11:49 ` Thomas Monjalon 2016-06-22 11:57 ` Olivier Matz 2016-06-22 12:20 ` Thomas Monjalon 2016-06-22 15:32 ` Olivier Matz 2016-06-23 8:53 ` Bruce Richardson 2016-06-23 8:59 ` Olivier Matz 2016-06-23 17:19 ` Dumitrescu, Cristian 2016-06-27 10:25 ` 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).