Skip to content

Commit 805ec72

Browse files
nybidarigvisor-bot
authored andcommitted
Return NetworkUnreachable instead of HostUnreachable when routes are not found
Updates #8105 PiperOrigin-RevId: 791875726
1 parent 7048e11 commit 805ec72

File tree

9 files changed

+38
-48
lines changed

9 files changed

+38
-48
lines changed

pkg/tcpip/adapters/gonet/gonet_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -825,8 +825,8 @@ func TestTCPDialError(t *testing.T) {
825825

826826
switch _, err := DialTCP(s, addr, ipv4.ProtocolNumber); err := err.(type) {
827827
case *net.OpError:
828-
if err.Err.Error() != (&tcpip.ErrHostUnreachable{}).String() {
829-
t.Errorf("got DialTCP() = %s, want = %s", err, &tcpip.ErrHostUnreachable{})
828+
if err.Err.Error() != (&tcpip.ErrNetworkUnreachable{}).String() {
829+
t.Errorf("got DialTCP() = %s, want = %s", err, &tcpip.ErrNetworkUnreachable{})
830830
}
831831
default:
832832
t.Errorf("got DialTCP(...) = %v, want %s", err, &tcpip.ErrHostUnreachable{})

pkg/tcpip/network/ipv4/ipv4.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -796,9 +796,7 @@ func (e *endpoint) forwardUnicastPacket(pkt *stack.PacketBuffer) ip.ForwardingEr
796796
r, err := stk.FindRoute(0, tcpip.Address{}, dstAddr, ProtocolNumber, false /* multicastLoop */)
797797
switch err.(type) {
798798
case nil:
799-
// TODO(https://gvisor.dev/issues/8105): We should not observe ErrHostUnreachable from route
800-
// lookups.
801-
case *tcpip.ErrHostUnreachable, *tcpip.ErrNetworkUnreachable:
799+
case *tcpip.ErrNetworkUnreachable:
802800
// We return the original error rather than the result of returning
803801
// the ICMP packet because the original error is more relevant to
804802
// the caller.

pkg/tcpip/network/ipv4/ipv4_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func TestExcludeBroadcast(t *testing.T) {
118118
// Cannot connect using a broadcast address as the source.
119119
{
120120
err := ep.Connect(randomAddr)
121-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
121+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
122122
t.Errorf("got ep.Connect(...) = %v, want = %v", err, &tcpip.ErrHostUnreachable{})
123123
}
124124
}

pkg/tcpip/network/ipv6/ipv6.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,9 +1018,7 @@ func (e *endpoint) forwardUnicastPacket(pkt *stack.PacketBuffer) ip.ForwardingEr
10181018
r, err := stk.FindRoute(0, tcpip.Address{}, dstAddr, ProtocolNumber, false /* multicastLoop */)
10191019
switch err.(type) {
10201020
case nil:
1021-
// TODO(https://gvisor.dev/issues/8105): We should not observe ErrHostUnreachable from route
1022-
// lookups.
1023-
case *tcpip.ErrHostUnreachable, *tcpip.ErrNetworkUnreachable:
1021+
case *tcpip.ErrNetworkUnreachable:
10241022
// We return the original error rather than the result of returning the
10251023
// ICMP packet because the original error is more relevant to the caller.
10261024
_ = e.protocol.returnError(&icmpReasonNetUnreachable{}, pkt, false /* deliveredLocally */)

pkg/tcpip/stack/ndp_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -612,17 +612,17 @@ func TestDADResolve(t *testing.T) {
612612
// tentative address.
613613
{
614614
r, err := s.FindRoute(nicID, tcpip.Address{}, addr2, header.IPv6ProtocolNumber, false)
615-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
616-
t.Errorf("got FindRoute(%d, '', %s, %d, false) = (%+v, %v), want = (_, %s)", nicID, addr2, header.IPv6ProtocolNumber, r, err, &tcpip.ErrHostUnreachable{})
615+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
616+
t.Errorf("got FindRoute(%d, '', %s, %d, false) = (%+v, %v), want = (_, %s)", nicID, addr2, header.IPv6ProtocolNumber, r, err, &tcpip.ErrNetworkUnreachable{})
617617
}
618618
if r != nil {
619619
r.Release()
620620
}
621621
}
622622
{
623623
r, err := s.FindRoute(nicID, addr1, addr2, header.IPv6ProtocolNumber, false)
624-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
625-
t.Errorf("got FindRoute(%d, %s, %s, %d, false) = (%+v, %v), want = (_, %s)", nicID, addr1, addr2, header.IPv6ProtocolNumber, r, err, &tcpip.ErrHostUnreachable{})
624+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
625+
t.Errorf("got FindRoute(%d, %s, %s, %d, false) = (%+v, %v), want = (_, %s)", nicID, addr1, addr2, header.IPv6ProtocolNumber, r, err, &tcpip.ErrNetworkUnreachable{})
626626
}
627627
if r != nil {
628628
r.Release()
@@ -3799,7 +3799,7 @@ func TestAutoGenAddrJobDeprecation(t *testing.T) {
37993799

38003800
{
38013801
err := ep.Connect(dstAddr)
3802-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
3802+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
38033803
t.Errorf("got ep.Connect(%+v) = %s, want = %s", dstAddr, err, &tcpip.ErrHostUnreachable{})
38043804
}
38053805
}

pkg/tcpip/stack/stack.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1598,8 +1598,7 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
15981598
}
15991599
}
16001600

1601-
// TODO(https://gvisor.dev/issues/8105): This should be ErrNetworkUnreachable.
1602-
return nil, &tcpip.ErrHostUnreachable{}
1601+
return nil, &tcpip.ErrNetworkUnreachable{}
16031602
}
16041603

16051604
if id == 0 {
@@ -1612,13 +1611,11 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
16121611
}
16131612

16141613
if needRoute {
1615-
// TODO(https://gvisor.dev/issues/8105): This should be ErrNetworkUnreachable.
1616-
return nil, &tcpip.ErrHostUnreachable{}
1614+
return nil, &tcpip.ErrNetworkUnreachable{}
16171615
}
16181616
if header.IsV6LoopbackAddress(remoteAddr) {
16191617
return nil, &tcpip.ErrBadLocalAddress{}
16201618
}
1621-
// TODO(https://gvisor.dev/issues/8105): This should be ErrNetworkUnreachable.
16221619
return nil, &tcpip.ErrNetworkUnreachable{}
16231620
}
16241621

pkg/tcpip/stack/stack_test.go

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -854,8 +854,8 @@ func testRoute(t *testing.T, s *stack.Stack, nic tcpip.NICID, srcAddr, dstAddr,
854854

855855
func testNoRoute(t *testing.T, s *stack.Stack, nic tcpip.NICID, srcAddr, dstAddr tcpip.Address) {
856856
_, err := s.FindRoute(nic, srcAddr, dstAddr, fakeNetNumber, false /* multicastLoop */)
857-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
858-
t.Fatalf("FindRoute returned unexpected error, got = %v, want = %s", err, &tcpip.ErrHostUnreachable{})
857+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
858+
t.Fatalf("FindRoute returned unexpected error, got = %v, want = %s", err, &tcpip.ErrNetworkUnreachable{})
859859
}
860860
}
861861

@@ -1383,7 +1383,7 @@ func TestAddressRemoval(t *testing.T) {
13831383
t.Fatal("RemoveAddress failed:", err)
13841384
}
13851385
testFailingRecv(t, fakeNet, localAddrByte, ep, buf)
1386-
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrHostUnreachable{})
1386+
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrNetworkUnreachable{})
13871387

