Skip to contentSkip to conversation

Virtual ethernet device (v2.1)

PATCH In review Pavel Emelianov 5 participants 13 messages Jul 11, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Pavel Emelianov

author
#0 lore raw

PATCH 1/1 Virtual ethernet device (v2.1)

=============================================================
Since the netlink NEWLINK interface is now in the netdev tree I resend the veth driver patch as "submittion for inclusion".
=============================================================

LOG:

Veth stands for Virtual ETHernet. It is a simple tunnel driver that works at the link layer and looks like a pair of ethernet devices interconnected with each other.

Mainly it allows to communicate between network namespaces but it can be used as is as well.

Eric recently sent a similar driver called etun with the sysfs interface. This implementation uses another interface - the RTM_NRELINK message introduced by Patric.

The newlink callback is organized that way to make it easy to create the peer device in the separate namespace when we have them in kernel.

Changes from v.2:
* Rebase over latest netdev tree. No actual changes;
* Small code rework.

Changes from v.1:
* percpu statistics;
* standard convention for nla policy names;
* module alias added;
* xmit function fixes noticed by Patric;
* code cleanup.

The patch for an ip utility is also provided.

Signed-off-by Pavel Emelianov <[email protected]>

---

 drivers/net/Kconfig  |    6 
 drivers/net/Makefile |    1 
 drivers/net/veth.c   |  452 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/net/veth.h   |   14 +
 4 files changed, 473 insertions(+)
drivers/net/Kconfig +6 -0
index d4e39ff..7bdc6e3 100644
@@ -124,6 +124,12 @@ config TUN
 
 	  If you don't know what to use this for, you don't need it.
 
+config VETH
+	tristate "Virtual ethernet device"
+	---help---
+	  The device is an ethernet tunnel. Devices are created in pairs. When
+	  one end receives the packet it appears on its pair and vice versa.
+
 config NET_SB1000
 	tristate "General Instruments Surfboard 1000"
 	depends on PNP
drivers/net/Makefile +1 -0
index a2241e6..d7b1103 100644
@@ -189,6 +189,7 @@ obj-$(CONFIG_MACSONIC) += macsonic.o
 obj-$(CONFIG_MACMACE) += macmace.o
 obj-$(CONFIG_MAC89x0) += mac89x0.o
 obj-$(CONFIG_TUN) += tun.o
+obj-$(CONFIG_VETH) += veth.o
 obj-$(CONFIG_NET_NETX) += netx-eth.o
 obj-$(CONFIG_DL2K) += dl2k.o
 obj-$(CONFIG_R8169) += r8169.o
drivers/net/veth.c new file +452 -0

452 lines not on this page · raw message

include/net/veth.h new file +14 -0
new file mode 100644
index 0000000..b84a530
@@ -0,0 +1,14 @@
+#ifndef __NET_VETH_H__
+#define __NET_VETH_H__
+
+enum {
+	VETH_INFO_UNSPEC,
+	VETH_INFO_MAC,
+	VETH_INFO_PEER,
+	VETH_INFO_PEER_MAC,
+
+	__VETH_INFO_MAX
+#define VETH_INFO_MAX	(__VETH_INFO_MAX - 1)
+};
+
+#endif
author
#1 lore raw

PATCH 1/1 Module for ip utility to support veth device (v.2.1)

The usage is

# ip link add [name] type veth [peer <name>] [mac <mac>] [peer_mac <mac>]

This version doesn't include the fix for ip/iplink.c as Patrick said that he had included it into his patches already.

Signed-off-by Pavel Emelianov <[email protected]>

---

 Makefile    |    6 +++-
 link_veth.c |   89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 veth.h      |   14 +++++++++
 3 files changed, 108 insertions(+), 1 deletion(-)
ip/Makefile +5 -1
index 9a5bfe3..b46bce3 100644
@@ -8,8 +8,9 @@ RTMONOBJ=rtmon.o
 ALLOBJ=$(IPOBJ) $(RTMONOBJ)
 SCRIPTS=ifcfg rtpr routel routef
 TARGETS=ip rtmon
+LIBS=link_veth.so
 
