* [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-24 23:11 [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2) Stephen Hemminger
@ 2013-07-24 23:13 ` Stephen Hemminger
2013-07-24 23:48 ` Ben Hutchings
2013-07-24 23:15 ` [PATCH net-next 3/3] tuntap: allow overriding link statistics Stephen Hemminger
2013-07-24 23:40 ` [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2) Ben Hutchings
2 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2013-07-24 23:13 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Helmut Grohne, David Miller, netdev
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.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/tun.c | 56 +++++++++++++++++++++++++++++++++-----------
include/uapi/linux/if_tun.h | 15 +++++++++++
2 files changed, 58 insertions(+), 13 deletions(-)
--- a/drivers/net/tun.c 2013-07-24 11:33:06.759433743 -0700
+++ b/drivers/net/tun.c 2013-07-24 11:39:56.233298852 -0700
@@ -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)
--- a/include/uapi/linux/if_tun.h 2013-07-24 11:24:55.543040360 -0700
+++ b/include/uapi/linux/if_tun.h 2013-07-24 11:35:54.620898504 -0700
@@ -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 */
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-24 23:13 ` [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info Stephen Hemminger
@ 2013-07-24 23:48 ` Ben Hutchings
2013-07-25 0:16 ` Stephen Hemminger
0 siblings, 1 reply; 12+ messages in thread
From: Ben Hutchings @ 2013-07-24 23:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Helmut Grohne, David Miller, netdev
On Wed, 2013-07-24 at 16:13 -0700, Stephen Hemminger wrote:
> 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?
> If the application does not override the ethtool settings, the
> original hard coded values are used.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
[...]
> --- a/include/uapi/linux/if_tun.h 2013-07-24 11:24:55.543040360 -0700
> +++ b/include/uapi/linux/if_tun.h 2013-07-24 11:35:54.620898504 -0700
> @@ -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 */
What about fw_version?
Ben.
--
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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-24 23:48 ` Ben Hutchings
@ 2013-07-25 0:16 ` Stephen Hemminger
2013-07-25 21:19 ` Ben Hutchings
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2013-07-25 0:16 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Helmut Grohne, David Miller, netdev
On Thu, 25 Jul 2013 00:48:07 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Wed, 2013-07-24 at 16:13 -0700, Stephen Hemminger wrote:
> > 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-25 0:16 ` Stephen Hemminger
@ 2013-07-25 21:19 ` Ben Hutchings
2013-07-25 21:32 ` Stephen Hemminger
0 siblings, 1 reply; 12+ messages in thread
From: Ben Hutchings @ 2013-07-25 21:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Helmut Grohne, David Miller, netdev
On Wed, 2013-07-24 at 17:16 -0700, Stephen Hemminger wrote:
> On Thu, 25 Jul 2013 00:48:07 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>
> > On Wed, 2013-07-24 at 16:13 -0700, Stephen Hemminger wrote:
> > > 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.
What is the problem that can't be solved purely in userland?
Ben.
--
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-25 21:19 ` Ben Hutchings
@ 2013-07-25 21:32 ` Stephen Hemminger
2013-07-25 21:36 ` Ben Hutchings
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2013-07-25 21:32 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Helmut Grohne, David Miller, netdev
On Thu, 25 Jul 2013 22:19:17 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Wed, 2013-07-24 at 17:16 -0700, Stephen Hemminger wrote:
> > On Thu, 25 Jul 2013 00:48:07 +0100
> > Ben Hutchings <bhutchings@solarflare.com> wrote:
> >
> > > On Wed, 2013-07-24 at 16:13 -0700, Stephen Hemminger wrote:
> > > > 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.
>
> What is the problem that can't be solved purely in userland?
>
> Ben.
>
There applications (like SNMP) that use ethtool info.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-25 21:32 ` Stephen Hemminger
@ 2013-07-25 21:36 ` Ben Hutchings
2013-07-25 21:56 ` Stephen Hemminger
0 siblings, 1 reply; 12+ messages in thread
From: Ben Hutchings @ 2013-07-25 21:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Helmut Grohne, David Miller, netdev
On Thu, 2013-07-25 at 14:32 -0700, Stephen Hemminger wrote:
> On Thu, 25 Jul 2013 22:19:17 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>
> > On Wed, 2013-07-24 at 17:16 -0700, Stephen Hemminger wrote:
> > > On Thu, 25 Jul 2013 00:48:07 +0100
> > > Ben Hutchings <bhutchings@solarflare.com> wrote:
> > >
> > > > On Wed, 2013-07-24 at 16:13 -0700, Stephen Hemminger wrote:
> > > > > 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.
> >
> > 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.
--
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info
2013-07-25 21:36 ` Ben Hutchings
@ 2013-07-25 21:56 ` Stephen Hemminger
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2013-07-25 21:56 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Helmut Grohne, David Miller, netdev
On Thu, 25 Jul 2013 22:36:22 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Thu, 2013-07-25 at 14:32 -0700, Stephen Hemminger wrote:
> > On Thu, 25 Jul 2013 22:19:17 +0100
> > Ben Hutchings <bhutchings@solarflare.com> wrote:
> >
> > > On Wed, 2013-07-24 at 17:16 -0700, Stephen Hemminger wrote:
> > > > On Thu, 25 Jul 2013 00:48:07 +0100
> > > > Ben Hutchings <bhutchings@solarflare.com> wrote:
> > > >
> > > > > On Wed, 2013-07-24 at 16:13 -0700, Stephen Hemminger wrote:
> > > > > > 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.
> > >
> > > 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.
>
It is more the bus info.
The user mode driver knows the PCI address of the device it is controlling.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next 3/3] tuntap: allow overriding link statistics
2013-07-24 23:11 [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2) Stephen Hemminger
2013-07-24 23:13 ` [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info Stephen Hemminger
@ 2013-07-24 23:15 ` Stephen Hemminger
2013-07-24 23:40 ` [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2) Ben Hutchings
2 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2013-07-24 23:15 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Helmut Grohne, David Miller, netdev
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.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2 - don't use exchange, use regular RCU
--- a/drivers/net/tun.c 2013-07-24 12:47:37.344206628 -0700
+++ b/drivers/net/tun.c 2013-07-24 16:14:00.964658040 -0700
@@ -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);
--- a/include/uapi/linux/if_tun.h 2013-07-24 12:47:37.344206628 -0700
+++ b/include/uapi/linux/if_tun.h 2013-07-24 12:47:37.352206531 -0700
@@ -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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2)
2013-07-24 23:11 [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2) Stephen Hemminger
2013-07-24 23:13 ` [PATCH net-next 2/3] tuntap: allow overriding ethtool driver info Stephen Hemminger
2013-07-24 23:15 ` [PATCH net-next 3/3] tuntap: allow overriding link statistics Stephen Hemminger
@ 2013-07-24 23:40 ` Ben Hutchings
2013-07-25 0:17 ` Stephen Hemminger
2 siblings, 1 reply; 12+ messages in thread
From: Ben Hutchings @ 2013-07-24 23:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Helmut Grohne, David Miller, netdev
On Wed, 2013-07-24 at 16:11 -0700, Stephen Hemminger wrote:
> 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>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
> ---
> v2 - allow setting duplex as well, fix patch formatting
It's logically full duplex so this doesn't make sense to me.
> ignore other settings in set
[...]
No, please never do that in ethtool_ops::set_settings. Setting
unsupported values is an error.
Ben.
--
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2)
2013-07-24 23:40 ` [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2) Ben Hutchings
@ 2013-07-25 0:17 ` Stephen Hemminger
2013-07-25 0:29 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2013-07-25 0:17 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Helmut Grohne, David Miller, netdev
On Thu, 25 Jul 2013 00:40:50 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Wed, 2013-07-24 at 16:11 -0700, Stephen Hemminger wrote:
> > 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>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> > ---
> > v2 - allow setting duplex as well, fix patch formatting
>
> It's logically full duplex so this doesn't make sense to me.
>
> > 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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 1/3] tuntap: allow changing ethtool speed (v2)
2013-07-25 0:17 ` Stephen Hemminger
@ 2013-07-25 0:29 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-07-25 0:29 UTC (permalink / raw)
To: stephen; +Cc: bhutchings, h.grohne, netdev
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 24 Jul 2013 17:17:50 -0700
> On Thu, 25 Jul 2013 00:40:50 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>
>> On Wed, 2013-07-24 at 16:11 -0700, Stephen Hemminger wrote:
>> > 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>
>> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> >
>> > ---
>> > v2 - allow setting duplex as well, fix patch formatting
>>
>> It's logically full duplex so this doesn't make sense to me.
>>
>> > 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.
^ permalink raw reply [flat|nested] 12+ messages in thread