[dpdk-dev] config: default to shared library

Message ID c826d83d78b0d24d0d840775b6ff6afaa848320c.1425449872.git.pmatilai@redhat.com (mailing list archive)
State Rejected, archived
Headers

Commit Message

Panu Matilainen March 4, 2015, 6:17 a.m. UTC
  With symbol versioning its vital that developers test their code in
shared library mode, otherwise we'll be playing "add the forgotten
symbol export" from here to eternity. By defaulting to shared we
should catch more of these cases early, but without taking away anybodys
ability to build static.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 config/common_bsdapp   | 2 +-
 config/common_linuxapp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Thomas Monjalon March 4, 2015, 9:24 a.m. UTC | #1
Hi Panu,

2015-03-04 08:17, Panu Matilainen:
> With symbol versioning its vital that developers test their code in
> shared library mode, otherwise we'll be playing "add the forgotten
> symbol export" from here to eternity.

Yes we must improve the sanity checks.
A lot of options must be tested (or removed) and not only shared libs.
But the error you reported before (missing export of rte_eth_dev_release_port)
cannot be seen even with this patch.
It means we need more tools.
Though, default configuration is not a tool.

> By defaulting to shared we should catch more of these cases early,
> but without taking away anybodys ability to build static.

Shared libraries are convenient for distributions but have a performance
impact. I think that static build must remain the default choice.
  
Bruce Richardson March 4, 2015, 10:42 a.m. UTC | #2
On Wed, Mar 04, 2015 at 10:24:44AM +0100, Thomas Monjalon wrote:
> Hi Panu,
> 
> 2015-03-04 08:17, Panu Matilainen:
> > With symbol versioning its vital that developers test their code in
> > shared library mode, otherwise we'll be playing "add the forgotten
> > symbol export" from here to eternity.
> 
> Yes we must improve the sanity checks.
> A lot of options must be tested (or removed) and not only shared libs.
> But the error you reported before (missing export of rte_eth_dev_release_port)
> cannot be seen even with this patch.
> It means we need more tools.
> Though, default configuration is not a tool.
> 
> > By defaulting to shared we should catch more of these cases early,
> > but without taking away anybodys ability to build static.
> 
> Shared libraries are convenient for distributions but have a performance
> impact. I think that static build must remain the default choice.
> 
+1
  
Panu Matilainen March 4, 2015, 11:05 a.m. UTC | #3
On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> Hi Panu,
>
> 2015-03-04 08:17, Panu Matilainen:
>> With symbol versioning its vital that developers test their code in
>> shared library mode, otherwise we'll be playing "add the forgotten
>> symbol export" from here to eternity.
>
> Yes we must improve the sanity checks.
> A lot of options must be tested (or removed) and not only shared libs.
> But the error you reported before (missing export of rte_eth_dev_release_port)
> cannot be seen even with this patch.

I know, I didn't say it would have directly caught it. It would've 
likely been found earlier though, if nothing else then in testing of the 
new librte_pmd_null which clearly nobody had tried in shared lib 
configuration.

> It means we need more tools.
> Though, default configuration is not a tool.

Yes, default config is not a tool, its a recommendation of sorts both 
for developers and users. It also tends to be the setup that is rarely 
broken because it happens to get the most testing :)

>
>> By defaulting to shared we should catch more of these cases early,
>> but without taking away anybodys ability to build static.
>
> Shared libraries are convenient for distributions but have a performance
> impact. I think that static build must remain the default choice.

For distros, this is not a matter of *convenience*, its the only 
technically feasible choice.

I didn't want to make the commit message into a shared library sermon, 
but if you look at the OSS landscape overall the common wisdom is that 
shared library benefits outweigh any performance impact by so much that 
static libs are almost nowhere to be found. I can change the text into a 
full-blown rationale why shared libraries should be the default if that 
makes any difference.

	- Panu -
  
Neil Horman March 4, 2015, 11:28 a.m. UTC | #4
On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
> On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> >Hi Panu,
> >
> >2015-03-04 08:17, Panu Matilainen:
> >>With symbol versioning its vital that developers test their code in
> >>shared library mode, otherwise we'll be playing "add the forgotten
> >>symbol export" from here to eternity.
> >
> >Yes we must improve the sanity checks.
> >A lot of options must be tested (or removed) and not only shared libs.
> >But the error you reported before (missing export of rte_eth_dev_release_port)
> >cannot be seen even with this patch.
> 
> I know, I didn't say it would have directly caught it. It would've likely
> been found earlier though, if nothing else then in testing of the new
> librte_pmd_null which clearly nobody had tried in shared lib configuration.
> 
This is accurate.  The default config is a tool, in the sense that it leverages
the implicit testing of any users who are experimenting with the DPDK.  Any
users out there using the DPDK test/example applications would have realized
something was amiss when the testpmd app refused to run with the null or pcap
pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
only works if the defaults are carefully selected.  Currently, building for
shared libraries exposes more existing bugs than static libraries, and so we
should set that as our default so as to catch them.

