Cannot set offload options to false

[Magnus Hyllander]

[Impact]
In https://bugs.launchpad.net/netplan/+bug/1771740, the ability to set the offload options that are exposed by systemd.link was added. Unfortunately, the implementation only allows setting the offload options to true. Setting the options to false results in nothing being written to the generated .link file, because "false" is treated the same as "unset" by netplan.

[Test Plan]
In addition to running & passing the full set of unit- and integration-tests
(that contains new tests to check for this new feature), as described in
https://wiki.ubuntu.com/NetplanUpdates we want to run the following commands
to make sure the link offload stanza is working properly:

$ cat /etc/netplan/test.yaml
network:
  version: 2
  ethernets:
    eth1:
      receive-checksum-offload: false
      transmit-checksum-offload: false
      tcp-segmentation-offload: false
      tcp6-segmentation-offload: false
      generic-segmentation-offload: false
      generic-receive-offload: false
      large-receive-offload: false
$ netplan apply
$ cat /run/systemd/network/10-netplan-eth1.link | grep Offload
=> Make sure the *Offload settings are correctly set to "=false".
$ ethtool -k eth1
=> Make sure the offloading options have been set to "off":
rx-checksumming: off
tx-checksumming: off
tcp-segmentation-offload: off
tx-tcp6-segmentation: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off
$ reboot
$ ethtool -k eth1
=> Make sure the offloading options have been set to "off" after reboot:
rx-checksumming: off
tx-checksumming: off
tcp-segmentation-offload: off
tx-tcp6-segmentation: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off

Furthermore, we want to validate the device renaming still works as expected and the new "udev test IFACE" call in "netplan apply" does not have side effects:
$ ip addr
=> observe interface name, e.g. "enp5s0"
$ netplan get # update your netplan config to match this output, adapting the MAC address of enp5s0
network:
  version: 2
  ethernets:
    id0:
      match:
        macaddress: "00:16:3e:1a:23:dd"
      dhcp4: true
      dhcp6: true
      set-name: "eth99"
$ netplan apply
$ ip addr
=> observe that "enp5s0" got renamed to "eth99"
$ netplan set ethernets.id0.set-name=eth42
$ netplan get
network:
  version: 2
  ethernets:
    id0:
      match:
        macaddress: "00:16:3e:1a:23:dd"
      dhcp4: true
      dhcp6: true
      set-name: "eth42"
$ reboot
=> observe the interface's name has now changed to "eth42"

autopkgtest logs:
* Jammy:
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/jammy-amd64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/jammy-arm64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/jammy-armhf.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/jammy-ppc64el.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/jammy-s390x.log

* Impish: EOL

* Focal:
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/focal-amd64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/focal-arm64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/focal-armhf.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/focal-ppc64el.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1956264/focal-s390x.log

[Where problems could occur]

 * The settings exist since systemd-232 which means Bionic and up can use
   this feature

This upload touches netplan's generator, if anything goes wrong it could impact
the rendering of network configuration and break a system's network connectivity

[Other Info]
The full set of autopkgtest logs will be attached after the upload is accepted
into -proposed and the tests have been run on the official autopkgtest.u.c
infrastructure.

=== original description ===

In https://bugs.launchpad.net/netplan/+bug/1771740, the ability to set the offload options that are exposed by systemd.link was added. Unfortunately, the implementation only allows setting the offload options to true. Setting the options to false results in nothing being written to the generated .link file, because "false" is treated the same as "unset" by netplan.

See the current implementation https://git.launchpad.net/netplan/tree/src/networkd.c#n260.

netplan needs to handle three different values for the offload options: "true", "false", and "unset"

