From 747b50bb7cfea29103873832a2e2828ad7bc5129 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 17 Jul 2019 11:13:53 +0100 Subject: [PATCH] Try to improve handling of timeouts --- src/tuntap/conn.go | 12 +++++++----- src/yggdrasil/conn.go | 22 ++++++++++++++-------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/tuntap/conn.go b/src/tuntap/conn.go index f45e4a14..c9afa6e1 100644 --- a/src/tuntap/conn.go +++ b/src/tuntap/conn.go @@ -52,26 +52,29 @@ func (s *tunConn) reader() error { default: } s.tun.log.Debugln("Starting conn reader for", s) + defer s.tun.log.Debugln("Stopping conn reader for", s) var n int var err error read := make(chan bool) b := make([]byte, 65535) go func() { s.tun.log.Debugln("Starting conn reader helper for", s) + defer s.tun.log.Debugln("Stopping conn reader helper for", s) for { s.conn.SetReadDeadline(time.Now().Add(tunConnTimeout)) if n, err = s.conn.Read(b); err != nil { s.tun.log.Errorln(s.conn.String(), "TUN/TAP conn read error:", err) if e, eok := err.(yggdrasil.ConnError); eok { + s.tun.log.Debugln("Conn reader helper", s, "error:", e) switch { case e.Temporary(): + fallthrough + case e.Timeout(): read <- false continue - case e.Timeout(): - s.tun.log.Debugln("Conn reader for helper", s, "timed out") + case e.Closed(): fallthrough default: - s.tun.log.Debugln("Stopping conn reader helper for", s) s.close() return } @@ -94,7 +97,6 @@ func (s *tunConn) reader() error { } s.stillAlive() // TODO? Only stay alive if we read >0 bytes? case <-s.stop: - s.tun.log.Debugln("Stopping conn reader for", s) return nil } } @@ -109,10 +111,10 @@ func (s *tunConn) writer() error { default: } s.tun.log.Debugln("Starting conn writer for", s) + defer s.tun.log.Debugln("Stopping conn writer for", s) for { select { case <-s.stop: - s.tun.log.Debugln("Stopping conn writer for", s) return nil case b, ok := <-s.send: if !ok { diff --git a/src/yggdrasil/conn.go b/src/yggdrasil/conn.go index 5d1e77ac..2a286b04 100644 --- a/src/yggdrasil/conn.go +++ b/src/yggdrasil/conn.go @@ -16,6 +16,7 @@ type ConnError struct { error timeout bool temporary bool + closed bool maxsize int } @@ -38,6 +39,11 @@ func (e *ConnError) PacketTooBig() (bool, int) { return e.maxsize > 0, e.maxsize } +// Closed returns if the session is already closed and is now unusable. +func (e *ConnError) Closed() bool { + return e.closed +} + type Conn struct { core *Core nodeID *crypto.NodeID @@ -122,11 +128,11 @@ func (c *Conn) Read(b []byte) (int, error) { // Wait for some traffic to come through from the session select { case <-timer.C: - return 0, ConnError{errors.New("timeout"), true, false, 0} + return 0, ConnError{errors.New("timeout"), true, false, false, 0} case p, ok := <-sinfo.recv: // If the session is closed then do nothing if !ok { - return 0, errors.New("session is closed") + return 0, ConnError{errors.New("session is closed"), false, false, true, 0} } defer util.PutBytes(p.Payload) var err error @@ -135,7 +141,7 @@ func (c *Conn) Read(b []byte) (int, error) { defer close(done) // If the nonce is bad then drop the packet and return an error if !sinfo.nonceIsOK(&p.Nonce) { - err = ConnError{errors.New("packet dropped due to invalid nonce"), false, true, 0} + err = ConnError{errors.New("packet dropped due to invalid nonce"), false, true, false, 0} return } // Decrypt the packet @@ -144,7 +150,7 @@ func (c *Conn) Read(b []byte) (int, error) { // Check if we were unable to decrypt the packet for some reason and // return an error if we couldn't if !isOK { - err = ConnError{errors.New("packet dropped due to decryption failure"), false, true, 0} + err = ConnError{errors.New("packet dropped due to decryption failure"), false, true, false, 0} return } // Return the newly decrypted buffer back to the slice we were given @@ -168,7 +174,7 @@ func (c *Conn) Read(b []byte) (int, error) { select { // Send to worker case sinfo.worker <- workerFunc: case <-timer.C: - return 0, ConnError{errors.New("timeout"), true, false, 0} + return 0, ConnError{errors.New("timeout"), true, false, false, 0} } <-done // Wait for the worker to finish, failing this can cause memory errors (util.[Get||Put]Bytes stuff) // Something went wrong in the session worker so abort @@ -194,7 +200,7 @@ func (c *Conn) Write(b []byte) (bytesWritten int, err error) { defer close(done) // Does the packet exceed the permitted size for the session? if uint16(len(b)) > sinfo.getMTU() { - written, err = 0, ConnError{errors.New("packet too big"), true, false, int(sinfo.getMTU())} + written, err = 0, ConnError{errors.New("packet too big"), true, false, false, int(sinfo.getMTU())} return } // Encrypt the packet @@ -244,14 +250,14 @@ func (c *Conn) Write(b []byte) (bytesWritten int, err error) { // Hand over to the session worker defer func() { if recover() != nil { - err = errors.New("write failed, session already closed") + err = ConnError{errors.New("write failed, session already closed"), false, false, true, 0} close(done) } }() // In case we're racing with a close select { // Send to worker case sinfo.worker <- workerFunc: case <-timer.C: - return 0, ConnError{errors.New("timeout"), true, false, 0} + return 0, ConnError{errors.New("timeout"), true, false, false, 0} } // Wait for the worker to finish, otherwise there are memory errors ([Get||Put]Bytes stuff) <-done