> >It means we need more tools.
> >Though, default configuration is not a tool.
> 
> Yes, default config is not a tool, its a recommendation of sorts both for
> developers and users. It also tends to be the setup that is rarely broken
> because it happens to get the most testing :)
> 
And it is a tool (see above).

> >
> >>By defaulting to shared we should catch more of these cases early,
> >>but without taking away anybodys ability to build static.
> >
> >Shared libraries are convenient for distributions but have a performance
> >impact. I think that static build must remain the default choice.
> 

If utmost performance is the concern, isn't it reasonable to assume that users
in that demographic will customize their configuration to achieve that?  No one
assumes that something is tuned to be perfect for their needs out of the box if
their needs are extreemely biased to a single quality.  The best course of
action here is to set the default to be adventageous toward catching bugs, and
document the changes needed to bias for performance.

> For distros, this is not a matter of *convenience*, its the only technically
> feasible choice.
> 
> I didn't want to make the commit message into a shared library sermon, but
> if you look at the OSS landscape overall the common wisdom is that shared
> library benefits outweigh any performance impact by so much that static libs
> are almost nowhere to be found. I can change the text into a full-blown
> rationale why shared libraries should be the default if that makes any
> difference.
> 
Embedded applications actually do make extensive use of static linking to try
achieve greater performance, but they tend to be proprietary, and as such are
the exception that proves the rule.  Once an application itself becomes open
source, it biases toward shared libraries, because the minor performance impact
is well worth the increased manageability and security found in DSO's

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> 	- Panu -
> 
>
  
Thomas Monjalon March 4, 2015, 11:29 a.m. UTC | #5
2015-03-04 13:05, Panu Matilainen:
> On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> > Hi Panu,
> >
> > 2015-03-04 08:17, Panu Matilainen:
> >> With symbol versioning its vital that developers test their code in
> >> shared library mode, otherwise we'll be playing "add the forgotten
> >> symbol export" from here to eternity.
> >
> > Yes we must improve the sanity checks.
> > A lot of options must be tested (or removed) and not only shared libs.
> > But the error you reported before (missing export of rte_eth_dev_release_port)
> > cannot be seen even with this patch.
> 
> I know, I didn't say it would have directly caught it. It would've 
> likely been found earlier though, if nothing else then in testing of the 
> new librte_pmd_null which clearly nobody had tried in shared lib 
> configuration.

Exactly, there was no test in shared lib mode.
Then I recommend having a basic testpmd trial which should be easy with
PMD null.
Any script to ease this test is welcome.

> > It means we need more tools.
> > Though, default configuration is not a tool.
> 
> Yes, default config is not a tool, its a recommendation of sorts both 
> for developers and users. It also tends to be the setup that is rarely 
> broken because it happens to get the most testing :)
> 
> >
> >> By defaulting to shared we should catch more of these cases early,
> >> but without taking away anybodys ability to build static.
> >
> > Shared libraries are convenient for distributions but have a performance
> > impact. I think that static build must remain the default choice.
> 
> For distros, this is not a matter of *convenience*, its the only 
> technically feasible choice.

"only" is a religious word here :) We could imagine an automatic rebuild
of DPDK applications when DPDK is updated. But it's not the topic :)

> I didn't want to make the commit message into a shared library sermon, 
> but if you look at the OSS landscape overall the common wisdom is that 
> shared library benefits outweigh any performance impact by so much that 
> static libs are almost nowhere to be found. I can change the text into a 
> full-blown rationale why shared libraries should be the default if that 
> makes any difference.

