Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LXD lowering host bridge MTU #11919

Merged
merged 3 commits into from
Jul 4, 2023
Merged

Fix LXD lowering host bridge MTU #11919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ed5dcc
lxd/device/veth: Fix MTU handling
stgraber Jul 4, 2023
569da41
lxd/device/tap: Fix MTU handling
stgraber Jul 4, 2023
d05d299
tests: Update for bridged host MTU behavior
stgraber Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 42 additions & 17 deletions lxd/device/device_utils_network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,24 +223,45 @@ func networkCreateVethPair(hostName string, m deviceConfig.Device) (string, uint
},
}

// Set the MTU on both ends. If not specified and has parent, will inherit MTU from parent.
// Set the MTU on both ends.
// The host side should always line up with the bridge to avoid accidentally lowering the bridge MTU.
// The instance side should use the configured MTU (if any), if not, it should match the host side.
var instanceMTU uint32
var parentMTU uint32

if m["parent"] != "" {
mtu, err := network.GetDevMTU(m["parent"])
if err != nil {
return "", 0, fmt.Errorf("Failed to get the parent MTU: %w", err)
}

parentMTU = uint32(mtu)
}

if m["mtu"] != "" {
mtu, err := strconv.ParseUint(m["mtu"], 10, 32)
if err != nil {
return "", 0, fmt.Errorf("Invalid MTU specified: %w", err)
}

veth.MTU = uint32(mtu)
} else if m["parent"] != "" {
mtu, err := network.GetDevMTU(m["parent"])
if err != nil {
return "", 0, fmt.Errorf("Failed to get the parent MTU: %w", err)
}
instanceMTU = uint32(mtu)
}

if instanceMTU == 0 && parentMTU > 0 {
instanceMTU = parentMTU
}

veth.MTU = mtu
if parentMTU == 0 && instanceMTU > 0 {
parentMTU = instanceMTU
}

veth.Peer.MTU = veth.MTU
if instanceMTU > 0 {
veth.Peer.MTU = instanceMTU
}

if parentMTU > 0 {
veth.MTU = parentMTU
}

// Set the MAC address on peer.
if m["hwaddr"] != "" {
Expand Down Expand Up @@ -276,7 +297,7 @@ func networkCreateVethPair(hostName string, m deviceConfig.Device) (string, uint
return "", 0, fmt.Errorf("Failed to create the veth interfaces %q and %q: %w", hostName, veth.Peer.Name, err)
}

return veth.Peer.Name, veth.MTU, nil
return veth.Peer.Name, veth.Peer.MTU, nil
}

// networkCreateTap creates and configures a TAP device.
Expand Down Expand Up @@ -304,7 +325,9 @@ func networkCreateTap(hostName string, m deviceConfig.Device) (uint32, error) {

revert.Add(func() { _ = network.InterfaceRemove(hostName) })

// Set the MTU on peer. If not specified and has parent, will inherit MTU from parent.
// Set the MTU on both ends.
// The host side should always line up with the bridge to avoid accidentally lowering the bridge MTU.
// The instance side should use the configured MTU (if any), if not, it should match the host side.
var mtu uint32
if m["mtu"] != "" {
nicMTU, err := strconv.ParseUint(m["mtu"], 10, 32)
Expand All @@ -313,20 +336,22 @@ func networkCreateTap(hostName string, m deviceConfig.Device) (uint32, error) {
}

mtu = uint32(nicMTU)
} else if m["parent"] != "" {
}

if m["parent"] != "" {
parentMTU, err := network.GetDevMTU(m["parent"])
if err != nil {
return 0, fmt.Errorf("Failed to get the parent MTU: %w", err)
}

mtu = parentMTU
}

if mtu > 0 {
err = NetworkSetDevMTU(hostName, mtu)
err = NetworkSetDevMTU(hostName, parentMTU)
if err != nil {
return 0, fmt.Errorf("Failed to set the MTU %d: %w", mtu, err)
}

if mtu == 0 {
mtu = parentMTU
}
}

// Set TX queue length on both ends.
Expand Down
12 changes: 6 additions & 6 deletions test/suites/container_devices_nic_bridged.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ test_container_devices_nic_bridged() {
false
fi

# Check profile custom MTU is applied on host side of veth.
if ! grep "1400" /sys/class/net/"${vethHostName}"/mtu ; then
# Check profile custom MTU doesn't affect the host.
if ! grep "1500" /sys/class/net/"${vethHostName}"/mtu ; then
echo "host veth mtu invalid"
false
fi
Expand Down Expand Up @@ -170,8 +170,8 @@ test_container_devices_nic_bridged() {
false
fi

# Check custom MTU is applied host-side on hot-plug.
if ! grep "1401" /sys/class/net/"${vethHostName}"/mtu ; then
# Check custom MTU doesn't affect the host.
if ! grep "1500" /sys/class/net/"${vethHostName}"/mtu ; then
echo "host veth mtu invalid"
false
fi
Expand Down Expand Up @@ -221,8 +221,8 @@ test_container_devices_nic_bridged() {
false
fi

# Checl profile custom MTU is applied host-side on hot-removal.
if ! grep "1400" /sys/class/net/"${vethHostName}"/mtu ; then
# Check custom MTU doesn't affect the host.
if ! grep "1500" /sys/class/net/"${vethHostName}"/mtu ; then
echo "host veth mtu invalid"
false
fi
Expand Down