[PATCH net-next 0/2] bridge: Redirect to backup port when port is administratively down

[PATCH net-next 0/2] bridge: Redirect to backup port when port is administratively down

[Ido Schimmel]

Patch #1 amends the bridge to redirect to the backup port when the
primary port is administratively down and not only when it does not have
a carrier. See the commit message for more details.

Patch #2 extends the bridge backup port selftest to cover this case.

Ido Schimmel (2):
  bridge: Redirect to backup port when port is administratively down
  selftests: net: Test bridge backup port when port is administratively
    down

 net/bridge/br_forward.c                       |  3 +-
 .../selftests/net/test_bridge_backup_port.sh  | 31 ++++++++++++++++---
 2 files changed, 29 insertions(+), 5 deletions(-)

-- 
2.50.1

[PATCH net-next 1/2] bridge: Redirect to backup port when port is administratively down

[Ido Schimmel]

If a backup port is configured for a bridge port, the bridge will
redirect known unicast traffic towards the backup port when the primary
port is administratively up but without a carrier. This is useful, for
example, in MLAG configurations where a system is connected to two
switches and there is a peer link between both switches. The peer link
serves as the backup port in case one of the switches loses its
connection to the multi-homed system.

In order to avoid flooding when the primary port loses its carrier, the
bridge does not flush dynamic FDB entries pointing to the port upon STP
disablement, if the port has a backup port.

The above means that known unicast traffic destined to the primary port
will be blackholed when the port is put administratively down, until the
FDB entries pointing to it are aged-out.

Given that the current behavior is quite weird and unlikely to be
depended on by anyone, amend the bridge to redirect to the backup port
also when the primary port is administratively down and not only when it
does not have a carrier.

