m-lab / ndt-server / 457

Committed 08 Mar 2019 08:53PM UTC coverage: 62.34% (+1.6%) from 60.737%

Build # 457

Build Type

Pull #76

travis-ci

Committed by

web-flow

Commit Message

fix(ndt-server.go): make ndt7 work again

Cherry pick of 27a942d58
with the required changes to make it apply cleanly.

With this diff applied, ndt7 works again.

Pull Request Pull Request #76: fix(ndt-server.go): make ndt7 work again

Run Details

1 of 1 new or added line in 1 file covered. (100.0%)

778 of 1248 relevant lines covered (62.34%)

0.66 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

91.67

/fdcache/fdcache.go

// Package fdcache contains a mechanism to obtain the file descriptor bound
// to a websocket.Conn. We initially implemented this mechanism for BBR hence
// the following explanation is mainly tailored on the BBR case. But we also
// have other use cases for this feature in tree, e.g. reading TCP_INFO.
//
// To read BBR variables, we need a file descriptor. When serving a WebSocket
// client we have a websocket.Conn instance. The UnderlyingConn() method allows
// us to get the corresponding net.Conn, which typically is a tls.Conn. Yet,
// obtaining a file descriptor from a tls.Conn seems complex, because the
// underlying socket connection is private. Still, we've a custom listener that
// is required to turn on BBR (see tcpListenerEx). At that point, we can
// obtain a *os.File from the *net.TCPConn. From such *os.File, we can then
// get a file descriptor. However, there are some complications:
//
// a) the returned *os.File is bound to another file descriptor that is
//    a dup() of the one inside the *net.Conn, so, we should keep that
//    *os.File alive during the whole ndt7 measurement;
//
// b) in Go < 1.11, using this technique makes the file descriptor use
//    blocking I/O, which spawns more threads (see below).
//
// For these reasons, we're keeping a cache mapping between the socket's four
// tuple (i.e. local and remote address and port) and the *os.File. We use the
// four tuple because, in principle, a server can be serving on more than a
// single local IP, so only using the remote endpoint may not be enough.
//
// In the good case, this is what is gonna happen:
//
// 1. a connection is accepted in tcpListenerEx, so we have a *net.TCPConn;
//
// 2. using the *net.Conn, we turn on BBR and cache the *os.File using
//    bbr.EnableAndRememberFile() with the four tuple as the key;
//
// 3. WebSocket negotiation is successful, so we have a websocket.Conn, from
//    which we can get the underlying connection and hence the four tuple;
//
// 4. using the four tuple, we can retrieve the *os.File, removing it from
//    the cache using bbr.GetAndForgetFile();
//
// 5. we defer *os.File.Close() until the end of the WebSocket serving loop and
//    periodically we use such file to obtain the file descriptor and read the
//    BBR variables using bbr.GetMaxBandwidthAndMinRTT().
//
// Because a connection might be closed between steps 2. and 3. (i.e. after
// the connection is accepted and before the HTTP layer finishes reading the
// request and determines that it should be routed to the handler that we
// have configured), we also need a stale entry management mechanism so that
// we delete *os.File instances cached for too much time.
//
// Depending on whether Golang calls shutdown() when a socket is closed or
// not, it might be that this caching mechanism keeps connections alive for
// more time than expected. The specific case where we can have this issue
// is the one where we receive a HTTP connection that is not a valid UPGRADE
// request, but a valid HTTP request. To avoid this issue, we SHOULD make
// sure to remove the *os.File from the cache basically everytime we got our
// handler called, regardless of whether the request is a valid UPGRADE.
package fdcache

import (
        "errors"
        "net"
        "os"
        "sync"
        "time"

        "github.com/m-lab/go/uuid"
)

// connKey is the key associated to a TCP connection.
type connKey string

// makekey creates a connKey from |conn|.
func makekey(conn net.Conn) connKey {
        return connKey(conn.LocalAddr().String() + "<=>" + conn.RemoteAddr().String())
}

// entry is an entry inside the cache.
type entry struct {
        Fp    *os.File
        Stamp time.Time
}

// cache maps a connKey to the corresponding *os.File.
var cache = make(map[connKey]entry)

// mutex serializes access to cache.
var mutex sync.Mutex

// lastCheck is last time when we checked the cache for stale entries.
var lastCheck time.Time

// checkInterval is the interval between each check for stale entries.
const checkInterval = 500 * time.Millisecond

// maxInactive is the amount of time after which an entry is stale.
const maxInactive = 3 * time.Second

// TCPConnToFile maps |tc| to the corresponding *os.File. Note that the
// returned *os.File is a dup() of the original, hence you now have ownership
// of two objects that you need to remember to defer Close() of.
func TCPConnToFile(tc *net.TCPConn) (*os.File, error) {
        // Implementation note: according to a 2013 message on golang-nuts [1], the
        // code that follows is broken on Unix because calling File() makes the socket
        // blocking so causing Go to use more threads and, additionally, "timer wheel
        // inside net package never fires". However, an April, 19 2018 commit
        // on src/net/tcpsock.go apparently has removed such restriction and so now
        // (i.e. since go1.11beta1) it's safe to use the code below [2, 3].
        //
        // [1] https://grokbase.com/t/gg/golang-nuts/1349whs82r
        //
        // [2] https://github.com/golang/go/commit/60e3ebb9cba
        //
        // [3] https://github.com/golang/go/issues/24942
        //
        // For this reason, this code only works correctly with go >= 1.11.
        return tc.File()
}

