Merge pull request #45887 from thaJeztah/move_mtu

daemon/config: move MTU to BridgeConfig, and warn when using on Windows

cmd/dockerd/config.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"runtime"
+
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/registry"
@@ -28,7 +30,16 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.BoolVar(&conf.CriContainerd, "cri-containerd", false, "start containerd with cri")
 
 	flags.Var(opts.NewNamedMapMapOpts("default-network-opts", conf.DefaultNetworkOpts, nil), "default-network-opt", "Default network options")
-	flags.IntVar(&conf.Mtu, "mtu", conf.Mtu, "Set the containers network MTU")
+	flags.IntVar(&conf.MTU, "mtu", conf.MTU, `Set the MTU for the default "bridge" network`)
+	if runtime.GOOS == "windows" {
+		// The mtu option is not used on Windows, but it has been available since
+		// "forever" (and always silently ignored). We hide the flag for now,
+		// to discourage using it (and print a warning if it's set), but not
+		// "hard-deprecating" it, to not break users, and in case it will be
+		// supported on Windows in future.
+		flags.MarkHidden("mtu")
+	}
+
 	flags.IntVar(&conf.NetworkControlPlaneMTU, "network-control-plane-mtu", conf.NetworkControlPlaneMTU, "Network Control plane MTU")
 	flags.IntVar(&conf.NetworkDiagnosticPort, "network-diagnostic-port", 0, "TCP port number of the network diagnostic server")
 	_ = flags.MarkHidden("network-diagnostic-port")

daemon/config/config.go

