From f2d9632bc0d42b40d266f02dd24a72e91214dc3c Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Mon, 11 Jun 2018 12:46:08 -0700 Subject: [PATCH] network: Always bind back physical interfaces In case of physical network interfaces, we explicitly pass through them to the VM. We need to bind them back to the host driver when the sandbox is stopped, irrespective if the network namespace has been created by virtcontainers or not. Fixes #384 Signed-off-by: Archana Shinde --- virtcontainers/cni.go | 14 ++++++++---- virtcontainers/cnm.go | 14 ++++++++---- virtcontainers/network.go | 42 ++++++++++++++++++++++------------ virtcontainers/noop_network.go | 2 +- virtcontainers/sandbox.go | 6 +---- 5 files changed, 48 insertions(+), 30 deletions(-) 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.