13881388
// Check that removing the same address fails.
13891389
err := s.RemoveAddress(1, localAddr)
@@ -1443,7 +1443,7 @@ func TestAddressRemovalWithRouteHeld(t *testing.T) {
14431443
}
14441444
testFailingRecv(t, fakeNet, localAddrByte, ep, buf)
14451445
testFailingSend(t, r, nil, &tcpip.ErrInvalidEndpointState{})
1446-
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrHostUnreachable{})
1446+
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrNetworkUnreachable{})
14471447

14481448
// Check that removing the same address fails.
14491449
{
@@ -1546,7 +1546,7 @@ func TestEndpointExpiration(t *testing.T) {
15461546
// FIXME(b/139841518):Spoofing doesn't work if there is no primary address.
15471547
// testSendTo(t, s, remoteAddr, ep, nil)
15481548
} else {
1549-
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrHostUnreachable{})
1549+
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrNetworkUnreachable{})
15501550
}
15511551

15521552
// 2. Add Address, everything should work.
@@ -1581,7 +1581,7 @@ func TestEndpointExpiration(t *testing.T) {
15811581
// FIXME(b/139841518):Spoofing doesn't work if there is no primary address.
15821582
// testSendTo(t, s, remoteAddr, ep, nil)
15831583
} else {
1584-
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrHostUnreachable{})
1584+
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrNetworkUnreachable{})
15851585
}
15861586

15871587
// 4. Add Address back, everything should work again.
@@ -1621,7 +1621,7 @@ func TestEndpointExpiration(t *testing.T) {
16211621
testSendTo(t, s, string(remoteAddr.AsSlice()), ep, nil)
16221622
} else {
16231623
testFailingSend(t, r, nil, &tcpip.ErrInvalidEndpointState{})
1624-
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrHostUnreachable{})
1624+
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrNetworkUnreachable{})
16251625
}
16261626

