From b050127ded8e861b22b7aa976f28f198ad51afc2 Mon Sep 17 00:00:00 2001 From: Mike Steinmetz Date: Thu, 16 Jul 2026 15:01:51 +0200 Subject: [PATCH] MM-12032 Fix bond/VLAN/bridge not removed from active list on delete delete_bifcs.cgi and save_bifc.cgi only called deactivate_interface() which brings the link down but never removes the virtual device. Added destroy_interface_device() that does ip link delete for these. Also had to add bridge creation (ip link add type bridge) because unlike bond (81d44f8) and VLAN (7761066) that was still missing. Other fixes in this commit: debian-linux-lib.pl: bonding opts were single-element arrays causing trailing spaces in /etc/network/interfaces on every save. linux-lib.pl: ip link set up/down was gated behind a bond/vlan/no-ifconfig check but ip addr add runs unconditionally - so interfaces got an IP but stayed DOWN. Dropped the unnecessary guard. --- net/debian-linux-lib.pl | 12 +++--- net/delete_bifcs.cgi | 4 ++ net/linux-lib.pl | 33 ++++++++++++++-- net/save_bifc.cgi | 4 ++ net/t/run-tests.t | 85 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 9 deletions(-) diff --git a/net/debian-linux-lib.pl b/net/debian-linux-lib.pl index f488d033f..63dda1904 100755 --- a/net/debian-linux-lib.pl +++ b/net/debian-linux-lib.pl @@ -230,12 +230,12 @@ if ($cfg->{'bridge'}) { # Set bonding parameters if(($cfg->{'bond'} == 1) && ($gconfig{'os_version'} >= 5)) { - push(@options, [&bonding_option('mode').' '.$cfg->{'mode'}]); - push(@options, [&bonding_option('miimon').' '.$cfg->{'miimon'}]) if ($cfg->{'miimon'}); - push(@options, [&bonding_option('updelay').' '.$cfg->{'updelay'}]) if ($cfg->{'updelay'}); - push(@options, [&bonding_option('downdelay').' '.$cfg->{'downdelay'}]) if ($cfg->{'downdelay'}); - push(@options, [&bonding_option('primary').' '.$cfg->{'primary'}]) if ($cfg->{'primary'}); - push(@options, ['slaves '.$cfg->{'partner'}]); + push(@options, [&bonding_option('mode'), $cfg->{'mode'}]); + push(@options, [&bonding_option('miimon'), $cfg->{'miimon'}]) if ($cfg->{'miimon'}); + push(@options, [&bonding_option('updelay'), $cfg->{'updelay'}]) if ($cfg->{'updelay'}); + push(@options, [&bonding_option('downdelay'), $cfg->{'downdelay'}]) if ($cfg->{'downdelay'}); + push(@options, [&bonding_option('primary'), $cfg->{'primary'}]) if ($cfg->{'primary'}); + push(@options, ['slaves', $cfg->{'partner'}]); } elsif ($cfg->{'bond'} == 1) { push(@options, ['up', '/sbin/ifenslave '.$cfg->{'name'}." ". diff --git a/net/delete_bifcs.cgi b/net/delete_bifcs.cgi index bd60b7d65..a8598d7f1 100755 --- a/net/delete_bifcs.cgi +++ b/net/delete_bifcs.cgi @@ -51,6 +51,10 @@ foreach $d (reverse(@d)) { else {&unload_module($b->{'name'});} } } + # Remove the virtual device after deactivation + if (defined(&destroy_interface_device)) { + &destroy_interface_device($b); + } } # Delete config diff --git a/net/linux-lib.pl b/net/linux-lib.pl index 54cb43877..33e9fbb27 100755 --- a/net/linux-lib.pl +++ b/net/linux-lib.pl @@ -329,9 +329,22 @@ if (&has_command("ip") && $a->{'bond'} && $a->{'up'} && !$old) { } } -if (($a->{'bond'} || $a->{'vlan'} || !&has_command("ifconfig")) && - &has_command("ip")) { - # For a real interface, activate or de-activate the link +if (&has_command("ip") && $a->{'bridge'} && $a->{'up'} && !$old) { + # Create the bridge before assigning addresses to it. + my $cmd = "ip link add ".quotemeta($a->{'name'})." type bridge"; + my $out = &backquote_logged("$cmd 2>&1"); + &error("Failed to create bridge device : $out") if ($?); + if ($a->{'bridgeto'}) { + $cmd = "ip link set dev ".quotemeta($a->{'bridgeto'}). + " master ".quotemeta($a->{'name'}); + $out = &backquote_logged("$cmd 2>&1"); + &error("Failed to add interface to bridge : $out") if ($?); + } + } + +if (&has_command("ip")) { + # Manage link state for all interfaces when ip is used, since ip is also + # used for address assignment below regardless of ifconfig availability. if ($a->{'virtual'} eq '' && $a->{'up'} && (!$old || !$old->{'up'})) { # Bring up my $cmd = "ip link set dev ".quotemeta($devname)." up"; @@ -595,6 +608,20 @@ else { } } +# destroy_interface_device(&details) +# Remove a virtual network device (bond, VLAN, bridge) from the kernel. +# Should be called after deactivate_interface when deleting, not just +# deactivating, a virtual interface. +sub destroy_interface_device +{ +my ($a) = @_; +if (&has_command("ip") && $a->{'virtual'} eq '' && + (&use_ifup_command($a) || $a->{'bridge'})) { + &backquote_logged("ip link delete ". + quotemeta($a->{'fullname'} || $a->{'name'})." 2>&1"); + } +} + # use_ifup_command(&iface) # Returns 1 if the ifup command must be used to bring up some interface. # True on Debian 5.0+ for non-ethernet, typically bonding and VLAN tagged interfaces. diff --git a/net/save_bifc.cgi b/net/save_bifc.cgi index ae2fc7fa4..2b130dcb0 100755 --- a/net/save_bifc.cgi +++ b/net/save_bifc.cgi @@ -32,6 +32,10 @@ if ($in{'delete'} || $in{'unapply'}) { else { &deactivate_interface($act); } + # Remove the virtual device after deactivation + if (defined(&destroy_interface_device)) { + &destroy_interface_device($b); + } } } diff --git a/net/t/run-tests.t b/net/t/run-tests.t index d0c6f4ae4..8aeb2f6ac 100644 --- a/net/t/run-tests.t +++ b/net/t/run-tests.t @@ -964,4 +964,89 @@ is_deeply(\@commands, "cd / ; ifconfig eth0.10 10\\.0\\.0\\.2 netmask 255\\.255\\.255\\.0 up 2>&1" ], "Linux VLAN interface falls back to vconfig without ip"); +# Test: Bond deactivation only brings it down, does not delete device +@commands = ( ); +{ +no warnings 'redefine'; +local *main::has_command = sub { + return $_[0] eq "ip" ? "/sbin/ip" : undef; + }; +main::deactivate_interface({ + 'name' => 'bond0', + 'fullname' => 'bond0', + 'virtual' => '', + 'address' => '10.0.0.2', + 'netmask' => '255.255.255.0', + 'address6' => [ ], + 'netmask6' => [ ], + 'up' => 1 + }); +} +is_deeply(\@commands, [ + "ip addr del 10\\.0\\.0\\.2\\/24 dev bond0 2>&1", + "ip link set dev bond0 down 2>&1" + ], "Linux bond deactivation removes address and brings link down"); + +# Test: Bond deletion removes virtual device after deactivation +@commands = ( ); +{ +no warnings 'redefine'; +no warnings 'once'; +local $main::gconfig{'os_type'} = 'debian-linux'; +local $main::gconfig{'os_version'} = 12; +local *main::has_command = sub { + return $_[0] eq "ip" ? "/sbin/ip" : + $_[0] eq "ifup" ? "/sbin/ifup" : undef; + }; +main::deactivate_interface({ + 'name' => 'bond0', + 'fullname' => 'bond0', + 'virtual' => '', + 'address' => '10.0.0.2', + 'netmask' => '255.255.255.0', + 'address6' => [ ], + 'netmask6' => [ ], + 'up' => 1 + }); +# Simulate delete path: destroy_interface_device after deactivation +my $b = { 'name' => 'bond0', 'fullname' => 'bond0', 'virtual' => '' }; +main::destroy_interface_device($b); +} +is_deeply(\@commands, [ + "ip addr del 10\\.0\\.0\\.2\\/24 dev bond0 2>&1", + "ip link set dev bond0 down 2>&1", + "ip link delete bond0 2>&1" + ], "Linux bond deletion removes device after deactivation"); + +# Test: VLAN deletion removes virtual device after deactivation +@commands = ( ); +{ +no warnings 'redefine'; +no warnings 'once'; +local $main::gconfig{'os_type'} = 'debian-linux'; +local $main::gconfig{'os_version'} = 12; +local *main::has_command = sub { + return $_[0] eq "ip" ? "/sbin/ip" : + $_[0] eq "ifup" ? "/sbin/ifup" : undef; + }; +main::deactivate_interface({ + 'name' => 'eth0.10', + 'fullname' => 'eth0.10', + 'virtual' => '', + 'address' => '10.0.10.2', + 'netmask' => '255.255.255.0', + 'address6' => [ ], + 'netmask6' => [ ], + 'up' => 1 + }); +# Simulate delete path: destroy_interface_device after deactivation +my $b = { 'name' => 'eth0.10', 'fullname' => 'eth0.10', 'virtual' => '' }; +main::destroy_interface_device($b); +} +is_deeply(\@commands, [ + "ip addr del 10\\.0\\.10\\.2\\/24 dev eth0\\.10 2>&1", + "ip link set dev eth0\\.10 down 2>&1", + "ip link delete eth0\\.10 2>&1" + ], "Linux VLAN deletion removes device after deactivation"); + done_testing();