[PATCHv8 0/2] Interface group patches, take 8

[PATCHv8 0/2] Interface group patches, take 8

[Laszlo Attila Toth]

Hello,

This is a subset of our ifgroup patches, the folowing two patches belongs to
the core part.

Each net device has an ifgroup value which is zero by default (unset), and can
be set via sysfs (e.g. /sys/class/net/eth0/ifgroup).

Previously I posted a patch for the ip command in iproute2 which can be another
interface to show and set the ifgroup of the device [1].

There is also a netfilter ifgroup match which is accepted by Patrick [2] [3]


References:
 [1] http://marc.info/?l=linux-netdev&m=119798102017216&w=2
 [2] http://marc.info/?l=linux-netdev&m=119635294305131&w=2
 [3] http://marc.info/?l=linux-netdev&m=119635294805150&w=2
--
Laszlo Attila Toth

[PATCHv8 1/2] Interface group: core and netlink part

[Laszlo Attila Toth]

Interface groups let handle different interfaces together.
Modified net device structure and netlink interface.

Signed-off-by: Laszlo Attila Toth <panther@balabit.hu>
---
 include/linux/if_link.h   |    2 ++
 include/linux/netdevice.h |    3 +++
 net/core/rtnetlink.c      |    9 +++++++++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 84c3492..722b25c 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -79,6 +79,8 @@ enum
 	IFLA_LINKINFO,
 #define IFLA_LINKINFO IFLA_LINKINFO
 	IFLA_NET_NS_PID,
+	IFLA_IFGROUP,
+#define IFLA_IFGROUP IFLA_IFGROUP
 	__IFLA_MAX
 };
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a2f0032..e50d439 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -546,6 +546,9 @@ struct net_device
 	/* Hardware header description */
 	const struct header_ops *header_ops;
 
+	/* interface group this interface belongs to */
+	unsigned int		ifgroup;
+
 	/*
 	 * This marks the end of the "visible" part of the structure. All
 	 * fields hereafter are internal to the system, and may change at
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 2bd9c5f..5fbbdc4 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -577,6 +577,7 @@ static inline size_t if_nlmsg_size(const struct net_device *dev)
 	       + nla_total_size(4) /* IFLA_MTU */
 	       + nla_total_size(4) /* IFLA_LINK */
 	       + nla_total_size(4) /* IFLA_MASTER */
+	       + nla_total_size(4) /* IFLA_IFGROUP */
 	       + nla_total_size(1) /* IFLA_OPERSTATE */
 	       + nla_total_size(1) /* IFLA_LINKMODE */
 	       + rtnl_link_get_size(dev); /* IFLA_LINKINFO */
@@ -614,6 +615,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (dev->master)
 		NLA_PUT_U32(skb, IFLA_MASTER, dev->master->ifindex);
 
+	if (dev->ifgroup)
+		NLA_PUT_U32(skb, IFLA_IFGROUP, dev->ifgroup);
+
 	if (dev->qdisc_sleeping)
 		NLA_PUT_STRING(skb, IFLA_QDISC, dev->qdisc_sleeping->ops->id);
 
@@ -863,6 +867,11 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 		write_unlock_bh(&dev_base_lock);
 	}
 
+	if (tb[IFLA_IFGROUP]) {
+		dev->ifgroup = nla_get_u32(tb[IFLA_IFGROUP]);
+	}
+
+
 	err = 0;
 
 errout:
-- 
1.5.2.5

Re: [PATCHv8 1/2] Interface group: core and netlink part

[David Miller]

From: Laszlo Attila Toth <panther@balabit.hu>
Date: Thu,  6 Mar 2008 15:03:58 +0100

> Interface groups let handle different interfaces together.
> Modified net device structure and netlink interface.
> 
> Signed-off-by: Laszlo Attila Toth <panther@balabit.hu>

I can't say whether this makes sense without seeing how
it will actually be used.

And both of your patches here do nothing but set and
read this group value, making it useless.

If that's all it is, userland can record such mappings
in the filesystem or elsewhere such that multiple
applications can work with and maintain the relationships.
There is zero reason to add this bloat to the kernel in
such a case.

Re: [PATCHv8 1/2] Interface group: core and netlink part

[Laszlo Attila Toth]

David Miller írta:
> From: Laszlo Attila Toth <panther@balabit.hu>
> Date: Thu,  6 Mar 2008 15:03:58 +0100
> 
>> Interface groups let handle different interfaces together.
>> Modified net device structure and netlink interface.
>>
>> Signed-off-by: Laszlo Attila Toth <panther@balabit.hu>
> 
> I can't say whether this makes sense without seeing how
> it will actually be used.

