Refactor setting up network during restore.

Previously, during sandbox restore the network configuration was restored by
creating a new network stack (which has all the config scraped from the host)
and the replacing this new config such as NICs, routes and IP addresses from
the new stack to the loaded/restored stack.

This change refactors the process by introducing a new `inet.NetworkArgs`
interface that abstracts how the network stack is configured. The `Loader` now
implements this interface and configures the loaded stack directly using the
configuration scraped from the host during restore. `Kernel.LoadFrom` accepts
this interface (instead of stack), allowing it to configure the restored stack
using either the Loader's configuration.

PiperOrigin-RevId: 941330876

Author: nybidari

pkg/sentry/inet/inet.go

@@ -106,13 +106,8 @@ type Stack interface {
 	// Restore restarts the network stack after restore.
 	Restore()
 
-	// ReplaceConfig replaces the new network stack configuration to the
-	// loaded or saved network stack after restore.
-	// TODO(b/379115439): This method is a workaround to update netstack config
-	// during restore. It should be removed after a new method is added to
-	// extract the complete config from the spec and update it in the loaded
-	// stack during restore.
-	ReplaceConfig(st Stack)
+	// ResetConfig resets the stack's NICs and configuration.
+	ResetConfig()
 
 	// Destroy the network stack.
 	Destroy()
@@ -301,3 +296,8 @@ type VethPeerReq struct {
 	// Stack is the stack where the second end has to be added.
 	Stack Stack
 }
+
+// NetworkArgs configures a network stack directly.
+type NetworkArgs interface {
+	ConfigureNetwork(s Stack) error
+}

pkg/sentry/inet/test_stack.go

@@ -184,8 +184,12 @@ func (s *TestStack) Pause() {}
 // Restore implements Stack.
 func (s *TestStack) Restore() {}
 
-// ReplaceConfig implements Stack.
-func (s *TestStack) ReplaceConfig(_ Stack) {}
+// ResetConfig implements Stack.
+func (s *TestStack) ResetConfig() {
+	s.InterfacesMap = make(map[int32]Interface)
+	s.InterfaceAddrsMap = make(map[int32][]InterfaceAddr)
+	s.RouteList = nil
+}
 
 // Resume implements Stack.
 func (s *TestStack) Resume() {}

pkg/sentry/kernel/kernel.go

@@ -915,7 +915,7 @@ func (k *Kernel) invalidateUnsavableMappings(ctx context.Context) error {
 }
 
 // LoadFrom returns a new Kernel loaded from args.
-func (k *Kernel) LoadFrom(ctx context.Context, r io.Reader, asyncMFLoader *AsyncMFLoader, timeReady chan struct{}, net inet.Stack, clocks sentrytime.Clocks, vfsOpts *vfs.CompleteRestoreOptions, timeline *timing.Timeline) error {
+func (k *Kernel) LoadFrom(ctx context.Context, r io.Reader, asyncMFLoader *AsyncMFLoader, timeReady chan struct{}, networkArgs inet.NetworkArgs, clocks sentrytime.Clocks, vfsOpts *vfs.CompleteRestoreOptions, timeline *timing.Timeline) error {
 	defer timeline.End()
 	if hostarch.PageSize != 4096 {
 		return fmt.Errorf("restore is not supported with %dK page size", hostarch.PageSize/1024)
@@ -987,11 +987,14 @@ func (k *Kernel) LoadFrom(ctx context.Context, r io.Reader, asyncMFLoader *Async
 	}
 
 	if s := k.rootNetworkNamespace.Stack(); s != nil {
-		if net != nil {
-			log.Infof("Reconfiguring network for restore")
-			s.ReplaceConfig(net)
+		if networkArgs == nil {
+			return fmt.Errorf("network configuration cannot be nil during restore")
+		}
+		log.Infof("Reconfiguring network for restore")
+		s.ResetConfig()
+		if err := networkArgs.ConfigureNetwork(s); err != nil {
+			return fmt.Errorf("configuring network: %w", err)
 		}
-		log.Debugf("Restore network stack")
 		s.Restore()
 		timeline.Reached("Network stack restored")
 	}

pkg/sentry/socket/hostinet/stack.go

@@ -409,8 +409,8 @@ func (*Stack) Pause() {}
 // Restore implements inet.Stack.Restore.
 func (*Stack) Restore() {}
 
-// ReplaceConfig implements inet.Stack.ReplaceConfig.
-func (s *Stack) ReplaceConfig(_ inet.Stack) {}
+// ResetConfig implements inet.Stack.ResetConfig.
+func (*Stack) ResetConfig() {}
 
 // Resume implements inet.Stack.Resume.
 func (*Stack) Resume() {}

pkg/sentry/socket/netstack/stack.go

@@ -1112,7 +1112,12 @@ func (s *Stack) Restore() {
 	s.Stack.Restore()
 }
 
-// ReplaceConfig implements inet.Stack.ReplaceConfig.
+// ResetConfig implements inet.Stack.ResetConfig.
+func (s *Stack) ResetConfig() {
+	s.Stack.ResetConfig()
+}
+
+// ReplaceConfig replaces config in the loaded stack.
 func (s *Stack) ReplaceConfig(st inet.Stack) {
 	if _, ok := st.(*Stack); !ok {
 		panic("netstack.Stack cannot be nil when netstack s/r is enabled")

pkg/tcpip/stack/icmp_rate_limit.go

@@ -15,6 +15,8 @@
 package stack
 
 import (
+	"context"
+
 	"golang.org/x/time/rate"
 	"gvisor.dev/gvisor/pkg/tcpip"
 )
@@ -34,9 +36,15 @@ const (
 //
 // +stateify savable
 type ICMPRateLimiter struct {
-	// TODO(b/341946753): Restore when netstack is savable.
 	limiter *rate.Limiter `state:"nosave"`
 	clock   tcpip.Clock
+	limit   rate.Limit
+	burst   int
+}
+
+// afterLoad is invoked by stateify.
+func (l *ICMPRateLimiter) afterLoad(context.Context) {
+	l.limiter = rate.NewLimiter(l.limit, l.burst)
 }
 
 // NewICMPRateLimiter returns a global rate limiter for controlling the rate
@@ -46,11 +54,14 @@ func NewICMPRateLimiter(clock tcpip.Clock) *ICMPRateLimiter {
 	return &ICMPRateLimiter{
 		clock:   clock,
 		limiter: rate.NewLimiter(icmpLimit, icmpBurst),
+		limit:   icmpLimit,
+		burst:   icmpBurst,
 	}
 }
 
 // SetLimit sets a new Limit for the limiter.
 func (l *ICMPRateLimiter) SetLimit(limit rate.Limit) {
+	l.limit = limit
 	l.limiter.SetLimitAt(l.clock.Now(), limit)
 }
 
@@ -61,6 +72,7 @@ func (l *ICMPRateLimiter) Limit() rate.Limit {
 
 // SetBurst sets a new burst size for the limiter.
 func (l *ICMPRateLimiter) SetBurst(burst int) {
+	l.burst = burst
 	l.limiter.SetBurstAt(l.clock.Now(), burst)
 }
 

pkg/tcpip/stack/stack.go

@@ -2086,10 +2086,19 @@ func (s *Stack) getNICs() map[tcpip.NICID]*nic {
 	return nics
 }
 
+// ResetConfig resets the stack's NICs and ID generator.
+func (s *Stack) ResetConfig() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.nics = make(map[tcpip.NICID]*nic)
+	s.loopbackNIC = nil
+	s.nicIDGen.Store(0)
+}
+
 // ReplaceConfig replaces config in the loaded stack.
 func (s *Stack) ReplaceConfig(st *Stack) {
 	if st == nil {
-		panic("stack.Stack cannot be nil when netstack s/r is enabled")
+		panic("stack.Stack cannot be nil when replacing config")
 	}
 
 	// Update route table.
@@ -2104,9 +2113,6 @@ func (s *Stack) ReplaceConfig(st *Stack) {
 	s.tables = st.IPTables()
 	s.nftables = st.NFTables()
 
-	// Update NICs.
-	s.nics = make(map[tcpip.NICID]*nic)
-	s.loopbackNIC = nil
 	for id, nic := range nics {
 		nic.stack = s
 		s.nics[id] = nic
@@ -2320,6 +2326,11 @@ func (s *Stack) IPTables() *IPTables {
 	return s.tables
 }
 
+// SetIPTables sets the stack's iptables.
+func (s *Stack) SetIPTables(tables *IPTables) {
+	s.tables = tables
+}
+
 // NFTables returns the stack's nftables.
 func (s *Stack) NFTables() NFTablesInterface {
 	return s.nftables

runsc/boot/controller.go

@@ -141,12 +141,12 @@ const (
 	// ContMgrContainerRuntimeState returns the runtime state of a container.
 	ContMgrContainerRuntimeState = "containerManager.ContainerRuntimeState"
 
-	// ContMgrCreateLinksAndRoutes creates links and routes, and sets network args in loader.
-	ContMgrCreateLinksAndRoutes = "containerManager.CreateLinksAndRoutes"
-
 	// ContMgrGetNetworkConfig returns the network interfaces and routes applied
 	// during the creation of root container.
 	ContMgrGetNetworkConfig = "containerManager.GetNetworkConfig"
+
+	// ContMgrSetNetworkArgs sets network args in loader without creating links.
+	ContMgrSetNetworkArgs = "containerManager.SetNetworkArgs"
 )
 
 const (
@@ -1234,25 +1234,27 @@ func (cm *containerManager) GetSavings(_ *struct{}, s *Savings) error {
 	return nil
 }
 
-// CreateLinksAndRoutes creates links and routes, and sets the network arguments.
-func (cm *containerManager) CreateLinksAndRoutes(args *CreateLinksAndRoutesArgs, _ *struct{}) error {
-	log.Debugf("containerManager.CreateLinksAndRoutes")
+// SetNetworkArgs sets the network arguments without creating links and routes.
+func (cm *containerManager) SetNetworkArgs(args *CreateLinksAndRoutesArgs, _ *struct{}) error {
+	log.Debugf("containerManager.SetNetworkArgs")
 	if args == nil {
 		return fmt.Errorf("cannot set nil networkArgs")
 	}
 
-	if eps, ok := cm.l.k.RootNetworkNamespace().Stack().(*netstack.Stack); ok {
-		n := &Network{
-			Stack:  eps.Stack,
-			Kernel: cm.l.k,
-		}
-		if err := n.CreateLinksAndRoutes(args, nil); err != nil {
-			return err
-		}
-		cm.l.mu.Lock()
-		cm.l.networkArgs = args
-		cm.l.mu.Unlock()
+	// Create a new CreateLinksAndRoutesArgs variable to store in the loader
+	// as the fds associated with the original argument passed to this urpc
+	// will be closed when it returns.
+	networkArgs := *args
+	dupedFDs, err := fd.NewFromFiles(args.FilePayload.Files)
+	if err != nil {
+		return fmt.Errorf("failed to dup network FDs: %w", err)
 	}
+	// Release the duplicated FDs back to os.File objects and store them in the copy.
+	networkArgs.FilePayload.Files = fd.ReleaseToFiles(dupedFDs, "network-fd")
+
+	cm.l.mu.Lock()
+	cm.l.networkArgs = &networkArgs
+	cm.l.mu.Unlock()
 	return nil
 }
 

runsc/boot/loader.go

@@ -821,6 +821,37 @@ func New(args Args) (*Loader, error) {
 	return l, nil
 }
 
+// ConfigureNetwork implements inet.NetworkArgs.ConfigureNetwork.
+func (l *Loader) ConfigureNetwork(s inet.Stack) error {
+	if l.networkArgs == nil {
+		return nil
+	}
+
+	// Close the FDs after they are used to configure the stack.
+	defer func() {
+		for _, f := range l.networkArgs.FilePayload.Files {
+			f.Close()
+		}
+		l.networkArgs.FilePayload.Files = nil
+	}()
+
+	eps, ok := s.(*netstack.Stack)
+	if !ok {
+		return nil
+	}
+	if eps.Stack.IPTables() == nil {
+		eps.Stack.SetIPTables(netfilter.DefaultLinuxTables(eps.Stack.Clock(), eps.Stack.InsecureRNG()))
+	}
+	if nftables.IsNFTablesEnabled() && eps.Stack.NFTables() == nil {
+		eps.Stack.SetNFTables(nftables.NewNFTables(eps.Stack.Clock(), eps.Stack.SecureRNG()))
+	}
+	n := &Network{
+		Stack:  eps.Stack,
+		Kernel: l.k,
+	}
+	return n.CreateLinksAndRoutes(l.networkArgs, nil)
+}
+
 // createProcessArgs creates args that can be used with kernel.CreateProcess.
 func createProcessArgs(id string, spec *specs.Spec, conf *config.Config, creds *auth.Credentials, k *kernel.Kernel, pidns *kernel.PIDNamespace) (kernel.CreateProcessArgs, error) {
 	// Create initial limits.
@@ -1086,6 +1117,12 @@ func (l *Loader) run() error {
 			pprof.Initialize()
 		}
 
+		if l.networkArgs != nil {
+			if err := l.ConfigureNetwork(l.k.RootNetworkNamespace().Stack()); err != nil {
+				return err
+			}
+		}
+
 		// Finally done with all configuration. Setup filters before user code
 		// is loaded.
 		if err := l.installSeccompFilters(); err != nil {

runsc/boot/loader_test.go

@@ -663,7 +663,7 @@ func TestNetworkConfig(t *testing.T) {
 			},
 		},
 	}
-	if err := l.ctrl.manager.CreateLinksAndRoutes(args, nil); err != nil {
+	if err := l.ctrl.manager.SetNetworkArgs(args, nil); err != nil {
 		t.Errorf("error calling SetNetworkConfig: %v", err)
 	}
 	var networkArgs CreateLinksAndRoutesArgs