[lxc-devel] [lxd/master] NIC: Bridged device fix openvswitch port leak

tomponline on Github lxc-bot at linuxcontainers.org
Tue Jun 2 09:27:06 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 804 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200602/0599807f/attachment-0001.bin>
-------------- next part --------------
From e1ca192063d262540c5c36f6945145e8716df311 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 2 Jun 2020 09:27:41 +0100
Subject: [PATCH 1/4] lxd/device/nic/bridged: Corrects vlan comment

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_bridged.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index a7d6a3a865..d270f80e1c 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -1197,7 +1197,7 @@ func (d *nicBridged) networkDHCPv6CreateIAAddress(IP net.IP) []byte {
 
 // setupBridgePortVLANs configures the bridge port with the specified VLAN settings in device config.
 func (d *nicBridged) setupBridgePortVLANs(hostName string) error {
-	// Enable vlan_filtering on bridge if needed.
+	// Check vlan_filtering is enabled on bridge if needed.
 	if d.config["vlan"] != "" || d.config["vlan.tagged"] != "" {
 		vlanFilteringStatus, err := network.BridgeVLANFilteringStatus(d.config["parent"])
 		if err != nil {

From c8f2731714810b90bb0d76729acde799a81563da Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 2 Jun 2020 10:11:28 +0100
Subject: [PATCH 2/4] lxd/network/network/utils: Improve comments on ovs switch
 attach/detach

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/network_utils.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lxd/network/network_utils.go b/lxd/network/network_utils.go
index 6d3b6b2330..577dd5583c 100644
--- a/lxd/network/network_utils.go
+++ b/lxd/network/network_utils.go
@@ -100,6 +100,7 @@ func AttachInterface(netName string, devName string) error {
 			return err
 		}
 	} else {
+		// Check if interface is already connected to a bridge, if not connect it to the specified bridge.
 		_, err := shared.RunCommand("ovs-vsctl", "port-to-br", devName)
 		if err != nil {
 			_, err := shared.RunCommand("ovs-vsctl", "add-port", netName, devName)
@@ -120,6 +121,7 @@ func DetachInterface(netName string, devName string) error {
 			return err
 		}
 	} else {
+		// Check if interface is connected to a bridge, if so, then remove it from the bridge.
 		_, err := shared.RunCommand("ovs-vsctl", "port-to-br", devName)
 		if err == nil {
 			_, err := shared.RunCommand("ovs-vsctl", "del-port", netName, devName)

From 39f9ef666866e2b9191f47fe99453b1f57d2be5e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 2 Jun 2020 10:15:52 +0100
Subject: [PATCH 3/4] lxd/network/network/utils: Improves arg name in network
 attach/detach functions

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/network_utils.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lxd/network/network_utils.go b/lxd/network/network_utils.go
index 577dd5583c..bb56b2b187 100644
--- a/lxd/network/network_utils.go
+++ b/lxd/network/network_utils.go
@@ -93,9 +93,9 @@ func GetIP(subnet *net.IPNet, host int64) net.IP {
 }
 
 // AttachInterface attaches an interface to a bridge.
-func AttachInterface(netName string, devName string) error {
-	if shared.PathExists(fmt.Sprintf("/sys/class/net/%s/bridge", netName)) {
-		_, err := shared.RunCommand("ip", "link", "set", "dev", devName, "master", netName)
+func AttachInterface(bridgeName string, devName string) error {
+	if shared.PathExists(fmt.Sprintf("/sys/class/net/%s/bridge", bridgeName)) {
+		_, err := shared.RunCommand("ip", "link", "set", "dev", devName, "master", bridgeName)
 		if err != nil {
 			return err
 		}
@@ -103,7 +103,7 @@ func AttachInterface(netName string, devName string) error {
 		// Check if interface is already connected to a bridge, if not connect it to the specified bridge.
 		_, err := shared.RunCommand("ovs-vsctl", "port-to-br", devName)
 		if err != nil {
-			_, err := shared.RunCommand("ovs-vsctl", "add-port", netName, devName)
+			_, err := shared.RunCommand("ovs-vsctl", "add-port", bridgeName, devName)
 			if err != nil {
 				return err
 			}
@@ -114,8 +114,8 @@ func AttachInterface(netName string, devName string) error {
 }
 
 // DetachInterface detaches an interface from a bridge.
-func DetachInterface(netName string, devName string) error {
-	if shared.PathExists(fmt.Sprintf("/sys/class/net/%s/bridge", netName)) {
+func DetachInterface(bridgeName string, devName string) error {
+	if shared.PathExists(fmt.Sprintf("/sys/class/net/%s/bridge", bridgeName)) {
 		_, err := shared.RunCommand("ip", "link", "set", "dev", devName, "nomaster")
 		if err != nil {
 			return err
@@ -124,7 +124,7 @@ func DetachInterface(netName string, devName string) error {
 		// Check if interface is connected to a bridge, if so, then remove it from the bridge.
 		_, err := shared.RunCommand("ovs-vsctl", "port-to-br", devName)
 		if err == nil {
-			_, err := shared.RunCommand("ovs-vsctl", "del-port", netName, devName)
+			_, err := shared.RunCommand("ovs-vsctl", "del-port", bridgeName, devName)
 			if err != nil {
 				return err
 			}

From da2f7177633bb57e96e6f8ede76a35a977d40b53 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 2 Jun 2020 10:21:58 +0100
Subject: [PATCH 4/4] lxd/device/bic/bridged: Fixes openvswitch port leak when
 device is stopped

Port was not being detached from bridge on stop causing disconnected ports to accumulate on bridge:

 sudo ovs-vsctl show
 84593e35-9a66-45ba-b647-929e396b3a59
    Bridge lxdbr1
        Port lxdbr1
            Interface lxdbr1
                type: internal
        Port veth1d5cd6a9
            Interface veth1d5cd6a9
                error: "could not open network device veth1d5cd6a9 (No such device)"

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_bridged.go | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index d270f80e1c..e906903d65 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -278,6 +278,7 @@ func (d *nicBridged) Start() (*deviceConfig.RunConfig, error) {
 	if err != nil {
 		return nil, err
 	}
+	revert.Add(func() { network.DetachInterface(d.config["parent"], saveData["host_name"]) })
 
 	// Attempt to disable router advertisement acceptance.
 	err = util.SysctlSet(fmt.Sprintf("net/ipv6/conf/%s/accept_ra", saveData["host_name"]), "0")
@@ -402,8 +403,14 @@ func (d *nicBridged) postStop() error {
 	}
 
 	if d.config["host_name"] != "" && shared.PathExists(fmt.Sprintf("/sys/class/net/%s", d.config["host_name"])) {
+		// Detach host-side end of veth pair from bridge (required for openvswitch particularly).
+		err := network.DetachInterface(d.config["parent"], d.config["host_name"])
+		if err != nil {
+			return err
+		}
+
 		// Removing host-side end of veth pair will delete the peer end too.
-		err := NetworkRemoveInterface(d.config["host_name"])
+		err = NetworkRemoveInterface(d.config["host_name"])
 		if err != nil {
 			return fmt.Errorf("Failed to remove interface %s: %s", d.config["host_name"], err)
 		}


More information about the lxc-devel mailing list