Only re-route on non-noop DNAT

https://github.com/google/gvisor/commit/115474bcf33c333d8eebad2e96d1b29463f455ba
made a change to always perform NAT on NAT-supported hooks so
stack.PacketBuffer.{D,S}NATDone will always be true after performing
their relevant hooks.

As an optimization, only re-routing packets if DNAT was not a no-op.

With this change, stack.PacketBuffer.{D,S}NATDone is no longer used
outside of the stack package so we can unexpose them.

PiperOrigin-RevId: 416400576
This commit is contained in:
Ghanan Gowripalan
2021-12-14 14:52:44 -08:00
committed by gVisor bot
parent 7f72261ad1
commit af571f421c
5 changed files with 42 additions and 39 deletions
+5 -5
View File
@@ -420,7 +420,8 @@ func (e *endpoint) handleFragments(_ *stack.Route, networkMTU uint32, pkt *stack
// WritePacket writes a packet to the given destination address and protocol.
func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams, pkt *stack.PacketBuffer) tcpip.Error {
if err := e.addIPHeader(r.LocalAddress(), r.RemoteAddress(), pkt, params, nil /* options */); err != nil {
dstAddr := r.RemoteAddress()
if err := e.addIPHeader(r.LocalAddress(), dstAddr, pkt, params, nil /* options */); err != nil {
return err
}
@@ -433,15 +434,14 @@ func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams,
return nil
}
// If the packet is manipulated as per NAT Output rules, handle packet
// If the packet is manipulated as per DNAT Output rules, handle packet
// based on destination address and do not send the packet to link
// layer.
//
// We should do this for every packet, rather than only NATted packets, but
// We should do this for every packet, rather than only DNATted packets, but
// removing this check short circuits broadcasts before they are sent out to
// other hosts.
if pkt.DNATDone {
netHeader := header.IPv4(pkt.NetworkHeader().View())
if netHeader := header.IPv4(pkt.NetworkHeader().View()); dstAddr != netHeader.DestinationAddress() {
if ep := e.protocol.findEndpointWithAddress(netHeader.DestinationAddress()); ep != nil {
// Since we rewrote the packet but it is being routed back to us, we
// can safely assume the checksum is valid.
+5 -5
View File
@@ -742,7 +742,8 @@ func (e *endpoint) handleFragments(r *stack.Route, networkMTU uint32, pkt *stack
// WritePacket writes a packet to the given destination address and protocol.
func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams, pkt *stack.PacketBuffer) tcpip.Error {
if err := addIPHeader(r.LocalAddress(), r.RemoteAddress(), pkt, params, nil /* extensionHeaders */); err != nil {
dstAddr := r.RemoteAddress()
if err := addIPHeader(r.LocalAddress(), dstAddr, pkt, params, nil /* extensionHeaders */); err != nil {
return err
}
@@ -755,15 +756,14 @@ func (e *endpoint) WritePacket(r *stack.Route, params stack.NetworkHeaderParams,
return nil
}
// If the packet is manipulated as per NAT Output rules, handle packet
// If the packet is manipulated as per DNAT Output rules, handle packet
// based on destination address and do not send the packet to link
// layer.
//
// We should do this for every packet, rather than only NATted packets, but
// We should do this for every packet, rather than only DNATted packets, but
// removing this check short circuits broadcasts before they are sent out to
// other hosts.
if pkt.DNATDone {
netHeader := header.IPv6(pkt.NetworkHeader().View())
if netHeader := header.IPv6(pkt.NetworkHeader().View()); dstAddr != netHeader.DestinationAddress() {
if ep := e.protocol.findEndpointWithAddress(netHeader.DestinationAddress()); ep != nil {
// Since we rewrote the packet but it is being routed back to us, we
// can safely assume the checksum is valid.
+3 -3
View File
@@ -813,7 +813,7 @@ func (cn *conn) handlePacket(pkt *PacketBuffer, hook Hook, rt *Route) bool {
fullChecksum := false
updatePseudoHeader := false
natDone := &pkt.SNATDone
natDone := &pkt.snatDone
dnat := false
switch hook {
case Prerouting:
@@ -822,13 +822,13 @@ func (cn *conn) handlePacket(pkt *PacketBuffer, hook Hook, rt *Route) bool {
fullChecksum = true
updatePseudoHeader = true
natDone = &pkt.DNATDone
natDone = &pkt.dnatDone
dnat = true
case Input:
case Forward:
panic("should not handle packet in the forwarding hook")
case Output:
natDone = &pkt.DNATDone
natDone = &pkt.dnatDone
dnat = true
fallthrough
case Postrouting:
+23 -20
View File
@@ -438,25 +438,23 @@ func (it *IPTables) checkNATRLocked(hook Hook, pkt *PacketBuffer, r *Route, addr
return true
}
var dnat bool
var natDone *bool
switch hook {
case Prerouting, Output:
dnat = true
natDone = &pkt.DNATDone
case Input, Postrouting:
dnat = false
natDone = &pkt.SNATDone
case Forward:
panic("should not attempt NAT in forwarding")
default:
panic(fmt.Sprintf("unhandled hook = %d", hook))
}
dnat, natDone := func() (bool, bool) {
switch hook {
case Prerouting, Output:
return true, pkt.dnatDone
case Input, Postrouting:
return false, pkt.snatDone
case Forward:
panic("should not attempt NAT in forwarding")
default:
panic(fmt.Sprintf("unhandled hook = %d", hook))
}
}()
// Make sure the connection is NATed.
//
// If the packet was already NATed, the connection must be NATed.
if !*natDone {
if !natDone {
t.conn.maybePerformNoopNAT(dnat)
_ = t.conn.handlePacket(pkt, hook, r)
}
@@ -566,12 +564,17 @@ func (it *IPTables) CheckPostroutingPackets(pkts PacketBufferList, r *Route, add
}, false /* dnat */)
}
func getAddr(pkt *PacketBuffer, dnat bool) tcpip.Address {
net := pkt.Network()
if dnat {
return net.DestinationAddress()
}
return net.SourceAddress()
}
func checkPackets(pkts PacketBufferList, f func(*PacketBuffer) bool, dnat bool) (drop map[*PacketBuffer]struct{}, natPkts map[*PacketBuffer]struct{}) {
for pkt := pkts.Front(); pkt != nil; pkt = pkt.Next() {
natDone := &pkt.SNATDone
if dnat {
natDone = &pkt.DNATDone
}
origAddr := getAddr(pkt, dnat)
if ok := f(pkt); !ok {
if drop == nil {
@@ -579,7 +582,7 @@ func checkPackets(pkts PacketBufferList, f func(*PacketBuffer) bool, dnat bool)
}
drop[pkt] = struct{}{}
}
if *natDone {
if newAddr := getAddr(pkt, dnat); newAddr != origAddr {
if natPkts == nil {
natPkts = make(map[*PacketBuffer]struct{})
}
+6 -6
View File
@@ -138,13 +138,13 @@ type PacketBuffer struct {
EgressRoute RouteInfo
GSOOptions GSO
// SNATDone indicates if the packet's source has been manipulated as per
// snatDone indicates if the packet's source has been manipulated as per
// iptables NAT table.
SNATDone bool
snatDone bool
// DNATDone indicates if the packet's destination has been manipulated as per
// dnatDone indicates if the packet's destination has been manipulated as per
// iptables NAT table.
DNATDone bool
dnatDone bool
// PktType indicates the SockAddrLink.PacketType of the packet as defined in
// https://www.man7.org/linux/man-pages/man7/packet.7.html.
@@ -346,8 +346,8 @@ func (pk *PacketBuffer) Clone() *PacketBuffer {
newPk.Owner = pk.Owner
newPk.GSOOptions = pk.GSOOptions
newPk.NetworkProtocolNumber = pk.NetworkProtocolNumber
newPk.DNATDone = pk.DNATDone
newPk.SNATDone = pk.SNATDone
newPk.dnatDone = pk.dnatDone
newPk.snatDone = pk.snatDone
newPk.TransportProtocolNumber = pk.TransportProtocolNumber
newPk.PktType = pk.PktType
newPk.NICID = pk.NICID