(I have a specific case in mind, https://bugzilla.kernel.org/show_bug.cgi?id=118721, where I need to set TSO to off when using the e1000e driver. The current netplan implementation does not support that; as a workaround I created a separate .link file in /etc/systemd/network to disable TSO.)

[Lukas Märdian (slyon)]

According to @rlaager on LP: #1771740 systemd-networkd should be able to handle those settings as a tristate unset/on/off. This needs to be confirmed and if valid, netplan should do the same.

> Upon further investigation, I see that the systemd networkd settings have similar documentation only listing true and unset. But the systemd NEWS file explicitly talks about disabling and the settings are parsed in networkd using config_parse_tristate, so I think networkd properly handles =0 on these options. (I'm still trying to test it.)

[Lukas Märdian (slyon)]

This is being worked on in https://github.com/canonical/netplan/pull/270

[Nicolas Bock (nicolasbock)]

Hi Lukas,

I have added the ABI check and tristate changes to the package for Focal.

[Richard Laager (rlaager)]

I built a package using focal.debdiff from comment #3 and re-tested on a different machine from my previous testing. It works as expected.

[Lukas Märdian (slyon)]

Hey, thank you for providing a focal.debdiff.

To get this change landed in Focal, we need to follow the full SRU process, though. Could you please file an SRU template, according to https://wiki.ubuntu.com/StableReleaseUpdates ?

Also, a few remarks about the debdiff:
1/ The version number for Focal should be 0.104-0ubuntu2~20.04.2, according to https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging (as this is a backport on top of -0ubuntu2, not -0ubuntu3)

2/ I'd like to reduce the backport to just the relevant changes, not pulling in the whole ABI validation logic, similar to what I've prepared for Kinetic: https://git.launchpad.net/~ubuntu-core-dev/netplan/+git/ubuntu/commit/?id=dc3b335e9a3ba0d22f7a62c52818e1c3ed2e6a3c

3/ We need to land this change in Kinetic (I've uploaded it recently), Jammy, Impish and then Focal, going top down and doing an SRU for each of them. Could you please file a git Merge-Proposal for each of them (instead of providing many debdiffs) on launchpad against those branches?
https://git.launchpad.net/~ubuntu-core-dev/netplan/+git/ubuntu/log/?h=ubuntu/jammy
https://git.launchpad.net/~ubuntu-core-dev/netplan/+git/ubuntu/log/?h=ubuntu/impish
https://git.launchpad.net/~ubuntu-core-dev/netplan/+git/ubuntu/log/?h=ubuntu/focal

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "focal.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Launchpad Janitor (janitor)]

This bug was fixed in the package netplan.io - 0.104-0ubuntu4

---------------
netplan.io (0.104-0ubuntu4) kinetic; urgency=medium

  * Cherry-pick upstream fix (565bd0f) for ethernets autopkgtest:
    - d/p/0004-tests-ethernets-fix-autopkgtest-with-alternating-def.patch
  * Cherry-pick upstream fix (8934a1b) for NM WPA3 passthrough:
    - d/p/0005-nm-fix-rendering-of-password-for-unknown-passthrough.patch

 -- Lukas Märdian <email address hidden> Thu, 19 May 2022 12:48:35 +0200

[Nicolas Bock (nicolasbock)]

Lukas,

I opened merge request

https://code.launchpad.net/~nicolasbock/netplan/+git/netplan/+merge/424017

Please let me know if I did that correctly.

Thanks!

[Lukas Märdian (slyon)]

Thank you very much! I left some comments inside the merge-proposal.

Please also adopt the description of this bug report according to the SRU template in https://wiki.ubuntu.com/StableReleaseUpdates#SRU_Bug_Template, describing a proper test case/reporducer and impact analysis, to make it fit for SRU.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Jim Pye (jimpye)]

Lukas

Could I also suggest that in the test plan that a reboot of the machine is included.

This will confirm the settings in the .yaml/.link files are preserved and are still applied during a startup.

Jim

[Lukas Märdian (slyon)]

Absolutely, adopted!

[Steve Langasek (vorlon)]

I am concerned by the changes included in netplan/cli/commands/apply.py in this patch, which seem only indirectly related to the bug described here; and the tests that have been added only test the offload support, whereas these changes to the handling of device renaming seem like they could impact unrelated devices. Could we see some additional tests here around the device renaming behavior, or else an explanation for why this cannot affect any behavior aside from the offload handling options?

[Lukas Märdian (slyon)]

Good catch, Steve! I've adopted the [Test Plan] section to add a check for the interface rename logic for "netplan apply" and "reboot" (netplan generator) cases.

[Steve Langasek (vorlon)]

Hello Magnus, or anyone else affected,

Accepted netplan.io into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/netplan.io/0.104-0ubuntu2.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Steve Langasek (vorlon)]

Hello Magnus, or anyone else affected,

Accepted netplan.io into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/netplan.io/0.104-0ubuntu2~20.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted netplan.io (0.104-0ubuntu2.1) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

netplan.io/0.104-0ubuntu2.1 (arm64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#netplan.io

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted netplan.io (0.104-0ubuntu2~20.04.2) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

initramfs-tools/unknown (s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#netplan.io

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Lukas Märdian (slyon)]

I attached all autopkgtest logs to the bug description.

Impish is EOL

[Lukas Märdian (slyon)]

I tested netplan.io 0.104-0ubuntu2.1 from jammy-proposed inside a VM and inside an LXD container.

* udev values (i.e. the ".link" file) contains the correct "=false" values.
* ethtool -k show the values have been correctly set to "off" (on "netplan apply" and "reboot")
  => it's noteworthy that some ethtool values are fixed in this setup and thus cannot be changed:
     large-receive-offload: off [fixed] # VM & container
     rx-checksumming: on [fixed] # VM only, works in container
* The renaming of the interface works as expected inside the VM ("netplan apply" & "reboot")
* The renaming of the interface works inside the container on "netplan apply"
  => it's noteworthy that the interface is not renamed after "reboot" inside the LXD container, this is most probably due to some LXD udev quirks and should not be considered to be a regression, as it happened the same before. I confirmed this using the previous version from -release.

root@jammy-vm:~# cat /run/systemd/network/10-netplan-enp5s0.link | grep Offload
ReceiveChecksumOffload=false
TransmitChecksumOffload=false
TCPSegmentationOffload=false
TCP6SegmentationOffload=false
GenericSegmentationOffload=false
GenericReceiveOffload=false
LargeReceiveOffload=false
root@jammy-vm:~# ethtool -k enp5s0
Features for enp5s0:
rx-checksumming: on [fixed]
tx-checksumming: off
tcp-segmentation-offload: off
 tx-tcp6-segmentation: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off [fixed]

root@jj:~# ethtool -k eth0
Features for eth0:
rx-checksumming: off
tx-checksumming: off
tcp-segmentation-offload: off
 tx-tcp6-segmentation: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off [fixed]

[Lukas Märdian (slyon)]

I tested netplan.io 0.104-0ubuntu2~20.04.2 from focal-proposed inside a VM and inside an LXD container.

* udev values (i.e. the ".link" file) contains the correct "=false" values.
* ethtool -k show the values have been correctly set to "off" (on "netplan apply" and "reboot")
  => it's noteworthy that some ethtool values are fixed in this setup and thus cannot be changed:
     large-receive-offload: off [fixed] # VM & container
     rx-checksumming: on [fixed] # VM only, works in container
  => Furthermore, there's a limitation that the offloading flags will not be applied reliably if matching on interface-name only, as described in the original upstream PR#270 (and https://github.com/canonical/netplan/pull/278). This should not be considered a regression as the feature did not exist before and the documentation has been updated accordingly. (Error message when skipping the udev settings: "enp5s0: Config file /run/systemd/network/10-netplan-enp5s0.link matches device based on renamed interface name, ignoring")
* The renaming of the interface works as expected inside the VM ("netplan apply" & "reboot")
* The renaming of the interface works inside the container on "netplan apply"
  => it's noteworthy that the interface is not renamed after "reboot" inside the LXD container, this is most probably due to some LXD udev quirks and should not be considered to be a regression, as it happened the same before. I confirmed this using the previous version from -release.

root@focal-vm:~# cat /run/systemd/network/10-netplan-enp5s0.link | grep Offload
ReceiveChecksumOffload=false
TransmitChecksumOffload=false
TCPSegmentationOffload=false
TCP6SegmentationOffload=false
GenericSegmentationOffload=false
GenericReceiveOffload=false
LargeReceiveOffload=false

root@focal-vm:~# ethtool -k eth99
Features for eth99:
rx-checksumming: on [fixed]
tx-checksumming: off
tcp-segmentation-offload: off
 tx-tcp6-segmentation: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off [fixed]

root@ff:~# ethtool -k eth99
Features for eth99:
rx-checksumming: off
tx-checksumming: off
 tx-checksum-sctp: off
tcp-segmentation-offload: off
 tx-tcp6-segmentation: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off [fixed]

[Nicolas Bock (nicolasbock)]

I followed the same test procedure from https://bugs.launchpad.net/netplan/+bug/1956264/comments/23 on a bare metal machine and can confirm that the package works as intended.

As already noted by Lukas, some of the offload / segmentation options are not exposed by the network driver and cannot be changed.

$ sudo ethtool -k eno1 | grep segmentation
tcp-segmentation-offload: off
        tx-tcp-segmentation: off [requested on]
        tx-tcp-ecn-segmentation: off [requested on]
        tx-tcp-mangleid-segmentation: off
        tx-tcp6-segmentation: off

These options are also marked as "fixed".

[Nicolas Bock (nicolasbock)]

I followed the same test procedure from https://bugs.launchpad.net/netplan/+bug/1956264/comments/23 on a bare metal machine running Jammy and can confirm that the package works as intended.

As already noted by Lukas, some of the offload / segmentation options are not exposed by the network driver and cannot be changed.

$ sudo ethtool -k eno1 | grep segmentation
tcp-segmentation-offload: off
        tx-tcp-segmentation: off [requested on]
        tx-tcp-ecn-segmentation: off [requested on]
        tx-tcp-mangleid-segmentation: off
        tx-tcp6-segmentation: off

These options are also marked as "fixed".

[Launchpad Janitor (janitor)]

This bug was fixed in the package netplan.io - 0.104-0ubuntu2.1

---------------
netplan.io (0.104-0ubuntu2.1) jammy; urgency=medium

  * Cherry-pick fix for rendering WPA3 password (8934a1b), LP: #1975576
    + d/p/0010-nm-fix-rendering-of-password-for-unknown-passthrough.patch
  * Backport offloading tristate patches (LP: #1956264)
    + d/p/0003-Add-tristate-type-for-offload-options-LP-1956264-270.patch
    + d/p/0004-tests-ethernets-fix-autopkgtest-with-alternating-def.patch
    + d/t/control: add 'ethtool' test-dep for link offloading tests

 -- Lukas Märdian <email address hidden> Wed, 29 Jun 2022 17:54:23 +0200

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for netplan.io has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package netplan.io - 0.104-0ubuntu2~20.04.2

---------------
netplan.io (0.104-0ubuntu2~20.04.2) focal; urgency=medium

  * Cherry pick d/p/dbus-Remove-the-upper-limit-on-try-timeout.patch
    (LP: #1967084)
  * Cherry-pick fix for rendering WPA3 password (8934a1b), LP: #1975576
    + d/p/0010-nm-fix-rendering-of-password-for-unknown-passthrough.patch
  * Backport offloading tristate patches (LP: #1956264)
    + d/p/0003-Add-tristate-type-for-offload-options-LP-1956264-270.patch
    + d/p/0004-tests-ethernets-fix-autopkgtest-with-alternating-def.patch
    + d/t/control: add 'ethtool' test-dep for link offloading tests

 -- Lukas Märdian <email address hidden> Wed, 29 Jun 2022 17:54:23 +0200