No, as Bruce agreed, it's better to keep static as default, because DPDK is
performance-oriented.
I agree we really have a problem with tests and I'd prefer we solve it by
adding some scripts. I'm currently thinking about which scripts to add,
like building different configurations. Then the second step will be to
automate the launch of these scripts to catch wrong submissions earlier.
Maybe that something like patchew (http://qemu.patchew.org) could help for
the second step (thanks David for the tip).
  
Bruce Richardson March 4, 2015, 1:08 p.m. UTC | #6
On Wed, Mar 04, 2015 at 06:28:05AM -0500, Neil Horman wrote:
> On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
> > On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> > >Hi Panu,
> > >
> > >2015-03-04 08:17, Panu Matilainen:
> > >>With symbol versioning its vital that developers test their code in
> > >>shared library mode, otherwise we'll be playing "add the forgotten
> > >>symbol export" from here to eternity.
> > >
> > >Yes we must improve the sanity checks.
> > >A lot of options must be tested (or removed) and not only shared libs.
> > >But the error you reported before (missing export of rte_eth_dev_release_port)
> > >cannot be seen even with this patch.
> > 
> > I know, I didn't say it would have directly caught it. It would've likely
> > been found earlier though, if nothing else then in testing of the new
> > librte_pmd_null which clearly nobody had tried in shared lib configuration.
> > 
> This is accurate.  The default config is a tool, in the sense that it leverages
> the implicit testing of any users who are experimenting with the DPDK.  Any
> users out there using the DPDK test/example applications would have realized
> something was amiss when the testpmd app refused to run with the null or pcap
> pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
> only works if the defaults are carefully selected.  Currently, building for
> shared libraries exposes more existing bugs than static libraries, and so we
> should set that as our default so as to catch them.
> 
> > >It means we need more tools.
> > >Though, default configuration is not a tool.
> > 
> > Yes, default config is not a tool, its a recommendation of sorts both for
> > developers and users. It also tends to be the setup that is rarely broken
> > because it happens to get the most testing :)
> > 
> And it is a tool (see above).
> 
> > >
> > >>By defaulting to shared we should catch more of these cases early,
> > >>but without taking away anybodys ability to build static.
> > >
> > >Shared libraries are convenient for distributions but have a performance
> > >impact. I think that static build must remain the default choice.
> > 
> 
> If utmost performance is the concern, isn't it reasonable to assume that users
> in that demographic will customize their configuration to achieve that?  No one
> assumes that something is tuned to be perfect for their needs out of the box if
> their needs are extreemely biased to a single quality.  The best course of
> action here is to set the default to be adventageous toward catching bugs, and
> document the changes needed to bias for performance.
> 
> > For distros, this is not a matter of *convenience*, its the only technically
> > feasible choice.

As I understand it, build for the "default" cpu rather than "native" is the only
feasible choice also, so how about re-introducing a new defconfig file for
"default" (or perhaps better name), where you have lowest-common denominator
instruction-set and building for shared libraries?
Would that work for everyone, or do people feel it would be too confusing to have
more defconfig files available?

/Bruce

> > 
> > I didn't want to make the commit message into a shared library sermon, but
> > if you look at the OSS landscape overall the common wisdom is that shared
> > library benefits outweigh any performance impact by so much that static libs
> > are almost nowhere to be found. I can change the text into a full-blown
> > rationale why shared libraries should be the default if that makes any
> > difference.
> > 
> Embedded applications actually do make extensive use of static linking to try
> achieve greater performance, but they tend to be proprietary, and as such are
> the exception that proves the rule.  Once an application itself becomes open
> source, it biases toward shared libraries, because the minor performance impact
> is well worth the increased manageability and security found in DSO's
> 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> 
> > 	- Panu -
> > 
> >
  
Panu Matilainen March 4, 2015, 1:24 p.m. UTC | #7
On 03/04/2015 03:08 PM, Bruce Richardson wrote:
> On Wed, Mar 04, 2015 at 06:28:05AM -0500, Neil Horman wrote:
>> On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
>>> On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
>>>> Hi Panu,
>>>>
>>>> 2015-03-04 08:17, Panu Matilainen:
>>>>> With symbol versioning its vital that developers test their code in
>>>>> shared library mode, otherwise we'll be playing "add the forgotten
>>>>> symbol export" from here to eternity.
>>>>
>>>> Yes we must improve the sanity checks.
>>>> A lot of options must be tested (or removed) and not only shared libs.
>>>> But the error you reported before (missing export of rte_eth_dev_release_port)
>>>> cannot be seen even with this patch.
>>>
>>> I know, I didn't say it would have directly caught it. It would've likely
>>> been found earlier though, if nothing else then in testing of the new
>>> librte_pmd_null which clearly nobody had tried in shared lib configuration.
>>>
>> This is accurate.  The default config is a tool, in the sense that it leverages
>> the implicit testing of any users who are experimenting with the DPDK.  Any
>> users out there using the DPDK test/example applications would have realized
>> something was amiss when the testpmd app refused to run with the null or pcap
>> pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
>> only works if the defaults are carefully selected.  Currently, building for
>> shared libraries exposes more existing bugs than static libraries, and so we
>> should set that as our default so as to catch them.
>>
>>>> It means we need more tools.
>>>> Though, default configuration is not a tool.
>>>
>>> Yes, default config is not a tool, its a recommendation of sorts both for
>>> developers and users. It also tends to be the setup that is rarely broken
>>> because it happens to get the most testing :)
>>>
>> And it is a tool (see above).
>>
>>>>
>>>>> By defaulting to shared we should catch more of these cases early,
>>>>> but without taking away anybodys ability to build static.
>>>>
>>>> Shared libraries are convenient for distributions but have a performance
>>>> impact. I think that static build must remain the default choice.
>>>
>>
>> If utmost performance is the concern, isn't it reasonable to assume that users
>> in that demographic will customize their configuration to achieve that?  No one
>> assumes that something is tuned to be perfect for their needs out of the box if
>> their needs are extreemely biased to a single quality.  The best course of
>> action here is to set the default to be adventageous toward catching bugs, and
>> document the changes needed to bias for performance.
>>
>>> For distros, this is not a matter of *convenience*, its the only technically
>>> feasible choice.
>
> As I understand it, build for the "default" cpu rather than "native" is the only
> feasible choice also, so how about re-introducing a new defconfig file for
> "default" (or perhaps better name), where you have lowest-common denominator
> instruction-set and building for shared libraries?
> Would that work for everyone, or do people feel it would be too confusing to have
> more defconfig files available?