// OwnFile transfers ownership of |fp| to the fdcache module. Passing a nil
// |fp| to this function is a programming error that will cause a panic.
func OwnFile(conn net.Conn, fp *os.File) {
        if fp == nil {
                panic("You passed me a nil *os.File")
        }
        curTime := time.Now()
        key := makekey(conn)
        mutex.Lock()
        defer mutex.Unlock()
        if curTime.Sub(lastCheck) > checkInterval {
                lastCheck = curTime
                // Note: in Golang it's safe to remove elements from the map while
                // iterating it. See <https://github.com/golang/go/issues/9926>.
                for key, entry := range cache {
                        if curTime.Sub(entry.Stamp) > maxInactive {
                                entry.Fp.Close()
                                delete(cache, key)
                        }
                }
        }
        cache[key] = entry{
                Fp:    fp, // This takes ownership of fp
                Stamp: curTime,
        }
}

// GetAndForgetFile returns the *os.File bound to |conn| that was previously
// saved with EnableAndRememberFile, or nil if no file was found. Note that you
// are given ownership of the returned file pointer. As the name implies, the
// *os.File is removed from the cache by this operation.
func GetAndForgetFile(conn net.Conn) *os.File {
        key := makekey(conn)
        mutex.Lock()
        defer mutex.Unlock()
        entry, found := cache[key]
        if !found {
                return nil
        }
        delete(cache, key)
        return entry.Fp // Pass ownership to caller
}

// GetUUID returns the UUID for a passed-in connection.
func GetUUID(conn net.Conn) (string, error) {
        key := makekey(conn)
        mutex.Lock()
        defer mutex.Unlock()
        entry, found := cache[key]
        if !found {
                return "", errors.New("fd not found")
        }
        id, err := uuid.FromFile(entry.Fp)
        return id, err
}

