Add per-route MTU option (LP: #1860201)

[hrasiq]

This patch introduces a way to set MTU for specific routes. It adds an 'mtu'
field to the NetplanIPRoute struct, and makes use of already existing route
handling code to add the MTU-related fields for networkd and NM backends.

Checklist

• Runs make check successfully.
• Retains 100% code coverage (make check-coverage).
• New/changed keys in YAML format are documented.
• (Optional) Closes an open bug in Launchpad. (LP: #1860201)

[codecov-commenter]

Codecov Report

Merging #160 into master will increase coverage by 1.54%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master      #160      +/-   ##
===========================================
+ Coverage   98.45%   100.00%   +1.54%     
===========================================
  Files          41        41              
  Lines        6230      5172    -1058     
===========================================
- Hits         6134      5172     -962     
+ Misses         96         0      -96     

Impacted Files	Coverage Δ
src/networkd.c	100.00% <100.00%> (+1.93%)	⬆️
src/nm.c	100.00% <100.00%> (+1.98%)	⬆️
src/parse.c	100.00% <100.00%> (+3.84%)	⬆️
tests/generator/test_errors.py	100.00% <100.00%> (ø)
tests/generator/test_routing.py	100.00% <100.00%> (ø)
src/validation.c	100.00% <0.00%> (+9.25%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85134d1...3f70bbb. Read the comment docs.

[slyon]

Thank you @hrasiq, this is looking very good!

I requested some some small inline changes (mostly just wording and some refactoring, to avoid duplicated code). And I'd be glad if you could work on those suggestions.

Also, it would be great if you could add a small integration test inside tests/integration/routing.py within the _CommonTests() class (so it is run for networkd and NetworkManager backends). E.g. similiar to the test_route_on_link function of that file, where you bring up the interface using you new config option and then verify it on the running system via a ip route ... + regex, to match for the relevant string.
This would be especially interesting, as I could not find a documentation about the NetworkManager route-data/mtu config key and it would be great if it could be verified to be working via integration-/autopkgtests.

[vorlonofportland]

fine from a schema perspective.

[hrasiq]

Thanks for the review, @slyon!

On the NM mtu key: I didn't find explicit mention of it, just entries like NM_IP_ROUTE_ATTRIBUTE_MTU [0]. This seems to be the same for the onlink key, and I tried following that for MTU.

I've been doing more tests with those on Ubuntu Bionic and Focal, and even if NetworkManager itself seems to pick up the mtu key (it shows up in nmcli connection show <con> as an attribute under ipv4.routes), it doesn't seem to be reflected in the actual routes (i.e. ip route doesn't pick up the new MTU values). I'm a bit confused as to why, but it does seem like something in NM is not passing those correctly down to the actual routes.

I'll implement the changes you've requested, and also give the integration test a try. I'm thinking of removing the MTU sections from the NM backend and just error out early, since I haven't been able to get it to work in my tests. Do you have any other suggestions, or does that make sense?

[0] https://developer.gnome.org/libnm/stable/NMSettingIPConfig.html#NM-IP-ROUTE-ATTRIBUTE-MTU:CAPS

[slyon]

Thank you @hrasiq

Okay... if the MTU shows up in nmcli it must be recognized by NM. I'm not sure how NM is supposed to pass that information down to the routing tables... I found this reference talking about MTUs in the routing cache, maybe you could test if it is showing up there: http://linux-ip.net/html/tools-ip-route.html Or you could try using wireshark/tcpdump to verify what the actual MTU size is on that route, once you configured it via NM...

For the integration test it would be great if you could find a way to verify the MTU is actually passed through to ip route. If not, I would not want to drop the mtu attribute, though, as it is picked up by NM. So if you cannot find a way to verify it via ip route (i.e. this might be a bug in NM passing it down to the routing table...), you could at least verify that NM picked it up via the ipv4.routes "mtu" attribute in the integration test. Once the (potential) bug is fixed in NM, there are no changes needed on our side.

[codecov-io]

Codecov Report

Merging #160 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #160   +/-   ##
=======================================
  Coverage   98.88%   98.88%           
=======================================
  Files          48       48           
  Lines        6737     6753   +16     
=======================================
+ Hits         6662     6678   +16     
  Misses         75       75           

Impacted Files	Coverage Δ
src/parse.c	100.00% <ø> (ø)
src/networkd.c	100.00% <100.00%> (ø)
src/nm.c	100.00% <100.00%> (ø)
tests/generator/test_errors.py	100.00% <100.00%> (ø)
tests/generator/test_routing.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4efd4e7...6519228. Read the comment docs.

[hrasiq]

@slyon Apologies for the long delays on this one. I've sorted out the NetworkManager issues on picking up the route MTU values, and it seems to be working fine now.

I've addressed the changes you've requested, and added an integration test as well. It passed correctly on my tests with Ubuntu Bionic and Focal, but please let me know if you'd like any changes there.

Thanks again for the review!

[slyon]

Thank you very much for your continued work @hrasiq
This is looking very good!

Let me run a full set of integration test on this, before I merge it.

Update:
All tests pass on hirsute as well. Great work! Merging.

autopkgtest [11:03:30]: @@@@@@@@@@@@@@@@@@@@ summary
ovs                  PASS
ethernets            PASS
bridges              PASS
bonds                PASS
routing              PASS
vlans                PASS
wifi                 PASS
tunnels              PASS
scenarios            PASS
regressions          PASS
autostart            PASS
cloud-init           PASS
qemu-system-x86_64: terminating on signal 15 from pid 395416 (/usr/bin/python3)