@@ -151,7 +151,6 @@ type CommonConfig struct {
 	GraphDriver           string   `json:"storage-driver,omitempty"`
 	GraphOptions          []string `json:"storage-opts,omitempty"`
 	Labels                []string `json:"labels,omitempty"`
-	Mtu                   int      `json:"mtu,omitempty"`
 	NetworkDiagnosticPort int      `json:"network-diagnostic-port,omitempty"`
 	Pidfile               string   `json:"pidfile,omitempty"`
 	RawLogs               bool     `json:"raw-logs,omitempty"`
@@ -280,7 +279,7 @@ func New() (*Config, error) {
 			MaxConcurrentDownloads: DefaultMaxConcurrentDownloads,
 			MaxConcurrentUploads:   DefaultMaxConcurrentUploads,
 			MaxDownloadAttempts:    DefaultDownloadAttempts,
-			Mtu:                    DefaultNetworkMtu,
+			BridgeConfig:           BridgeConfig{MTU: DefaultNetworkMtu},
 			NetworkConfig: NetworkConfig{
 				NetworkControlPlaneMTU: DefaultNetworkMtu,
 				DefaultNetworkOpts:     make(map[string]map[string]string),
@@ -615,8 +614,8 @@ func Validate(config *Config) error {
 	}
 
 	// TODO(thaJeztah) Validations below should not accept "0" to be valid; see Validate() for a more in-depth description of this problem
-	if config.Mtu < 0 {
-		return errors.Errorf("invalid default MTU: %d", config.Mtu)
+	if config.MTU < 0 {
+		return errors.Errorf("invalid default MTU: %d", config.MTU)
 	}
 	if config.MaxConcurrentDownloads < 0 {
 		return errors.Errorf("invalid max concurrent downloads: %d", config.MaxConcurrentDownloads)

daemon/config/config_linux.go

@@ -37,6 +37,7 @@ type BridgeConfig struct {
 	commonBridgeConfig
 
 	// Fields below here are platform specific.
+	MTU                         int    `json:"mtu,omitempty"`
 	DefaultIP                   net.IP `json:"ip,omitempty"`
 	IP                          string `json:"bip,omitempty"`
 	DefaultGatewayIPv4          net.IP `json:"default-gateway,omitempty"`

daemon/config/config_test.go

@@ -286,7 +286,7 @@ func TestValidateConfigurationErrors(t *testing.T) {
 			name: "negative MTU",
 			config: &Config{
 				CommonConfig: CommonConfig{
-					Mtu: -10,
+					BridgeConfig: BridgeConfig{MTU: -10},
 				},
 			},
 			expectedErr: "invalid default MTU: -10",
@@ -440,10 +440,10 @@ func TestValidateConfiguration(t *testing.T) {
 		},
 		{
 			name:  "with mtu",
-			field: "Mtu",
+			field: "MTU",
 			config: &Config{
 				CommonConfig: CommonConfig{
-					Mtu: 1234,
+					BridgeConfig: BridgeConfig{MTU: 1234},
 				},
 			},
 		},

daemon/config/config_windows.go

@@ -1,8 +1,11 @@
 package config // import "github.com/docker/docker/daemon/config"
 
 import (
+	"context"
 	"os"
 	"path/filepath"
+
+	"github.com/containerd/containerd/log"
 )
 
 const (
@@ -16,6 +19,10 @@ const (
 // configuration.
 type BridgeConfig struct {
 	commonBridgeConfig
+
+	// MTU is not actually used on Windows, but the --mtu option has always
+	// been there on Windows (but ignored).
+	MTU int `json:"mtu,omitempty"`
 }
 
 // Config defines the configuration of a docker daemon.
@@ -45,6 +52,9 @@ func (conf *Config) IsSwarmCompatible() error {
 
 // ValidatePlatformConfig checks if any platform-specific configuration settings are invalid.
 func (conf *Config) ValidatePlatformConfig() error {
+	if conf.MTU != 0 && conf.MTU != DefaultNetworkMtu {
+		log.G(context.TODO()).Warn(`WARNING: MTU for the default network is not configurable on Windows, and this option will be ignored.`)
+	}
 	return nil
 }
 

daemon/daemon_unix.go

@@ -883,7 +883,7 @@ func configureNetworking(controller *libnetwork.Controller, conf *config.Config)
 
 	if !conf.DisableBridge {
 		// Initialize default driver "bridge"
-		if err := initBridgeDriver(controller, conf); err != nil {
+		if err := initBridgeDriver(controller, conf.BridgeConfig); err != nil {
 			return err
 		}
 	} else {
@@ -922,22 +922,22 @@ func driverOptions(config *config.Config) nwconfig.Option {
 	})
 }
 
-func initBridgeDriver(controller *libnetwork.Controller, config *config.Config) error {
+func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig) error {
 	bridgeName := bridge.DefaultBridgeName
-	if config.BridgeConfig.Iface != "" {
-		bridgeName = config.BridgeConfig.Iface
+	if cfg.Iface != "" {
+		bridgeName = cfg.Iface
 	}
 	netOption := map[string]string{
 		bridge.BridgeName:         bridgeName,
 		bridge.DefaultBridge:      strconv.FormatBool(true),
-		netlabel.DriverMTU:        strconv.Itoa(config.Mtu),
-		bridge.EnableIPMasquerade: strconv.FormatBool(config.BridgeConfig.EnableIPMasq),
-		bridge.EnableICC:          strconv.FormatBool(config.BridgeConfig.InterContainerCommunication),
+		netlabel.DriverMTU:        strconv.Itoa(cfg.MTU),
+		bridge.EnableIPMasquerade: strconv.FormatBool(cfg.EnableIPMasq),
+		bridge.EnableICC:          strconv.FormatBool(cfg.InterContainerCommunication),
 	}
 
 	// --ip processing
-	if config.BridgeConfig.DefaultIP != nil {
-		netOption[bridge.DefaultBindingIP] = config.BridgeConfig.DefaultIP.String()
+	if cfg.DefaultIP != nil {
+		netOption[bridge.DefaultBindingIP] = cfg.DefaultIP.String()
 	}
 
 	ipamV4Conf := &libnetwork.IpamConf{AuxAddresses: make(map[string]string)}
@@ -954,8 +954,8 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 
 	if len(nwList) > 0 {
 		nw := nwList[0]
-		if len(nwList) > 1 && config.BridgeConfig.FixedCIDR != "" {
-			_, fCIDR, err := net.ParseCIDR(config.BridgeConfig.FixedCIDR)
+		if len(nwList) > 1 && cfg.FixedCIDR != "" {
+			_, fCIDR, err := net.ParseCIDR(cfg.FixedCIDR)
 			if err != nil {
 				return errors.Wrap(err, "parse CIDR failed")
 			}
@@ -975,8 +975,8 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 		}
 	}
 
-	if config.BridgeConfig.IP != "" {
-		ip, ipNet, err := net.ParseCIDR(config.BridgeConfig.IP)
+	if cfg.IP != "" {
+		ip, ipNet, err := net.ParseCIDR(cfg.IP)
 		if err != nil {
 			return err
 		}
@@ -986,8 +986,8 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 		log.G(context.TODO()).Infof("Default bridge (%s) is assigned with an IP address %s. Daemon option --bip can be used to set a preferred IP address", bridgeName, ipamV4Conf.PreferredPool)
 	}
 
-	if config.BridgeConfig.FixedCIDR != "" {
-		_, fCIDR, err := net.ParseCIDR(config.BridgeConfig.FixedCIDR)
+	if cfg.FixedCIDR != "" {
+		_, fCIDR, err := net.ParseCIDR(cfg.FixedCIDR)
 		if err != nil {
 			return err
 		}
@@ -998,8 +998,8 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 		}
 	}
 
-	if config.BridgeConfig.DefaultGatewayIPv4 != nil {
-		ipamV4Conf.AuxAddresses["DefaultGatewayIPv4"] = config.BridgeConfig.DefaultGatewayIPv4.String()
+	if cfg.DefaultGatewayIPv4 != nil {
+		ipamV4Conf.AuxAddresses["DefaultGatewayIPv4"] = cfg.DefaultGatewayIPv4.String()
 	}
 
 	var (
@@ -1007,10 +1007,10 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 		ipamV6Conf     *libnetwork.IpamConf
 	)
 
-	if config.BridgeConfig.EnableIPv6 && config.BridgeConfig.FixedCIDRv6 == "" {
+	if cfg.EnableIPv6 && cfg.FixedCIDRv6 == "" {
 		return errdefs.InvalidParameter(errors.New("IPv6 is enabled for the default bridge, but no subnet is configured. Specify an IPv6 subnet using --fixed-cidr-v6"))
-	} else if config.BridgeConfig.FixedCIDRv6 != "" {
-		_, fCIDRv6, err := net.ParseCIDR(config.BridgeConfig.FixedCIDRv6)
+	} else if cfg.FixedCIDRv6 != "" {
+		_, fCIDRv6, err := net.ParseCIDR(cfg.FixedCIDRv6)
 		if err != nil {
 			return err
 		}
@@ -1040,11 +1040,11 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 		}
 	}
 
-	if config.BridgeConfig.DefaultGatewayIPv6 != nil {
+	if cfg.DefaultGatewayIPv6 != nil {
 		if ipamV6Conf == nil {
 			ipamV6Conf = &libnetwork.IpamConf{AuxAddresses: make(map[string]string)}
 		}
-		ipamV6Conf.AuxAddresses["DefaultGatewayIPv6"] = config.BridgeConfig.DefaultGatewayIPv6.String()
+		ipamV6Conf.AuxAddresses["DefaultGatewayIPv6"] = cfg.DefaultGatewayIPv6.String()
 	}
 
 	v4Conf := []*libnetwork.IpamConf{ipamV4Conf}
@@ -1054,7 +1054,7 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 	}
 	// Initialize default network on "bridge" with the same name
 	_, err = controller.NewNetwork("bridge", "bridge", "",
-		libnetwork.NetworkOptionEnableIPv6(config.BridgeConfig.EnableIPv6),
+		libnetwork.NetworkOptionEnableIPv6(cfg.EnableIPv6),
 		libnetwork.NetworkOptionDriverOpts(netOption),
 		libnetwork.NetworkOptionIpam("default", "", v4Conf, v6Conf, nil),
 		libnetwork.NetworkOptionDeferIPv6Alloc(deferIPv6Alloc))

daemon/daemon_windows.go

@@ -397,7 +397,7 @@ func (daemon *Daemon) initNetworkController(daemonCfg *config.Config, activeSand
 
 	if !daemonCfg.DisableBridge {
 		// Initialize default driver "bridge"
-		if err := initBridgeDriver(daemon.netController, daemonCfg); err != nil {
+		if err := initBridgeDriver(daemon.netController, daemonCfg.BridgeConfig); err != nil {
 			return err
 		}
 	}
@@ -405,7 +405,7 @@ func (daemon *Daemon) initNetworkController(daemonCfg *config.Config, activeSand
 	return nil
 }
 
-func initBridgeDriver(controller *libnetwork.Controller, config *config.Config) error {
+func initBridgeDriver(controller *libnetwork.Controller, config config.BridgeConfig) error {
 	if _, err := controller.NetworkByName(runconfig.DefaultDaemonNetworkMode().NetworkName()); err == nil {
 		return nil
 	}
@@ -417,8 +417,8 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
 	var ipamOption libnetwork.NetworkOption
 	var subnetPrefix string
 
-	if config.BridgeConfig.FixedCIDR != "" {
-		subnetPrefix = config.BridgeConfig.FixedCIDR
+	if config.FixedCIDR != "" {
+		subnetPrefix = config.FixedCIDR
 	}
 
 	if subnetPrefix != "" {