| Message ID | 20240103094846.2397083-3-nicolas.dichtel@6wind.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | rtnetlink: allow to enslave with one msg an up interface | expand |
On Wed, Jan 03, 2024 at 10:48:46AM +0100, Nicolas Dichtel wrote: > +kci_test_enslave_bonding() > +{ > + local testns="testns" > + local bond="bond123" > + local dummy="dummy123" > + local ret=0 > + > + run_cmd ip netns add "$testns" > + if [ $? -ne 0 ]; then > + end_test "SKIP bonding tests: cannot add net namespace $testns" > + return $ksft_skip > + fi > + > + # test native tunnel > + run_cmd ip -netns $testns link add dev $bond type bond mode balance-rr Hi Nicolas, The net-next added new function setup_ns in lib.sh and converted all hard code netns setup. I think It may be good to post this patch set to net-next to reduce future merge conflicts. Jakub, Paolo, please correct me if we can't post fixes to net-next. Thanks Hangbin
Le 03/01/2024 à 14:00, Hangbin Liu a écrit : > On Wed, Jan 03, 2024 at 10:48:46AM +0100, Nicolas Dichtel wrote: >> +kci_test_enslave_bonding() >> +{ >> + local testns="testns" >> + local bond="bond123" >> + local dummy="dummy123" >> + local ret=0 >> + >> + run_cmd ip netns add "$testns" >> + if [ $? -ne 0 ]; then >> + end_test "SKIP bonding tests: cannot add net namespace $testns" >> + return $ksft_skip >> + fi >> + >> + # test native tunnel >> + run_cmd ip -netns $testns link add dev $bond type bond mode balance-rr > > Hi Nicolas, > > The net-next added new function setup_ns in lib.sh and converted all hard code > netns setup. I think It may be good to post this patch set to net-next > to reduce future merge conflicts. The first patch is for net. I can post the second one to net-next if it eases the merge. > > Jakub, Paolo, please correct me if we can't post fixes to net-next. Please, let me know if I should target net-next for the second patch. Regards, Nicolas
On 1/3/24 2:48 AM, Nicolas Dichtel wrote: > @@ -1239,6 +1240,44 @@ kci_test_address_proto() > return $ret > } > > +kci_test_enslave_bonding() > +{ > + local testns="testns" > + local bond="bond123" > + local dummy="dummy123" > + local ret=0 > + > + run_cmd ip netns add "$testns" > + if [ $? -ne 0 ]; then > + end_test "SKIP bonding tests: cannot add net namespace $testns" > + return $ksft_skip > + fi > + > + # test native tunnel > + run_cmd ip -netns $testns link add dev $bond type bond mode balance-rr > + run_cmd ip -netns $testns link add dev $dummy type dummy > + run_cmd ip -netns $testns link set dev $dummy up > + run_cmd ip -netns $testns link set dev $dummy master $bond down > + if [ $ret -ne 0 ]; then > + end_test "FAIL: enslave an up interface in a bonding" interface is up, being put as a port on a bond and taken down at the same time. That does not match this error message. Thanks for adding test cases. Besides the error message: Reviewed-by: David Ahern <dsahern@kernel.org>
Le 03/01/2024 à 16:42, David Ahern a écrit : > On 1/3/24 2:48 AM, Nicolas Dichtel wrote: >> @@ -1239,6 +1240,44 @@ kci_test_address_proto() >> return $ret >> } >> >> +kci_test_enslave_bonding() >> +{ >> + local testns="testns" >> + local bond="bond123" >> + local dummy="dummy123" >> + local ret=0 >> + >> + run_cmd ip netns add "$testns" >> + if [ $? -ne 0 ]; then >> + end_test "SKIP bonding tests: cannot add net namespace $testns" >> + return $ksft_skip >> + fi >> + >> + # test native tunnel >> + run_cmd ip -netns $testns link add dev $bond type bond mode balance-rr >> + run_cmd ip -netns $testns link add dev $dummy type dummy >> + run_cmd ip -netns $testns link set dev $dummy up >> + run_cmd ip -netns $testns link set dev $dummy master $bond down >> + if [ $ret -ne 0 ]; then >> + end_test "FAIL: enslave an up interface in a bonding" > > interface is up, being put as a port on a bond and taken down at the > same time. That does not match this error message. The idea was "the command used to enslave an up interface fails". What about: "FAIL: set down and enslave an up interface in a bonding" > > > Thanks for adding test cases. Besides the error message: > > Reviewed-by: David Ahern <dsahern@kernel.org Thank you, Nicolas
On 1/3/24 9:17 AM, Nicolas Dichtel wrote: > Le 03/01/2024 à 16:42, David Ahern a écrit : >> On 1/3/24 2:48 AM, Nicolas Dichtel wrote: >>> @@ -1239,6 +1240,44 @@ kci_test_address_proto() >>> return $ret >>> } >>> >>> +kci_test_enslave_bonding() >>> +{ >>> + local testns="testns" >>> + local bond="bond123" >>> + local dummy="dummy123" >>> + local ret=0 >>> + >>> + run_cmd ip netns add "$testns" >>> + if [ $? -ne 0 ]; then >>> + end_test "SKIP bonding tests: cannot add net namespace $testns" >>> + return $ksft_skip >>> + fi >>> + >>> + # test native tunnel >>> + run_cmd ip -netns $testns link add dev $bond type bond mode balance-rr >>> + run_cmd ip -netns $testns link add dev $dummy type dummy >>> + run_cmd ip -netns $testns link set dev $dummy up >>> + run_cmd ip -netns $testns link set dev $dummy master $bond down >>> + if [ $ret -ne 0 ]; then >>> + end_test "FAIL: enslave an up interface in a bonding" >> >> interface is up, being put as a port on a bond and taken down at the >> same time. That does not match this error message. > The idea was "the command used to enslave an up interface fails". > What about: "FAIL: set down and enslave an up interface in a bonding" The 'set down' is part of the adding to the bond command. how about: FAIL: Initially up interface added to a bond and set down.
On Wed, 3 Jan 2024 15:15:33 +0100 Nicolas Dichtel wrote: > > The net-next added new function setup_ns in lib.sh and converted all hard code > > netns setup. I think It may be good to post this patch set to net-next > > to reduce future merge conflicts. > > The first patch is for net. I can post the second one to net-next if it eases > the merge. > > > Jakub, Paolo, please correct me if we can't post fixes to net-next. > > Please, let me know if I should target net-next for the second patch. Looks like the patch applies to net-next, so hopefully there won't be any actual conflicts. But it'd be good to follow up and refactor it in net-next once net gets merged in. As long as I'm not missing anything - up to you - I'm fine with either sending the test to net-next like Hangbin suggests, or following up in net-next to use setup_ns.
Le 03/01/2024 à 22:21, Jakub Kicinski a écrit : > On Wed, 3 Jan 2024 15:15:33 +0100 Nicolas Dichtel wrote: >>> The net-next added new function setup_ns in lib.sh and converted all hard code >>> netns setup. I think It may be good to post this patch set to net-next >>> to reduce future merge conflicts. >> >> The first patch is for net. I can post the second one to net-next if it eases >> the merge. >> >>> Jakub, Paolo, please correct me if we can't post fixes to net-next. >> >> Please, let me know if I should target net-next for the second patch. > > Looks like the patch applies to net-next, so hopefully there won't > be any actual conflicts. But it'd be good to follow up and refactor > it in net-next once net gets merged in. As long as I'm not missing > anything - up to you - I'm fine with either sending the test to > net-next like Hangbin suggests, or following up in net-next to use > setup_ns. I will send a follow-up once net gets merged in net-next. Thank you, Nicolas
On Thu, 4 Jan 2024 14:51:08 +0100 Nicolas Dichtel wrote: > > Looks like the patch applies to net-next, so hopefully there won't > > be any actual conflicts. But it'd be good to follow up and refactor > > it in net-next once net gets merged in. As long as I'm not missing > > anything - up to you - I'm fine with either sending the test to > > net-next like Hangbin suggests, or following up in net-next to use > > setup_ns. > I will send a follow-up once net gets merged in net-next. Ack, there's still going to be a v3 to update the error message, tho, right?
Le 04/01/2024 à 16:17, Jakub Kicinski a écrit : > On Thu, 4 Jan 2024 14:51:08 +0100 Nicolas Dichtel wrote: >>> Looks like the patch applies to net-next, so hopefully there won't >>> be any actual conflicts. But it'd be good to follow up and refactor >>> it in net-next once net gets merged in. As long as I'm not missing >>> anything - up to you - I'm fine with either sending the test to >>> net-next like Hangbin suggests, or following up in net-next to use >>> setup_ns. >> I will send a follow-up once net gets merged in net-next. > > Ack, there's still going to be a v3 to update the error message, > tho, right? Right, I was waiting for the conclusion about this patch ;-)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh index 26827ea4e3e5..130be7de76af 100755 --- a/tools/testing/selftests/net/rtnetlink.sh +++ b/tools/testing/selftests/net/rtnetlink.sh @@ -28,6 +28,7 @@ ALL_TESTS=" kci_test_neigh_get kci_test_bridge_parent_id kci_test_address_proto + kci_test_enslave_bonding " devdummy="test-dummy0" @@ -1239,6 +1240,44 @@ kci_test_address_proto() return $ret } +kci_test_enslave_bonding() +{ + local testns="testns" + local bond="bond123" + local dummy="dummy123" + local ret=0 + + run_cmd ip netns add "$testns" + if [ $? -ne 0 ]; then + end_test "SKIP bonding tests: cannot add net namespace $testns" + return $ksft_skip + fi + + # test native tunnel + run_cmd ip -netns $testns link add dev $bond type bond mode balance-rr + run_cmd ip -netns $testns link add dev $dummy type dummy + run_cmd ip -netns $testns link set dev $dummy up + run_cmd ip -netns $testns link set dev $dummy master $bond down + if [ $ret -ne 0 ]; then + end_test "FAIL: enslave an up interface in a bonding" + ip netns del "$testns" + return 1 + fi + + run_cmd ip -netns $testns link del dev $dummy + run_cmd ip -netns $testns link add dev $dummy type dummy + run_cmd ip -netns $testns link set dev $dummy down + run_cmd ip -netns $testns link set dev $dummy master $bond up + if [ $ret -ne 0 ]; then + end_test "FAIL: enslave an down interface in a bonding and set it up" + ip netns del "$testns" + return 1 + fi + + end_test "PASS: enslave iface in a bonding" + ip netns del "$testns" +} + kci_test_rtnl() { local current_test