Repository: moby/moby (language: go)

Issue:
Docker daemon MTU not used when creating a new bridge network
**Description**
I configure the `/etc/docker/daemon.json` with the "mtu" parameter to 1450.
Default bridge is ok with mtu=1450.
But when a new bridge network is created (either `docker network create `or `docker-compose`), the mtu parameter is false. It's always 1500.


**Steps to reproduce the issue:**
1. Add `"mtu": 1450 `to `/etc/docker/daemon.json`
2. Restart docker daemon
3. Create a new bridge : `docker network create my-net`

**Describe the results you received:**
my-net MTU = 1500 

**Describe the results you expected:**
my-net MTU = 1450

**Output of `docker version`:**

```
Client:
 Version:      17.06.2-ce
 API version:  1.30
 Go version:   go1.8.3
 Git commit:   cec0b72
 Built:        Tue Sep  5 20:00:17 2017
 OS/Arch:      linux/amd64

Server:
 Version:      17.06.2-ce
 API version:  1.30 (minimum version 1.12)
 Go version:   go1.8.3
 Git commit:   cec0b72
 Built:        Tue Sep  5 19:59:11 2017
 OS/Arch:      linux/amd64
 Experimental: false
```

**Output of `docker info`:**

```
Containers: 4
 Running: 1
 Paused: 0
 Stopped: 3
Images: 41
Server Version: 17.06.2-ce
Storage Driver: aufs
 Root Dir: /var/lib/docker/100000.100000/aufs
 Backing Filesystem: extfs
 Dirs: 163
 Dirperm1 Supported: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: bridge host macvlan null overlay
 Log: awslogs fluentd gcplogs gelf journald json-file logentries splunk syslog
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: 6e23458c129b551d5c9871e5174f6b1b7f6d1170
runc version: 810190ceaa507aa2727d7ae6f4790c76ec150bd2
init version: 949e6fa
Security Options:
 apparmor
 seccomp
  Profile: default
 userns
Kernel Version: 4.4.0-93-generic
Operating System: Ubuntu 16.04.3 LTS
OSType: linux
Architecture: x86_64
CPUs: 2
Total Memory: 15.67GiB
Name: test-vm-04
ID: OZSP:AWLJ:HAPO:CZD6:NNLX:YDIT:LGWR:RMEI:P4SK:SBY4:2RAT:5YIK
Docker Root Dir: /var/lib/docker/100000.100000
Debug Mode (client): false
Debug Mode (server): false
Registry: https://index.docker.io/v1/
Experimental: false
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: true
```



Discussion / hints:
If I'm not mistaken, the `--mtu` option is indeed only for the default bridge network (because that network cannot be customised using `docker network create`), for custom networks, the options can be passed through the `com.docker.network.xyz` options, as described here: https://docs.docker.com/engine/userguide/networking/work-with-networks/

I do see this is not being mentioned in the documentation (i.e., the "equivalent" does not mention that applies to the _default_ bridge network), also we should update the dockerd flag descriptions for those flags that only apply to the default network.

ping @mstanleyjones @mavenugo perhaps you're able to help pointing out which options only apply to the default bridge network
@thaJeztah I'm ok with your remark but the problem is when you have an openstack environment, all networks should have a mtu=1450.
I think if we can have an default option for MTU when we are created a network, it could be very appreciate. If default MTU change, a stop/start for the network without update the configuration should be better.
I faced the same situation on openstack instance.
When I run curl to specific page, i cannot get GET responce.Hence I used tcpdump for debug, I found the MTU should be 1450.
So, I added /etc/docker/daemon.json ({"mtu": 1450}), then restart docker.service, but the http get doesn't works properly.
:+1: 
Have you reported an issue with the openstack maintainers?
I think it's not an openstack problem. 
If a host has a MTU < 1500, docker should detect the size and adapt the data packets.

Auto-detecting MTU was removed at some point, because it would still be hit-and-miss, and cause issues in various situations; see https://github.com/moby/moby/pull/18108
Ok I understand the reasons but in this case, is it possible to add a parameter to configure by default all containers with a default MTU. Otherwise, we have to add it on each `docker run `or `docker-compose` ?
> I think it's not an openstack problem.