Given the opposition to defaulting to shared, another config file seems 
like a fair compromise to me, whether "default" or something else. As 
for the naming, one possibility would be calling it "shared", implying 
both lowest-common denominator instruction set to be shareable across 
many systems and shared libraries.

	- Panu -
  
Bruce Richardson March 4, 2015, 1:31 p.m. UTC | #8
On Wed, Mar 04, 2015 at 03:24:12PM +0200, Panu Matilainen wrote:
> On 03/04/2015 03:08 PM, Bruce Richardson wrote:
> >On Wed, Mar 04, 2015 at 06:28:05AM -0500, Neil Horman wrote:
> >>On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
> >>>On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> >>>>Hi Panu,
> >>>>
> >>>>2015-03-04 08:17, Panu Matilainen:
> >>>>>With symbol versioning its vital that developers test their code in
> >>>>>shared library mode, otherwise we'll be playing "add the forgotten
> >>>>>symbol export" from here to eternity.
> >>>>
> >>>>Yes we must improve the sanity checks.
> >>>>A lot of options must be tested (or removed) and not only shared libs.
> >>>>But the error you reported before (missing export of rte_eth_dev_release_port)
> >>>>cannot be seen even with this patch.
> >>>
> >>>I know, I didn't say it would have directly caught it. It would've likely
> >>>been found earlier though, if nothing else then in testing of the new
> >>>librte_pmd_null which clearly nobody had tried in shared lib configuration.
> >>>
> >>This is accurate.  The default config is a tool, in the sense that it leverages
> >>the implicit testing of any users who are experimenting with the DPDK.  Any
> >>users out there using the DPDK test/example applications would have realized
> >>something was amiss when the testpmd app refused to run with the null or pcap
> >>pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
> >>only works if the defaults are carefully selected.  Currently, building for
> >>shared libraries exposes more existing bugs than static libraries, and so we
> >>should set that as our default so as to catch them.
> >>
> >>>>It means we need more tools.
> >>>>Though, default configuration is not a tool.
> >>>
> >>>Yes, default config is not a tool, its a recommendation of sorts both for
> >>>developers and users. It also tends to be the setup that is rarely broken
> >>>because it happens to get the most testing :)
> >>>
> >>And it is a tool (see above).
> >>
> >>>>
> >>>>>By defaulting to shared we should catch more of these cases early,
> >>>>>but without taking away anybodys ability to build static.
> >>>>
> >>>>Shared libraries are convenient for distributions but have a performance
> >>>>impact. I think that static build must remain the default choice.
> >>>
> >>
> >>If utmost performance is the concern, isn't it reasonable to assume that users
> >>in that demographic will customize their configuration to achieve that?  No one
> >>assumes that something is tuned to be perfect for their needs out of the box if
> >>their needs are extreemely biased to a single quality.  The best course of
> >>action here is to set the default to be adventageous toward catching bugs, and
> >>document the changes needed to bias for performance.
> >>
> >>>For distros, this is not a matter of *convenience*, its the only technically
> >>>feasible choice.
> >
> >As I understand it, build for the "default" cpu rather than "native" is the only
> >feasible choice also, so how about re-introducing a new defconfig file for
> >"default" (or perhaps better name), where you have lowest-common denominator
> >instruction-set and building for shared libraries?
> >Would that work for everyone, or do people feel it would be too confusing to have
> >more defconfig files available?
> 
> Given the opposition to defaulting to shared, another config file seems like
> a fair compromise to me, whether "default" or something else. As for the
> naming, one possibility would be calling it "shared", implying both
> lowest-common denominator instruction set to be shareable across many
> systems and shared libraries.
> 
> 	- Panu -

