From 7473c93668e42cfc13c89ab660b6ea262ebf9bf4 Mon Sep 17 00:00:00 2001 From: subtly Date: Wed, 13 May 2015 19:03:00 +0200 Subject: [PATCH 1/3] 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(-) diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index 1213c12c82..ab3559ad82 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 { From a32693770c607a17eab6d8d068e4c9b1cddbbd54 Mon Sep 17 00:00:00 2001 From: subtly Date: Wed, 13 May 2015 20:03:17 +0200 Subject: [PATCH 2/3] 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(-) diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index ab3559ad82..2b215b45ce 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 f175835a8b..ae9e412512 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 { From 8eef2b765a035b2bb1dd59c3630649ca371152ff Mon Sep 17 00:00:00 2001 From: subtly Date: Wed, 13 May 2015 20:15:01 +0200 Subject: [PATCH 3/3] fix test. --- p2p/discover/udp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/discover/udp_test.go b/p2p/discover/udp_test.go index ae9e412512..7bf6df5abc 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]) } }