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

RFC New Helmut Grohne 4 participants 9 messages Jul 23, 2013 – Jul 30, 2013 5 messages not in the archive

Subsystems (3)

Networking Network drivers TUN/TAP driver

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Helmut Grohne

author

2013-07-23 13:42 UTC #5 lore raw

Quoted 3 lines · Max Krasnyansky

The patch looks fine to me (must admit that I only glanced at it). Please send it to the netdev mailing list [email protected], if you have't already done so.
CC David Miller <[email protected]> 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

From 9e0d2928ebed1d74ec92d131dcdc7511dde867ac Mon Sep 17 00:00:00 2001
From: Helmut Grohne <[email protected]>
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 <[email protected]>

---
 drivers/net/tun.c |   25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

drivers/net/tun.c +24 -1

index f042b03..2555a11 100644
@@ -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,

Signature · 1 line

1.7.10.4

Rick Jones

2013-07-23 15:49 UTC #6 lore raw

Quoted 25 lines · Helmut Grohne

Quoted 3 lines · Max Krasnyansky
The patch looks fine to me (must admit that I only glanced at it). Please send it to the netdev mailing list [email protected], if you have't already done so.
CC David Miller <[email protected]> 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.

Quoted 7 lines

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

I like the idea of being able to set the reported speed for a tun/tap interface.

rick jones

Stephen Hemminger

2013-07-23 15:56 UTC #7 lore raw

Quoted 31 lines · On Tue, 23 Jul 2013 08:49:01 -0700 Rick Jones <[email protected]> wrote:

Quoted 30 lines · Helmut Grohne
Quoted 3 lines · Max Krasnyansky
The patch looks fine to me (must admit that I only glanced at it). Please send it to the netdev mailing list [email protected], if you have't already done so.
CC David Miller <[email protected]> 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.

Quoted 7 lines

Quoted 2 lines
Helmut
I like the idea of being able to set the reported speed for a tun/tap interface.
rick jones

I have hack patch to allow overwriting speed, info, and statistics that is part of how we use TUN as dummy devices. How far do you think makes sense?

Helmut Grohne

author

2013-07-23 16:03 UTC #8 lore raw

Quoted 2 lines · Stephen Hemminger

I have hack patch to allow overwriting speed, info, and statistics that is part of how we use TUN as dummy devices. How far do you think makes sense?

That could prove useful for creating test environments for tools, that use ethtool. If you have a better patch, then go for it. The reason for why I did not include other settings was that I don't need them. I'm open to other solutions as well, as long as they either make the speed modifiable or something faster than SPEED_10.

Helmut

Stephen Hemminger

2013-07-23 17:29 UTC #9 lore raw

RFC 1/2 tun: allow overrriding ethtool info

In reply to Helmut Grohne

This patch adds new ioctl to allow overrriding 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.

This version is compile tested only, included to show how API works.

Signed-off-by Stephen Hemminger <[email protected]>

---
 drivers/net/tun.c           |   56 +++++++++++++++++++++++++++++++++-----------
 include/uapi/linux/if_tun.h |   15 +++++++++++
 2 files changed, 58 insertions(+), 13 deletions(-)

drivers/net/tun.c +38 ~~-13~~

@@ -187,6 +187,8 @@ struct tun_struct {
 	struct list_head disabled;
 	void *security;
 	u32 flow_count;
+
+	struct tun_info info;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)

@@ -870,9 +872,12 @@ 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));
+
 	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;

@@ -887,6 +892,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;

@@ -1426,6 +1433,9 @@ static void tun_setup(struct net_device
 
 	tun->owner = INVALID_UID;
 	tun->group = INVALID_GID;
+	tun->info.speed = SPEED_10;
+	tun->info.duplex = DUPLEX_FULL;
+	tun->info.port = PORT_TP;
 
 	dev->ethtool_ops = &tun_ethtool_ops;
 	dev->destructor = tun_free_netdev;

@@ -2088,6 +2098,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(argp, &info, sizeof(info)))
+			ret = -EFAULT;
+
+		tun->info = info;
+		break;
+	}
+
 	default:
 		ret = -EINVAL;
 		break;

@@ -2229,11 +2248,13 @@ 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;
-	cmd->port		= PORT_TP;
+	ethtool_cmd_speed_set(cmd, tun->info.speed);
+	cmd->duplex		= tun->info.duplex;
+	cmd->port		= tun->info.port;
 	cmd->phy_address	= 0;
 	cmd->transceiver	= XCVR_INTERNAL;
 	cmd->autoneg		= AUTONEG_DISABLE;

@@ -2242,21 +2263,24 @@ static int tun_get_settings(struct net_d
 	return 0;
 }
 
+static int tun_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+
+	tun->info.speed  = ethtool_cmd_speed(ecmd);
+	tun->info.duplex = ecmd->duplex;
+	tun->info.port   = ecmd->port;
+
+	return 0;
+}
+
 static void tun_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 {
 	struct tun_struct *tun = netdev_priv(dev);
 
-	strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
+	strlcpy(info->driver, tun->info.driver, 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->bus_info, tun->info.bus, sizeof(info->bus_info));
 }
 
 static u32 tun_get_msglevel(struct net_device *dev)

@@ -2279,6 +2303,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,

include/uapi/linux/if_tun.h +14 -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,17 @@ struct tun_filter {
 	__u8   addr[0][ETH_ALEN];
 };
 
+/*
+ * Ethtool info
+ * This is used to allow spoofing the speed/duplex and driver information
+ */
+struct tun_info {
+	__u32 speed;
+	__u8  duplex;
+	__u8  port;
+
+	char driver[32];
+	char bus[32];
+};
+
 #endif /* _UAPI__IF_TUN_H */

