On 1/9/24 13:53, Simon Horman wrote:
> On Mon, Jan 08, 2024 at 08:34:36AM -0300, Roberto Bartzen Acosta via dev 
> wrote:
>> Current version of debian/rules simply uses the default lto GCC
>> optimization settings during the linkage process.
>>
>> The main problem with this approach is that GCC on OS like Ubuntu
>> Jammy, for example, can enable the -flto=auto option during the
>> openvswitch building and linking process. In this case, the linked
>> dynamic libraries would need to be builded based on the same lto
>> optimization options, at the risk of not working, according to
>> documentation [1].
>>
>> I'm not sure of the real benefits of using this link-time optimization

At least for DPDK-enabled builds LTO usually provides noticeable
performance improvement.  In the past I measured up to 10% packet
rate increase with LTO.  Though the gap went a bit lower with modern
compilers.  I think, it should still provide noticeable performance
improvements at least for the userspace datapath.

>> option, and since when it is enabled it causes problems with shared
>> libs link libjemalloc, for example, it seems safer overwritten
>> compiler decision by passing -fno-lto command.

Building shared libraries with LTO is indeed a bit problematic.
I agree that correctness should have a priority over potential
performance benefits.

Just out of curiosity, how do you add jemalloc into dependencies?
Using one of the environment variables?

>>
>> [1] https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flto
>>
>> Reported-at: 
>> https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748
>> Signed-off-by: Roberto Bartzen Acosta <[email protected]>
> 
> Hi,
> 
> for one reason or another our automation was unable to apply this patch.
> But I have done so locally in my own tree (it is not upstream)
> and run the GitHub based tests with success:
> 
> https://github.com/horms/ovs/actions/runs/7448358289
> 
> From my point of view this patch seems reasonable.
> 
> Acked-by: Simon Horman <[email protected]>
> 
> But I would be interested to hear feedback from others before applying it
> to the upstream tree.

@Frode, do you have any thoughts on this change?  Is it a problem
also for the main (in-distribution) Ubuntu/Debian builds?

> 
>> ---
>>  debian/rules | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/debian/rules b/debian/rules
>> index dc5cc8a65..de8771813 100755
>> --- a/debian/rules
>> +++ b/debian/rules
>> @@ -2,7 +2,7 @@
>>  # -*- makefile -*-
>>  #export DH_VERBOSE=1
>>  export DEB_BUILD_MAINT_OPTIONS = hardening=+all
>> -export DEB_CFLAGS_MAINT_APPEND = -fPIC
>> +export DEB_CFLAGS_MAINT_APPEND = -fPIC -fno-lto

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to