Consider tid == tid.reply() when finalizing

https://github.com/google/gvisor/commit/4ab52f3cfdbe2c75c6525aa6732210a6d5d64b11
introduced a change to drop packets if we fail to insert the reply
tuple. This change did not take into consideration the case where the
original tuple ID is the same as the reply tuple ID. In this case, we
will have a "reply tuple conflict". However, since the reply tuple is
the same as the original tuple, we should not consider the conflict
as a real conflict since reply packets will map to the original tuple.

Updates #6850.

The change referenced above is cl/410368440.

PiperOrigin-RevId: 417634147
This commit is contained in:
Ghanan Gowripalan
2021-12-21 09:24:58 -08:00
committed by gVisor bot
parent b91cc35b40
commit e49295ddeb
3 changed files with 130 additions and 1 deletions
+10 -1
View File
@@ -620,12 +620,21 @@ func (ct *ConnTrack) finalize(cn *conn) finalizeResult {
bkt := &buckets[id]
bkt.mu.Lock()
if bkt.connForTIDRLocked(tid, ct.clock.NowMonotonic()) == nil {
t := bkt.connForTIDRLocked(tid, ct.clock.NowMonotonic())
if t == nil {
bkt.tuples.PushFront(&cn.reply)
bkt.mu.Unlock()
return finalizeResultSuccess
}
bkt.mu.Unlock()
if t.conn == cn {
// We already have an entry for the reply tuple.
//
// This can occur when the source address/port is the same as the
// destination address/port. In this scenario, tid == tid.reply().
return finalizeResultSuccess
}
}
// Another connection for the reply already exists. Remove the original and
+1
View File
@@ -34,6 +34,7 @@ go_test(
"//pkg/tcpip/checker",
"//pkg/tcpip/header",
"//pkg/tcpip/link/channel",
"//pkg/tcpip/link/loopback",
"//pkg/tcpip/network/arp",
"//pkg/tcpip/network/ipv4",
"//pkg/tcpip/network/ipv6",
@@ -26,6 +26,7 @@ import (
"gvisor.dev/gvisor/pkg/tcpip/checker"
"gvisor.dev/gvisor/pkg/tcpip/header"
"gvisor.dev/gvisor/pkg/tcpip/link/channel"
"gvisor.dev/gvisor/pkg/tcpip/link/loopback"
"gvisor.dev/gvisor/pkg/tcpip/network/arp"
"gvisor.dev/gvisor/pkg/tcpip/network/ipv4"
"gvisor.dev/gvisor/pkg/tcpip/network/ipv6"
@@ -2870,3 +2871,121 @@ func TestSNATHandlePortOrIdentConflicts(t *testing.T) {
})
}
}
func TestLocallyRoutedPackets(t *testing.T) {
const nicID = 1
tests := []struct {
name string
netProto tcpip.NetworkProtocolNumber
addr tcpip.Address
}{
{
name: "IPv4",
netProto: ipv4.ProtocolNumber,
addr: utils.Host1IPv4Addr.AddressWithPrefix.Address,
},
{
name: "IPv6",
netProto: ipv6.ProtocolNumber,
addr: utils.Host1IPv6Addr.AddressWithPrefix.Address,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
s := stack.New(stack.Options{
NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol},
TransportProtocols: []stack.TransportProtocolFactory{udp.NewProtocol},
})
if err := s.CreateNIC(nicID, loopback.New()); err != nil {
t.Fatalf("CreateNIC(%d, _) = %s", nicID, err)
}
protocolAddr := tcpip.ProtocolAddress{
Protocol: test.netProto,
AddressWithPrefix: test.addr.WithPrefix(),
}
if err := s.AddProtocolAddress(nicID, protocolAddr, stack.AddressProperties{}); err != nil {
t.Fatalf("AddProtocolAddress(%d, %+v, {}): %s", nicID, protocolAddr, err)
}
s.SetRouteTable([]tcpip.Route{
{
Destination: protocolAddr.AddressWithPrefix.Subnet(),
NIC: nicID,
},
})
// Set IPTables so we create entries in the conntrack table.
{
ipv6 := test.netProto == ipv6.ProtocolNumber
ipt := s.IPTables()
filter := ipt.GetTable(stack.FilterID, ipv6)
if err := ipt.ReplaceTable(stack.FilterID, filter, ipv6); err != nil {
t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.FilterID, ipv6, err)
}
}
var wq waiter.Queue
we, ch := waiter.NewChannelEntry(waiter.ReadableEvents)
wq.EventRegister(&we)
defer wq.EventUnregister(&we)
ep, err := s.NewEndpoint(udp.ProtocolNumber, test.netProto, &wq)
if err != nil {
t.Fatalf("s.NewEndpoint(%d, %d, _): %s", udp.ProtocolNumber, test.netProto, err)
}
defer ep.Close()
fullAddr := tcpip.FullAddress{Addr: test.addr, Port: 1234}
if err := ep.Bind(fullAddr); err != nil {
t.Fatalf("ep.Bind(%#v): %s", fullAddr, err)
}
if err := ep.Connect(fullAddr); err != nil {
t.Fatalf("ep.Connect(%#v): %s", fullAddr, err)
}
data := []byte{1, 2, 3, 4}
var r bytes.Reader
r.Reset(data)
var wOpts tcpip.WriteOptions
n, err := ep.Write(&r, wOpts)
if err != nil {
t.Fatalf("ep.Write(_, %#v): %s", wOpts, err)
}
if want := int64(len(data)); n != want {
t.Fatalf("got ep.Write(_, %#v) = (%d, _), want = (%d, _)", wOpts, n, want)
}
var buf bytes.Buffer
var res tcpip.ReadResult
for {
var err tcpip.Error
res, err = ep.Read(&buf, tcpip.ReadOptions{})
if _, ok := err.(*tcpip.ErrWouldBlock); ok {
<-ch
continue
}
if err != nil {
t.Fatalf("ep.Read(_, {}): %s", err)
}
break
}
if diff := cmp.Diff(
tcpip.ReadResult{
Count: len(data),
Total: len(data),
},
res,
checker.IgnoreCmpPath("ControlMessages"),
); diff != "" {
t.Errorf("ep.Read: unexpected result (-want +got):\n%s", diff)
}
if diff := cmp.Diff(buf.Bytes(), data); diff != "" {
t.Errorf("received data mismatch (-want +got):\n%s", diff)
}
})
}
}