| Message ID | 20130724161156.4a8cf02b@nehalam.linuxnetplumber.net |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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.
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/net/tun.c 2013-07-24 11:24:55.543040360 -0700 +++ b/drivers/net/tun.c 2013-07-24 11:33:06.759433743 -0700 @@ -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,