TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2025-02-18 02:48:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

health, wgengine/magicsock: avoid receive function false positives

Browse Source 
Avery reported a sub-ms health transition from "receiveIPv4 not running" to "ok".

To avoid these transient false-positives, be more precise about
the expected lifetime of receive funcs. The problematic case is one in which
they were started but exited prior to a call to connBind.Close.
Explicitly represent started vs running state, taking care with the order of updates.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
This commit is contained in:
Josh Bleecher Snyder 2021-04-22 12:37:53 -07:00
parent 3411bb959a
commit 5835a3f553
2 changed files with 13 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
health/health.go
View File

@ -37,9 +37,11 @@ var (
ipnWantRunning bool
anyInterfaceUp = true // until told otherwise
receiveIPv4Started bool
receiveIPv4Running bool
receiveIPv6Started bool
receiveIPv6Running bool
receiveDERPStarted bool
receiveDERPRunning bool
)
@ -218,9 +220,11 @@ func SetAnyInterfaceUp(up bool) {
selfCheckLocked()
}
func SetReceiveIPv4Started(running bool) { setHealthBool(&receiveIPv4Started, running) }
func SetReceiveIPv4Running(running bool) { setHealthBool(&receiveIPv4Running, running) }
func SetReceiveIPv6Started(running bool) { setHealthBool(&receiveIPv6Started, running) }
func SetReceiveIPv6Running(running bool) { setHealthBool(&receiveIPv6Running, running) }
func SetReceiveDERPStarted(running bool) { setHealthBool(&receiveDERPStarted, running) }
func SetReceiveDERPRunning(running bool) { setHealthBool(&receiveDERPRunning, running) }
func setHealthBool(dst *bool, b bool) {
@ -280,10 +284,10 @@ func overallErrorLocked() error {
_ = lastMapRequestHeard
var errs []error
if !receiveIPv4Running {
if receiveIPv4Started && !receiveIPv4Running {
errs = append(errs, errors.New("receiveIPv4 not running"))
}
if !receiveDERPRunning {
if receiveDERPStarted && !receiveDERPRunning {
errs = append(errs, errors.New("receiveDERP not running"))
}
if receiveIPv6Started && !receiveIPv6Running {

8
wgengine/magicsock/magicsock.go
View File

@ -1581,8 +1581,8 @@ func (c *Conn) noteRecvActivityFromEndpoint(e conn.Endpoint) {
// receiveIPv6 receives a UDP IPv6 packet. It is called by wireguard-go.
func (c *Conn) receiveIPv6(b []byte) (int, conn.Endpoint, error) {
health.SetReceiveIPv6Started(true)
health.SetReceiveIPv6Running(true)
health.SetReceiveIPv6Started(true)
defer health.SetReceiveIPv6Running(false)
for {
n, ipp, err := c.pconn6.ReadFromNetaddr(b)
@ -1598,6 +1598,7 @@ func (c *Conn) receiveIPv6(b []byte) (int, conn.Endpoint, error) {
// receiveIPv4 receives a UDP IPv4 packet. It is called by wireguard-go.
func (c *Conn) receiveIPv4(b []byte) (n int, ep conn.Endpoint, err error) {
health.SetReceiveIPv4Running(true)
health.SetReceiveIPv4Started(true)
defer health.SetReceiveIPv4Running(false)
for {
n, ipp, err := c.pconn4.ReadFromNetaddr(b)
@ -1652,6 +1653,7 @@ func (c *Conn) receiveIP(b []byte, ipp netaddr.IPPort, cache *ippEndpointCache)
// found, the returned error is errLoopAgain.
func (c *connBind) receiveDERP(b []byte) (n int, ep conn.Endpoint, err error) {
health.SetReceiveDERPRunning(true)
health.SetReceiveDERPStarted(true)
defer health.SetReceiveDERPRunning(false)
for dm := range c.derpRecvCh {
if c.Closed() {
@ -2441,6 +2443,9 @@ func (c *connBind) Close() error {
}
c.closed = true
// Unblock all outstanding receives.
// Tell the health checker that we're closing the connections
// before actually closing them to avoid false positives.
health.SetReceiveIPv4Started(false)
c.pconn4.Close()
if c.pconn6 != nil {
health.SetReceiveIPv6Started(false)
@ -2448,6 +2453,7 @@ func (c *connBind) Close() error {
}
// Send an empty read result to unblock receiveDERP,
// which will then check connBind.Closed.
health.SetReceiveDERPStarted(false)
c.derpRecvCh <- derpReadResult{}
return nil
}