svn commit: r280759 - head/sys/netinet
Slawa Olhovchenkov
slw at zxy.spb.ru
Sat Mar 28 21:34:08 UTC 2015
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 at 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?
More information about the svn-src-all
mailing list