The naming scheme for configs is meant to be:
"ARCH-MACHINE-EXECENV-TOOLCHAIN" 
as documented in the Getting Started Guide. "Default" has been used up till now
to refer to the lowest common denominator instruction set supported, which for
x86_64 is a core2 baseline, I believe. "shared" doesn't really fit into this
naming scheme, and there is nothing to allow extra notes to be added to the
name.
Without changing this scheme, I would suggest we rename "default" to "generic",
which I think is a slightly better term for it, and we set the
"x86_64-generic-linuxapp-gcc" target to build shared libs.

/Bruce
  
Panu Matilainen March 4, 2015, 1:41 p.m. UTC | #9
On 03/04/2015 03:31 PM, Bruce Richardson wrote:
> On Wed, Mar 04, 2015 at 03:24:12PM +0200, Panu Matilainen wrote:
>> On 03/04/2015 03:08 PM, Bruce Richardson wrote:
>>> On Wed, Mar 04, 2015 at 06:28:05AM -0500, Neil Horman wrote:
>>>> On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
>>>>> On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
>>>>>> Hi Panu,
>>>>>>
>>>>>> 2015-03-04 08:17, Panu Matilainen:
>>>>>>> With symbol versioning its vital that developers test their code in
>>>>>>> shared library mode, otherwise we'll be playing "add the forgotten
>>>>>>> symbol export" from here to eternity.
>>>>>>
>>>>>> Yes we must improve the sanity checks.
>>>>>> A lot of options must be tested (or removed) and not only shared libs.
>>>>>> But the error you reported before (missing export of rte_eth_dev_release_port)
>>>>>> cannot be seen even with this patch.
>>>>>
>>>>> I know, I didn't say it would have directly caught it. It would've likely
>>>>> been found earlier though, if nothing else then in testing of the new
>>>>> librte_pmd_null which clearly nobody had tried in shared lib configuration.
>>>>>
>>>> This is accurate.  The default config is a tool, in the sense that it leverages
>>>> the implicit testing of any users who are experimenting with the DPDK.  Any
>>>> users out there using the DPDK test/example applications would have realized
>>>> something was amiss when the testpmd app refused to run with the null or pcap
>>>> pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
>>>> only works if the defaults are carefully selected.  Currently, building for
>>>> shared libraries exposes more existing bugs than static libraries, and so we
>>>> should set that as our default so as to catch them.
>>>>
>>>>>> It means we need more tools.
>>>>>> Though, default configuration is not a tool.
>>>>>
>>>>> Yes, default config is not a tool, its a recommendation of sorts both for
>>>>> developers and users. It also tends to be the setup that is rarely broken
>>>>> because it happens to get the most testing :)
>>>>>
>>>> And it is a tool (see above).
>>>>
>>>>>>
>>>>>>> By defaulting to shared we should catch more of these cases early,
>>>>>>> but without taking away anybodys ability to build static.
>>>>>>
>>>>>> Shared libraries are convenient for distributions but have a performance
>>>>>> impact. I think that static build must remain the default choice.
>>>>>
>>>>
>>>> If utmost performance is the concern, isn't it reasonable to assume that users
>>>> in that demographic will customize their configuration to achieve that?  No one
>>>> assumes that something is tuned to be perfect for their needs out of the box if
>>>> their needs are extreemely biased to a single quality.  The best course of
>>>> action here is to set the default to be adventageous toward catching bugs, and
>>>> document the changes needed to bias for performance.
>>>>
>>>>> For distros, this is not a matter of *convenience*, its the only technically
>>>>> feasible choice.
>>>
>>> As I understand it, build for the "default" cpu rather than "native" is the only
>>> feasible choice also, so how about re-introducing a new defconfig file for
>>> "default" (or perhaps better name), where you have lowest-common denominator
>>> instruction-set and building for shared libraries?
>>> Would that work for everyone, or do people feel it would be too confusing to have
>>> more defconfig files available?
>>
>> Given the opposition to defaulting to shared, another config file seems like
>> a fair compromise to me, whether "default" or something else. As for the
>> naming, one possibility would be calling it "shared", implying both
>> lowest-common denominator instruction set to be shareable across many
>> systems and shared libraries.
>>
>> 	- Panu -
>
> The naming scheme for configs is meant to be:
> "ARCH-MACHINE-EXECENV-TOOLCHAIN"
> as documented in the Getting Started Guide. "Default" has been used up till now
> to refer to the lowest common denominator instruction set supported, which for
> x86_64 is a core2 baseline, I believe. "shared" doesn't really fit into this
> naming scheme, and there is nothing to allow extra notes to be added to the
> name.

