Skip to content

Commit

Permalink
udp: Make rehash4 independent in udp_lib_rehash()
Browse files Browse the repository at this point in the history
As discussed in [0], rehash4 could be missed in udp_lib_rehash() when
udp hash4 changes while hash2 doesn't change. This patch fixes this by
moving rehash4 codes out of rehash2 checking, and then rehash2 and
rehash4 are done separately.

By doing this, we no longer need to call rehash4 explicitly in
udp_lib_hash4(), as the rehash callback in __ip4_datagram_connect takes
it. Thus, now udp_lib_hash4() returns directly if the sk is already
hashed.

Note that uhash4 may fail to work under consecutive connect(<dst
address>) calls because rehash() is not called with every connect(). To
overcome this, connect(<AF_UNSPEC>) needs to be called after the next
connect to a new destination.

[0]
https://lore.kernel.org/all/4761e466ab9f7542c68cdc95f248987d127044d2.1733499715.git.pabeni@redhat.com/

Fixes: 78c91ae ("ipv4/udp: Add 4-tuple hash for connected socket")
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
Link: https://patch.msgid.link/20250110010810.107145-1-lulie@linux.alibaba.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Philo Lu authored and Paolo Abeni committed Jan 14, 2025
1 parent 1f691a1 commit 644f910
Showing 1 changed file with 27 additions and 19 deletions.
46 changes: 27 additions & 19 deletions net/ipv4/udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ static struct sock *udp4_lib_lookup4(const struct net *net,
return NULL;
}

/* In hash4, rehash can happen in connect(), where hash4_cnt keeps unchanged. */
/* udp_rehash4() only checks hslot4, and hash4_cnt is not processed. */
static void udp_rehash4(struct udp_table *udptable, struct sock *sk,
u16 newhash4)
{
Expand Down Expand Up @@ -582,15 +582,13 @@ 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,
* so rehash4 is needed.
/* Connected udp socket can re-connect to another remote address, which
* will be handled by rehash. Thus no need to redo hash4 here.
*/
udptable = net->ipv4.udp_table;
if (udp_hashed4(sk)) {
udp_rehash4(udptable, sk, hash);
if (udp_hashed4(sk))
return;
}

udptable = net->ipv4.udp_table;
hslot = udp_hashslot(udptable, net, udp_sk(sk)->udp_port_hash);
hslot2 = udp_hashslot2(udptable, udp_sk(sk)->udp_portaddr_hash);
hslot4 = udp_hashslot4(udptable, hash);
Expand Down Expand Up @@ -2173,14 +2171,14 @@ void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
struct udp_table *udptable = udp_get_table_prot(sk);
struct udp_hslot *hslot, *hslot2, *nhslot2;

hslot = udp_hashslot(udptable, sock_net(sk),
udp_sk(sk)->udp_port_hash);
hslot2 = udp_hashslot2(udptable, udp_sk(sk)->udp_portaddr_hash);
nhslot2 = udp_hashslot2(udptable, newhash);
udp_sk(sk)->udp_portaddr_hash = newhash;

if (hslot2 != nhslot2 ||
rcu_access_pointer(sk->sk_reuseport_cb)) {
hslot = udp_hashslot(udptable, sock_net(sk),
udp_sk(sk)->udp_port_hash);
/* we must lock primary chain too */
spin_lock_bh(&hslot->lock);
if (rcu_access_pointer(sk->sk_reuseport_cb))
Expand All @@ -2199,19 +2197,29 @@ void udp_lib_rehash(struct sock *sk, u16 newhash, u16 newhash4)
spin_unlock(&nhslot2->lock);
}

if (udp_hashed4(sk)) {
udp_rehash4(udptable, sk, newhash4);
spin_unlock_bh(&hslot->lock);
}

/* Now process hash4 if necessary:
* (1) update hslot4;
* (2) update hslot2->hash4_cnt.
* Note that hslot2/hslot4 should be checked separately, as
* either of them may change with the other unchanged.
*/
if (udp_hashed4(sk)) {
spin_lock_bh(&hslot->lock);

if (hslot2 != nhslot2) {
spin_lock(&hslot2->lock);
udp_hash4_dec(hslot2);
spin_unlock(&hslot2->lock);
udp_rehash4(udptable, sk, newhash4);
if (hslot2 != nhslot2) {
spin_lock(&hslot2->lock);
udp_hash4_dec(hslot2);
spin_unlock(&hslot2->lock);

spin_lock(&nhslot2->lock);
udp_hash4_inc(nhslot2);
spin_unlock(&nhslot2->lock);
}
spin_lock(&nhslot2->lock);
udp_hash4_inc(nhslot2);
spin_unlock(&nhslot2->lock);
}

spin_unlock_bh(&hslot->lock);
}
}
Expand Down

0 comments on commit 644f910

Please sign in to comment.