diff --git a/ecs-agent/netlib/platform/isolated_linux.go b/ecs-agent/netlib/platform/isolated_linux.go index cfe0422c78c..a3f094f322b 100644 --- a/ecs-agent/netlib/platform/isolated_linux.go +++ b/ecs-agent/netlib/platform/isolated_linux.go @@ -18,6 +18,7 @@ import ( "fmt" "net" "path/filepath" + "time" "github.com/aws/amazon-ecs-agent/ecs-agent/logger" netlibdata "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/data" @@ -136,8 +137,44 @@ func (il *isolatedLinux) configureBranchENI(ctx context.Context, netNSPath strin return err } +var ( + // gatewayNeighborResolveTimeout bounds how long we wait for the task-netns + // kernel to ARP-resolve the VPC gateway's MAC. It is set to match the + // kernel's own worst-case resolution horizon for a new neighbor: + // mcast_solicit (default 3) x retrans_time_ms (default 1000ms) = ~3s + // (see man 7 arp, /proc/sys/net/ipv4/neigh//). + gatewayNeighborResolveTimeout = 3 * time.Second + // gatewayNeighborResolveInterval is the poll/probe interval while waiting. + gatewayNeighborResolveInterval = 100 * time.Millisecond +) + +// gatewayProbePort is an arbitrary UDP port used only to nudge the kernel into +// ARP-resolving the gateway; nothing is expected to listen there. +const gatewayProbePort = "9" + +// gatewayProbeFn prompts the kernel to resolve gwIP by emitting a single +// throwaway datagram from the current network namespace. It is a package var +// so unit tests can stub it. Errors are intentionally ignored: the datagram +// exists only to trigger ARP; the ARP reply (not the datagram's delivery) is +// what populates the neighbor table. +var gatewayProbeFn = func(gwIP net.IP) { + conn, err := net.DialTimeout("udp", net.JoinHostPort(gwIP.String(), gatewayProbePort), gatewayNeighborResolveInterval) + if err != nil { + return + } + _, _ = conn.Write([]byte{0}) + _ = conn.Close() +} + // addGatewayNeighbor installs a permanent ARP entry and /32 link-scope route // for the gateway in the task netns. +// +// The gateway MAC is resolved from *inside* the task netns, where the task ENI +// (carrying the task's own VPC IP) has L2 reachability to the gateway. On ECS +// managed instances the host instance's subnet may or may not match the task's +// subnet; when it differs, the host root netns ARP cache does not contain the +// task's gateway. Resolving in the task netns works in both cases, regardless +// of the host's subnet. func (il *isolatedLinux) addGatewayNeighbor(netNSPath string, eni *networkinterface.NetworkInterface) error { // IPv6-only interfaces have no IPv4 gateway to pre-resolve, even when the // payload carries a subnet gateway IPv4 address. The guest resolves the @@ -157,16 +194,8 @@ func (il *isolatedLinux) addGatewayNeighbor(netNSPath string, eni *networkinterf return fmt.Errorf("failed to parse gateway IP: %s", gwIPStr) } - // The gateway MAC must be resolved from the host before entering the task - // netns, because the ENI has already been moved out of the host namespace. - gwMAC, err := il.resolveHostNeighbor(gwIP) - if err != nil { - return errors.Wrap(err, "gateway MAC not in host neighbor cache") - } - logger.Info("Installing gateway neighbor in task netns", map[string]interface{}{ "GatewayIP": gwIP.String(), - "GatewayMAC": gwMAC.String(), "NetNSPath": netNSPath, "DeviceName": eni.DeviceName, }) @@ -177,6 +206,11 @@ func (il *isolatedLinux) addGatewayNeighbor(netNSPath string, eni *networkinterf return errors.Wrapf(linkErr, "failed to find device %s in task netns", eni.DeviceName) } + gwMAC, err := il.resolveGatewayNeighbor(link, gwIP) + if err != nil { + return errors.Wrap(err, "gateway MAC not resolvable in task netns") + } + neigh := &netlink.Neigh{ LinkIndex: link.Attrs().Index, State: netlink.NUD_PERMANENT, @@ -205,21 +239,51 @@ func (il *isolatedLinux) addGatewayNeighbor(netNSPath string, eni *networkinterf }) } -// resolveHostNeighbor looks up the MAC address for the given IP in the host's -// neighbor (ARP) table. Returns an error if no entry is found. -func (il *isolatedLinux) resolveHostNeighbor(ip net.IP) (net.HardwareAddr, error) { - neighbors, err := il.common.netlink.NeighList(0, netlink.FAMILY_V4) +// resolveGatewayNeighbor resolves the gateway's MAC from within the current +// (task) network namespace. It polls the interface's neighbor table, nudging +// the kernel to ARP the gateway between polls, until an entry with a resolved +// MAC appears or the timeout elapses. It must be called inside the task netns +// (e.g. from within ExecInNSPath). +func (il *isolatedLinux) resolveGatewayNeighbor(link netlink.Link, gwIP net.IP) (net.HardwareAddr, error) { + linkIndex := link.Attrs().Index + deadline := time.Now().Add(gatewayNeighborResolveTimeout) + + for { + mac, err := il.lookupNeighbor(linkIndex, gwIP) + if err != nil { + return nil, err + } + if mac != nil { + return mac, nil + } + + if time.Now().After(deadline) { + break + } + + // Nudge the kernel to resolve the gateway, then wait before re-checking. + gatewayProbeFn(gwIP) + time.Sleep(gatewayNeighborResolveInterval) + } + + return nil, fmt.Errorf("no neighbor entry for %s in task netns after %s", gwIP, gatewayNeighborResolveTimeout) +} + +// lookupNeighbor returns the resolved MAC for ip on the given interface, or nil +// if there is no usable (resolved, non-failed) entry yet. +func (il *isolatedLinux) lookupNeighbor(linkIndex int, ip net.IP) (net.HardwareAddr, error) { + neighbors, err := il.common.netlink.NeighList(linkIndex, netlink.FAMILY_V4) if err != nil { - return nil, errors.Wrap(err, "failed to list host neighbors") + return nil, errors.Wrap(err, "failed to list neighbors") } for _, n := range neighbors { - if n.IP.Equal(ip) && len(n.HardwareAddr) > 0 { + if n.IP.Equal(ip) && len(n.HardwareAddr) > 0 && n.State != netlink.NUD_FAILED { return n.HardwareAddr, nil } } - return nil, fmt.Errorf("no neighbor entry for %s in host ARP table", ip) + return nil, nil } // CreateDNSConfig creates the task DNS config files and backfills the diff --git a/ecs-agent/netlib/platform/isolated_linux_test.go b/ecs-agent/netlib/platform/isolated_linux_test.go index 159e8fc602f..c3c0a2ce24f 100644 --- a/ecs-agent/netlib/platform/isolated_linux_test.go +++ b/ecs-agent/netlib/platform/isolated_linux_test.go @@ -21,6 +21,7 @@ import ( "net" "path/filepath" "testing" + "time" mock_ecscni2 "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/ecscni/mocks_ecscni" mock_ecscni "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/ecscni/mocks_nsutil" @@ -191,18 +192,19 @@ func TestIsolatedLinux_AddGatewayNeighbor(t *testing.T) { eni := getTestRegularV4ENI() eni.DeviceName = "eth0" - // resolveHostNeighbor: return gateway MAC - mockNetLink.EXPECT().NeighList(0, netlink.FAMILY_V4).Return([]netlink.Neigh{ - {IP: net.ParseIP("10.1.0.1"), HardwareAddr: gwMAC}, - }, nil) - - // ExecInNSPath: execute the closure + // ExecInNSPath: execute the closure (resolution now happens inside the netns). mockNSUtil.EXPECT().ExecInNSPath(netNSPath, gomock.Any()).DoAndReturn( func(path string, fn func(cnins.NetNS) error) error { return fn(nil) }) mockNetLink.EXPECT().LinkByName("eth0").Return(mockLink, nil) + + // resolveGatewayNeighbor: look up the gateway on the task-netns interface. + mockNetLink.EXPECT().NeighList(7, netlink.FAMILY_V4).Return([]netlink.Neigh{ + {IP: net.ParseIP("10.1.0.1"), HardwareAddr: gwMAC, State: netlink.NUD_REACHABLE}, + }, nil) + mockNetLink.EXPECT().NeighSet(gomock.Any()).DoAndReturn(func(neigh *netlink.Neigh) error { assert.Equal(t, 7, neigh.LinkIndex) assert.Equal(t, netlink.NUD_PERMANENT, neigh.State) @@ -233,17 +235,34 @@ func TestIsolatedLinux_AddGatewayNeighbor_NoGateway(t *testing.T) { require.NoError(t, err) } -func TestIsolatedLinux_AddGatewayNeighbor_HostNeighborNotFound(t *testing.T) { +func TestIsolatedLinux_AddGatewayNeighbor_GatewayNotResolvable(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - p, _, _, _, mockNetLink := newIsolatedLinuxPlatform(ctrl) + mockLink := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Index: 7, Name: "eth0"}} + + p, _, _, mockNSUtil, mockNetLink := newIsolatedLinuxPlatform(ctrl) eni := getTestRegularV4ENI() eni.DeviceName = "eth0" - mockNetLink.EXPECT().NeighList(0, netlink.FAMILY_V4).Return([]netlink.Neigh{}, nil) + // Keep the test fast and avoid a real probe datagram. + origTimeout, origInterval, origProbe := gatewayNeighborResolveTimeout, gatewayNeighborResolveInterval, gatewayProbeFn + gatewayNeighborResolveTimeout = 0 + gatewayNeighborResolveInterval = time.Millisecond + gatewayProbeFn = func(net.IP) {} + defer func() { + gatewayNeighborResolveTimeout, gatewayNeighborResolveInterval, gatewayProbeFn = origTimeout, origInterval, origProbe + }() + + mockNSUtil.EXPECT().ExecInNSPath(netNSPath, gomock.Any()).DoAndReturn( + func(path string, fn func(cnins.NetNS) error) error { + return fn(nil) + }) + mockNetLink.EXPECT().LinkByName("eth0").Return(mockLink, nil) + // Gateway never resolves: neighbor table stays empty. + mockNetLink.EXPECT().NeighList(7, netlink.FAMILY_V4).Return([]netlink.Neigh{}, nil).AnyTimes() err := p.addGatewayNeighbor(netNSPath, eni) assert.Error(t, err) - assert.Contains(t, err.Error(), "gateway MAC not in host neighbor cache") + assert.Contains(t, err.Error(), "gateway MAC not resolvable in task netns") }