Right, but then there's "ivshmem" that doesn't fit that description 
either AFAICS.

> Without changing this scheme, I would suggest we rename "default" to "generic",
> which I think is a slightly better term for it, and we set the
> "x86_64-generic-linuxapp-gcc" target to build shared libs.

Works for me. It is indeed more descriptive than either "default" or 
"shared" for the purpose.

	- Panu -
  
Bruce Richardson March 4, 2015, 1:49 p.m. UTC | #10
On Wed, Mar 04, 2015 at 03:41:49PM +0200, Panu Matilainen wrote:
> On 03/04/2015 03:31 PM, Bruce Richardson wrote:
> >On Wed, Mar 04, 2015 at 03:24:12PM +0200, Panu Matilainen wrote:
> >>On 03/04/2015 03:08 PM, Bruce Richardson wrote:
> >>>On Wed, Mar 04, 2015 at 06:28:05AM -0500, Neil Horman wrote:
> >>>>On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
> >>>>>On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> >>>>>>Hi Panu,
> >>>>>>
> >>>>>>2015-03-04 08:17, Panu Matilainen:
> >>>>>>>With symbol versioning its vital that developers test their code in
> >>>>>>>shared library mode, otherwise we'll be playing "add the forgotten
> >>>>>>>symbol export" from here to eternity.
> >>>>>>
> >>>>>>Yes we must improve the sanity checks.
> >>>>>>A lot of options must be tested (or removed) and not only shared libs.
> >>>>>>But the error you reported before (missing export of rte_eth_dev_release_port)
> >>>>>>cannot be seen even with this patch.
> >>>>>
> >>>>>I know, I didn't say it would have directly caught it. It would've likely
> >>>>>been found earlier though, if nothing else then in testing of the new
> >>>>>librte_pmd_null which clearly nobody had tried in shared lib configuration.
> >>>>>
> >>>>This is accurate.  The default config is a tool, in the sense that it leverages
> >>>>the implicit testing of any users who are experimenting with the DPDK.  Any
> >>>>users out there using the DPDK test/example applications would have realized
> >>>>something was amiss when the testpmd app refused to run with the null or pcap
> >>>>pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
> >>>>only works if the defaults are carefully selected.  Currently, building for
> >>>>shared libraries exposes more existing bugs than static libraries, and so we
> >>>>should set that as our default so as to catch them.
> >>>>
> >>>>>>It means we need more tools.
> >>>>>>Though, default configuration is not a tool.
> >>>>>
> >>>>>Yes, default config is not a tool, its a recommendation of sorts both for
> >>>>>developers and users. It also tends to be the setup that is rarely broken
> >>>>>because it happens to get the most testing :)
> >>>>>
> >>>>And it is a tool (see above).
> >>>>
> >>>>>>
> >>>>>>>By defaulting to shared we should catch more of these cases early,
> >>>>>>>but without taking away anybodys ability to build static.
> >>>>>>
> >>>>>>Shared libraries are convenient for distributions but have a performance
> >>>>>>impact. I think that static build must remain the default choice.
> >>>>>
> >>>>
> >>>>If utmost performance is the concern, isn't it reasonable to assume that users
> >>>>in that demographic will customize their configuration to achieve that?  No one
> >>>>assumes that something is tuned to be perfect for their needs out of the box if
> >>>>their needs are extreemely biased to a single quality.  The best course of
> >>>>action here is to set the default to be adventageous toward catching bugs, and
> >>>>document the changes needed to bias for performance.
> >>>>
> >>>>>For distros, this is not a matter of *convenience*, its the only technically
> >>>>>feasible choice.
> >>>
> >>>As I understand it, build for the "default" cpu rather than "native" is the only
> >>>feasible choice also, so how about re-introducing a new defconfig file for
> >>>"default" (or perhaps better name), where you have lowest-common denominator
> >>>instruction-set and building for shared libraries?
> >>>Would that work for everyone, or do people feel it would be too confusing to have
> >>>more defconfig files available?
> >>
> >>Given the opposition to defaulting to shared, another config file seems like
> >>a fair compromise to me, whether "default" or something else. As for the
> >>naming, one possibility would be calling it "shared", implying both
> >>lowest-common denominator instruction set to be shareable across many
> >>systems and shared libraries.
> >>
> >>	- Panu -
> >
> >The naming scheme for configs is meant to be:
> >"ARCH-MACHINE-EXECENV-TOOLCHAIN"
> >as documented in the Getting Started Guide. "Default" has been used up till now
> >to refer to the lowest common denominator instruction set supported, which for
> >x86_64 is a core2 baseline, I believe. "shared" doesn't really fit into this
> >naming scheme, and there is nothing to allow extra notes to be added to the
> >name.
> 
> Right, but then there's "ivshmem" that doesn't fit that description either
> AFAICS.