The userspace has two parts, one is in iproute2:

  ip link set dev eth0 group 4

  ip link show dev eth0
3: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast 
group 0x04 qlen 1000
     link/ether 00:0c:29:97:52:99 brd ff:ff:ff:ff:ff:ff

This is still nothing special.

Where can it be used? The netfilter part is an ifgroup match:

  ifgroup -A INPUT -m ifgroup --ifgroup-in 4/0xf ...
  ifgroup -A OUTPUT -m ifgroup --ifgroup-out 4/0xf ...

Also multiple devices can be grouped and handled with one rule. Although 
  the interfaces can be named as the user wants, it is not always enough 
(I mean for instance:  iptables ... -i ppp+ ...).


Consider the following example (a bit complex, I think): a VPN server 
has many clients but not all services and networks has to be accessible 
for each clients, also an ACL is needed (Or the same situation on a 
router with several interfaces) The ifgroup value has 32 bits also 32 
different rule can be set up. The only necessary thing for the 
corresponding "if-up" scripts to calculate the value corresponding to 
the acl list and set this as an ifgroup value for the device. Next in 
the netfilter rules  access to unneeded services can be rejected with a 
single rule:

   iptables -A PREROUTING -m ifgroup --ifgroup-in 0/0x4 ... -j REJECT ...

Also the ifgroup match helps iptables rules to be simplier. Whithout it 
a new chain is necessary and on every interface change a new rule has to 
be added/removed. If two iptables commands are running, AFAIK a 
concurrency problem can be occured because two iptables command 
downloads the ruleset, change it and uploaded. But only one of them is 
taken into account.


Another possible use case is - I have no patch yet:
currently the ip command can access information of exactly one device or 
all of them, but only a few of them cannot be used. With ifgroup this 
problem can be solved, also instead of:

  ip link show dev eth0

use

   ip link show group 3

and so on.


> If that's all it is, userland can record such mappings
> in the filesystem or elsewhere such that multiple
> applications can work with and maintain the relationships.
> There is zero reason to add this bloat to the kernel in
> such a case.

Yeah, without the netfilter rules this would be unnecessary in the 
kernel but it depends on it.

--
Attila

Re: [PATCHv8 1/2] Interface group: core and netlink part

[David Miller]

From: Laszlo Attila Toth <panther@balabit.hu>
Date: Fri, 07 Mar 2008 13:40:11 +0100

> David Miller írta:
> > If that's all it is, userland can record such mappings
> > in the filesystem or elsewhere such that multiple
> > applications can work with and maintain the relationships.
> > There is zero reason to add this bloat to the kernel in
> > such a case.
> 
> Yeah, without the netfilter rules this would be unnecessary in the 
> kernel but it depends on it.

Ok, I'm convinced, please resubmit your patch against
net-2.6.26

Thanks!

[PATCHv8 2/2] Ifgroup read/write support in sysfs

[Laszlo Attila Toth]

The ifgroup member of each net device can be read and changed in sysfs.

Author: Lutz Jaenicke <ljaenicke@innominate.com>

Signed-off-by: Laszlo Attila Toth <panther@balabit.hu>
---
 net/core/net-sysfs.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 7635d3f..6652974 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -208,6 +208,20 @@ static ssize_t store_tx_queue_len(struct device *dev,
 	return netdev_store(dev, attr, buf, len, change_tx_queue_len);
 }
 
+NETDEVICE_SHOW(ifgroup, fmt_hex);
+
+static int change_ifgroup(struct net_device *net, unsigned long new_ifgroup)
+{
+	net->ifgroup = new_ifgroup;
+	return 0;
+}
+
+static ssize_t store_ifgroup(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t len)
+{
+	return netdev_store(dev, attr, buf, len, change_ifgroup);
+}
+
 static struct device_attribute net_class_attributes[] = {
 	__ATTR(addr_len, S_IRUGO, show_addr_len, NULL),
 	__ATTR(iflink, S_IRUGO, show_iflink, NULL),
@@ -224,6 +238,7 @@ static struct device_attribute net_class_attributes[] = {
 	__ATTR(flags, S_IRUGO | S_IWUSR, show_flags, store_flags),
 	__ATTR(tx_queue_len, S_IRUGO | S_IWUSR, show_tx_queue_len,
 	       store_tx_queue_len),
+	__ATTR(ifgroup, S_IRUGO | S_IWUSR, show_ifgroup, store_ifgroup),
 	{}
 };
 
-- 
1.5.2.5