netplan-developers team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/netplan:fix-mtu into netplan:master.

Requested reviews:
  Developers of netplan (netplan-developers)
Related bugs:
  Bug #1668693 in nplan (Ubuntu): "netplan fails to parse 'mtu' key"
  https://bugs.launchpad.net/ubuntu/+source/nplan/+bug/1668693

For more details, see:
https://code.launchpad.net/~raharper/netplan/+git/netplan/+merge/318545

Add support for MTU setting in all network devices

Any interface, physical or netdev can have a .link file to configure the MTU
of a device (eth, bond, bridge, etc).

(LP: #1668693)

-- 
Your team Developers of netplan is requested to review the proposed merge of ~raharper/netplan:fix-mtu into netplan:master.

diff --git a/src/networkd.c b/src/networkd.c
index 20b0621..ba60123 100644
--- a/src/networkd.c
+++ b/src/networkd.c
@@ -85,10 +85,8 @@ write_link_file(net_definition* def, const char* rootdir, const char* path)
 {
     GString* s = NULL;
 
-    g_assert(def->type < ND_VIRTUAL);
-
     /* do we need to write a .link file? */
-    if (!def->set_name && !def->wake_on_lan)
+    if (!def->set_name && !def->wake_on_lan && !def->mtubytes)
         return;
 
     /* build file contents */
@@ -100,6 +98,9 @@ write_link_file(net_definition* def, const char* rootdir, const char* path)
         g_string_append_printf(s, "Name=%s\n", def->set_name);
     /* FIXME: Should this be turned from bool to str and support multiple values? */
     g_string_append_printf(s, "WakeOnLan=%s\n", def->wake_on_lan ? "magic" : "off");
+    if (def->mtubytes)
+        g_string_append_printf(s, "MTUBytes=%u\n", def->mtubytes);
+
 
     g_string_free_to_file(s, rootdir, path, ".link");
 }
@@ -326,9 +327,7 @@ write_networkd_conf(net_definition* def, const char* rootdir)
 
     /* We want this for all backends when renaming, as *.link files are
      * evaluated by udev, not networkd itself or NetworkManager. */
-    if (def->type < ND_VIRTUAL &&
-            (def->backend == BACKEND_NETWORKD || def->set_name))
-        write_link_file(def, rootdir, path_base);
+    write_link_file(def, rootdir, path_base);
 
     if (def->backend != BACKEND_NETWORKD) {
         g_debug("networkd: definition %s is not for us (backend %i)", def->id, def->backend);
diff --git a/src/parse.c b/src/parse.c
index 345c8ad..ff05e0c 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -865,75 +865,80 @@ const mapping_entry_handler nameservers_handlers[] = {
 };
 
 const mapping_entry_handler ethernet_def_handlers[] = {
-    {"set-name", YAML_SCALAR_NODE, handle_netdef_str, NULL, netdef_offset(set_name)},
-    {"match", YAML_MAPPING_NODE, handle_match},
-    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
-    {"wakeonlan", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(wake_on_lan)},
+    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"dhcp4", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp4)},
     {"dhcp6", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp6)},
-    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"gateway4", YAML_SCALAR_NODE, handle_gateway4},
     {"gateway6", YAML_SCALAR_NODE, handle_gateway6},
+    {"match", YAML_MAPPING_NODE, handle_match},
+    {"mtu", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(mtubytes)},
     {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
+    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
     {"routes", YAML_SEQUENCE_NODE, handle_routes},
+    {"set-name", YAML_SCALAR_NODE, handle_netdef_str, NULL, netdef_offset(set_name)},
+    {"wakeonlan", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(wake_on_lan)},
     {NULL}
 };
 
 const mapping_entry_handler wifi_def_handlers[] = {
-    {"set-name", YAML_SCALAR_NODE, handle_netdef_str, NULL, netdef_offset(set_name)},
-    {"match", YAML_MAPPING_NODE, handle_match},
-    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
-    {"wakeonlan", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(wake_on_lan)},
+    {"access-points", YAML_MAPPING_NODE, handle_wifi_access_points},
+    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"dhcp4", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp4)},
     {"dhcp6", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp6)},
-    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"gateway4", YAML_SCALAR_NODE, handle_gateway4},
     {"gateway6", YAML_SCALAR_NODE, handle_gateway6},
+    {"match", YAML_MAPPING_NODE, handle_match},
+    {"mtu", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(mtubytes)},
     {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
-    {"access-points", YAML_MAPPING_NODE, handle_wifi_access_points},
+    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
     {"routes", YAML_SEQUENCE_NODE, handle_routes},
+    {"set-name", YAML_SCALAR_NODE, handle_netdef_str, NULL, netdef_offset(set_name)},
+    {"wakeonlan", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(wake_on_lan)},
     {NULL}
 };
 
 const mapping_entry_handler bridge_def_handlers[] = {
-    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
+    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"dhcp4", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp4)},
     {"dhcp6", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp6)},
-    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"gateway4", YAML_SCALAR_NODE, handle_gateway4},
     {"gateway6", YAML_SCALAR_NODE, handle_gateway6},
