Date: Sun, 29 Mar 2015 00:34:04 +0300 From: Slawa Olhovchenkov <slw@zxy.spb.ru> To: John-Mark Gurney <jmg@funkthat.com> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org>, src-committers@FreeBSD.org, Fabien Thomas <fabient@FreeBSD.org> Subject: Re: svn commit: r280759 - head/sys/netinet Message-ID: <20150328213403.GB74532@zxy.spb.ru> In-Reply-To: <20150328204333.GF51048@funkthat.com> References: <201503271326.t2RDQxd3056112@svn.freebsd.org> <20150328083443.GV64665@FreeBSD.org> <20150328172313.GC51048@funkthat.com> <20150328181833.GX64665@FreeBSD.org> <20150328204333.GF51048@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150328213403.GB74532>