16271627
// 7. Add Address back, everything should work again.
@@ -1657,7 +1657,7 @@ func TestEndpointExpiration(t *testing.T) {
16571657
// FIXME(b/139841518):Spoofing doesn't work if there is no primary address.
16581658
// testSendTo(t, s, remoteAddr, ep, nil)
16591659
} else {
1660-
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrHostUnreachable{})
1660+
testFailingSendTo(t, s, remoteAddr, nil, &tcpip.ErrNetworkUnreachable{})
16611661
}
16621662
})
16631663
}
@@ -1700,8 +1700,8 @@ func TestPromiscuousMode(t *testing.T) {
17001700

17011701
// Check that we can't get a route as there is no local address.
17021702
_, err := s.FindRoute(0, tcpip.Address{}, tcpip.AddrFromSlice([]byte("\x02\x00\x00\x00")), fakeNetNumber, false /* multicastLoop */)
1703-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
1704-
t.Fatalf("FindRoute returned unexpected error: got = %v, want = %s", err, &tcpip.ErrHostUnreachable{})
1703+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
1704+
t.Fatalf("FindRoute returned unexpected error: got = %v, want = %s", err, &tcpip.ErrNetworkUnreachable{})
17051705
}
17061706

17071707
// Set promiscuous mode to false, then check that packet can't be
@@ -1922,7 +1922,7 @@ func TestSpoofingNoAddress(t *testing.T) {
19221922
t.Errorf("FindRoute succeeded with route %+v when it should have failed", r)
19231923
}
19241924
// Sending a packet fails.
1925-
testFailingSendTo(t, s, dstAddr, nil, &tcpip.ErrHostUnreachable{})
1925+
testFailingSendTo(t, s, dstAddr, nil, &tcpip.ErrNetworkUnreachable{})
19261926

19271927
// With address spoofing enabled, FindRoute permits any address to be used
19281928
// as the source.
@@ -2130,9 +2130,6 @@ func TestMulticastOrIPv6LinkLocalNeedsNoRoute(t *testing.T) {
21302130
}
21312131

21322132
var want tcpip.Error = &tcpip.ErrNetworkUnreachable{}
2133-
if tc.routeNeeded {
2134-
want = &tcpip.ErrHostUnreachable{}
2135-
}
21362133

