netdev - Re: [PATCH v2 net-next 2/6] net: add add indirect call wrapper in skb_release_head

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f51acd3e-dc76-450d-a036-01852fab6aaf@intel.com>
Date: Wed, 15 Oct 2025 14:30:13 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Eric Dumazet <edumazet@...gle.com>
CC: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
	<horms@...nel.org>, Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang
	<xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us>, Kuniyuki Iwashima
	<kuniyu@...gle.com>, Willem de Bruijn <willemb@...gle.com>,
	<netdev@...r.kernel.org>, <eric.dumazet@...il.com>
Subject: Re: [PATCH v2 net-next 2/6] net: add add indirect call wrapper in
 skb_release_head_state()

From: Eric Dumazet <edumazet@...gle.com>
Date: Wed, 15 Oct 2025 05:16:05 -0700

> On Wed, Oct 15, 2025 at 5:02 AM Alexander Lobakin
> <aleksander.lobakin@...el.com> wrote:
>>
>> From: Eric Dumazet <edumazet@...gle.com>
>> Date: Tue, 14 Oct 2025 17:19:03 +0000
>>
>>> While stress testing UDP senders on a host with expensive indirect
>>> calls, I found cpus processing TX completions where showing
>>> a very high cost (20%) in sock_wfree() due to
>>> CONFIG_MITIGATION_RETPOLINE=y.
>>>
>>> Take care of TCP and UDP TX destructors and use INDIRECT_CALL_3() macro.
>>>
>>> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>>> ---
>>>  net/core/skbuff.c | 11 ++++++++++-
>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>>> index bc12790017b0..692e3a70e75e 100644
>>> --- a/net/core/skbuff.c
>>> +++ b/net/core/skbuff.c
>>> @@ -1136,7 +1136,16 @@ void skb_release_head_state(struct sk_buff *skb)
>>>       skb_dst_drop(skb);
>>>       if (skb->destructor) {
>>>               DEBUG_NET_WARN_ON_ONCE(in_hardirq());
>>> -             skb->destructor(skb);
>>> +#ifdef CONFIG_INET
>>> +             INDIRECT_CALL_3(skb->destructor,
>>> +                             tcp_wfree, __sock_wfree, sock_wfree,
>>> +                             skb);
>>> +#else
>>> +             INDIRECT_CALL_1(skb->destructor,
>>> +                             sock_wfree,
>>> +                             skb);
>>> +
>>> +#endif
>>
>> Is it just me or seems like you ignored the suggestion/discussion under
>> v1 of this patch...
>>
> 
> I did not. Please send a patch when you can demonstrate the difference.

You "did not", but you didn't reply there, only sent v2 w/o any mention.

> 
> We are not going to add all the possible destructors unless there is evidence.

There are numbers in the original discussion, you'd have noticed if you
did read.

We only ask to add one more destructor which will help certain
perf-critical workloads. Add it to the end of the list, so that it won't
hurt your optimization.

"Send a patch" means you're now changing these lines now and then they
would be changed once again, why...

Thanks,
Olek