-all: $(TARGETS) $(SCRIPTS)
+all: $(TARGETS) $(SCRIPTS) $(LIBS)
 
 ip: $(IPOBJ) $(LIBNETLINK) $(LIBUTIL)
 

@@ -24,3 +25,6 @@ clean:
 
 LDLIBS	+= -ldl
 LDFLAGS	+= -Wl,-export-dynamic
+
+%.so: %.c
+	$(CC) $(CFLAGS) -shared $< -o $@
ip/link_veth.c new file +89 -0

89 lines not on this page · raw message

ip/veth.h new file +14 -0
new file mode 100644
index 0000000..b84a530
@@ -0,0 +1,14 @@
+#ifndef __NET_VETH_H__
+#define __NET_VETH_H__
+
+enum {
+	VETH_INFO_UNSPEC,
+	VETH_INFO_MAC,
+	VETH_INFO_PEER,
+	VETH_INFO_PEER_MAC,
+
+	__VETH_INFO_MAX
+#define VETH_INFO_MAX	(__VETH_INFO_MAX - 1)
+};
+
+#endif
#2 lore raw
In reply to Pavel Emelianov

Hi.

Quoted 6 lines · Pavel Emelianov

Veth stands for Virtual ETHernet. It is a simple tunnel driver that works at the link layer and looks like a pair of ethernet devices interconnected with each other.

Mainly it allows to communicate between network namespaces but it can be used as is as well.

I have minor comment about flags used in veth. It looks like you only set LLTX bit, but check for other flags in the code, so why not to include another performance features?

Quoted 4 lines
drivers/net/veth.c new file +4 -0
+static void veth_setup(struct net_device *dev)
+{
+	ether_setup(dev);
+

drivers/net/veth.c line 291

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+	dev->hard_start_xmit = veth_xmit;
+	dev->get_stats = veth_get_stats;
+	dev->open = veth_open;
+	dev->stop = veth_close;
+	dev->ethtool_ops = &veth_ethtool_ops;
+	dev->features |= NETIF_F_LLTX;
+	dev->init = veth_dev_init;
+	dev->destructor = veth_dev_free;
+	netif_carrier_off(dev);
+}

Don't you want to include NETIF_F_SG, NETIF_F_NO_CSUM, NETIF_F_FRAGLIST, NETIF_F_HIGHDMA and probably TSO fancy flags also to reduce cpu usage more?

Signature · 1 line
	Evgeniy Polyakov
#3 lore raw
In reply to Pavel Emelianov

Am Mittwoch, 11. Juli 2007 schrieb Pavel Emelianov:

Quoted 2 lines
 drivers/net/veth.c   |  452 
 include/net/veth.h   |   14 +

I know, I am late in the game, but wont the name collide somewhat with drivers/net/ibmveth.h, drivers/net/iseries_veth.c, and drivers/net/ibmveth.c?

Christian

author
#4 lore raw
Quoted 6 lines · Christian Borntraeger

Am Mittwoch, 11. Juli 2007 schrieb Pavel Emelianov:

Quoted 2 lines
 drivers/net/veth.c   |  452 
 include/net/veth.h   |   14 +

I know, I am late in the game, but wont the name collide somewhat with drivers/net/ibmveth.h, drivers/net/iseries_veth.c, and drivers/net/ibmveth.c?

David asked the same thing after v.1. The answer is :
No it will not. These devices use eth%d strings to generate their names.

Quoted 2 lines

Christian

Thanks,
Pavel

maintainer
#5 lore raw
In reply to Christian Borntraeger

From: Christian Borntraeger <[email protected]>
Date: Wed, 11 Jul 2007 12:54:24 +0200

Quoted 6 lines

Am Mittwoch, 11. Juli 2007 schrieb Pavel Emelianov:

Quoted 2 lines
 drivers/net/veth.c   |  452 
 include/net/veth.h   |   14 +

I know, I am late in the game, but wont the name collide somewhat with drivers/net/ibmveth.h, drivers/net/iseries_veth.c, and drivers/net/ibmveth.c?

We discussed that and decided that it's not an issue.

#6 lore raw
In reply to Pavel Emelianov

