HEADS UP: socket and pcb reference changes entering tree today

[Robert Watson]

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
> @@ -671,9 +671,11 @@ 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
> @@ -1057,9 +1057,11 @@ 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
>