diff --git a/daemon/libnetwork/osl/interface_linux.go b/daemon/libnetwork/osl/interface_linux.go index 90e351e1e5..62062b5951 100644 --- a/daemon/libnetwork/osl/interface_linux.go +++ b/daemon/libnetwork/osl/interface_linux.go @@ -665,6 +665,17 @@ func (n *Namespace) advertiseAddrs(ctx context.Context, ifIndex int, i *Interfac defer span.End() mac := i.MacAddress() + // If MAC is not stored in the interface struct, get it from the actual link. + // This can happen with some network drivers (e.g., SR-IOV, macvlan) that don't + // store the MAC in the endpoint configuration. + if len(mac) == 0 { + link, err := nlh.LinkByIndex(ifIndex) + if err != nil { + log.G(ctx).WithFields(log.Fields{"error": err, "ifi": ifIndex}).Warn("Failed to lookup link by index to determine MAC address; treating as no MAC to advertise") + } else if hw := link.Attrs().HardwareAddr; len(hw) > 0 { + mac = hw + } + } address4 := i.Address() address6 := i.AddressIPv6() ctx = log.WithLogger(ctx, log.G(ctx).WithFields(log.Fields{ @@ -681,7 +692,7 @@ func (n *Namespace) advertiseAddrs(ctx context.Context, ifIndex int, i *Interfac log.G(ctx).Debug("No IP addresses to advertise") return nil } - if mac == nil { + if len(mac) == 0 { // Nothing to do - for example, a layer-3 ipvlan. log.G(ctx).Debug("No MAC address to advertise") return nil @@ -691,7 +702,7 @@ func (n *Namespace) advertiseAddrs(ctx context.Context, ifIndex int, i *Interfac return nil } - arpSender, naSender := n.prepAdvertiseAddrs(ctx, i, ifIndex) + arpSender, naSender := n.prepAdvertiseAddrs(ctx, i, ifIndex, mac) if arpSender == nil && naSender == nil { return nil } @@ -740,9 +751,13 @@ func (n *Namespace) advertiseAddrs(ctx context.Context, ifIndex int, i *Interfac return errors.Join(errs...) } - // Send an initial message. If it fails, skip the resends. + // Send an initial message. If it fails, log a warning but don't fail container + // creation - NA is an optimization, neighbors will still discover addresses via + // normal NDP solicitation. This can happen with L3 ipvlan which doesn't support + // multicast. if err := send(ctx); err != nil { - return err + log.G(ctx).WithError(err).Warn("Failed to send initial neighbor advertisement") + return nil } if i.advertiseAddrNMsgs == 1 { return nil @@ -775,20 +790,20 @@ func (n *Namespace) advertiseAddrs(ctx context.Context, ifIndex int, i *Interfac return nil } -func (n *Namespace) prepAdvertiseAddrs(ctx context.Context, i *Interface, ifIndex int) (*l2disco.UnsolARP, *l2disco.UnsolNA) { +func (n *Namespace) prepAdvertiseAddrs(ctx context.Context, i *Interface, ifIndex int, mac net.HardwareAddr) (*l2disco.UnsolARP, *l2disco.UnsolNA) { var ua *l2disco.UnsolARP var un *l2disco.UnsolNA if err := n.InvokeFunc(func() { if address4 := i.Address(); address4 != nil { var err error - ua, err = l2disco.NewUnsolARP(ctx, address4.IP, i.MacAddress(), ifIndex) + ua, err = l2disco.NewUnsolARP(ctx, address4.IP, mac, ifIndex) if err != nil { log.G(ctx).WithError(err).Warn("Failed to prepare unsolicited ARP") } } if address6 := i.AddressIPv6(); address6 != nil { var err error - un, err = l2disco.NewUnsolNA(ctx, address6.IP, i.MacAddress(), ifIndex) + un, err = l2disco.NewUnsolNA(ctx, address6.IP, mac, ifIndex) if err != nil { log.G(ctx).WithError(err).Warn("Failed to prepare unsolicited NA") } diff --git a/daemon/libnetwork/osl/namespace_linux.go b/daemon/libnetwork/osl/namespace_linux.go index 0139c3f359..3d97939be4 100644 --- a/daemon/libnetwork/osl/namespace_linux.go +++ b/daemon/libnetwork/osl/namespace_linux.go @@ -397,10 +397,14 @@ func (n *Namespace) Destroy() error { return nil } -// RestoreInterfaces restores the network namespace's interfaces. -func (n *Namespace) RestoreInterfaces(interfaces map[Iface][]IfaceOption) error { +// RestoreInterfaces restores the network namespace's interfaces and sends +// unsolicited ARP/NA messages to update neighbor caches. +func (n *Namespace) RestoreInterfaces(ctx context.Context, interfaces map[Iface][]IfaceOption) error { // restore interfaces for iface, opts := range interfaces { + if err := ctx.Err(); err != nil { + return err + } i, err := newInterface(n, iface.SrcName, iface.DstPrefix, iface.DstName, opts...) if err != nil { return err @@ -459,6 +463,29 @@ func (n *Namespace) RestoreInterfaces(interfaces map[Iface][]IfaceOption) error n.iFaces = append(n.iFaces, i) n.mu.Unlock() } + + // Send unsolicited ARP/NA messages to update neighbor caches with the + // MAC address associated with the interface's IP addresses. This is + // necessary after a daemon restart because other hosts may have stale + // neighbor cache entries. + if i.dstName != "" { + log.G(ctx).WithFields(log.Fields{ + "interface": i.dstName, + "ipv4": i.address, + "ipv6": i.addressIPv6, + }).Debug("Sending neighbor advertisements during restore") + link, err := n.nlHandle.LinkByName(i.dstName) + if err != nil { + log.G(ctx).WithFields(log.Fields{"error": err, "interface": i.dstName}).Warn("Failed to get link for neighbor advertisement during restore") + continue + } + ifIndex := link.Attrs().Index + waitForBridgePort(ctx, ns.NlHandle(), link) + mcastRouteOk := waitForMcastRoute(ctx, ifIndex, i, n.nlHandle) + if err := n.advertiseAddrs(ctx, ifIndex, i, n.nlHandle, mcastRouteOk); err != nil { + log.G(ctx).WithError(err).WithField("interface", i.dstName).Warn("Failed to send neighbor advertisement during restore") + } + } } return nil } diff --git a/daemon/libnetwork/sandbox_linux.go b/daemon/libnetwork/sandbox_linux.go index 27a37dfd8c..58d89e95e1 100644 --- a/daemon/libnetwork/sandbox_linux.go +++ b/daemon/libnetwork/sandbox_linux.go @@ -259,7 +259,7 @@ func (sb *Sandbox) releaseOSSbox() error { return osSbox.Destroy() } -func (sb *Sandbox) restoreOslSandbox() error { +func (sb *Sandbox) restoreOslSandbox(ctx context.Context) error { var routes []*types.StaticRoute // restore osl sandbox @@ -271,7 +271,7 @@ func (sb *Sandbox) restoreOslSandbox() error { ep.mu.Unlock() if i == nil { - log.G(context.TODO()).Errorf("error restoring endpoint %s for container %s", ep.Name(), sb.ContainerID()) + log.G(ctx).Errorf("error restoring endpoint %s for container %s", ep.Name(), sb.ContainerID()) continue } @@ -298,7 +298,9 @@ func (sb *Sandbox) restoreOslSandbox() error { } } - if err := sb.osSbox.RestoreInterfaces(interfaces); err != nil { + // Use WithoutCancel so that restore completes even if the parent context is + // cancelled - we don't want to leave containers with partially restored networking. + if err := sb.osSbox.RestoreInterfaces(context.WithoutCancel(ctx), interfaces); err != nil { return err } if len(routes) > 0 { diff --git a/daemon/libnetwork/sandbox_store.go b/daemon/libnetwork/sandbox_store.go index 4b9e94132d..8d32dc6106 100644 --- a/daemon/libnetwork/sandbox_store.go +++ b/daemon/libnetwork/sandbox_store.go @@ -256,7 +256,7 @@ func (c *Controller) sandboxRestore(activeSandboxes map[string]any) error { // reconstruct osl sandbox field if !sb.config.useDefaultSandBox { - if err := sb.restoreOslSandbox(); err != nil { + if err := sb.restoreOslSandbox(ctx); err != nil { log.G(ctx).WithError(err).Error("Failed to populate fields for osl sandbox") continue } diff --git a/daemon/libnetwork/sandbox_windows.go b/daemon/libnetwork/sandbox_windows.go index 61c5dc76d8..3a984e47fd 100644 --- a/daemon/libnetwork/sandbox_windows.go +++ b/daemon/libnetwork/sandbox_windows.go @@ -24,7 +24,7 @@ func (sb *Sandbox) releaseOSSbox() error { return nil } -func (sb *Sandbox) restoreOslSandbox() error { +func (sb *Sandbox) restoreOslSandbox(_ context.Context) error { // not implemented on Windows (Sandbox.osSbox is always nil) return nil } diff --git a/integration/networking/bridge_linux_test.go b/integration/networking/bridge_linux_test.go index 40db16c39b..0937f2e992 100644 --- a/integration/networking/bridge_linux_test.go +++ b/integration/networking/bridge_linux_test.go @@ -1897,6 +1897,128 @@ func TestAdvertiseAddresses(t *testing.T) { } } +// TestAdvertiseAddressesLiveRestore verifies that unsolicited ARP/NA messages are +// sent when the daemon restarts with live-restore enabled. This ensures that +// neighbor caches on other hosts are updated with the container's MAC address +// after a daemon restart. +func TestAdvertiseAddressesLiveRestore(t *testing.T) { + skip.If(t, testEnv.IsRootless, "can't listen for ARP/NA messages in rootlesskit's namespace") + + ctx := setupTest(t) + d := daemon.New(t) + d.StartWithBusybox(ctx, t, "--live-restore") + defer d.Stop(t) + c := d.NewClientT(t) + defer c.Close() + + const netName = "dsnet-lr" + const brName = "br-advaddrlr" + network.CreateNoError(ctx, t, c, netName, + network.WithOption(bridge.BridgeName, brName), + network.WithIPv6(), + network.WithIPAM("172.23.23.0/24", "172.23.23.1"), + network.WithIPAM("fd4c:f70b:973d::/64", "fd4c:f70b:973d::1"), + ) + defer network.RemoveNoError(ctx, t, c, netName) + + // Create ctr1 which will be used to verify neighbor cache updates. + ctr1Id := container.Run(ctx, t, c, container.WithName("ctr1-lr"), container.WithNetworkMode(netName)) + defer c.ContainerRemove(ctx, ctr1Id, client.ContainerRemoveOptions{Force: true}) + + // Create ctr2 with fixed IP addresses. + const ctr2Name = "ctr2-lr" + const ctr2Addr4 = "172.23.23.22" + const ctr2Addr6 = "fd4c:f70b:973d::2222" + ctr2Id := container.Run(ctx, t, c, + container.WithName(ctr2Name), + container.WithNetworkMode(netName), + container.WithIPv4(netName, ctr2Addr4), + container.WithIPv6(netName, ctr2Addr6), + ) + defer c.ContainerRemove(ctx, ctr2Id, client.ContainerRemoveOptions{Force: true}) + + ctr2MAC := container.Inspect(ctx, t, c, ctr2Id).NetworkSettings.Networks[netName].MacAddress + + // Ping from ctr1 to ctr2 to populate ctr1's neighbor caches. + pingRes := container.ExecT(ctx, t, c, ctr1Id, []string{"ping", "-4", "-c1", ctr2Name}) + assert.Assert(t, is.Equal(pingRes.ExitCode, 0)) + pingRes = container.ExecT(ctx, t, c, ctr1Id, []string{"ping", "-6", "-c1", ctr2Name}) + assert.Assert(t, is.Equal(pingRes.ExitCode, 0)) + + // Verify ctr1 has neighbor entries for ctr2. + ctr1Neighs := container.ExecT(ctx, t, c, ctr1Id, []string{"ip", "neigh", "show"}) + assert.Assert(t, is.Equal(ctr1Neighs.ExitCode, 0)) + t.Logf("ctr1 neighbours before restart:\n%s", ctr1Neighs.Combined()) + + // Wait for initial ARP/NA retransmits from container creation to settle. + // The daemon sends unsolicited ARP/NA messages for a couple of seconds after + // AddInterface, so we need to wait before starting to listen to avoid counting + // those messages instead of the ones sent during restore. + t.Log("Waiting for initial ARP/NA retransmits to settle...") + time.Sleep(5 * time.Second) + + // Now start listening for ARP/NA messages. + stopARPListen := network.CollectBcastARPs(t, brName) + defer stopARPListen() + stopICMP6Listen := network.CollectICMP6(t, brName) + defer stopICMP6Listen() + + // Restart the daemon - this should trigger RestoreInterfaces which sends ARP/NA. + d.Restart(t, "--live-restore") + + // Give time for ARP/NA messages to be sent after restart. + t.Log("Sleeping for 5s to collect ARP/NA messages after daemon restart...") + time.Sleep(5 * time.Second) + + // Verify that ARP/NA messages were sent for ctr2's addresses. + arps := stopARPListen() + var arpCount int + for i, p := range arps { + ha, pa, err := network.UnpackUnsolARP(p) + if err != nil { + t.Logf("ARP %d: %s: %s: %s", i+1, p.ReceivedAt.Format("15:04:05.000"), hex.EncodeToString(p.Data), err) + continue + } + t.Logf("ARP %d: %s '%s' is at '%s'", i+1, p.ReceivedAt.Format("15:04:05.000"), pa, ha) + if pa == netip.MustParseAddr(ctr2Addr4) && slices.Compare(ha, net.HardwareAddr(ctr2MAC)) == 0 { + arpCount++ + t.Logf("---> found ARP for ctr2") + } + } + assert.Check(t, arpCount >= 1, "expected at least 1 ARP message for ctr2 after live-restore, got %d", arpCount) + + icmps := stopICMP6Listen() + var naCount int + for i, p := range icmps { + ha, pa, err := network.UnpackUnsolNA(p) + if err != nil { + t.Logf("ICMP6 %d: %s: %s: %s", i+1, p.ReceivedAt.Format("15:04:05.000"), hex.EncodeToString(p.Data), err) + continue + } + t.Logf("ICMP6 %d: %s '%s' is at '%s'", i+1, p.ReceivedAt.Format("15:04:05.000"), pa, ha) + if pa == netip.MustParseAddr(ctr2Addr6) && slices.Compare(ha, net.HardwareAddr(ctr2MAC)) == 0 { + naCount++ + t.Logf("---> found NA for ctr2") + } + } + assert.Check(t, naCount >= 1, "expected at least 1 NA message for ctr2 after live-restore, got %d", naCount) + + // Verify ctr1 still has valid neighbor entries (connectivity should work). + ctr1Neighs = container.ExecT(ctx, t, c, ctr1Id, []string{"ip", "neigh", "show"}) + assert.Assert(t, is.Equal(ctr1Neighs.ExitCode, 0)) + t.Logf("ctr1 neighbours after restart:\n%s", ctr1Neighs.Combined()) + + // Verify connectivity still works after restart. + pingRes = container.ExecT(ctx, t, c, ctr1Id, []string{"ping", "-4", "-c1", ctr2Name}) + assert.Assert(t, is.Equal(pingRes.ExitCode, 0)) + pingRes = container.ExecT(ctx, t, c, ctr1Id, []string{"ping", "-6", "-c1", ctr2Name}) + assert.Assert(t, is.Equal(pingRes.ExitCode, 0)) + + if t.Failed() { + d.TailLogsT(t, 100) + } +} + // TestNetworkInspectGateway checks that gateways reported in inspect output are parseable as addresses. func TestNetworkInspectGateway(t *testing.T) { ctx := setupTest(t)