bridge: Bridge adding non existing interfaces is allowed but does not work
Closed, ResolvedPublicFEATURE REQUEST
Actions

Assigned To

Authored By

	runar
	Aug 23 2019, 8:42 AM

Description

when adding interfaces to the bridge it is possible to add interfaces no currently created.
and when the interface is created it is not tied into the bridge.

Steps to replicate:

Create a interface with a vif

# set interface ethernet eth0 vif 1
# commit

Create the bridge and add two interfaces. one of those interfaces is currently not existing

[email protected]# show int
# set interfaces bridge br1 address 10.3.4.5/24
# set interfaces bridge br1 member interface eth0.1
# set interfaces bridge br1 member interface eth1.1
# commit

Create the missing interface

# set interface ethernet eth1 vif 1
# commit

The vif is created, but is not tied to the bridge group

[email protected]# show int eth eth1
 ...
 vif 1 {
 }

[email protected]# show int bridge br1
 address 10.3.4.5/24
 member {
     interface eth0.1 {
     }
     interface eth1.1 {
     }
 }

[email protected]# ip a s eth1.1
6: [email protected]: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 52:54:05:12:34:57 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::5054:5ff:fe12:3457/64 scope link
       valid_lft forever preferred_lft forever

[email protected]# brctl show
bridge name	bridge id		STP enabled	interfaces
br1		8000.525405123456	yes		eth0.1         <-- interface eth1.1 is not added to the bridge

To fix this two posibillities exists.

Do not allow additions of un-initialized interfaces . this also means that deletion of an interface in a bridge-group also should be disallowed.

On interface creation, make it check if it should be in a bridge-group and run the bridge-script to re-add the interface to the bridge-group

out of these two methods i would prefer if solution 2 is implemented.

I am still not 100% sure that this syntax is better then having the bridge mapping on the interface itself. because this is actually a function of the interface and not of the bridge. and will be added/removed with the interface. so this is something that also could have been in the ethernet-script

Details

Difficulty level: Normal (likely a few hours)
Version: -
Why the issue appeared?: Will be filled on close

Related Objects
Search...

Status	Subtype	Assigned	Task
In progress	FEATURE REQUEST	None	T3355 Remove all remaining legacy Vyatta code
Resolved	FEATURE REQUEST	None	T1579 Rewrite all interface types in new XML/Python style
Resolved	FEATURE REQUEST	c-po	T1556 Rewrite Bridge in new style XML syntax
Resolved	FEATURE REQUEST	c-po	T1608 bridge: Bridge adding non existing interfaces is allowed but does not work

Event Timeline

runar renamed this task from bridge: possible to add non-created interfaces. to bridge: Bridge adding non existing interfaces is allowed but does not work.Aug 23 2019, 8:42 AM

runar created this task.

If the bridge priority is higher then the ethernet vif this should work out of the box

i've updated the description. the bridge is created and comitted prior to the creation of the interface.

the current only way to get the interface added to the group is to remove and readd it to the group.

[email protected]# brctl show
bridge name	bridge id		STP enabled	interfaces
br1		8000.525405123456	yes		eth0.1

[email protected]# show int bridge br1
 address 10.3.4.5/24
 member {
     interface eth0.1 {
     }
     interface eth1.1 {
     }
 }

[email protected]# del int bri br1 mem interface eth1.1
[email protected]# commit
[ interfaces bridge br1 ]
device eth1.1 is not a slave of br1

[email protected]# set int bri br1 mem interface eth1.1
[email protected]# commit

[email protected]# brctl show
bridge name	bridge id		STP enabled	interfaces
br1		8000.525405123456	yes		eth0.1
							eth1.1

Full console dump to reproduce with comments:

##
## Start without anything

[email protected]# show
 interfaces {
     ethernet eth0 {
         duplex auto
         hw-id 52:54:05:12:34:56
         smp-affinity auto
         speed auto
     }
     ethernet eth1 {
         duplex auto
         hw-id 52:54:05:12:34:57
         smp-affinity auto
         speed auto
     }
     loopback lo {
     }
 }
....






##
## Create a vif on eth0 and set up the bridge
[edit]
[email protected]# set int eth eth0 vif 1
[edit]
[email protected]# set int bri br1 address 10.3.4.5/24
[edit]
[email protected]# set int bri br1 member int eth0.1
[edit]
[email protected]# set int bri br1 member int eth1.1
[edit]
[email protected]# commit
[ interfaces bridge br1 ]
interface eth1.1 does not exist!
Assigning 10.3.4.5/24 to br1

## The script complains about you trying to add an non-existant interface, but the config is allowed
## From now on, the CLI and brctl is out of sync config-wise 

[edit]
[email protected]# show int
 bridge br1 {
     address 10.3.4.5/24
     member {
         interface eth0.1 {
         }
         interface eth1.1 {
         }
     }
 }
 ethernet eth0 {
     duplex auto
     hw-id 52:54:05:12:34:56
     smp-affinity auto
     speed auto
     vif 1 {
     }
 }
 ethernet eth1 {
     duplex auto
     hw-id 52:54:05:12:34:57
     smp-affinity auto
     speed auto
 }
[edit]
[email protected]# brctl show
bridge name	bridge id		STP enabled	interfaces
br1		8000.525405123456	no		eth0.1
[edit]
[email protected]# ip a | egrep -e 'eth[0123456789]'
2: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
3: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
7: [email protected]: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master br1 state UP group default qlen 1000
8: br1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000





## Then create the interface missing on eth1.1
[edit]
[email protected]# set int eth eth1 vif 1
[edit]
[email protected]# commit
[edit]