21372134
// If there is no endpoint, it won't work.
21382135
address := tcpip.AddrFromSlice([]byte(tc.address))
@@ -2153,8 +2150,8 @@ func TestMulticastOrIPv6LinkLocalNeedsNoRoute(t *testing.T) {
21532150

21542151
if r, err := s.FindRoute(1, anyAddr, address, fakeNetNumber, false /* multicastLoop */); tc.routeNeeded {
21552152
// Route table is empty but we need a route, this should cause an error.
2156-
if _, ok := err.(*tcpip.ErrHostUnreachable); !ok {
2157-
t.Fatalf("got FindRoute(1, %v, %v, %v) = %v, want = %v", anyAddr, address, fakeNetNumber, err, &tcpip.ErrHostUnreachable{})
2153+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); !ok {
2154+
t.Fatalf("got FindRoute(1, %v, %v, %v) = %v, want = %v", anyAddr, address, fakeNetNumber, err, &tcpip.ErrNetworkUnreachable{})
21582155
}
21592156
} else {
21602157
if err != nil {
@@ -4548,7 +4545,7 @@ func TestFindRouteWithForwarding(t *testing.T) {
45484545
forwardingEnabled: false,
45494546
addrNIC: nicID1,
45504547
localAddrWithPrefix: fakeNetCfg.nic2AddrWithPrefix,
4551-
findRouteErr: &tcpip.ErrHostUnreachable{},
4548+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
45524549
dependentOnForwarding: false,
45534550
},
45544551
{
@@ -4557,7 +4554,7 @@ func TestFindRouteWithForwarding(t *testing.T) {
45574554
forwardingEnabled: true,
45584555
addrNIC: nicID1,
45594556
localAddrWithPrefix: fakeNetCfg.nic2AddrWithPrefix,
4560-
findRouteErr: &tcpip.ErrHostUnreachable{},
4557+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
45614558
dependentOnForwarding: false,
45624559
},
45634560
{
@@ -4566,7 +4563,7 @@ func TestFindRouteWithForwarding(t *testing.T) {
45664563
forwardingEnabled: false,
45674564
addrNIC: nicID1,
45684565
localAddrWithPrefix: fakeNetCfg.nic1AddrWithPrefix,
4569-
findRouteErr: &tcpip.ErrHostUnreachable{},
4566+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
45704567
dependentOnForwarding: false,
45714568
},
45724569
{
@@ -4602,7 +4599,7 @@ func TestFindRouteWithForwarding(t *testing.T) {
46024599
forwardingEnabled: false,
46034600
addrNIC: nicID2,
46044601
localAddrWithPrefix: fakeNetCfg.nic1AddrWithPrefix,
4605-
findRouteErr: &tcpip.ErrHostUnreachable{},
4602+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46064603
dependentOnForwarding: false,
46074604
},
46084605
{
@@ -4611,7 +4608,7 @@ func TestFindRouteWithForwarding(t *testing.T) {
46114608
forwardingEnabled: true,
46124609
addrNIC: nicID2,
46134610
localAddrWithPrefix: fakeNetCfg.nic1AddrWithPrefix,
4614-
findRouteErr: &tcpip.ErrHostUnreachable{},
4611+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46154612
dependentOnForwarding: false,
46164613
},
46174614
{
@@ -4635,7 +4632,7 @@ func TestFindRouteWithForwarding(t *testing.T) {
46354632
netCfg: fakeNetCfg,
46364633
forwardingEnabled: false,
46374634
localAddrWithPrefix: fakeNetCfg.nic1AddrWithPrefix,
4638-
findRouteErr: &tcpip.ErrHostUnreachable{},
4635+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46394636
dependentOnForwarding: false,
46404637
},
46414638
{
@@ -4651,31 +4648,31 @@ func TestFindRouteWithForwarding(t *testing.T) {
46514648
netCfg: ipv6LinkLocalNIC1WithGlobalRemote,
46524649
forwardingEnabled: false,
46534650
addrNIC: nicID1,
4654-
findRouteErr: &tcpip.ErrHostUnreachable{},
4651+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46554652
dependentOnForwarding: false,
46564653
},
46574654
{
46584655
name: "forwarding enabled and specified NIC only has link-local addr with route on different NIC",
46594656
netCfg: ipv6LinkLocalNIC1WithGlobalRemote,
46604657
forwardingEnabled: true,
46614658
addrNIC: nicID1,
4662-
findRouteErr: &tcpip.ErrHostUnreachable{},
4659+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46634660
dependentOnForwarding: false,
46644661
},
46654662
{
46664663
name: "forwarding disabled and link-local local addr with route on different NIC",
46674664
netCfg: ipv6LinkLocalNIC1WithGlobalRemote,
46684665
forwardingEnabled: false,
46694666
localAddrWithPrefix: ipv6LinkLocalNIC1WithGlobalRemote.nic1AddrWithPrefix,
4670-
findRouteErr: &tcpip.ErrHostUnreachable{},
4667+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46714668
dependentOnForwarding: false,
46724669
},
46734670
{
46744671
name: "forwarding enabled and link-local local addr with route on same NIC",
46754672
netCfg: ipv6LinkLocalNIC1WithGlobalRemote,
46764673
forwardingEnabled: true,
46774674
localAddrWithPrefix: ipv6LinkLocalNIC1WithGlobalRemote.nic1AddrWithPrefix,
4678-
findRouteErr: &tcpip.ErrHostUnreachable{},
4675+
findRouteErr: &tcpip.ErrNetworkUnreachable{},
46794676
dependentOnForwarding: false,
46804677
},
46814678
{
@@ -5793,7 +5790,7 @@ func TestFindRoute(t *testing.T) {
57935790
false, /* multicastLoop */
57945791
)
57955792
if err != nil {
5796-
if _, ok := err.(*tcpip.ErrHostUnreachable); query.wantErr && ok {
5793+
if _, ok := err.(*tcpip.ErrNetworkUnreachable); query.wantErr && ok {
57975794
return
57985795
}
57995796
t.Fatalf("FoundRoute failed: %v", err)

pkg/tcpip/tests/integration/route_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ func TestLocalUDP(t *testing.T) {
282282
{
283283
name: "Unassigned local address",
284284
addAddress: false,
285-
expectedWriteErr: &tcpip.ErrHostUnreachable{},
285+
expectedWriteErr: &tcpip.ErrNetworkUnreachable{},
286286
},
287287
{
288288
name: "Assigned local address",

pkg/tcpip/transport/tcp/test/e2e/tcp_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func TestActiveFailedConnectionAttemptIncrement(t *testing.T) {
260260

261261
{
262262
err := c.EP.Connect(tcpip.FullAddress{NIC: 2, Addr: context.TestAddr, Port: context.TestPort})
263-
if d := cmp.Diff(&tcpip.ErrHostUnreachable{}, err); d != "" {
263+
if d := cmp.Diff(&tcpip.ErrNetworkUnreachable{}, err); d != "" {
264264
t.Errorf("c.EP.Connect(...) mismatch (-want +got):\n%s", d)
265265
}
266266
}

0 commit comments

Comments
 (0)