[lxc-devel] [lxd/master] Network: Adds optimal MTU detection for bridge.mtu setting based on OVN's underlay MTU and address family

tomponline on Github lxc-bot at linuxcontainers.org
Tue Sep 22 11:36:21 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 698 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200922/eb083932/attachment-0001.bin>
-------------- next part --------------
From 6a43c6dcc11ed81d42a91f352ff70cc1d8216206 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 11:19:32 +0100
Subject: [PATCH 01/11] shared/validate: Use ParseUint in IsNetworkMTU

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

diff --git a/shared/validate/validate.go b/shared/validate/validate.go
index 2db3f92efd..d9645f6dfc 100644
--- a/shared/validate/validate.go
+++ b/shared/validate/validate.go
@@ -389,7 +389,7 @@ func IsNetworkVLAN(value string) error {
 // Anything below 68 and the kernel doesn't allow IPv4, anything below 1280 and the kernel doesn't allow IPv6.
 // So require an IPv6-compatible MTU as the low value and cap at the max ethernet jumbo frame size.
 func IsNetworkMTU(value string) error {
-	mtu, err := strconv.ParseInt(value, 10, 32)
+	mtu, err := strconv.ParseUint(value, 10, 32)
 	if err != nil {
 		return fmt.Errorf("Invalid MTU %q", value)
 	}

From cefdfacababb21e9a4768b58103a4c3684685091 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 11:20:02 +0100
Subject: [PATCH 02/11] lxd/device/device/utils/network: Change argument for
 NetworkSetDevMTU to uint32

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

diff --git a/lxd/device/device_utils_network.go b/lxd/device/device_utils_network.go
index 3e779b10c7..15069d2216 100644
--- a/lxd/device/device_utils_network.go
+++ b/lxd/device/device_utils_network.go
@@ -29,7 +29,7 @@ import (
 var networkCreateSharedDeviceLock sync.Mutex
 
 // NetworkSetDevMTU sets the MTU setting for a named network device if different from current.
-func NetworkSetDevMTU(devName string, mtu uint64) error {
+func NetworkSetDevMTU(devName string, mtu uint32) error {
 	curMTU, err := network.GetDevMTU(devName)
 	if err != nil {
 		return err

From b61d96bf5ca63e8699dad37747b889c90e85cbc4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 11:20:36 +0100
Subject: [PATCH 03/11] lxd/device/device/utils/network: NetworkSetDevMTU usage

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/device_utils_network.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lxd/device/device_utils_network.go b/lxd/device/device_utils_network.go
index 15069d2216..eb95d3501b 100644
--- a/lxd/device/device_utils_network.go
+++ b/lxd/device/device_utils_network.go
@@ -192,7 +192,7 @@ func networkRestorePhysicalNic(hostName string, volatile map[string]string) erro
 			return fmt.Errorf("Failed to convert mtu for \"%s\" mtu \"%s\": %v", hostName, volatile["last_state.mtu"], err)
 		}
 
-		err = NetworkSetDevMTU(hostName, mtuInt)
+		err = NetworkSetDevMTU(hostName, uint32(mtuInt))
 		if err != nil {
 			return fmt.Errorf("Failed to restore physical dev \"%s\" mtu to \"%d\": %v", hostName, mtuInt, err)
 		}
@@ -238,18 +238,18 @@ func networkCreateVethPair(hostName string, m deviceConfig.Device) (string, erro
 
 	// Set the MTU on peer. If not specified and has parent, will inherit MTU from parent.
 	if m["mtu"] != "" {
-		MTU, err := strconv.ParseUint(m["mtu"], 10, 32)
+		mtu, err := strconv.ParseUint(m["mtu"], 10, 32)
 		if err != nil {
 			return "", fmt.Errorf("Invalid MTU specified: %v", err)
 		}
 
-		err = NetworkSetDevMTU(peerName, MTU)
+		err = NetworkSetDevMTU(peerName, uint32(mtu))
 		if err != nil {
 			NetworkRemoveInterface(peerName)
 			return "", fmt.Errorf("Failed to set the MTU: %v", err)
 		}
 
-		err = NetworkSetDevMTU(hostName, MTU)
+		err = NetworkSetDevMTU(hostName, uint32(mtu))
 		if err != nil {
 			NetworkRemoveInterface(peerName)
 			return "", fmt.Errorf("Failed to set the MTU: %v", err)
@@ -294,12 +294,12 @@ func networkCreateTap(hostName string, m deviceConfig.Device) error {
 
 	// Set the MTU on peer. If not specified and has parent, will inherit MTU from parent.
 	if m["mtu"] != "" {
-		MTU, err := strconv.ParseUint(m["mtu"], 10, 32)
+		mtu, err := strconv.ParseUint(m["mtu"], 10, 32)
 		if err != nil {
 			return errors.Wrap(err, "Invalid MTU specified")
 		}
 
-		err = NetworkSetDevMTU(hostName, MTU)
+		err = NetworkSetDevMTU(hostName, uint32(mtu))
 		if err != nil {
 			return errors.Wrap(err, "Failed to set the MTU")
 		}

From 13cda6c28fcdfa075937cbd30f6518f594b3101c Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 11:20:55 +0100
Subject: [PATCH 04/11] lxd/network/network/utils: Changes GetDevMTU to return
 uint32

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

diff --git a/lxd/network/network_utils.go b/lxd/network/network_utils.go
index fdd0b904ee..c195228a3d 100644
--- a/lxd/network/network_utils.go
+++ b/lxd/network/network_utils.go
@@ -292,7 +292,7 @@ func DetachInterface(bridgeName string, devName string) error {
 }
 
 // GetDevMTU retrieves the current MTU setting for a named network device.
-func GetDevMTU(devName string) (uint64, error) {
+func GetDevMTU(devName string) (uint32, error) {
 	content, err := ioutil.ReadFile(fmt.Sprintf("/sys/class/net/%s/mtu", devName))
 	if err != nil {
 		return 0, err
@@ -304,7 +304,7 @@ func GetDevMTU(devName string) (uint64, error) {
 		return 0, err
 	}
 
-	return mtu, nil
+	return uint32(mtu), nil
 }
 
 // DefaultGatewaySubnetV4 returns subnet of default gateway interface.

From 7376a8b6158877b9c74ae35aadabe067031b4bf1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 11:21:13 +0100
Subject: [PATCH 05/11] lxd/network/openvswitch/ovs: Adds OVNEncapIP function

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

diff --git a/lxd/network/openvswitch/ovs.go b/lxd/network/openvswitch/ovs.go
index a8d4002d6a..478d9325aa 100644
--- a/lxd/network/openvswitch/ovs.go
+++ b/lxd/network/openvswitch/ovs.go
@@ -2,6 +2,7 @@ package openvswitch
 
 import (
 	"fmt"
+	"net"
 	"os/exec"
 	"strconv"
 	"strings"
@@ -169,6 +170,31 @@ func (o *OVS) ChassisID() (string, error) {
 	return chassisID, nil
 }
 
+// OVNEncapIP returns the enscapsulation IP used for OVN underlay tunnels.
+func (o *OVS) OVNEncapIP() (net.IP, error) {
+	// ovs-vsctl's get command doesn't support its --format flag, so we always get the output quoted.
+	// However ovs-vsctl's find and list commands don't support retrieving a single column's map field.
+	// And ovs-vsctl's JSON output is unfriendly towards statically typed languages as it mixes data types
+	// in a slice. So stick with "get" command and use Go's strconv.Unquote to return the actual values.
+	encapIPStr, err := shared.RunCommand("ovs-vsctl", "get", "open_vswitch", ".", "external_ids:ovn-encap-ip")
+	if err != nil {
+		return nil, err
+	}
+
+	encapIPStr = strings.TrimSpace(encapIPStr)
+	encapIPStr, err = strconv.Unquote(encapIPStr)
+	if err != nil {
+		return nil, err
+	}
+
+	encapIP := net.ParseIP(encapIPStr)
+	if encapIP == nil {
+		return nil, fmt.Errorf("Invalid ovn-encap-ip address")
+	}
+
+	return encapIP, nil
+}
+
 // OVNBridgeMappings gets the current OVN bridge mappings.
 func (o *OVS) OVNBridgeMappings(bridgeName string) ([]string, error) {
 	// ovs-vsctl's get command doesn't support its --format flag, so we always get the output quoted.

From 2f43a2aa00c15f4af7ada85be3a29d21a42db635 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 12:27:28 +0100
Subject: [PATCH 06/11] lxd/network/driver/ovn: Removes ovnGeneveTunnelMTU
 constant

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 0f62cc7d4c..5482dd0583 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -28,9 +28,6 @@ import (
 	"github.com/lxc/lxd/shared/validate"
 )
 
-// ovnGeneveTunnelMTU is the MTU that is safe to use when tunneling using geneve.
-const ovnGeneveTunnelMTU = 1442
-
 const ovnChassisPriorityMax = 32767
 const ovnVolatileParentIPv4 = "volatile.network.ipv4.address"
 const ovnVolatileParentIPv6 = "volatile.network.ipv6.address"

From b477c2ad2f469e5b924b280a214eca137f32f6e1 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 12:28:39 +0100
Subject: [PATCH 07/11] lxd/network/network/utils/ovn: Removes
 OVNInstanceDeviceMTU function

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/network_utils_ovn.go | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/lxd/network/network_utils_ovn.go b/lxd/network/network_utils_ovn.go
index 34ad39b59e..d603034019 100644
--- a/lxd/network/network_utils_ovn.go
+++ b/lxd/network/network_utils_ovn.go
@@ -29,14 +29,3 @@ func OVNInstanceDevicePortDelete(network Network, instanceID int, deviceName str
 
 	return n.instanceDevicePortDelete(instanceID, deviceName)
 }
-
-// OVNInstanceDeviceMTU returns the MTU that should be used for an OVN instance device.
-func OVNInstanceDeviceMTU(network Network) (uint32, error) {
-	// Check network is of type OVN.
-	n, ok := network.(*ovn)
-	if !ok {
-		return 0, fmt.Errorf("Network is not OVN type")
-	}
-
-	return n.getBridgeMTU(), nil
-}

From fb736aa95fda2ed776214fa246b61f6925a6027e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 12:30:29 +0100
Subject: [PATCH 08/11] lxd/network/driver/ovn: Updates getBridgeMTU() to not
 depend on ovnGeneveTunnelMTU

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 5482dd0583..c566cdc851 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -127,19 +127,19 @@ func (n *ovn) getClient() (*openvswitch.OVN, error) {
 	return client, nil
 }
 
-// getBridgeMTU returns MTU that should be used for the bridge and instance devices. Will also be used to configure
-// the OVN DHCP and IPv6 RA options.
+// getBridgeMTU returns MTU that should be used for the bridge and instance devices.
+// Will also be used to configure the OVN DHCP and IPv6 RA options. Returns 0 if the bridge.mtu is not set/invalid.
 func (n *ovn) getBridgeMTU() uint32 {
 	if n.config["bridge.mtu"] != "" {
 		mtu, err := strconv.ParseUint(n.config["bridge.mtu"], 10, 32)
 		if err != nil {
-			return ovnGeneveTunnelMTU
+			return 0
 		}
 
 		return uint32(mtu)
 	}
 
-	return ovnGeneveTunnelMTU
+	return 0
 }
 
 // getNetworkPrefix returns OVN network prefix to use for object names.

From 42fdd5f6ff47841c4b621acdcb42c2177ece90dd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 12:31:10 +0100
Subject: [PATCH 09/11] lxd/network/driver/ovn: Adds getOptimalBridgeMTU and
 getUnderlayInfo functions

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index c566cdc851..da4108331e 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -142,6 +142,82 @@ func (n *ovn) getBridgeMTU() uint32 {
 	return 0
 }
 
+// getUnderlayInfo returns the MTU for the underlay network interface and the enscapsulation IP for OVN tunnels.
+func (n *ovn) getUnderlayInfo() (uint32, net.IP, error) {
+	ovs := openvswitch.NewOVS()
+	encapIP, err := ovs.OVNEncapIP()
+	if err != nil {
+		return 0, nil, errors.Wrapf(err, "Failed getting OVN enscapsulation IP from OVS")
+	}
+
+	// Look for interface that has the OVN enscapsulation IP assigned.
+	ifaces, err := net.Interfaces()
+	if err != nil {
+		return 0, nil, errors.Wrapf(err, "Failed getting local network interfaces")
+	}
+
+	var encapMTU uint32
+
+	for _, iface := range ifaces {
+		addrs, err := iface.Addrs()
+		if err != nil {
+			continue
+		}
+
+		for _, addr := range addrs {
+			ip, _, err := net.ParseCIDR(addr.String())
+			if err != nil {
+				continue
+			}
+
+			if ip.Equal(encapIP) {
+				encapMTU, err = GetDevMTU(iface.Name)
+				if err != nil {
+					return 0, nil, errors.Wrapf(err, "Failed getting MTU for %q", iface.Name)
+				}
+			}
+		}
+	}
+
+	if encapMTU <= 0 {
+		return 0, nil, fmt.Errorf("No matching interface found for OVN enscapsulation IP %q", encapIP.String())
+	}
+
+	return encapMTU, encapIP, nil
+}
+
+// getOptimalBridgeMTU returns the MTU that can be used for the bridge and instance devices based on the MTU value
+// of the OVN underlay network interface. This assumes that the OVN tunnel mechanism used is geneve and that the
+// same underlying network settings (MTU and encapsulation IP family) are used on all OVN nodes.
+func (n *ovn) getOptimalBridgeMTU() (uint32, error) {
+	// Get underlay MTU and encapsulation IP.
+	underlayMTU, encapIP, err := n.getUnderlayInfo()
+	if err != nil {
+		return 0, errors.Wrapf(err, "Failed getting OVN underlay info")
+	}
+
+	// Encapsulation family is IPv6.
+	if encapIP.To4() == nil {
+		// If the underlay's MTU is large enough to accomodate a 1500 overlay MTU and the geneve tunnel
+		// overhead of 78 bytes (when used with IPv6 encapsulation) then indicate 1500 MTU can be used.
+		if underlayMTU >= 1578 {
+			return 1500, nil
+		}
+
+		// Default to 1422 which can work with an underlay MTU of 1500.
+		return 1422, nil
+	}
+
+	// If the underlay's MTU is large enough to accomodate a 1500 overlay MTU and the geneve tunnel
+	// overhead of 58 bytes (when used with IPv4 encapsulation) then indicate 1500 MTU can be used.
+	if underlayMTU >= 1558 {
+		return 1500, nil
+	}
+
+	// Default to 1442 which can work with underlay MTU of 1500.
+	return 1442, nil
+}
+
 // getNetworkPrefix returns OVN network prefix to use for object names.
 func (n *ovn) getNetworkPrefix() string {
 	return fmt.Sprintf("lxd-net%d", n.id)

From 735226d74a4b09f532da811b945af1581255cf68 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 12:31:45 +0100
Subject: [PATCH 10/11] lxd/network/driver/ovn: Updates setup to generate an
 optimal bridge.mtu setting if not specified manually

The bridge.mtu setting is derived from the OVN underlay network's MTU and encapsulation IP address family.

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

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index da4108331e..ae4560e691 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -838,6 +838,30 @@ func (n *ovn) setup(update bool) error {
 	var routerExtPortIPv4, routerIntPortIPv4, routerExtPortIPv6, routerIntPortIPv6 net.IP
 	var routerExtPortIPv4Net, routerIntPortIPv4Net, routerExtPortIPv6Net, routerIntPortIPv6Net *net.IPNet
 
+	// Get bridge MTU to use.
+	bridgeMTU := n.getBridgeMTU()
+	if bridgeMTU == 0 {
+		// If no manual bridge MTU specified, derive it from the underlay network.
+		bridgeMTU, err = n.getOptimalBridgeMTU()
+		if err != nil {
+			return errors.Wrapf(err, "Failed getting optimal bridge MTU")
+		}
+
+		// Save to config so the value can be read by instances connecting to network.
+		n.config["bridge.mtu"] = fmt.Sprintf("%d", bridgeMTU)
+		err := n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
+			err = tx.UpdateNetwork(n.id, n.description, n.config)
+			if err != nil {
+				return errors.Wrapf(err, "Failed saving optimal bridge MTU")
+			}
+
+			return nil
+		})
+		if err != nil {
+			return err
+		}
+	}
+
 	// Get router MAC address.
 	routerMAC, err := n.getRouterMAC()
 	if err != nil {
@@ -1043,7 +1067,7 @@ func (n *ovn) setup(update bool) error {
 		RecursiveDNSServer: parent.dnsIPv4,
 		DomainName:         n.getDomainName(),
 		LeaseTime:          time.Duration(time.Hour * 1),
-		MTU:                n.getBridgeMTU(),
+		MTU:                bridgeMTU,
 	})
 	if err != nil {
 		return errors.Wrapf(err, "Failed adding DHCPv4 settings for internal switch")
@@ -1089,7 +1113,7 @@ func (n *ovn) setup(update bool) error {
 			SendPeriodic:       true,
 			DNSSearchList:      n.getDNSSearchList(),
 			RecursiveDNSServer: parent.dnsIPv6,
-			MTU:                n.getBridgeMTU(),
+			MTU:                bridgeMTU,
 
 			// Keep these low until we support DNS search domains via DHCPv4, as otherwise RA DNSSL
 			// won't take effect until advert after DHCPv4 has run on instance.

From c44796b1d248fa446aafa7c9d699347a3cb5c427 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 22 Sep 2020 12:29:02 +0100
Subject: [PATCH 11/11] lxd/device/nic/ovn: Read mtu directly from parent
 network config bridge.mtu setting

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

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 66c58ab4f3..79aa018feb 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -125,12 +125,7 @@ func (d *nicOVN) validateConfig(instConf instance.ConfigReader) error {
 	}
 
 	// Apply network level config options to device config before validation.
-	mtu, err := network.OVNInstanceDeviceMTU(n)
-	if err != nil {
-		return err
-	}
-
-	d.config["mtu"] = fmt.Sprintf("%d", mtu)
+	d.config["mtu"] = fmt.Sprintf("%s", netConfig["bridge.mtu"])
 
 	rules := nicValidationRules(requiredFields, optionalFields)
 


More information about the lxc-devel mailing list