1	// Package fdcache contains a mechanism to obtain the file descriptor bound
2	// to a websocket.Conn. We initially implemented this mechanism for BBR hence
3	// the following explanation is mainly tailored on the BBR case. But we also
4	// have other use cases for this feature in tree, e.g. reading TCP_INFO.
5	//
6	// To read BBR variables, we need a file descriptor. When serving a WebSocket
7	// client we have a websocket.Conn instance. The UnderlyingConn() method allows
8	// us to get the corresponding net.Conn, which typically is a tls.Conn. Yet,
9	// obtaining a file descriptor from a tls.Conn seems complex, because the
10	// underlying socket connection is private. Still, we've a custom listener that
11	// is required to turn on BBR (see tcpListenerEx). At that point, we can
12	// obtain a os.File from the net.TCPConn. From such *os.File, we can then
13	// get a file descriptor. However, there are some complications:
14	//
15	// a) the returned *os.File is bound to another file descriptor that is
16	// a dup() of the one inside the *net.Conn, so, we should keep that
17	// *os.File alive during the whole ndt7 measurement;
18	//
19	// b) in Go < 1.11, using this technique makes the file descriptor use
20	// blocking I/O, which spawns more threads (see below).
21	//
22	// For these reasons, we're keeping a cache mapping between the socket's four
23	// tuple (i.e. local and remote address and port) and the *os.File. We use the
24	// four tuple because, in principle, a server can be serving on more than a
25	// single local IP, so only using the remote endpoint may not be enough.
26	//
27	// In the good case, this is what is gonna happen:
28	//
29	// 1. a connection is accepted in tcpListenerEx, so we have a *net.TCPConn;
30	//
31	// 2. using the net.Conn, we turn on BBR and cache the os.File using
32	// bbr.EnableAndRememberFile() with the four tuple as the key;
33	//
34	// 3. WebSocket negotiation is successful, so we have a websocket.Conn, from
35	// which we can get the underlying connection and hence the four tuple;
36	//
37	// 4. using the four tuple, we can retrieve the *os.File, removing it from
38	// the cache using bbr.GetAndForgetFile();
39	//
40	// 5. we defer *os.File.Close() until the end of the WebSocket serving loop and
41	// periodically we use such file to obtain the file descriptor and read the
42	// BBR variables using bbr.GetMaxBandwidthAndMinRTT().
43	//
44	// Because a connection might be closed between steps 2. and 3. (i.e. after
45	// the connection is accepted and before the HTTP layer finishes reading the
46	// request and determines that it should be routed to the handler that we
47	// have configured), we also need a stale entry management mechanism so that
48	// we delete *os.File instances cached for too much time.
49	//
50	// Depending on whether Golang calls shutdown() when a socket is closed or
51	// not, it might be that this caching mechanism keeps connections alive for
52	// more time than expected. The specific case where we can have this issue
53	// is the one where we receive a HTTP connection that is not a valid UPGRADE
54	// request, but a valid HTTP request. To avoid this issue, we SHOULD make
55	// sure to remove the *os.File from the cache basically everytime we got our
56	// handler called, regardless of whether the request is a valid UPGRADE.
57	package fdcache
58
59	import (
60	"errors"
61	"net"
62	"os"
63	"sync"
64	"time"
65
66	"github.com/m-lab/go/uuid"
67	)
68
69	// connKey is the key associated to a TCP connection.
70	type connKey string
71
72	// makekey creates a connKey from \|conn\|.
73	func makekey(conn net.Conn) connKey {	1✔
74	return connKey(conn.LocalAddr().String() + "<=>" + conn.RemoteAddr().String())	1✔
75	}	1✔
76
77	// entry is an entry inside the cache.
78	type entry struct {
79	Fp *os.File
80	Stamp time.Time
81	}
82
83	// cache maps a connKey to the corresponding *os.File.
84	var cache = make(map[connKey]entry)
85
86	// mutex serializes access to cache.
87	var mutex sync.Mutex
88
89	// lastCheck is last time when we checked the cache for stale entries.
90	var lastCheck time.Time
91
92	// checkInterval is the interval between each check for stale entries.
93	const checkInterval = 500 * time.Millisecond
94
95	// maxInactive is the amount of time after which an entry is stale.
96	const maxInactive = 3 * time.Second
97
98	// TCPConnToFile maps \|tc\| to the corresponding *os.File. Note that the
99	// returned *os.File is a dup() of the original, hence you now have ownership
100	// of two objects that you need to remember to defer Close() of.
101	func TCPConnToFile(tc net.TCPConn) (os.File, error) {	1✔
102	// Implementation note: according to a 2013 message on golang-nuts [1], the	1✔
103	// code that follows is broken on Unix because calling File() makes the socket	1✔
104	// blocking so causing Go to use more threads and, additionally, "timer wheel	1✔
105	// inside net package never fires". However, an April, 19 2018 commit	1✔
106	// on src/net/tcpsock.go apparently has removed such restriction and so now	1✔
107	// (i.e. since go1.11beta1) it's safe to use the code below [2, 3].	1✔
108	//	1✔
109	// [1] https://grokbase.com/t/gg/golang-nuts/1349whs82r	1✔
110	//	1✔
111	// [2] https://github.com/golang/go/commit/60e3ebb9cba	1✔
112	//	1✔
113	// [3] https://github.com/golang/go/issues/24942	1✔
114	//	1✔
115	// For this reason, this code only works correctly with go >= 1.11.	1✔
116	return tc.File()	1✔
117	}	1✔
118
119	// OwnFile transfers ownership of \|fp\| to the fdcache module. Passing a nil
120	// \|fp\| to this function is a programming error that will cause a panic.
121	func OwnFile(conn net.Conn, fp *os.File) {	1✔
122	if fp == nil {	1✔
123	panic("You passed me a nil *os.File")	×
124	}
125	curTime := time.Now()	1✔
126	key := makekey(conn)	1✔
127	mutex.Lock()	1✔
128	defer mutex.Unlock()	1✔
129	if curTime.Sub(lastCheck) > checkInterval {	2✔
130	lastCheck = curTime	1✔
131	// Note: in Golang it's safe to remove elements from the map while	1✔
132	// iterating it. See <https://github.com/golang/go/issues/9926>.	1✔
133	for key, entry := range cache {	2✔
134	if curTime.Sub(entry.Stamp) > maxInactive {	2✔
135	entry.Fp.Close()	1✔
136	delete(cache, key)	1✔
137	}	1✔
138	}
139	}
140	cache[key] = entry{	1✔
141	Fp: fp, // This takes ownership of fp	1✔
142	Stamp: curTime,	1✔
143	}	1✔
144	}
145
146	// GetAndForgetFile returns the *os.File bound to \|conn\| that was previously
147	// saved with EnableAndRememberFile, or nil if no file was found. Note that you
148	// are given ownership of the returned file pointer. As the name implies, the
149	// *os.File is removed from the cache by this operation.
150	func GetAndForgetFile(conn net.Conn) *os.File {	1✔
151	key := makekey(conn)	1✔
152	mutex.Lock()	1✔
153	defer mutex.Unlock()	1✔
154	entry, found := cache[key]	1✔
155	if !found {	1✔
156	return nil	×
157	}	×
158	delete(cache, key)	1✔
159	return entry.Fp // Pass ownership to caller	1✔
160	}
161
162	// GetUUID returns the UUID for a passed-in connection.
163	func GetUUID(conn net.Conn) (string, error) {	1✔
164	key := makekey(conn)	1✔
165	mutex.Lock()	1✔
166	defer mutex.Unlock()	1✔
167	entry, found := cache[key]	1✔
168	if !found {	1✔
169	return "", errors.New("fd not found")	×
170	}	×
171	id, err := uuid.FromFile(entry.Fp)	1✔
172	return id, err	1✔
173	}

m-lab / ndt-server / 457

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous