Skip to contentSkip to conversation

Virtual ethernet device (v3)

PATCH In review Pavel Emelianov 2 participants 20 messages Jul 12, 2007 – Jul 16, 2007

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

Pavel Emelianov

author
#0 lore raw

cover Virtual ethernet device (v3)

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.1:
* Made the generic routine for link creation to be used by veth driver, any other tunnel driver that needs to create several devices at once and rtnl_newlink() code.

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]>
Cc Patrick McHardy <[email protected]>
author
#1 lore raw

PATCH 1/2 Introduce the generic rtnl_create_link()

This routine gets the parsed rtnl attributes and creates a new link with generic info (IFLA_LINKINFO policy). Its intention is to help the drivers, that need to create several links at once (like VETH).

This is nothing but a copy-paste-ed part of rtnl_newlink() function that is responsible for creation of new device.

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

---

 include/net/rtnetlink.h |    4 ++
 net/core/rtnetlink.c    |   83 ++++++++++++++++++++++++++++++------------------
 2 files changed, 57 insertions(+), 30 deletions(-)
include/net/rtnetlink.h +4 -0
index 3861c05..8218288 100644
@@ -78,6 +78,10 @@ extern void	__rtnl_link_unregister(struc
 extern int	rtnl_link_register(struct rtnl_link_ops *ops);
 extern void	rtnl_link_unregister(struct rtnl_link_ops *ops);
 
+extern struct net_device *rtnl_create_link(char *ifname,
+		const struct rtnl_link_ops *ops, struct nlattr *tb[]);
+extern const struct nla_policy ifla_policy[IFLA_MAX+1];
+
 #define MODULE_ALIAS_RTNL_LINK(kind) MODULE_ALIAS("rtnl-link-" kind)
 
 #endif
net/core/rtnetlink.c +53 -30

109 lines not on this page · raw message

author
#2 lore raw

PATCH 2/2 Virtual ethernet device driver

In reply to 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.

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.

This implementation uses another interface - the RTM_NRELINK message introduced by Patric.

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

---

 drivers/net/Kconfig  |    6 
 drivers/net/Makefile |    1 
 drivers/net/veth.c   |  426 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/net/veth.h   |   12 +
 4 files changed, 445 insertions(+)

diff-tree 293fc37c28eee0a27e19179ab0d809460c73eced (from 15028aad00ddf241581fbe74a02ec89cbb28d35d)

Author: Pavel <[email protected]>
Date:   Thu Jul 12 13:05:44 2007 +0400

Veth support

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 +426 -0

426 lines not on this page · raw message

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

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 11 lines · Pavel Emelianov
drivers/net/veth.c new file +11 -0
+static int veth_newlink(struct net_device *dev,
+			 struct nlattr *tb[], struct nlattr *data[])
+{
+	int err;
+	struct net_device *peer;
+	struct veth_priv *priv;
+	char ifname[IFNAMSIZ];
+
+	/*
+	 * prepare the devices info
+	 */

drivers/net/veth.c line 319

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(dev->dev_addr);
+
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

Not having a peer should be an error, no?

drivers/net/veth.c line 324

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+
+	if (tb[IFLA_IFNAME])
+		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+	else
+		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");

Does this work? The other device is not registered at this time, so I think the allocated names could clash ..

If it does work you could also perform name allocation in the rtnl_create_link function.

Quoted 44 lines
drivers/net/veth.c new file +43 -0
+
+	peer = rtnl_create_link(ifname, &veth_link_ops, tb);
+	if (IS_ERR(peer))
+		return PTR_ERR(peer);
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(peer->dev_addr);
+
+	/*
+	 * register devices
+	 */
+
+	err = register_netdevice(peer);
+	if (err < 0)
+		goto err_register_peer;
+
+	err = register_netdevice(dev);
+	if (err < 0)
+		goto err_register_dev;
+
+	/*
+	 * tie the deviced together
+	 */
+
+	priv = netdev_priv(dev);
+	priv->dev = dev;
+	priv->peer = peer;
+	list_add(&priv->list, &veth_list);
+
+	priv = netdev_priv(peer);
+	priv->dev = peer;
+	priv->peer = dev;
+	INIT_LIST_HEAD(&priv->list);
+	return 0;
+
+err_register_dev:
+	unregister_netdevice(peer);
+	return err;
+
+err_register_peer:
+	free_netdev(peer);
+	return err;
+}

drivers/net/veth.c line 418

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+static __exit void veth_exit(void)
+{
+	struct veth_priv *priv, *next;
+
+	rtnl_lock();
+	__rtnl_link_unregister(&veth_link_ops);
+
+	list_for_each_entry_safe(priv, next, &veth_list, list)
+		veth_dellink(priv->dev);
+	rtnl_unlock();

Devices are unregistered automatically through the dellink function, rtnl_link_unregister(..) is enough.

author
#4 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 17 lines · Patrick McHardy
Quoted 16 lines · Pavel Emelianov
drivers/net/veth.c new file +16 -0
+static int veth_newlink(struct net_device *dev,
+			 struct nlattr *tb[], struct nlattr *data[])
+{
+	int err;
+	struct net_device *peer;
+	struct veth_priv *priv;
+	char ifname[IFNAMSIZ];
+
+	/*
+	 * prepare the devices info
+	 */
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(dev->dev_addr);
+
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
Quoted 8 lines

drivers/net/veth.c line 319

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

Not having a peer should be an error, no?

No. That's the intention - if the user doesn't specify "peer" in the command line then two _identical_ devices are created. Of course, if he specifies one name - there'll be a collision, but one can say "my_own_veth_number_%d" and everything will be ok. Or just use the default name provided. E.g. "ip link add type veth" will send a packet with data[VETH_INFO_PEER} == NULL, but this is OK! User just wants a default tunnel and he will get it :)

Does this answer your second comment below?

Quoted 58 lines

drivers/net/veth.c line 324

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+
+	if (tb[IFLA_IFNAME])
+		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+	else
+		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");

Does this work? The other device is not registered at this time, so I think the allocated names could clash ..

If it does work you could also perform name allocation in the rtnl_create_link function.

Quoted 43 lines
drivers/net/veth.c new file +43 -0
+
+	peer = rtnl_create_link(ifname, &veth_link_ops, tb);
+	if (IS_ERR(peer))
+		return PTR_ERR(peer);
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(peer->dev_addr);
+
+	/*
+	 * register devices
+	 */
+
+	err = register_netdevice(peer);
+	if (err < 0)
+		goto err_register_peer;
+
+	err = register_netdevice(dev);
+	if (err < 0)
+		goto err_register_dev;
+
+	/*
+	 * tie the deviced together
+	 */
+
+	priv = netdev_priv(dev);
+	priv->dev = dev;
+	priv->peer = peer;
+	list_add(&priv->list, &veth_list);
+
+	priv = netdev_priv(peer);
+	priv->dev = peer;
+	priv->peer = dev;
+	INIT_LIST_HEAD(&priv->list);
+	return 0;
+
+err_register_dev:
+	unregister_netdevice(peer);
+	return err;
+
+err_register_peer:
+	free_netdev(peer);
+	return err;
+}
Quoted 14 lines

drivers/net/veth.c line 418

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+static __exit void veth_exit(void)
+{
+	struct veth_priv *priv, *next;
+
+	rtnl_lock();
+	__rtnl_link_unregister(&veth_link_ops);
+
+	list_for_each_entry_safe(priv, next, &veth_list, list)
+		veth_dellink(priv->dev);
+	rtnl_unlock();

Devices are unregistered automatically through the dellink function, rtnl_link_unregister(..) is enough.

OK. This looks like a minor and not-significant comment, so do I need to resend the patch or David is OK to take it and I will send an incremental one?

Thanks,
Pavel

#5 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 30 lines · Pavel Emelianov
Quoted 18 lines · Patrick McHardy
Quoted 16 lines · Pavel Emelianov
drivers/net/veth.c new file +16 -0
+static int veth_newlink(struct net_device *dev,
+			 struct nlattr *tb[], struct nlattr *data[])
+{
+	int err;
+	struct net_device *peer;
+	struct veth_priv *priv;
+	char ifname[IFNAMSIZ];
+
+	/*
+	 * prepare the devices info
+	 */
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(dev->dev_addr);
+
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
Quoted 8 lines

drivers/net/veth.c line 319

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

Not having a peer should be an error, no?

Quoted 7 lines

No. That's the intention - if the user doesn't specify "peer" in the command line then two _identical_ devices are created. Of course, if he specifies one name - there'll be a collision, but one can say "my_own_veth_number_%d" and everything will be ok. Or just use the default name provided. E.g. "ip link add type veth" will send a packet with data[VETH_INFO_PEER} == NULL, but this is OK! User just wants a default tunnel and he will get it :)

I see.

Quoted 1 line

Does this answer your second comment below?

No, to get unique names the sequence has to be:

dev_alloc_name
register_netdevice
dev_alloc_name
register_netdevice

But you have:

dev_alloc_name
dev_alloc_name (<- might allocate same name as first call) register_netdevice
register_netdevice

Quoted 16 lines
Quoted 6 lines
Quoted 6 lines
drivers/net/veth.c new file +6 -0
+static __exit void veth_exit(void)
+{
+	struct veth_priv *priv, *next;
+
+	rtnl_lock();
+	__rtnl_link_unregister(&veth_link_ops);
Quoted 8 lines

drivers/net/veth.c line 418

Quoted 4 lines
drivers/net/veth.c new file +4 -0
+
+	list_for_each_entry_safe(priv, next, &veth_list, list)
+		veth_dellink(priv->dev);
+	rtnl_unlock();

Devices are unregistered automatically through the dellink function, rtnl_link_unregister(..) is enough.

Quoted 3 lines

OK. This looks like a minor and not-significant comment, so do I need to resend the patch or David is OK to take it and I will send an incremental one?

An incremental patch for this is fine I guess, your code is correct, its merely a simplification.

author
#6 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 49 lines · Patrick McHardy
Quoted 28 lines · Pavel Emelianov
Quoted 17 lines · Patrick McHardy
Quoted 15 lines · Pavel Emelianov
drivers/net/veth.c new file +15 -0
+static int veth_newlink(struct net_device *dev,
+			 struct nlattr *tb[], struct nlattr *data[])
+{
+	int err;
+	struct net_device *peer;
+	struct veth_priv *priv;
+	char ifname[IFNAMSIZ];
+
+	/*
+	 * prepare the devices info
+	 */
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(dev->dev_addr);
+
Quoted 8 lines

drivers/net/veth.c line 319

Quoted 6 lines
drivers/net/veth.c new file +6 -0
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

Not having a peer should be an error, no?

Quoted 7 lines

No. That's the intention - if the user doesn't specify "peer" in the command line then two _identical_ devices are created. Of course, if he specifies one name - there'll be a collision, but one can say "my_own_veth_number_%d" and everything will be ok. Or just use the default name provided. E.g. "ip link add type veth" will send a packet with data[VETH_INFO_PEER} == NULL, but this is OK! User just wants a default tunnel and he will get it :)

I see.

Quoted 1 line

Does this answer your second comment below?

No, to get unique names the sequence has to be:

dev_alloc_name
register_netdevice
dev_alloc_name
register_netdevice

Quoted 6 lines

But you have:

dev_alloc_name
dev_alloc_name (<- might allocate same name as first call) register_netdevice
register_netdevice

Oops :) You're right. That's the problem. I was carried away by testing the "peer" options and checking for names rather than "veth%d" to work...

By the way, that will create some problems. You see, your patches imply that the register_netdevice() will be called at the very end of the ->newlink callback. Otherwise, the error path of any code following the registering will have to call unregister_netdevice() which will BUG() in free_netdev() in rtnl_newlink() - the device state will be neither UNINITIALIZED nor UNREGISTERED :(

Quoted 14 lines
Quoted 14 lines
Quoted 13 lines

drivers/net/veth.c line 418

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+static __exit void veth_exit(void)
+{
+	struct veth_priv *priv, *next;
+
+	rtnl_lock();
+	__rtnl_link_unregister(&veth_link_ops);
+
+	list_for_each_entry_safe(priv, next, &veth_list, list)
+		veth_dellink(priv->dev);
+	rtnl_unlock();

Devices are unregistered automatically through the dellink function, rtnl_link_unregister(..) is enough.

Quoted 8 lines
Quoted 3 lines

OK. This looks like a minor and not-significant comment, so do I need to resend the patch or David is OK to take it and I will send an incremental one?

An incremental patch for this is fine I guess, your code is correct, its merely a simplification.

Thanks,
Pavel

#7 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 21 lines · Pavel Emelianov
Quoted 7 lines · Patrick McHardy

No, to get unique names the sequence has to be:

dev_alloc_name
register_netdevice
dev_alloc_name
register_netdevice

Quoted 6 lines

But you have:

dev_alloc_name
dev_alloc_name (<- might allocate same name as first call) register_netdevice
register_netdevice

Oops :) You're right. That's the problem. I was carried away by testing the "peer" options and checking for names rather than "veth%d" to work...

Quoted 6 lines

By the way, that will create some problems. You see, your patches imply that the register_netdevice() will be called at the very end of the ->newlink callback. Otherwise, the error path of any code following the registering will have to call unregister_netdevice() which will BUG() in free_netdev() in rtnl_newlink() - the device state will be neither UNINITIALIZED nor UNREGISTERED :(

Thats true. I think you could do:

- use name of the supplied device for the second device
- register second device
- allocate new name for first device
- register first device

author
#8 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 29 lines · Patrick McHardy
Quoted 20 lines · Pavel Emelianov
Quoted 7 lines · Patrick McHardy

No, to get unique names the sequence has to be:

dev_alloc_name
register_netdevice
dev_alloc_name
register_netdevice

Quoted 6 lines

But you have:

dev_alloc_name
dev_alloc_name (<- might allocate same name as first call) register_netdevice
register_netdevice

Oops :) You're right. That's the problem. I was carried away by testing the "peer" options and checking for names rather than "veth%d" to work...

Quoted 6 lines

By the way, that will create some problems. You see, your patches imply that the register_netdevice() will be called at the very end of the ->newlink callback. Otherwise, the error path of any code following the registering will have to call unregister_netdevice() which will BUG() in free_netdev() in rtnl_newlink() - the device state will be neither UNINITIALIZED nor UNREGISTERED :(

Quoted 7 lines

Thats true. I think you could do:

- use name of the supplied device for the second device
- register second device
- allocate new name for first device
- register first device

Not 100% like this (since the first name is expected to be associated with the device with the first address), but reallocating the name for the first device sounds like a good idea.

David, which way would be more preferable - to fix booth issues pointed by Patric and send the v.4 patch, or to wait for this patch to be committed and then send two incremental fixes?

Pavel

author
#9 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

In reply to Patrick McHardy
Quoted 26 lines · Patrick McHardy
Quoted 11 lines · Pavel Emelianov
drivers/net/veth.c new file +11 -0
+static int veth_newlink(struct net_device *dev,
+			 struct nlattr *tb[], struct nlattr *data[])
+{
+	int err;
+	struct net_device *peer;
+	struct veth_priv *priv;
+	char ifname[IFNAMSIZ];
+
+	/*
+	 * prepare the devices info
+	 */

drivers/net/veth.c line 319

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(dev->dev_addr);
+
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

Not having a peer should be an error, no?

Quoted 9 lines

drivers/net/veth.c line 324

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+
+	if (tb[IFLA_IFNAME])
+		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+	else
+		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");

Does this work? The other device is not registered at this time, so I think the allocated names could clash ..

Oh! And one more thing, I've just thought about - even two devices with similar name would be ok when we have a namespaces and one of the ends of this tunnel is to be in some other namespace rather that the init one.

Quoted 63 lines

If it does work you could also perform name allocation in the rtnl_create_link function.

Quoted 44 lines
drivers/net/veth.c new file +43 -0
+
+	peer = rtnl_create_link(ifname, &veth_link_ops, tb);
+	if (IS_ERR(peer))
+		return PTR_ERR(peer);
+
+	if (tb[IFLA_ADDRESS] == NULL)
+		random_ether_addr(peer->dev_addr);
+
+	/*
+	 * register devices
+	 */
+
+	err = register_netdevice(peer);
+	if (err < 0)
+		goto err_register_peer;
+
+	err = register_netdevice(dev);
+	if (err < 0)
+		goto err_register_dev;
+
+	/*
+	 * tie the deviced together
+	 */
+
+	priv = netdev_priv(dev);
+	priv->dev = dev;
+	priv->peer = peer;
+	list_add(&priv->list, &veth_list);
+
+	priv = netdev_priv(peer);
+	priv->dev = peer;
+	priv->peer = dev;
+	INIT_LIST_HEAD(&priv->list);
+	return 0;
+
+err_register_dev:
+	unregister_netdevice(peer);
+	return err;
+
+err_register_peer:
+	free_netdev(peer);
+	return err;
+}

drivers/net/veth.c line 418

Quoted 10 lines
drivers/net/veth.c new file +10 -0
+static __exit void veth_exit(void)
+{
+	struct veth_priv *priv, *next;
+
+	rtnl_lock();
+	__rtnl_link_unregister(&veth_link_ops);
+
+	list_for_each_entry_safe(priv, next, &veth_list, list)
+		veth_dellink(priv->dev);
+	rtnl_unlock();

Devices are unregistered automatically through the dellink function, rtnl_link_unregister(..) is enough.

#10 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 13 lines · Pavel Emelianov
Quoted 9 lines · Patrick McHardy

drivers/net/veth.c line 324

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+
+	if (tb[IFLA_IFNAME])
+		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+	else
+		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");

Does this work? The other device is not registered at this time, so I think the allocated names could clash ..

Quoted 3 lines

Oh! And one more thing, I've just thought about - even two devices with similar name would be ok when we have a namespaces and one of the ends of this tunnel is to be in some other namespace rather that the init one.

Yes, but we don't have network namespaces yet. Which brings up another
question: is this driver of any use without namespaces?

author
#11 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 12 lines · Patrick McHardy
Quoted 11 lines · Pavel Emelianov
Quoted 8 lines · Patrick McHardy

drivers/net/veth.c line 324

Quoted 5 lines
drivers/net/veth.c new file +5 -0
+
+	if (tb[IFLA_IFNAME])
+		nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+	else
+		snprintf(ifname, IFNAMSIZ, DRV_NAME "%%d");

Does this work? The other device is not registered at this time, so I think the allocated names could clash ..

Quoted 9 lines
Quoted 3 lines

Oh! And one more thing, I've just thought about - even two devices with similar name would be ok when we have a namespaces and one of the ends of this tunnel is to be in some other namespace rather that the init one.

Yes, but we don't have network namespaces yet. Which brings up another
question: is this driver of any use without namespaces?

Sure. For example, you can join two bridges together.

Thanks,
Pavel

#12 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

In reply to Pavel Emelianov

drivers/net/veth.c line 319

Quoted 6 lines · Pavel Emelianov
drivers/net/veth.c new file +6 -0
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

One more suggestion regarding the PEER attribute: you only nest IFLA attributes below it, but some information that might be interesting to use on device creation is contained in ifinfomsg (flags and ifindex). So I think it would be better to use a complete message, including header.

author
#13 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 14 lines · Patrick McHardy

drivers/net/veth.c line 319

Quoted 6 lines · Pavel Emelianov
drivers/net/veth.c new file +6 -0
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

One more suggestion regarding the PEER attribute: you only nest IFLA attributes below it, but some information that might be interesting to use on device creation is contained in ifinfomsg (flags and ifindex). So I think it would be better to use a complete message, including header.

I don't get it. Can you elaborate, please.

Pavel

#14 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 19 lines · Pavel Emelianov
Quoted 15 lines · Patrick McHardy

drivers/net/veth.c line 319

Quoted 6 lines · Pavel Emelianov
drivers/net/veth.c new file +6 -0
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

One more suggestion regarding the PEER attribute: you only nest IFLA attributes below it, but some information that might be interesting to use on device creation is contained in ifinfomsg (flags and ifindex). So I think it would be better to use a complete message, including header.

Quoted 1 line

I don't get it. Can you elaborate, please.

You don't have a struct ifinfomsg for the peer device. At some point we might want to add support for specifying initial flags for the device (some easily supportable ones are IFF_PROMISC, IFF_ALLMULTI, IFF_NOARP) and ideally that should also be possible for the peer device.

So I suggest you use a complete ifinfomsg including the header instead of just the attributes for VETH_INFO_PEER.

author
#15 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 21 lines · Patrick McHardy
Quoted 17 lines · Pavel Emelianov
Quoted 14 lines · Patrick McHardy

drivers/net/veth.c line 319

Quoted 6 lines · Pavel Emelianov
drivers/net/veth.c new file +6 -0
+	if (data != NULL && data[VETH_INFO_PEER] != NULL) {
+		err = nla_parse_nested(tb, IFLA_INFO_MAX,
+				data[VETH_INFO_PEER], ifla_policy);
+		if (err < 0)
+			return err;
+	}

One more suggestion regarding the PEER attribute: you only nest IFLA attributes below it, but some information that might be interesting to use on device creation is contained in ifinfomsg (flags and ifindex). So I think it would be better to use a complete message, including header.

Quoted 1 line

I don't get it. Can you elaborate, please.

Quoted 10 lines

You don't have a struct ifinfomsg for the peer device. At some point we might want to add support for specifying initial flags for the device (some easily supportable ones are IFF_PROMISC, IFF_ALLMULTI, IFF_NOARP) and ideally that should also be possible for the peer device.

So I suggest you use a complete ifinfomsg including the header instead of just the attributes for VETH_INFO_PEER.

I don't see any information from this struct being used on the link-creation paths...

Pavel

#16 lore raw

Re: [PATCH 2/2] Virtual ethernet device driver

Quoted 8 lines · Pavel Emelianov
Quoted 6 lines · Patrick McHardy

You don't have a struct ifinfomsg for the peer device. At some point we might want to add support for specifying initial flags for the device (some easily supportable ones are IFF_PROMISC, IFF_ALLMULTI, IFF_NOARP) and ideally that should also be possible for the peer device.

Quoted 7 lines
Quoted 4 lines

So I suggest you use a complete ifinfomsg including the header instead of just the attributes for VETH_INFO_PEER.

I don't see any information from this struct being used on the link-creation paths...

Thats what I explained twice, its not used today, it might be used in the future for ifindex and flags.

author
#18 lore raw

PATCH 1/2 Intruduce iplink_parse() routine

This routine parses CLI attributes, describing generic link parameters such as name, address, etc.

This is mostly copy-pasted from iplink_modify().

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

---

 include/utils.h |    3 +
 ip/iplink.c     |  127 ++++++++++++++++++++++++++++++++------------------------
 2 files changed, 76 insertions(+), 54 deletions(-)
include/utils.h +3 -0
index a3fd335..3fd851d 100644
@@ -146,4 +146,7 @@ extern int cmdlineno;
 extern size_t getcmdline(char **line, size_t *len, FILE *in);
 extern int makeargs(char *line, char *argv[], int maxargs);
 
+struct iplink_req;
+int iplink_parse(int argc, char **argv, struct iplink_req *req,
+		char **name, char **type, char **link, char **dev);
 #endif /* __UTILS_H__ */
ip/iplink.c +73 -54

213 lines not on this page · raw message

author
#19 lore raw

PATCH 2/2 Module for ip utility to support veth device

In reply to Pavel Emelianov

The link_veth.so itself.

The usage is
ip link add <name> type veth [peer ...]

where ... are the same options as in regular ip link add usage.

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

---

 Makefile    |    6 +++++-
 link_veth.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 veth.h      |   12 ++++++++++++
 3 files changed, 77 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 +60 -0

60 lines not on this page · raw message

ip/veth.h new file +12 -0
new file mode 100644
index 0000000..aa2e6f9
@@ -0,0 +1,12 @@
+#ifndef __NET_VETH_H__
+#define __NET_VETH_H__
+
+enum {
+	VETH_INFO_UNSPEC,
+	VETH_INFO_PEER,
+
+	__VETH_INFO_MAX
+#define VETH_INFO_MAX	(__VETH_INFO_MAX - 1)
+};
+
+#endif