[PATCH v2 0/2] net: add device groups

[PATCH v2 0/2] net: add device groups

[Vlad Dogaru]

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

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

Changes since version 1:
 * we avoid adding a new attribute type by using the following
   convention: if no device name is specified, the interface index is
   negative, and there is a group specified, we change parameters for
   the whole group.
 * the dummy module is no longer modified to include an initial group
   for the devices it creates. The user is responsible for moving them
   to a different group by means of the provided netlink interface.

Vlad Dogaru (2):
  net_device: add support for network device groups
  netlink: support setting devgroup parameters

 include/linux/if_link.h   |    1 +
 include/linux/netdevice.h |    7 +++++++
 net/core/dev.c            |   12 ++++++++++++
 net/core/rtnetlink.c      |   38 ++++++++++++++++++++++++++++++++++----
 4 files changed, 54 insertions(+), 4 deletions(-)

[PATCH v2 1/2] net_device: add support for network device groups

[Vlad Dogaru]

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 <ddvlad@rosedu.org>
---
 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

Re: [PATCH v2 1/2] net_device: add support for network device groups

[jamal]

On Tue, 2011-01-11 at 18:35 +0200, Vlad Dogaru wrote:
> Net devices can now be grouped, enabling simpler manipulation from
> userspace. This patch adds a group field to the net_device strucure, as
                                                             ^^^^^
typo

> well as rtnetlink support to query and modify it.
> 


> +	unsigned int group;

Should that be int?

cheers,
jamal

[PATCH v2 2/2] netlink: support setting devgroup parameters

[Vlad Dogaru]

If a rtnetlink request specifies a negative or zero ifindex and has no
interface name attribute, but has a group attribute, then the chenges
are made to all the interfaces belonging to the specified group.

Signed-off-by: Vlad Dogaru <ddvlad@rosedu.org>
---
 net/core/rtnetlink.c |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 012b0f0..f208397 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);
@@ -1585,10 +1603,16 @@ replay:
 	ifm = nlmsg_data(nlh);
 	if (ifm->ifi_index > 0)
 		dev = __dev_get_by_index(net, ifm->ifi_index);
-	else if (ifname[0])
-		dev = __dev_get_by_name(net, ifname);
-	else
-		dev = NULL;
+	else {
+		if (ifname[0])
+			dev = __dev_get_by_name(net, ifname);
+		else if (tb[IFLA_GROUP])
+			return rtnl_group_changelink(net,
+					nla_get_u32(tb[IFLA_GROUP]),
+					ifm, tb);
+		else
+			dev = NULL;
+	}
 
 	err = validate_linkmsg(dev, tb);
 	if (err < 0)
-- 
1.7.1

Re: [PATCH v2 2/2] netlink: support setting devgroup parameters

[jamal]

On Tue, 2011-01-11 at 18:35 +0200, Vlad Dogaru wrote:
> If a rtnetlink request specifies a negative or zero ifindex and has no
> interface name attribute, but has a group attribute, then the chenges
> are made to all the interfaces belonging to the specified group.
> 
> Signed-off-by: Vlad Dogaru <ddvlad@rosedu.org>

Looks good. Please just do some basic tests like setting a few
interfaces to the same group, changing their MTU and setting admin
up then down. If it works add my ACKed-by on both patches.
The only thing that will still generate a lot of noise is the netlink
events that will be generated afterwards for each change on a netdev in
a group i.e a single netlink message..

Maybe we could batch those in a future patch

cheers,
jamal