The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7913
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
- Auto generates `bridge.mtu` setting for OVN networks if not specified (on create and update).
- Bridge MTU setting is derived from OVN's underlay network's `ovn-encap-ip`, which is used to ascertain the underlay network interface's MTU and IP address family (which both influence the maximum MTU size that can be used without fragmentation in the overlay network due to geneve tunnel overhead).
From 6a43c6dcc11ed81d42a91f352ff70cc1d8216206 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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)
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
