Date: Sun, 21 May 2006 19:53:36 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: Maxim Konovalov <maxim@macomnet.ru> Cc: current@FreeBSD.org Subject: Re: HEADS UP: socket and pcb reference changes entering tree today Message-ID: <20060521195004.H8068@fledge.watson.org> In-Reply-To: <20060521221016.F6324@mp2.macomnet.net> References: <20060317141627.W2181@fledge.watson.org> <20060329100839.V19236@fledge.watson.org> <20060401102918.P79188@fledge.watson.org> <20060401170554.R82503@fledge.watson.org> <20060402233436.P76562@fledge.watson.org> <20060515025600.U70399@mp2.macomnet.net> <20060521185034.K8068@fledge.watson.org> <20060521215823.H6324@mp2.macomnet.net> <20060521190358.G8068@fledge.watson.org> <20060521221016.F6324@mp2.macomnet.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060521195004.H8068>