Re: [Linuxptp-devel] Workaround for 'timed out while polling for tx timestamp' on IGB

[Keller, J. E <jac...@in...>]

> -----Original Message-----
> From: Keller, Jacob E [mailto:jac...@in...]
> Sent: Thursday, April 27, 2017 3:58 PM
> To: Richard Cochran <ric...@gm...>; David Mirabito
> <da...@me...>
> Cc: lin...@li...
> Subject: Re: [Linuxptp-devel] Workaround for 'timed out while polling for tx
> timestamp' on IGB
> 
> > -----Original Message-----
> > From: Richard Cochran [mailto:ric...@gm...]
> > Sent: Thursday, April 27, 2017 11:25 AM
> > To: David Mirabito <da...@me...>
> > Cc: lin...@li...
> > Subject: Re: [Linuxptp-devel] Workaround for 'timed out while polling for tx
> > timestamp' on IGB
> >
> > On Thu, Apr 27, 2017 at 03:41:03PM +1000, David Mirabito wrote:
> > > * "Fixing" (if this is indeed a bug) was reasonably straight forward - more
> > > or less reordering steps 4,5,6 so that we wake the app only *after* we've
> > > unlocked the bit.
> >
> > If your analysis was correct, then yes, indeed this is a driver bug.
> > Please submit a patch on netdev.
> >
> 
> Can I clarify here, David, you're suggesting that you instead clear the bitlock
> earlier before you call the timestamp function? I think that's probably a good
> thing and minimizing the time that we hold a lock is good. I suspect that most of
> the Intel drivers are at fault here and can make some patches for them. Or you
> can if you wish.
> 
> There *is* a fundamental limit that the hardware assumes that only one transmit
> timestamp request at a time, so we can't actually do any better. But we can
> unlock as soon as we read the timestamp registers, which should help this race.
> 
> Thanks,
> Jake

I sent a patch to Intel Wired LAN, and I Cc'd both of you on it. Feel free to comment. I fixed up e1000e, igb, ixgbe, and i40e. Please comment indicating if you think this fix is correct. I believe it is correct from your original comment.

One change we could make as well, is to increment the timestamp dropped counter for Tx whenever we get a request that we can't fulfill, but I didn't change that in my current patch.

Thanks,
Jake