-    {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
     {"interfaces", YAML_SEQUENCE_NODE, handle_interfaces, NULL, netdef_offset(bridge)},
+    {"mtu", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(mtubytes)},
+    {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
     {"parameters", YAML_MAPPING_NODE, NULL, bridge_params_handlers},
+    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
     {"routes", YAML_SEQUENCE_NODE, handle_routes},
     {NULL}
 };
 
 const mapping_entry_handler bond_def_handlers[] = {
-    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
+    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"dhcp4", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp4)},
     {"dhcp6", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp6)},
-    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"gateway4", YAML_SCALAR_NODE, handle_gateway4},
     {"gateway6", YAML_SCALAR_NODE, handle_gateway6},
-    {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
     {"interfaces", YAML_SEQUENCE_NODE, handle_interfaces, NULL, netdef_offset(bond)},
     {"macaddress", YAML_SCALAR_NODE, handle_netdef_mac, NULL, netdef_offset(set_mac)},
-    {"routes", YAML_SEQUENCE_NODE, handle_routes},
+    {"mtu", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(mtubytes)},
+    {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
     {"parameters", YAML_MAPPING_NODE, handle_bonding},
+    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
+    {"routes", YAML_SEQUENCE_NODE, handle_routes},
     {NULL}
 };
 
 const mapping_entry_handler vlan_def_handlers[] = {
-    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
+    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"dhcp4", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp4)},
     {"dhcp6", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(dhcp6)},
-    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},
     {"gateway4", YAML_SCALAR_NODE, handle_gateway4},
     {"gateway6", YAML_SCALAR_NODE, handle_gateway6},
-    {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
     {"id", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(vlan_id)},
     {"link", YAML_SCALAR_NODE, handle_netdef_id_ref, NULL, netdef_offset(vlan_link)},
+    {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
+    {"mtu", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(mtubytes)},
+    {"renderer", YAML_SCALAR_NODE, handle_netdef_renderer},
     {"routes", YAML_SEQUENCE_NODE, handle_routes},
     {NULL}
 };
diff --git a/src/parse.h b/src/parse.h
index 75d6531..9a6c95f 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -75,6 +75,9 @@ typedef struct net_definition {
     /* Configured custom MAC address */
     char* set_mac;
 
+    /* interface mtu */
+    guint mtubytes;
+
     /* these properties are only valid for physical interfaces (type < ND_VIRTUAL) */
     char* set_name;
     struct {
diff --git a/tests/generate.py b/tests/generate.py
index e168283..36fe0f4 100755
--- a/tests/generate.py
+++ b/tests/generate.py
@@ -23,6 +23,7 @@ import re
 import sys
 import stat
 import tempfile
+import textwrap
 import subprocess
 import unittest
 
@@ -275,6 +276,42 @@ unmanaged-devices+=interface-name:eth0,''')
         # should not allow NM to manage everything
         self.assertFalse(os.path.exists(self.nm_enable_all_conf))
 
+    def test_eth_mtu(self):
+        self.generate('''network:
+  version: 2
+  ethernets:
+    eth1:
+      mtu: 1280
+      dhcp4: n''')
+
+        self.assert_networkd({'eth1.link': '[Match]\nOriginalName=eth1\n\n[Link]\nWakeOnLan=off\nMTUBytes=1280\n'})
+
+    def test_mtu_all(self):
+        self.generate(textwrap.dedent("""
+            network:
+              version: 2
+              ethernets:
+                eth1:
+                  mtu: 1280
+                  dhcp4: n
+              bonds:
+                bond0:
+                  interfaces:
+                  - eth1
+                  mtu: 9000
+              vlans:
+                bond0.108:
+                  link: bond0
+                  id: 108"""))
+        self.assert_networkd({
+            'bond0.108.netdev': '[NetDev]\nName=bond0.108\nKind=vlan\n\n[VLAN]\nId=108\n',
+            'bond0.link': '[Match]\n\n[Link]\nWakeOnLan=off\nMTUBytes=9000\n',
+            'bond0.netdev': '[NetDev]\nName=bond0\nKind=bond\n',
+            'bond0.network': '[Match]\nName=bond0\n\n[Network]\nVLAN=bond0.108\n',
+            'eth1.link': '[Match]\nOriginalName=eth1\n\n[Link]\nWakeOnLan=off\nMTUBytes=1280\n',
+            'eth1.network': '[Match]\nName=eth1\n\n[Network]\nBond=bond0\nLinkLocalAddressing=no\nIPv6AcceptRA=no\n'
+        })
+
     def test_eth_match_by_driver_rename(self):
         self.generate('''network:
   version: 2
@@ -1043,7 +1080,7 @@ method=link-local
 '''})
         # should allow NM to manage everything else
         self.assertTrue(os.path.exists(self.nm_enable_all_conf))
-        self.assert_networkd({})
+        self.assert_networkd({'eth0.link': '[Match]\nOriginalName=eth0\n\n[Link]\nWakeOnLan=magic\n'})
         self.assert_udev(None)
 
     def test_eth_match_by_driver(self):