Skip to contentSkip to conversation

rtnetlink: implement setting of master device

PATCH Applied Jiri Pirko 4 participants 14 messages Feb 13, 2011 – Feb 16, 2011

Subsystems (4)
Networking Network drivers Ethernet bridge Bonding driver
lore
Share & tools
lore

Jiri Pirko

author
#0 lore raw

PATCH 1/4 rtnetlink: implement setting of master device

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Signed-off-by Jiri Pirko <[email protected]>
---
 include/linux/netdevice.h |   12 ++++++++++++
 net/core/rtnetlink.c      |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)
include/linux/netdevice.h +12 -0
index 5a42b10..d08ef65 100644
@@ -783,6 +783,14 @@ struct netdev_tc_txq {
  *	Set hardware filter for RFS.  rxq_index is the target queue index;
  *	flow_id is a flow ID to be passed to rps_may_expire_flow() later.
  *	Return the filter ID on success, or a negative error code.
+ *
+ *	Slave management functions (for bridge, bonding, etc). User should
+ *	call netdev_set_master() to set dev->master properly.
+ * int (*ndo_add_slave)(struct net_device *dev, struct net_device *slave_dev);
+ *	Called to make another netdev an underling.
+ *
+ * int (*ndo_del_slave)(struct net_device *dev, struct net_device *slave_dev);
+ *	Called to release previously enslaved netdev.
  */
 #define HAVE_NET_DEVICE_OPS
 struct net_device_ops {

@@ -862,6 +870,10 @@ struct net_device_ops {
 						     u16 rxq_index,
 						     u32 flow_id);
 #endif
+	int			(*ndo_add_slave)(struct net_device *dev,
+						 struct net_device *slave_dev);
+	int			(*ndo_del_slave)(struct net_device *dev,
+						 struct net_device *slave_dev);
 };
 
 /*
net/core/rtnetlink.c +42 -0

60 lines not on this page · raw message

Signature · 1 line
1.7.3.4
author
#1 lore raw

PATCH 2/4 iplink: implement setting of master device

Signed-off-by Jiri Pirko <[email protected]>
---
 ip/iplink.c   |   16 ++++++++++++++++
 man/man8/ip.8 |   15 ++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletions(-)
ip/iplink.c +16 -0
index cb2c4f5..7a1722c 100644
@@ -71,6 +71,8 @@ void iplink_usage(void)
 	fprintf(stderr, "	                  [ vf NUM [ mac LLADDR ]\n");
 	fprintf(stderr, "				   [ vlan VLANID [ qos VLAN-QOS ] ]\n");
 	fprintf(stderr, "				   [ rate TXRATE ] ] \n");
+	fprintf(stderr, "			  [ master DEVICE ]\n");
+	fprintf(stderr, "			  [ nomaster ]\n");
 	fprintf(stderr, "       ip link show [ DEVICE ]\n");
 
 	if (iplink_have_newlink()) {

@@ -361,6 +363,20 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			if (len < 0)
 				return -1;
 			addattr_nest_end(&req->n, vflist);
+#ifdef IFLA_MASTER
+		} else if (matches(*argv, "master") == 0) {
+			int ifindex;
+			NEXT_ARG();
+			ifindex = ll_name_to_index(*argv);
+			if (!ifindex)
+				invarg("Device does not exist\n", *argv);
+			addattr_l(&req->n, sizeof(*req), IFLA_MASTER,
+				  &ifindex, 4);
+		} else if (matches(*argv, "nomaster") == 0) {
+			int ifindex = 0;
+			addattr_l(&req->n, sizeof(*req), IFLA_MASTER,
+				  &ifindex, 4);
+#endif
 #ifdef IFF_DYNAMIC
 		} else if (matches(*argv, "dynamic") == 0) {
 			NEXT_ARG();
man/man8/ip.8 +14 -1
index 8d55fa9..a473868 100644
@@ -95,7 +95,12 @@ ip \- show / manipulate routing, devices, policy routing and tunnels
 .B qos
 .IR VLAN-QOS " ] ] ["
 .B rate
-.IR TXRATE " ]"
+.IR TXRATE " ] |"
+.br
+.B master
+.IR DEVICE
+.br
+.B nomaster
 
 .ti -8
 .B ip link show

@@ -1038,6 +1043,14 @@ Setting this parameter to 0 disables rate limiting. The
 parameter must be specified.
 .in -8
 
+.TP
+.BI master " DEVICE"
+set master device of the device (enslave device).
+
+.TP
+.BI nomaster
+unset master device of the device (release device).
+
 .PP
 .B Warning:
 If multiple parameter changes are requested,
Signature · 1 line
1.7.3.4
author
#2 lore raw

PATCH 3/4 bond: implement [add/del]_slave ops

In reply to Jiri Pirko

allow enslaving/releasing using netlink interface

Signed-off-by Jiri Pirko <[email protected]>
---
 drivers/net/bonding/bond_main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/net/bonding/bond_main.c +2 -0
index 9f87787..77e3c6a 100644
@@ -4657,6 +4657,8 @@ static const struct net_device_ops bond_netdev_ops = {
 	.ndo_netpoll_cleanup	= bond_netpoll_cleanup,
 	.ndo_poll_controller	= bond_poll_controller,
 #endif
+	.ndo_add_slave		= bond_enslave,
+	.ndo_del_slave		= bond_release,
 };
 
 static void bond_destructor(struct net_device *bond_dev)
Signature · 1 line
1.7.3.4
author
#4 lore raw

PATCH 4/4 bridge: implement [add/del]_slave ops

In reply to Jiri Pirko

add possibility to addif/delif via rtnetlink

Signed-off-by Jiri Pirko <[email protected]>
---
 net/bridge/br_device.c |   17 +++++++++++++++++
 net/bridge/br_if.c     |   11 ++++++++++-
 2 files changed, 27 insertions(+), 1 deletions(-)
net/bridge/br_device.c +17 -0
index 5564435..1461b19 100644
@@ -297,6 +297,21 @@ void br_netpoll_disable(struct net_bridge_port *p)
 
 #endif
 
+static int br_add_slave(struct net_device *dev, struct net_device *slave_dev)
+
+{
+	struct net_bridge *br = netdev_priv(dev);
+
+	return br_add_if(br, slave_dev);
+}
+
+static int br_del_slave(struct net_device *dev, struct net_device *slave_dev)
+{
+	struct net_bridge *br = netdev_priv(dev);
+
+	return br_del_if(br, slave_dev);
+}
+
 static const struct ethtool_ops br_ethtool_ops = {
 	.get_drvinfo    = br_getinfo,
 	.get_link	= ethtool_op_get_link,

@@ -326,6 +341,8 @@ static const struct net_device_ops br_netdev_ops = {
 	.ndo_netpoll_cleanup	 = br_netpoll_cleanup,
 	.ndo_poll_controller	 = br_poll_controller,
 #endif
+	.ndo_add_slave		 = br_add_slave,
+	.ndo_del_slave		 = br_del_slave,
 };
 
 static void br_dev_free(struct net_device *dev)
net/bridge/br_if.c +10 -1
index 2a6801d..dce8f00 100644
@@ -148,6 +148,8 @@ static void del_nbp(struct net_bridge_port *p)
 
 	netdev_rx_handler_unregister(dev);
 
+	netdev_set_master(dev, NULL);
+
 	br_multicast_del_port(p);
 
 	kobject_uevent(&p->kobj, KOBJ_REMOVE);

@@ -429,10 +431,14 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 	if (br_netpoll_info(br) && ((err = br_netpoll_enable(p))))
 		goto err3;
 
-	err = netdev_rx_handler_register(dev, br_handle_frame, p);
+	err = netdev_set_master(dev, br->dev);
 	if (err)
 		goto err3;
 
+	err = netdev_rx_handler_register(dev, br_handle_frame, p);
+	if (err)
+		goto err4;
+
 	dev->priv_flags |= IFF_BRIDGE_PORT;
 
 	dev_disable_lro(dev);

@@ -455,6 +461,9 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 	kobject_uevent(&p->kobj, KOBJ_ADD);
 
 	return 0;
+
+err4:
+	netdev_set_master(dev, NULL);
 err3:
 	sysfs_remove_link(br->ifobj, p->dev->name);
 err2:
Signature · 1 line
1.7.3.4
#6 lore raw

Re: [patch net-next-2.6 4/4] bridge: implement [add/del]_slave ops

Quoted 8 lines · On Sun, 13 Feb 2011 16:59:03 -0800 (PST) David Miller <[email protected]>…

From: Jiri Pirko <[email protected]>
Date: Sun, 13 Feb 2011 20:33:42 +0100

Quoted 3 lines

add possibility to addif/delif via rtnetlink

Signed-off-by Jiri Pirko <[email protected]>

Applied.

You should follow established protocol and wait until I have had time to review code that impacts areas which I maintain. The linux-foundation email address is not listed in MAINTAINERS file and is mainly a spam catcher that I never read.

Maybe I should just start sending networking patches to Linus.

#7 lore raw
In reply to Jiri Pirko

Am 13.02.2011 20:31, schrieb Jiri Pirko:

Quoted 3 lines

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Looks good to me, just one question:

net/core/rtnetlink.c static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, · line 1344

Quoted 9 lines
net/core/rtnetlink.c +5 -0
@@ -1301,6 +1337,12 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 			goto errout;
 	}
 
+	if (tb[IFLA_MASTER]) {
+		err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]));
+		if (err)
+			goto errout;
+	}

Any reason why you're not setting "modified" here?

Quoted 4 lines
net/core/rtnetlink.c +1 -0
+
 	if (tb[IFLA_TXQLEN])
 		dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
author
#8 lore raw
Quoted 11 lines · Sun, Feb 13, 2011 at 08:43:35PM CET, [email protected] wrote:

Am 13.02.2011 20:31, schrieb Jiri Pirko:

Quoted 3 lines

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Looks good to me, just one question:

Quoted 4 lines
(quoted diff) +0 -0
@@ -1301,6 +1337,12 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 			goto errout;
 	}
Quoted 7 lines

Review static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, · line 1344

Quoted 5 lines
(quoted diff) +5 -0
+	if (tb[IFLA_MASTER]) {
+		err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]));
+		if (err)
+			goto errout;
+	}

Any reason why you're not setting "modified" here?

Ugh, forgot that, thanks. Here's repost:

Subject: [patch net-next-2.6 1/4] rtnetlink: implement setting of master device

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Signed-off-by Jiri Pirko <[email protected]>
---
 include/linux/netdevice.h |   12 ++++++++++++
 net/core/rtnetlink.c      |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)
include/linux/netdevice.h +12 -0
index 5a42b10..d08ef65 100644
@@ -783,6 +783,14 @@ struct netdev_tc_txq {
  *	Set hardware filter for RFS.  rxq_index is the target queue index;
  *	flow_id is a flow ID to be passed to rps_may_expire_flow() later.
  *	Return the filter ID on success, or a negative error code.
+ *
+ *	Slave management functions (for bridge, bonding, etc). User should
+ *	call netdev_set_master() to set dev->master properly.
+ * int (*ndo_add_slave)(struct net_device *dev, struct net_device *slave_dev);
+ *	Called to make another netdev an underling.
+ *
+ * int (*ndo_del_slave)(struct net_device *dev, struct net_device *slave_dev);
+ *	Called to release previously enslaved netdev.
  */
 #define HAVE_NET_DEVICE_OPS
 struct net_device_ops {

@@ -862,6 +870,10 @@ struct net_device_ops {
 						     u16 rxq_index,
 						     u32 flow_id);
 #endif
+	int			(*ndo_add_slave)(struct net_device *dev,
+						 struct net_device *slave_dev);
+	int			(*ndo_del_slave)(struct net_device *dev,
+						 struct net_device *slave_dev);
 };
 
 /*
net/core/rtnetlink.c +43 -0

61 lines not on this page · raw message

Signature · 1 line
1.7.3.4
maintainer Applied
#10 lore raw

From: Patrick McHardy <[email protected]>
Date: Sun, 13 Feb 2011 21:16:47 +0100

Quoted 10 lines

Am 13.02.2011 21:15, schrieb Jiri Pirko:

Quoted 7 lines

Subject: [patch net-next-2.6 1/4] rtnetlink: implement setting of master device

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Signed-off-by Jiri Pirko <[email protected]>
Acked-by Patrick McHardy <[email protected]>

Applied.

#11 lore raw
In reply to Jiri Pirko
Quoted 5 lines · On Sun, 13 Feb 2011 20:31:06 +0100 Jiri Pirko <[email protected]> wrote:

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Signed-off-by Jiri Pirko <[email protected]>

But, setting master means something different for each type of device? What happens if you move eth0 from br0 to bond0?

The name "master" is only used in the bonding spec. It is not used in description of bridges in the 802.1 spec. There are also some companies that have very "politically correct" HR departments that think that any reference to master or slave is racist.

author
#12 lore raw
Quoted 10 lines · Wed, Feb 16, 2011 at 02:18:44PM CET, [email protected] wrote:
Quoted 5 lines · On Sun, 13 Feb 2011 20:31:06 +0100 Jiri Pirko <[email protected]> wrote:

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Signed-off-by Jiri Pirko <[email protected]>

But, setting master means something different for each type of device?

Why isn't correct to use master also for bridge? It had no meaning there.

Quoted 1 line

What happens if you move eth0 from br0 to bond0?

you mean by:
ip link set eth0 master br0
ip link set eth0 master bond0

It's first removed from bridge, then added into bond. No problem here.

Quoted 7 lines

The name "master" is only used in the bonding spec. It is not used in description of bridges in the 802.1 spec. There are also some companies that have very "politically correct" HR departments that think that any reference to master or slave is racist.

#13 lore raw
Quoted 8 lines · Jiri Pirko
Quoted 7 lines · Wed, Feb 16, 2011 at 02:18:44PM CET, [email protected] wrote:
Quoted 4 lines · On Sun, 13 Feb 2011 20:31:06 +0100 Jiri Pirko <[email protected]> wrote:

This patch allows userspace to enslave/release slave devices via netlink interface using IFLA_MASTER. This introduces generic way to add/remove underling devices.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Jiri Pirko <[email protected]>

But, setting master means something different for each type of device?

Why isn't correct to use master also for bridge? It had no meaning there.

In fact the bridge netlink family uses IFLA_MASTER for exactly the same purpose.