ipn/ipnlocal: do not process old status messages received out of order

When `setWgengineStatus` is invoked concurrently from multiple
goroutines, it is possible that the call invoked with a newer status is
processed before a call with an older status. e.g. a status that has
endpoints might be followed by a status without endpoints. This causes
unnecessary work in the engine and can result in packet loss.

This patch adds an `AsOf time.Time` field to the status to specifiy when the
status was calculated, which later allows `setWgengineStatus` to ignore
any status messages it receives that are older than the one it has
already processed.

Updates tailscale/corp#2579

Signed-off-by: Maisem Ali <maisem@tailscale.com>
This commit is contained in:
Maisem Ali
2022-03-26 20:12:12 -07:00
committed by Maisem Ali
parent 753f1bfad4
commit 6fecc16c3b
4 changed files with 14 additions and 2 deletions

View File

@@ -62,6 +62,7 @@ func (nt *notifyThrottler) put(n ipn.Notify) {
// drain pulls the notifications out of the queue, asserting that there are
// exactly count notifications that have been put so far.
func (nt *notifyThrottler) drain(count int) []ipn.Notify {
nt.t.Helper()
nt.mu.Lock()
ch := nt.ch
nt.mu.Unlock()
@@ -923,7 +924,7 @@ func TestStateMachine(t *testing.T) {
}
notifies.expect(1)
// Fake a DERP connection.
b.setWgengineStatus(&wgengine.Status{DERPs: 1}, nil)
b.setWgengineStatus(&wgengine.Status{DERPs: 1, AsOf: time.Now()}, nil)
{
nn := notifies.drain(1)
cc.assertCalls("unpause")
@@ -1016,7 +1017,7 @@ func TestWGEngineStatusRace(t *testing.T) {
if i == 0 {
n = 1
}
b.setWgengineStatus(&wgengine.Status{DERPs: n}, nil)
b.setWgengineStatus(&wgengine.Status{AsOf: time.Now(), DERPs: n}, nil)
}(i)
}
wg.Wait()