svn commit: r280759 - head/sys/netinet

[Ian Lepore]

On Mon, 2015-03-30 at 00:07 +0300, Gleb Smirnoff wrote:
> On Sun, Mar 29, 2015 at 08:16:46AM -0700, Adrian Chadd wrote:
> A> On 29 March 2015 at 01:13, Hans Petter Selasky <hps at selasky.org> wrote:
> A> > On 03/28/15 20:16, Gleb Smirnoff wrote:
> A> >>
> A> >> +uint16_t
> A> >> +ip_newid(void)
> A> >> +{
> A> >> +
> A> >> +       counter_u64_add(V_ip_id, 1);
> A> >> +       return (htons((*(uint64_t *)zpcpu_get(V_ip_id)) & 0xffff));
> A> >> +}
> A> >
> A> >
> A> > Technically you would need to enter a critical section here, so that the
> A> > current process doesn't get swapped to a different CPU between the counter
> A> > add and the zpcpu_get.
> A> 
> A> +10000 here.
> A> 
> A> You can get preempted and shifted to another CPU at almost any point
> A> in time you're not explicitly asking not to be. It's.. frustrating.
> 
> What happens in case of the race is that one CPU will use counter of the
> other CPU, reusing the ID that was just used by the other CPU. And this
> isn't a fatal race, it is just reuse of ID. The probability of such event
> is much lower than probability of a collision due to overflowing uint16_t.
> 
> For example, if we emit 1 Mpps, then we overflow the ID counter 15 times
> per second. Every ID is reused 15 times within this small interval of time,
> which of course is larger than TTL. And we find this kinda acceptable.
> 
> To illustrate the probability of 1 instruction race, I would suggest to
> look at PCPU_ADD() implementation for all arches except amd64/i386 and
> find out that it is prone to this race. Hitting the race in PCPU_ADD()
> will usually lead to mangling vm_meter statistics. Did we ever observe
> this in practice? No.
> 

I was under the impression that PCPU_ADD(), like all pcpu r-m-w
accesses, requires the calling code to have taken steps to ensure the
code doesn't migrate during the sequence.  If callers aren't doing so,
that seems like a bug in the calling code, not the PCPU_ADD() code.

-- Ian