Re: [PATCH v3 bpf-next 4/5] [RFC] udp: Fix destroying UDP listening sockets

[Martin KaFai Lau <martin.lau@linux.dev>]

On 3/21/23 11:45 AM, Aditi Ghag wrote:
> Previously, UDP listening sockets that bind'ed to a port
> weren't getting properly destroyed via udp_abort.
> Specifically, the sockets were left in the UDP hash table with
> unset source port.
> Fix the issue by unconditionally unhashing and resetting source
> port for sockets that are getting destroyed. This would mean
> that in case of sockets listening on wildcarded address and
> on a specific address with a common port, users would have to
> explicitly select the socket(s) they intend to destroy.
> 
> Signed-off-by: Aditi Ghag <aditi.ghag@isovalent.com>
> ---
>   net/ipv4/udp.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 02d357713838..a495ac88fcee 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1965,6 +1965,25 @@ int udp_pre_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
>   }
>   EXPORT_SYMBOL(udp_pre_connect);
>   
> +int __udp_disconnect_with_abort(struct sock *sk)
> +{
> +	struct inet_sock *inet = inet_sk(sk);
> +
> +	sk->sk_state = TCP_CLOSE;
> +	inet->inet_daddr = 0;
> +	inet->inet_dport = 0;
> +	sock_rps_reset_rxhash(sk);
> +	sk->sk_bound_dev_if = 0;
> +	inet_reset_saddr(sk);
> +	inet->inet_sport = 0;
> +	sk_dst_reset(sk);
> +	/* (TBD) In case of sockets listening on wildcard and specific address
> +	 * with a common port, socket will be removed from {hash, hash2} table.
> +	 */
> +	sk->sk_prot->unhash(sk);

hmm... not sure if I understand the use case. The idea is to enforce the user 
space to bind() again when it gets error from read(fd) because the source 
ip/port needs to change when sending to another dst IP/port? Does it have a 
usage example in the selftests?

> +	return 0;
> +}
> +
>   int __udp_disconnect(struct sock *sk, int flags)
>   {
>   	struct inet_sock *inet = inet_sk(sk);
> @@ -2937,7 +2956,7 @@ int udp_abort(struct sock *sk, int err)
>   
>   	sk->sk_err = err;
>   	sk_error_report(sk);
> -	__udp_disconnect(sk, 0);
> +	__udp_disconnect_with_abort(sk);
>   
>   out:
>   	if (!has_current_bpf_ctx())