From owner-svn-src-all@FreeBSD.ORG Sat Jun 4 14:54:32 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4A5481065672; Sat, 4 Jun 2011 14:54:32 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 217538FC17; Sat, 4 Jun 2011 14:54:32 +0000 (UTC) Received: from [192.168.2.112] (host86-173-95-198.range86-173.btcentralplus.com [86.173.95.198]) by cyrus.watson.org (Postfix) with ESMTPSA id D8CBA46B03; Sat, 4 Jun 2011 10:54:29 -0400 (EDT) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: multipart/mixed; boundary=Apple-Mail-7-327068992 From: "Robert N. M. Watson" In-Reply-To: <20110604143043.GE17764@nereid> Date: Sat, 4 Jun 2011 15:54:26 +0100 Message-Id: <64903CD6-7AED-4BF6-9FDF-B6748690EA8A@FreeBSD.org> References: <201105300943.p4U9htjI070096@svn.freebsd.org> <20110604143043.GE17764@nereid> To: Kristof Provost X-Mailer: Apple Mail (2.1084) X-Mailman-Approved-At: Sat, 04 Jun 2011 15:07:11 +0000 Cc: "freebsd-current@FreeBSD.org Current" Subject: Divert socket problem (was: Re: svn commit: r222488 - in head/sys: contrib/pf/net netinet netinet/ipfw netinet6) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 Jun 2011 14:54:32 -0000 --Apple-Mail-7-327068992 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On 4 Jun 2011, at 15:30, Kristof Provost wrote: > div_bind probably also needs to surround the call to in_pcbbind with > INP_HASHW(UN)LOCK(...) >=20 > I'm currently running 222680. I've only now seen the issue, but I've = also > just now activated INVARIANTS. Hi Kristof: Thanks for the detailed report, and yes, it looks like that is exactly = what is required. Could you try the attached patch? Robert --Apple-Mail-7-327068992 Content-Disposition: attachment; filename=20110604-divert-fix.diff Content-Type: application/octet-stream; x-unix-mode=0644; name="20110604-divert-fix.diff" Content-Transfer-Encoding: 7bit IP divert sockets use their inpcbinfo for port reservation, although not for lookup. I missed its call to in_pcbbind() when preparing previous patches, which would lead to a lock assertion failure (although problem not an actual race condition due to global pcbinfo locks providing required synchronisation -- in this particular case only). This change adds the missing locking of the pcbhash lock. (Existing comments in the ipdivert code question the need for using the global hash to manage the namespace, as really it's a simple port namespace and not an address/port namespace. Also, although in_pcbbind is used to manage reservations, the hash tables aren't used for lookup. It might be a good idea to make them use hashed lookup, or to use a different reservation scheme.) Reviewed by: bz Reported by: Kristof Provost Sponsored by: Juniper Networks Index: ip_divert.c =================================================================== --- ip_divert.c (revision 222672) +++ ip_divert.c (working copy) @@ -530,7 +530,9 @@ ((struct sockaddr_in *)nam)->sin_addr.s_addr = INADDR_ANY; INP_INFO_WLOCK(&V_divcbinfo); INP_WLOCK(inp); + INP_HASH_WLOCK(&V_divcbinfo); error = in_pcbbind(inp, nam, td->td_ucred); + INP_HASH_WUNLOCK(&V_divcbinfo); INP_WUNLOCK(inp); INP_INFO_WUNLOCK(&V_divcbinfo); return error; --Apple-Mail-7-327068992--