Re: HEADS UP: socket and pcb reference changes entering tree today

From: Robert Watson <rwatson_at_FreeBSD.org>
Date: Sun, 21 May 2006 19:53:36 +0100 (BST)
On Sun, 21 May 2006, Maxim Konovalov wrote:

>>> I "copied" this code from udp_usrreq.c::udp_disconnect().  There is no 
>>> such lock.  Is it a bug too?
>>
>> Yes.
>>
>> I have some intuitions about why the datagram protocols manually frob the 
>> disconnected flag rather than calling soisdisconnected(), but am generally 
>> unsure that this is the right thing.
>
> I thought about that but hadn't find any explanations.
>
> I'm testing a patch below:

This patch looks good to me.  The races here are quite minor in practice, 
involving simultaneous changes of connection state between different threads 
acting on the same socket, but also the sort of thing that would be almost 
impossible to debug if it occurred in practice :-).

Robert N M Watson

> Index: raw_ip.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.161
> diff -u -p -r1.161 raw_ip.c
> --- raw_ip.c	15 May 2006 09:28:57 -0000	1.161
> +++ raw_ip.c	21 May 2006 18:13:14 -0000
> _at__at_ -671,9 +671,11 _at__at_ rip_disconnect(struct socket *so)
> 	INP_INFO_WLOCK(&ripcbinfo);
> 	INP_LOCK(inp);
> 	inp->inp_faddr.s_addr = INADDR_ANY;
> +	SOCK_LOCK(so);
> +	so->so_state &= ~SS_ISCONNECTED;
> +	SOCK_UNLOCK(so);
> 	INP_UNLOCK(inp);
> 	INP_INFO_WUNLOCK(&ripcbinfo);
> -	so->so_state &= ~SS_ISCONNECTED;
> 	return (0);
> }
>
> Index: udp_usrreq.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.188
> diff -u -p -r1.188 udp_usrreq.c
> --- udp_usrreq.c	6 May 2006 11:24:59 -0000	1.188
> +++ udp_usrreq.c	21 May 2006 18:12:43 -0000
> _at__at_ -1057,9 +1057,11 _at__at_ udp_disconnect(struct socket *so)
>
> 	in_pcbdisconnect(inp);
> 	inp->inp_laddr.s_addr = INADDR_ANY;
> +	SOCK_LOCK(so)
> +	so->so_state &= ~SS_ISCONNECTED;		/* XXX */
> +	SOCK_UNLOCK(so)
> 	INP_UNLOCK(inp);
> 	INP_INFO_WUNLOCK(&udbinfo);
> -	so->so_state &= ~SS_ISCONNECTED;		/* XXX */
> 	return 0;
> }
>
> %%%
>
> -- 
> Maxim Konovalov
>
Received on Sun May 21 2006 - 16:53:37 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:56 UTC