Yes, I wanna change MTU of NIC which is created and used by docker.
We're running OpenStack(which is using VXLan) and we have to explicitly set the MTU for all overlay networks that are created.

It would be useful to have an option to set the default MTU for all networks as right now using some else's compose file doesn't work without modifications.
Hi @agxs , does `--mtu` option to `dockerd` not do what you want? You can also set it in the `daemon.json` (recommended) as:

```json
...
mtu: "1500",
...
```
@mistyhacks that doesn't affect swarm networks. Sorry if I wasn't clear.
That's surprising and I wonder if that is a bug. @aaronlehmann @stevvooe  🙏 
@mistyhacks no, it's not a bug, the `--mtu` option is only for the default bridge network (both because it predates the "custom networks", and because that one cannot be modified manually); 

Custom networks and overlay networks can have a custom MTU defined on a per-network basis (through the [`com.docker.network.driver.mtu` option](https://docs.docker.com/engine/reference/commandline/network_create/#specify-advanced-options)).
@thaJeztah I understand but when you use Openstack (for example) you have to put a custom MTU for each network you want to create. So you have to update all docker-compose files.
And if openstack change the MTU, all your containers will be KO. So you have to update all your docker-compose and restart again. It's difficult and dangerous in production environment.

> So it would be useful to have an option to set the default MTU for all networks as right now using some else's compose file doesn't work without modifications.
I just realised that you can specify multiple compose files to a `docker-compose` command, so that makes things easier when overriding network settings and saves having to modify the original file.
Maybe I'll try again.

Can we set default configurations for network drivers when docker starts instead of asking the world to rewrite their recipe/compose file ?

Astonishing the number of easy fixes to improve way of life that are simply ignored for multiple years.
Would really love to see this feature!

Fix patch:
diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go
index be332626ae098..a90c1bbee62b3 100644
--- a/cmd/dockerd/config.go
+++ b/cmd/dockerd/config.go
@@ -27,6 +27,7 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.StringVar(&conf.ContainerdAddr, "containerd", "", "containerd grpc address")
 	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.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")
diff --git a/daemon/config/config.go b/daemon/config/config.go
index 42f7add76d809..60b215227a436 100644
--- a/daemon/config/config.go
+++ b/daemon/config/config.go
@@ -126,6 +126,8 @@ type NetworkConfig struct {
 	DefaultAddressPools opts.PoolsOpt `json:"default-address-pools,omitempty"`
 	// NetworkControlPlaneMTU allows to specify the control plane MTU, this will allow to optimize the network use in some components
 	NetworkControlPlaneMTU int `json:"network-control-plane-mtu,omitempty"`
+	// Default options for newly created networks
+	DefaultNetworkOpts map[string]map[string]string `json:"default-network-opts,omitempty"`
 }
 
 // TLSOptions defines TLS configuration for the daemon server.
@@ -289,6 +291,7 @@ func New() (*Config, error) {
 			Mtu:                    DefaultNetworkMtu,
 			NetworkConfig: NetworkConfig{
 				NetworkControlPlaneMTU: DefaultNetworkMtu,
+				DefaultNetworkOpts:     make(map[string]map[string]string),
 			},
 			ContainerdNamespace:       DefaultContainersNamespace,
 			ContainerdPluginNamespace: DefaultPluginNamespace,
diff --git a/daemon/network.go b/daemon/network.go
index 5b54d5d296245..3bf8852d4fc25 100644
--- a/daemon/network.go
+++ b/daemon/network.go
@@ -315,9 +315,22 @@ func (daemon *Daemon) createNetwork(create types.NetworkCreateRequest, id string
 		driver = c.Config().DefaultDriver
 	}
 
+	networkOptions := make(map[string]string)
+	for k, v := range create.Options {
+		networkOptions[k] = v
+	}
+	if defaultOpts, ok := daemon.configStore.DefaultNetworkOpts[driver]; create.ConfigFrom == nil && ok {
+		for k, v := range defaultOpts {
+			if _, ok := networkOptions[k]; !ok {
+				logrus.WithFields(logrus.Fields{"driver": driver, "network": id, k: v}).Debug("Applying network default option")
+				networkOptions[k] = v
+			}
+		}
+	}
+
 	nwOptions := []libnetwork.NetworkOption{
 		libnetwork.NetworkOptionEnableIPv6(create.EnableIPv6),
-		libnetwork.NetworkOptionDriverOpts(create.Options),
+		libnetwork.NetworkOptionDriverOpts(networkOptions),
 		libnetwork.NetworkOptionLabels(create.Labels),
 		libnetwork.NetworkOptionAttachable(create.Attachable),
 		libnetwork.NetworkOptionIngress(create.Ingress),
diff --git a/opts/opts.go b/opts/opts.go
index aacd30af08a50..dbcd0285258a7 100644
--- a/opts/opts.go
+++ b/opts/opts.go
@@ -146,6 +146,83 @@ func (o *NamedListOpts) Name() string {
 	return o.name
 }
 
+// NamedMapMapOpts is a MapMapOpts with a configuration name.
+// This struct is useful to keep reference to the assigned
+// field name in the internal configuration struct.
+type NamedMapMapOpts struct {
+	name string
+	MapMapOpts
+}
+
+// NewNamedMapMapOpts creates a reference to a new NamedMapOpts struct.
+func NewNamedMapMapOpts(name string, values map[string]map[string]string, validator ValidatorFctType) *NamedMapMapOpts {
+	return &NamedMapMapOpts{
+		name:       name,
+		MapMapOpts: *NewMapMapOpts(values, validator),
+	}
+}
+
+// Name returns the name of the NamedListOpts in the configuration.
+func (o *NamedMapMapOpts) Name() string {
+	return o.name
+}
+
+// MapMapOpts holds a map of maps of values and a validation function.
+type MapMapOpts struct {
+	values    map[string]map[string]string
+	validator ValidatorFctType
+}
+
+// Set validates if needed the input value and add it to the
+// internal map, by splitting on '='.
+func (opts *MapMapOpts) Set(value string) error {
+	if opts.validator != nil {
+		v, err := opts.validator(value)
+		if err != nil {
+			return err
+		}
+		value = v
+	}
+	rk, rv, found := strings.Cut(value, "=")
+	if !found {
+		return fmt.Errorf("invalid value %q for map option, should be root-key=key=value", value)
+	}
+	k, v, found := strings.Cut(rv, "=")
+	if !found {
+		return fmt.Errorf("invalid value %q for map option, should be root-key=key=value", value)
+	}
+	if _, ok := opts.values[rk]; !ok {
+		opts.values[rk] = make(map[string]string)
+	}
+	opts.values[rk][k] = v
+	return nil
+}
+
+// GetAll returns the values of MapOpts as a map.
+func (opts *MapMapOpts) GetAll() map[string]map[string]string {
+	return opts.values
+}
+
+func (opts *MapMapOpts) String() string {
+	return fmt.Sprintf("%v", opts.values)
+}
+
+// Type returns a string name for this Option type
+func (opts *MapMapOpts) Type() string {
+	return "mapmap"
+}
+
+// NewMapMapOpts creates a new MapMapOpts with the specified map of values and a validator.
+func NewMapMapOpts(values map[string]map[string]string, validator ValidatorFctType) *MapMapOpts {
+	if values == nil {
+		values = make(map[string]map[string]string)
+	}
+	return &MapMapOpts{
+		values:    values,
+		validator: validator,
+	}
+}
+
 // MapOpts holds a map of values and a validation function.
 type MapOpts struct {
 	values    map[string]string


Test patch:
diff --git a/integration/network/network_test.go b/integration/network/network_test.go
index 00603094a2bca..bfc6e2998e315 100644
--- a/integration/network/network_test.go
+++ b/integration/network/network_test.go
@@ -4,12 +4,14 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"os/exec"
 	"strings"
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	ntypes "github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/integration/internal/network"
 	"github.com/docker/docker/testutil/daemon"
@@ -175,3 +177,75 @@ func TestHostIPv4BridgeLabel(t *testing.T) {
 	// Make sure the SNAT rule exists
 	icmd.RunCommand("iptables", "-t", "nat", "-C", "POSTROUTING", "-s", out.IPAM.Config[0].Subnet, "!", "-o", bridgeName, "-j", "SNAT", "--to-source", ipv4SNATAddr).Assert(t, icmd.Success)
 }
+
+func TestDefaultNetworkOpts(t *testing.T) {
+	skip.If(t, testEnv.OSType == "windows")
+	skip.If(t, testEnv.IsRemoteDaemon)
+	skip.If(t, testEnv.IsRootless, "rootless mode has different view of network")
+
+	tests := []struct {
+		name       string
+		mtu        int
+		configFrom bool
+		args       []string
+	}{
+		{
+			name: "default value",
+			mtu:  1500,
+			args: []string{},
+		},
+		{
+			name: "cmdline value",
+			mtu:  1234,
+			args: []string{"--default-network-opt", "bridge=com.docker.network.driver.mtu=1234"},
+		},
+		{
+			name:       "config-from value",
+			configFrom: true,
+			mtu:        1233,
+			args:       []string{"--default-network-opt", "bridge=com.docker.network.driver.mtu=1234"},
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			d := daemon.New(t)
+			d.StartWithBusybox(t, tc.args...)
+			defer d.Stop(t)
+			c := d.NewClientT(t)
+			defer c.Close()
+			ctx := context.Background()
+
+			if tc.configFrom {
+				// Create a new network config
+				network.CreateNoError(ctx, t, c, "from-net", func(create *types.NetworkCreate) {
+					create.ConfigOnly = true
+					create.Options = map[string]string{
+						"com.docker.network.driver.mtu": fmt.Sprint(tc.mtu),
+					}
+				})
+				defer c.NetworkRemove(ctx, "from-net")
+			}
+
+			// Create a new network
+			networkName := "testnet"
+			network.CreateNoError(ctx, t, c, networkName, func(create *types.NetworkCreate) {
+				if tc.configFrom {
+					create.ConfigFrom = &ntypes.ConfigReference{
+						Network: "from-net",
+					}
+				}
+			})
+			defer c.NetworkRemove(ctx, networkName)
+
+			// 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, types.ContainerRemoveOptions{Force: true})
+
+			result, err := container.Exec(ctx, c, id1, []string{"ip", "l", "show", "eth0"})
+			assert.NilError(t, err)
+			assert.Check(t, is.Contains(result.Combined(), fmt.Sprintf(" mtu %d ", tc.mtu)), "Network MTU should have been set to %d", tc.mtu)
+		})
+	}
+}
diff --git a/opts/opts_test.go b/opts/opts_test.go
index 850618e320689..fe4b7f5ce5c43 100644
--- a/opts/opts_test.go
+++ b/opts/opts_test.go
@@ -334,3 +334,34 @@ func TestParseLink(t *testing.T) {
 		assert.Check(t, is.Equal(alias, "bar"))
 	})
 }
+
+func TestMapMapOpts(t *testing.T) {
+	tmpMap := make(map[string]map[string]string)
+	validator := func(val string) (string, error) {
+		if strings.HasPrefix(val, "invalid-key=") {
+			return "", fmt.Errorf("invalid key %s", val)
+		}
+		return val, nil
+	}
+	o := NewMapMapOpts(tmpMap, validator)
+	o.Set("r1=k11=v11")
+	assert.Check(t, is.DeepEqual(tmpMap, map[string]map[string]string{"r1": {"k11": "v11"}}))
+
+	o.Set("r2=k21=v21")
+	assert.Check(t, is.Len(tmpMap, 2))
+
+	if err := o.Set("invalid-syntax"); err == nil {
+		t.Error("invalid mapping syntax is not being caught")
+	}
+
+	if err := o.Set("k=invalid-syntax"); err == nil {
+		t.Error("invalid value syntax is not being caught")
+	}
+
+	o.Set("r1=k12=v12")
+	assert.Check(t, is.DeepEqual(tmpMap["r1"], map[string]string{"k11": "v11", "k12": "v12"}))
+
+	if o.Set("invalid-key={\"k\":\"v\"}") == nil {
+		t.Error("validator is not being called")
+	}
+}