Skip to content

Commit

Permalink
Explicitly set MTU on bridge devices.
Browse files Browse the repository at this point in the history
This is purely cosmetic - if a non-default MTU is configured, the bridge
will have the default MTU=1500 until a container's 'veth' is connected
and an MTU is set on the veth. That's a disconcerting, it looks like the
config has been ignored - so, set the bridge's MTU explicitly.

Fixes moby#37937

Signed-off-by: Rob Murray <rob.murray@docker.com>
  • Loading branch information
robmry committed Nov 27, 2023
1 parent ce1ee98 commit 964ab71
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
11 changes: 10 additions & 1 deletion integration/network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func TestDefaultNetworkOpts(t *testing.T) {

// Create a new network
networkName := "testnet"
network.CreateNoError(ctx, t, c, networkName, func(create *types.NetworkCreate) {
networkId := network.CreateNoError(ctx, t, c, networkName, func(create *types.NetworkCreate) {
if tc.configFrom {
create.ConfigFrom = &ntypes.ConfigReference{
Network: "from-net",
Expand All @@ -249,6 +249,15 @@ func TestDefaultNetworkOpts(t *testing.T) {
})
defer c.NetworkRemove(ctx, networkName)

// Check the MTU of the bridge itself, before any devices are connected. (The
// bridge's MTU will be set to the minimum MTU of anything connected to it, but
// it's set explicitly on the bridge anyway - so it doesn't look like the option
// was ignored.)
cmd := exec.Command("ip", "link", "show", "br-"+networkId[:12])
output, err := cmd.CombinedOutput()
assert.NilError(t, err)
assert.Check(t, is.Contains(string(output), fmt.Sprintf(" mtu %d ", tc.mtu)), "Bridge MTU should have been set to %d", tc.mtu)

// Start a container to inspect the MTU of its network interface
id1 := container.Run(ctx, t, c, container.WithNetworkMode(networkName))
defer c.ContainerRemove(ctx, id1, containertypes.RemoveOptions{Force: true})
Expand Down
8 changes: 8 additions & 0 deletions libnetwork/drivers/bridge/bridge_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,14 @@ func (d *driver) createNetwork(config *networkConfiguration) (err error) {
bridgeSetup.queueStep(setupDefaultSysctl)
}

// Always set the bridge's MTU if specified. This is purely cosmetic; a bridge's
// MTU is the min MTU of device connected to it, and MTU will be set on each
// 'veth'. But, for a non-default MTU, the bridge's MTU will look wrong until a
// container is attached.
if config.Mtu > 0 {
bridgeSetup.queueStep(setupMTU)
}

// Even if a bridge exists try to setup IPv4.
bridgeSetup.queueStep(setupBridgeIPv4)

Expand Down
8 changes: 8 additions & 0 deletions libnetwork/drivers/bridge/setup_device_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ func setupDevice(config *networkConfiguration, i *bridgeInterface) error {
return nil
}

func setupMTU(config *networkConfiguration, i *bridgeInterface) error {
if err := i.nlh.LinkSetMTU(i.Link, config.Mtu); err != nil {
log.G(context.TODO()).WithError(err).Errorf("Failed to set bridge MTU %s via netlink", config.BridgeName)
return err
}
return nil
}

func setupDefaultSysctl(config *networkConfiguration, i *bridgeInterface) error {
// Disable IPv6 router advertisements originating on the bridge
sysPath := filepath.Join("/proc/sys/net/ipv6/conf/", config.BridgeName, "accept_ra")
Expand Down

0 comments on commit 964ab71

Please sign in to comment.