diff mbox series

[net,v4,1/2] Revert "net: rtnetlink: Enslave device before bringing it up"

Message ID 20240108094103.2001224-2-nicolas.dichtel@6wind.com (mailing list archive)
State Accepted
Commit ec4ffd100ffb396eca13ebe7d18938ea80f399c3
Delegated to: Netdev Maintainers
Headers show
Series rtnetlink: allow to enslave with one msg an up interface | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1141 this patch: 1141
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 26 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Nicolas Dichtel Jan. 8, 2024, 9:41 a.m. UTC 
This reverts commit a4abfa627c3865c37e036bccb681619a50d3d93c.

The patch broke:
> ip link set dummy0 up
> ip link set dummy0 master bond0 down

This last command is useful to be able to enslave an interface with only
one netlink message.

After discussion, there is no good reason to support:
> ip link set dummy0 down
> ip link set dummy0 master bond0 up
because the bond interface already set the slave up when it is up.

Cc: stable@vger.kernel.org
Fixes: a4abfa627c38 ("net: rtnetlink: Enslave device before bringing it up")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/core/rtnetlink.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Jiri Pirko Jan. 8, 2024, 10:29 a.m. UTC | #1 
Mon, Jan 08, 2024 at 10:41:02AM CET, nicolas.dichtel@6wind.com wrote:
>This reverts commit a4abfa627c3865c37e036bccb681619a50d3d93c.
>
>The patch broke:
>> ip link set dummy0 up
>> ip link set dummy0 master bond0 down
>
>This last command is useful to be able to enslave an interface with only
>one netlink message.
>
>After discussion, there is no good reason to support:
>> ip link set dummy0 down
>> ip link set dummy0 master bond0 up
>because the bond interface already set the slave up when it is up.
>
>Cc: stable@vger.kernel.org
>Fixes: a4abfa627c38 ("net: rtnetlink: Enslave device before bringing it up")
>Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

Thanks!
Hangbin Liu Jan. 9, 2024, 3:38 a.m. UTC | #2 
On Mon, Jan 08, 2024 at 10:41:02AM +0100, Nicolas Dichtel wrote:
> This reverts commit a4abfa627c3865c37e036bccb681619a50d3d93c.
> 
> The patch broke:
> > ip link set dummy0 up
> > ip link set dummy0 master bond0 down
> 
> This last command is useful to be able to enslave an interface with only
> one netlink message.
> 
> After discussion, there is no good reason to support:
> > ip link set dummy0 down
> > ip link set dummy0 master bond0 up
> because the bond interface already set the slave up when it is up.
> 
> Cc: stable@vger.kernel.org
> Fixes: a4abfa627c38 ("net: rtnetlink: Enslave device before bringing it up")
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>
diff mbox series

Patch

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e8431c6c8490..bf4c3f65ad99 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2905,13 +2905,6 @@  static int do_setlink(const struct sk_buff *skb,
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	}
 
-	if (tb[IFLA_MASTER]) {
-		err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]), extack);
-		if (err)
-			goto errout;
-		status |= DO_SETLINK_MODIFIED;
-	}
-
 	if (ifm->ifi_flags || ifm->ifi_change) {
 		err = dev_change_flags(dev, rtnl_dev_combine_flags(dev, ifm),
 				       extack);
@@ -2919,6 +2912,13 @@  static int do_setlink(const struct sk_buff *skb,
 			goto errout;
 	}
 
+	if (tb[IFLA_MASTER]) {
+		err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]), extack);
+		if (err)
+			goto errout;
+		status |= DO_SETLINK_MODIFIED;
+	}
+
 	if (tb[IFLA_CARRIER]) {
 		err = dev_change_carrier(dev, nla_get_u8(tb[IFLA_CARRIER]));
 		if (err)