* [PATCH net v2 0/2] rtnetlink: allow to enslave with one msg an up interface
@ 2024-01-03 9:48 Nicolas Dichtel
2024-01-03 9:48 ` [PATCH net v2 1/2] rtnetlink: allow to set iface down before enslaving it Nicolas Dichtel
2024-01-03 9:48 ` [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond Nicolas Dichtel
0 siblings, 2 replies; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-03 9:48 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern
Cc: netdev
The first patch fixes a regression introduced in linux v6.1. The second
patch adds some tests to verify this API.
v1 -> v2:
- add the #2 patch
net/core/rtnetlink.c | 8 +++++++
tools/testing/selftests/net/rtnetlink.sh | 39 ++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
Regards,
Nicolas
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net v2 1/2] rtnetlink: allow to set iface down before enslaving it
2024-01-03 9:48 [PATCH net v2 0/2] rtnetlink: allow to enslave with one msg an up interface Nicolas Dichtel
@ 2024-01-03 9:48 ` Nicolas Dichtel
2024-01-03 15:38 ` David Ahern
2024-01-03 9:48 ` [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond Nicolas Dichtel
1 sibling, 1 reply; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-03 9:48 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern
Cc: netdev, Nicolas Dichtel, stable
The below commit adds support for:
> ip link set dummy0 down
> ip link set dummy0 master bond0 up
but breaks the opposite:
> ip link set dummy0 up
> ip link set dummy0 master bond0 down
Let's add a workaround to have both commands working.
Cc: stable@vger.kernel.org
Fixes: a4abfa627c38 ("net: rtnetlink: Enslave device before bringing it up")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Acked-by: Phil Sutter <phil@nwl.cc>
---
net/core/rtnetlink.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e8431c6c8490..dd79693c2d91 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2905,6 +2905,14 @@ static int do_setlink(const struct sk_buff *skb,
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
}
+ /* Backward compat: enable to set interface down before enslaving it */
+ if (!(ifm->ifi_flags & IFF_UP) && ifm->ifi_change & IFF_UP) {
+ err = dev_change_flags(dev, rtnl_dev_combine_flags(dev, ifm),
+ extack);
+ if (err < 0)
+ goto errout;
+ }
+
if (tb[IFLA_MASTER]) {
err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]), extack);
if (err)
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 9:48 [PATCH net v2 0/2] rtnetlink: allow to enslave with one msg an up interface Nicolas Dichtel
2024-01-03 9:48 ` [PATCH net v2 1/2] rtnetlink: allow to set iface down before enslaving it Nicolas Dichtel
@ 2024-01-03 9:48 ` Nicolas Dichtel
2024-01-03 13:00 ` Hangbin Liu
2024-01-03 15:42 ` David Ahern
1 sibling, 2 replies; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-03 9:48 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern
Cc: netdev, Nicolas Dichtel
The goal is to check the following two sequences:
> ip link set dummy0 down
> ip link set dummy0 master bond0 up
and
> ip link set dummy0 up
> ip link set dummy0 master bond0 down
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
tools/testing/selftests/net/rtnetlink.sh | 39 ++++++++++++++++++++++++
1 file changed, 39 insertions(+)
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
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 9:48 ` [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond Nicolas Dichtel
@ 2024-01-03 13:00 ` Hangbin Liu
2024-01-03 14:15 ` Nicolas Dichtel
2024-01-03 15:42 ` David Ahern
1 sibling, 1 reply; 13+ messages in thread
From: Hangbin Liu @ 2024-01-03 13:00 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern, netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 13:00 ` Hangbin Liu
@ 2024-01-03 14:15 ` Nicolas Dichtel
2024-01-03 21:21 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-03 14:15 UTC (permalink / raw)
To: Hangbin Liu
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern, netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 1/2] rtnetlink: allow to set iface down before enslaving it
2024-01-03 9:48 ` [PATCH net v2 1/2] rtnetlink: allow to set iface down before enslaving it Nicolas Dichtel
@ 2024-01-03 15:38 ` David Ahern
0 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2024-01-03 15:38 UTC (permalink / raw)
To: Nicolas Dichtel, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Phil Sutter
Cc: netdev, stable
On 1/3/24 2:48 AM, Nicolas Dichtel wrote:
> The below commit adds support for:
>> ip link set dummy0 down
>> ip link set dummy0 master bond0 up
>
> but breaks the opposite:
>> ip link set dummy0 up
>> ip link set dummy0 master bond0 down
>
> Let's add a workaround to have both commands working.
>
> Cc: stable@vger.kernel.org
> Fixes: a4abfa627c38 ("net: rtnetlink: Enslave device before bringing it up")
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Acked-by: Phil Sutter <phil@nwl.cc>
> ---
> net/core/rtnetlink.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 9:48 ` [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond Nicolas Dichtel
2024-01-03 13:00 ` Hangbin Liu
@ 2024-01-03 15:42 ` David Ahern
2024-01-03 16:17 ` Nicolas Dichtel
1 sibling, 1 reply; 13+ messages in thread
From: David Ahern @ 2024-01-03 15:42 UTC (permalink / raw)
To: Nicolas Dichtel, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Phil Sutter
Cc: netdev
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 15:42 ` David Ahern
@ 2024-01-03 16:17 ` Nicolas Dichtel
2024-01-03 16:36 ` David Ahern
0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-03 16:17 UTC (permalink / raw)
To: David Ahern, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Phil Sutter
Cc: netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 16:17 ` Nicolas Dichtel
@ 2024-01-03 16:36 ` David Ahern
0 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2024-01-03 16:36 UTC (permalink / raw)
To: nicolas.dichtel, David S . Miller, Jakub Kicinski, Paolo Abeni,
Eric Dumazet, Phil Sutter
Cc: netdev
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 14:15 ` Nicolas Dichtel
@ 2024-01-03 21:21 ` Jakub Kicinski
2024-01-04 13:51 ` Nicolas Dichtel
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2024-01-03 21:21 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Hangbin Liu, David S . Miller, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern, netdev
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-03 21:21 ` Jakub Kicinski
@ 2024-01-04 13:51 ` Nicolas Dichtel
2024-01-04 15:17 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-04 13:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Hangbin Liu, David S . Miller, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern, netdev
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-04 13:51 ` Nicolas Dichtel
@ 2024-01-04 15:17 ` Jakub Kicinski
2024-01-04 16:39 ` Nicolas Dichtel
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2024-01-04 15:17 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Hangbin Liu, David S . Miller, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern, netdev
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?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond
2024-01-04 15:17 ` Jakub Kicinski
@ 2024-01-04 16:39 ` Nicolas Dichtel
0 siblings, 0 replies; 13+ messages in thread
From: Nicolas Dichtel @ 2024-01-04 16:39 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Hangbin Liu, David S . Miller, Paolo Abeni, Eric Dumazet,
Phil Sutter, David Ahern, netdev
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 ;-)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-01-04 16:39 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-03 9:48 [PATCH net v2 0/2] rtnetlink: allow to enslave with one msg an up interface Nicolas Dichtel
2024-01-03 9:48 ` [PATCH net v2 1/2] rtnetlink: allow to set iface down before enslaving it Nicolas Dichtel
2024-01-03 15:38 ` David Ahern
2024-01-03 9:48 ` [PATCH net v2 2/2] selftests: rtnetlink: check enslaving iface in a bond Nicolas Dichtel
2024-01-03 13:00 ` Hangbin Liu
2024-01-03 14:15 ` Nicolas Dichtel
2024-01-03 21:21 ` Jakub Kicinski
2024-01-04 13:51 ` Nicolas Dichtel
2024-01-04 15:17 ` Jakub Kicinski
2024-01-04 16:39 ` Nicolas Dichtel
2024-01-03 15:42 ` David Ahern
2024-01-03 16:17 ` Nicolas Dichtel
2024-01-03 16:36 ` David Ahern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.