drivers/net/veth.c line 414

Quoted 5 lines · Pavel Emelianov
drivers/net/veth.c new file +5 -0
+static const struct nla_policy veth_policy[VETH_INFO_MAX + 1] = {
+	[VETH_INFO_MAC]		= { .type = NLA_BINARY, .len = ETH_ALEN },
+	[VETH_INFO_PEER]	= { .type = NLA_STRING },
+	[VETH_INFO_PEER_MAC]	= { .type = NLA_BINARY, .len = ETH_ALEN },
+};

Looks good, just one question. What happended to the IFLA_PARTNER attribute idea? I have a patch to allow specifying the initial MAC address for a device, IFLA_PARTNER would make the whole thing symetrical.

author
#7 lore raw
Quoted 12 lines · Patrick McHardy

drivers/net/veth.c line 414

Quoted 5 lines · Pavel Emelianov
drivers/net/veth.c new file +5 -0
+static const struct nla_policy veth_policy[VETH_INFO_MAX + 1] = {
+	[VETH_INFO_MAC]		= { .type = NLA_BINARY, .len = ETH_ALEN },
+	[VETH_INFO_PEER]	= { .type = NLA_STRING },
+	[VETH_INFO_PEER_MAC]	= { .type = NLA_BINARY, .len = ETH_ALEN },
+};

Looks good, just one question. What happended to the IFLA_PARTNER attribute idea? I have a patch to allow specifying the initial MAC address for a device, IFLA_PARTNER would make the whole thing symetrical.

Well, the notion of a partner is not applicable to any generic link unlike the device name, physical (MAC) address or MTU value. So i think that it's better to keep this as a specific driver information not to confuse the generic code. I think it's worth making this as some more generic code than it is now, but since this driver is the only user of "partner" maybe it's better not to make it right now. Later, when we do know what do we want "partner" to be.

Thanks,
Pavel

#8 lore raw
Quoted 17 lines · Pavel Emelianov
Quoted 13 lines · Patrick McHardy

drivers/net/veth.c line 414

Quoted 5 lines · Pavel Emelianov
drivers/net/veth.c new file +5 -0
+static const struct nla_policy veth_policy[VETH_INFO_MAX + 1] = {
+	[VETH_INFO_MAC]		= { .type = NLA_BINARY, .len = ETH_ALEN },
+	[VETH_INFO_PEER]	= { .type = NLA_STRING },
+	[VETH_INFO_PEER_MAC]	= { .type = NLA_BINARY, .len = ETH_ALEN },
+};

Looks good, just one question. What happended to the IFLA_PARTNER attribute idea? I have a patch to allow specifying the initial MAC address for a device, IFLA_PARTNER would make the whole thing symetrical.

Quoted 7 lines

Well, the notion of a partner is not applicable to any generic link unlike the device name, physical (MAC) address or MTU value. So i think that it's better to keep this as a specific driver information not to confuse the generic code. I think it's worth making this as some more generic code than it is now, but since this driver is the only user of "partner" maybe it's better not to make it right now. Later, when we do know what do we want "partner" to be.

Mhh doing it later means dealing with compatibility issues, which is why I'm asking now. We currently support IFLA_NAME, IFLA_MTU, IFLA_TXQLEN, IFLA_WEIGTH, IFLA_OPERSTATE and IFLA_LINKMODE, and with my patch additionally IFLA_ADDRESS and IFLA_BROADCAST. AFAICT they are all applicable for the partner link as well.

author
#9 lore raw
Quoted 25 lines · Patrick McHardy
Quoted 15 lines · Pavel Emelianov
Quoted 12 lines · Patrick McHardy

drivers/net/veth.c line 414

Quoted 5 lines · Pavel Emelianov
drivers/net/veth.c new file +5 -0
+static const struct nla_policy veth_policy[VETH_INFO_MAX + 1] = {
+	[VETH_INFO_MAC]		= { .type = NLA_BINARY, .len = ETH_ALEN },
+	[VETH_INFO_PEER]	= { .type = NLA_STRING },
+	[VETH_INFO_PEER_MAC]	= { .type = NLA_BINARY, .len = ETH_ALEN },
+};

