Expose link offload options

Bug #1771740 reported by Ryan Finnie
26
This bug affects 3 people
Affects Status Importance Assigned to Milestone
netplan
Fix Released
Medium
Unassigned
netplan.io (Ubuntu)
Fix Released
Wishlist
Unassigned
Focal
Fix Released
Wishlist
Unassigned
Hirsute
Fix Released
Wishlist
Unassigned
Impish
Fix Released
Wishlist
Unassigned
Jammy
Fix Released
Wishlist
Unassigned

Bug Description

[Impact]

 * On virtualization hosts disable GRO and LRO, otherwise, with
   receive offload on, the guests will receive packets that are
   larger than the MTU. This can cause issues in certain scenarios,
   e.g. when the guest is a VPN server that needs to forward the
   (inner) packet onward.

[Test Plan]
In addition to runing & 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:

$ mkdir -p tmp/etc/netplan
$ cat tmp/etc/netplan/test.yaml
network:
  version: 2
  ethernets:
    eth1:
      receive-checksum-offload: true
      transmit-checksum-offload: true
      tcp-segmentation-offload: true
      tcp6-segmentation-offload: true
      generic-segmentation-offload: true
      generic-receive-offload: true
      large-receive-offload: true
$ /usr/lib/netplan/generate -r tmp/
$ cat tmp/run/systemd/network/10-netplan-eth1.link | grep Offload

=> Make sure the *Offload settings are correctly set to "=1".

autopkgtest logs:
* Impish:
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/impish_amd64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/impish_arm64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/impish_armhf.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/impish_ppc64el.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/impish_s390x.log
* Hirsute:
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/hirsute_amd64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/hirsute_arm64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/hirsute_armhf.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/hirsute_ppc64el.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/hirsute_s390x.log
* Focal:
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/focal_amd64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/focal_arm64.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/focal_armhf.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/focal_ppc64el.log
https://git.launchpad.net/~slyon/+git/files/tree/LP1949104/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.

This also contains partial-d4884cfd40e1e33540b274371c3272df6595d22c.patch, to
keep libnetplan's ABI forward compatible with current upstream/Github, by
partially cherry-picking the ignore_carrier field addition into the
NetplanNetDefinition struct.

=== Original Description ===

https://www.freedesktop.org/software/systemd/man/systemd.link.html has a
number of [Link] options which I need to use for a flaky network card
(TCPSegmentationOffload, TCP6SegmentationOffload,
GenericSegmentationOffload, GenericReceiveOffload, LargeReceiveOffload)
which are not exposed via netplan.

Steve Langasek (vorlon)
Changed in netplan:
status: New → In Progress
importance: Undecided → Medium
Lukas Märdian (slyon)
tags: added: fr-1489
Revision history for this message
Mateusz Pawlowski (mateusz-p) wrote :

+ ReceiveChecksumOffload
+ TransmitChecksumOffload

Revision history for this message
Nicolas Bock (nicolasbock) wrote :

Submitted pull request upstream: https://github.com/canonical/netplan/pull/225

Revision history for this message
Lukas Märdian (slyon) wrote :

This is now merged upstream and will land with the next netplan release.

Changed in netplan:
status: In Progress → Fix Committed
Revision history for this message
Nicolas Bock (nicolasbock) wrote :
Revision history for this message
Nicolas Bock (nicolasbock) wrote :

This debdiff includes upstream fix for this bug and for LP: #1943120

description: updated
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

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.]

tags: added: patch
Mathew Hodson (mhodson)
Changed in netplan.io (Ubuntu Focal):
importance: Undecided → Wishlist
Changed in netplan.io (Ubuntu Hirsute):
importance: Undecided → Wishlist
Changed in netplan.io (Ubuntu Impish):
importance: Undecided → Wishlist
Revision history for this message
Dan Streetman (ddstreet) wrote :

@nicolasbock before sponsoring, can you cover a few of the steps in the SRU process:

1) The SRU template in the description isn't actually filled in, I think you simply copy-n-pasted the actual template text in...it does need to be actually filled out

2) This is marked as affecting f/h/i, but you only included a debdiff for focal; are h and i already patched or not?

3) You mentioned in chat that the netplan maintainer suggested you just SRU the changes as the next scheduled release for netplan is not for a while; I assume it was @slyon you talked to? I added him in this bug as a fyi; @slyon if you have any concerns about SRUing this before the next regular netplan release please let us know.

Thanks!

tags: added: sts-sponsor-ddstreet
Revision history for this message
Lukas Märdian (slyon) wrote :

I am generally +1 with SRUing this, BUT this is a special in a few ways:

