netdev - Re: [PATCH net-next] net/tcp: move TCP hash fail messages out of line

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: <CAGrbwDQ40DWXZ1AYesdXMXrb3HWW6bs6dAYVaUrwDriCekwg3w@mail.gmail.com>
Date: Wed, 3 Apr 2024 18:00:49 +0100
From: Dmitry Safonov <dima@...sta.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Francesco Ruggeri <fruggeri@...sta.com>, Salam Noureddine <noureddine@...sta.com>, 
	David Ahern <dsahern@...nel.org>, nex.sw.ncis.osdt.itp.upstreaming@...el.com, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net/tcp: move TCP hash fail messages out of line

Hi Alexander,

On Wed, Apr 3, 2024 at 2:26 PM Alexander Lobakin
<aleksander.lobakin@...el.com> wrote:
>
> tcp_hash_fail() is used multiple times on hotpath, including static
> inlines. It contains a couple branches and a lot of code, which all
> gets inlined into the call sites. For example, one call emits two
> calls to net_ratelimit() etc.
> Move as much as we can out of line to a new global function. Use enum
> to determine the type of failure. Check for net_ratelimit() only once,
> format common fields only once as well to pass only unique strings to
> pr_info().
> The result for vmlinux with Clang 19:
>
> add/remove: 2/0 grow/shrink: 0/4 up/down: 773/-4908 (-4135)
> Function                                     old     new   delta
> __tcp_hash_fail                                -     757    +757
> __pfx___tcp_hash_fail                          -      16     +16
> tcp_inbound_ao_hash                         1819    1062    -757
> tcp_ao_verify_hash                          1217     451    -766
> tcp_inbound_md5_hash                        1591     374   -1217
> tcp_inbound_hash                            3566    1398   -2168

I can see that as an improvement, albeit that enum and the resulting switch
are quite gross, sorry.
I had patches to convert those messages to tracepoints (by Jakub's suggestion).
That seems to work quite nice and will remove this macro entirely:
https://lore.kernel.org/all/20240224-tcp-ao-tracepoints-v1-0-15f31b7f30a7@arista.com/

I need to send version 2 for that. Unfortunately, that got delayed by
me migrating
from my previous work laptop. That was not my choice, resulting in
little disruption.
I'm planning to send the new version optimistically by the end of this week,
at worst the next week.

Thanks,
            Dmitry