[PATCH 0/3] net: add device groups

This patchset implements network device grouping and simple manipulation
of groups. Netlink has been update to provide group information and
means of applying changes to members of a specific group via a single
message.

In addition, for testing purposes, we add a parameter to the  dummy
module. If supplied, all devices that are created are assigned to the
specified group.

I will follow up with a patchset which updates iproute2 to use the new
parameters.


Vlad Dogaru (3):
  net_device: add support for network device groups
  net/dummy: add device group parameter
  netlink: support setting devgroup parameters

 drivers/net/dummy.c       |    5 +++++
 include/linux/if_link.h   |    2 ++
 include/linux/netdevice.h |    7 +++++++
 net/core/dev.c            |   12 ++++++++++++
 net/core/rtnetlink.c      |   28 ++++++++++++++++++++++++++++
 5 files changed, 54 insertions(+), 0 deletions(-)

Net devices can now be grouped, enabling simpler manipulation from
userspace. This patch adds a group field to the net_device strucure, as
well as rtnetlink support to query and modify it.

Signed-off-by: Vlad Dogaru <redacted>
---
 include/linux/if_link.h   |    1 +
 include/linux/netdevice.h |    7 +++++++
 net/core/dev.c            |   12 ++++++++++++
 net/core/rtnetlink.c      |    6 ++++++
 4 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 6485d2a..f4a2e6b 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h

@@ -135,6 +135,7 @@ enum {
 	IFLA_VF_PORTS,
 	IFLA_PORT_SELF,
 	IFLA_AF_SPEC,
+	IFLA_GROUP,		/* Group the device belongs to */
 	__IFLA_MAX
 };
 

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0f6b1c9..5f624ad 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -75,6 +75,9 @@ struct wireless_dev;
 #define NET_RX_SUCCESS		0	/* keep 'em coming, baby */
 #define NET_RX_DROP		1	/* packet dropped */
 
+/* Initial net device group. All devices belong to group 0 by default. */
+#define INIT_NETDEV_GROUP	0
+
 /*
  * Transmit return codes: transmit return codes originate from three different
  * namespaces:

@@ -1156,6 +1159,9 @@ struct net_device {
 
 	/* phy device may attach itself for hardware timestamping */
 	struct phy_device *phydev;
+
+	/* group the device belongs to */
+	unsigned int group;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 

@@ -1847,6 +1853,7 @@ extern int		dev_set_alias(struct net_device *, const char *, size_t);
 extern int		dev_change_net_namespace(struct net_device *,
 						 struct net *, const char *);
 extern int		dev_set_mtu(struct net_device *, int);
