mirror of
https://github.com/torvalds/linux.git
synced 2026-10-07 11:06:03 +02:00
Merge branch 'udp-two-fixes-for-the-4-tuple-hash-table'
Shardul Bankar says: ==================== udp: two fixes for the 4-tuple hash table Two ways a UDP socket ends up in the wrong place in the 4-tuple hash table. The patches are independent, with different Fixes: tags and no dependency between them. Patch 1: a socket that connects a second time is not relocated, so it stays filed under its first peer's hash and packets for it fall back to scoring the hash2 chain for its address and port. Patch 2: a socket bound to a specific address and port is not taken out of the table when it disconnects, because __udp_disconnect() only does that via ->rehash() or ->unhash() and neither runs for it. Both are in code shared by IPv4 and IPv6. Patch 1's cost, with N sockets sharing a port and one of them misfiled, 200k packets sent to its 4-tuple: N without with 200 1,061,652 2,093,259 pps 500 522,553 2,055,078 pps 1000 279,729 2,136,606 pps Correctly filed sockets measure ~2.1M pps throughout, so the cost scales with the number of sockets on the port, as the fallback scan does. For comparison, commit78c91ae2c6("ipv4/udp: Add 4-tuple hash for connected socket") measured 290,860 pps without the table and 1,889,658 with it at 500 connected sockets. Patch 2's cost is not in throughput. Its stale entry keeps hash4_cnt raised for the life of the socket, so every packet for that address and port is sent through the 4-tuple lookup first; on IPv6 the entry is also matchable, because __udp_disconnect() does not clear sk_v6_daddr. That last one is a separate defect, which I will send on its own. Neither patch has a selftest. Nothing in tree reports which 4-tuple bucket a socket is filed under, so a test can only measure the cost indirectly. What I did instead was add pr_info() to the hash4 paths and a knob that dumps bucket occupancy, then run the same scenarios on two kernels differing only by these patches; that is where the numbers above come from. The instrumentation, the reproducers and the benchmark are at [1]. If exposing the bucket through diag would be welcome, that would make both defects testable in tree and I am glad to do it for net-next. Removing the connect(AF_UNSPEC) limitation described in644f9108f3is a side effect of patch 1 fixing the general case. I can make it narrower if you would rather that limitation stayed. Tooling, per Documentation/process/generated-content.rst: this series was developed in an assisted session with an LLM. The assistant did most of the code reading, wrote the instrumentation and reproducers behind [1], drafted these changelogs, and ran the A/B builds and the regression suites below. Every claim in these messages was checked against the source, and the IPv6 behaviour described in patch 2 was confirmed at runtime. Tested on x86-64, IPv4 and IPv6. No regressions across reuseport_bpf, reuseport_bpf_cpu, reuseport_addr_any.sh, reuseport_dualstack, udpgso_bench.sh, udpgro_bench.sh and socket. [1] https://github.com/shardulsdk-mpiric/linux/tree/udp-hash4-fix-verification To: Willem de Bruijn <willemdebruijn.kernel@gmail.com> To: "David S. Miller" <davem@davemloft.net> To: Eric Dumazet <edumazet@google.com> To: Jakub Kicinski <kuba@kernel.org> To: Paolo Abeni <pabeni@redhat.com> To: Simon Horman <horms@kernel.org> To: Philo Lu <lulie@linux.alibaba.com> To: Fred Chen <fred.cc@alibaba-inc.com> To: Yubing Qiu <yubing.qiuyubing@alibaba-inc.com> Cc: Kuniyuki Iwashima <kuniyu@google.com> Cc: Willem de Bruijn <willemb@google.com> Cc: Cambda Zhu <cambda@linux.alibaba.com> Cc: Janak Bhatt <janak@mpiric.us> Cc: Kalpan Jani <kalpan.jani@mpiricsoftware.com> Cc: Shardul Bankar <shardulsb08@gmail.com> Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com> ==================== Link: https://patch.msgid.link/20260917-udp_hash4_fix_v1-v1-0-718891af0d7a@mpiricsoftware.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
commit
177a59665d
|
|
@ -617,14 +617,23 @@ void udp_lib_hash4(struct sock *sk, u16 hash)
|
|||
struct net *net = sock_net(sk);
|
||||
struct udp_table *udptable;
|
||||
|
||||
/* Connected udp socket can re-connect to another remote address, which
|
||||
* will be handled by rehash. Thus no need to redo hash4 here.
|
||||
*/
|
||||
if (udp_hashed4(sk))
|
||||
return;
|
||||
|
||||
udptable = net->ipv4.udp_table;
|
||||
hslot = udp_hashslot(udptable, net, udp_sk(sk)->udp_port_hash);
|
||||
|
||||
/* A connected socket can re-connect to another address. rehash()
|
||||
* relocates it, but only runs when the local address changes, so a
|
||||
* socket bound to a specific address would stay filed under the
|
||||
* previous peer's hash. Move it here.
|
||||
*/
|
||||
if (udp_hashed4(sk)) {
|
||||
if (udp_sk(sk)->udp_lrpa_hash != hash) {
|
||||
spin_lock_bh(&hslot->lock);
|
||||
udp_rehash4(udptable, sk, hash);
|
||||
spin_unlock_bh(&hslot->lock);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
hslot2 = udp_hashslot2(udptable, udp_sk(sk)->udp_portaddr_hash);
|
||||
hslot4 = udp_hashslot4(udptable, hash);
|
||||
udp_sk(sk)->udp_lrpa_hash = hash;
|
||||
|
|
@ -2197,9 +2206,31 @@ int __udp_disconnect(struct sock *sk, int flags)
|
|||
}
|
||||
EXPORT_SYMBOL(__udp_disconnect);
|
||||
|
||||
/* __udp_disconnect() takes a socket out of the 4-tuple hash table only via
|
||||
* ->rehash() or ->unhash(), and neither runs for a socket bound to a
|
||||
* specific address and port. Remove it here, before its peer is cleared.
|
||||
*/
|
||||
static void udp_unhash4_on_disconnect(struct sock *sk)
|
||||
{
|
||||
struct net *net = sock_net(sk);
|
||||
struct udp_table *udptable;
|
||||
struct udp_hslot *hslot;
|
||||
|
||||
if (!udp_hashed4(sk))
|
||||
return;
|
||||
|
||||
udptable = net->ipv4.udp_table;
|
||||
hslot = udp_hashslot(udptable, net, udp_sk(sk)->udp_port_hash);
|
||||
|
||||
spin_lock_bh(&hslot->lock);
|
||||
udp_unhash4(udptable, sk);
|
||||
spin_unlock_bh(&hslot->lock);
|
||||
}
|
||||
|
||||
int udp_disconnect(struct sock *sk, int flags)
|
||||
{
|
||||
lock_sock(sk);
|
||||
udp_unhash4_on_disconnect(sk);
|
||||
__udp_disconnect(sk, flags);
|
||||
release_sock(sk);
|
||||
return 0;
|
||||
|
|
@ -3131,6 +3162,7 @@ int udp_abort(struct sock *sk, int err)
|
|||
|
||||
sk->sk_err = err;
|
||||
sk_error_report(sk);
|
||||
udp_unhash4_on_disconnect(sk);
|
||||
__udp_disconnect(sk, 0);
|
||||
|
||||
out:
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user