Skip to content
47 changes: 35 additions & 12 deletions pkg/networkmanager/tap.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"net"

"github.com/inclusionAI/sandboxd/internal/util"
"github.com/sirupsen/logrus"
"github.com/vishvananda/netlink"
"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -190,16 +191,36 @@ func (m *InterfaceManager) setTapState(resource *NetResource, up bool) error {
if link.Type() != "tuntap" {
return fmt.Errorf("pooled endpoint %s has type %q, want tuntap", expectedName, link.Type())
}
if m.bridgeLink == nil || link.Attrs().MasterIndex != m.bridgeLink.Attrs().Index {
return fmt.Errorf("pooled TAP %s is not attached to %s", expectedName, BridgeName)
// Host-side drift (stale bridge attach, host MAC randomised by an
// unclean predecessor or foreign tooling) is repaired in place instead of
// failing the allocation or daemon recovery: the endpoint identity is
// fully determined by its name, so re-stamping the deterministic
// attributes can only converge it back to the expected state. Identity
// violations (wrong type above, wrong name earlier) stay hard errors.
if m.bridgeLink == nil {
return fmt.Errorf("pooled TAP %s has no %s to attach to", expectedName, BridgeName)
}
if link.Attrs().MasterIndex != m.bridgeLink.Attrs().Index {
if err := netlink.LinkSetMaster(link, m.bridgeLink); err != nil {
return fmt.Errorf(
"reattach pooled TAP %s to %s: %w", expectedName, BridgeName, err,
)
}
logrus.Warnf(
"networkmanager: repaired pooled TAP %s bridge attach (was not on %s)",
expectedName, BridgeName,
)
}
expectedHostMAC, _ := tapHostMAC(ip4)
if !bytes.Equal(link.Attrs().HardwareAddr, expectedHostMAC) {
return fmt.Errorf(
"pooled TAP %s host MAC is %s, want %s",
expectedName,
link.Attrs().HardwareAddr,
expectedHostMAC,
if err := netlink.LinkSetHardwareAddr(link, expectedHostMAC); err != nil {
return fmt.Errorf(
"restore pooled TAP %s host MAC to %s: %w", expectedName, expectedHostMAC, err,
)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False positive: net.HardwareAddr implements Stringer; fmt %s invokes it (verified output is colon-separated), and the pre-PR code used the same pattern.

logrus.Warnf(
"networkmanager: repaired pooled TAP %s host MAC %s -> %s",
expectedName, link.Attrs().HardwareAddr, expectedHostMAC,
)
}
expectedGuestMAC, _ := tapGuestMAC(ip4)
Expand All @@ -212,12 +233,14 @@ func (m *InterfaceManager) setTapState(resource *NetResource, up bool) error {
)
}
if resource.Interface.Index != 0 && resource.Interface.Index != link.Attrs().Index {
return fmt.Errorf(
"pooled TAP %s index is %d, lease records %d",
expectedName,
link.Attrs().Index,
resource.Interface.Index,
// The kernel ifindex is bookkeeping, not identity (it changes whenever
// the device is recreated); refresh the lease record instead of
// failing over a value nobody consumes.
logrus.Warnf(
"networkmanager: pooled TAP %s index drifted (%d, lease records %d); refreshing lease",
expectedName, link.Attrs().Index, resource.Interface.Index,
)
resource.Interface.Index = link.Attrs().Index
Comment on lines +243 to +247

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in f7eae7f (durable key swap on recovery)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by f1c3f7c: active-lease ifindex drift is now rejected outright at recovery (see tianyuzhou95's thread), so recovery never rewrites the durable key anymore. The remaining refresh paths (markUsing re-handout, Recycle) persist the refreshed serialization through the returned/queued string, which is what the durable key and the handed-out annotation both become.

}
if up {
if err := netlink.LinkSetUp(link); err != nil {
Expand Down
Loading