Ah, yes, forgotten about that one! :-)

> 
> >Without changing this scheme, I would suggest we rename "default" to "generic",
> >which I think is a slightly better term for it, and we set the
> >"x86_64-generic-linuxapp-gcc" target to build shared libs.
> 
> Works for me. It is indeed more descriptive than either "default" or
> "shared" for the purpose.
> 
> 	- Panu -
>
  
Thomas Monjalon March 4, 2015, 1:56 p.m. UTC | #11
2015-03-04 13:49, Bruce Richardson:
> On Wed, Mar 04, 2015 at 03:41:49PM +0200, Panu Matilainen wrote:
> > On 03/04/2015 03:31 PM, Bruce Richardson wrote:
> > >On Wed, Mar 04, 2015 at 03:24:12PM +0200, Panu Matilainen wrote:
> > >>On 03/04/2015 03:08 PM, Bruce Richardson wrote:
> > >>>On Wed, Mar 04, 2015 at 06:28:05AM -0500, Neil Horman wrote:
> > >>>>On Wed, Mar 04, 2015 at 01:05:07PM +0200, Panu Matilainen wrote:
> > >>>>>On 03/04/2015 11:24 AM, Thomas Monjalon wrote:
> > >>>>>>Hi Panu,
> > >>>>>>
> > >>>>>>2015-03-04 08:17, Panu Matilainen:
> > >>>>>>>With symbol versioning its vital that developers test their code in
> > >>>>>>>shared library mode, otherwise we'll be playing "add the forgotten
> > >>>>>>>symbol export" from here to eternity.
> > >>>>>>
> > >>>>>>Yes we must improve the sanity checks.
> > >>>>>>A lot of options must be tested (or removed) and not only shared libs.
> > >>>>>>But the error you reported before (missing export of rte_eth_dev_release_port)
> > >>>>>>cannot be seen even with this patch.
> > >>>>>
> > >>>>>I know, I didn't say it would have directly caught it. It would've likely
> > >>>>>been found earlier though, if nothing else then in testing of the new
> > >>>>>librte_pmd_null which clearly nobody had tried in shared lib configuration.
> > >>>>>
> > >>>>This is accurate.  The default config is a tool, in the sense that it leverages
> > >>>>the implicit testing of any users who are experimenting with the DPDK.  Any
> > >>>>users out there using the DPDK test/example applications would have realized
> > >>>>something was amiss when the testpmd app refused to run with the null or pcap
> > >>>>pmd, since there was a missing symbol.  That "social fuzzing" has value, but it
> > >>>>only works if the defaults are carefully selected.  Currently, building for
> > >>>>shared libraries exposes more existing bugs than static libraries, and so we
> > >>>>should set that as our default so as to catch them.
> > >>>>
> > >>>>>>It means we need more tools.
> > >>>>>>Though, default configuration is not a tool.
> > >>>>>
> > >>>>>Yes, default config is not a tool, its a recommendation of sorts both for
> > >>>>>developers and users. It also tends to be the setup that is rarely broken
> > >>>>>because it happens to get the most testing :)
> > >>>>>
> > >>>>And it is a tool (see above).
> > >>>>
> > >>>>>>
> > >>>>>>>By defaulting to shared we should catch more of these cases early,
> > >>>>>>>but without taking away anybodys ability to build static.
> > >>>>>>
> > >>>>>>Shared libraries are convenient for distributions but have a performance
> > >>>>>>impact. I think that static build must remain the default choice.
> > >>>>>
> > >>>>
> > >>>>If utmost performance is the concern, isn't it reasonable to assume that users
> > >>>>in that demographic will customize their configuration to achieve that?  No one
> > >>>>assumes that something is tuned to be perfect for their needs out of the box if
> > >>>>their needs are extreemely biased to a single quality.  The best course of
> > >>>>action here is to set the default to be adventageous toward catching bugs, and
> > >>>>document the changes needed to bias for performance.
> > >>>>
> > >>>>>For distros, this is not a matter of *convenience*, its the only technically
> > >>>>>feasible choice.
> > >>>
> > >>>As I understand it, build for the "default" cpu rather than "native" is the only
> > >>>feasible choice also, so how about re-introducing a new defconfig file for
> > >>>"default" (or perhaps better name), where you have lowest-common denominator
> > >>>instruction-set and building for shared libraries?
> > >>>Would that work for everyone, or do people feel it would be too confusing to have
> > >>>more defconfig files available?
> > >>
> > >>Given the opposition to defaulting to shared, another config file seems like
> > >>a fair compromise to me, whether "default" or something else. As for the
> > >>naming, one possibility would be calling it "shared", implying both
> > >>lowest-common denominator instruction set to be shareable across many
> > >>systems and shared libraries.
> > >>
> > >>	- Panu -
> > >
> > >The naming scheme for configs is meant to be:
> > >"ARCH-MACHINE-EXECENV-TOOLCHAIN"
> > >as documented in the Getting Started Guide. "Default" has been used up till now
> > >to refer to the lowest common denominator instruction set supported, which for
> > >x86_64 is a core2 baseline, I believe. "shared" doesn't really fit into this
> > >naming scheme, and there is nothing to allow extra notes to be added to the
> > >name.
> > 
> > Right, but then there's "ivshmem" that doesn't fit that description either
> > AFAICS.
> 
> Ah, yes, forgotten about that one! :-)

