From 7473c93668e42cfc13c89ab660b6ea262ebf9bf4 Mon Sep 17 00:00:00 2001 From: subtly Date: Wed, 13 May 2015 19:03:00 +0200 Subject: UDP Interop. Limit datagrams to 1280bytes. We don't have a UDP which specifies any messages that will be 4KB. Aside from being implemented for months and a necessity for encryption and piggy-backing packets, 1280bytes is ideal, and, means this TODO can be completed! Why 1280 bytes? * It's less than the default MTU for most WAN/LAN networks. That means fewer fragmented datagrams (esp on well-connected networks). * Fragmented datagrams and dropped packets suck and add latency while OS waits for a dropped fragment to never arrive (blocking readLoop()) * Most of our packets are < 1280 bytes. * 1280 bytes is minimum datagram size and MTU for IPv6 -- on IPv6, a datagram < 1280bytes will *never* be fragmented. UDP datagrams are dropped. A lot! And fragmented datagrams are worse. If a datagram has a 30% chance of being dropped, then a fragmented datagram has a 60% chance of being dropped. More importantly, we have signed packets and can't do anything with a packet unless we receive the entire datagram because the signature can't be verified. The same is true when we have encrypted packets. So the solution here to picking an ideal buffer size for receiving datagrams is a number under 1400bytes. And the lower-bound value for IPv6 of 1280 bytes make's it a non-decision. On IPv4 most ISPs and 3g/4g/let networks have an MTU just over 1400 -- and *never* over 1500. Never -- that means packets over 1500 (in reality: ~1450) bytes are fragmented. And probably dropped a lot. Just to prove the point, here are pings sending non-fragmented packets over wifi/ISP, and a second set of pings via cell-phone tethering. It's important to note that, if *any* router between my system and the EC2 node has a lower MTU, the message would not go through: On wifi w/normal ISP: localhost:Debug $ ping -D -s 1450 52.6.250.242 PING 52.6.250.242 (52.6.250.242): 1450 data bytes 1458 bytes from 52.6.250.242: icmp_seq=0 ttl=42 time=104.831 ms 1458 bytes from 52.6.250.242: icmp_seq=1 ttl=42 time=119.004 ms ^C --- 52.6.250.242 ping statistics --- 2 packets transmitted, 2 packets received, 0.0% packet loss round-trip min/avg/max/stddev = 104.831/111.918/119.004/7.087 ms localhost:Debug $ ping -D -s 1480 52.6.250.242 PING 52.6.250.242 (52.6.250.242): 1480 data bytes ping: sendto: Message too long ping: sendto: Message too long Request timeout for icmp_seq 0 ping: sendto: Message too long Request timeout for icmp_seq 1 Tethering to O2: localhost:Debug $ ping -D -s 1480 52.6.250.242 PING 52.6.250.242 (52.6.250.242): 1480 data bytes ping: sendto: Message too long ping: sendto: Message too long Request timeout for icmp_seq 0 ^C --- 52.6.250.242 ping statistics --- 2 packets transmitted, 0 packets received, 100.0% packet loss localhost:Debug $ ping -D -s 1450 52.6.250.242 PING 52.6.250.242 (52.6.250.242): 1450 data bytes 1458 bytes from 52.6.250.242: icmp_seq=0 ttl=42 time=107.844 ms 1458 bytes from 52.6.250.242: icmp_seq=1 ttl=42 time=105.127 ms 1458 bytes from 52.6.250.242: icmp_seq=2 ttl=42 time=120.483 ms 1458 bytes from 52.6.250.242: icmp_seq=3 ttl=42 time=102.136 ms --- p2p/discover/udp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'p2p') diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index 1213c12c8..ab3559ad8 100644 --- a/p2p/discover/udp.go +++ b/p2p/discover/udp.go @@ -402,7 +402,7 @@ func encodePacket(priv *ecdsa.PrivateKey, ptype byte, req interface{}) ([]byte, // readLoop runs in its own goroutine. it handles incoming UDP packets. func (t *udp) readLoop() { defer t.conn.Close() - buf := make([]byte, 4096) // TODO: good buffer size + buf := make([]byte, 1280) for { nbytes, from, err := t.conn.ReadFromUDP(buf) if err != nil { -- cgit v1.2.3 From a32693770c607a17eab6d8d068e4c9b1cddbbd54 Mon Sep 17 00:00:00 2001 From: subtly Date: Wed, 13 May 2015 20:03:17 +0200 Subject: Manual send of multiple neighbours packets. Test receiving multiple neighbours packets. --- p2p/discover/udp.go | 8 +++++++- p2p/discover/udp_test.go | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) (limited to 'p2p') diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index ab3559ad8..2b215b45c 100644 --- a/p2p/discover/udp.go +++ b/p2p/discover/udp.go @@ -510,9 +510,15 @@ func (req *findnode) handle(t *udp, from *net.UDPAddr, fromID NodeID, mac []byte closestrpc[i] = nodeToRPC(n) } t.send(from, neighborsPacket, neighbors{ - Nodes: closestrpc, + Nodes: closestrpc[:13], Expiration: uint64(time.Now().Add(expiration).Unix()), }) + if len(closestrpc) > 13 { + t.send(from, neighborsPacket, neighbors{ + Nodes: closestrpc[13:], + Expiration: uint64(time.Now().Add(expiration).Unix()), + }) + } return nil } diff --git a/p2p/discover/udp_test.go b/p2p/discover/udp_test.go index f175835a8..ae9e41251 100644 --- a/p2p/discover/udp_test.go +++ b/p2p/discover/udp_test.go @@ -163,9 +163,19 @@ func TestUDP_findnode(t *testing.T) { )) // check that closest neighbors are returned. test.packetIn(nil, findnodePacket, &findnode{Target: testTarget, Expiration: futureExp}) + expected := test.table.closest(targetHash, bucketSize) test.waitPacketOut(func(p *neighbors) { - expected := test.table.closest(targetHash, bucketSize) - if len(p.Nodes) != bucketSize { + if len(p.Nodes) != 13 { + t.Errorf("wrong number of results: got %d, want %d", len(p.Nodes), bucketSize) + } + for i := range p.Nodes { + if p.Nodes[i].ID != expected.entries[i].ID { + t.Errorf("result mismatch at %d:\n got: %v\n want: %v", i, p.Nodes[i], expected.entries[i]) + } + } + }) + test.waitPacketOut(func(p *neighbors) { + if len(p.Nodes) != 3 { t.Errorf("wrong number of results: got %d, want %d", len(p.Nodes), bucketSize) } for i := range p.Nodes { -- cgit v1.2.3 From 8eef2b765a035b2bb1dd59c3630649ca371152ff Mon Sep 17 00:00:00 2001 From: subtly Date: Wed, 13 May 2015 20:15:01 +0200 Subject: fix test. --- p2p/discover/udp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'p2p') diff --git a/p2p/discover/udp_test.go b/p2p/discover/udp_test.go index ae9e41251..7bf6df5ab 100644 --- a/p2p/discover/udp_test.go +++ b/p2p/discover/udp_test.go @@ -179,7 +179,7 @@ func TestUDP_findnode(t *testing.T) { t.Errorf("wrong number of results: got %d, want %d", len(p.Nodes), bucketSize) } for i := range p.Nodes { - if p.Nodes[i].ID != expected.entries[i].ID { + if p.Nodes[i].ID != expected.entries[i + 13].ID { t.Errorf("result mismatch at %d:\n got: %v\n want: %v", i, p.Nodes[i], expected.entries[i]) } } -- cgit v1.2.3 From 251846d65a87ed1c4a880e25622b09284eff873d Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 13 May 2015 21:29:32 +0200 Subject: p2p/discover: fix out-of-bounds slicing for chunked neighbors packets The code assumed that Table.closest always returns at least 13 nodes. This is not true for small tables (e.g. during bootstrap). --- p2p/discover/udp.go | 50 ++++++++++++++++++++++++++++++++++-------------- p2p/discover/udp_test.go | 31 +++++++++++++----------------- 2 files changed, 49 insertions(+), 32 deletions(-) (limited to 'p2p') diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index 2b215b45c..539ccd460 100644 --- a/p2p/discover/udp.go +++ b/p2p/discover/udp.go @@ -363,7 +363,31 @@ const ( headSize = macSize + sigSize // space of packet frame data ) -var headSpace = make([]byte, headSize) +var ( + headSpace = make([]byte, headSize) + + // Neighbors responses are sent across multiple packets to + // stay below the 1280 byte limit. We compute the maximum number + // of entries by stuffing a packet until it grows too large. + maxNeighbors int +) + +func init() { + p := neighbors{Expiration: ^uint64(0)} + maxSizeNode := rpcNode{IP: make(net.IP, 16), UDP: ^uint16(0), TCP: ^uint16(0)} + for n := 0; ; n++ { + p.Nodes = append(p.Nodes, maxSizeNode) + size, _, err := rlp.EncodeToReader(p) + if err != nil { + // If this ever happens, it will be caught by the unit tests. + panic("cannot encode: " + err.Error()) + } + if headSize+size+1 >= 1280 { + maxNeighbors = n + break + } + } +} func (t *udp) send(toaddr *net.UDPAddr, ptype byte, req interface{}) error { packet, err := encodePacket(t.priv, ptype, req) @@ -402,6 +426,9 @@ func encodePacket(priv *ecdsa.PrivateKey, ptype byte, req interface{}) ([]byte, // readLoop runs in its own goroutine. it handles incoming UDP packets. func (t *udp) readLoop() { defer t.conn.Close() + // Discovery packets are defined to be no larger than 1280 bytes. + // Packets larger than this size will be cut at the end and treated + // as invalid because their hash won't match. buf := make([]byte, 1280) for { nbytes, from, err := t.conn.ReadFromUDP(buf) @@ -504,20 +531,15 @@ func (req *findnode) handle(t *udp, from *net.UDPAddr, fromID NodeID, mac []byte closest := t.closest(target, bucketSize).entries t.mutex.Unlock() - // TODO: this conversion could use a cached version of the slice - closestrpc := make([]rpcNode, len(closest)) + p := neighbors{Expiration: uint64(time.Now().Add(expiration).Unix())} + // Send neighbors in chunks with at most maxNeighbors per packet + // to stay below the 1280 byte limit. for i, n := range closest { - closestrpc[i] = nodeToRPC(n) - } - t.send(from, neighborsPacket, neighbors{ - Nodes: closestrpc[:13], - Expiration: uint64(time.Now().Add(expiration).Unix()), - }) - if len(closestrpc) > 13 { - t.send(from, neighborsPacket, neighbors{ - Nodes: closestrpc[13:], - Expiration: uint64(time.Now().Add(expiration).Unix()), - }) + p.Nodes = append(p.Nodes, nodeToRPC(n)) + if len(p.Nodes) == maxNeighbors || i == len(closest)-1 { + t.send(from, neighborsPacket, p) + p.Nodes = p.Nodes[:0] + } } return nil } diff --git a/p2p/discover/udp_test.go b/p2p/discover/udp_test.go index 7bf6df5ab..11fa31d7c 100644 --- a/p2p/discover/udp_test.go +++ b/p2p/discover/udp_test.go @@ -164,26 +164,21 @@ func TestUDP_findnode(t *testing.T) { // check that closest neighbors are returned. test.packetIn(nil, findnodePacket, &findnode{Target: testTarget, Expiration: futureExp}) expected := test.table.closest(targetHash, bucketSize) - test.waitPacketOut(func(p *neighbors) { - if len(p.Nodes) != 13 { - t.Errorf("wrong number of results: got %d, want %d", len(p.Nodes), bucketSize) - } - for i := range p.Nodes { - if p.Nodes[i].ID != expected.entries[i].ID { - t.Errorf("result mismatch at %d:\n got: %v\n want: %v", i, p.Nodes[i], expected.entries[i]) + + waitNeighbors := func(want []*Node) { + test.waitPacketOut(func(p *neighbors) { + if len(p.Nodes) != len(want) { + t.Errorf("wrong number of results: got %d, want %d", len(p.Nodes), bucketSize) } - } - }) - test.waitPacketOut(func(p *neighbors) { - if len(p.Nodes) != 3 { - t.Errorf("wrong number of results: got %d, want %d", len(p.Nodes), bucketSize) - } - for i := range p.Nodes { - if p.Nodes[i].ID != expected.entries[i + 13].ID { - t.Errorf("result mismatch at %d:\n got: %v\n want: %v", i, p.Nodes[i], expected.entries[i]) + for i := range p.Nodes { + if p.Nodes[i].ID != want[i].ID { + t.Errorf("result mismatch at %d:\n got: %v\n want: %v", i, p.Nodes[i], expected.entries[i]) + } } - } - }) + }) + } + waitNeighbors(expected.entries[:maxNeighbors]) + waitNeighbors(expected.entries[maxNeighbors:]) } func TestUDP_findnodeMultiReply(t *testing.T) { -- cgit v1.2.3 From 7efeb4bd9647b6ab5376e171c1ff7cb516da5698 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 14 May 2015 01:43:30 +0200 Subject: p2p: bump maxAcceptConns and defaultDialTimout On the test network, we've seen that it becomes harder to connect if the queues are so short. --- p2p/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'p2p') diff --git a/p2p/server.go b/p2p/server.go index 3c6fb5893..8f768bdff 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -18,12 +18,12 @@ import ( ) const ( - defaultDialTimeout = 10 * time.Second + defaultDialTimeout = 15 * time.Second refreshPeersInterval = 30 * time.Second staticPeerCheckInterval = 15 * time.Second // Maximum number of concurrently handshaking inbound connections. - maxAcceptConns = 10 + maxAcceptConns = 50 // Maximum number of concurrently dialing outbound connections. maxDialingConns = 10 -- cgit v1.2.3 From 983f5a717a35a604847ae4a4964c19000dc44016 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 13 May 2015 16:04:06 +0200 Subject: p2p/nat: fix concurrent access to autodisc Interface Concurrent calls to Interface methods on autodisc could return a "not discovered" error if the discovery did not finish before the call. autodisc.wait expected the done channel to carry the found Interface but it was closed instead. The fix is to use sync.Once for now, which is easier to get right. And there is a test. Finally. This will have to change again when we introduce re-discovery. --- p2p/nat/nat.go | 32 +++++++++++++++----------------- p2p/nat/nat_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 17 deletions(-) create mode 100644 p2p/nat/nat_test.go (limited to 'p2p') diff --git a/p2p/nat/nat.go b/p2p/nat/nat.go index fe00bdab0..58d50976d 100644 --- a/p2p/nat/nat.go +++ b/p2p/nat/nat.go @@ -172,8 +172,9 @@ func PMP(gateway net.IP) Interface { // This type is useful because discovery can take a while but we // want return an Interface value from UPnP, PMP and Auto immediately. type autodisc struct { - what string - done <-chan Interface + what string // type of interface being autodiscovered + once sync.Once + doit func() Interface mu sync.Mutex found Interface @@ -181,9 +182,10 @@ type autodisc struct { func startautodisc(what string, doit func() Interface) Interface { // TODO: monitor network configuration and rerun doit when it changes. - done := make(chan Interface) - ad := &autodisc{what: what, done: done} - go func() { done <- doit(); close(done) }() + ad := &autodisc{what: what, doit: doit} + // Start the auto discovery as early as possible so it is already + // in progress when the rest of the stack calls the methods. + go ad.wait() return ad } @@ -218,19 +220,15 @@ func (n *autodisc) String() string { } } +// wait blocks until auto-discovery has been performed. func (n *autodisc) wait() error { - n.mu.Lock() - found := n.found - n.mu.Unlock() - if found != nil { - // already discovered - return nil - } - if found = <-n.done; found == nil { - return errors.New("no UPnP or NAT-PMP router discovered") + n.once.Do(func() { + n.mu.Lock() + n.found = n.doit() + n.mu.Unlock() + }) + if n.found == nil { + return fmt.Errorf("no %s router discovered", n.what) } - n.mu.Lock() - n.found = found - n.mu.Unlock() return nil } diff --git a/p2p/nat/nat_test.go b/p2p/nat/nat_test.go new file mode 100644 index 000000000..95c50522e --- /dev/null +++ b/p2p/nat/nat_test.go @@ -0,0 +1,48 @@ +package nat + +import ( + "bytes" + "net" + "testing" + "time" +) + +// This test checks that autodisc doesn't hang and returns +// consistent results when multiple goroutines call its methods +// concurrently. +func TestAutoDiscRace(t *testing.T) { + ad := startautodisc("thing", func() Interface { + time.Sleep(500 * time.Millisecond) + return extIP{33, 44, 55, 66} + }) + + // Spawn a few concurrent calls to ad.ExternalIP. + type rval struct { + ip net.IP + err error + } + results := make(chan rval, 50) + for i := 0; i < cap(results); i++ { + go func() { + ip, err := ad.ExternalIP() + results <- rval{ip, err} + }() + } + + // Check that they all return the correct result within the deadline. + deadline := time.After(550 * time.Millisecond) + for i := 0; i < cap(results); i++ { + select { + case <-deadline: + t.Fatal("deadline exceeded") + case rval := <-results: + if rval.err != nil { + t.Errorf("result %d: unexpected error: %v", i, rval.err) + } + wantIP := net.IP{33, 44, 55, 66} + if !bytes.Equal(rval.ip, wantIP) { + t.Errorf("result %d: got IP %v, want %v", i, rval.ip, wantIP) + } + } + } +} -- cgit v1.2.3 From 663d4e0aff4cad98a9c5b17de18b2385a5874f6c Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 13 May 2015 16:12:46 +0200 Subject: p2p/nat: add test for UPnP auto discovery via SSDP The test listens for multicast UDP packets on the default interface because I couldn't get it to work reliably on loopback without massive changes to goupnp. This means that the test might fail when there is a UPnP-enabled router attached on that interface. I checked that locally by looping the test and it passes reliably because the local SSDP server always responds faster. --- p2p/nat/natupnp_test.go | 223 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 p2p/nat/natupnp_test.go (limited to 'p2p') diff --git a/p2p/nat/natupnp_test.go b/p2p/nat/natupnp_test.go new file mode 100644 index 000000000..074e97c81 --- /dev/null +++ b/p2p/nat/natupnp_test.go @@ -0,0 +1,223 @@ +package nat + +import ( + "fmt" + "io" + "net" + "net/http" + "strings" + "testing" + + "github.com/huin/goupnp/httpu" +) + +func TestUPNP_DDWRT(t *testing.T) { + dev := &fakeIGD{ + t: t, + ssdpResp: "HTTP/1.1 200 OK\r\n" + + "Cache-Control: max-age=300\r\n" + + "Date: Sun, 10 May 2015 10:05:33 GMT\r\n" + + "Ext: \r\n" + + "Location: http://{{listenAddr}}/InternetGatewayDevice.xml\r\n" + + "Server: POSIX UPnP/1.0 DD-WRT Linux/V24\r\n" + + "ST: urn:schemas-upnp-org:device:WANConnectionDevice:1\r\n" + + "USN: uuid:CB2471CC-CF2E-9795-8D9C-E87B34C16800::urn:schemas-upnp-org:device:WANConnectionDevice:1\r\n" + + "\r\n", + httpResps: map[string]string{ + "GET /InternetGatewayDevice.xml": ` + + + + 1 + 0 + + + urn:schemas-upnp-org:device:InternetGatewayDevice:1 + DD-WRT + http://www.dd-wrt.com + Gateway + Asus RT-N16:DD-WRT + Asus RT-N16 + V24 + 0000001 + http://www.dd-wrt.com + uuid:A13AB4C3-3A14-E386-DE6A-EFEA923A06FE + + + urn:schemas-upnp-org:service:Layer3Forwarding:1 + urn:upnp-org:serviceId:L3Forwarding1 + /x_layer3forwarding.xml + /control?Layer3Forwarding + /event?Layer3Forwarding + + + + + urn:schemas-upnp-org:device:WANDevice:1 + WANDevice + DD-WRT + http://www.dd-wrt.com + Gateway + router + http://www.dd-wrt.com + uuid:48FD569B-F9A9-96AE-4EE6-EB403D3DB91A + + + urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1 + urn:upnp-org:serviceId:WANCommonIFC1 + /x_wancommoninterfaceconfig.xml + /control?WANCommonInterfaceConfig + /event?WANCommonInterfaceConfig + + + + + urn:schemas-upnp-org:device:WANConnectionDevice:1 + WAN Connection Device + DD-WRT + http://www.dd-wrt.com + Gateway + router + http://www.dd-wrt.com + uuid:CB2471CC-CF2E-9795-8D9C-E87B34C16800 + + + urn:schemas-upnp-org:service:WANIPConnection:1 + urn:upnp-org:serviceId:WANIPConn1 + /x_wanipconnection.xml + /control?WANIPConnection + /event?WANIPConnection + + + + + + + urn:schemas-upnp-org:device:LANDevice:1 + LANDevice + DD-WRT + http://www.dd-wrt.com + Gateway + router + http://www.dd-wrt.com + uuid:04021998-3B35-2BDB-7B3C-99DA4435DA09 + + + urn:schemas-upnp-org:service:LANHostConfigManagement:1 + urn:upnp-org:serviceId:LANHostCfg1 + /x_lanhostconfigmanagement.xml + /control?LANHostConfigManagement + /event?LANHostConfigManagement + + + + + http://{{listenAddr}} + + + `, + // The response to our GetNATRSIPStatus call. This + // particular implementation has a bug where the elements + // inside u:GetNATRSIPStatusResponse are not properly + // namespaced. + "POST /control?WANIPConnection": ` + + + + 0 + 1 + + + + `, + }, + } + if err := dev.listen(); err != nil { + t.Skipf("cannot listen: %v", err) + } + dev.serve() + defer dev.close() + + // Attempt to discover the fake device. + discovered := discoverUPnP() + if discovered == nil { + t.Fatalf("not discovered") + } + upnp, _ := discovered.(*upnp) + if upnp.service != "IGDv1-IP1" { + t.Errorf("upnp.service mismatch: got %q, want %q", upnp.service, "IGDv1-IP1") + } + wantURL := "http://" + dev.listener.Addr().String() + "/InternetGatewayDevice.xml" + if upnp.dev.URLBaseStr != wantURL { + t.Errorf("upnp.dev.URLBaseStr mismatch: got %q, want %q", upnp.dev.URLBaseStr, wantURL) + } +} + +// fakeIGD presents itself as a discoverable UPnP device which sends +// canned responses to HTTPU and HTTP requests. +type fakeIGD struct { + t *testing.T // for logging + + listener net.Listener + mcastListener *net.UDPConn + + // This should be a complete HTTP response (including headers). + // It is sent as the response to any sspd packet. Any occurrence + // of "{{listenAddr}}" is replaced with the actual TCP listen + // address of the HTTP server. + ssdpResp string + // This one should contain XML payloads for all requests + // performed. The keys contain method and path, e.g. "GET /foo/bar". + // As with ssdpResp, "{{listenAddr}}" is replaced with the TCP + // listen address. + httpResps map[string]string +} + +// httpu.Handler +func (dev *fakeIGD) ServeMessage(r *http.Request) { + dev.t.Logf(`HTTPU request %s %s`, r.Method, r.RequestURI) + conn, err := net.Dial("udp4", r.RemoteAddr) + if err != nil { + fmt.Printf("reply Dial error: %v", err) + return + } + defer conn.Close() + io.WriteString(conn, dev.replaceListenAddr(dev.ssdpResp)) +} + +// http.Handler +func (dev *fakeIGD) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if resp, ok := dev.httpResps[r.Method+" "+r.RequestURI]; ok { + dev.t.Logf(`HTTP request "%s %s" --> %d`, r.Method, r.RequestURI, 200) + io.WriteString(w, dev.replaceListenAddr(resp)) + } else { + dev.t.Logf(`HTTP request "%s %s" --> %d`, r.Method, r.RequestURI, 404) + w.WriteHeader(http.StatusNotFound) + } +} + +func (dev *fakeIGD) replaceListenAddr(resp string) string { + return strings.Replace(resp, "{{listenAddr}}", dev.listener.Addr().String(), -1) +} + +func (dev *fakeIGD) listen() (err error) { + if dev.listener, err = net.Listen("tcp", "127.0.0.1:0"); err != nil { + return err + } + laddr := &net.UDPAddr{IP: net.ParseIP("239.255.255.250"), Port: 1900} + if dev.mcastListener, err = net.ListenMulticastUDP("udp", nil, laddr); err != nil { + dev.listener.Close() + return err + } + return nil +} + +func (dev *fakeIGD) serve() { + go httpu.Serve(dev.mcastListener, dev) + go http.Serve(dev.listener, dev) +} + +func (dev *fakeIGD) close() { + dev.mcastListener.Close() + dev.listener.Close() +} -- cgit v1.2.3 From c14de2e9733685e60f165ee5f2dd1f7c17f3f6cd Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 14 May 2015 12:54:59 +0200 Subject: p2p/nat: tweak port mapping log messages and levels People stil get confused about the messages. This commit changes the levels so that the only thing printed at the default level (info) is a successful mapping. --- p2p/nat/nat.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'p2p') diff --git a/p2p/nat/nat.go b/p2p/nat/nat.go index 58d50976d..9acb34398 100644 --- a/p2p/nat/nat.go +++ b/p2p/nat/nat.go @@ -86,13 +86,13 @@ func Map(m Interface, c chan struct{}, protocol string, extport, intport int, na refresh := time.NewTimer(mapUpdateInterval) defer func() { refresh.Stop() - glog.V(logger.Debug).Infof("Deleting port mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m) + glog.V(logger.Debug).Infof("deleting port mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m) m.DeleteMapping(protocol, extport, intport) }() - glog.V(logger.Debug).Infof("add mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m) if err := m.AddMapping(protocol, intport, extport, name, mapTimeout); err != nil { - glog.V(logger.Warn).Infof("network port %d could not be mapped: %v\n", intport, err) - glog.V(logger.Debug).Infof("mapping with %v returned %v\n", m, err) + glog.V(logger.Debug).Infof("network port %s:%d could not be mapped: %v\n", protocol, intport, err) + } else { + glog.V(logger.Info).Infof("mapped network port %s:%d -> %d (%s) using %s\n", protocol, extport, intport, name, m) } for { select { @@ -101,10 +101,9 @@ func Map(m Interface, c chan struct{}, protocol string, extport, intport int, na return } case <-refresh.C: - glog.V(logger.Detail).Infof("refresh mapping: %s %d -> %d (%s) using %s\n", protocol, extport, intport, name, m) + glog.V(logger.Detail).Infof("refresh port mapping %s:%d -> %d (%s) using %s\n", protocol, extport, intport, name, m) if err := m.AddMapping(protocol, intport, extport, name, mapTimeout); err != nil { - glog.V(logger.Warn).Infof("network port %d could not be mapped: %v\n", intport, err) - glog.V(logger.Debug).Infof("mapping with %v returned %v\n", m, err) + glog.V(logger.Debug).Infof("network port %s:%d could not be mapped: %v\n", protocol, intport, err) } refresh.Reset(mapUpdateInterval) } -- cgit v1.2.3 From 691cb90284b9b63dc9c80bf0716ba35036ca78fe Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 14 May 2015 03:04:04 +0200 Subject: p2p: log remote reason when disconnect is requested The returned reason is currently not used except for the log message. This change makes the log messages a bit more useful. The handshake code also returns the remote reason. --- p2p/peer.go | 5 +++-- p2p/peer_test.go | 13 +++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'p2p') diff --git a/p2p/peer.go b/p2p/peer.go index ac691f2ce..c7ec08887 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -131,10 +131,11 @@ func (p *Peer) run() DiscReason { case err := <-p.protoErr: reason = discReasonForError(err) case reason = <-p.disc: + p.politeDisconnect(reason) + reason = DiscRequested } close(p.closed) - p.politeDisconnect(reason) p.wg.Wait() glog.V(logger.Debug).Infof("%v: Disconnected: %v\n", p, reason) return reason @@ -191,7 +192,7 @@ func (p *Peer) handle(msg Msg) error { // check errors because, the connection will be closed after it. rlp.Decode(msg.Payload, &reason) glog.V(logger.Debug).Infof("%v: Disconnect Requested: %v\n", p, reason[0]) - return DiscRequested + return reason[0] case msg.Code < baseProtocolLength: // ignore other base protocol messages return msg.Discard() diff --git a/p2p/peer_test.go b/p2p/peer_test.go index fb76818a0..7d17d447c 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -172,12 +172,13 @@ func TestPeerDisconnect(t *testing.T) { if err := SendItems(rw, discMsg, DiscQuitting); err != nil { t.Fatal(err) } - if err := ExpectMsg(rw, discMsg, []interface{}{DiscRequested}); err != nil { - t.Error(err) - } - closer() - if reason := <-disc; reason != DiscRequested { - t.Errorf("run returned wrong reason: got %v, want %v", reason, DiscRequested) + select { + case reason := <-disc: + if reason != DiscQuitting { + t.Errorf("run returned wrong reason: got %v, want %v", reason, DiscRequested) + } + case <-time.After(500 * time.Millisecond): + t.Error("peer did not return") } } -- cgit v1.2.3 From 7fa2607bd12290d7ae84d74b9dcc59b1777f8d58 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 14 May 2015 01:49:39 +0200 Subject: p2p/discover: bump maxBondingPingPongs to 16 This should increase the speed a bit because all findnode results (up to 16) can be verified at the same time. --- p2p/discover/table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'p2p') diff --git a/p2p/discover/table.go b/p2p/discover/table.go index 2c9cb80d5..5e6dd8d0d 100644 --- a/p2p/discover/table.go +++ b/p2p/discover/table.go @@ -25,7 +25,7 @@ const ( hashBits = len(common.Hash{}) * 8 nBuckets = hashBits + 1 // Number of buckets - maxBondingPingPongs = 10 + maxBondingPingPongs = 16 ) type Table struct { -- cgit v1.2.3 From 206fe259718c015df43dd25b59a9dfd370428b53 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 14 May 2015 14:56:34 +0200 Subject: p2p: remove testlog --- p2p/peer_test.go | 11 ----------- p2p/server_test.go | 15 --------------- p2p/testlog_test.go | 25 ------------------------- 3 files changed, 51 deletions(-) delete mode 100644 p2p/testlog_test.go (limited to 'p2p') diff --git a/p2p/peer_test.go b/p2p/peer_test.go index 7d17d447c..59dcb7ba4 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -50,8 +50,6 @@ func testPeer(protos []Protocol) (func(), *conn, *Peer, <-chan DiscReason) { } func TestPeerProtoReadMsg(t *testing.T) { - defer testlog(t).detach() - done := make(chan struct{}) proto := Protocol{ Name: "a", @@ -88,8 +86,6 @@ func TestPeerProtoReadMsg(t *testing.T) { } func TestPeerProtoEncodeMsg(t *testing.T) { - defer testlog(t).detach() - proto := Protocol{ Name: "a", Length: 2, @@ -112,8 +108,6 @@ func TestPeerProtoEncodeMsg(t *testing.T) { } func TestPeerWriteForBroadcast(t *testing.T) { - defer testlog(t).detach() - closer, rw, peer, peerErr := testPeer([]Protocol{discard}) defer closer() @@ -152,8 +146,6 @@ func TestPeerWriteForBroadcast(t *testing.T) { } func TestPeerPing(t *testing.T) { - defer testlog(t).detach() - closer, rw, _, _ := testPeer(nil) defer closer() if err := SendItems(rw, pingMsg); err != nil { @@ -165,8 +157,6 @@ func TestPeerPing(t *testing.T) { } func TestPeerDisconnect(t *testing.T) { - defer testlog(t).detach() - closer, rw, _, disc := testPeer(nil) defer closer() if err := SendItems(rw, discMsg, DiscQuitting); err != nil { @@ -185,7 +175,6 @@ func TestPeerDisconnect(t *testing.T) { // This test is supposed to verify that Peer can reliably handle // multiple causes of disconnection occurring at the same time. func TestPeerDisconnectRace(t *testing.T) { - defer testlog(t).detach() maybe := func() bool { return rand.Intn(1) == 1 } for i := 0; i < 1000; i++ { diff --git a/p2p/server_test.go b/p2p/server_test.go index bf9df31ab..55fc81572 100644 --- a/p2p/server_test.go +++ b/p2p/server_test.go @@ -46,8 +46,6 @@ func startTestServer(t *testing.T, pf newPeerHook) *Server { } func TestServerListen(t *testing.T) { - defer testlog(t).detach() - // start the test server connected := make(chan *Peer) srv := startTestServer(t, func(p *Peer) { @@ -78,8 +76,6 @@ func TestServerListen(t *testing.T) { } func TestServerDial(t *testing.T) { - defer testlog(t).detach() - // run a one-shot TCP server to handle the connection. listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -126,8 +122,6 @@ func TestServerDial(t *testing.T) { } func TestServerBroadcast(t *testing.T) { - defer testlog(t).detach() - var connected sync.WaitGroup srv := startTestServer(t, func(p *Peer) { p.running = matchProtocols([]Protocol{discard}, []Cap{discard.cap()}, p.rw) @@ -172,8 +166,6 @@ func TestServerBroadcast(t *testing.T) { // // It also serves as a light-weight integration test. func TestServerDisconnectAtCap(t *testing.T) { - defer testlog(t).detach() - started := make(chan *Peer) srv := &Server{ ListenAddr: "127.0.0.1:0", @@ -224,8 +216,6 @@ func TestServerDisconnectAtCap(t *testing.T) { // Tests that static peers are (re)connected, and done so even above max peers. func TestServerStaticPeers(t *testing.T) { - defer testlog(t).detach() - // Create a test server with limited connection slots started := make(chan *Peer) server := &Server{ @@ -312,7 +302,6 @@ func TestServerStaticPeers(t *testing.T) { // Tests that trusted peers and can connect above max peer caps. func TestServerTrustedPeers(t *testing.T) { - defer testlog(t).detach() // Create a trusted peer to accept connections from key := newkey() @@ -374,8 +363,6 @@ func TestServerTrustedPeers(t *testing.T) { // Tests that a failed dial will temporarily throttle a peer. func TestServerMaxPendingDials(t *testing.T) { - defer testlog(t).detach() - // Start a simple test server server := &Server{ ListenAddr: "127.0.0.1:0", @@ -443,8 +430,6 @@ func TestServerMaxPendingDials(t *testing.T) { } func TestServerMaxPendingAccepts(t *testing.T) { - defer testlog(t).detach() - // Start a test server and a peer sink for synchronization started := make(chan *Peer) server := &Server{ diff --git a/p2p/testlog_test.go b/p2p/testlog_test.go deleted file mode 100644 index ac973bcf5..000000000 --- a/p2p/testlog_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package p2p - -import ( - "testing" - - "github.com/ethereum/go-ethereum/logger" -) - -type testLogger struct{ t *testing.T } - -func testlog(t *testing.T) testLogger { - logger.Reset() - l := testLogger{t} - logger.AddLogSystem(l) - return l -} - -func (l testLogger) LogPrint(msg logger.LogMsg) { - l.t.Logf("%s", msg.String()) -} - -func (testLogger) detach() { - logger.Flush() - logger.Reset() -} -- cgit v1.2.3 From d2f119cf9b30a7568b5ebe7c290c3be30dc0f2de Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 14 May 2015 15:01:13 +0200 Subject: p2p/discover: limit open files for node database --- p2p/discover/database.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'p2p') diff --git a/p2p/discover/database.go b/p2p/discover/database.go index dc0b97ddf..2b9da0423 100644 --- a/p2p/discover/database.go +++ b/p2p/discover/database.go @@ -17,6 +17,7 @@ import ( "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/errors" "github.com/syndtr/goleveldb/leveldb/iterator" + "github.com/syndtr/goleveldb/leveldb/opt" "github.com/syndtr/goleveldb/leveldb/storage" "github.com/syndtr/goleveldb/leveldb/util" ) @@ -72,8 +73,8 @@ func newMemoryNodeDB() (*nodeDB, error) { // newPersistentNodeDB creates/opens a leveldb backed persistent node database, // also flushing its contents in case of a version mismatch. func newPersistentNodeDB(path string, version int) (*nodeDB, error) { - // Try to open the cache, recovering any corruption - db, err := leveldb.OpenFile(path, nil) + opts := &opt.Options{OpenFilesCacheCapacity: 5} + db, err := leveldb.OpenFile(path, opts) if _, iscorrupted := err.(*errors.ErrCorrupted); iscorrupted { db, err = leveldb.RecoverFile(path, nil) } -- cgit v1.2.3