diff --git a/router/connection.go b/router/connection.go index e000c516..8a844cb7 100644 --- a/router/connection.go +++ b/router/connection.go @@ -446,8 +446,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 0593873c..ebc9b252 100644 --- a/router/ethernet_decoder.go +++ b/router/ethernet_decoder.go @@ -29,7 +29,7 @@ func (dec *EthernetDecoder) DecodeLayers(data []byte) { 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, @@ -57,11 +57,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", dec.IP.DstIP, dec.IP.SrcIP, mtu) - return sendFrame(buf.Bytes()) + return buf.Bytes(), nil } var ( diff --git a/router/local_peer.go b/router/local_peer.go index 9dc92bb3..7f331556 100644 --- a/router/local_peer.go +++ b/router/local_peer.go @@ -27,29 +27,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) { @@ -58,15 +58,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.Warningf("dropping too big DF broadcast frame (%v -> %v): PMTU= %v", dec.IP.DstIP, dec.IP.SrcIP, ftbe.EPMTU) - } else { - log.Errorln(err) - } - } + conn.(*LocalConnection).Forward(srcPeer, conn.Remote(), frame, + dec, true) } } diff --git a/router/overlay.go b/router/overlay.go index 163541eb..b8ac0699 100644 --- a/router/overlay.go +++ b/router/overlay.go @@ -61,7 +61,8 @@ type OverlayForwarder 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) Stop() @@ -87,8 +88,8 @@ func (NullOverlay) MakeForwarder(ForwarderParams) (OverlayForwarder, error) { func (NullOverlay) SetListener(OverlayForwarderListener) { } -func (NullOverlay) Forward(*Peer, *Peer, []byte, *EthernetDecoder) error { - return nil +func (NullOverlay) Forward(*Peer, *Peer, []byte, *EthernetDecoder, + bool) { } func (NullOverlay) Stop() { diff --git a/router/router.go b/router/router.go index 3717497a..a26745f0 100644 --- a/router/router.go +++ b/router/router.go @@ -138,11 +138,7 @@ func (router *Router) handleCapturedPacket(frameData []byte, dec *EthernetDecode return } - err := router.Ourself.Forward(dstPeer, frameCopy, dec) - if ftbe, ok := err.(FrameTooBigError); ok { - err = dec.sendICMPFragNeeded(ftbe.EPMTU, router.Bridge.InjectPacket) - } - checkWarn(err) + router.Ourself.Forward(dstPeer, frameCopy, dec) } func (router *Router) listenTCP(localPort int) { @@ -179,14 +175,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 58caff8b..0c002bd8 100644 --- a/router/sleeve.go +++ b/router/sleeve.go @@ -390,7 +390,7 @@ func (fwd *sleeveForwarder) SetListener(listener OverlayForwarderListener) { } func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, - dec *EthernetDecoder) error { + dec *EthernetDecoder, broadcast bool) { fwd.lock.RLock() haveContact := (fwd.remoteAddr != nil) mtu := fwd.mtu @@ -400,7 +400,7 @@ func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, if !haveContact { log.Print(fwd.logPrefix(), "Cannot forward frame yet - awaiting contact") - return nil + return } srcName := src.NameByte @@ -417,34 +417,57 @@ func (fwd *sleeveForwarder) Forward(src *Peer, dst *Peer, frame []byte, // multiple times. So it's better to drop things at the beginning // of our pipeline. if dec.DF() { - if frameTooBig(frame, mtu) { - return FrameTooBigError{EPMTU: mtu} + if !frameTooBig(frame, mtu) { + 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.Print(fwd.logPrefix(), "dropping too big DF broadcast frame (", dec.IP.DstIP, " -> ", dec.IP.SrcIP, "): MTU=", mtu) + return + } + + // Send an ICMP back to where the frame came from + fragNeededPacket, err := dec.makeICMPFragNeeded(mtu) + if err != nil { + log.Print(fwd.logPrefix(), err) + return + } + + dec.DecodeLayers(fragNeededPacket) + + // The frag-needed packet does not have DF set, so the + // potential recursion here is bounded. + fwd.sleeve.consumer(dst, src, fragNeededPacket, 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, mtu) { fwd.aggregate(fwd.aggregatorDFChan, srcName, dstName, frame) - return nil + return } // 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, mtu, + checkWarn(fragment(dec.Eth, dec.IP, mtu, func(segFrame []byte) { fwd.aggregate(fwd.aggregatorDFChan, srcName, dstName, segFrame) - }) + })) } func (fwd *sleeveForwarder) aggregate(ch chan<- aggregatorFrame, src []byte, @@ -455,14 +478,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, mtu int, forward func([]byte)) error { // We are not doing any sort of NAT, so we don't need to worry