Re: TUN/TAP: tap driver reports bogus interface speed in ethtool operations

* Re: TUN/TAP: tap driver reports bogus interface speed in ethtool operations
       [not found]       ` <51E5BE27.3060207@qti.qualcomm.com>
@ 2013-07-23 13:32         ` Helmut Grohne
  2013-07-23 15:49           ` Rick Jones
                             ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Helmut Grohne @ 2013-07-23 13:32 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Max Krasnyansky

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]


On 16.07.2013, at 23:41, Max Krasnyansky <maxk@qti.qualcomm.com> wrote:
> The patch looks fine to me (must admit that I only glanced at it). Please send it to the netdev
> mailing list netdev@vger.kernel.org, if you have't already done so.
> CC David Miller <davem@davemloft.net> and me.

Doing that. Now summarizing the issue for the new recipients:

Problem:

When querying a tap device for its speed using ethtool the tun driver reports a speed
of SPEED_10. This number is hard coded into the driver. Nowadays virtual network devices
can go way faster than that. When doing automatic bandwidth monitoring based on the
speed reported by ethtool, tap devices tend to come up as false positives. Arguably the
hard coded speed is wrong.

Proposed solution:

To that end I propose supporting the ETHTOOL_SSET command in addition to the already
supported ETHTOOL_GSET. It would deny setting any setting except for the bandwidth
where it would allow arbitrary values. You can find this patch attached.

With this patch an administrator can increase the reported speed for tap devices and
keep using automatic detection of interface speeds.

Workarounds:

Using ethtool a detection utility can determine the driver for an interface. If the
driver matches the string "tun", the reported speed should not be used.

Helmut


[-- Attachment #2: 0001-tuntap-allow-changing-ethtool-speed.patch --]
[-- Type: application/octet-stream, Size: 2816 bytes --]

From 9e0d2928ebed1d74ec92d131dcdc7511dde867ac Mon Sep 17 00:00:00 2001
From: Helmut Grohne <h.grohne@cygnusnetworks.de>
Date: Fri, 17 May 2013 14:03:47 +0200
Subject: [PATCH] tuntap: allow changing ethtool speed

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.

Signed-off-by: Helmut Grohne <h.grohne@cygnusnetworks.de>
---
 drivers/net/tun.c |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index f042b03..2555a11 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -187,6 +187,7 @@ struct tun_struct {
 	struct list_head disabled;
 	void *security;
 	u32 flow_count;
+	u32 advertised_speed;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)
@@ -1404,6 +1405,7 @@ static void tun_setup(struct net_device *dev)
 
 	tun->owner = INVALID_UID;
 	tun->group = INVALID_GID;
+	tun->advertised_speed = SPEED_10;
 
 	dev->ethtool_ops = &tun_ethtool_ops;
 	dev->destructor = tun_free_netdev;
@@ -2197,9 +2199,11 @@ 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);
+	ethtool_cmd_speed_set(cmd, tun->advertised_speed);
 	cmd->duplex		= DUPLEX_FULL;
 	cmd->port		= PORT_TP;
 	cmd->phy_address	= 0;
@@ -2210,6 +2214,24 @@ static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	return 0;
 }
 
+static int tun_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+
+	if(cmd->supported != 0 ||
+			cmd->advertising != 0 ||
+			cmd->duplex != DUPLEX_FULL ||
+			cmd->port != PORT_TP ||
+			cmd->phy_address != 0 ||
+			cmd->transceiver != XCVR_INTERNAL ||
+			cmd->autoneg != AUTONEG_DISABLE ||
+			cmd->maxtxpkt != 0 ||
+			cmd->maxrxpkt != 0)
+		return -EOPNOTSUPP;
+	tun->advertised_speed = ethtool_cmd_speed(cmd);
+	return 0;
+}
+
 static void tun_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 {
 	struct tun_struct *tun = netdev_priv(dev);
@@ -2247,6 +2269,7 @@ static void tun_set_msglevel(struct net_device *dev, u32 value)
 
 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.7.10.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread