netplan needs bridge port-priority support

Bug #1735821 reported by Ryan Harper on 2017-12-01
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
cloud-init
Medium
Unassigned
nplan (Ubuntu)
Medium
Unassigned
Xenial
Medium
Unassigned
Artful
Medium
Unassigned

Bug Description

[Impact]
Users of netplan configuring any bridge. Port priority is a very common setting to change when setting up bridge devices that might have multiple interfaces.

[Test case]
1) Write a netplan configuration:
network:
    version: 2
    ethernets:
        eth0:
            match:
                name: eth0
    bridges:
        br0:
            addresses:
            - 192.168.14.2/24
            interfaces:
            - eth0
            parameters:
                path-cost:
                    eth0: 50
                priority: 22
                port-priority:
                    eth0: 14

2) Run 'sudo netplan apply'

3) Validate that the config generated by netplan is correct:

In /run/systemd/network/10-netplan-eth0.network:

[...]
[Bridge]
[...]
Priority=14

4) Validate that the port-priority value for the bridge has been correctly set:

$ cat /sys/class/net/mybr/brif/eth0/priority

[Regression potential]
This might impact STP behavior, such that while the port priority for a bridge changes, the general network topology might change -- this may lead to loss of connectivity on the bridge itself or on other devices on the network, invalid packet traffic (packets showing up where they should not), etc.

---

Now that systemd supports port-priority for bridges (LP: #1668347)
netplan should handle port-priority like it does path-cost.

1) % lsb_release -rd
Description: Ubuntu 16.04.3 LTS
Release: 16.04

1) # lsb_release -rd
Description: Ubuntu Bionic Beaver (development branch)
Release: 18.04

2) # apt-cache policy nplan
nplan:
  Installed: 0.30
  Candidate: 0.32
  Version table:
     0.32 500
        500 http://archive.ubuntu.com/ubuntu bionic/main amd64 Packages
 *** 0.30 100
        100 /var/lib/dpkg/status

3) netplan generate renders a networkd .network file which has [Bridge] section including Priority value set on each of the bridge ports specified

4) netplan fails to parse the input yaml with

Sample config that should parse:

% cat br-pp.yaml
network:
    version: 2
    ethernets:
        eth0:
            match:
                macaddress: '52:54:00:12:34:04'
    bridges:
        br0:
            addresses:
            - 192.168.14.2/24
            interfaces:
            - eth0
            parameters:
                path-cost:
                    eth0: 50
                priority: 22
                port-priority:
                    eth0: 14

% netplan generate
Error in network definition br-pp.yaml line 13 column 16: unknown key port-priority

If fixed, then I would expect a /run/systemd/network/10-netplan-eth0.network that looks like
[Match]
MACAddress=52:54:00:12:34:00
Name=eth0

[Network]
Bridge=br0
LinkLocalAddressing=no
IPv6AcceptRA=no

[Bridge]
Cost=50
Priority=14

Related branches

Ryan Harper (raharper) wrote :

Partial patch; implements networkd side, needs NetworkManager portion and additional unit and integration tests.

The attachment "add_bridge_port_priority.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

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

tags: added: patch

Merged in https://git.launchpad.net/netplan/commit/?id=b1f3623093e2a25ed73bbc94d25527888e30c775

Waiting in bionic-proposed for now due to the need to coordinate with a juju-core SRU to xenial.

Changed in nplan (Ubuntu):
status: New → Fix Committed
Ryan Harper (raharper) wrote :
Download full text (3.4 KiB)

FWIW, I tested the package in bionic-proposed and it passes the test I describe above:

root@b2:~# netplan generate
Error in network definition //etc/netplan/50-cloud-init.yaml line 19 column 16: unknown key port-priority
root@b2:~# sudo vi /etc/apt/sources.list
root@b2:~# sudo apt update && sudo apt policy nplan
Hit:1 http://security.ubuntu.com/ubuntu bionic-security InRelease
Get:2 http://archive.ubuntu.com/ubuntu bionic InRelease [235 kB]
Hit:3 http://ppa.launchpad.net/raharper/experimental/ubuntu bionic InRelease
Hit:4 http://archive.ubuntu.com/ubuntu bionic-updates InRelease
Get:5 http://archive.ubuntu.com/ubuntu bionic-proposed InRelease [235 kB]
Hit:6 http://archive.ubuntu.com/ubuntu bionic-backports InRelease
Get:7 http://archive.ubuntu.com/ubuntu bionic/universe Sources [9019 kB]
Get:8 http://archive.ubuntu.com/ubuntu bionic/main amd64 Packages [1010 kB]
Get:9 http://archive.ubuntu.com/ubuntu bionic/universe amd64 Packages [8372 kB]
Get:10 http://archive.ubuntu.com/ubuntu bionic/universe Translation-en [4921 kB]
Get:11 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 Packages [146 kB]
Get:12 http://archive.ubuntu.com/ubuntu bionic-proposed/main Translation-en [59.8 kB]
Fetched 24.0 MB in 5s (4439 kB/s)
Reading package lists... Done
Building dependency tree
Reading state information... Done
45 packages can be upgraded. Run 'apt list --upgradable' to see them.
nplan:
  Installed: 0.32
  Candidate: 0.33
  Version table:
     0.33 500
        500 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 Packages
 *** 0.32 500
        500 http://archive.ubuntu.com/ubuntu bionic/main amd64 Packages
        100 /var/lib/dpkg/status
root@b2:~# sudo apt install nplan
Reading package lists... Done
Building dependency tree
Reading state information... Done
Suggested packages:
  network-manager | wpasupplicant
The following packages will be upgraded:
  nplan
1 upgraded, 0 newly installed, 0 to remove and 44 not upgraded.
Need to get 41.6 kB of archives.
After this operation, 3072 B of additional disk space will be used.
Get:1 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 nplan amd64 0.33 [41.6 kB]
Fetched 41.6 kB in 1s (81.1 kB/s)
(Reading database ... 27026 files and directories currently installed.)
Preparing to unpack .../archives/nplan_0.33_amd64.deb ...
Unpacking nplan (0.33) over (0.32) ...
Setting up nplan (0.33) ...
Processing triggers for man-db (2.8.1-1) ...
root@b2:~# cat /etc/netplan/50-cloud-init.yaml
# This file is generated from information provided by
# the datasource. Changes to it will not persist across an instance.
# To disable cloud-init's network configuration capabilities, write a file
# /etc/cloud/cloud.cfg.d/99-disable-network-config.cfg with the following:
# network: {config: disabled}
network:
    version: 2
    ethernets:
        eth0:
            match:
                name: 'eth0'

    bridges:
        br0:
            addresses:
            - 192.168.14.2/24
            interfaces:
            - eth0
            parameters:
                path-cost:
                    eth0: 50
                priority: 22
      ...

Read more...

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nplan - 0.33