+extern void		dev_set_group(struct net_device *, int);
 extern int		dev_set_mac_address(struct net_device *,
 					    struct sockaddr *);
 extern int		dev_hard_start_xmit(struct sk_buff *skb,

diff --git a/net/core/dev.c b/net/core/dev.c
index a215269..6cf0cd2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -4573,6 +4573,17 @@ int dev_set_mtu(struct net_device *dev, int new_mtu)
 EXPORT_SYMBOL(dev_set_mtu);
 
 /**
+ *	dev_set_group - Change group this device belongs to
+ *	@dev: device
+ *	@new_group: group this device should belong to
+ */
+void dev_set_group(struct net_device *dev, int new_group)
+{
+	dev->group = new_group;
+}
+EXPORT_SYMBOL(dev_set_group);
+
+/**
  *	dev_set_mac_address - Change Media Access Control Address
  *	@dev: device
  *	@sa: new address

@@ -5698,6 +5709,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	dev->priv_flags = IFF_XMIT_DST_RELEASE;
 	setup(dev);
 	strcpy(dev->name, name);
+	dev->group = INIT_NETDEV_GROUP;
 	return dev;
 
 free_pcpu:

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 750db57..012b0f0 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c

@@ -868,6 +868,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 		   netif_running(dev) ? dev->operstate : IF_OPER_DOWN);
 	NLA_PUT_U8(skb, IFLA_LINKMODE, dev->link_mode);
 	NLA_PUT_U32(skb, IFLA_MTU, dev->mtu);
+	NLA_PUT_U32(skb, IFLA_GROUP, dev->group);
 
 	if (dev->ifindex != dev->iflink)
 		NLA_PUT_U32(skb, IFLA_LINK, dev->iflink);

@@ -1265,6 +1266,11 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 		modified = 1;
 	}
 
+	if (tb[IFLA_GROUP]) {
+		dev_set_group(dev, nla_get_u32(tb[IFLA_GROUP]));
+		modified = 1;
+	}
+
 	/*
 	 * Interface selected by interface index but interface
 	 * name provided implies that a name change has been

-- 
1.7.1

When inserting the dummy module, the user can specify the group of the
dummy devices. This avoids manually moving the newly created pseudo
devices to a different group.

Signed-off-by: Vlad Dogaru <redacted>
---
 drivers/net/dummy.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index ff2d29b..a974c05 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c

@@ -39,6 +39,7 @@
 #include <linux/u64_stats_sync.h>
 
 static int numdummies = 1;
+static int ingroup;
 
 static int dummy_set_address(struct net_device *dev, void *p)
 {

@@ -158,6 +159,9 @@ static struct rtnl_link_ops dummy_link_ops __read_mostly = {
 /* Number of dummy devices to be set up by this module. */
 module_param(numdummies, int, 0);
 MODULE_PARM_DESC(numdummies, "Number of dummy pseudo devices");
+/* Group to add the dummy devices to. */
+module_param(ingroup, int, 0);
+MODULE_PARM_DESC(ingroup, "Group the dummy devices belong to");
 
 static int __init dummy_init_one(void)
 {

@@ -167,6 +171,7 @@ static int __init dummy_init_one(void)
 	dev_dummy = alloc_netdev(0, "dummy%d", dummy_setup);
 	if (!dev_dummy)
 		return -ENOMEM;
+	dev_set_group(dev_dummy, ingroup);
 
 	err = dev_alloc_name(dev_dummy, dev_dummy->name);
 	if (err < 0)

-- 
1.7.1

Add a new type of message, IFLA_FILTERGROUP, which, if present in a
userspace request, specifies that parameters should be changed for all
devices in the group.

Signed-off-by: Vlad Dogaru <redacted>
---
 include/linux/if_link.h |    1 +
 net/core/rtnetlink.c    |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index f4a2e6b..1bbacf9 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h

@@ -136,6 +136,7 @@ enum {
 	IFLA_PORT_SELF,
 	IFLA_AF_SPEC,
 	IFLA_GROUP,		/* Group the device belongs to */
+	IFLA_FILTERGROUP,	/* Set parameters for a specific device group */
 	__IFLA_MAX
 };
 

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 012b0f0..8d7af8c 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c

@@ -1558,6 +1558,24 @@ err:
 }
 EXPORT_SYMBOL(rtnl_create_link);
 
+static int rtnl_group_changelink(struct net *net, int group,
+		struct ifinfomsg *ifm,
+		struct nlattr **tb)
+{
+	struct net_device *dev;
+	int err;
+
+	for_each_netdev(net, dev) {
+		if (dev->group == group) {
+			err = do_setlink(dev, ifm, tb, NULL, 0);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return 0;
+}
+
 static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 {
 	struct net *net = sock_net(skb->sk);

@@ -1587,6 +1605,10 @@ replay:
 		dev = __dev_get_by_index(net, ifm->ifi_index);
 	else if (ifname[0])
 		dev = __dev_get_by_name(net, ifname);
+	else if (tb[IFLA_FILTERGROUP])
+		return rtnl_group_changelink(net,
+				nla_get_u32(tb[IFLA_FILTERGROUP]),
+				ifm, tb);
 	else
 		dev = NULL;
 

-- 
1.7.1

Nice - short and sweet.

IMO: I dont think you need this new attribute, IFLA_GROUP
should suffice...
You can use the trick that if a <=0 ifindex is specified,
and no name is passed but a GROUP is passed, then we
operate on the group i.e something like:

        if (ifm->ifi_index > 0)
                dev = __dev_get_by_index(net, ifm->ifi_index);
        else {
                if (tb[IFLA_IFNAME])
                     dev = __dev_get_by_name(net, ifname);
                else if (tb[IFLA_GROUP])
                     new/get/set/del specific stuff..
                else
                   return -EINVAL;
       }


cheers,
jamal

On Mon, 2011-01-10 at 13:38 +0200, Vlad Dogaru wrote:

Add a new type of message, IFLA_FILTERGROUP, which, if present in a
userspace request, specifies that parameters should be changed for all
devices in the group.

On Mon, 2011-01-10 at 13:38 +0200, Vlad Dogaru wrote:

This patchset implements network device grouping and simple manipulation
of groups. Netlink has been update to provide group information and
means of applying changes to members of a specific group via a single
message.

Can you explain the purpose of this? I'm wondering if it would make
sense to automatically group all virtual interfaces belonging to a
single 802.11 device, for instance.

johannes

On Mon, 2011-01-10 at 14:41 +0100, Johannes Berg wrote:

Can you explain the purpose of this? I'm wondering if it would make
sense to automatically group all virtual interfaces belonging to a
single 802.11 device, for instance.

It depends what you want to do with that grouping.
In a nutshell, this greatly reduces the amount of kernel-user netlink
traffic in presence of multi interfaces.
you can do things like:
 
ip link set dev ppp0 group 1
...
...
ip link set dev pppN group 1

ip link ls group 1
ip link set down group 1
ip link set mtu 512 group 1
etc

Although now that i am thinking of it,
I am not sure whether it would be a legit thing to change
the MAC address of a group of devices - we may need to put
some restrictions.

Note: this grouping thing can also be potentially used in
packet policies etc (but i dont want to distract the simple
requirement we have at the moment).

cheers,
jamal 

On Mon, 2011-01-10 at 09:00 -0500, jamal wrote:

On Mon, 2011-01-10 at 14:41 +0100, Johannes Berg wrote:

quoted

Can you explain the purpose of this? I'm wondering if it would make
sense to automatically group all virtual interfaces belonging to a
single 802.11 device, for instance.

It depends what you want to do with that grouping.
In a nutshell, this greatly reduces the amount of kernel-user netlink
traffic in presence of multi interfaces.
you can do things like:
 
ip link set dev ppp0 group 1
...
...
ip link set dev pppN group 1

ip link ls group 1
ip link set down group 1
ip link set mtu 512 group 1
etc

Right, but where would you want to use it -- what's the use case right
now? Your ppp example here makes me think the use case is some PPP
endpoint server that has lots of these interfaces. It also means the
grouping is entirely user-space controlled. In this case, the idea of
automatically grouping based on the device structure seems pointless.

johannes

On Mon, 2011-01-10 at 15:08 +0100, Johannes Berg wrote:

Right, but where would you want to use it -- what's the use case right
now? Your ppp example here makes me think the use case is some PPP
endpoint server that has lots of these interfaces. 

That was just an example because someone mentioned PPP earlier.
Originally, this came from someone (Octavian?) trying to setup
a gazillion netdevs for testing purposes and then needing to
do "ip link ls dummy*" which causes a gazillion messages between
kernel and user space and all the filtering of what dummy* being
done in user space.

It also means the
grouping is entirely user-space controlled. In this case, the idea of
automatically grouping based on the device structure seems pointless.

You still havent said what you want this grouping for. What is your
use case? ;->

cheers,
jamal

On Mon, 2011-01-10 at 09:24 -0500, jamal wrote:

quoted

It also means the
grouping is entirely user-space controlled. In this case, the idea of
automatically grouping based on the device structure seems pointless.

You still havent said what you want this grouping for. What is your
use case? ;->

I don't have a use case. I was just wondering if there's any benefit in
grouping virtual devices that belong to a single physical device, but I
suppose there isn't.

johannes

From: Vlad Dogaru <redacted>
Date: Mon, 10 Jan 2011 13:38:43 +0200

When inserting the dummy module, the user can specify the group of the
dummy devices. This avoids manually moving the newly created pseudo
devices to a different group.

Signed-off-by: Vlad Dogaru <redacted>

The amount of hacks people wish to add to the dummy driver is one step
away from amazing.

Just use the standard APIs to add dummy devices to the group you want,
instead of these ad-hoc device specific module parameters.

Thanks.

On Mon, Jan 10, 2011 at 08:14:33AM -0500, jamal wrote:

Nice - short and sweet.

IMO: I dont think you need this new attribute, IFLA_GROUP
should suffice...
You can use the trick that if a <=0 ifindex is specified,
and no name is passed but a GROUP is passed, then we
operate on the group i.e something like:

        if (ifm->ifi_index > 0)
                dev = __dev_get_by_index(net, ifm->ifi_index);
        else {
                if (tb[IFLA_IFNAME])
                     dev = __dev_get_by_name(net, ifname);
                else if (tb[IFLA_GROUP])
                     new/get/set/del specific stuff..
                else
                   return -EINVAL;
       }

That sounds like a good idea, but, by using the same attribute,
userspace won't be able to change the group of all the interfaces in a
group, i.e. no more
	ip l set group 1 devgroup 0

However, I'm not sure that the use case above would be very common.

Vlad

On Mon, Jan 10, 2011 at 01:37:48PM -0800, David Miller wrote:

From: Vlad Dogaru <redacted>
Date: Mon, 10 Jan 2011 13:38:43 +0200

quoted

When inserting the dummy module, the user can specify the group of the
dummy devices. This avoids manually moving the newly created pseudo
devices to a different group.

Signed-off-by: Vlad Dogaru <redacted>

The amount of hacks people wish to add to the dummy driver is one step
away from amazing.

Just use the standard APIs to add dummy devices to the group you want,
instead of these ad-hoc device specific module parameters.

Sorry for attempting to bloat the dummy module. The intention was to be
able to add many dummy interfaces directly into a group without much
user-kernel communication.

However, I think that can be done by moving the existing (hopefully few)
interfaces to another group prior to adding the dummies to group 0 and
using that to manipulate them.

Vlad

On Tue, 2011-01-11 at 14:13 +0200, Vlad Dogaru wrote:

That sounds like a good idea, but, by using the same attribute,
userspace won't be able to change the group of all the interfaces in a
group, i.e. no more
	ip l set group 1 devgroup 0

However, I'm not sure that the use case above would be very common.

I think for the simple case we have been discussing this goes beyond
that. So maybe to KISS defer this to a later time.

cheers,
jamal