-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Nikolay Aleksandrov says:
====================
bonding: properly restore flags when bond changes ether type
A bug was reported by syzbot[1] that causes a warning and a myriad of
other potential issues if a bond, that is also a slave, fails to enslave a
non-eth device. While fixing that bug I found that we have the same
issues when such enslave passes and after that the bond changes back to
ARPHRD_ETHER (again due to ether_setup). This set fixes all issues by
extracting the ether_setup() sequence in a helper which does the right
thing about bond flags when it needs to change back to ARPHRD_ETHER. It
also adds selftests for these cases.
Patch 01 adds the new bond_ether_setup helper and fixes the issues when a
bond device changes its ether type due to successful enslave. Patch 02
fixes the issues when it changes its ether type due to an unsuccessful
enslave. Note we need two patches because the bugs were introduced by
different commits. Patch 03 adds the new selftests.
Due to the comment adjustment and squash, could you please review
patch 01 again? I've kept the other acks since there were no code
changes.
v3: squash the helper patch and the first fix, adjust the comment above
it to be explicit about the bond device, no code changes
v2: new set, all patches are new due to new approach of fixing these bugs
[1] https://syzkaller.appspot.com/bug?id=391c7b1f6522182899efba27d891f1743e8eb3ef
====================
Signed-off-by: David S. Miller <davem@davemloft.net>- Loading branch information
Showing
3 changed files
with
103 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
85 changes: 85 additions & 0 deletions
85
tools/testing/selftests/drivers/net/bonding/bond-eth-type-change.sh
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| #!/bin/bash | ||
| # SPDX-License-Identifier: GPL-2.0 | ||
| # | ||
| # Test bond device ether type changing | ||
| # | ||
|
|
||
| ALL_TESTS=" | ||
| bond_test_unsuccessful_enslave_type_change | ||
| bond_test_successful_enslave_type_change | ||
| " | ||
| REQUIRE_MZ=no | ||
| NUM_NETIFS=0 | ||
| lib_dir=$(dirname "$0") | ||
| source "$lib_dir"/net_forwarding_lib.sh | ||
|
|
||
| bond_check_flags() | ||
| { | ||
| local bonddev=$1 | ||
|
|
||
| ip -d l sh dev "$bonddev" | grep -q "MASTER" | ||
| check_err $? "MASTER flag is missing from the bond device" | ||
|
|
||
| ip -d l sh dev "$bonddev" | grep -q "SLAVE" | ||
| check_err $? "SLAVE flag is missing from the bond device" | ||
| } | ||
|
|
||
| # test enslaved bond dev type change from ARPHRD_ETHER and back | ||
| # this allows us to test both MASTER and SLAVE flags at once | ||
| bond_test_enslave_type_change() | ||
| { | ||
| local test_success=$1 | ||
| local devbond0="test-bond0" | ||
| local devbond1="test-bond1" | ||
| local devbond2="test-bond2" | ||
| local nonethdev="test-noneth0" | ||
|
|
||
| # create a non-ARPHRD_ETHER device for testing (e.g. nlmon type) | ||
| ip link add name "$nonethdev" type nlmon | ||
| check_err $? "could not create a non-ARPHRD_ETHER device (nlmon)" | ||
| ip link add name "$devbond0" type bond | ||
| if [ $test_success -eq 1 ]; then | ||
| # we need devbond0 in active-backup mode to successfully enslave nonethdev | ||
| ip link set dev "$devbond0" type bond mode active-backup | ||
| check_err $? "could not change bond mode to active-backup" | ||
| fi | ||
| ip link add name "$devbond1" type bond | ||
| ip link add name "$devbond2" type bond | ||
| ip link set dev "$devbond0" master "$devbond1" | ||
| check_err $? "could not enslave $devbond0 to $devbond1" | ||
| # change bond type to non-ARPHRD_ETHER | ||
| ip link set dev "$nonethdev" master "$devbond0" 1>/dev/null 2>/dev/null | ||
| ip link set dev "$nonethdev" nomaster 1>/dev/null 2>/dev/null | ||
| # restore ARPHRD_ETHER type by enslaving such device | ||
| ip link set dev "$devbond2" master "$devbond0" | ||
| check_err $? "could not enslave $devbond2 to $devbond0" | ||
| ip link set dev "$devbond1" nomaster | ||
|
|
||
| bond_check_flags "$devbond0" | ||
|
|
||
| # clean up | ||
| ip link del dev "$devbond0" | ||
| ip link del dev "$devbond1" | ||
| ip link del dev "$devbond2" | ||
| ip link del dev "$nonethdev" | ||
| } | ||
|
|
||
| bond_test_unsuccessful_enslave_type_change() | ||
| { | ||
| RET=0 | ||
|
|
||
| bond_test_enslave_type_change 0 | ||
| log_test "Change ether type of an enslaved bond device with unsuccessful enslave" | ||
| } | ||
|
|
||
| bond_test_successful_enslave_type_change() | ||
| { | ||
| RET=0 | ||
|
|
||
| bond_test_enslave_type_change 1 | ||
| log_test "Change ether type of an enslaved bond device with successful enslave" | ||
| } | ||
|
|
||
| tests_run | ||
|
|
||
| exit "$EXIT_STATUS" |