1/ In order to land this in Focal, we first need to apply it to Impish and Hirsute (according to SRU process), as ddstreet mentioned.

2/ We are already post Feature Freeze for Impish and this is adding new functionality IMO (in contrast to fixing a bug). So I wonder if we'd need a Feature Freeze exception – that should be clarified with the release team.

3/ The debdiff looks good to me at first glance, BUT: We need to also inject the "gboolean ignore_carrier" field into the "net_definition" struct before the new offload fields (i.e. parse.h change from this commit https://github.com/canonical/netplan/commit/d4884cfd40e1e33540b274371c3272df6595d22c). Otherwise this patch will break ABI compatibility once the next upstream release would be SRUed...

description: updated
Revision history for this message
Nicolas Bock (nicolasbock) wrote :

Hi Lukas,

When you write "inject" that field, do you mean to simply add it to the struct or also apply the commit you cite?

Revision history for this message
Nicolas Bock (nicolasbock) wrote :
Revision history for this message
Nicolas Bock (nicolasbock) wrote :
Revision history for this message
Lukas Märdian (slyon) wrote :

Hi Nicolas,

yes I meant to simply add it to the struct at the correct (upstream) location, so that the data structure won't change (only append new fields) when we release the next upstream version.

Revision history for this message
Nicolas Bock (nicolasbock) wrote :
Revision history for this message
Nicolas Bock (nicolasbock) wrote :
Revision history for this message
Nicolas Bock (nicolasbock) wrote :
Revision history for this message
Nicolas Bock (nicolasbock) wrote :

Hi Lukas and Dan,

I applied a partial patch to the three packages (Focal, Hirsute, and Impish) and uploaded the new debdiffs. Please review.

Thanks!

Nick

Revision history for this message
Lukas Märdian (slyon) wrote :

Thank you Nicolas,

I cleaned up some fuzz and whitespace around the "activation_mode" field in src/parse.h but other than that the patch looks like I wanted it to be (wrt. ABI compatibility).

I've uploaded it to Jammy. Let's see how that works out wrt. autopkgtests. If everything is good and fixed in devel we can continue to push this down into the stable series.

Could you please improve the [Test Case] section in the SRU template a bit, I think in the current form it's most probably not being accepted by the SRU team... Yes, this is a new feature, so maybe create a test-case like: create /etc/netplan/test.yaml with content XYZ – run 'netplan generate' – verify the new offloading configuration has been generated in /run/systemd/network/* & /run/NetworkManager/system-connections/*

In addition to adding the full autopkgtest logs (once it is accepted into -proposed), as described here: https://wiki.ubuntu.com/NetplanUpdates

Lukas Märdian (slyon)
Changed in netplan.io (Ubuntu Jammy):
status: New → Fix Committed
Revision history for this message
Lukas Märdian (slyon) wrote :

Some more smaller remarks:
* We can drop the bundled "Implement-YAML-state-tracking-and-use-it-in-the-DBus.patch" from the Hirsute and Focal debdiffs (it has already been SRUed in the meantime).
* We need to fix the version strings (https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging), by basically just adding 0.1 to the current version for a normal SRU like this.

Impish => 0.103-0ubuntu7.1
Hirsute => 0.103-0ubuntu5~21.04.3
Focal => 0.103-0ubuntu5~20.04.3

I can apply those small fixes locally, so no need to update the debdiffs.

description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package netplan.io - 0.103-0ubuntu8

---------------
netplan.io (0.103-0ubuntu8) jammy; urgency=medium

  * d/p/0001-Add-support-for-additional-Link-options-225-LP-17717.patch:
    - Add offload configuration options.
      (LP: #1771740)
  * Add d/p/partial-d4884cfd40e1e33540b274371c3272df6595d22c.patch:
    - Partial application of d4884cfd40e1e33540b274371c3272df6595d22c in order
      preserve ABI compatibility for future updates.

 -- Nicolas Bock <email address hidden> Sat, 09 Oct 2021 16:27:57 -0600

Changed in netplan.io (Ubuntu Jammy):
status: Fix Committed → Fix Released
Lukas Märdian (slyon)
description: updated
Revision history for this message
Steve Langasek (vorlon) wrote : Please test proposed package

Hello Ryan, 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.103-0ubuntu5~20.04.3 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.

Changed in netplan.io (Ubuntu Focal):
status: New → Fix Committed
tags: added: verification-needed verification-needed-focal
Changed in netplan.io (Ubuntu Impish):
status: New → Fix Committed
tags: added: verification-needed-impish
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello Ryan, or anyone else affected,

Accepted netplan.io into impish-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/netplan.io/0.103-0ubuntu7.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-impish to verification-done-impish. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-impish. 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.

Revision history for this message
Steve Langasek (vorlon) wrote :

Hello Ryan, or anyone else affected,

Accepted netplan.io into hirsute-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/netplan.io/0.103-0ubuntu5~21.04.3 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-hirsute to verification-done-hirsute. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-hirsute. 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.

Changed in netplan.io (Ubuntu Hirsute):
status: New → Fix Committed
tags: added: verification-needed-hirsute
Revision history for this message
Ryan Finnie (fo0bar) wrote :

I no longer have the network card in question and don't have a suitable environment to test, sorry.

Lukas Märdian (slyon)
description: updated
Revision history for this message
Lukas Märdian (slyon) wrote :

I've tested netplan.io 0.103-0ubuntu7.1 from impish-proposed and attached the autopkgtest logs to the bug description, according to https://wiki.ubuntu.com/NetplanUpdates.

The netplan generator command was run successfully, producing correct *Offload=1 settings for networkd.

root@sru-ii:~# dpkg -l | grep netplan
ii libnetplan0:amd64 0.103-0ubuntu7.1 amd64 YAML network configuration abstraction runtime library
ii netplan.io 0.103-0ubuntu7.1 amd64 YAML network configuration abstraction for various backends

root@sru-ii:~# cat tmp/etc/netplan/test.yaml
network:
  version: 2
  ethernets:
    eth1:
      receive-checksum-offload: true
      transmit-checksum-offload: true
      tcp-segmentation-offload: true
      tcp6-segmentation-offload: true
      generic-segmentation-offload: true
      generic-receive-offload: true
      large-receive-offload: true
root@sru-ii:~# /usr/lib/netplan/generate -r tmp/
root@sru-ii:~# cat tmp/run/systemd/network/10-netplan-eth1.link | grep Offload
ReceiveChecksumOffload=1
TransmitChecksumOffload=1
TCPSegmentationOffload=1
TCP6SegmentationOffload=1
GenericSegmentationOffload=1
GenericReceiveOffload=1
LargeReceiveOffload=1

tags: added: verification-done-impish
removed: verification-needed-impish
Revision history for this message
Lukas Märdian (slyon) wrote :

I've tested netplan.io 0.103-0ubuntu5~21.04.3 from hirsute-proposed.

The netplan generator command was run successfully, producing correct *Offload=1 settings for networkd.

root@sru-hh:~# dpkg -l | grep netplan
ii libnetplan0:amd64 0.103-0ubuntu5~21.04.3 amd64 YAML network configuration abstraction runtime library
ii netplan.io 0.103-0ubuntu5~21.04.3 amd64 YAML network configuration abstraction for various backends

root@sru-hh:~# vim tmp/etc/netplan/test.yaml
root@sru-hh:~# cat tmp/etc/netplan/test.yaml
network:
  version: 2
  ethernets:
    eth1:
      receive-checksum-offload: true
      transmit-checksum-offload: true
      tcp-segmentation-offload: true
      tcp6-segmentation-offload: true
      generic-segmentation-offload: true
      generic-receive-offload: true
      large-receive-offload: true
root@sru-hh:~# /usr/lib/netplan/generate -r tmp/
root@sru-hh:~# cat tmp/run/systemd/network/10-netplan-eth1.link | grep Offload
ReceiveChecksumOffload=1
TransmitChecksumOffload=1
TCPSegmentationOffload=1
TCP6SegmentationOffload=1
GenericSegmentationOffload=1
GenericReceiveOffload=1
LargeReceiveOffload=1

tags: added: verification-done-hirsute
removed: verification-needed-hirsute
Revision history for this message
Lukas Märdian (slyon) wrote (last edit ):

I've tested netplan.io 0.103-0ubuntu5~20.04.3 from focal-proposed.

The netplan generator command was run successfully, producing correct *Offload=1 settings for networkd.

root@sru-ff:~# dpkg -l | grep netplan
ii libnetplan0:amd64 0.103-0ubuntu5~20.04.3 amd64 YAML network configuration abstraction runtime library
ii netplan.io 0.103-0ubuntu5~20.04.3 amd64 YAML network configuration abstraction for various backends

root@sru-ff:~# cat tmp/etc/netplan/test.yaml
network:
  version: 2
  ethernets:
    eth1:
      receive-checksum-offload: true
      transmit-checksum-offload: true
      tcp-segmentation-offload: true
      tcp6-segmentation-offload: true
      generic-segmentation-offload: true
      generic-receive-offload: true
      large-receive-offload: true
root@sru-ff:~# /usr/lib/netplan/generate -r tmp/
root@sru-ff:~# cat tmp/run/systemd/network/10-netplan-eth1.link | grep Offload
ReceiveChecksumOffload=1
TransmitChecksumOffload=1
TCPSegmentationOffload=1
TCP6SegmentationOffload=1
GenericSegmentationOffload=1
GenericReceiveOffload=1
LargeReceiveOffload=1

tags: added: verification-done-focal
removed: verification-needed-focal
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package netplan.io - 0.103-0ubuntu7.1

---------------
netplan.io (0.103-0ubuntu7.1) impish; urgency=medium

  [ Nicolas Bock ]
  * d/p/0001-Add-support-for-additional-Link-options-225-LP-17717.patch:
    - Add offload configuration options.
      (LP: #1771740)
  * Add d/p/partial-d4884cfd40e1e33540b274371c3272df6595d22c.patch:
    - Partial application of d4884cfd40e1e33540b274371c3272df6595d22c in order
      preserve ABI compatibility for future updates.

  [ Lukas Märdian ]
  * Add d/p/0010-parse-nm-Handle-missing-gateway-in-keyfile-routes-ke.patch
    (LP: #1949761)
  * Fix regression in 'netplan try' (LP: #1949104)
    + d/p/lp1949104/cli-apply-initialize-self.state-LP-1949104-243.patch
    + d/p/lp1949104/tests-regressions-make-netplan_try-autopkgtest-more-.patch

 -- Lukas Märdian <email address hidden> Fri, 05 Nov 2021 14:44:08 +0100

Changed in netplan.io (Ubuntu Impish):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

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.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package netplan.io - 0.103-0ubuntu5~21.04.3

---------------
netplan.io (0.103-0ubuntu5~21.04.3) hirsute; urgency=medium

  [ Nicolas Bock ]
  * d/p/0001-Add-support-for-additional-Link-options-225-LP-17717.patch:
    - Add offload configuration options.
      (LP: #1771740)
  * Add d/p/partial-d4884cfd40e1e33540b274371c3272df6595d22c.patch:
    - Partial application of d4884cfd40e1e33540b274371c3272df6595d22c in order
      preserve ABI compatibility for future updates.

  [ Lukas Märdian ]
  * Add d/p/0010-parse-nm-Handle-missing-gateway-in-keyfile-routes-ke.patch
    (LP: #1949761)
  * Fix regression in 'netplan try' (LP: #1949104)
    + d/p/lp1949104/cli-apply-initialize-self.state-LP-1949104-243.patch
    + d/p/lp1949104/tests-regressions-make-netplan_try-autopkgtest-more-.patch

 -- Lukas Märdian <email address hidden> Fri, 05 Nov 2021 15:18:41 +0100

Changed in netplan.io (Ubuntu Hirsute):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package netplan.io - 0.103-0ubuntu5~20.04.3

---------------
netplan.io (0.103-0ubuntu5~20.04.3) focal; urgency=medium

  [ Nicolas Bock ]
  * d/p/0001-Add-support-for-additional-Link-options-225-LP-17717.patch:
    - Add offload configuration options.
      (LP: #1771740)
  * Add d/p/partial-d4884cfd40e1e33540b274371c3272df6595d22c.patch:
    - Partial application of d4884cfd40e1e33540b274371c3272df6595d22c in order
      preserve ABI compatibility for future updates.

  [ Lukas Märdian ]
  * Add d/p/0010-parse-nm-Handle-missing-gateway-in-keyfile-routes-ke.patch
    (LP: #1949761)
  * Fix regression in 'netplan try' (LP: #1949104)
    + d/p/lp1949104/cli-apply-initialize-self.state-LP-1949104-243.patch
    + d/p/lp1949104/tests-regressions-make-netplan_try-autopkgtest-more-.patch

 -- Lukas Märdian <email address hidden> Fri, 05 Nov 2021 15:15:57 +0100

Changed in netplan.io (Ubuntu Focal):
status: Fix Committed → Fix Released
Revision history for this message
Richard Laager (rlaager) wrote :

This change does NOT fix the issue from the [Impact] statement. The [Impact] talks about disabling offload, but the test case talks about enabling offload. The patch only implements enabling offload, not disabling it.

Changed in netplan:
status: Fix Committed → Confirmed
Revision history for this message
Richard Laager (rlaager) wrote :

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.)

Revision history for this message
Lukas Märdian (slyon) wrote :

This bug was mostly about exposing the offload options in general. But indeed the tristate should be handled properly.

Let's continue that discussion in LP: #1956264

Lukas Märdian (slyon)
Changed in netplan:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers