Netdev List
 help / color / mirror / Atom feed
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;


  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