Skip to contentSkip to conversation

tuntap: allow changing ethtool speed (v2)

PATCH In review Stephen Hemminger 3 participants 12 messages Jul 24, 2013 – Jul 25, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 tuntap: allow changing ethtool speed (v2)

The data reported by the ethtool interface for tun/tap devices is artificial. The default speed is 10Mbit. Virtual interfaces are often faster than this nowadays, so the observed bandwidth may exceed the available bandwidth. This patch allows an administrator to change the available bandwidth as reported by ethtool.

---
v2 - allow setting duplex as well, fix patch formatting
ignore other settings in set

 drivers/net/tun.c |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
drivers/net/tun.c +24 -2
@@ -187,6 +187,8 @@ struct tun_struct {
 	struct list_head disabled;
 	void *security;
 	u32 flow_count;
+	u32 speed;
+	u8 duplex;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)
@@ -1426,6 +1428,8 @@ static void tun_setup(struct net_device
 
 	tun->owner = INVALID_UID;
 	tun->group = INVALID_GID;
+	tun->speed = SPEED_10;
+	tun->duplex = DUPLEX_FULL;
 
 	dev->ethtool_ops = &tun_ethtool_ops;
 	dev->destructor = tun_free_netdev;
@@ -2229,10 +2233,12 @@ static struct miscdevice tun_miscdev = {
 
 static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
+	struct tun_struct *tun = netdev_priv(dev);
+
 	cmd->supported		= 0;
 	cmd->advertising	= 0;
-	ethtool_cmd_speed_set(cmd, SPEED_10);
-	cmd->duplex		= DUPLEX_FULL;
+	ethtool_cmd_speed_set(cmd, tun->speed);
+	cmd->duplex		= tun->duplex;
 	cmd->port		= PORT_TP;
 	cmd->phy_address	= 0;
 	cmd->transceiver	= XCVR_INTERNAL;
@@ -2242,6 +2248,21 @@ static int tun_get_settings(struct net_d
 	return 0;
 }
 
+static int tun_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+
+	if (cmd->autoneg != AUTONEG_DISABLE)
+		return -EINVAL;
+
+	if (!(cmd->duplex == DUPLEX_FULL || cmd->duplex == DUPLEX_HALF))
+		return -EINVAL;
+
+	tun->speed = ethtool_cmd_speed(cmd);
+	tun->duplex = cmd->duplex;
+	return 0;
+}
+
 static void tun_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 {
 	struct tun_struct *tun = netdev_priv(dev);
@@ -2279,6 +2300,7 @@ static void tun_set_msglevel(struct net_
 
 static const struct ethtool_ops tun_ethtool_ops = {
 	.get_settings	= tun_get_settings,
+	.set_settings	= tun_set_settings,
 	.get_drvinfo	= tun_get_drvinfo,
 	.get_msglevel	= tun_get_msglevel,
 	.set_msglevel	= tun_set_msglevel,
#1 lore raw

PATCH 2/3 tuntap: allow overriding ethtool driver info

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

If the application does not override the ethtool settings, the original hard coded values are used.

---
 drivers/net/tun.c           |   56 +++++++++++++++++++++++++++++++++-----------
 include/uapi/linux/if_tun.h |   15 +++++++++++
 2 files changed, 58 insertions(+), 13 deletions(-)
drivers/net/tun.c +20 -11
@@ -189,6 +189,7 @@ struct tun_struct {
 	u32 flow_count;
 	u32 speed;
 	u8 duplex;
+	struct tun_info info;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)
@@ -872,9 +873,13 @@ static void tun_net_init(struct net_devi
 {
 	struct tun_struct *tun = netdev_priv(dev);
 
+	strlcpy(tun->info.driver, DRV_NAME, sizeof(tun->info.driver));
+	strlcpy(tun->info.version, DRV_VERSION, sizeof(tun->info.version));
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		dev->netdev_ops = &tun_netdev_ops;
+		strlcpy(tun->info.bus, "tun", sizeof(tun->info.bus));
 
 		/* Point-to-Point TUN Device */
 		dev->hard_header_len = 0;
@@ -889,6 +894,8 @@ static void tun_net_init(struct net_devi
 
 	case TUN_TAP_DEV:
 		dev->netdev_ops = &tap_netdev_ops;
+		strlcpy(tun->info.bus, "tap", sizeof(tun->info.bus));
+
 		/* Ethernet TAP Device */
 		ether_setup(dev);
 		dev->priv_flags &= ~IFF_TX_SKB_SHARING;
@@ -2092,6 +2099,15 @@ static long __tun_chr_ioctl(struct file
 		tun_detach_filter(tun, tun->numqueues);
 		break;
 
+	case TUNSETINFO: {
+		struct tun_info info;
+
+		if (copy_from_user(&info, argp, sizeof(info)))
+			return -EFAULT;
+
+		tun->info = info;
+		break;
+	}
 	default:
 		ret = -EINVAL;
 		break;
@@ -2120,6 +2136,7 @@ static long tun_chr_compat_ioctl(struct
 	case TUNSETTXFILTER:
 	case TUNGETSNDBUF:
 	case TUNSETSNDBUF:
+	case TUNSETINFO:
 	case SIOCGIFHWADDR:
 	case SIOCSIFHWADDR:
 		arg = (unsigned long)compat_ptr(arg);
@@ -2267,17 +2284,9 @@ static void tun_get_drvinfo(struct net_d
 {
 	struct tun_struct *tun = netdev_priv(dev);
 
-	strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
-	strlcpy(info->version, DRV_VERSION, sizeof(info->version));
-
-	switch (tun->flags & TUN_TYPE_MASK) {
-	case TUN_TUN_DEV:
-		strlcpy(info->bus_info, "tun", sizeof(info->bus_info));
-		break;
-	case TUN_TAP_DEV:
-		strlcpy(info->bus_info, "tap", sizeof(info->bus_info));
-		break;
-	}
+	strlcpy(info->driver, tun->info.driver, sizeof(info->driver));
+	strlcpy(info->version, tun->info.version, sizeof(info->version));
+	strlcpy(info->bus_info, tun->info.bus, sizeof(info->bus_info));
 }
 
 static u32 tun_get_msglevel(struct net_device *dev)
include/uapi/linux/if_tun.h +8 -0
@@ -56,6 +56,7 @@
 #define TUNGETVNETHDRSZ _IOR('T', 215, int)
 #define TUNSETVNETHDRSZ _IOW('T', 216, int)
 #define TUNSETQUEUE  _IOW('T', 217, int)
+#define TUNSETINFO     _IOW('T', 219, struct tun_info)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
@@ -103,4 +104,11 @@ struct tun_filter {
 	__u8   addr[0][ETH_ALEN];
 };
 
+/* Subset of ethtool_info */
+struct tun_info {
+	char driver[32];
+	char bus[32];
+	char version[32];
+};
+
 #endif /* _UAPI__IF_TUN_H */
#2 lore raw

Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info

Quoted 3 lines · Stephen Hemminger

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

I don't like this idea. Which tools are you trying to fool? How does this work when you don't implement any driver-specific behaviour (e.g. SIOCDEVPRIVATE) they expect?

Quoted 4 lines

If the application does not override the ethtool settings, the original hard coded values are used.

[...]

Quoted 12 lines
include/uapi/linux/if_tun.h +1 -0
@@ -56,6 +56,7 @@
 #define TUNGETVNETHDRSZ _IOR('T', 215, int)
 #define TUNSETVNETHDRSZ _IOW('T', 216, int)
 #define TUNSETQUEUE  _IOW('T', 217, int)
+#define TUNSETINFO     _IOW('T', 219, struct tun_info)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
@@ -103,4 +104,11 @@ struct tun_filter {
 	__u8   addr[0][ETH_ALEN];

include/uapi/linux/if_tun.h struct tun_filter { · line 114

Quoted 10 lines
include/uapi/linux/if_tun.h +7 -0
 };
 
+/* Subset of ethtool_info */
+struct tun_info {
+	char driver[32];
+	char bus[32];
+	char version[32];
+};
+
 #endif /* _UAPI__IF_TUN_H */

What about fw_version?

Ben.

Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
#3 lore raw

Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info

Quoted 8 lines · On Thu, 25 Jul 2013 00:48:07 +0100 Ben Hutchings <[email protected]>…
Quoted 3 lines · Stephen Hemminger

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

I don't like this idea. Which tools are you trying to fool? How does this work when you don't implement any driver-specific behaviour (e.g. SIOCDEVPRIVATE) they expect?

We use surrogate interfaces in user mode application and want to display different information for these than the normal TUN device.

#4 lore raw

Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info

Quoted 8 lines · Stephen Hemminger
Quoted 5 lines · On Thu, 25 Jul 2013 00:48:07 +0100 Ben Hutchings <[email protected]>…
Quoted 3 lines · Stephen Hemminger

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

Quoted 6 lines
Quoted 3 lines

I don't like this idea. Which tools are you trying to fool? How does this work when you don't implement any driver-specific behaviour (e.g. SIOCDEVPRIVATE) they expect?

We use surrogate interfaces in user mode application and want to display different information for these than the normal TUN device.

What is the problem that can't be solved purely in userland?

Ben.

Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
#5 lore raw

Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info

Quoted 13 lines · On Thu, 25 Jul 2013 22:19:17 +0100 Ben Hutchings <[email protected]>…
Quoted 12 lines · Stephen Hemminger
Quoted 8 lines · On Thu, 25 Jul 2013 00:48:07 +0100 Ben Hutchings <[email protected]>…
Quoted 3 lines · Stephen Hemminger

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

I don't like this idea. Which tools are you trying to fool? How does this work when you don't implement any driver-specific behaviour (e.g. SIOCDEVPRIVATE) they expect?

Quoted 7 lines
Quoted 2 lines

We use surrogate interfaces in user mode application and want to display different information for these than the normal TUN device.

What is the problem that can't be solved purely in userland?

Ben.

There applications (like SNMP) that use ethtool info.

#6 lore raw

Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info

Quoted 19 lines · Stephen Hemminger
Quoted 16 lines · On Thu, 25 Jul 2013 22:19:17 +0100 Ben Hutchings <[email protected]>…
Quoted 14 lines · Stephen Hemminger
Quoted 8 lines · On Thu, 25 Jul 2013 00:48:07 +0100 Ben Hutchings <[email protected]>…
Quoted 3 lines · Stephen Hemminger

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

I don't like this idea. Which tools are you trying to fool? How does this work when you don't implement any driver-specific behaviour (e.g. SIOCDEVPRIVATE) they expect?

We use surrogate interfaces in user mode application and want to display different information for these than the normal TUN device.

Quoted 6 lines
Quoted 4 lines

What is the problem that can't be solved purely in userland?

Ben.

There applications (like SNMP) that use ethtool info.

Huh, I never expected they would record the driver and version. So you want them to report the userland software name and version? I can sort of see how this might be useful as part of a test harness, but I would hate to see this get used in production. Users ought to be able to trust that 'ethtool -i' shows them the kernel driver information.

Ben.

Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
#7 lore raw

Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info

Quoted 27 lines · On Thu, 25 Jul 2013 22:36:22 +0100 Ben Hutchings <[email protected]>…
Quoted 19 lines · Stephen Hemminger
Quoted 16 lines · On Thu, 25 Jul 2013 22:19:17 +0100 Ben Hutchings <[email protected]>…
Quoted 14 lines · Stephen Hemminger
Quoted 8 lines · On Thu, 25 Jul 2013 00:48:07 +0100 Ben Hutchings <[email protected]>…
Quoted 3 lines · Stephen Hemminger

This patch adds new ioctl to allow setting the ethtool information returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation.

I don't like this idea. Which tools are you trying to fool? How does this work when you don't implement any driver-specific behaviour (e.g. SIOCDEVPRIVATE) they expect?

We use surrogate interfaces in user mode application and want to display different information for these than the normal TUN device.

Quoted 6 lines
Quoted 4 lines

What is the problem that can't be solved purely in userland?

Ben.

There applications (like SNMP) that use ethtool info.

Quoted 8 lines

Huh, I never expected they would record the driver and version. So you want them to report the userland software name and version? I can sort of see how this might be useful as part of a test harness, but I would hate to see this get used in production. Users ought to be able to trust that 'ethtool -i' shows them the kernel driver information.

Ben.

It is more the bus info.
The user mode driver knows the PCI address of the device it is controlling.

#8 lore raw

PATCH 3/3 tuntap: allow overriding link statistics

In reply to Stephen Hemminger

This patch adds new ioctl to allow overriding the link statistics returned by the TUN device. This is useful when using tun device as a surrogate for hardware or other software emulation. To use this application periodically makes ioctl(TUNSETSTATS) to update statistics.

If TUNSETSTATS is not used the original software based statistics are used.

---
v2 - don't use exchange, use regular RCU

drivers/net/tun.c +66 -0
@@ -152,6 +152,11 @@ struct tun_flow_entry {
 	unsigned long updated;
 };
 
+struct tun_link_stats {
+	struct rtnl_link_stats64 link_stats;
+	struct rcu_head rcu;
+};
+
 #define TUN_NUM_FLOW_ENTRIES 1024
 
 /* Since the socket were moved to tun_file, to preserve the behavior of persist
@@ -190,6 +195,7 @@ struct tun_struct {
 	u32 speed;
 	u8 duplex;
 	struct tun_info info;
+	struct tun_link_stats __rcu *stats;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)
@@ -803,6 +809,32 @@ static netdev_features_t tun_net_fix_fea
 
 	return (features & tun->set_features) | (features & ~TUN_USER_FEATURES);
 }
+
+static struct rtnl_link_stats64 *
+tun_net_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+	struct tun_link_stats *stats;
+
+	rcu_read_lock();
+	stats = rcu_dereference(tun->stats);
+	if (stats) {
+		/* Stats received from device */
+		*storage = stats->link_stats;
+		rcu_read_unlock();
+
+		/* Add tunnel detected errors to mix */
+		storage->tx_dropped += dev->stats.tx_dropped;
+		storage->rx_dropped += dev->stats.rx_dropped;
+		storage->rx_frame_errors += dev->stats.rx_frame_errors;
+		return storage;
+	}
+	rcu_read_unlock();
+
+	netdev_stats_to_stats64(storage, &dev->stats);
+	return storage;
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void tun_poll_controller(struct net_device *dev)
 {
@@ -825,6 +857,7 @@ static const struct net_device_ops tun_n
 	.ndo_open		= tun_net_open,
 	.ndo_stop		= tun_net_close,
 	.ndo_start_xmit		= tun_net_xmit,
+	.ndo_get_stats64	= tun_net_get_stats64,
 	.ndo_change_mtu		= tun_net_change_mtu,
 	.ndo_fix_features	= tun_net_fix_features,
 	.ndo_select_queue	= tun_select_queue,
@@ -838,6 +871,7 @@ static const struct net_device_ops tap_n
 	.ndo_open		= tun_net_open,
 	.ndo_stop		= tun_net_close,
 	.ndo_start_xmit		= tun_net_xmit,
+	.ndo_get_stats64	= tun_net_get_stats64,
 	.ndo_change_mtu		= tun_net_change_mtu,
 	.ndo_fix_features	= tun_net_fix_features,
 	.ndo_set_rx_mode	= tun_net_mclist,
@@ -1422,9 +1456,13 @@ out:
 static void tun_free_netdev(struct net_device *dev)
 {
 	struct tun_struct *tun = netdev_priv(dev);
+	struct tun_link_stats *stats;
 
 	BUG_ON(!(list_empty(&tun->disabled)));
 	tun_flow_uninit(tun);
+	stats = rtnl_dereference(tun->stats);
+	if (stats)
+		kfree_rcu(stats, rcu);
 	security_tun_dev_free_security(tun->security);
 	free_netdev(dev);
 }
@@ -1886,6 +1924,28 @@ unlock:
 	return ret;
 }
 
+static int tun_set_stats(struct tun_struct *tun, void __user *argp)
+{
+	struct tun_link_stats *stats, *old;
+
+	stats = kmalloc(sizeof(struct tun_link_stats), GFP_KERNEL);
+	if (!stats)
+		return -ENOMEM;
+
+	if (copy_from_user(&stats->link_stats, argp,
+			   sizeof(struct rtnl_link_stats64))) {
+		kfree(stats);
+		return -EFAULT;
+	}
+
+	old = rtnl_dereference(tun->stats);
+	rcu_assign_pointer(tun->stats, stats);
+	if (old)
+		kfree_rcu(old, rcu);
+
+	return 0;
+}
+
 static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg, int ifreq_len)
 {
@@ -2108,6 +2168,11 @@ static long __tun_chr_ioctl(struct file
 		tun->info = info;
 		break;
 	}
+
+	case TUNSETSTATS:
+		ret = tun_set_stats(tun, argp);
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;
@@ -2137,6 +2202,7 @@ static long tun_chr_compat_ioctl(struct
 	case TUNGETSNDBUF:
 	case TUNSETSNDBUF:
 	case TUNSETINFO:
+	case TUNSETSTATS:
 	case SIOCGIFHWADDR:
 	case SIOCSIFHWADDR:
 		arg = (unsigned long)compat_ptr(arg);
include/uapi/linux/if_tun.h +1 -0
@@ -57,6 +57,7 @@
 #define TUNSETVNETHDRSZ _IOW('T', 216, int)
 #define TUNSETQUEUE  _IOW('T', 217, int)
 #define TUNSETINFO     _IOW('T', 219, struct tun_info)
+#define TUNSETSTATS    _IOW('T', 220, struct rtnl_link_stats64)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
#9 lore raw
In reply to Stephen Hemminger
Quoted 11 lines · Stephen Hemminger

The data reported by the ethtool interface for tun/tap devices is artificial. The default speed is 10Mbit. Virtual interfaces are often faster than this nowadays, so the observed bandwidth may exceed the available bandwidth. This patch allows an administrator to change the available bandwidth as reported by ethtool.

---
v2 - allow setting duplex as well, fix patch formatting

It's logically full duplex so this doesn't make sense to me.

Quoted 1 line

ignore other settings in set

[...]

No, please never do that in ethtool_ops::set_settings. Setting unsupported values is an error.

Ben.

Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
#10 lore raw
Quoted 13 lines · On Thu, 25 Jul 2013 00:40:50 +0100 Ben Hutchings <[email protected]>…
Quoted 11 lines · Stephen Hemminger

The data reported by the ethtool interface for tun/tap devices is artificial. The default speed is 10Mbit. Virtual interfaces are often faster than this nowadays, so the observed bandwidth may exceed the available bandwidth. This patch allows an administrator to change the available bandwidth as reported by ethtool.

---
v2 - allow setting duplex as well, fix patch formatting

Quoted 7 lines

It's logically full duplex so this doesn't make sense to me.

Quoted 1 line

ignore other settings in set

[...]

No, please never do that in ethtool_ops::set_settings. Setting unsupported values is an error.

Almost every hardware driver out there allows mucking with settings that it doesn't care about (like transceiver and port). Why be picky now?

maintainer
#11 lore raw

From: Stephen Hemminger <[email protected]>
Date: Wed, 24 Jul 2013 17:17:50 -0700

Quoted 18 lines
Quoted 15 lines · On Thu, 25 Jul 2013 00:40:50 +0100 Ben Hutchings <[email protected]>…
Quoted 11 lines · Stephen Hemminger

The data reported by the ethtool interface for tun/tap devices is artificial. The default speed is 10Mbit. Virtual interfaces are often faster than this nowadays, so the observed bandwidth may exceed the available bandwidth. This patch allows an administrator to change the available bandwidth as reported by ethtool.

---
v2 - allow setting duplex as well, fix patch formatting

It's logically full duplex so this doesn't make sense to me.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

ignore other settings in set

[...]

No, please never do that in ethtool_ops::set_settings. Setting unsupported values is an error.

Almost every hardware driver out there allows mucking with settings that it doesn't care about (like transceiver and port). Why be picky now?

Those are bugs to be fixed, rather than things to emulate.

And honestly this is an unrelated thing to mention given the point Ben is trying to make.