Make stack.Route safe to access concurrently

Multiple goroutines may use the same stack.Route concurrently so
the stack.Route should make sure that any functions called on it
are thread-safe.

Fixes #4073

PiperOrigin-RevId: 344320491
This commit is contained in:
Ghanan Gowripalan
2020-11-25 14:52:59 -08:00
committed by gVisor bot
parent 4d59a5a622
commit 2485a4e2cb
19 changed files with 183 additions and 109 deletions
+1 -1
View File
@@ -31,7 +31,7 @@ type PacketInfo struct {
Pkt *stack.PacketBuffer
Proto tcpip.NetworkProtocolNumber
GSO *stack.GSO
Route stack.Route
Route *stack.Route
}
// Notification is the interface for receiving notification from the packet
+2 -2
View File
@@ -155,7 +155,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, protocol tcpip.Ne
// WritePacket caller's do not set the following fields in PacketBuffer
// so we populate them here.
newRoute := r.Clone()
pkt.EgressRoute = &newRoute
pkt.EgressRoute = newRoute
pkt.GSOOptions = gso
pkt.NetworkProtocolNumber = protocol
d := e.dispatchers[int(pkt.Hash)%len(e.dispatchers)]
@@ -182,7 +182,7 @@ func (e *endpoint) WritePackets(_ *stack.Route, _ *stack.GSO, pkts stack.PacketB
// the route here to ensure it doesn't get released while the
// packet is still in our queue.
newRoute := pkt.EgressRoute.Clone()
pkt.EgressRoute = &newRoute
pkt.EgressRoute = newRoute
if !d.q.enqueue(pkt) {
if enqueued > 0 {
d.newPacketWaker.Assert()
+4 -4
View File
@@ -203,7 +203,7 @@ func (*testObject) AddHeader(local, remote tcpip.LinkAddress, protocol tcpip.Net
panic("not implemented")
}
func buildIPv4Route(local, remote tcpip.Address) (stack.Route, *tcpip.Error) {
func buildIPv4Route(local, remote tcpip.Address) (*stack.Route, *tcpip.Error) {
s := stack.New(stack.Options{
NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol},
TransportProtocols: []stack.TransportProtocolFactory{udp.NewProtocol, tcp.NewProtocol},
@@ -219,7 +219,7 @@ func buildIPv4Route(local, remote tcpip.Address) (stack.Route, *tcpip.Error) {
return s.FindRoute(nicID, local, remote, ipv4.ProtocolNumber, false /* multicastLoop */)
}
func buildIPv6Route(local, remote tcpip.Address) (stack.Route, *tcpip.Error) {
func buildIPv6Route(local, remote tcpip.Address) (*stack.Route, *tcpip.Error) {
s := stack.New(stack.Options{
NetworkProtocols: []stack.NetworkProtocolFactory{ipv6.NewProtocol},
TransportProtocols: []stack.TransportProtocolFactory{udp.NewProtocol, tcp.NewProtocol},
@@ -550,7 +550,7 @@ func TestIPv4Send(t *testing.T) {
if err != nil {
t.Fatalf("could not find route: %v", err)
}
if err := ep.WritePacket(&r, nil /* gso */, stack.NetworkHeaderParams{
if err := ep.WritePacket(r, nil /* gso */, stack.NetworkHeaderParams{
Protocol: 123,
TTL: 123,
TOS: stack.DefaultTOS,
@@ -933,7 +933,7 @@ func TestIPv6Send(t *testing.T) {
if err != nil {
t.Fatalf("could not find route: %v", err)
}
if err := ep.WritePacket(&r, nil /* gso */, stack.NetworkHeaderParams{
if err := ep.WritePacket(r, nil /* gso */, stack.NetworkHeaderParams{
Protocol: 123,
TTL: 123,
TOS: stack.DefaultTOS,
+2 -2
View File
@@ -2617,7 +2617,7 @@ func TestWriteStats(t *testing.T) {
test.setup(t, rt.Stack())
nWritten, _ := writer.writePackets(&rt, pkts)
nWritten, _ := writer.writePackets(rt, pkts)
if got := int(rt.Stats().IP.PacketsSent.Value()); got != test.expectSent {
t.Errorf("sent %d packets, but expected to send %d", got, test.expectSent)
@@ -2634,7 +2634,7 @@ func TestWriteStats(t *testing.T) {
}
}
func buildRoute(t *testing.T, ep stack.LinkEndpoint) stack.Route {
func buildRoute(t *testing.T, ep stack.LinkEndpoint) *stack.Route {
s := stack.New(stack.Options{
NetworkProtocols: []stack.NetworkProtocolFactory{ipv4.NewProtocol},
})
+2 -2
View File
@@ -2464,7 +2464,7 @@ func TestWriteStats(t *testing.T) {
test.setup(t, rt.Stack())
nWritten, _ := writer.writePackets(&rt, pkts)
nWritten, _ := writer.writePackets(rt, pkts)
if got := int(rt.Stats().IP.PacketsSent.Value()); got != test.expectSent {
t.Errorf("sent %d packets, but expected to send %d", got, test.expectSent)
@@ -2481,7 +2481,7 @@ func TestWriteStats(t *testing.T) {
}
}
func buildRoute(t *testing.T, ep stack.LinkEndpoint) stack.Route {
func buildRoute(t *testing.T, ep stack.LinkEndpoint) *stack.Route {
s := stack.New(stack.Options{
NetworkProtocols: []stack.NetworkProtocolFactory{NewProtocol},
})
+9 -3
View File
@@ -465,14 +465,18 @@ func TestDADResolve(t *testing.T) {
if err != tcpip.ErrNoRoute {
t.Errorf("got FindRoute(%d, '', %s, %d, false) = (%+v, %v), want = (_, %s)", nicID, addr2, header.IPv6ProtocolNumber, r, err, tcpip.ErrNoRoute)
}
r.Release()
if r != nil {
r.Release()
}
}
{
r, err := s.FindRoute(nicID, addr1, addr2, header.IPv6ProtocolNumber, false)
if err != tcpip.ErrNoRoute {
t.Errorf("got FindRoute(%d, %s, %s, %d, false) = (%+v, %v), want = (_, %s)", nicID, addr1, addr2, header.IPv6ProtocolNumber, r, err, tcpip.ErrNoRoute)
}
r.Release()
if r != nil {
r.Release()
}
}
if t.Failed() {
@@ -510,7 +514,9 @@ func TestDADResolve(t *testing.T) {
} else if r.LocalAddress != addr1 {
t.Errorf("got r.LocalAddress = %s, want = %s", r.LocalAddress, addr1)
}
r.Release()
if r != nil {
r.Release()
}
}
if t.Failed() {
+1 -1
View File
@@ -267,7 +267,7 @@ func (n *NIC) WritePacket(r *Route, gso *GSO, protocol tcpip.NetworkProtocolNumb
if ch, err := r.Resolve(nil); err != nil {
if err == tcpip.ErrWouldBlock {
r := r.Clone()
n.stack.linkResQueue.enqueue(ch, &r, protocol, pkt)
n.stack.linkResQueue.enqueue(ch, r, protocol, pkt)
return nil
}
return err
+74 -26
View File
@@ -18,11 +18,18 @@ import (
"fmt"
"gvisor.dev/gvisor/pkg/sleep"
"gvisor.dev/gvisor/pkg/sync"
"gvisor.dev/gvisor/pkg/tcpip"
"gvisor.dev/gvisor/pkg/tcpip/header"
)
// Route represents a route through the networking stack to a given destination.
//
// It is safe to call Route's methods from multiple goroutines.
//
// The exported fields are immutable.
//
// TODO(gvisor.dev/issue/4902): Unexpose immutable fields.
type Route struct {
// RemoteAddress is the final destination of the route.
RemoteAddress tcpip.Address
@@ -52,8 +59,12 @@ type Route struct {
// address's assigned status without the NIC.
localAddressNIC *NIC
// localAddressEndpoint is the local address this route is associated with.
localAddressEndpoint AssignableAddressEndpoint
mu struct {
sync.RWMutex
// localAddressEndpoint is the local address this route is associated with.
localAddressEndpoint AssignableAddressEndpoint
}
// outgoingNIC is the interface this route uses to write packets.
outgoingNIC *NIC
@@ -71,14 +82,14 @@ type Route struct {
// ownership of the provided local address.
//
// Returns an empty route if validation fails.
func constructAndValidateRoute(netProto tcpip.NetworkProtocolNumber, addressEndpoint AssignableAddressEndpoint, localAddressNIC, outgoingNIC *NIC, gateway, localAddr, remoteAddr tcpip.Address, handleLocal, multicastLoop bool) Route {
func constructAndValidateRoute(netProto tcpip.NetworkProtocolNumber, addressEndpoint AssignableAddressEndpoint, localAddressNIC, outgoingNIC *NIC, gateway, localAddr, remoteAddr tcpip.Address, handleLocal, multicastLoop bool) *Route {
if len(localAddr) == 0 {
localAddr = addressEndpoint.AddressWithPrefix().Address
}
if localAddressNIC != outgoingNIC && header.IsV6LinkLocalAddress(localAddr) {
addressEndpoint.DecRef()
return Route{}
return nil
}
// If no remote address is provided, use the local address.
@@ -110,7 +121,7 @@ func constructAndValidateRoute(netProto tcpip.NetworkProtocolNumber, addressEndp
// makeRoute initializes a new route. It takes ownership of the provided
// AssignableAddressEndpoint.
func makeRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, handleLocal, multicastLoop bool) Route {
func makeRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, handleLocal, multicastLoop bool) *Route {
if localAddressNIC.stack != outgoingNIC.stack {
panic(fmt.Sprintf("cannot create a route with NICs from different stacks"))
}
@@ -139,18 +150,21 @@ func makeRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip
return makeRouteInner(netProto, localAddr, remoteAddr, outgoingNIC, localAddressNIC, localAddressEndpoint, loop)
}
func makeRouteInner(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, loop PacketLooping) Route {
r := Route{
NetProto: netProto,
LocalAddress: localAddr,
LocalLinkAddress: outgoingNIC.LinkEndpoint.LinkAddress(),
RemoteAddress: remoteAddr,
localAddressNIC: localAddressNIC,
localAddressEndpoint: localAddressEndpoint,
outgoingNIC: outgoingNIC,
Loop: loop,
func makeRouteInner(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint, loop PacketLooping) *Route {
r := &Route{
NetProto: netProto,
LocalAddress: localAddr,
LocalLinkAddress: outgoingNIC.LinkEndpoint.LinkAddress(),
RemoteAddress: remoteAddr,
localAddressNIC: localAddressNIC,
outgoingNIC: outgoingNIC,
Loop: loop,
}
r.mu.Lock()
r.mu.localAddressEndpoint = localAddressEndpoint
r.mu.Unlock()
if r.outgoingNIC.LinkEndpoint.Capabilities()&CapabilityResolutionRequired != 0 {
if linkRes, ok := r.outgoingNIC.stack.linkAddrResolvers[r.NetProto]; ok {
r.linkRes = linkRes
@@ -165,7 +179,7 @@ func makeRouteInner(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr
// provided AssignableAddressEndpoint.
//
// A local route is a route to a destination that is local to the stack.
func makeLocalRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint) Route {
func makeLocalRoute(netProto tcpip.NetworkProtocolNumber, localAddr, remoteAddr tcpip.Address, outgoingNIC, localAddressNIC *NIC, localAddressEndpoint AssignableAddressEndpoint) *Route {
loop := PacketLoop
// TODO(gvisor.dev/issue/4689): Loopback interface loops back packets at the
// link endpoint level. We can remove this check once loopback interfaces
@@ -327,7 +341,10 @@ func (r *Route) isValidForOutgoing() bool {
return false
}
if !r.localAddressNIC.isValidForOutgoing(r.localAddressEndpoint) {
r.mu.RLock()
localAddressEndpoint := r.mu.localAddressEndpoint
r.mu.RUnlock()
if localAddressEndpoint == nil || !r.localAddressNIC.isValidForOutgoing(localAddressEndpoint) {
return false
}
@@ -381,20 +398,44 @@ func (r *Route) MTU() uint32 {
// Release frees all resources associated with the route.
func (r *Route) Release() {
if r.localAddressEndpoint != nil {
r.localAddressEndpoint.DecRef()
r.localAddressEndpoint = nil
r.mu.Lock()
defer r.mu.Unlock()
if r.mu.localAddressEndpoint != nil {
r.mu.localAddressEndpoint.DecRef()
r.mu.localAddressEndpoint = nil
}
}
// Clone clones the route.
func (r *Route) Clone() Route {
if r.localAddressEndpoint != nil {
if !r.localAddressEndpoint.IncRef() {
panic(fmt.Sprintf("failed to increment reference count for local address endpoint = %s", r.LocalAddress))
func (r *Route) Clone() *Route {
r.mu.RLock()
defer r.mu.RUnlock()
newRoute := &Route{
RemoteAddress: r.RemoteAddress,
RemoteLinkAddress: r.RemoteLinkAddress,
LocalAddress: r.LocalAddress,
LocalLinkAddress: r.LocalLinkAddress,
NextHop: r.NextHop,
NetProto: r.NetProto,
Loop: r.Loop,
localAddressNIC: r.localAddressNIC,
outgoingNIC: r.outgoingNIC,
linkCache: r.linkCache,
linkRes: r.linkRes,
}
newRoute.mu.Lock()
defer newRoute.mu.Unlock()
newRoute.mu.localAddressEndpoint = r.mu.localAddressEndpoint
if newRoute.mu.localAddressEndpoint != nil {
if !newRoute.mu.localAddressEndpoint.IncRef() {
panic(fmt.Sprintf("failed to increment reference count for local address endpoint = %s", newRoute.LocalAddress))
}
}
return *r
return newRoute
}
// Stack returns the instance of the Stack that owns this route.
@@ -407,7 +448,14 @@ func (r *Route) isV4Broadcast(addr tcpip.Address) bool {
return true
}
subnet := r.localAddressEndpoint.Subnet()
r.mu.RLock()
localAddressEndpoint := r.mu.localAddressEndpoint
r.mu.RUnlock()
if localAddressEndpoint == nil {
return false
}
subnet := localAddressEndpoint.Subnet()
return subnet.IsBroadcast(addr)
}
+21 -21
View File
@@ -1218,10 +1218,10 @@ func (s *Stack) getAddressEP(nic *NIC, localAddr, remoteAddr tcpip.Address, netP
// from the specified NIC.
//
// Precondition: s.mu must be read locked.
func (s *Stack) findLocalRouteFromNICRLocked(localAddressNIC *NIC, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber) (route Route, ok bool) {
func (s *Stack) findLocalRouteFromNICRLocked(localAddressNIC *NIC, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber) *Route {
localAddressEndpoint := localAddressNIC.getAddressOrCreateTempInner(netProto, localAddr, false /* createTemp */, NeverPrimaryEndpoint)
if localAddressEndpoint == nil {
return Route{}, false
return nil
}
var outgoingNIC *NIC
@@ -1245,7 +1245,7 @@ func (s *Stack) findLocalRouteFromNICRLocked(localAddressNIC *NIC, localAddr, re
// route.
if outgoingNIC == nil {
localAddressEndpoint.DecRef()
return Route{}, false
return nil
}
r := makeLocalRoute(
@@ -1259,10 +1259,10 @@ func (s *Stack) findLocalRouteFromNICRLocked(localAddressNIC *NIC, localAddr, re
if r.IsOutboundBroadcast() {
r.Release()
return Route{}, false
return nil
}
return r, true
return r
}
// findLocalRouteRLocked returns a local route.
@@ -1271,26 +1271,26 @@ func (s *Stack) findLocalRouteFromNICRLocked(localAddressNIC *NIC, localAddr, re
// is, a local route is a route where packets never have to leave the stack.
//
// Precondition: s.mu must be read locked.
func (s *Stack) findLocalRouteRLocked(localAddressNICID tcpip.NICID, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber) (route Route, ok bool) {
func (s *Stack) findLocalRouteRLocked(localAddressNICID tcpip.NICID, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber) *Route {
if len(localAddr) == 0 {
localAddr = remoteAddr
}
if localAddressNICID == 0 {
for _, localAddressNIC := range s.nics {
if r, ok := s.findLocalRouteFromNICRLocked(localAddressNIC, localAddr, remoteAddr, netProto); ok {
return r, true
if r := s.findLocalRouteFromNICRLocked(localAddressNIC, localAddr, remoteAddr, netProto); r != nil {
return r
}
}
return Route{}, false
return nil
}
if localAddressNIC, ok := s.nics[localAddressNICID]; ok {
return s.findLocalRouteFromNICRLocked(localAddressNIC, localAddr, remoteAddr, netProto)
}
return Route{}, false
return nil
}
// FindRoute creates a route to the given destination address, leaving through
@@ -1304,7 +1304,7 @@ func (s *Stack) findLocalRouteRLocked(localAddressNICID tcpip.NICID, localAddr,
// If no local address is provided, the stack will select a local address. If no
// remote address is provided, the stack wil use a remote address equal to the
// local address.
func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber, multicastLoop bool) (Route, *tcpip.Error) {
func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber, multicastLoop bool) (*Route, *tcpip.Error) {
s.mu.RLock()
defer s.mu.RUnlock()
@@ -1315,7 +1315,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
needRoute := !(isLocalBroadcast || isMulticast || isLinkLocal || isLoopback)
if s.handleLocal && !isMulticast && !isLocalBroadcast {
if r, ok := s.findLocalRouteRLocked(id, localAddr, remoteAddr, netProto); ok {
if r := s.findLocalRouteRLocked(id, localAddr, remoteAddr, netProto); r != nil {
return r, nil
}
}
@@ -1339,9 +1339,9 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
}
if isLoopback {
return Route{}, tcpip.ErrBadLocalAddress
return nil, tcpip.ErrBadLocalAddress
}
return Route{}, tcpip.ErrNetworkUnreachable
return nil, tcpip.ErrNetworkUnreachable
}
canForward := s.Forwarding(netProto) && !header.IsV6LinkLocalAddress(localAddr) && !isLinkLocal
@@ -1365,7 +1365,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
gateway = route.Gateway
}
r := constructAndValidateRoute(netProto, addressEndpoint, nic /* outgoingNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop)
if r == (Route{}) {
if r == nil {
panic(fmt.Sprintf("non-forwarding route validation failed with route table entry = %#v, id = %d, localAddr = %s, remoteAddr = %s", route, id, localAddr, remoteAddr))
}
return r, nil
@@ -1401,13 +1401,13 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
if id != 0 {
if aNIC, ok := s.nics[id]; ok {
if addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, netProto); addressEndpoint != nil {
if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != (Route{}) {
if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != nil {
return r, nil
}
}
}
return Route{}, tcpip.ErrNoRoute
return nil, tcpip.ErrNoRoute
}
if id == 0 {
@@ -1419,7 +1419,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
continue
}
if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != (Route{}) {
if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != nil {
return r, nil
}
}
@@ -1427,12 +1427,12 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
}
if needRoute {
return Route{}, tcpip.ErrNoRoute
return nil, tcpip.ErrNoRoute
}
if header.IsV6LoopbackAddress(remoteAddr) {
return Route{}, tcpip.ErrBadLocalAddress
return nil, tcpip.ErrBadLocalAddress
}
return Route{}, tcpip.ErrNetworkUnreachable
return nil, tcpip.ErrNetworkUnreachable
}
// CheckNetworkProtocol checks if a given network protocol is enabled in the
+19 -17
View File
@@ -407,7 +407,7 @@ func sendTo(s *stack.Stack, addr tcpip.Address, payload buffer.View) *tcpip.Erro
return send(r, payload)
}
func send(r stack.Route, payload buffer.View) *tcpip.Error {
func send(r *stack.Route, payload buffer.View) *tcpip.Error {
return r.WritePacket(nil /* gso */, stack.NetworkHeaderParams{Protocol: fakeTransNumber, TTL: 123, TOS: stack.DefaultTOS}, stack.NewPacketBuffer(stack.PacketBufferOptions{
ReserveHeaderBytes: int(r.MaxHeaderLength()),
Data: payload.ToVectorisedView(),
@@ -425,7 +425,7 @@ func testSendTo(t *testing.T, s *stack.Stack, addr tcpip.Address, ep *channel.En
}
}
func testSend(t *testing.T, r stack.Route, ep *channel.Endpoint, payload buffer.View) {
func testSend(t *testing.T, r *stack.Route, ep *channel.Endpoint, payload buffer.View) {
t.Helper()
ep.Drain()
if err := send(r, payload); err != nil {
@@ -436,7 +436,7 @@ func testSend(t *testing.T, r stack.Route, ep *channel.Endpoint, payload buffer.
}
}
func testFailingSend(t *testing.T, r stack.Route, ep *channel.Endpoint, payload buffer.View, wantErr *tcpip.Error) {
func testFailingSend(t *testing.T, r *stack.Route, ep *channel.Endpoint, payload buffer.View, wantErr *tcpip.Error) {
t.Helper()
if gotErr := send(r, payload); gotErr != wantErr {
t.Errorf("send failed: got = %s, want = %s ", gotErr, wantErr)
@@ -1563,7 +1563,7 @@ func TestSpoofingNoAddress(t *testing.T) {
// testSendTo(t, s, remoteAddr, ep, nil)
}
func verifyRoute(gotRoute, wantRoute stack.Route) error {
func verifyRoute(gotRoute, wantRoute *stack.Route) error {
if gotRoute.LocalAddress != wantRoute.LocalAddress {
return fmt.Errorf("bad local address: got %s, want = %s", gotRoute.LocalAddress, wantRoute.LocalAddress)
}
@@ -1603,7 +1603,7 @@ func TestOutgoingBroadcastWithEmptyRouteTable(t *testing.T) {
if err != nil {
t.Fatalf("FindRoute(1, %v, %v, %d) failed: %v", header.IPv4Any, header.IPv4Broadcast, fakeNetNumber, err)
}
if err := verifyRoute(r, stack.Route{LocalAddress: header.IPv4Any, RemoteAddress: header.IPv4Broadcast}); err != nil {
if err := verifyRoute(r, &stack.Route{LocalAddress: header.IPv4Any, RemoteAddress: header.IPv4Broadcast}); err != nil {
t.Errorf("FindRoute(1, %v, %v, %d) returned unexpected Route: %v", header.IPv4Any, header.IPv4Broadcast, fakeNetNumber, err)
}
@@ -1657,7 +1657,7 @@ func TestOutgoingBroadcastWithRouteTable(t *testing.T) {
if err != nil {
t.Fatalf("FindRoute(1, %v, %v, %d) failed: %v", nic1Addr.Address, header.IPv4Broadcast, fakeNetNumber, err)
}
if err := verifyRoute(r, stack.Route{LocalAddress: nic1Addr.Address, RemoteAddress: header.IPv4Broadcast}); err != nil {
if err := verifyRoute(r, &stack.Route{LocalAddress: nic1Addr.Address, RemoteAddress: header.IPv4Broadcast}); err != nil {
t.Errorf("FindRoute(1, %v, %v, %d) returned unexpected Route: %v", nic1Addr.Address, header.IPv4Broadcast, fakeNetNumber, err)
}
@@ -1667,7 +1667,7 @@ func TestOutgoingBroadcastWithRouteTable(t *testing.T) {
if err != nil {
t.Fatalf("FindRoute(0, \"\", %s, %d) failed: %s", header.IPv4Broadcast, fakeNetNumber, err)
}
if err := verifyRoute(r, stack.Route{LocalAddress: nic2Addr.Address, RemoteAddress: header.IPv4Broadcast}); err != nil {
if err := verifyRoute(r, &stack.Route{LocalAddress: nic2Addr.Address, RemoteAddress: header.IPv4Broadcast}); err != nil {
t.Errorf("FindRoute(0, \"\", %s, %d) returned unexpected Route: %s)", header.IPv4Broadcast, fakeNetNumber, err)
}
@@ -1683,7 +1683,7 @@ func TestOutgoingBroadcastWithRouteTable(t *testing.T) {
if err != nil {
t.Fatalf("FindRoute(0, \"\", %s, %d) failed: %s", header.IPv4Broadcast, fakeNetNumber, err)
}
if err := verifyRoute(r, stack.Route{LocalAddress: nic1Addr.Address, RemoteAddress: header.IPv4Broadcast}); err != nil {
if err := verifyRoute(r, &stack.Route{LocalAddress: nic1Addr.Address, RemoteAddress: header.IPv4Broadcast}); err != nil {
t.Errorf("FindRoute(0, \"\", %s, %d) returned unexpected Route: %s)", header.IPv4Broadcast, fakeNetNumber, err)
}
}
@@ -3355,7 +3355,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
nicAddr tcpip.ProtocolAddress
routes []tcpip.Route
remoteAddr tcpip.Address
expectedRoute stack.Route
expectedRoute *stack.Route
}{
// Broadcast to a locally attached subnet populates the broadcast MAC.
{
@@ -3371,7 +3371,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: ipv4SubnetBcast,
expectedRoute: stack.Route{
expectedRoute: &stack.Route{
LocalAddress: ipv4Addr.Address,
RemoteAddress: ipv4SubnetBcast,
RemoteLinkAddress: header.EthernetBroadcastAddress,
@@ -3394,7 +3394,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: ipv4Subnet31Bcast,
expectedRoute: stack.Route{
expectedRoute: &stack.Route{
LocalAddress: ipv4AddrPrefix31.Address,
RemoteAddress: ipv4Subnet31Bcast,
NetProto: header.IPv4ProtocolNumber,
@@ -3416,7 +3416,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: ipv4Subnet32Bcast,
expectedRoute: stack.Route{
expectedRoute: &stack.Route{
LocalAddress: ipv4AddrPrefix32.Address,
RemoteAddress: ipv4Subnet32Bcast,
NetProto: header.IPv4ProtocolNumber,
@@ -3437,7 +3437,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: ipv6SubnetBcast,
expectedRoute: stack.Route{
expectedRoute: &stack.Route{
LocalAddress: ipv6Addr.Address,
RemoteAddress: ipv6SubnetBcast,
NetProto: header.IPv6ProtocolNumber,
@@ -3460,7 +3460,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: remNetSubnetBcast,
expectedRoute: stack.Route{
expectedRoute: &stack.Route{
LocalAddress: ipv4Addr.Address,
RemoteAddress: remNetSubnetBcast,
NextHop: ipv4Gateway,
@@ -3485,7 +3485,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: remNetSubnetBcast,
expectedRoute: stack.Route{
expectedRoute: &stack.Route{
LocalAddress: ipv4Addr.Address,
RemoteAddress: remNetSubnetBcast,
NextHop: ipv4Gateway,
@@ -3522,7 +3522,7 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
if r, err := s.FindRoute(unspecifiedNICID, "" /* localAddr */, test.remoteAddr, netProto, false /* multicastLoop */); err != nil {
t.Fatalf("FindRoute(%d, '', %s, %d): %s", unspecifiedNICID, test.remoteAddr, netProto, err)
} else if diff := cmp.Diff(r, test.expectedRoute, cmpopts.IgnoreUnexported(r)); diff != "" {
} else if diff := cmp.Diff(r, test.expectedRoute, cmpopts.IgnoreUnexported(stack.Route{})); diff != "" {
t.Errorf("route mismatch (-want +got):\n%s", diff)
}
})
@@ -4091,10 +4091,12 @@ func TestFindRouteWithForwarding(t *testing.T) {
s.SetRouteTable([]tcpip.Route{{Destination: test.netCfg.remoteAddr.WithPrefix().Subnet(), NIC: nicID2}})
r, err := s.FindRoute(test.addrNIC, test.localAddr, test.netCfg.remoteAddr, test.netCfg.proto, false /* multicastLoop */)
if r != nil {
defer r.Release()
}
if err != test.findRouteErr {
t.Fatalf("FindRoute(%d, %s, %s, %d, false) = %s, want = %s", test.addrNIC, test.localAddr, test.netCfg.remoteAddr, test.netCfg.proto, err, test.findRouteErr)
}
defer r.Release()
if test.findRouteErr != nil {
return
+1 -1
View File
@@ -42,7 +42,7 @@ type fakeTransportEndpoint struct {
proto *fakeTransportProtocol
peerAddr tcpip.Address
route stack.Route
route *stack.Route
uniqueID uint64
// acceptQueue is non-nil iff bound.
+7 -4
View File
@@ -72,7 +72,7 @@ type endpoint struct {
// shutdownFlags represent the current shutdown state of the endpoint.
shutdownFlags tcpip.ShutdownFlags
state endpointState
route stack.Route `state:"manual"`
route *stack.Route `state:"manual"`
ttl uint8
stats tcpip.TransportEndpointStats `state:"nosave"`
// linger is used for SO_LINGER socket option.
@@ -132,7 +132,10 @@ func (e *endpoint) Close() {
}
e.rcvMu.Unlock()
e.route.Release()
if e.route != nil {
e.route.Release()
e.route = nil
}
// Update the state.
e.state = stateClosed
@@ -272,7 +275,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
var route *stack.Route
if to == nil {
route = &e.route
route = e.route
if route.IsResolutionRequired() {
// Promote lock to exclusive if using a shared route,
@@ -313,7 +316,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
}
defer r.Release()
route = &r
route = r
}
if route.IsResolutionRequired() {
+9 -7
View File
@@ -84,7 +84,7 @@ type endpoint struct {
bound bool
// route is the route to a remote network endpoint. It is set via
// Connect(), and is valid only when conneted is true.
route stack.Route `state:"manual"`
route *stack.Route `state:"manual"`
stats tcpip.TransportEndpointStats `state:"nosave"`
// linger is used for SO_LINGER socket option.
linger tcpip.LingerOption
@@ -173,9 +173,11 @@ func (e *endpoint) Close() {
e.rcvList.Remove(e.rcvList.Front())
}
if e.connected {
e.connected = false
if e.route != nil {
e.route.Release()
e.connected = false
e.route = nil
}
e.closed = true
@@ -299,7 +301,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
}
if e.route.IsResolutionRequired() {
savedRoute := &e.route
savedRoute := e.route
// Promote lock to exclusive if using a shared route,
// given that it may need to change in finishWrite.
e.mu.RUnlock()
@@ -307,7 +309,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
// Make sure that the route didn't change during the
// time we didn't hold the lock.
if !e.connected || savedRoute != &e.route {
if !e.connected || savedRoute != e.route {
e.mu.Unlock()
return 0, nil, tcpip.ErrInvalidEndpointState
}
@@ -317,7 +319,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
return n, ch, err
}
n, ch, err := e.finishWrite(payloadBytes, &e.route)
n, ch, err := e.finishWrite(payloadBytes, e.route)
e.mu.RUnlock()
return n, ch, err
}
@@ -338,7 +340,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
return 0, nil, err
}
n, ch, err := e.finishWrite(payloadBytes, &route)
n, ch, err := e.finishWrite(payloadBytes, route)
route.Release()
e.mu.RUnlock()
return n, ch, err
+7 -1
View File
@@ -73,7 +73,13 @@ func (e *endpoint) Resume(s *stack.Stack) {
// If the endpoint is connected, re-connect.
if e.connected {
var err *tcpip.Error
e.route, err = e.stack.FindRoute(e.RegisterNICID, e.BindAddr, e.route.RemoteAddress, e.NetProto, false)
// TODO(gvisor.dev/issue/4906): Properly restore the route with the right
// remote address. We used to pass e.remote.RemoteAddress which was
// effectively the empty address but since moving e.route to hold a pointer
// to a route instead of the route by value, we pass the empty address
// directly. Obviously this was always wrong since we should provide the
// remote address we were connected to, to properly restore the route.
e.route, err = e.stack.FindRoute(e.RegisterNICID, e.BindAddr, "", e.NetProto, false)
if err != nil {
panic(err)
}
+1 -1
View File
@@ -599,7 +599,7 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) *tcpip.Er
ack: s.sequenceNumber + 1,
rcvWnd: ctx.rcvWnd,
}
if err := e.sendSynTCP(&route, fields, synOpts); err != nil {
if err := e.sendSynTCP(route, fields, synOpts); err != nil {
return err
}
e.stack.Stats().TCP.ListenOverflowSynCookieSent.Increment()
+5 -5
View File
@@ -300,7 +300,7 @@ func (h *handshake) synSentState(s *segment) *tcpip.Error {
if ttl == 0 {
ttl = h.ep.route.DefaultTTL()
}
h.ep.sendSynTCP(&h.ep.route, tcpFields{
h.ep.sendSynTCP(h.ep.route, tcpFields{
id: h.ep.ID,
ttl: ttl,
tos: h.ep.sendTOS,
@@ -361,7 +361,7 @@ func (h *handshake) synRcvdState(s *segment) *tcpip.Error {
SACKPermitted: h.ep.sackPermitted,
MSS: h.ep.amss,
}
h.ep.sendSynTCP(&h.ep.route, tcpFields{
h.ep.sendSynTCP(h.ep.route, tcpFields{
id: h.ep.ID,
ttl: h.ep.ttl,
tos: h.ep.sendTOS,
@@ -547,7 +547,7 @@ func (h *handshake) start() *tcpip.Error {
}
h.sendSYNOpts = synOpts
h.ep.sendSynTCP(&h.ep.route, tcpFields{
h.ep.sendSynTCP(h.ep.route, tcpFields{
id: h.ep.ID,
ttl: h.ep.ttl,
tos: h.ep.sendTOS,
@@ -596,7 +596,7 @@ func (h *handshake) complete() *tcpip.Error {
// the connection with another ACK or data (as ACKs are never
// retransmitted on their own).
if h.active || !h.acked || h.deferAccept != 0 && time.Since(h.startTime) > h.deferAccept {
h.ep.sendSynTCP(&h.ep.route, tcpFields{
h.ep.sendSynTCP(h.ep.route, tcpFields{
id: h.ep.ID,
ttl: h.ep.ttl,
tos: h.ep.sendTOS,
@@ -939,7 +939,7 @@ func (e *endpoint) sendRaw(data buffer.VectorisedView, flags byte, seq, ack seqn
sackBlocks = e.sack.Blocks[:e.sack.NumBlocks]
}
options := e.makeOptions(sackBlocks)
err := e.sendTCP(&e.route, tcpFields{
err := e.sendTCP(e.route, tcpFields{
id: e.ID,
ttl: e.ttl,
tos: e.sendTOS,
+7 -3
View File
@@ -440,7 +440,7 @@ type endpoint struct {
isPortReserved bool `state:"manual"`
isRegistered bool `state:"manual"`
boundNICID tcpip.NICID
route stack.Route `state:"manual"`
route *stack.Route `state:"manual"`
ttl uint8
v6only bool
isConnectNotified bool
@@ -705,7 +705,7 @@ func (e *endpoint) UniqueID() uint64 {
//
// If userMSS is non-zero and is not greater than the maximum possible MSS for
// r, it will be used; otherwise, the maximum possible MSS will be used.
func calculateAdvertisedMSS(userMSS uint16, r stack.Route) uint16 {
func calculateAdvertisedMSS(userMSS uint16, r *stack.Route) uint16 {
// The maximum possible MSS is dependent on the route.
// TODO(b/143359391): Respect TCP Min and Max size.
maxMSS := uint16(r.MTU() - header.TCPMinimumSize)
@@ -1173,7 +1173,11 @@ func (e *endpoint) cleanupLocked() {
e.boundPortFlags = ports.Flags{}
e.boundDest = tcpip.FullAddress{}
e.route.Release()
if e.route != nil {
e.route.Release()
e.route = nil
}
e.stack.CompleteTransportEndpointCleanup(e)
tcpip.DeleteDanglingEndpoint(e)
}
+1 -1
View File
@@ -250,7 +250,7 @@ func replyWithReset(stack *stack.Stack, s *segment, tos, ttl uint8) *tcpip.Error
ttl = route.DefaultTTL()
}
return sendTCP(&route, tcpFields{
return sendTCP(route, tcpFields{
id: s.id,
ttl: ttl,
tos: tos,
+10 -7
View File
@@ -99,7 +99,7 @@ type endpoint struct {
sndBufSize int
sndBufSizeMax int
state EndpointState
route stack.Route `state:"manual"`
route *stack.Route `state:"manual"`
dstPort uint16
v6only bool
ttl uint8
@@ -259,7 +259,10 @@ func (e *endpoint) Close() {
}
e.rcvMu.Unlock()
e.route.Release()
if e.route != nil {
e.route.Release()
e.route = nil
}
// Update the state.
e.state = StateClosed
@@ -368,7 +371,7 @@ func (e *endpoint) prepareForWrite(to *tcpip.FullAddress) (retry bool, err *tcpi
// connectRoute establishes a route to the specified interface or the
// configured multicast interface if no interface is specified and the
// specified address is a multicast address.
func (e *endpoint) connectRoute(nicID tcpip.NICID, addr tcpip.FullAddress, netProto tcpip.NetworkProtocolNumber) (stack.Route, tcpip.NICID, *tcpip.Error) {
func (e *endpoint) connectRoute(nicID tcpip.NICID, addr tcpip.FullAddress, netProto tcpip.NetworkProtocolNumber) (*stack.Route, tcpip.NICID, *tcpip.Error) {
localAddr := e.ID.LocalAddress
if e.isBroadcastOrMulticast(nicID, netProto, localAddr) {
// A packet can only originate from a unicast address (i.e., an interface).
@@ -387,7 +390,7 @@ func (e *endpoint) connectRoute(nicID tcpip.NICID, addr tcpip.FullAddress, netPr
// Find a route to the desired destination.
r, err := e.stack.FindRoute(nicID, localAddr, addr.Addr, netProto, e.multicastLoop)
if err != nil {
return stack.Route{}, 0, err
return nil, 0, err
}
return r, nicID, nil
}
@@ -459,7 +462,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
var resolve func(waker *sleep.Waker) (ch <-chan struct{}, err *tcpip.Error)
var dstPort uint16
if to == nil {
route = &e.route
route = e.route
dstPort = e.dstPort
resolve = func(waker *sleep.Waker) (ch <-chan struct{}, err *tcpip.Error) {
// Promote lock to exclusive if using a shared route, given that it may
@@ -512,7 +515,7 @@ func (e *endpoint) write(p tcpip.Payloader, opts tcpip.WriteOptions) (int64, <-c
}
defer r.Release()
route = &r
route = r
dstPort = dst.Port
resolve = route.Resolve
}
@@ -1083,7 +1086,7 @@ func (e *endpoint) Disconnect() *tcpip.Error {
e.ID = id
e.boundBindToDevice = btd
e.route.Release()
e.route = stack.Route{}
e.route = nil
e.dstPort = 0
return nil