netdev - Re: [PATCH net-next v3 0/3] net: tunnel: introduce noref xmit flows for tunnels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250924163114.2adc671b@kernel.org>
Date: Wed, 24 Sep 2025 16:31:14 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Marek Mietus <mmietus97@...oo.com>
Cc: netdev@...r.kernel.org, sd@...asysnail.net, antonio@...nvpn.net,
 openvpn-devel@...ts.sourceforge.net
Subject: Re: [PATCH net-next v3 0/3] net: tunnel: introduce noref xmit flows
 for tunnels

On Wed, 24 Sep 2025 18:27:46 +0200 Marek Mietus wrote:
> > On Mon, 22 Sep 2025 13:06:19 +0200 Marek Mietus wrote:  
> >> This patchset introduces new noref xmit helpers and incorporates
> >> them in the OpenVPN driver. A similar improvement can also be
> >> applied to other tunnel code in the future. The implementation
> >> for OpenVPN is a good starting point as it doesn't use the
> >> udp_tunnel_dst_lookup helper which adds some complexity.  
> > 
> > You're basically refactoring an API, it's fairly unusual to leave both
> > APIs in place upstream. Unless the number of callers is really huge,
> > say >100, or complexity very high. Not sure how others feel but IMHO
> > you should try to convert all the tunnels.
> >   
> 
> I'm introducing an opt-in API, which is useful in some cases, but not
> always as it optimizes flows that follow a specific pattern.
> 
> Since this API is opt-in, there is no need to over-complicate code
> to integrate the new API. The current API is still retained and is not 
> made redundant by the new API. Some tunnels may benefit from the new
> API with only minor complications, and should be modified in separate
> patchsets after this one.

My objection stands. Unless you have a reason why some tunnels need 
to ref the dst you should just convert all. Otherwise this is just
technical debt you're pushing on posterity.

> >> There are already noref optimizations in both ipv4 and ip6 
> >> (See __ip_queue_xmit, inet6_csk_xmit). This patchset allows for
> >> similar optimizations in udp tunnels. Referencing the dst_entry
> >> is now redundant, as the entire flow is protected under RCU, so
> >> it is removed.
> >>
> >> With this patchset, I was able to observe a 4% decrease in the total
> >> time for ovpn_udp_send_skb using perf.  
> > 
> > Please provide more meaningful perf wins. Relative change of perf in
> > one function doesn't tell use.. well.. anything.
> 
> Okay. Currently, I'm getting a consistent 2% increase in throughput on a VM,
> using iperf. Is this what I should mention in the next cover-letter?

Yes, that's much better! Some kind of average over multiple runs and/or
stddev would be ideal.