Stephen Hemminger

2013-07-23 17:29 UTC #10 lore raw

RFC 2/2 tun: allow overriding statistics

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

The application perodically uses ioctl(TUNSETSTTS) to periodically update statistics. Exchange and RCU is used to make this operation atomic.

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

This version compile tested only, to show how API works.

Signed-off-by Stephen Hemminger <[email protected]>

drivers/net/tun.c +62 -0

@@ -189,6 +189,10 @@ struct tun_struct {
 	u32 flow_count;
 
 	struct tun_info info;
+	struct tun_stats __rcu {
+		struct rtnl_link_stats64 link_stats;
+		struct rcu_head rcu;
+	} *stats;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)

@@ -802,6 +806,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_stats __rcu *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)
 {

@@ -824,6 +854,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,

@@ -837,6 +868,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,

@@ -1420,9 +1452,13 @@ out:
 static void tun_free_netdev(struct net_device *dev)
 {
 	struct tun_struct *tun = netdev_priv(dev);
+	struct tun_stats *stats;
 
 	BUG_ON(!(list_empty(&tun->disabled)));
 	tun_flow_uninit(tun);
+	stats = xchg(&tun->stats, NULL);
+	if (stats)
+		kfree_rcu(stats, rcu);
 	security_tun_dev_free_security(tun->security);
 	free_netdev(dev);
 }

@@ -1885,6 +1921,28 @@ unlock:
 	return ret;
 }
 
+static int tun_set_stats(struct tun_struct *tun, struct ifreq *ifr)
+{
+	struct tun_stats *stats, *old;
+
+	stats = kmalloc(sizeof(struct tun_stats), GFP_KERNEL);
+	if (!stats)
+		return -ENOMEM;
+
+	if (copy_from_user(&stats->link_stats,
+			   ifr->ifr_ifru.ifru_data,
+			   sizeof(struct rtnl_link_stats64))) {
+		kfree(stats);
+		return -EFAULT;
+	}
+
+	old = xchg(&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)
 {

@@ -2107,6 +2165,10 @@ static long __tun_chr_ioctl(struct file
 		break;
 	}
 
+	case TUNSETSTATS:
+		ret = tun_set_stats(tun, &ifr);
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;

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

Ben Hutchings

2013-07-23 19:04 UTC #11 lore raw

In reply to Helmut Grohne

Quoted 15 lines · Helmut Grohne

Quoted 3 lines · Max Krasnyansky
The patch looks fine to me (must admit that I only glanced at it). Please send it to the netdev mailing list [email protected], if you have't already done so.
CC David Miller <[email protected]> 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.

Quoted 5 lines

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.

Yes, I think this makes some sense.

[...]

Quoted 2 lines

drivers/net/tun.c +0 -0

[...]

Quoted 21 lines

drivers/net/tun.c +3 -1

@@ -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;

drivers/net/tun.c +3 -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);

drivers/net/tun.c static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd) · line 2229

Quoted 10 lines

drivers/net/tun.c +10 -0

+
+       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)

This is formatted wrongly - needs a space after 'if' and all the continuation lines should be lined up under 'cmd->supported'.

drivers/net/tun.c static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd) · line 2230

Quoted 1 line

drivers/net/tun.c +1 -0

+               return -EOPNOTSUPP;

EINVAL

drivers/net/tun.c static int tun_get_settings(struct net_device *dev, struct ethtool_cmd *cmd) · line 2237

Quoted 7 lines

drivers/net/tun.c +4 -0

+       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);

[...]

I notice the port type is reported as PORT_TP. Perhaps that should also be changed to PORT_NONE (in a separate patch)?

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.

Helmut Grohne

author

2013-07-30 06:20 UTC #12 lore raw

Thanks for your review! Given the v2 patch Stephen Hemminger posted, I think that his series is where development should continue.

Quoted 2 lines · Ben Hutchings

This is formatted wrongly - needs a space after 'if' and all the continuation lines should be lined up under 'cmd->supported'.

I am sorry for not having run checkpatch and will try to remember next time.

Quoted 3 lines

Quoted 1 line
+ return -EOPNOTSUPP;
EINVAL

Makes sense, once you are pointed to it.

Quoted 2 lines

I notice the port type is reported as PORT_TP. Perhaps that should also be changed to PORT_NONE (in a separate patch)?

This appears not yet addressed in the v2 patch yet.

I am not sure how to go on here. Clearly the v2 patch set, is an improvement over my initial version. The additional capabilities of changing the reported driver and similar aspects appear controversial though. Would it make sense to split this into two patch sets to be handled independently?

Helmut

Ben Hutchings

2013-07-30 10:51 UTC #13 lore raw

Quoted 18 lines · Helmut Grohne

Thanks for your review! Given the v2 patch Stephen Hemminger posted, I think that his series is where development should continue.
Quoted 2 lines · Ben Hutchings
This is formatted wrongly - needs a space after 'if' and all the continuation lines should be lined up under 'cmd->supported'.
I am sorry for not having run checkpatch and will try to remember next time.
Quoted 3 lines
Quoted 1 line
+ return -EOPNOTSUPP;
EINVAL
Makes sense, once you are pointed to it.
Quoted 2 lines
I notice the port type is reported as PORT_TP. Perhaps that should also be changed to PORT_NONE (in a separate patch)?

Quoted 6 lines

This appears not yet addressed in the v2 patch yet.
I am not sure how to go on here. Clearly the v2 patch set, is an improvement over my initial version. The additional capabilities of changing the reported driver and similar aspects appear controversial though. Would it make sense to split this into two patch sets to be handled independently?

Yes, I think that would make sense.

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.