diff --git a/virtcontainers/cni.go b/virtcontainers/cni.go index 96074b23b6..3406e59cb3 100644 --- a/virtcontainers/cni.go +++ b/virtcontainers/cni.go @@ -153,10 +153,10 @@ func (n *cni) add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netN return networkNS, nil } -// remove unbridges and deletes TAP interfaces. It also removes virtual network -// interfaces and deletes the network namespace for the CNI network. -func (n *cni) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { - if err := removeNetworkCommon(networkNS); err != nil { +// remove network endpoints in the network namespace. It also deletes the network +// namespace in case the namespace has been created by us. +func (n *cni) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error { + if err := removeNetworkCommon(networkNS, netNsCreated); err != nil { return err } @@ -164,5 +164,9 @@ func (n *cni) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { return err } - return deleteNetNS(networkNS.NetNsPath, true) + if netNsCreated { + return deleteNetNS(networkNS.NetNsPath, true) + } + + return nil } diff --git a/virtcontainers/cnm.go b/virtcontainers/cnm.go index 98c7c467fc..c508b0938c 100644 --- a/virtcontainers/cnm.go +++ b/virtcontainers/cnm.go @@ -47,12 +47,16 @@ func (n *cnm) add(sandbox *Sandbox, config NetworkConfig, netNsPath string, netN return networkNS, nil } -// remove unbridges and deletes TAP interfaces. It also removes virtual network -// interfaces and deletes the network namespace for the CNM network. -func (n *cnm) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { - if err := removeNetworkCommon(networkNS); err != nil { +// remove network endpoints in the network namespace. It also deletes the network +// namespace in case the namespace has been created by us. +func (n *cnm) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error { + if err := removeNetworkCommon(networkNS, netNsCreated); err != nil { return err } - return deleteNetNS(networkNS.NetNsPath, true) + if netNsCreated { + return deleteNetNS(networkNS.NetNsPath, true) + } + + return nil } diff --git a/virtcontainers/network.go b/virtcontainers/network.go index fb6f109aab..197f1324f9 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -153,7 +153,7 @@ type Endpoint interface { SetProperties(NetworkInfo) Attach(hypervisor) error - Detach() error + Detach(netNsCreated bool, netNsPath string) error } // VirtualEndpoint gathers a network pair and its properties. @@ -230,9 +230,18 @@ func (endpoint *VirtualEndpoint) Attach(h hypervisor) error { // Detach for the virtual endpoint tears down the tap and bridge // created for the veth interface. -func (endpoint *VirtualEndpoint) Detach() error { +func (endpoint *VirtualEndpoint) Detach(netNsCreated bool, netNsPath string) error { + // The network namespace would have been deleted at this point + // if it has not been created by virtcontainers. + if !netNsCreated { + return nil + } + networkLogger().Info("Detaching virtual endpoint") - return xconnectVMNetwork(&(endpoint.NetPair), false) + + return doNetNS(netNsPath, func(_ ns.NetNS) error { + return xconnectVMNetwork(&(endpoint.NetPair), false) + }) } // Properties returns the properties of the interface. @@ -281,7 +290,7 @@ func (endpoint *VhostUserEndpoint) Attach(h hypervisor) error { } // Detach for vhostuser endpoint -func (endpoint *VhostUserEndpoint) Detach() error { +func (endpoint *VhostUserEndpoint) Detach(netNsCreated bool, netNsPath string) error { networkLogger().Info("Detaching vhostuser based endpoint") return nil } @@ -343,9 +352,14 @@ func (endpoint *PhysicalEndpoint) Attach(h hypervisor) error { // Detach for physical endpoint unbinds the physical network interface from vfio-pci // and binds it back to the saved host driver. -func (endpoint *PhysicalEndpoint) Detach() error { +func (endpoint *PhysicalEndpoint) Detach(netNsCreated bool, netNsPath string) error { // Bind back the physical network interface to host. + // We need to do this even if a new network namespace has not + // been created by virtcontainers. networkLogger().Info("Detaching physical endpoint") + + // We do not need to enter the network namespace to bind back the + // physical interface to host driver. return bindNICToHost(endpoint) } @@ -606,16 +620,16 @@ func addNetworkCommon(sandbox *Sandbox, networkNS *NetworkNamespace) error { return err } -func removeNetworkCommon(networkNS NetworkNamespace) error { - return doNetNS(networkNS.NetNsPath, func(_ ns.NetNS) error { - for _, endpoint := range networkNS.Endpoints { - if err := endpoint.Detach(); err != nil { - return err - } +func removeNetworkCommon(networkNS NetworkNamespace, netNsCreated bool) error { + for _, endpoint := range networkNS.Endpoints { + // Detach for an endpoint should enter the network namespace + // if required. + if err := endpoint.Detach(netNsCreated, networkNS.NetNsPath); err != nil { + return err } + } - return nil - }) + return nil } func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Link) (netlink.Link, []*os.File, error) { @@ -1410,5 +1424,5 @@ type network interface { // remove unbridges and deletes TAP interfaces. It also removes virtual network // interfaces and deletes the network namespace. - remove(sandbox *Sandbox, networkNS NetworkNamespace) error + remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error } diff --git a/virtcontainers/noop_network.go b/virtcontainers/noop_network.go index 7d37476866..e4ef408753 100644 --- a/virtcontainers/noop_network.go +++ b/virtcontainers/noop_network.go @@ -32,6 +32,6 @@ func (n *noopNetwork) add(sandbox *Sandbox, config NetworkConfig, netNsPath stri // remove unbridges and deletes TAP interfaces. It also removes virtual network // interfaces and deletes the network namespace for the Noop network. // It does nothing. -func (n *noopNetwork) remove(sandbox *Sandbox, networkNS NetworkNamespace) error { +func (n *noopNetwork) remove(sandbox *Sandbox, networkNS NetworkNamespace, netNsCreated bool) error { return nil } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 7b7459e5ac..5f48066f59 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -918,11 +918,7 @@ func (s *Sandbox) createNetwork() error { } func (s *Sandbox) removeNetwork() error { - if s.networkNS.NetNsCreated { - return s.network.remove(s, s.networkNS) - } - - return nil + return s.network.remove(s, s.networkNS, s.networkNS.NetNsCreated) } // startVM starts the VM.