---------------
nplan (0.33) bionic; urgency=medium

  * replug: Do not unbind ath9kl_sdio. Thanks Oliver! (LP: #1741910)
  * doc: fix syntax for IPv6 example addresses. IPv6 needs to be escaped in
    YAML. (LP: #1735317)
  * debian/postinst: Write breadcrumbs on disk in /etc/network/interfaces to
    denote the migration to using netplan. (LP: #1744968)
  * bridge: implement port-priority support for the NM and networkd backends.
    (LP: #1735821)
  * doc: routes are not top-level but per-interface. (LP: #1726695)
  * Rework CLI parsing / code layout to better handle subcommands.

 -- Mathieu Trudel-Lapierre <email address hidden> Tue, 23 Jan 2018 11:32:47 -0500

Changed in nplan (Ubuntu):
status: Fix Committed → Fix Released
description: updated

Hello Ryan, or anyone else affected,

Accepted nplan into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nplan/0.32~17.10.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 and change the tag from verification-needed-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. 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!

Changed in nplan (Ubuntu Artful):
status: New → Fix Committed
tags: added: verification-needed verification-needed-artful
Łukasz Zemczak (sil2100) wrote :

Hello Ryan, or anyone else affected,

Accepted nplan into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nplan/0.32~17.10.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 and change the tag from verification-needed-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. 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!

Łukasz Zemczak (sil2100) wrote :

Accepted for xenial-proposed. Please test the package and set the verification-done-xenial on success. Thank you!

Changed in nplan (Ubuntu Xenial):
status: New → Fix Committed
tags: added: verification-done-xenial
tags: added: verification-needed-xenial
removed: verification-done-xenial
Scott Moser (smoser) on 2018-03-16
Changed in nplan (Ubuntu):
importance: Undecided → Medium
Changed in nplan (Ubuntu Xenial):
importance: Undecided → Medium
Changed in nplan (Ubuntu Artful):
importance: Undecided → Medium
Changed in cloud-init:
status: New → Confirmed
importance: Undecided → Medium
Scott Moser (smoser) on 2018-03-16
Changed in cloud-init:
status: Confirmed → Fix Committed

Verification done for xenial (nplan 0.32~16.04.4):
Verification done for artful (nplan 0.32~17.10.3):

Verified that the configuration supplied in the test case is accepted by netplan and correctly results in a bridge being brought up with a port-priority of "14" for eth0, and a bridge priority of 22:

# pleasing-koala (16.04):
root@pleasing-koala:/etc/netplan# cat /sys/class/net/br0/brif/eth0/priority
14
root@pleasing-koala:/etc/netplan# cat /sys/class/net/br0/bridge/priority
22

# needed-rat (17.10):
root@needed-rat:~# cat /sys/class/net/br0/brif/eth0/priority
14
root@needed-rat:~# cat /sys/class/net/br0/bridge/priority
22

tags: added: verification-done-artful verification-done-xenial
removed: patch verification-needed verification-needed-artful verification-needed-xenial

This bug is believed to be fixed in cloud-init in 18.2. If this is still a problem for you, please make a comment and set the state back to New

Thank you.

Changed in cloud-init:
status: Fix Committed → Fix Released
Changed in nplan (Ubuntu):
assignee: nobody → MOHAMMED FAHAD MUSHAHID (89mfahad)
Changed in nplan (Ubuntu):
assignee: MOHAMMED FAHAD MUSHAHID (89mfahad) → nobody

The verification of the Stable Release Update for nplan has completed successfully and the package has now been 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) wrote :

This bug was fixed in the package nplan - 0.32~17.10.3

---------------
nplan (0.32~17.10.3) artful; urgency=medium

  * Don't silently break Bridge Priority by adding port-priority.

nplan (0.32~17.10.2) artful; urgency=medium

  * Fix syntax for IPv6 addresses in doc. (LP: #1735317)
  * doc: routes are not top-level but per-interface. (LP: #1726695)
  * Implement bridge port-priority parameter. (LP: #1735821)
  * Implement "optional: true" to correctly write systemd network definitions
    with "RequiredForOnline=false", such that these networks do not block boot.
    (LP: #1664844)
  * Various documentation fixes. (LP: #1751814)

 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 02 Mar 2018 16:50:47 -0500

Changed in nplan (Ubuntu Artful):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nplan - 0.32~16.04.4

---------------
nplan (0.32~16.04.4) xenial; urgency=medium

  [ Oliver Grawert ]
  * Prevent unbinding ath6kl_sdio, driver does not support it correctly.
    (LP: #1741910)

  [ Mathieu Trudel-Lapierre ]
  * Re-add snap support patch. (LP: #1747714)
  * Fix syntax for IPv6 addresses in doc. (LP: #1735317)
  * doc: routes are not top-level but per-interface. (LP: #1726695)
  * Implement bridge port-priority parameter. (LP: #1735821)
  * Implement "optional: true" to correctly write systemd network definitions
    with "RequiredForOnline=false", such that these networks do not block boot.
    (LP: #1664844)
  * Various documentation fixes. (LP: #1751814)

 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 02 Mar 2018 17:02:03 -0500

Changed in nplan (Ubuntu Xenial):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers