From owner-svn-src-all@FreeBSD.ORG Sat Mar 28 21:34:08 2015 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 47A66A4B; Sat, 28 Mar 2015 21:34:08 +0000 (UTC) Received: from zxy.spb.ru (zxy.spb.ru [195.70.199.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F164CC79; Sat, 28 Mar 2015 21:34:07 +0000 (UTC) Received: from slw by zxy.spb.ru with local (Exim 4.84 (FreeBSD)) (envelope-from ) id 1YbyMm-0002N8-2e; Sun, 29 Mar 2015 00:34:04 +0300 Date: Sun, 29 Mar 2015 00:34:04 +0300 From: Slawa Olhovchenkov To: John-Mark Gurney Subject: Re: svn commit: r280759 - head/sys/netinet Message-ID: <20150328213403.GB74532@zxy.spb.ru> References: <201503271326.t2RDQxd3056112@svn.freebsd.org> <20150328083443.GV64665@FreeBSD.org> <20150328172313.GC51048@funkthat.com> <20150328181833.GX64665@FreeBSD.org> <20150328204333.GF51048@funkthat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150328204333.GF51048@funkthat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: slw@zxy.spb.ru X-SA-Exim-Scanned: No (on zxy.spb.ru); SAEximRunCond expanded to false Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Gleb Smirnoff , src-committers@FreeBSD.org, Fabien Thomas X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 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, 28 Mar 2015 21:34:08 -0000 On Sat, Mar 28, 2015 at 01:43:33PM -0700, John-Mark Gurney wrote: > Gleb Smirnoff wrote this message on Sat, Mar 28, 2015 at 21:18 +0300: > > On Sat, Mar 28, 2015 at 10:23:13AM -0700, John-Mark Gurney wrote: > > J> > On Fri, Mar 27, 2015 at 01:26:59PM +0000, Fabien Thomas wrote: > > J> > F> Author: fabient > > J> > F> Date: Fri Mar 27 13:26:59 2015 > > J> > F> New Revision: 280759 > > J> > F> URL: https://svnweb.freebsd.org/changeset/base/280759 > > J> > F> > > J> > F> Log: > > J> > F> On multi CPU systems, we may emit successive packets with the same id. > > J> > F> Fix the race by using an atomic operation. > > J> > F> > > J> > F> Differential Revision: https://reviews.freebsd.org/D2141 > > J> > F> Obtained from: emeric.poupon@stormshield.eu > > J> > F> MFC after: 1 week > > J> > F> Sponsored by: Stormshield > > J> > > > J> > The D2141 says that benchmarking were done in presence of IPSEC, which > > J> > of course is the bottleneck and performance of this instruction can't > > J> > be benchmarked in its presence. Anyway, I believe that results of > > J> > right benchmark would still show little difference between atomic and > > J> > non-atomic increment of a shared value. > > J> > > > J> > I think we can use per-cpu ID counters, each CPU incrementing its > > J> > own. If we start with random values, then probability of two packets with > > J> > the same ID emitting at the allowed timeframe will be acceptably small. > > J> > > J> Please do not use per-cpu id counters.. That will just push the > > J> duplicate ids to being more rare, but just as much of a problem... > > J> > > J> Please read: > > J> https://tools.ietf.org/html/rfc6864 > > J> > > J> And then implement one hased upon source/dest/protocol... > > > > I know about this RFC, but using per-cpu ID counter if a low hanging > > fruit and is improvement. What is important not only improvement in > > terms of lowering probability of collision, but also easy improvement in > > performance, since now global ip_id increment is a cache line thrasher. > > > > So, I don't see how existense of the RFC blocks introducing an easy > > improvement. And if anyone works on implementing the RFC, he shouldn't > > care what is in head before his work, either global or per-cpu counter. > > Your comment: "fix the race", to me reads that it was a final solution, > not simply a stop gap... > > If you had simply said that this is a stop gap till someone properly > implements RFC6864, that would make it obvious that you were aware of > it.. Nothing in your commit message said that you were aware that this > wasn't the correct final solution... Information like that should be > included in the commit message so people don't assume that this is the > final solution... > > Anyways, are we really sending so many fragments that we are thrashing > the cache line? I'd imagine a much lower hanging fruit is only provide > ip_id when a non-atomic packet is being sent... In this case may be do range allocation of ID (per-CPU)? For example, allocate 128 ID, not one ID?