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

[Marek Mietus <mmietus97@yahoo.com>]

W dniu 9/16/25 o 12:43, Sabrina Dubroca pisze:
> 2025-09-09, 16:43:01 +0200, Marek Mietus wrote:
>> W dniu 9/9/25 o 13:17, Antonio Quartulli pisze:
>>> On 09/09/2025 07:43, Marek Mietus wrote:
>>>> Currently, all xmit flows use dst_cache in a way that references
>>>> its dst_entry for each xmitted packet. These atomic operations
>>>> are redundant in some flows.
>>>
>>> Can you elaborate on the current limits/drawbacks and explain what we gain with this new approach?
>>>
>>> It may be obvious for some, but it's not for me.
>>>
>>
>> The only difference with the new approach is that we avoid taking an unnecessary
>> reference on dst_entry. This is possible since the entire flow is protected by RCU.
>> This change reduces an atomic write operation on every xmit, resulting in a performance
>> improvement.
> 
> It would be nice to incorporate this information into the cover
> letter, and to provide numbers showing the performance
> improvement. That way, it's available to other reviewers and gets
> recorded in the git log. Are you doing crypto with some accelerator?
> Otherwise I'd imagine skipping a few atomic operations is not really
> noticeable.
> 
>> There are other flows in the kernel where a similar approach is used (e.g. __ip_queue_xmit
>> uses skb_dst_set_noref).
>>
>>> Also it sounded as if more tunnels were affected, but in the end only ovpn is being changed.
>>> Does it mean all other tunnels don't need this?
>>>
>>
>> More tunneling code can be updated to utilize these new helpers. I only worked
>> on OpenVPN, as I am more familiar with it. It was very easy to implement the
>> changes in OpenVPN because it doesn't use the udp_tunnel_dst_lookup helper
>> that adds some complexity.
>>
>> I hope to incorporate these changes in more tunnels in the future.
> 
> It would also be good to add this to the cover letter. Something like
> "For now, only ovpn is updated <for $reason>, but other tunnels could
> also take advantage of this."
> 

Noted, will add in the next revision.