netplan ignores NetworkManager ipv4.route-metric (LP: #2076172)

[calexandru2018]

Background

Dummy connections (from my testing) are not being passed the ipv4.route-metric value from NM, instead they have the default value of 550, though this behavior was not observed with the IPv6 protocol. The change is to allow to pass ipv4.route-metric to NM if either dhcp4: True or ipv4.route-metric values are passed using a method: manual setup.

This was tested by cloning the repo, making the code change and following build install instructions.

You can also re-create this by running the following commands.

With ipv4.method manual:

• Runnmcli c a type dummy ifname pvpnksintrf0 con-name pvpn-killswitch ipv4.method manual ipv4.addresses "100.85.0.1/24" ipv4.gateway "100.85.0.1" ipv6.method manual ipv6.addresses "fdeb:446c:912d:08da::/64" ipv6.gateway "fdeb:446c:912d:08da::1" ipv4.route-metric 98 ipv6.route-metric 95
• nmcli c s pvpn-killswitch shows that ipv6.route-metric has been taken but not ipv4.route-metric, even though the data is present under /etc/netplan/90*.yml

The current behavior breaks the Proton VPN linux app on all Ubuntu 24.04 machines.

What is expected: ipv4.method manual

• The ipv4.route-metric value should be passed regardless of ipv4.method

Checklist

• Runs make check successfully. (for some reason fails locally due to linting and two failing tests which do not seem related to CI tests)
• Retains 100% code coverage (make check-coverage).
• New/changed keys in YAML format are documented.
• (Optional) Adds example YAML for new feature.
• (Optional) Closes an open bug in Launchpad: https://bugs.launchpad.net/netplan/+bug/2076172

[daniloegea]

Hi, thanks for your contribution.

So, this is what nm-settings(5) says about `route-metric:

ipv4.route-metric
The metric applies to dynamic routes, manual (static) routes that don't have an explicit
metric setting, address prefix routes, and the default route.

So you seem to be right, we shouldn't set it based on DHCP with NetworkManager. Interestingly we seem to be doing the right thing for IPv6... I suppose we should do the same for IPv4.

The problem is that the field dhcp_overrides.metric is mapped to the YAML property dhcp4-overrides.route-metric. So we basically don't allow it to be set outside of the DHCP context... It works well for networkd, where this YAML settings ends up in [DHCP].RouteMetric but Network Manager has different settings.

This change broke lots of unit tests. The reason is that the fix is not quite correct. We don't want to emit the route-metric setting when the metric is unspecified. Checking the test results, I see that netplan started to add route-metric=4294967295 to Network Manager files.

If you create a simple interface with nmcli con add type dummy ifname dummy0 ipv4.method auto you will see the same in the .nmconnection file.

I believe that something like this is more appropriate:

if ((def->dhcp4 || def->ip4_addresses || def->gateway4 || def->routes) && def->dhcp4_overrides.metric != NETPLAN_METRIC_UNSPEC)

It would also be nice to have some tests checking that the route-metric is being added at least for the test case you added to the PR.

Thanks again for reporting the problem and working on a fix for it :)

[calexandru2018]

Thanks for the feedback 👍🏻

Not sure why but when running meson test -C build --verbose codestyle and make check complains about codestyle, do you have an idea on why it could be failing from the master branch ?

edit:

test:         codestyle
start time:   13:43:56
duration:     2.30s
result:       exit status 1
command:      UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=79 /usr/bin/pycodestyle --max-line-length=130 '--exclude=doc/env,*meson-private/pycompile.py' /home/test/Documents/netplan
----------------------------------- stdout -----------------------------------
/home/test/Documents/netplan/_build/meson-private/pycompile.py:20:12: E401 multiple imports on one line
/home/test/Documents/netplan/_build/meson-private/pycompile.py:25:1: E302 expected 2 blank lines, found 1
/home/test/Documents/netplan/_build/meson-private/pycompile.py:50:1: E302 expected 2 blank lines, found 1
/home/test/Documents/netplan/_build/meson-private/pycompile.py:56:1: E305 expected 2 blank lines after class or function definition, found 1

I know how to solve this, I'm just wondering why it's happening since the master branch seems fine.

[daniloegea]

Oh it's because you have two build directories "build" and "_build". Sadly the style checker is looking at one of them. The "_build" one was created by "make check".

That is how I build an test Netplan:

meson setup build --prefix=/usr -Db_coverage=true
meson compile -C build -v
meson test -C build -v

[calexandru2018]

Hey @daniloegea I've added a couple of tests, independent test for ipv4 and ipv6, ensuring that route-metric is passed without specifying the method: auto to force the route being applied (which would enable dhcp*).

Still had issues with make meson test -C build -v as codestyle was failing even on master branch.

[daniloegea]

Thank you. It's looking good to me.

@calexandru2018 have you tested the latest commit against Proton VPN and checked that it resolved the problem?

@slyon What do you think? Network Manager uses the route-metric setting for manual addresses as well but we map it to dhcp(4|6)-overrides.route-metric, which is confusing. We already do something similar to what is proposed in this PR for IPv6 so it sounds fine.

[calexandru2018]

Hey @daniloegea and @slyon I confirm that this is working and resolves the problem. 👍🏻

[calexandru2018]

@daniloegea do I need to make any changes or are we just waiting for @slyon to review ? Asking mainly because I see "Change requested" and I'm not sure if this is still the case or not.

[slyon]

Thanks for all the discussion and for addressing Danilo's remarks! I can see that the CI is all green now, including your new test case.

I think the logic isn't semantically fully correct. The root cause for the confusion is in parse-nm.c, which should parse NM's ipv4/6.route-metric setting to another/additional internal data field if there is only static configuration and no DHCP, as the "dhcp4/6-overrides" Netplan setting doesn't really match the static IP case (semantically). We should probably introduce a new Netplan YAML setting, something like routes.default-metric (or similar), to be used for such cases.

Anyway, given the precedence we have in the IPv6 configuration and for consistency’s sake, I agree that this is an acceptable workaround for the given bug/issue. The dhcp4-overrides.route-metric Netplan setting is already written today (producing wrong semantics), so this patch only improves the situation by correctly mapping it back to the NM keyfile.

Thank you very much for your contribution to Netplan, let's get this merged!

[calexandru2018]

Hey @slyon thanks for merging.

Would it be possible to have an estimate of when this fix would land in Ubuntu repos ? We currently have a workaround in place which we'd like to remove as soon as this goes live.

[slyon]

@calexandru2018 I'm about to cut a Netplan 1.1 release, that should land in the Ubuntu development series (24.10, Oracular) in the next few weeks, which would be including your fix.

What Ubuntu version are you targetting with this fix?

The backport of Netplan 1.1 to 24.04 LTS (Noble) usually takes a bit longer and can potentially be expected towards the end of the year.

If you think the deployment of this fix should be accelerated, feel free to create an SRU bug report on the netplan.io package, so we might be able to land a targetted fix sooner than the full version backport.

File a bug: https://bugs.launchpad.net/ubuntu/+source/netplan.io/+filebug

[calexandru2018]

@slyon we detected this on Ubuntu 24.04 and we have a portion of users impacted by this thus we'd like to have this back-ported asap. I'll do what you suggested thank you. Should I wait until 1.1 is released for 24.10 or should I submit a SRU bug report straight away ?

[slyon]

@calexandru2018 You should probably wait for the pending backport of v1.0.1 in Ubuntu 24.04 (https://bugs.launchpad.net/ubuntu/+source/netplan.io/+bug/2074197), then apply your fixes on top of that.