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

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

[Jiri Pirko]

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 <jpirko@redhat.com>
---
 include/linux/netdevice.h |   12 ++++++++++++
 net/core/rtnetlink.c      |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5a42b10..d08ef65 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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);
 };
 
 /*
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index da0fe45..20d067a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1036,6 +1036,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_MAP]		= { .len = sizeof(struct rtnl_link_ifmap) },
 	[IFLA_MTU]		= { .type = NLA_U32 },
 	[IFLA_LINK]		= { .type = NLA_U32 },
+	[IFLA_MASTER]		= { .type = NLA_U32 },
 	[IFLA_TXQLEN]		= { .type = NLA_U32 },
 	[IFLA_WEIGHT]		= { .type = NLA_U32 },
 	[IFLA_OPERSTATE]	= { .type = NLA_U8 },
@@ -1178,6 +1179,41 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 	return err;
 }
 
+static int do_set_master(struct net_device *dev, int ifindex)
+{
+	struct net_device *master_dev;
+	const struct net_device_ops *ops;
+	int err;
+
+	if (dev->master) {
+		if (dev->master->ifindex == ifindex)
+			return 0;
+		ops = dev->master->netdev_ops;
+		if (ops->ndo_del_slave) {
+			err = ops->ndo_del_slave(dev->master, dev);
+			if (err)
+				return err;
+		} else {
+			return -EOPNOTSUPP;
+		}
+	}
+
+	if (ifindex) {
+		master_dev = __dev_get_by_index(dev_net(dev), ifindex);
+		if (!master_dev)
+			return -EINVAL;
+		ops = master_dev->netdev_ops;
+		if (ops->ndo_add_slave) {
+			err = ops->ndo_add_slave(master_dev, dev);
+			if (err)
+				return err;
+		} else {
+			return -EOPNOTSUPP;
+		}
+	}
+	return 0;
+}
+
 static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 		      struct nlattr **tb, char *ifname, int modified)
 {
@@ -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;
+	}
+
 	if (tb[IFLA_TXQLEN])
 		dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
 
-- 
1.7.3.4

[patch iproute2 2/4] iplink: implement setting of master device

[Jiri Pirko]

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 ip/iplink.c   |   16 ++++++++++++++++
 man/man8/ip.8 |   15 ++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/ip/iplink.c b/ip/iplink.c
index cb2c4f5..7a1722c 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -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();
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 8d55fa9..a473868 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -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,
-- 
1.7.3.4

[patch net-next-2.6 3/4] bond: implement [add/del]_slave ops

[Jiri Pirko]

allow enslaving/releasing using netlink interface

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/bonding/bond_main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9f87787..77e3c6a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -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)
-- 
1.7.3.4

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

[Jiri Pirko]

add possibility to addif/delif via rtnetlink

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 net/bridge/br_device.c |   17 +++++++++++++++++
 net/bridge/br_if.c     |   11 ++++++++++-
 2 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 5564435..1461b19 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -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)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 2a6801d..dce8f00 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -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:
-- 
1.7.3.4

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

[Patrick McHardy]

Am 13.02.2011 20:31, schrieb Jiri Pirko:
> 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:

> @@ -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?

> +
>  	if (tb[IFLA_TXQLEN])
>  		dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
>  

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

[Jiri Pirko]

Sun, Feb 13, 2011 at 08:43:35PM CET, kaber@trash.net wrote:
>Am 13.02.2011 20:31, schrieb Jiri Pirko:
>> 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:
>
>> @@ -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?

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 <jpirko@redhat.com>
---
 include/linux/netdevice.h |   12 ++++++++++++
 net/core/rtnetlink.c      |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5a42b10..d08ef65 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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);
 };
 
 /*
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index da0fe45..49f7ea5 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1036,6 +1036,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_MAP]		= { .len = sizeof(struct rtnl_link_ifmap) },
 	[IFLA_MTU]		= { .type = NLA_U32 },
 	[IFLA_LINK]		= { .type = NLA_U32 },
+	[IFLA_MASTER]		= { .type = NLA_U32 },
 	[IFLA_TXQLEN]		= { .type = NLA_U32 },
 	[IFLA_WEIGHT]		= { .type = NLA_U32 },
 	[IFLA_OPERSTATE]	= { .type = NLA_U8 },
@@ -1178,6 +1179,41 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 	return err;
 }
 
+static int do_set_master(struct net_device *dev, int ifindex)
+{
+	struct net_device *master_dev;
+	const struct net_device_ops *ops;
+	int err;
+
+	if (dev->master) {
+		if (dev->master->ifindex == ifindex)
+			return 0;
+		ops = dev->master->netdev_ops;
+		if (ops->ndo_del_slave) {
+			err = ops->ndo_del_slave(dev->master, dev);
+			if (err)
+				return err;
+		} else {
+			return -EOPNOTSUPP;
+		}
+	}
+
+	if (ifindex) {
+		master_dev = __dev_get_by_index(dev_net(dev), ifindex);
+		if (!master_dev)
+			return -EINVAL;
+		ops = master_dev->netdev_ops;
+		if (ops->ndo_add_slave) {
+			err = ops->ndo_add_slave(master_dev, dev);
+			if (err)
+				return err;
+		} else {
+			return -EOPNOTSUPP;
+		}
+	}
+	return 0;
+}
+
 static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 		      struct nlattr **tb, char *ifname, int modified)
 {
@@ -1301,6 +1337,13 @@ 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;
+		modified = 1;
+	}
+
 	if (tb[IFLA_TXQLEN])
 		dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
 
-- 
1.7.3.4

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

[Patrick McHardy]

Am 13.02.2011 21:15, schrieb Jiri Pirko:
> 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 <jpirko@redhat.com>

Acked-by: Patrick McHardy <kaber@trash.net>

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

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Sun, 13 Feb 2011 21:16:47 +0100

> Am 13.02.2011 21:15, schrieb Jiri Pirko:
>> 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 <jpirko@redhat.com>
> 
> Acked-by: Patrick McHardy <kaber@trash.net>

Applied.

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

[David Miller]

From: Jiri Pirko <jpirko@redhat.com>
Date: Sun, 13 Feb 2011 20:33:01 +0100

> allow enslaving/releasing using netlink interface
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Applied.

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

[David Miller]

From: Jiri Pirko <jpirko@redhat.com>
Date: Sun, 13 Feb 2011 20:33:42 +0100

> add possibility to addif/delif via rtnetlink
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Applied.

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

[Stephen Hemminger]

On Sun, 13 Feb 2011 20:31:06 +0100
Jiri Pirko <jpirko@redhat.com> 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 <jpirko@redhat.com>

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.

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

[Stephen Hemminger]

On Sun, 13 Feb 2011 16:59:03 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Jiri Pirko <jpirko@redhat.com>
> Date: Sun, 13 Feb 2011 20:33:42 +0100
> 
> > add possibility to addif/delif via rtnetlink
> > 
> > Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> 
> 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.

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

[Jiri Pirko]

Wed, Feb 16, 2011 at 02:18:44PM CET, shemminger@vyatta.com wrote:
>On Sun, 13 Feb 2011 20:31:06 +0100
>Jiri Pirko <jpirko@redhat.com> 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 <jpirko@redhat.com>
>
>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.

>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.


>
>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.
>
>

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

[Patrick McHardy]

On 16.02.2011 15:39, Jiri Pirko wrote:
> Wed, Feb 16, 2011 at 02:18:44PM CET, shemminger@vyatta.com wrote:
>> On Sun, 13 Feb 2011 20:31:06 +0100
>> Jiri Pirko <jpirko@redhat.com> 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 <jpirko@redhat.com>
>>
>> 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.