Looks good, just one question. What happended to the IFLA_PARTNER attribute idea? I have a patch to allow specifying the initial MAC address for a device, IFLA_PARTNER would make the whole thing symetrical.

Quoted 7 lines

Well, the notion of a partner is not applicable to any generic link unlike the device name, physical (MAC) address or MTU value. So i think that it's better to keep this as a specific driver information not to confuse the generic code. I think it's worth making this as some more generic code than it is now, but since this driver is the only user of "partner" maybe it's better not to make it right now. Later, when we do know what do we want "partner" to be.

Quoted 2 lines

Mhh doing it later means dealing with compatibility issues, which is why I'm asking now. We currently support IFLA_NAME, IFLA_MTU,

Oh, I see.

Quoted 3 lines

IFLA_TXQLEN, IFLA_WEIGTH, IFLA_OPERSTATE and IFLA_LINKMODE, and with my patch additionally IFLA_ADDRESS and IFLA_BROADCAST. AFAICT they are all applicable for the partner link as well.

Agree. Maybe it is better to make some generic routine to create the device with the parameters specified in the netlink packet. Then the generic code creates one end of a tunnel and calls ->new_link callback. This callback extracts the PARTNER packet part and calls this generic routine to create the second pair.

Thanks,
Pavel

#10 lore raw
Quoted 14 lines · Pavel Emelianov
Quoted 2 lines · Patrick McHardy

Mhh doing it later means dealing with compatibility issues, which is why I'm asking now. We currently support IFLA_NAME, IFLA_MTU,

Oh, I see.

Quoted 3 lines

IFLA_TXQLEN, IFLA_WEIGTH, IFLA_OPERSTATE and IFLA_LINKMODE, and with my patch additionally IFLA_ADDRESS and IFLA_BROADCAST. AFAICT they are all applicable for the partner link as well.

Quoted 5 lines

Agree. Maybe it is better to make some generic routine to create the device with the parameters specified in the netlink packet. Then the generic code creates one end of a tunnel and calls ->new_link callback. This callback extracts the PARTNER packet part and calls this generic routine to create the second pair.

Something like that. Moving the part between NLM_F_CREATE and the ops->newlink call of rtnl_newlink to a new function should work.

For now you could even parse the IFLA_PARTNER attribute and nested IFLA_NAME/IFLA_ADDRESS attributes yourself and ignore the rest, this will at least leave us the option of handling it generically later.

author
#11 lore raw
Quoted 20 lines · Patrick McHardy
Quoted 12 lines · Pavel Emelianov
Quoted 2 lines · Patrick McHardy

Mhh doing it later means dealing with compatibility issues, which is why I'm asking now. We currently support IFLA_NAME, IFLA_MTU,

Oh, I see.

Quoted 3 lines

IFLA_TXQLEN, IFLA_WEIGTH, IFLA_OPERSTATE and IFLA_LINKMODE, and with my patch additionally IFLA_ADDRESS and IFLA_BROADCAST. AFAICT they are all applicable for the partner link as well.

Quoted 5 lines

Agree. Maybe it is better to make some generic routine to create the device with the parameters specified in the netlink packet. Then the generic code creates one end of a tunnel and calls ->new_link callback. This callback extracts the PARTNER packet part and calls this generic routine to create the second pair.

Quoted 7 lines

Something like that. Moving the part between NLM_F_CREATE and the ops->newlink call of rtnl_newlink to a new function should work.

For now you could even parse the IFLA_PARTNER attribute and nested IFLA_NAME/IFLA_ADDRESS attributes yourself and ignore the rest, this will at least leave us the option of handling it generically later.

OK. I'll try to make the generic call. Could you please send me the patches with IFLA_ADDRESS support for booth kernel and ip utility.

Thanks,
Pavel

#12 lore raw
Quoted 2 lines · Pavel Emelianov

OK. I'll try to make the generic call. Could you please send me the patches with IFLA_ADDRESS support for booth kernel and ip utility.

I'll send the kernel patches seperately in a few minutes. iproute doesn't need any further patches.