The change is motivated by a report from a user who expected traffic to
be redirected to the backup port when the primary port was put
administratively down while debugging a network issue.

Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_forward.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 29097e984b4f..870bdf2e082c 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -148,7 +148,8 @@ void br_forward(const struct net_bridge_port *to,
 		goto out;
 
 	/* redirect to backup link if the destination port is down */
-	if (rcu_access_pointer(to->backup_port) && !netif_carrier_ok(to->dev)) {
+	if (rcu_access_pointer(to->backup_port) &&
+	    (!netif_carrier_ok(to->dev) || !netif_running(to->dev))) {
 		struct net_bridge_port *backup_port;
 
 		backup_port = rcu_dereference(to->backup_port);
-- 
2.50.1

[PATCH net-next 2/2] selftests: net: Test bridge backup port when port is administratively down

[Ido Schimmel]

Test that packets are redirected to the backup port when the primary
port is administratively down.

With the previous patch:

 # ./test_bridge_backup_port.sh
 [...]
 TEST: swp1 administratively down                                    [ OK ]
 TEST: No forwarding out of swp1                                     [ OK ]
 TEST: Forwarding out of vx0                                         [ OK ]
 TEST: swp1 administratively up                                      [ OK ]
 TEST: Forwarding out of swp1                                        [ OK ]
 TEST: No forwarding out of vx0                                      [ OK ]
 [...]
 Tests passed:  89
 Tests failed:   0

Without the previous patch:

 # ./test_bridge_backup_port.sh
 [...]
 TEST: swp1 administratively down                                    [ OK ]
 TEST: No forwarding out of swp1                                     [ OK ]
 TEST: Forwarding out of vx0                                         [FAIL]
 TEST: swp1 administratively up                                      [ OK ]
 TEST: Forwarding out of swp1                                        [ OK ]
 [...]
 Tests passed:  85
 Tests failed:   4

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../selftests/net/test_bridge_backup_port.sh  | 31 ++++++++++++++++---
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/test_bridge_backup_port.sh b/tools/testing/selftests/net/test_bridge_backup_port.sh
index 1b3f89e2b86e..2a7224fe74f2 100755
--- a/tools/testing/selftests/net/test_bridge_backup_port.sh
+++ b/tools/testing/selftests/net/test_bridge_backup_port.sh
@@ -315,6 +315,29 @@ backup_port()
 	tc_check_packets $sw1 "dev vx0 egress" 101 1
 	log_test $? 0 "No forwarding out of vx0"
 
+	# Check that packets are forwarded out of vx0 when swp1 is
+	# administratively down and out of swp1 when it is administratively up
+	# again.
+	run_cmd "ip -n $sw1 link set dev swp1 down"
+	busywait $BUSYWAIT_TIMEOUT bridge_link_check $sw1 swp1 disabled
+	log_test $? 0 "swp1 administratively down"
+
+	run_cmd "ip netns exec $sw1 mausezahn br0.10 -a $smac -b $dmac -A 198.51.100.1 -B 198.51.100.2 -t ip -p 100 -q -c 1"
+	tc_check_packets $sw1 "dev swp1 egress" 101 3
+	log_test $? 0 "No forwarding out of swp1"
+	tc_check_packets $sw1 "dev vx0 egress" 101 2
+	log_test $? 0 "Forwarding out of vx0"
+
+	run_cmd "ip -n $sw1 link set dev swp1 up"
+	busywait $BUSYWAIT_TIMEOUT bridge_link_check $sw1 swp1 forwarding
+	log_test $? 0 "swp1 administratively up"
+
+	run_cmd "ip netns exec $sw1 mausezahn br0.10 -a $smac -b $dmac -A 198.51.100.1 -B 198.51.100.2 -t ip -p 100 -q -c 1"
+	tc_check_packets $sw1 "dev swp1 egress" 101 4
+	log_test $? 0 "Forwarding out of swp1"
+	tc_check_packets $sw1 "dev vx0 egress" 101 2
+	log_test $? 0 "No forwarding out of vx0"
+
 	# Remove vx0 as the backup port of swp1 and check that packets are no
 	# longer forwarded out of vx0 when swp1 does not have a carrier.
 	run_cmd "bridge -n $sw1 link set dev swp1 nobackup_port"
@@ -322,9 +345,9 @@ backup_port()
 	log_test $? 1 "vx0 not configured as backup port of swp1"
 
 	run_cmd "ip netns exec $sw1 mausezahn br0.10 -a $smac -b $dmac -A 198.51.100.1 -B 198.51.100.2 -t ip -p 100 -q -c 1"
-	tc_check_packets $sw1 "dev swp1 egress" 101 4
+	tc_check_packets $sw1 "dev swp1 egress" 101 5
 	log_test $? 0 "Forwarding out of swp1"
-	tc_check_packets $sw1 "dev vx0 egress" 101 1
+	tc_check_packets $sw1 "dev vx0 egress" 101 2
 	log_test $? 0 "No forwarding out of vx0"
 
 	run_cmd "ip -n $sw1 link set dev swp1 carrier off"
@@ -332,9 +355,9 @@ backup_port()
 	log_test $? 0 "swp1 carrier off"
 
 	run_cmd "ip netns exec $sw1 mausezahn br0.10 -a $smac -b $dmac -A 198.51.100.1 -B 198.51.100.2 -t ip -p 100 -q -c 1"
-	tc_check_packets $sw1 "dev swp1 egress" 101 4
+	tc_check_packets $sw1 "dev swp1 egress" 101 5
 	log_test $? 0 "No forwarding out of swp1"
-	tc_check_packets $sw1 "dev vx0 egress" 101 1
+	tc_check_packets $sw1 "dev vx0 egress" 101 2
 	log_test $? 0 "No forwarding out of vx0"
 }
 
-- 
2.50.1

Re: [PATCH net-next 1/2] bridge: Redirect to backup port when port is administratively down

[Nikolay Aleksandrov]

On 8/12/25 11:02, Ido Schimmel wrote:
> If a backup port is configured for a bridge port, the bridge will
> redirect known unicast traffic towards the backup port when the primary
> port is administratively up but without a carrier. This is useful, for
> example, in MLAG configurations where a system is connected to two
> switches and there is a peer link between both switches. The peer link
> serves as the backup port in case one of the switches loses its
> connection to the multi-homed system.
> 
> In order to avoid flooding when the primary port loses its carrier, the
> bridge does not flush dynamic FDB entries pointing to the port upon STP
> disablement, if the port has a backup port.
> 
> The above means that known unicast traffic destined to the primary port
> will be blackholed when the port is put administratively down, until the
> FDB entries pointing to it are aged-out.
> 
> Given that the current behavior is quite weird and unlikely to be
> depended on by anyone, amend the bridge to redirect to the backup port
> also when the primary port is administratively down and not only when it
> does not have a carrier.
> 

hehe I did ask that question long time ago while adding support for backup ports,
at the time wasn't needed for the MLAG case :-)

> The change is motivated by a report from a user who expected traffic to
> be redirected to the backup port when the primary port was put
> administratively down while debugging a network issue.
> 
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>   net/bridge/br_forward.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index 29097e984b4f..870bdf2e082c 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -148,7 +148,8 @@ void br_forward(const struct net_bridge_port *to,
>   		goto out;
>   
>   	/* redirect to backup link if the destination port is down */
> -	if (rcu_access_pointer(to->backup_port) && !netif_carrier_ok(to->dev)) {
> +	if (rcu_access_pointer(to->backup_port) &&
> +	    (!netif_carrier_ok(to->dev) || !netif_running(to->dev))) {
>   		struct net_bridge_port *backup_port;
>   
>   		backup_port = rcu_dereference(to->backup_port);

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 2/2] selftests: net: Test bridge backup port when port is administratively down

[Nikolay Aleksandrov]

On 8/12/25 11:02, Ido Schimmel wrote:
> Test that packets are redirected to the backup port when the primary
> port is administratively down.
> 
> With the previous patch:
> 
>   # ./test_bridge_backup_port.sh
>   [...]
>   TEST: swp1 administratively down                                    [ OK ]
>   TEST: No forwarding out of swp1                                     [ OK ]
>   TEST: Forwarding out of vx0                                         [ OK ]
>   TEST: swp1 administratively up                                      [ OK ]
>   TEST: Forwarding out of swp1                                        [ OK ]
>   TEST: No forwarding out of vx0                                      [ OK ]
>   [...]
>   Tests passed:  89
>   Tests failed:   0
> 
> Without the previous patch:
> 
>   # ./test_bridge_backup_port.sh
>   [...]
>   TEST: swp1 administratively down                                    [ OK ]
>   TEST: No forwarding out of swp1                                     [ OK ]
>   TEST: Forwarding out of vx0                                         [FAIL]
>   TEST: swp1 administratively up                                      [ OK ]
>   TEST: Forwarding out of swp1                                        [ OK ]
>   [...]
>   Tests passed:  85
>   Tests failed:   4
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>   .../selftests/net/test_bridge_backup_port.sh  | 31 ++++++++++++++++---
>   1 file changed, 27 insertions(+), 4 deletions(-)
> 

Nice!
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 1/2] bridge: Redirect to backup port when port is administratively down

[Jakub Kicinski]

On Tue, 12 Aug 2025 11:02:12 +0300 Ido Schimmel wrote:
>  	/* redirect to backup link if the destination port is down */
> -	if (rcu_access_pointer(to->backup_port) && !netif_carrier_ok(to->dev)) {
> +	if (rcu_access_pointer(to->backup_port) &&
> +	    (!netif_carrier_ok(to->dev) || !netif_running(to->dev))) {

Not really blocking this patch, but I always wondered why we allow
devices with carrier on in admin down state. Is his just something we
have because updating 200 drivers which don't manage carrier today
would be a PITA? Or there's a stronger reason to allow this?
Hopefully I'm not misreading the patch..

Re: [PATCH net-next 1/2] bridge: Redirect to backup port when port is administratively down

[Ido Schimmel]

On Wed, Aug 13, 2025 at 05:20:17PM -0700, Jakub Kicinski wrote:
> On Tue, 12 Aug 2025 11:02:12 +0300 Ido Schimmel wrote:
> >  	/* redirect to backup link if the destination port is down */
> > -	if (rcu_access_pointer(to->backup_port) && !netif_carrier_ok(to->dev)) {
> > +	if (rcu_access_pointer(to->backup_port) &&
> > +	    (!netif_carrier_ok(to->dev) || !netif_running(to->dev))) {
> 
> Not really blocking this patch, but I always wondered why we allow
> devices with carrier on in admin down state. Is his just something we
> have because updating 200 drivers which don't manage carrier today
> would be a PITA? Or there's a stronger reason to allow this?
> Hopefully I'm not misreading the patch..

Probably the first reason.

The primary bridge port and its backup are usually bonds (the feature is
called "LAG Redirect" in hardware) and the bond driver does not turn off
its carrier upon admin down:

$ ip link show dev bond1
83: bond1: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 9c:05:91:9b:5e:f1 brd ff:ff:ff:ff:ff:ff
$ cat /sys/class/net/bond1/carrier_changes 
2
# ip link set dev bond1 down
$ cat /sys/class/net/bond1/carrier_changes 
2

Same thing happens with the dummy device that is used in the selftest. 

Re: [PATCH net-next 1/2] bridge: Redirect to backup port when port is administratively down

[Jakub Kicinski]

On Thu, 14 Aug 2025 11:28:58 +0300 Ido Schimmel wrote:
> On Wed, Aug 13, 2025 at 05:20:17PM -0700, Jakub Kicinski wrote:
> > On Tue, 12 Aug 2025 11:02:12 +0300 Ido Schimmel wrote:  
> > >  	/* redirect to backup link if the destination port is down */
> > > -	if (rcu_access_pointer(to->backup_port) && !netif_carrier_ok(to->dev)) {
> > > +	if (rcu_access_pointer(to->backup_port) &&
> > > +	    (!netif_carrier_ok(to->dev) || !netif_running(to->dev))) {  
> > 
> > Not really blocking this patch, but I always wondered why we allow
> > devices with carrier on in admin down state. Is his just something we
> > have because updating 200 drivers which don't manage carrier today
> > would be a PITA? Or there's a stronger reason to allow this?
> > Hopefully I'm not misreading the patch..  
> 
> Probably the first reason.

Thanks, let me add clearing carrier to our list of potential cleanup
projects.

Re: [PATCH net-next 0/2] bridge: Redirect to backup port when port is administratively down

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 12 Aug 2025 11:02:11 +0300 you wrote:
> Patch #1 amends the bridge to redirect to the backup port when the
> primary port is administratively down and not only when it does not have
> a carrier. See the commit message for more details.
> 
> Patch #2 extends the bridge backup port selftest to cover this case.
> 
> Ido Schimmel (2):
>   bridge: Redirect to backup port when port is administratively down
>   selftests: net: Test bridge backup port when port is administratively
>     down
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] bridge: Redirect to backup port when port is administratively down
    https://git.kernel.org/netdev/net-next/c/3d05b24429e1
  - [net-next,2/2] selftests: net: Test bridge backup port when port is administratively down
    https://git.kernel.org/netdev/net-next/c/51ca1e67f416

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html