From 289757e903cb8dd837e1e89d0ea07935e154e430 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Mon, 4 Dec 2023 14:28:09 -0800 Subject: [PATCH] netstack: support source hints in routes PiperOrigin-RevId: 587841841 --- pkg/tcpip/network/ipv4/igmp.go | 2 +- pkg/tcpip/network/ipv4/ipv4.go | 8 +-- pkg/tcpip/network/ipv6/icmp.go | 2 +- pkg/tcpip/network/ipv6/ipv6.go | 10 ++-- pkg/tcpip/network/ipv6/ndp.go | 2 +- pkg/tcpip/stack/addressable_endpoint_state.go | 13 +++-- pkg/tcpip/stack/iptables_targets.go | 2 +- pkg/tcpip/stack/nic.go | 4 +- pkg/tcpip/stack/registration.go | 2 +- pkg/tcpip/stack/stack.go | 20 +++---- pkg/tcpip/stack/stack_test.go | 53 +++++++++++++++++-- pkg/tcpip/tcpip.go | 4 ++ 12 files changed, 88 insertions(+), 34 deletions(-) diff --git a/pkg/tcpip/network/ipv4/igmp.go b/pkg/tcpip/network/ipv4/igmp.go index 664f6e2f3..d9eb62316 100644 --- a/pkg/tcpip/network/ipv4/igmp.go +++ b/pkg/tcpip/network/ipv4/igmp.go @@ -521,7 +521,7 @@ func (igmp *igmpState) writePacketInner(buf *buffer.View, reportStat tcpip.Multi }) defer pkt.DecRef() - addressEndpoint := igmp.ep.acquireOutgoingPrimaryAddressRLocked(destAddress, false /* allowExpired */) + addressEndpoint := igmp.ep.acquireOutgoingPrimaryAddressRLocked(destAddress, tcpip.Address{} /* srcHint */, false /* allowExpired */) if addressEndpoint == nil { return false, nil } diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index 8aa4a92a7..6dfb07fcb 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -1423,18 +1423,18 @@ func (e *endpoint) AcquireAssignedAddress(localAddr tcpip.Address, allowTemp boo } // AcquireOutgoingPrimaryAddress implements stack.AddressableEndpoint. -func (e *endpoint) AcquireOutgoingPrimaryAddress(remoteAddr tcpip.Address, allowExpired bool) stack.AddressEndpoint { +func (e *endpoint) AcquireOutgoingPrimaryAddress(remoteAddr, srcHint tcpip.Address, allowExpired bool) stack.AddressEndpoint { e.mu.RLock() defer e.mu.RUnlock() - return e.acquireOutgoingPrimaryAddressRLocked(remoteAddr, allowExpired) + return e.acquireOutgoingPrimaryAddressRLocked(remoteAddr, srcHint, allowExpired) } // acquireOutgoingPrimaryAddressRLocked is like AcquireOutgoingPrimaryAddress // but with locking requirements // // +checklocksread:e.mu -func (e *endpoint) acquireOutgoingPrimaryAddressRLocked(remoteAddr tcpip.Address, allowExpired bool) stack.AddressEndpoint { - return e.addressableEndpointState.AcquireOutgoingPrimaryAddress(remoteAddr, allowExpired) +func (e *endpoint) acquireOutgoingPrimaryAddressRLocked(remoteAddr, srcHint tcpip.Address, allowExpired bool) stack.AddressEndpoint { + return e.addressableEndpointState.AcquireOutgoingPrimaryAddress(remoteAddr, srcHint, allowExpired) } // PrimaryAddresses implements stack.AddressableEndpoint. diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index c5ad41596..c8fb2df86 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -914,7 +914,7 @@ func (e *endpoint) LinkAddressRequest(targetAddr, localAddr tcpip.Address, remot if localAddr.BitLen() == 0 { // Find an address that we can use as our source address. - addressEndpoint := e.AcquireOutgoingPrimaryAddress(remoteAddr, false /* allowExpired */) + addressEndpoint := e.AcquireOutgoingPrimaryAddress(remoteAddr, tcpip.Address{} /* srcHint */, false /* allowExpired */) if addressEndpoint == nil { return &tcpip.ErrNetworkUnreachable{} } diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index e8d02ea6b..c7e546e97 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/pkg/tcpip/network/ipv6/ipv6.go @@ -2051,10 +2051,10 @@ func (e *endpoint) acquireAddressOrCreateTempLocked(localAddr tcpip.Address, all } // AcquireOutgoingPrimaryAddress implements stack.AddressableEndpoint. -func (e *endpoint) AcquireOutgoingPrimaryAddress(remoteAddr tcpip.Address, allowExpired bool) stack.AddressEndpoint { +func (e *endpoint) AcquireOutgoingPrimaryAddress(remoteAddr, srcHint tcpip.Address, allowExpired bool) stack.AddressEndpoint { e.mu.RLock() defer e.mu.RUnlock() - return e.acquireOutgoingPrimaryAddressRLocked(remoteAddr, allowExpired) + return e.acquireOutgoingPrimaryAddressRLocked(remoteAddr, srcHint, allowExpired) } // getLinkLocalAddressRLocked returns a link-local address from the primary list @@ -2081,7 +2081,9 @@ func (e *endpoint) getLinkLocalAddressRLocked() tcpip.Address { // but with locking requirements. // // Precondition: e.mu must be read locked. -func (e *endpoint) acquireOutgoingPrimaryAddressRLocked(remoteAddr tcpip.Address, allowExpired bool) stack.AddressEndpoint { +func (e *endpoint) acquireOutgoingPrimaryAddressRLocked(remoteAddr, srcHint tcpip.Address, allowExpired bool) stack.AddressEndpoint { + // TODO(b/309216156): Support IPv6 hints. + // addrCandidate is a candidate for Source Address Selection, as per // RFC 6724 section 5. type addrCandidate struct { @@ -2094,7 +2096,7 @@ func (e *endpoint) acquireOutgoingPrimaryAddressRLocked(remoteAddr tcpip.Address } if remoteAddr.BitLen() == 0 { - return e.mu.addressableEndpointState.AcquireOutgoingPrimaryAddress(remoteAddr, allowExpired) + return e.mu.addressableEndpointState.AcquireOutgoingPrimaryAddress(remoteAddr, srcHint, allowExpired) } // Create a candidate set of available addresses we can potentially use as a diff --git a/pkg/tcpip/network/ipv6/ndp.go b/pkg/tcpip/network/ipv6/ndp.go index 274ba5057..b44879bc7 100644 --- a/pkg/tcpip/network/ipv6/ndp.go +++ b/pkg/tcpip/network/ipv6/ndp.go @@ -1837,7 +1837,7 @@ func (ndp *ndpState) startSolicitingRouters() { // the unspecified address if no address is assigned // to the sending interface. localAddr := header.IPv6Any - if addressEndpoint := ndp.ep.AcquireOutgoingPrimaryAddress(header.IPv6AllRoutersLinkLocalMulticastAddress, false); addressEndpoint != nil { + if addressEndpoint := ndp.ep.AcquireOutgoingPrimaryAddress(header.IPv6AllRoutersLinkLocalMulticastAddress, tcpip.Address{} /* srcHint */, false); addressEndpoint != nil { localAddr = addressEndpoint.AddressWithPrefix().Address addressEndpoint.DecRef() } diff --git a/pkg/tcpip/stack/addressable_endpoint_state.go b/pkg/tcpip/stack/addressable_endpoint_state.go index f472bbf0a..815c8b121 100644 --- a/pkg/tcpip/stack/addressable_endpoint_state.go +++ b/pkg/tcpip/stack/addressable_endpoint_state.go @@ -434,7 +434,7 @@ func (a *AddressableEndpointState) MainAddress() tcpip.AddressWithPrefix { a.mu.RLock() defer a.mu.RUnlock() - ep := a.acquirePrimaryAddressRLocked(tcpip.Address{}, func(ep *addressState) bool { + ep := a.acquirePrimaryAddressRLocked(tcpip.Address{}, tcpip.Address{} /* srcHint */, func(ep *addressState) bool { switch kind := ep.GetKind(); kind { case Permanent: return a.networkEndpoint.Enabled() || !a.options.HiddenWhileDisabled @@ -462,7 +462,7 @@ func (a *AddressableEndpointState) MainAddress() tcpip.AddressWithPrefix { // valid according to isValid. // // +checklocksread:a.mu -func (a *AddressableEndpointState) acquirePrimaryAddressRLocked(remoteAddr tcpip.Address, isValid func(*addressState) bool) *addressState { +func (a *AddressableEndpointState) acquirePrimaryAddressRLocked(remoteAddr, srcHint tcpip.Address, isValid func(*addressState) bool) *addressState { // TODO: Move this out into IPv4-specific code. // IPv6 handles source IP selection elsewhere. We have to do source // selection only for IPv4, in which case ep is never deprecated. Thus @@ -474,6 +474,11 @@ func (a *AddressableEndpointState) acquirePrimaryAddressRLocked(remoteAddr tcpip if !isValid(state) { continue } + // Source hint takes precedent over prefix matching. + if state.addr.Address == srcHint && srcHint != (tcpip.Address{}) { + best = state + break + } stateLen := state.addr.Address.MatchingPrefix(remoteAddr) if best == nil || bestLen < stateLen { best = state @@ -618,11 +623,11 @@ func (a *AddressableEndpointState) AcquireAssignedAddress(localAddr tcpip.Addres } // AcquireOutgoingPrimaryAddress implements AddressableEndpoint. -func (a *AddressableEndpointState) AcquireOutgoingPrimaryAddress(remoteAddr tcpip.Address, allowExpired bool) AddressEndpoint { +func (a *AddressableEndpointState) AcquireOutgoingPrimaryAddress(remoteAddr tcpip.Address, srcHint tcpip.Address, allowExpired bool) AddressEndpoint { a.mu.Lock() defer a.mu.Unlock() - ep := a.acquirePrimaryAddressRLocked(remoteAddr, func(ep *addressState) bool { + ep := a.acquirePrimaryAddressRLocked(remoteAddr, srcHint, func(ep *addressState) bool { return ep.IsAssigned(allowExpired) }) diff --git a/pkg/tcpip/stack/iptables_targets.go b/pkg/tcpip/stack/iptables_targets.go index e3cedaf0d..9c9f21f51 100644 --- a/pkg/tcpip/stack/iptables_targets.go +++ b/pkg/tcpip/stack/iptables_targets.go @@ -380,7 +380,7 @@ func (mt *MasqueradeTarget) Action(pkt PacketBufferPtr, hook Hook, r *Route, add } // addressEP is expected to be set for the postrouting hook. - ep := addressEP.AcquireOutgoingPrimaryAddress(pkt.Network().DestinationAddress(), false /* allowExpired */) + ep := addressEP.AcquireOutgoingPrimaryAddress(pkt.Network().DestinationAddress(), tcpip.Address{} /* srcHint */, false /* allowExpired */) if ep == nil { // No address exists that we can use as a source address. return RuleDrop, 0 diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index 3e8a0a3ea..8bf61bf60 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -420,7 +420,7 @@ func (n *nic) Spoofing() bool { // primaryAddress returns an address that can be used to communicate with // remoteAddr. -func (n *nic) primaryEndpoint(protocol tcpip.NetworkProtocolNumber, remoteAddr tcpip.Address) AssignableAddressEndpoint { +func (n *nic) primaryEndpoint(protocol tcpip.NetworkProtocolNumber, remoteAddr, srcHint tcpip.Address) AssignableAddressEndpoint { ep := n.getNetworkEndpoint(protocol) if ep == nil { return nil @@ -431,7 +431,7 @@ func (n *nic) primaryEndpoint(protocol tcpip.NetworkProtocolNumber, remoteAddr t return nil } - return addressableEndpoint.AcquireOutgoingPrimaryAddress(remoteAddr, n.Spoofing()) + return addressableEndpoint.AcquireOutgoingPrimaryAddress(remoteAddr, srcHint, n.Spoofing()) } type getAddressBehaviour int diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go index fe8c8ad3b..d9950928c 100644 --- a/pkg/tcpip/stack/registration.go +++ b/pkg/tcpip/stack/registration.go @@ -681,7 +681,7 @@ type AddressableEndpoint interface { // The returned endpoint's reference count is incremented. // // Returns nil if a primary address is not available. - AcquireOutgoingPrimaryAddress(remoteAddr tcpip.Address, allowExpired bool) AddressEndpoint + AcquireOutgoingPrimaryAddress(remoteAddr, srcHint tcpip.Address, allowExpired bool) AddressEndpoint // PrimaryAddresses returns the primary addresses. PrimaryAddresses() []tcpip.AddressWithPrefix diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 699da4fe1..23ce2d2ed 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -1168,9 +1168,9 @@ func (s *Stack) GetMainNICAddress(id tcpip.NICID, protocol tcpip.NetworkProtocol return nic.PrimaryAddress(protocol) } -func (s *Stack) getAddressEP(nic *nic, localAddr, remoteAddr tcpip.Address, netProto tcpip.NetworkProtocolNumber) AssignableAddressEndpoint { +func (s *Stack) getAddressEP(nic *nic, localAddr, remoteAddr, srcHint tcpip.Address, netProto tcpip.NetworkProtocolNumber) AssignableAddressEndpoint { if localAddr.BitLen() == 0 { - return nic.primaryEndpoint(netProto, remoteAddr) + return nic.primaryEndpoint(netProto, remoteAddr, srcHint) } return nic.findEndpoint(netProto, localAddr, CanBePrimaryEndpoint) } @@ -1188,7 +1188,7 @@ func (s *Stack) NewRouteForMulticast(nicID tcpip.NICID, remoteAddr tcpip.Address return nil } - if addressEndpoint := s.getAddressEP(nic, tcpip.Address{} /* localAddr */, remoteAddr, netProto); addressEndpoint != nil { + if addressEndpoint := s.getAddressEP(nic, tcpip.Address{} /* localAddr */, remoteAddr, tcpip.Address{} /* srcHint */, netProto); addressEndpoint != nil { return constructAndValidateRoute(netProto, addressEndpoint, nic, nic, tcpip.Address{} /* gateway */, tcpip.Address{} /* localAddr */, remoteAddr, s.handleLocal, false /* multicastLoop */) } return nil @@ -1300,9 +1300,9 @@ func isNICForwarding(nic *nic, proto tcpip.NetworkProtocolNumber) bool { // endpoint. // // +checklocksread:s.mu -func (s *Stack) findRouteWithLocalAddrFromAnyInterfaceRLocked(outgoingNIC *nic, localAddr, remoteAddr, gateway tcpip.Address, netProto tcpip.NetworkProtocolNumber, multicastLoop bool) *Route { +func (s *Stack) findRouteWithLocalAddrFromAnyInterfaceRLocked(outgoingNIC *nic, localAddr, remoteAddr, srcHint, gateway tcpip.Address, netProto tcpip.NetworkProtocolNumber, multicastLoop bool) *Route { for _, aNIC := range s.nics { - addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, netProto) + addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, srcHint, netProto) if addressEndpoint == nil { continue } @@ -1350,7 +1350,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n // through the interface if the interface is valid and enabled. if id != 0 && !needRoute { if nic, ok := s.nics[id]; ok && nic.Enabled() { - if addressEndpoint := s.getAddressEP(nic, localAddr, remoteAddr, netProto); addressEndpoint != nil { + if addressEndpoint := s.getAddressEP(nic, localAddr, remoteAddr, tcpip.Address{} /* srcHint */, netProto); addressEndpoint != nil { return makeRoute( netProto, tcpip.Address{}, /* gateway */ @@ -1390,7 +1390,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n } if id == 0 || id == route.NIC { - if addressEndpoint := s.getAddressEP(nic, localAddr, remoteAddr, netProto); addressEndpoint != nil { + if addressEndpoint := s.getAddressEP(nic, localAddr, remoteAddr, route.SourceHint, netProto); addressEndpoint != nil { var gateway tcpip.Address if needRoute { gateway = route.Gateway @@ -1421,7 +1421,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n chosenRoute = route continue } - if r := s.findRouteWithLocalAddrFromAnyInterfaceRLocked(nic, localAddr, remoteAddr, route.Gateway, netProto, multicastLoop); r != nil { + if r := s.findRouteWithLocalAddrFromAnyInterfaceRLocked(nic, localAddr, remoteAddr, route.SourceHint, route.Gateway, netProto, multicastLoop); r != nil { return r } } @@ -1449,7 +1449,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n // Use the specified NIC to get the local address endpoint. if id != 0 { if aNIC, ok := s.nics[id]; ok { - if addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, netProto); addressEndpoint != nil { + if addressEndpoint := s.getAddressEP(aNIC, localAddr, remoteAddr, chosenRoute.SourceHint, netProto); addressEndpoint != nil { if r := constructAndValidateRoute(netProto, addressEndpoint, aNIC /* localAddressNIC */, nic /* outgoingNIC */, gateway, localAddr, remoteAddr, s.handleLocal, multicastLoop); r != nil { return r, nil } @@ -1463,7 +1463,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n if id == 0 { // If an interface is not specified, try to find a NIC that holds the local // address endpoint to construct a route. - if r := s.findRouteWithLocalAddrFromAnyInterfaceRLocked(nic, localAddr, remoteAddr, gateway, netProto, multicastLoop); r != nil { + if r := s.findRouteWithLocalAddrFromAnyInterfaceRLocked(nic, localAddr, remoteAddr, chosenRoute.SourceHint, gateway, netProto, multicastLoop); r != nil { return r, nil } } diff --git a/pkg/tcpip/stack/stack_test.go b/pkg/tcpip/stack/stack_test.go index e9ef5ded2..8ebcbb930 100644 --- a/pkg/tcpip/stack/stack_test.go +++ b/pkg/tcpip/stack/stack_test.go @@ -3582,7 +3582,7 @@ func TestIPv6SourceAddressSelectionScopeAndSameAddress(t *testing.T) { t.Fatal("network endpoint is not addressable") } - addressEP := addressableEndpoint.AcquireOutgoingPrimaryAddress(test.remoteAddr, false /* allowExpired */) + addressEP := addressableEndpoint.AcquireOutgoingPrimaryAddress(test.remoteAddr, tcpip.Address{} /* srcHint */, false /* allowExpired */) if addressEP == nil { t.Fatal("expected a non-nil address endpoint") } @@ -5565,6 +5565,7 @@ func TestFindRoute(t *testing.T) { gateway string subnet string nic tcpip.NICID + srcHint string } type query struct { name string @@ -5681,6 +5682,38 @@ func TestFindRoute(t *testing.T) { }, }, }, + { + name: "one NIC, multiple addresses, no gateways", + nics: []nic{{id: 1, addresses: []string{"169.254.169.1", "169.254.9.1"}}}, + routes: []route{ + {subnet: "10.0.0.0/8", nic: 1}, + }, + queries: []query{ + { + name: "match first", + remote: "10.132.0.4", + wantID: 1, + wantLocal: "169.254.169.1", + wantNextHop: "", // No gateway, so no next hop. + }, + }, + }, + { + name: "one NIC, multiple addresses, source hints, no gateways", + nics: []nic{{id: 1, addresses: []string{"169.254.169.1", "169.254.9.1", "10.132.1.1"}}}, + routes: []route{ + {subnet: "10.0.0.0/8", nic: 1, srcHint: "169.254.9.1"}, + }, + queries: []query{ + { + name: "match hint", + remote: "10.132.0.4", + wantID: 1, + wantLocal: "169.254.9.1", + wantNextHop: "", // No gateway, so no next hop. + }, + }, + }, } for _, stackConfig := range stacks { @@ -5713,11 +5746,17 @@ func TestFindRoute(t *testing.T) { // Setup the route table. var routeTable []tcpip.Route for _, route := range stackConfig.routes { - routeTable = append(routeTable, tcpip.Route{ + rt := tcpip.Route{ Destination: testutil.MustParseSubnet4(route.subnet), - Gateway: testutil.MustParse4(route.gateway), NIC: route.nic, - }) + } + if len(route.gateway) > 0 { + rt.Gateway = testutil.MustParse4(route.gateway) + } + if len(route.srcHint) > 0 { + rt.SourceHint = testutil.MustParse4(route.srcHint) + } + routeTable = append(routeTable, rt) } stk.SetRouteTable(routeTable) @@ -5742,7 +5781,11 @@ func TestFindRoute(t *testing.T) { if got, want := route.LocalAddress(), testutil.MustParse4(query.wantLocal); got != want { t.Errorf("got local address %s, but wanted %s", got, want) } - if got, want := route.NextHop(), testutil.MustParse4(query.wantNextHop); got != want { + var nextHop tcpip.Address + if len(query.wantNextHop) > 0 { + nextHop = testutil.MustParse4(query.wantNextHop) + } + if got, want := route.NextHop(), nextHop; got != want { t.Errorf("got next hop %s, but wanted %s", got, want) } }) diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index fa8462b60..9549819fa 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -1499,6 +1499,10 @@ type Route struct { // NIC is the id of the nic to be used if this row is viable. NIC NICID + + // SourceHint indicates a preferred source address to use when NICs + // have multiple addresses. + SourceHint Address } // String implements the fmt.Stringer interface.