{"action":"created","comment":{"url":"https://api.github.com/repos/weaveworks/weave/pulls/comments/36639233","id":36639233,"diff_hunk":"@@ -230,138 +185,44 @@ func (router *Router) acceptTCP(tcpConn *net.TCPConn) {\n \tStartLocalConnection(connRemote, tcpConn, nil, router, true)\n }\n \n-func (router *Router) listenUDP(localPort int, po PacketSink) *net.UDPConn {\n-\tlocalAddr, err := net.ResolveUDPAddr(\"udp4\", fmt.Sprint(\":\", localPort))\n-\tcheckFatal(err)\n-\tconn, err := net.ListenUDP(\"udp4\", localAddr)\n-\tcheckFatal(err)\n-\tf, err := conn.File()\n-\tdefer f.Close()\n-\tcheckFatal(err)\n-\tfd := int(f.Fd())\n-\t// This one makes sure all packets we send out do not have DF set on them.\n-\terr = syscall.SetsockoptInt(fd, syscall.IPPROTO_IP, syscall.IP_MTU_DISCOVER, syscall.IP_PMTUDISC_DONT)\n-\tcheckFatal(err)\n-\tgo router.udpReader(conn, po)\n-\treturn conn\n-}\n-\n-func (router *Router) udpReader(conn *net.UDPConn, po PacketSink) {\n-\tdefer conn.Close()\n-\tdec := NewEthernetDecoder()\n-\tbuf := make([]byte, MaxUDPPacketSize)\n-\tfor {\n-\t\tn, sender, err := conn.ReadFromUDP(buf)\n-\t\tif err == io.EOF {\n-\t\t\treturn\n-\t\t} else if err != nil {\n-\t\t\tlog.Warnln(\"ignoring UDP read error\", err)\n-\t\t\tcontinue\n-\t\t} else if n < NameSize {\n-\t\t\tlog.Warnln(\"ignoring too short UDP packet from\", sender)\n-\t\t\tcontinue\n-\t\t}\n-\t\tname := PeerNameFromBin(buf[:NameSize])\n-\t\tpacket := make([]byte, n-NameSize)\n-\t\tcopy(packet, buf[NameSize:n])\n-\t\tpeerConn, found := router.Ourself.ConnectionTo(name)\n-\t\tif !found {\n-\t\t\tcontinue\n-\t\t}\n-\t\trelayConn, ok := peerConn.(*LocalConnection)\n-\t\tif !ok {\n-\t\t\tcontinue\n-\t\t}\n-\t\tif err := relayConn.Decryptor.IterateFrames(packet, router.handleUDPPacketFunc(relayConn, dec, sender, po)); err != nil {\n-\t\t\t// Errors during UDP packet decoding / processing are\n-\t\t\t// non-fatal. One common cause is that we receive and\n-\t\t\t// attempt to decrypt a \"stray\" packet. This can actually\n-\t\t\t// happen quite easily if there is some connection churn\n-\t\t\t// between two peers. After all, UDP isn't a\n-\t\t\t// connection-oriented protocol, yet we pretend it is.\n-\t\t\t//\n-\t\t\t// If anything really is seriously, unrecoverably amiss\n-\t\t\t// with a connection, that will typically result in missed\n-\t\t\t// heartbeats and the connection getting shut down because\n-\t\t\t// of that.\n-\t\t\trelayConn.Log(err)\n-\t\t}\n+func (router *Router) handleForwardedPacket(key ForwardPacketKey) FlowOp {\n+\tif key.DstPeer != router.Ourself.Peer {\n+\t\t// it's not for us, we're just relaying it\n+\t\trouter.PacketLogging.LogForwardPacket(\"Relaying\", key)\n+\t\treturn router.Ourself.Relay(key)\n \t}\n-}\n \n-func (router *Router) handleUDPPacketFunc(relayConn *LocalConnection, dec *EthernetDecoder, sender *net.UDPAddr, po PacketSink) FrameConsumer {\n-\treturn func(srcNameByte, dstNameByte []byte, frame []byte) {\n-\t\tsrcPeer := router.Peers.Fetch(PeerNameFromBin(srcNameByte))\n-\t\tdstPeer := router.Peers.Fetch(PeerNameFromBin(dstNameByte))\n-\t\tif srcPeer == nil || dstPeer == nil {\n-\t\t\treturn\n-\t\t}\n+\t// At this point, it's either unicast to us, or a broadcast\n+\t// (because the DstPeer on a forwarded broadcast packet is\n+\t// always set to the peer being forwarded to)\n \n-\t\tdec.DecodeLayers(frame)\n-\t\tdecodedLen := len(dec.decoded)\n-\t\tif decodedLen == 0 {\n-\t\t\treturn\n-\t\t}\n-\t\t// Handle special frames produced internally (rather than\n-\t\t// captured/forwarded) by the remote router.\n-\t\t//\n-\t\t// We really shouldn't be decoding these above, since they are\n-\t\t// not genuine Ethernet frames. However, it is actually more\n-\t\t// efficient to do so, as we want to optimise for the common\n-\t\t// (i.e. non-special) frames. These always need decoding, and\n-\t\t// detecting special frames is cheaper post decoding than pre.\n-\t\tif decodedLen == 1 && dec.IsSpecial() {\n-\t\t\tif srcPeer == relayConn.Remote() && dstPeer == router.Ourself.Peer {\n-\t\t\t\thandleSpecialFrame(relayConn, sender, frame)\n-\t\t\t}\n-\t\t\treturn\n-\t\t}\n-\n-\t\tif dstPeer != router.Ourself.Peer {\n-\t\t\t// it's not for us, we're just relaying it\n-\t\t\trouter.LogFrame(\"Relaying\", frame, dec)\n-\t\t\terr := router.Ourself.Relay(srcPeer, dstPeer, frame, dec)\n-\t\t\tif ftbe, ok := err.(FrameTooBigError); ok {\n-\t\t\t\terr = dec.sendICMPFragNeeded(ftbe.EPMTU, func(icmpFrame []byte) error {\n-\t\t\t\t\treturn router.Ourself.Forward(srcPeer, icmpFrame, nil)\n-\t\t\t\t})\n-\t\t\t}\n-\n-\t\t\tcheckWarn(err)\n-\t\t\treturn\n-\t\t}\n-\n-\t\tsrcMac := dec.Eth.SrcMAC\n-\t\tdstMac := dec.Eth.DstMAC\n-\n-\t\tif router.Macs.Enter(srcMac, srcPeer) {\n-\t\t\tlog.Println(\"Discovered remote MAC\", srcMac, \"at\", srcPeer)\n-\t\t}\n-\t\tif po != nil {\n-\t\t\trouter.LogFrame(\"Injecting\", frame, dec)\n-\t\t\tcheckWarn(po.WritePacket(frame))\n-\t\t}\n+\tsrcMac := net.HardwareAddr(key.SrcMAC[:])\n+\tdstMac := net.HardwareAddr(key.DstMAC[:])\n+\tif router.Macs.Enter(srcMac, key.SrcPeer) {","path":"router/router.go","position":334,"original_position":334,"commit_id":"f12ad3d8eea5039dbcd8ce6d03a76505c9df86ab","original_commit_id":"f12ad3d8eea5039dbcd8ce6d03a76505c9df86ab","user":{"login":"awh","id":1504438,"avatar_url":"https://avatars.githubusercontent.com/u/1504438?v=3","gravatar_id":"","url":"https://api.github.com/users/awh","html_url":"https://github.com/awh","followers_url":"https://api.github.com/users/awh/followers","following_url":"https://api.github.com/users/awh/following{/other_user}","gists_url":"https://api.github.com/users/awh/gists{/gist_id}","starred_url":"https://api.github.com/users/awh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/awh/subscriptions","organizations_url":"https://api.github.com/users/awh/orgs","repos_url":"https://api.github.com/users/awh/repos","events_url":"https://api.github.com/users/awh/events{/privacy}","received_events_url":"https://api.github.com/users/awh/received_events","type":"User","site_admin":false},"body":"Is there a reason not to record the mac/peer mapping for relayed packets too? The previous code has the same behaviour","created_at":"2015-08-10T14:49:35Z","updated_at":"2015-08-10T14:49:35Z","html_url":"https://github.com/weaveworks/weave/pull/991#discussion_r36639233","pull_request_url":"https://api.github.com/repos/weaveworks/weave/pulls/991","_links":{"self":{"href":"https://api.github.com/repos/weaveworks/weave/pulls/comments/36639233"},"html":{"href":"https://github.com/weaveworks/weave/pull/991#discussion_r36639233"},"pull_request":{"href":"https://api.github.com/repos/weaveworks/weave/pulls/991"}}},"pull_request":{"url":"https://api.github.com/repos/weaveworks/weave/pulls/991","id":38350988,"html_url":"https://github.com/weaveworks/weave/pull/991","diff_url":"https://github.com/weaveworks/weave/pull/991.diff","patch_url":"https://github.com/weaveworks/weave/pull/991.patch","issue_url":"https://api.github.com/repos/weaveworks/weave/issues/991","number":991,"state":"open","locked":false,"title":"Fast datapath preparations","user":{"login":"dpw","id":15074,"avatar_url":"https://avatars.githubusercontent.com/u/15074?v=3","gravatar_id":"","url":"https://api.github.com/users/dpw","html_url":"https://github.com/dpw","followers_url":"https://api.github.com/users/dpw/followers","following_url":"https://api.github.com/users/dpw/following{/other_user}","gists_url":"https://api.github.com/users/dpw/gists{/gist_id}","starred_url":"https://api.github.com/users/dpw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dpw/subscriptions","organizations_url":"https://api.github.com/users/dpw/orgs","repos_url":"https://api.github.com/users/dpw/repos","events_url":"https://api.github.com/users/dpw/events{/privacy}","received_events_url":"https://api.github.com/users/dpw/received_events","type":"User","site_admin":false},"body":"These are the changes to the existing router code to prepare the way for fast datapath.  I didn't include the short peer ID changes, as they are much more tailored to ODP specifics, and so they make more sense as part of the fast datapath PR.\r\n\r\nI think this is clean enough for a review pass.  There is still one point I'm not quite happy with yet:  The names \"IntraHost\" and \"InterHost\" were intended as stand-ins, until I came up with better names.  But I still haven't come up with replacements that seem obviously better (the best so far is PacketCapture and PacketTransport but they are longer, and PacketCapture isn't right because that interface includes sending, plus PacketCapture has too much affinity with pcap).  I'm hoping inspiration will strike by the time this is merge-ready, and welcome any suggestions.","created_at":"2015-06-23T15:41:47Z","updated_at":"2015-08-10T14:49:35Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f44fa11102d4b48bfa097f7909242ce8ea036677","assignee":{"login":"inercia","id":1841612,"avatar_url":"https://avatars.githubusercontent.com/u/1841612?v=3","gravatar_id":"","url":"https://api.github.com/users/inercia","html_url":"https://github.com/inercia","followers_url":"https://api.github.com/users/inercia/followers","following_url":"https://api.github.com/users/inercia/following{/other_user}","gists_url":"https://api.github.com/users/inercia/gists{/gist_id}","starred_url":"https://api.github.com/users/inercia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/inercia/subscriptions","organizations_url":"https://api.github.com/users/inercia/orgs","repos_url":"https://api.github.com/users/inercia/repos","events_url":"https://api.github.com/users/inercia/events{/privacy}","received_events_url":"https://api.github.com/users/inercia/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/weaveworks/weave/pulls/991/commits","review_comments_url":"https://api.github.com/repos/weaveworks/weave/pulls/991/comments","review_comment_url":"https://api.github.com/repos/weaveworks/weave/pulls/comments{/number}","comments_url":"https://api.github.com/repos/weaveworks/weave/issues/991/comments","statuses_url":"https://api.github.com/repos/weaveworks/weave/statuses/f12ad3d8eea5039dbcd8ce6d03a76505c9df86ab","head":{"label":"dpw:fdp-prep","ref":"fdp-prep","sha":"f12ad3d8eea5039dbcd8ce6d03a76505c9df86ab","user":{"login":"dpw","id":15074,"avatar_url":"https://avatars.githubusercontent.com/u/15074?v=3","gravatar_id":"","url":"https://api.github.com/users/dpw","html_url":"https://github.com/dpw","followers_url":"https://api.github.com/users/dpw/followers","following_url":"https://api.github.com/users/dpw/following{/other_user}","gists_url":"https://api.github.com/users/dpw/gists{/gist_id}","starred_url":"https://api.github.com/users/dpw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dpw/subscriptions","organizations_url":"https://api.github.com/users/dpw/orgs","repos_url":"https://api.github.com/users/dpw/repos","events_url":"https://api.github.com/users/dpw/events{/privacy}","received_events_url":"https://api.github.com/users/dpw/received_events","type":"User","site_admin":false},"repo":{"id":23701178,"name":"weave","full_name":"dpw/weave","owner":{"login":"dpw","id":15074,"avatar_url":"https://avatars.githubusercontent.com/u/15074?v=3","gravatar_id":"","url":"https://api.github.com/users/dpw","html_url":"https://github.com/dpw","followers_url":"https://api.github.com/users/dpw/followers","following_url":"https://api.github.com/users/dpw/following{/other_user}","gists_url":"https://api.github.com/users/dpw/gists{/gist_id}","starred_url":"https://api.github.com/users/dpw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dpw/subscriptions","organizations_url":"https://api.github.com/users/dpw/orgs","repos_url":"https://api.github.com/users/dpw/repos","events_url":"https://api.github.com/users/dpw/events{/privacy}","received_events_url":"https://api.github.com/users/dpw/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/dpw/weave","description":"The Docker Network","fork":true,"url":"https://api.github.com/repos/dpw/weave","forks_url":"https://api.github.com/repos/dpw/weave/forks","keys_url":"https://api.github.com/repos/dpw/weave/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dpw/weave/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dpw/weave/teams","hooks_url":"https://api.github.com/repos/dpw/weave/hooks","issue_events_url":"https://api.github.com/repos/dpw/weave/issues/events{/number}","events_url":"https://api.github.com/repos/dpw/weave/events","assignees_url":"https://api.github.com/repos/dpw/weave/assignees{/user}","branches_url":"https://api.github.com/repos/dpw/weave/branches{/branch}","tags_url":"https://api.github.com/repos/dpw/weave/tags","blobs_url":"https://api.github.com/repos/dpw/weave/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dpw/weave/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dpw/weave/git/refs{/sha}","trees_url":"https://api.github.com/repos/dpw/weave/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dpw/weave/statuses/{sha}","languages_url":"https://api.github.com/repos/dpw/weave/languages","stargazers_url":"https://api.github.com/repos/dpw/weave/stargazers","contributors_url":"https://api.github.com/repos/dpw/weave/contributors","subscribers_url":"https://api.github.com/repos/dpw/weave/subscribers","subscription_url":"https://api.github.com/repos/dpw/weave/subscription","commits_url":"https://api.github.com/repos/dpw/weave/commits{/sha}","git_commits_url":"https://api.github.com/repos/dpw/weave/git/commits{/sha}","comments_url":"https://api.github.com/repos/dpw/weave/comments{/number}","issue_comment_url":"https://api.github.com/repos/dpw/weave/issues/comments{/number}","contents_url":"https://api.github.com/repos/dpw/weave/contents/{+path}","compare_url":"https://api.github.com/repos/dpw/weave/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dpw/weave/merges","archive_url":"https://api.github.com/repos/dpw/weave/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dpw/weave/downloads","issues_url":"https://api.github.com/repos/dpw/weave/issues{/number}","pulls_url":"https://api.github.com/repos/dpw/weave/pulls{/number}","milestones_url":"https://api.github.com/repos/dpw/weave/milestones{/number}","notifications_url":"https://api.github.com/repos/dpw/weave/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dpw/weave/labels{/name}","releases_url":"https://api.github.com/repos/dpw/weave/releases{/id}","created_at":"2014-09-05T11:47:37Z","updated_at":"2015-04-16T13:03:57Z","pushed_at":"2015-08-03T16:20:04Z","git_url":"git://github.com/dpw/weave.git","ssh_url":"git@github.com:dpw/weave.git","clone_url":"https://github.com/dpw/weave.git","svn_url":"https://github.com/dpw/weave","homepage":"","size":4892,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"weaveworks:master","ref":"master","sha":"281bf02871e62275c50cd7b55e0edb58926b774d","user":{"login":"weaveworks","id":9976052,"avatar_url":"https://avatars.githubusercontent.com/u/9976052?v=3","gravatar_id":"","url":"https://api.github.com/users/weaveworks","html_url":"https://github.com/weaveworks","followers_url":"https://api.github.com/users/weaveworks/followers","following_url":"https://api.github.com/users/weaveworks/following{/other_user}","gists_url":"https://api.github.com/users/weaveworks/gists{/gist_id}","starred_url":"https://api.github.com/users/weaveworks/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/weaveworks/subscriptions","organizations_url":"https://api.github.com/users/weaveworks/orgs","repos_url":"https://api.github.com/users/weaveworks/repos","events_url":"https://api.github.com/users/weaveworks/events{/privacy}","received_events_url":"https://api.github.com/users/weaveworks/received_events","type":"Organization","site_admin":false},"repo":{"id":23059575,"name":"weave","full_name":"weaveworks/weave","owner":{"login":"weaveworks","id":9976052,"avatar_url":"https://avatars.githubusercontent.com/u/9976052?v=3","gravatar_id":"","url":"https://api.github.com/users/weaveworks","html_url":"https://github.com/weaveworks","followers_url":"https://api.github.com/users/weaveworks/followers","following_url":"https://api.github.com/users/weaveworks/following{/other_user}","gists_url":"https://api.github.com/users/weaveworks/gists{/gist_id}","starred_url":"https://api.github.com/users/weaveworks/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/weaveworks/subscriptions","organizations_url":"https://api.github.com/users/weaveworks/orgs","repos_url":"https://api.github.com/users/weaveworks/repos","events_url":"https://api.github.com/users/weaveworks/events{/privacy}","received_events_url":"https://api.github.com/users/weaveworks/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/weaveworks/weave","description":"Weaving Docker containers into applications","fork":false,"url":"https://api.github.com/repos/weaveworks/weave","forks_url":"https://api.github.com/repos/weaveworks/weave/forks","keys_url":"https://api.github.com/repos/weaveworks/weave/keys{/key_id}","collaborators_url":"https://api.github.com/repos/weaveworks/weave/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/weaveworks/weave/teams","hooks_url":"https://api.github.com/repos/weaveworks/weave/hooks","issue_events_url":"https://api.github.com/repos/weaveworks/weave/issues/events{/number}","events_url":"https://api.github.com/repos/weaveworks/weave/events","assignees_url":"https://api.github.com/repos/weaveworks/weave/assignees{/user}","branches_url":"https://api.github.com/repos/weaveworks/weave/branches{/branch}","tags_url":"https://api.github.com/repos/weaveworks/weave/tags","blobs_url":"https://api.github.com/repos/weaveworks/weave/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/weaveworks/weave/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/weaveworks/weave/git/refs{/sha}","trees_url":"https://api.github.com/repos/weaveworks/weave/git/trees{/sha}","statuses_url":"https://api.github.com/repos/weaveworks/weave/statuses/{sha}","languages_url":"https://api.github.com/repos/weaveworks/weave/languages","stargazers_url":"https://api.github.com/repos/weaveworks/weave/stargazers","contributors_url":"https://api.github.com/repos/weaveworks/weave/contributors","subscribers_url":"https://api.github.com/repos/weaveworks/weave/subscribers","subscription_url":"https://api.github.com/repos/weaveworks/weave/subscription","commits_url":"https://api.github.com/repos/weaveworks/weave/commits{/sha}","git_commits_url":"https://api.github.com/repos/weaveworks/weave/git/commits{/sha}","comments_url":"https://api.github.com/repos/weaveworks/weave/comments{/number}","issue_comment_url":"https://api.github.com/repos/weaveworks/weave/issues/comments{/number}","contents_url":"https://api.github.com/repos/weaveworks/weave/contents/{+path}","compare_url":"https://api.github.com/repos/weaveworks/weave/compare/{base}...{head}","merges_url":"https://api.github.com/repos/weaveworks/weave/merges","archive_url":"https://api.github.com/repos/weaveworks/weave/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/weaveworks/weave/downloads","issues_url":"https://api.github.com/repos/weaveworks/weave/issues{/number}","pulls_url":"https://api.github.com/repos/weaveworks/weave/pulls{/number}","milestones_url":"https://api.github.com/repos/weaveworks/weave/milestones{/number}","notifications_url":"https://api.github.com/repos/weaveworks/weave/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/weaveworks/weave/labels{/name}","releases_url":"https://api.github.com/repos/weaveworks/weave/releases{/id}","created_at":"2014-08-18T05:19:07Z","updated_at":"2015-08-09T23:39:21Z","pushed_at":"2015-08-10T13:32:05Z","git_url":"git://github.com/weaveworks/weave.git","ssh_url":"git@github.com:weaveworks/weave.git","clone_url":"https://github.com/weaveworks/weave.git","svn_url":"https://github.com/weaveworks/weave","homepage":"http://weave.works","size":13859,"stargazers_count":3600,"watchers_count":3600,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":217,"mirror_url":null,"open_issues_count":151,"forks":217,"open_issues":151,"watchers":3600,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/weaveworks/weave/pulls/991"},"html":{"href":"https://github.com/weaveworks/weave/pull/991"},"issue":{"href":"https://api.github.com/repos/weaveworks/weave/issues/991"},"comments":{"href":"https://api.github.com/repos/weaveworks/weave/issues/991/comments"},"review_comments":{"href":"https://api.github.com/repos/weaveworks/weave/pulls/991/comments"},"review_comment":{"href":"https://api.github.com/repos/weaveworks/weave/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/weaveworks/weave/pulls/991/commits"},"statuses":{"href":"https://api.github.com/repos/weaveworks/weave/statuses/f12ad3d8eea5039dbcd8ce6d03a76505c9df86ab"}}}}