off-topic:
I think we should remove "#ifdef RTE_LIBRTE_IVSHMEM" in EAL and then remove
defconfig_x86_64-ivshmem-linuxapp-*.

> > >Without changing this scheme, I would suggest we rename "default" to "generic",
> > >which I think is a slightly better term for it, and we set the
> > >"x86_64-generic-linuxapp-gcc" target to build shared libs.
> > 
> > Works for me. It is indeed more descriptive than either "default" or
> > "shared" for the purpose.

+1 for x86_64-generic-linuxapp-gcc
  
David Marchand March 4, 2015, 1:57 p.m. UTC | #12
On Wed, Mar 4, 2015 at 2:49 PM, Bruce Richardson <bruce.richardson@intel.com
> wrote:

> On Wed, Mar 04, 2015 at 03:41:49PM +0200, Panu Matilainen wrote:
> > Right, but then there's "ivshmem" that doesn't fit that description
> either
> > AFAICS.
>
> Ah, yes, forgotten about that one! :-)


Well, this is out of scope, but this config file should not exist in the
first place.
From my point of view, the ivshmem implementation is just badly hooked into
the eal, and this is the only reason why ivshmem should have a build option
(disabled) by default.

If we could cleanup this, then there would be no exception to the naming
convention.
  
Bruce Richardson March 4, 2015, 2:10 p.m. UTC | #13
On Wed, Mar 04, 2015 at 02:57:50PM +0100, David Marchand wrote:
> On Wed, Mar 4, 2015 at 2:49 PM, Bruce Richardson <bruce.richardson@intel.com
> > wrote:
> 
> > On Wed, Mar 04, 2015 at 03:41:49PM +0200, Panu Matilainen wrote:
> > > Right, but then there's "ivshmem" that doesn't fit that description
> > either
> > > AFAICS.
> >
> > Ah, yes, forgotten about that one! :-)
> 
> 
> Well, this is out of scope, but this config file should not exist in the
> first place.
> From my point of view, the ivshmem implementation is just badly hooked into
> the eal, and this is the only reason why ivshmem should have a build option
> (disabled) by default.
> 
> If we could cleanup this, then there would be no exception to the naming
> convention.
> 
No objections here! :-)

> 
> -- 
> David Marchand
  

Patch

diff --git a/config/common_bsdapp b/config/common_bsdapp
index 8ff4dc2..76b97be 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -76,7 +76,7 @@  CONFIG_RTE_FORCE_INTRINSICS=n
 #
 # Compile to share library
 #
-CONFIG_RTE_BUILD_SHARED_LIB=n
+CONFIG_RTE_BUILD_SHARED_LIB=y
 
 #
 # Combine to one single library
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 97f1c9e..1852951 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -76,7 +76,7 @@  CONFIG_RTE_FORCE_INTRINSICS=n
 #
 # Compile to share library
 #
-CONFIG_RTE_BUILD_SHARED_LIB=n
+CONFIG_RTE_BUILD_SHARED_LIB=y
 
 #
 # Combine to one single library