diff --git a/router/connection.go b/router/connection.go index 6614d0a9..66f7e20a 100644 --- a/router/connection.go +++ b/router/connection.go @@ -379,8 +379,8 @@ func (conn *LocalConnection) extendReadDeadline() { conn.TCPConn.SetReadDeadline(time.Now().Add(TCPHeartbeat * 2)) } -func (conn *LocalConnection) Forward(srcPeer, dstPeer *Peer, frame []byte, dec *EthernetDecoder) error { - return conn.forwarder.Forward(srcPeer, dstPeer, frame, dec) +func (conn *LocalConnection) Forward(srcPeer, dstPeer *Peer, frame []byte, dec *EthernetDecoder, broadcast bool) { + conn.forwarder.Forward(srcPeer, dstPeer, frame, dec, broadcast) } func tickerChan(ticker *time.Ticker) <-chan time.Time { diff --git a/router/ethernet_decoder.go b/router/ethernet_decoder.go index 555ce033..95d222af 100644 --- a/router/ethernet_decoder.go +++ b/router/ethernet_decoder.go @@ -25,7 +25,7 @@ func (dec *EthernetDecoder) DecodeLayers(data []byte) error { return dec.parser.DecodeLayers(data, &dec.decoded) } -func (dec *EthernetDecoder) sendICMPFragNeeded(mtu int, sendFrame func([]byte) error) error { +func (dec *EthernetDecoder) makeICMPFragNeeded(mtu int) ([]byte, error) { buf := gopacket.NewSerializeBuffer() opts := gopacket.SerializeOptions{ FixLengths: true, @@ -53,11 +53,11 @@ func (dec *EthernetDecoder) sendICMPFragNeeded(mtu int, sendFrame func([]byte) e Seq: uint16(mtu)}, &payload) if err != nil { - return err + return nil, err } log.Printf("Sending ICMP 3,4 (%v -> %v): PMTU= %v\n", dec.IP.DstIP, dec.IP.SrcIP, mtu) - return sendFrame(buf.Bytes()) + return buf.Bytes(), nil } var ( diff --git a/router/interhost.go b/router/interhost.go index f3ec29ee..2cc1950b 100644 --- a/router/interhost.go +++ b/router/interhost.go @@ -41,7 +41,8 @@ type InterHostForwarder interface { // Forward a packet across the connection. The caller must // supply an EthernetDecoder specific to this thread, which // has already been used to decode the frame. - Forward(src *Peer, dest *Peer, frame []byte, dec *EthernetDecoder) error + Forward(src *Peer, dest *Peer, frame []byte, dec *EthernetDecoder, + broadcast bool) Close() @@ -68,8 +69,8 @@ func (NullInterHost) MakeForwarder(*Peer, net.IP, *net.UDPAddr, uint64, func (NullInterHost) SetListener(InterHostForwarderListener) { } -func (NullInterHost) Forward(*Peer, *Peer, []byte, *EthernetDecoder) error { - return nil +func (NullInterHost) Forward(*Peer, *Peer, []byte, *EthernetDecoder, + bool) { } func (NullInterHost) Close() { diff --git a/router/local_peer.go b/router/local_peer.go index 5afc6d48..6bac121d 100644 --- a/router/local_peer.go +++ b/router/local_peer.go @@ -28,29 +28,29 @@ func NewLocalPeer(name PeerName, nickName string, router *Router) *LocalPeer { return peer } -func (peer *LocalPeer) Forward(dstPeer *Peer, frame []byte, dec *EthernetDecoder) error { - return peer.Relay(peer.Peer, dstPeer, frame, dec) +func (peer *LocalPeer) Forward(dstPeer *Peer, frame []byte, dec *EthernetDecoder) { + peer.Relay(peer.Peer, dstPeer, frame, dec) } func (peer *LocalPeer) Broadcast(frame []byte, dec *EthernetDecoder) { peer.RelayBroadcast(peer.Peer, frame, dec) } -func (peer *LocalPeer) Relay(srcPeer, dstPeer *Peer, frame []byte, dec *EthernetDecoder) error { +func (peer *LocalPeer) Relay(srcPeer, dstPeer *Peer, frame []byte, dec *EthernetDecoder) { relayPeerName, found := peer.router.Routes.Unicast(dstPeer.Name) if !found { // Not necessarily an error as there could be a race with the // dst disappearing whilst the frame is in flight log.Println("Received packet for unknown destination:", dstPeer) - return nil + return } conn, found := peer.ConnectionTo(relayPeerName) if !found { // Again, could just be a race, not necessarily an error log.Println("Unable to find connection to relay peer", relayPeerName) - return nil + return } - return conn.(*LocalConnection).Forward(srcPeer, dstPeer, frame, dec) + conn.(*LocalConnection).Forward(srcPeer, dstPeer, frame, dec, false) } func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, frame []byte, dec *EthernetDecoder) { @@ -59,15 +59,8 @@ func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, frame []byte, dec *Ethernet return } for _, conn := range peer.ConnectionsTo(nextHops) { - err := conn.(*LocalConnection).Forward(srcPeer, conn.Remote(), - frame, dec) - if err != nil { - if ftbe, ok := err.(FrameTooBigError); ok { - log.Printf("dropping too big DF broadcast frame (%v -> %v): PMTU= %v\n", dec.IP.DstIP, dec.IP.SrcIP, ftbe.EPMTU) - } else { - log.Println(err) - } - } + conn.(*LocalConnection).Forward(srcPeer, conn.Remote(), frame, + dec, true) } } diff --git a/router/router.go b/router/router.go index fc9c9bd8..2b58c14f 100644 --- a/router/router.go +++ b/router/router.go @@ -135,11 +135,7 @@ func (router *Router) CapturedPacket(frameData []byte, dec *EthernetDecoder) { return } - err := router.Ourself.Forward(dstPeer, frameCopy, dec) - if ftbe, ok := err.(FrameTooBigError); ok { - err = dec.sendICMPFragNeeded(ftbe.EPMTU, router.IntraHost.InjectPacket) - } - checkWarn(err) + router.Ourself.Forward(dstPeer, frameCopy, dec) } func (router *Router) listenTCP(localPort int) { @@ -175,14 +171,7 @@ func (router *Router) handleForwardedPacket(srcPeer *Peer, dstPeer *Peer, if dstPeer != router.Ourself.Peer { // it's not for us, we're just relaying it router.LogFrame("Relaying", frame, dec) - err := router.Ourself.Relay(srcPeer, dstPeer, frame, dec) - if ftbe, ok := err.(FrameTooBigError); ok { - err = dec.sendICMPFragNeeded(ftbe.EPMTU, func(icmpFrame []byte) error { - return router.Ourself.Forward(srcPeer, icmpFrame, nil) - }) - } - - checkWarn(err) + router.Ourself.Relay(srcPeer, dstPeer, frame, dec) return } diff --git a/router/sleeve.go b/router/sleeve.go index 7507294e..7d813554 100644 --- a/router/sleeve.go +++ b/router/sleeve.go @@ -346,12 +346,7 @@ func (fwd *sleeveForwarder) SetListener(listener InterHostForwarderListener) { } func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, - dec *EthernetDecoder) error { - var df bool - if dec != nil { - df = dec.DF() - } - + dec *EthernetDecoder, broadcast bool) { fwd.lock.RLock() haveContact := (fwd.remoteAddr != nil) effectivePMTU := fwd.effectivePMTU @@ -360,7 +355,7 @@ func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, if !haveContact { fwd.log("Cannot forward frame yet - awaiting contact") - return nil + return } srcName := src.NameByte @@ -376,25 +371,48 @@ func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, // drop will likely get re-transmitted we end up paying that cost // multiple times. So it's better to drop things at the beginning // of our pipeline. - if df { - if frameTooBig(frame, effectivePMTU) { - return FrameTooBigError{EPMTU: effectivePMTU} + if dec.DF() { + if !frameTooBig(frame, effectivePMTU) { + fwd.aggregate(fwd.aggregatorDFChan, srcName, dstName, + frame) + return } - fwd.aggregate(fwd.aggregatorDFChan, srcName, dstName, frame) - return nil + // Why do we need an explicit broadcast hint here, + // rather than just checking the frame for a broadcast + // destination MAC address? Because even + // non-broadcast frames can be broadcast, if the + // destination MAC was not in our MAC cache. + if broadcast { + log.Printf("dropping too big DF broadcast frame (%v -> %v): PMTU= %v\n", dec.IP.DstIP, dec.IP.SrcIP, effectivePMTU) + return + } + + // Send an ICMP back to where the frame came from + fn, err := dec.makeICMPFragNeeded(effectivePMTU) + if err != nil { + log.Println(err) + return + } + + dec.DecodeLayers(fn) + + // The frag-needed packet does not have DF set, so the + // potential recursion here is bounded. + fwd.sleeve.consumer(dst, src, fn, dec) + return } - if stackFrag || dec == nil || len(dec.decoded) < 2 { + if stackFrag || len(dec.decoded) < 2 { fwd.aggregate(fwd.aggregatorChan, srcName, dstName, frame) - return nil + return } // Don't have trustworthy stack, so we're going to have to // send it DF in any case. if !frameTooBig(frame, effectivePMTU) { fwd.aggregate(fwd.aggregatorDFChan, srcName, dstName, frame) - return nil + return } fwd.sleeve.logFrame("Fragmenting", frame, &dec.Eth) @@ -402,11 +420,11 @@ func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, // We can't trust the stack to fragment, we have IP, and we // have a frame that's too big for the MTU, so we have to // fragment it ourself. - return fragment(dec.Eth, dec.IP, effectivePMTU, + checkWarn(fragment(dec.Eth, dec.IP, effectivePMTU, func(segFrame []byte) { fwd.aggregate(fwd.aggregatorDFChan, srcName, dstName, segFrame) - }) + })) } func (fwd *sleeveForwarder) aggregate(ch chan<- aggregatorFrame, src []byte, @@ -417,14 +435,6 @@ func (fwd *sleeveForwarder) aggregate(ch chan<- aggregatorFrame, src []byte, } } -type FrameTooBigError struct { - EPMTU int // effective pmtu, i.e. what we tell packet senders -} - -func (ftbe FrameTooBigError) Error() string { - return fmt.Sprint("Frame too big error. Effective PMTU is ", ftbe.EPMTU) -} - func fragment(eth layers.Ethernet, ip layers.IPv4, pmtu int, forward func([]byte)) error { // We are not doing any sort of NAT, so we don't need to worry