From: David Laight <david.laight.linux@gmail.com>
To: Eric Dumazet <edumazet@google.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Willem de Bruijn <willemb@google.com>,
Kuniyuki Iwashima <kuniyu@google.com>,
Simon Horman <horms@kernel.org>,
netdev@vger.kernel.org, eric.dumazet@gmail.com,
Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Subject: Re: [PATCH net] udp: restore hlist_nulls for primary and secondary hash tables
Date: Tue, 22 Sep 2026 13:08:56 +0100 [thread overview]
Message-ID: <20260922130856.2c4f409a@pumpkin> (raw)
In-Reply-To: <20260922095747.3059954-1-edumazet@google.com>
On Tue, 22 Sep 2026 09:57:47 +0000
Eric Dumazet <edumazet@google.com> wrote:
> Commit ca065d0cf80f ("udp: no longer use SLAB_DESTROY_BY_RCU") switched
> UDP sockets to SOCK_RCU_FREE and converted udptable->hash and
> udptable->hash2 from hlist_nulls_head to hlist_head.
>
> While SOCK_RCU_FREE guarantees that a socket is not freed before an RCU
> grace period elapses, a live UDP socket can still be unhashed or moved
> to a different hash bucket without an RCU grace period:
>
> 1. udp_lib_rehash() moves sk->skc_portaddr_node from hslot2 to a
> different nhslot2 when inet_rcv_saddr changes (for instance via
> connect() or disconnect() on an wildcard-bound socket).
> 2. __udp_disconnect() calls udp_lib_unhash() when an implicitly bound
> port was used, and a subsequent connect() or bind() can immediately
> re-insert sk->sk_nulls_node and sk->skc_portaddr_node into different
> hash and hash2 buckets.
>
> Because hlist_add_head_rcu() overwrites node->next with the new bucket
> chain without waiting for an RCU grace period, a concurrent lockless
> reader in udp4_lib_lookup1/2() or udp6_lib_lookup1/2() traversing the
> old bucket can silently jump to the new bucket chain, terminate early,
> and miss a matching socket that was located later in the original bucket.
>
> Restore hlist_nulls for udptable->hash and udptable->hash2 while keeping
> SOCK_RCU_FREE (so RCU lookups remain refcount-free), and restart the
> bucket traversal if get_nulls_value(node) does not match the expected
> bucket index.
Thanks - I've reported it before.
This is hit by a real application and causes unexpected ICMP port unreachable
messages on localhost.
I'm not sure why that application hits it, it does use a lot of udp sockets
but I don't believe any are 'connected'.
The work around is to ignore a single icmp error message.
I can't test the change - the fault could never be reliably reproduced and
the failing systems run on the Amazon cloud.
(And I've retired and don't work for that company any more.)
David
>
> Fixes: ca065d0cf80f ("udp: no longer use SLAB_DESTROY_BY_RCU")
> Assisted-by: LLM
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> ---
> include/linux/udp.h | 11 +++--
> include/net/sock.h | 23 +++++----
> include/net/udp.h | 5 +-
> net/ipv4/udp.c | 112 ++++++++++++++++++++++++++++++--------------
> net/ipv4/udp_diag.c | 5 +-
> net/ipv6/udp.c | 40 ++++++++++++----
> 6 files changed, 131 insertions(+), 65 deletions(-)
>
> diff --git a/include/linux/udp.h b/include/linux/udp.h
> index 998906ec3b32add4652bde3ee9c28e8b2b98ce51..a1990350fd22f3418724c0e06d4b7405779f7c2b 100644
> --- a/include/linux/udp.h
> +++ b/include/linux/udp.h
> @@ -243,14 +243,15 @@ static inline void udp_allow_gso(struct sock *sk)
> udp_set_bit(ACCEPT_FRAGLIST, sk);
> }
>
> -#define udp_portaddr_for_each_entry(__sk, list) \
> - hlist_for_each_entry(__sk, list, __sk_common.skc_portaddr_node)
> +#define udp_portaddr_for_each_entry(__sk, node, list) \
> + hlist_nulls_for_each_entry(__sk, node, list, __sk_common.skc_portaddr_node)
>
> #define udp_portaddr_for_each_entry_from(__sk) \
> - hlist_for_each_entry_from(__sk, __sk_common.skc_portaddr_node)
> + for (; __sk; __sk = hlist_nulls_entry_safe((__sk)->__sk_common.skc_portaddr_node.next, \
> + typeof(*(__sk)), __sk_common.skc_portaddr_node))
>
> -#define udp_portaddr_for_each_entry_rcu(__sk, list) \
> - hlist_for_each_entry_rcu(__sk, list, __sk_common.skc_portaddr_node)
> +#define udp_portaddr_for_each_entry_rcu(__sk, node, list) \
> + hlist_nulls_for_each_entry_rcu(__sk, node, list, __sk_common.skc_portaddr_node)
>
> #if !IS_ENABLED(CONFIG_BASE_SMALL)
> #define udp_lrpa_for_each_entry_rcu(__up, node, list) \
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 60ea55dc18854a9759f5df618cc8c904d2323e95..78626aba52dca6ab9fcae60f5bc277089f143633 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -184,7 +184,7 @@ struct sock_common {
> int skc_bound_dev_if;
> union {
> struct hlist_node skc_bind_node;
> - struct hlist_node skc_portaddr_node;
> + struct hlist_nulls_node skc_portaddr_node;
> };
> struct proto *skc_prot;
> possible_net_t skc_net;
> @@ -930,7 +930,11 @@ static inline void __sk_nulls_add_node_tail_rcu(struct sock *sk, struct hlist_nu
> static inline void sk_nulls_add_node_rcu(struct sock *sk, struct hlist_nulls_head *list)
> {
> sock_hold(sk);
> - __sk_nulls_add_node_rcu(sk, list);
> + if (IS_ENABLED(CONFIG_IPV6) && sk->sk_reuseport &&
> + sk->sk_family == AF_INET6)
> + __sk_nulls_add_node_tail_rcu(sk, list);
> + else
> + __sk_nulls_add_node_rcu(sk, list);
> }
>
> static inline void __sk_del_bind_node(struct sock *sk)
> @@ -965,18 +969,19 @@ static inline void sk_add_bind_node(struct sock *sk,
> hlist_for_each_entry_safe(__sk, tmp, list, sk_bind_node)
>
> /**
> - * sk_for_each_entry_offset_rcu - iterate over a list at a given struct offset
> + * sk_nulls_for_each_entry_offset_rcu - iterate over a list at a given struct offset
> * @tpos: the type * to use as a loop cursor.
> - * @pos: the &struct hlist_node to use as a loop cursor.
> + * @pos: the &struct hlist_nulls_node to use as a loop cursor.
> * @head: the head for your list.
> - * @offset: offset of hlist_node within the struct.
> + * @offset: offset of hlist_nulls_node within the struct.
> *
> */
> -#define sk_for_each_entry_offset_rcu(tpos, pos, head, offset) \
> - for (pos = rcu_dereference(hlist_first_rcu(head)); \
> - pos != NULL && \
> +#define sk_nulls_for_each_entry_offset_rcu(tpos, pos, head, offset) \
> + for (({ barrier(); }), \
> + pos = rcu_dereference_raw(hlist_nulls_first_rcu(head)); \
> + (!is_a_nulls(pos)) && \
> ({ tpos = (typeof(*tpos) *)((void *)pos - offset); 1;}); \
> - pos = rcu_dereference(hlist_next_rcu(pos)))
> + pos = rcu_dereference_raw(hlist_nulls_next_rcu(pos)))
>
> static inline struct user_namespace *sk_user_ns(const struct sock *sk)
> {
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 1fee17274745f0b52837b7eb2498dbc423a450fd..1bba33479341e07f2dde9acc19b0b09cafa3ef29 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -56,10 +56,7 @@ struct udp_skb_cb {
> */
> struct udp_hslot {
> union {
> - struct hlist_head head;
> - /* hash4 uses hlist_nulls to avoid moving wrongly onto another
> - * hlist, because rehash() can happen with lookup().
> - */
> + struct hlist_nulls_head head;
> struct hlist_nulls_head nulls_head;
> };
> int count;
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index b090bd1f59e86cd22edd9622b17b5679346ea24d..309220bf2fba9e7ccf48676d312924a6881bc34f 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -136,10 +136,11 @@ static int udp_lib_lport_inuse(struct net *net, __u16 num,
> unsigned long *bitmap,
> struct sock *sk, unsigned int log)
> {
> + struct hlist_nulls_node *node;
> kuid_t uid = sk_uid(sk);
> struct sock *sk2;
>
> - sk_for_each(sk2, &hslot->head) {
> + sk_nulls_for_each(sk2, node, &hslot->head) {
> if (net_eq(sock_net(sk2), net) &&
> sk2 != sk &&
> (bitmap || udp_sk(sk2)->udp_port_hash == num) &&
> @@ -171,12 +172,13 @@ static int udp_lib_lport_inuse2(struct net *net, __u16 num,
> struct udp_hslot *hslot2,
> struct sock *sk)
> {
> + struct hlist_nulls_node *node;
> kuid_t uid = sk_uid(sk);
> struct sock *sk2;
> int res = 0;
>
> spin_lock(&hslot2->lock);
> - udp_portaddr_for_each_entry(sk2, &hslot2->head) {
> + udp_portaddr_for_each_entry(sk2, node, &hslot2->head) {
> if (net_eq(sock_net(sk2), net) &&
> sk2 != sk &&
> (udp_sk(sk2)->udp_port_hash == num) &&
> @@ -201,10 +203,11 @@ static int udp_lib_lport_inuse2(struct net *net, __u16 num,
> static int udp_reuseport_add_sock(struct sock *sk, struct udp_hslot *hslot)
> {
> struct net *net = sock_net(sk);
> + struct hlist_nulls_node *node;
> kuid_t uid = sk_uid(sk);
> struct sock *sk2;
>
> - sk_for_each(sk2, &hslot->head) {
> + sk_nulls_for_each(sk2, node, &hslot->head) {
> if (net_eq(sock_net(sk2), net) &&
> sk2 != sk &&
> sk2->sk_family == sk->sk_family &&
> @@ -323,7 +326,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
>
> sock_set_flag(sk, SOCK_RCU_FREE);
>
> - sk_add_node_rcu(sk, &hslot->head);
> + sk_nulls_add_node_rcu(sk, &hslot->head);
> hslot->count++;
> sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
>
> @@ -331,11 +334,11 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
> spin_lock(&hslot2->lock);
> if (IS_ENABLED(CONFIG_IPV6) && sk->sk_reuseport &&
> sk->sk_family == AF_INET6)
> - hlist_add_tail_rcu(&udp_sk(sk)->udp_portaddr_node,
> - &hslot2->head);
> + hlist_nulls_add_tail_rcu(&udp_sk(sk)->udp_portaddr_node,
> + &hslot2->head);
> else
> - hlist_add_head_rcu(&udp_sk(sk)->udp_portaddr_node,
> - &hslot2->head);
> + hlist_nulls_add_head_rcu(&udp_sk(sk)->udp_portaddr_node,
> + &hslot2->head);
> hslot2->count++;
> spin_unlock(&hslot2->lock);
> }
> @@ -440,10 +443,14 @@ static struct sock *udp4_lib_lookup1(const struct net *net,
> {
> unsigned int slot = udp_hashfn(net, hnum, udptable->mask);
> struct udp_hslot *hslot = &udptable->hash[slot];
> - struct sock *sk, *result = NULL;
> - int score, badness = 0;
> + struct hlist_nulls_node *node;
> + struct sock *sk, *result;
> + int score, badness;
>
> - sk_for_each_rcu(sk, &hslot->head) {
> +begin:
> + result = NULL;
> + badness = 0;
> + sk_nulls_for_each_rcu(sk, node, &hslot->head) {
> score = compute_score(sk, net,
> saddr, sport, daddr, hnum, dif, sdif);
> if (score > badness) {
> @@ -451,6 +458,13 @@ static struct sock *udp4_lib_lookup1(const struct net *net,
> badness = score;
> }
> }
> + /*
> + * if the nulls value we got at the end of this lookup is
> + * not the expected one, we must restart lookup.
> + * We probably met an item that was moved to another chain.
> + */
> + if (unlikely(get_nulls_value(node) != slot))
> + goto begin;
>
> return result;
> }
> @@ -463,13 +477,16 @@ static struct sock *udp4_lib_lookup2(const struct net *net,
> struct udp_hslot *hslot2,
> struct sk_buff *skb)
> {
> + unsigned int slot2 = UDP_HSLOT_MAIN(hslot2) - net->ipv4.udp_table->hash2;
> + struct hlist_nulls_node *node;
> struct sock *sk, *result;
> int score, badness;
> bool need_rescore;
>
> +begin:
> result = NULL;
> badness = 0;
> - udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) {
> + udp_portaddr_for_each_entry_rcu(sk, node, &hslot2->head) {
> need_rescore = false;
> rescore:
> score = compute_score(need_rescore ? result : sk, net, saddr,
> @@ -510,6 +527,13 @@ static struct sock *udp4_lib_lookup2(const struct net *net,
> goto rescore;
> }
> }
> + /*
> + * if the nulls value we got at the end of this lookup is
> + * not the expected one, we must restart lookup.
> + * We probably met an item that was moved to another chain.
> + */
> + if (unlikely(get_nulls_value(node) != slot2))
> + goto begin;
> return result;
> }
>
> @@ -562,7 +586,7 @@ static struct sock *udp4_lib_lookup4(const struct net *net,
> * expected one, we must restart lookup. We probably met an item that
> * was moved to another chain due to rehash.
> */
> - if (get_nulls_value(node) != slot)
> + if (unlikely(get_nulls_value(node) != slot))
> goto begin;
>
> return NULL;
> @@ -2251,13 +2275,13 @@ void udp_lib_unhash(struct sock *sk)
> spin_lock_bh(&hslot->lock);
> if (rcu_access_pointer(sk->sk_reuseport_cb))
> reuseport_detach_sock(sk);
> - if (sk_del_node_init_rcu(sk)) {
> + if (sk_nulls_del_node_init_rcu(sk)) {
> hslot->count--;
> inet_sk(sk)->inet_num = 0;
> sock_prot_inuse_add(net, sk->sk_prot, -1);
>
> spin_lock(&hslot2->lock);
> - hlist_del_init_rcu(&udp_sk(sk)->udp_portaddr_node);
> + hlist_nulls_del_init_rcu(&udp_sk(sk)->udp_portaddr_node);
> hslot2->count--;
> spin_unlock(&hslot2->lock);
>
> @@ -2291,13 +2315,18 @@ void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
>
> if (hslot2 != nhslot2) {
> spin_lock(&hslot2->lock);
> - hlist_del_init_rcu(&udp_sk(sk)->udp_portaddr_node);
> + hlist_nulls_del_init_rcu(&udp_sk(sk)->udp_portaddr_node);
> hslot2->count--;
> spin_unlock(&hslot2->lock);
>
> spin_lock(&nhslot2->lock);
> - hlist_add_head_rcu(&udp_sk(sk)->udp_portaddr_node,
> - &nhslot2->head);
> + if (IS_ENABLED(CONFIG_IPV6) && sk->sk_reuseport &&
> + sk->sk_family == AF_INET6)
> + hlist_nulls_add_tail_rcu(&udp_sk(sk)->udp_portaddr_node,
> + &nhslot2->head);
> + else
> + hlist_nulls_add_head_rcu(&udp_sk(sk)->udp_portaddr_node,
> + &nhslot2->head);
> nhslot2->count++;
> spin_unlock(&nhslot2->lock);
> }
> @@ -2513,9 +2542,9 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> unsigned int hash2, hash2_any, offset;
> unsigned short hnum = ntohs(uh->dest);
> struct sock *sk, *first = NULL;
> + struct hlist_nulls_node *node;
> int dif = skb->dev->ifindex;
> int sdif = inet_sdif(skb);
> - struct hlist_node *node;
> struct udp_hslot *hslot;
> struct sk_buff *nskb;
> bool use_hash2;
> @@ -2525,7 +2554,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> hash2 = 0;
> hslot = udp_hashslot(udptable, net, hnum);
> use_hash2 = hslot->count > 10;
> - offset = offsetof(typeof(*sk), sk_node);
> + offset = offsetof(typeof(*sk), sk_nulls_node);
>
> if (use_hash2) {
> hash2_any = ipv4_portaddr_hash(net, htonl(INADDR_ANY), hnum) &
> @@ -2536,7 +2565,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> offset = offsetof(typeof(*sk), __sk_common.skc_portaddr_node);
> }
>
> - sk_for_each_entry_offset_rcu(sk, node, &hslot->head, offset) {
> + sk_nulls_for_each_entry_offset_rcu(sk, node, &hslot->head, offset) {
> if (!__udp_is_mcast_sock(net, sk, uh->dest, daddr,
> uh->source, saddr, dif, sdif, hnum))
> continue;
> @@ -2749,6 +2778,7 @@ static struct sock *__udp4_lib_mcast_demux_lookup(struct net *net,
> {
> struct udp_table *udptable = net->ipv4.udp_table;
> unsigned short hnum = ntohs(loc_port);
> + struct hlist_nulls_node *node;
> struct sock *sk, *result;
> struct udp_hslot *hslot;
> unsigned int slot;
> @@ -2760,8 +2790,9 @@ static struct sock *__udp4_lib_mcast_demux_lookup(struct net *net,
> if (hslot->count > 10)
> return NULL;
>
> +begin:
> result = NULL;
> - sk_for_each_rcu(sk, &hslot->head) {
> + sk_nulls_for_each_rcu(sk, node, &hslot->head) {
> if (__udp_is_mcast_sock(net, sk, loc_port, loc_addr,
> rmt_port, rmt_addr, dif, sdif, hnum)) {
> if (result)
> @@ -2769,6 +2800,13 @@ static struct sock *__udp4_lib_mcast_demux_lookup(struct net *net,
> result = sk;
> }
> }
> + /*
> + * if the nulls value we got at the end of this lookup is
> + * not the expected one, we must restart lookup.
> + * We probably met an item that was moved to another chain.
> + */
> + if (unlikely(get_nulls_value(node) != slot))
> + goto begin;
>
> return result;
> }
> @@ -2785,6 +2823,7 @@ static struct sock *__udp4_lib_demux_lookup(struct net *net,
> struct udp_table *udptable = net->ipv4.udp_table;
> INET_ADDR_COOKIE(acookie, rmt_addr, loc_addr);
> unsigned short hnum = ntohs(loc_port);
> + struct hlist_nulls_node *node;
> struct udp_hslot *hslot2;
> unsigned int hash2;
> __portpair ports;
> @@ -2794,7 +2833,7 @@ static struct sock *__udp4_lib_demux_lookup(struct net *net,
> hslot2 = udp_hashslot2(udptable, hash2);
> ports = INET_COMBINED_PORTS(rmt_port, hnum);
>
> - udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) {
> + udp_portaddr_for_each_entry_rcu(sk, node, &hslot2->head) {
> if (inet_match(net, sk, acookie, ports, dif, sdif))
> return sk;
> /* Only check first socket in chain */
> @@ -3228,6 +3267,7 @@ static struct sock *udp_get_first(struct seq_file *seq, int start)
> {
> struct udp_iter_state *state = seq->private;
> struct net *net = seq_file_net(seq);
> + struct hlist_nulls_node *node;
> struct udp_table *udptable;
> struct sock *sk;
>
> @@ -3237,11 +3277,11 @@ static struct sock *udp_get_first(struct seq_file *seq, int start)
> ++state->bucket) {
> struct udp_hslot *hslot = &udptable->hash[state->bucket];
>
> - if (hlist_empty(&hslot->head))
> + if (hlist_nulls_empty(&hslot->head))
> continue;
>
> spin_lock_bh(&hslot->lock);
> - sk_for_each(sk, &hslot->head) {
> + sk_nulls_for_each(sk, node, &hslot->head) {
> if (seq_sk_match(seq, sk))
> goto found;
> }
> @@ -3259,7 +3299,7 @@ static struct sock *udp_get_next(struct seq_file *seq, struct sock *sk)
> struct udp_table *udptable;
>
> do {
> - sk = sk_next(sk);
> + sk = sk_nulls_next(sk);
> } while (sk && !seq_sk_match(seq, sk));
>
> if (!sk) {
> @@ -3431,12 +3471,12 @@ static struct sock *bpf_iter_udp_batch(struct seq_file *seq)
> for (; state->bucket <= udptable->mask; state->bucket++) {
> struct udp_hslot *hslot2 = &udptable->hash2[state->bucket].hslot;
>
> - if (hlist_empty(&hslot2->head))
> + if (hlist_nulls_empty(&hslot2->head))
> goto next_bucket;
>
> spin_lock_bh(&hslot2->lock);
> - sk = hlist_entry_safe(hslot2->head.first, struct sock,
> - __sk_common.skc_portaddr_node);
> + sk = hlist_nulls_entry_safe(hslot2->head.first, struct sock,
> + __sk_common.skc_portaddr_node);
> /* Resume from the first (in iteration order) unseen socket from
> * the last batch that still exists in resume_bucket. Most of
> * the time this will just be where the last iteration left off
> @@ -3488,9 +3528,9 @@ static struct sock *bpf_iter_udp_batch(struct seq_file *seq)
>
> /* Pick up where we left off. */
> sk = iter->batch[iter->end_sk - 1].sk;
> - sk = hlist_entry_safe(sk->__sk_common.skc_portaddr_node.next,
> - struct sock,
> - __sk_common.skc_portaddr_node);
> + sk = hlist_nulls_entry_safe(sk->__sk_common.skc_portaddr_node.next,
> + struct sock,
> + __sk_common.skc_portaddr_node);
> batch_sks = iter->end_sk;
> goto fill_batch;
> }
> @@ -3717,12 +3757,12 @@ static void __init udp_table_init(struct udp_table *table, const char *name)
>
> table->hash2 = (void *)(table->hash + (table->mask + 1));
> for (i = 0; i <= table->mask; i++) {
> - INIT_HLIST_HEAD(&table->hash[i].head);
> + INIT_HLIST_NULLS_HEAD(&table->hash[i].head, i);
> table->hash[i].count = 0;
> spin_lock_init(&table->hash[i].lock);
> }
> for (i = 0; i <= table->mask; i++) {
> - INIT_HLIST_HEAD(&table->hash2[i].hslot.head);
> + INIT_HLIST_NULLS_HEAD(&table->hash2[i].hslot.head, i);
> table->hash2[i].hslot.count = 0;
> spin_lock_init(&table->hash2[i].hslot.lock);
> }
> @@ -3771,11 +3811,11 @@ static struct udp_table __net_init *udp_pernet_table_alloc(unsigned int hash_ent
> udptable->log = ilog2(hash_entries);
>
> for (i = 0; i < hash_entries; i++) {
> - INIT_HLIST_HEAD(&udptable->hash[i].head);
> + INIT_HLIST_NULLS_HEAD(&udptable->hash[i].head, i);
> udptable->hash[i].count = 0;
> spin_lock_init(&udptable->hash[i].lock);
>
> - INIT_HLIST_HEAD(&udptable->hash2[i].hslot.head);
> + INIT_HLIST_NULLS_HEAD(&udptable->hash2[i].hslot.head, i);
> udptable->hash2[i].hslot.count = 0;
> spin_lock_init(&udptable->hash2[i].hslot.lock);
> }
> diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
> index f4b24e628cf8ded821d0c1887dd6ca5b83c4c8e2..5e0b4e07d9c1d80a727647273c9793ca11626827 100644
> --- a/net/ipv4/udp_diag.c
> +++ b/net/ipv4/udp_diag.c
> @@ -100,15 +100,16 @@ static void udp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>
> for (slot = s_slot; slot <= table->mask; s_num = 0, slot++) {
> struct udp_hslot *hslot = &table->hash[slot];
> + struct hlist_nulls_node *node;
> struct sock *sk;
>
> num = 0;
>
> - if (hlist_empty(&hslot->head))
> + if (hlist_nulls_empty(&hslot->head))
> continue;
>
> spin_lock_bh(&hslot->lock);
> - sk_for_each(sk, &hslot->head) {
> + sk_nulls_for_each(sk, node, &hslot->head) {
> struct inet_sock *inet = inet_sk(sk);
>
> if (!net_eq(sock_net(sk), net))
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 93478d1ad5769c6058567ff4deb433b0656b8129..f14132d4006718ce1e065986b734fb098bae4d9a 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -201,10 +201,14 @@ static struct sock *udp6_lib_lookup1(const struct net *net,
> {
> unsigned int slot = udp_hashfn(net, hnum, udptable->mask);
> struct udp_hslot *hslot = &udptable->hash[slot];
> - struct sock *sk, *result = NULL;
> - int score, badness = 0;
> + struct hlist_nulls_node *node;
> + struct sock *sk, *result;
> + int score, badness;
>
> - sk_for_each_rcu(sk, &hslot->head) {
> +begin:
> + result = NULL;
> + badness = 0;
> + sk_nulls_for_each_rcu(sk, node, &hslot->head) {
> score = compute_score(sk, net,
> saddr, sport, daddr, hnum, dif, sdif);
> if (score > badness) {
> @@ -212,6 +216,13 @@ static struct sock *udp6_lib_lookup1(const struct net *net,
> badness = score;
> }
> }
> + /*
> + * if the nulls value we got at the end of this lookup is
> + * not the expected one, we must restart lookup.
> + * We probably met an item that was moved to another chain.
> + */
> + if (unlikely(get_nulls_value(node) != slot))
> + goto begin;
>
> return result;
> }
> @@ -223,13 +234,16 @@ static struct sock *udp6_lib_lookup2(const struct net *net,
> int dif, int sdif, struct udp_hslot *hslot2,
> struct sk_buff *skb)
> {
> + unsigned int slot2 = UDP_HSLOT_MAIN(hslot2) - net->ipv4.udp_table->hash2;
> + struct hlist_nulls_node *node;
> struct sock *sk, *result;
> int score, badness;
> bool need_rescore;
>
> +begin:
> result = NULL;
> badness = -1;
> - udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) {
> + udp_portaddr_for_each_entry_rcu(sk, node, &hslot2->head) {
> need_rescore = false;
> rescore:
> score = compute_score(need_rescore ? result : sk, net, saddr,
> @@ -270,6 +284,13 @@ static struct sock *udp6_lib_lookup2(const struct net *net,
> goto rescore;
> }
> }
> + /*
> + * if the nulls value we got at the end of this lookup is
> + * not the expected one, we must restart lookup.
> + * We probably met an item that was moved to another chain.
> + */
> + if (unlikely(get_nulls_value(node) != slot2))
> + goto begin;
> return result;
> }
>
> @@ -315,7 +336,7 @@ static struct sock *udp6_lib_lookup4(const struct net *net,
> * expected one, we must restart lookup. We probably met an item that
> * was moved to another chain due to rehash.
> */
> - if (get_nulls_value(node) != slot)
> + if (unlikely(get_nulls_value(node) != slot))
> goto begin;
>
> return NULL;
> @@ -956,9 +977,9 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> unsigned int hash2, hash2_any, offset;
> unsigned short hnum = ntohs(uh->dest);
> struct sock *sk, *first = NULL;
> + struct hlist_nulls_node *node;
> int sdif = inet6_sdif(skb);
> int dif = inet6_iif(skb);
> - struct hlist_node *node;
> struct udp_hslot *hslot;
> struct sk_buff *nskb;
> bool use_hash2;
> @@ -968,7 +989,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> hash2 = 0;
> hslot = udp_hashslot(udptable, net, hnum);
> use_hash2 = hslot->count > 10;
> - offset = offsetof(typeof(*sk), sk_node);
> + offset = offsetof(typeof(*sk), sk_nulls_node);
>
> if (use_hash2) {
> hash2_any = ipv6_portaddr_hash(net, &in6addr_any, hnum) &
> @@ -979,7 +1000,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> offset = offsetof(typeof(*sk), __sk_common.skc_portaddr_node);
> }
>
> - sk_for_each_entry_offset_rcu(sk, node, &hslot->head, offset) {
> + sk_nulls_for_each_entry_offset_rcu(sk, node, &hslot->head, offset) {
> if (!__udp_v6_is_mcast_sock(net, sk, uh->dest, daddr,
> uh->source, saddr, dif, sdif,
> hnum))
> @@ -1205,6 +1226,7 @@ static struct sock *__udp6_lib_demux_lookup(struct net *net,
> {
> struct udp_table *udptable = net->ipv4.udp_table;
> unsigned short hnum = ntohs(loc_port);
> + struct hlist_nulls_node *node;
> struct udp_hslot *hslot2;
> unsigned int hash2;
> __portpair ports;
> @@ -1214,7 +1236,7 @@ static struct sock *__udp6_lib_demux_lookup(struct net *net,
> hslot2 = udp_hashslot2(udptable, hash2);
> ports = INET_COMBINED_PORTS(rmt_port, hnum);
>
> - udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) {
> + udp_portaddr_for_each_entry_rcu(sk, node, &hslot2->head) {
> if (sk->sk_state == TCP_ESTABLISHED &&
> inet6_match(net, sk, rmt_addr, loc_addr, ports, dif, sdif))
> return sk;
next prev parent reply other threads:[~2026-09-22 12:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-22 9:57 [PATCH net] udp: restore hlist_nulls for primary and secondary hash tables Eric Dumazet
2026-09-22 12:08 ` David Laight [this message]
2026-09-23 12:59 ` netdev-bot+sashiko
2026-09-23 13:32 ` Eric Dumazet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260922130856.2c4f409a@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eric.dumazet@gmail.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=kuniyu@google.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=willemb@google.com \
--cc=willemdebruijn.kernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox