From e543bff1bae7f5eb03690e29800b962f8129141d Mon Sep 17 00:00:00 2001 From: David Wragg Date: Thu, 26 Feb 2015 13:10:51 +0000 Subject: [PATCH] Don't propagate errors from RelayBroadcast If forwarding of a broadcast frame resulted in a FrameTooBigError, we would generate an ICMP "fragmentation needed" packet that contravened RFC1122 on two counts: First, we should not produce a ICMP error message in response to a broadcast/multicast packet. Secondly, we should not produce an IP packet with a broadcast/multicast source IP address. (It also gave an ethernet frame with a broadcast source MAC, which probably contravenes some IEEE spec.) So when RelayBroadcast gets a FrameTooBigError, it should just drop the packet. Futhermore, if we fail to relay a broadcast on one connection, that's not a good reason to give up entirely - we can proceed to attempt to relay on the other next hop connections. Errors other than FrameTooBigError that propagated upwards were simply logged, so that logging is moved down into RelayBroadcast. --- router/local_peer.go | 13 ++++++++----- router/router.go | 6 ++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/router/local_peer.go b/router/local_peer.go index 1f2cf810..ff5b4e50 100644 --- a/router/local_peer.go +++ b/router/local_peer.go @@ -32,8 +32,8 @@ func (peer *LocalPeer) Forward(dstPeer *Peer, df bool, frame []byte, dec *Ethern return peer.Relay(peer.Peer, dstPeer, df, frame, dec) } -func (peer *LocalPeer) Broadcast(df bool, frame []byte, dec *EthernetDecoder) error { - return peer.RelayBroadcast(peer.Peer, df, frame, dec) +func (peer *LocalPeer) Broadcast(df bool, frame []byte, dec *EthernetDecoder) { + peer.RelayBroadcast(peer.Peer, df, frame, dec) } func (peer *LocalPeer) Relay(srcPeer, dstPeer *Peer, df bool, frame []byte, dec *EthernetDecoder) error { @@ -57,7 +57,7 @@ func (peer *LocalPeer) Relay(srcPeer, dstPeer *Peer, df bool, frame []byte, dec dec) } -func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, df bool, frame []byte, dec *EthernetDecoder) error { +func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, df bool, frame []byte, dec *EthernetDecoder) { for _, conn := range peer.NextBroadcastHops(srcPeer) { err := conn.Forward(df, &ForwardedFrame{ srcPeer: srcPeer, @@ -65,10 +65,13 @@ func (peer *LocalPeer) RelayBroadcast(srcPeer *Peer, df bool, frame []byte, dec frame: frame}, dec) if err != nil { - return err + 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) + } } } - return nil } func (peer *LocalPeer) NextBroadcastHops(srcPeer *Peer) []*LocalConnection { diff --git a/router/router.go b/router/router.go index feb37cb2..8eaa34e4 100644 --- a/router/router.go +++ b/router/router.go @@ -160,7 +160,8 @@ func (router *Router) handleCapturedPacket(frameData []byte, dec *EthernetDecode copy(frameCopy, frameData) if !found { - return checkFrameTooBig(router.Ourself.Broadcast(df, frameCopy, dec)) + router.Ourself.Broadcast(df, frameCopy, dec) + return nil } else { return checkFrameTooBig(router.Ourself.Forward(dstPeer, df, frameCopy, dec)) } @@ -344,7 +345,8 @@ func (router *Router) handleUDPPacketFunc(dec *EthernetDecoder, po PacketSink) F dstPeer, found = router.Macs.Lookup(dstMac) if !found || dstPeer != router.Ourself.Peer { - return checkFrameTooBig(router.Ourself.RelayBroadcast(srcPeer, df, frame, dec), srcPeer) + router.LogFrame("Relaying broadcast", frame, &dec.eth) + router.Ourself.RelayBroadcast(srcPeer, df, frame, dec) } return nil