## Now the interface is added but is not in the bridge-group
[email protected]# brctl show
bridge name	bridge id		STP enabled	interfaces
br1		8000.525405123456	no		eth0.1
[edit]

[email protected]# ip a | egrep -e 'eth[0123456789]'
2: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
3: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
7: [email protected]: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master br1 state UP group default qlen 1000
9: [email protected]: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000








## To force it into the bridge, it needs to be readded. 
## CLI complains when deleting because the CLI and brctl is out of sync

[edit]
[email protected]# del int bri br1 mem int eth1.1
[edit]
[email protected]# commit
[ interfaces bridge br1 ]
device eth1.1 is not a slave of br1

[edit]
[email protected]# set int bri br1 mem int eth1.1
[edit]
[email protected]# commit
[edit]






## Now, the interface is part of the bridge again.

[email protected]# brctl show
bridge name	bridge id		STP enabled	interfaces
br1		8000.525405123456	no		eth0.1
							eth1.1
[edit]
[email protected]#

c-po claimed this task.Aug 23 2019, 9:49 AM

This can be solved using multiple ways:

add option to the interfaces-bridge.py script to recreate the bridge when triggered externally
- advantage: the conde is already there
- disadvantage: the script can easily grow to a wastebin of code executed from 100 places
add a dedicated bridge-group-sync.py script which synchronizes the bridge groups and interfaces, called whenever a interface is added to the system (e.g OpenVPN vtun or ethernet vif). We will walk through all available bridges configured and add possibly missing interfaces
- advantage: code is contained in a dedicated scirpt
- disadvantage: more and more scripts might evolve
Write a vyos-interfaced which acts as message receiver and will handle tasks as adding/removing IP addresses, adding/removing bridge and bond members
- advantage: most generic and single source
- disadvantage: most complex

I think I would start of with bridge-group-sync.py and see how this scales. In the end I think we once will definately require a vyos-interfaced or even a complete vyosd

c-po triaged this task as High priority.Aug 23 2019, 10:17 AM

c-po changed Difficulty level from Unknown (require assessment) to Hard (possibly days).

Would something like https://openwrt.org/docs/techref/netifd be useful? The drawback that it's dependent on ubus https://openwrt.org/docs/techref/ubus

Turns out this can be done with the following code:

#!/usr/bin/env python3
#
# Copyright (C) 2019 VyOS maintainers and contributors
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 2 or later as
# published by the Free Software Foundation.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.
#

# Script is used to synchronize configured bridge interfaces.
# one can add a non existing interface to a bridge group (e.g. VLAN)
# but the vlan interface itself does yet not exist. It should be added
# to the bridge automatically once it's available

import argparse
import subprocess

from sys import exit
from time import sleep
from vyos.config import Config

def subprocess_cmd(command):
    process = subprocess.Popen(command,stdout=subprocess.PIPE, shell=True)
    proc_stdout = process.communicate()[0].strip()
    pass

if __name__ == '__main__':
    parser = argparse.ArgumentParser()
    parser.add_argument('-i', '--interface', action='store', help='Interface name which should be added to bridge it is configured for', required=True)
    args, unknownargs = parser.parse_known_args()

    conf = Config()
    if not conf.list_nodes('interfaces bridge'):
        #  no bridge interfaces exist .. bail out early
        exit(0)
    else:
        for bridge in conf.list_nodes('interfaces bridge'):
            for member_if in conf.list_nodes('interfaces bridge {} member interface'.format(bridge)):
                if args.interface == member_if:
                    cmd = 'brctl addif "{}" "{}"'.format(bridge, args.interface)
                    # let interfaces etc. settle - especially required for OpenVPN bridged interfaces
                    sleep(4)
                    subprocess_cmd(cmd)

    exit(0)

The problem is, when using an OpenVPN --up script it is executed but the interface is unfortuntely not added to the bridge. Calling the same commandline manually works. Looks like some sort of internal synchronisation when adding interfaces to the system.

On subsequent thoughts this is "bug" is no problem at all (as of now) b/c in VyOS 1.2.0 or 1.1.8 it is not possible to add an interface to a bridge which does not exist:

[email protected]# set interfaces ethernet eth1 bridge-group bridge br1
[edit]
[email protected]# set interfaces ethernet eth2 bridge-group bridge br1
[edit]
[email protected]# commit
[ interfaces ethernet eth1 bridge-group bridge br1 ]
bridge interface br1 does not exist on system

[[interfaces ethernet eth1]] failed
[ interfaces ethernet eth2 bridge-group bridge br1 ]
bridge interface br1 does not exist on system

[[interfaces ethernet eth2]] failed
Commit failed

Adding an interface to a non existing bridge is almost the same as adding an non existing interface to a bridge. Both make no sense and should be forbidden.

c-po closed this task as Resolved.Aug 26 2019, 2:21 PM

c-po changed Difficulty level from Hard (possibly days) to Normal (likely a few hours).

c-po moved this task from Need Triage to Finished on the VyOS 1.3 Equuleus board.Oct 13 2019, 3:06 PM

syncer edited projects, added VyOS 1.3 Equuleus (1.3.0); removed VyOS 1.3 Equuleus.Aug 30 2022, 2:07 PM

syncer moved this task from Need Triage to Finished on the VyOS 1.3 Equuleus (1.3.0) board.

bridge: Bridge adding non existing interfaces is allowed but does not workClosed, ResolvedPublicFEATURE REQUESTActions

Description

Details

Related ObjectsSearch...

Event Timeline

bridge: Bridge adding non existing interfaces is allowed but does not work
Closed, ResolvedPublicFEATURE REQUEST
Actions

Related Objects
Search...