Also, adds support for the first flag, LRO.
Earlier version of this patch posted about 12 hours ago.
commit 84bf82f38dacde0fa86986e23c061728e819810e
Author: Jeff Garzik <[email protected]>
Date: Fri Aug 10 05:27:23 2007 -0400
[ETHTOOL] Add ETHTOOL_[GS]FLAGS sub-ioctls
Signed-off-by: Jeff Garzik <[email protected]>
include/linux/ethtool.h | 21 +++++++++++++++
include/linux/netdevice.h | 1
net/core/ethtool.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+)
84bf82f38dacde0fa86986e23c061728e819810e
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 23ccea8..0e5de2f 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -256,6 +256,19 @@ struct ethtool_perm_addr {
__u8 data[0];
};
+/* boolean flags controlling per-interface behavior characteristics.
+ * When reading, the flag indicates whether or not a certain behavior
+ * is enabled/present. When writing, the flag indicates whether
+ * or not the driver should turn on (set) or off (clear) a behavior.
+ *
+ * Some behaviors may read-only (unconditionally absent or present).
+ * If such is the case, return EINVAL in the set-flags operation if the
+ * flag differs from the read-only value.
+ */
+enum ethtool_flags {
+ ETH_FLAG_LRO = (1 << 15), /* LRO is enabled */
+};
+
#ifdef __KERNEL__
struct net_device;
@@ -272,6 +285,8 @@ u32 ethtool_op_get_tso(struct net_device *dev);
int ethtool_op_set_tso(struct net_device *dev, u32 data);
u32 ethtool_op_get_ufo(struct net_device *dev);
int ethtool_op_set_ufo(struct net_device *dev, u32 data);
+u32 ethtool_op_get_flags(struct net_device *dev);
+int ethtool_op_set_flags(struct net_device *dev, u32 data);
/**
* ðtool_ops - Alter and report network device settings
@@ -307,6 +322,8 @@ int ethtool_op_set_ufo(struct net_device *dev, u32 data);
* get_strings: Return a set of strings that describe the requested objects
* phys_id: Identify the device
* get_stats: Return statistics about the device
+ * get_flags: get 32-bit flags bitmap
+ * set_flags: set 32-bit flags bitmap
*
* Description:
*
@@ -369,6 +386,8 @@ struct ethtool_ops {
void (*complete)(struct net_device *);
u32 (*get_ufo)(struct net_device *);
int (*set_ufo)(struct net_device *, u32);
+ u32 (*get_flags)(struct net_device *);
+ int (*set_flags)(struct net_device *, u32);
};
#endif /* __KERNEL__ */
@@ -410,6 +429,8 @@ struct ethtool_ops {
#define ETHTOOL_SUFO 0x00000022 /* Set UFO enable (ethtool_value) */
#define ETHTOOL_GGSO 0x00000023 /* Get GSO enable (ethtool_value) */
#define ETHTOOL_SGSO 0x00000024 /* Set GSO enable (ethtool_value) */
+#define ETHTOOL_GFLAGS 0x00000025 /* Get flags bitmap(ethtool_value) */
+#define ETHTOOL_SFLAGS 0x00000026 /* Set flags bitmap(ethtool_value) */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4a616d7..559a4dc 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -341,6 +341,7 @@ struct net_device
#define NETIF_F_GSO 2048 /* Enable software GSO. */
#define NETIF_F_LLTX 4096 /* LockLess TX */
#define NETIF_F_MULTI_QUEUE 16384 /* Has multiple TX/RX queues */
+#define NETIF_F_LRO 32768 /* large receive offload */
/* Segmentation offload features */
#define NETIF_F_GSO_SHIFT 16
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 2ab0a60..6e8563e 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -109,6 +109,32 @@ int ethtool_op_set_ufo(struct net_device *dev, u32 data)
return 0;
}
+/* the following list of flags are the same as their associated
+ * NETIF_F_xxx values in include/linux/netdevice.h
+ */
+static const u32 flags_dup_features =
+ ETH_FLAG_LRO;
+
+u32 ethtool_op_get_flags(struct net_device *dev)
+{
+ /* in the future, this function will probably contain additional
+ * handling for flags which are not so easily handled
+ * by a simple masking operation
+ */
+
+ return dev->features & flags_dup_features;
+}
+
+int ethtool_op_set_flags(struct net_device *dev, u32 data)
+{
+ if (data & ETH_FLAG_LRO)
+ dev->features |= NETIF_F_LRO;
+ else
+ dev->features &= ~NETIF_F_LRO;
+
+ return 0;
+}
+
/* Handlers for each ethtool command */
static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
@@ -783,6 +809,33 @@ static int ethtool_get_perm_addr(struct net_device *dev, void __user *useraddr)
return 0;
}
+static int ethtool_get_flags(struct net_device *dev, char __user *useraddr)
+{
+ struct ethtool_value edata = { ETHTOOL_GFLAGS };
+
+ if (!dev->ethtool_ops->get_flags)
+ return -EOPNOTSUPP;
+
+ edata.data = dev->ethtool_ops->get_flags(dev);
+
+ if (copy_to_user(useraddr, &edata, sizeof(edata)))
+ return -EFAULT;
+ return 0;
+}
+
+static int ethtool_set_flags(struct net_device *dev, char __user *useraddr)
+{
+ struct ethtool_value edata;
+
+ if (!dev->ethtool_ops->set_flags)
+ return -EOPNOTSUPP;
+
+ if (copy_from_user(&edata, useraddr, sizeof(edata)))
+ return -EFAULT;
+
+ return dev->ethtool_ops->set_flags(dev, edata.data);
+}
+
/* The main entry point in this file. Called from net/core/dev.c */
int dev_ethtool(struct ifreq *ifr)
@@ -935,6 +988,12 @@ int dev_ethtool(struct ifreq *ifr)
case ETHTOOL_SGSO:
rc = ethtool_set_gso(dev, useraddr);
break;
+ case ETHTOOL_GFLAGS:
+ rc = ethtool_get_flags(dev, useraddr);
+ break;
+ case ETHTOOL_SFLAGS:
+ rc = ethtool_set_flags(dev, useraddr);
+ break;
default:
rc = -EOPNOTSUPP;
}
@@ -960,3 +1019,5 @@ EXPORT_SYMBOL(ethtool_op_set_tx_hw_csum);
EXPORT_SYMBOL(ethtool_op_set_tx_ipv6_csum);
EXPORT_SYMBOL(ethtool_op_set_ufo);
EXPORT_SYMBOL(ethtool_op_get_ufo);
+EXPORT_SYMBOL(ethtool_op_set_flags);
+EXPORT_SYMBOL(ethtool_op_get_flags);
Required prep for the priv-flags changeset (next patch), which also uses
ethtool string sets.
commit 86fe0ff220a795c82aa9dea5dc4b7359519a1366
Author: Jeff Garzik <[email protected]>
Date: Fri Aug 10 15:49:21 2007 -0400
[ETHTOOL] Introduce get_sset_count. Obsolete get_stats_count, self_test_count
Signed-off-by: Jeff Garzik <[email protected]>
include/linux/ethtool.h | 7 ++-
net/core/ethtool.c | 95 +++++++++++++++++++++++++++++++++++-------------
2 files changed, 75 insertions(+), 27 deletions(-)
86fe0ff220a795c82aa9dea5dc4b7359519a1366
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 0e5de2f..f617998 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -376,11 +376,9 @@ struct ethtool_ops {
int (*set_sg)(struct net_device *, u32);
u32 (*get_tso)(struct net_device *);
int (*set_tso)(struct net_device *, u32);
- int (*self_test_count)(struct net_device *);
void (*self_test)(struct net_device *, struct ethtool_test *, u64 *);
void (*get_strings)(struct net_device *, u32 stringset, u8 *);
int (*phys_id)(struct net_device *, u32);
- int (*get_stats_count)(struct net_device *);
void (*get_ethtool_stats)(struct net_device *, struct ethtool_stats *, u64 *);
int (*begin)(struct net_device *);
void (*complete)(struct net_device *);
@@ -388,6 +386,11 @@ struct ethtool_ops {
int (*set_ufo)(struct net_device *, u32);
u32 (*get_flags)(struct net_device *);
int (*set_flags)(struct net_device *, u32);
+ int (*get_sset_count)(struct net_device *, int);
+
+ /* the following hooks are obsolete */
+ int (*self_test_count)(struct net_device *);/* use get_sset_count */
+ int (*get_stats_count)(struct net_device *);/* use get_sset_count */
};
#endif /* __KERNEL__ */
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6e8563e..2b366a5 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -179,10 +179,23 @@ static int ethtool_get_drvinfo(struct net_device *dev, void __user *useraddr)
info.cmd = ETHTOOL_GDRVINFO;
ops->get_drvinfo(dev, &info);
- if (ops->self_test_count)
- info.testinfo_len = ops->self_test_count(dev);
- if (ops->get_stats_count)
- info.n_stats = ops->get_stats_count(dev);
+ if (ops->get_sset_count) {
+ int rc;
+
+ rc = ops->get_sset_count(dev, ETH_SS_TEST);
+ if (rc >= 0)
+ info.testinfo_len = rc;
+ rc = ops->get_sset_count(dev, ETH_SS_STATS);
+ if (rc >= 0)
+ info.n_stats = rc;
+ } else {
+ /* code path for obsolete hooks */
+
+ if (ops->self_test_count)
+ info.testinfo_len = ops->self_test_count(dev);
+ if (ops->get_stats_count)
+ info.n_stats = ops->get_stats_count(dev);
+ }
if (ops->get_regs_len)
info.regdump_len = ops->get_regs_len(dev);
if (ops->get_eeprom_len)
@@ -669,16 +682,27 @@ static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
struct ethtool_test test;
const struct ethtool_ops *ops = dev->ethtool_ops;
u64 *data;
- int ret;
+ int ret, test_len;
- if (!ops->self_test || !ops->self_test_count)
+ if (!ops->self_test)
+ return -EOPNOTSUPP;
+ if (!ops->get_sset_count && !ops->self_test_count)
return -EOPNOTSUPP;
+ if (ops->get_sset_count)
+ test_len = ops->get_sset_count(dev, ETH_SS_TEST);
+ else
+ /* code path for obsolete hook */
+ test_len = ops->self_test_count(dev);
+ if (test_len < 0)
+ return test_len;
+ WARN_ON(test_len == 0);
+
if (copy_from_user(&test, useraddr, sizeof(test)))
return -EFAULT;
- test.len = ops->self_test_count(dev);
- data = kmalloc(test.len * sizeof(u64), GFP_USER);
+ test.len = test_len;
+ data = kmalloc(test_len * sizeof(u64), GFP_USER);
if (!data)
return -ENOMEM;
@@ -710,19 +734,29 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
if (copy_from_user(&gstrings, useraddr, sizeof(gstrings)))
return -EFAULT;
- switch (gstrings.string_set) {
- case ETH_SS_TEST:
- if (!ops->self_test_count)
- return -EOPNOTSUPP;
- gstrings.len = ops->self_test_count(dev);
- break;
- case ETH_SS_STATS:
- if (!ops->get_stats_count)
- return -EOPNOTSUPP;
- gstrings.len = ops->get_stats_count(dev);
- break;
- default:
- return -EINVAL;
+ if (ops->get_sset_count) {
+ ret = ops->get_sset_count(dev, gstrings.string_set);
+ if (ret < 0)
+ return ret;
+
+ gstrings.len = ret;
+ } else {
+ /* code path for obsolete hooks */
+
+ switch (gstrings.string_set) {
+ case ETH_SS_TEST:
+ if (!ops->self_test_count)
+ return -EOPNOTSUPP;
+ gstrings.len = ops->self_test_count(dev);
+ break;
+ case ETH_SS_STATS:
+ if (!ops->get_stats_count)
+ return -EOPNOTSUPP;
+ gstrings.len = ops->get_stats_count(dev);
+ break;
+ default:
+ return -EINVAL;
+ }
}
data = kmalloc(gstrings.len * ETH_GSTRING_LEN, GFP_USER);
@@ -762,16 +796,27 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
struct ethtool_stats stats;
const struct ethtool_ops *ops = dev->ethtool_ops;
u64 *data;
- int ret;
+ int ret, n_stats;
- if (!ops->get_ethtool_stats || !ops->get_stats_count)
+ if (!ops->get_ethtool_stats)
return -EOPNOTSUPP;
+ if (!ops->get_sset_count && !ops->get_stats_count)
+ return -EOPNOTSUPP;
+
+ if (ops->get_sset_count)
+ n_stats = ops->get_sset_count(dev, ETH_SS_STATS);
+ else
+ /* code path for obsolete hook */
+ n_stats = ops->get_stats_count(dev);
+ if (n_stats < 0)
+ return n_stats;
+ WARN_ON(n_stats == 0);
if (copy_from_user(&stats, useraddr, sizeof(stats)))
return -EFAULT;
- stats.n_stats = ops->get_stats_count(dev);
- data = kmalloc(stats.n_stats * sizeof(u64), GFP_USER);
+ stats.n_stats = n_stats;
+ data = kmalloc(n_stats * sizeof(u64), GFP_USER);
if (!data)
return -ENOMEM;
Driver-private flags. Driver writer's guide will follow in a reply to this
patch.
commit 4901236cec047029b970261b95e47d6be60f523e
Author: Jeff Garzik <[email protected]>
Date: Fri Aug 10 15:52:06 2007 -0400
[ETHTOOL] Introduce ->{get,set}_priv_flags, ETHTOOL_[GS]PFLAGS
Signed-off-by: Jeff Garzik <[email protected]>
include/linux/ethtool.h | 8 +++++++-
net/core/ethtool.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 1 deletion(-)
4901236cec047029b970261b95e47d6be60f523e
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f617998..71d4ada 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -39,7 +39,8 @@ struct ethtool_drvinfo {
char bus_info[ETHTOOL_BUSINFO_LEN]; /* Bus info for this IF. */
/* For PCI devices, use pci_name(pci_dev). */
char reserved1[32];
- char reserved2[16];
+ char reserved2[12];
+ __u32 n_priv_flags; /* number of flags valid in ETHTOOL_GPFLAGS */
__u32 n_stats; /* number of u64's from ETHTOOL_GSTATS */
__u32 testinfo_len;
__u32 eedump_len; /* Size of data from ETHTOOL_GEEPROM (bytes) */
@@ -219,6 +220,7 @@ struct ethtool_pauseparam {
enum ethtool_stringset {
ETH_SS_TEST = 0,
ETH_SS_STATS,
+ ETH_SS_PRIV_FLAGS,
};
/* for passing string sets for data tagging */
@@ -386,6 +388,8 @@ struct ethtool_ops {
int (*set_ufo)(struct net_device *, u32);
u32 (*get_flags)(struct net_device *);
int (*set_flags)(struct net_device *, u32);
+ u32 (*get_priv_flags)(struct net_device *);
+ int (*set_priv_flags)(struct net_device *, u32);
int (*get_sset_count)(struct net_device *, int);
/* the following hooks are obsolete */
@@ -434,6 +438,8 @@ struct ethtool_ops {
#define ETHTOOL_SGSO 0x00000024 /* Set GSO enable (ethtool_value) */
#define ETHTOOL_GFLAGS 0x00000025 /* Get flags bitmap(ethtool_value) */
#define ETHTOOL_SFLAGS 0x00000026 /* Set flags bitmap(ethtool_value) */
+#define ETHTOOL_GPFLAGS 0x00000027 /* Get driver-private flags bitmap */
+#define ETHTOOL_SPFLAGS 0x00000028 /* Set driver-private flags bitmap */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 2b366a5..f721e38 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -188,6 +188,9 @@ static int ethtool_get_drvinfo(struct net_device *dev, void __user *useraddr)
rc = ops->get_sset_count(dev, ETH_SS_STATS);
if (rc >= 0)
info.n_stats = rc;
+ rc = ops->get_sset_count(dev, ETH_SS_PRIV_FLAGS);
+ if (rc >= 0)
+ info.n_priv_flags = rc;
} else {
/* code path for obsolete hooks */
@@ -881,6 +884,33 @@ static int ethtool_set_flags(struct net_device *dev, char __user *useraddr)
return dev->ethtool_ops->set_flags(dev, edata.data);
}
+static int ethtool_get_priv_flags(struct net_device *dev, char __user *useraddr)
+{
+ struct ethtool_value edata = { ETHTOOL_GPFLAGS };
+
+ if (!dev->ethtool_ops->get_priv_flags)
+ return -EOPNOTSUPP;
+
+ edata.data = dev->ethtool_ops->get_priv_flags(dev);
+
+ if (copy_to_user(useraddr, &edata, sizeof(edata)))
+ return -EFAULT;
+ return 0;
+}
+
+static int ethtool_set_priv_flags(struct net_device *dev, char __user *useraddr)
+{
+ struct ethtool_value edata;
+
+ if (!dev->ethtool_ops->set_priv_flags)
+ return -EOPNOTSUPP;
+
+ if (copy_from_user(&edata, useraddr, sizeof(edata)))
+ return -EFAULT;
+
+ return dev->ethtool_ops->set_priv_flags(dev, edata.data);
+}
+
/* The main entry point in this file. Called from net/core/dev.c */
int dev_ethtool(struct ifreq *ifr)
@@ -915,6 +945,8 @@ int dev_ethtool(struct ifreq *ifr)
case ETHTOOL_GPERMADDR:
case ETHTOOL_GUFO:
case ETHTOOL_GGSO:
+ case ETHTOOL_GFLAGS:
+ case ETHTOOL_GPFLAGS:
break;
default:
if (!capable(CAP_NET_ADMIN))
@@ -1039,6 +1071,12 @@ int dev_ethtool(struct ifreq *ifr)
case ETHTOOL_SFLAGS:
rc = ethtool_set_flags(dev, useraddr);
break;
+ case ETHTOOL_GPFLAGS:
+ rc = ethtool_get_priv_flags(dev, useraddr);
+ break;
+ case ETHTOOL_SPFLAGS:
+ rc = ethtool_set_priv_flags(dev, useraddr);
+ break;
default:
rc = -EOPNOTSUPP;
}
commit 758522226b3e7758a84e721998bce8f63855caca
Author: Jeff Garzik <[email protected]>
Date: Fri Aug 10 16:17:09 2007 -0400
[ETHTOOL] Internal cleanup of ethtool_value-related handlers
Several get/set functions can be handled by a passing the ethtool_op
function pointer directly to a generic function. This permits deletion
of a fair bit of redundant code.
Signed-off-by: Jeff Garzik <[email protected]>
net/core/ethtool.c | 199 ++++++++++-------------------------------------------
1 file changed, 39 insertions(+), 160 deletions(-)
758522226b3e7758a84e721998bce8f63855caca
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index f721e38..4559200 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -272,34 +272,6 @@ static int ethtool_set_wol(struct net_device *dev, char __user *useraddr)
return dev->ethtool_ops->set_wol(dev, &wol);
}
-static int ethtool_get_msglevel(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GMSGLVL };
-
- if (!dev->ethtool_ops->get_msglevel)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_msglevel(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
-static int ethtool_set_msglevel(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata;
-
- if (!dev->ethtool_ops->set_msglevel)
- return -EOPNOTSUPP;
-
- if (copy_from_user(&edata, useraddr, sizeof(edata)))
- return -EFAULT;
-
- dev->ethtool_ops->set_msglevel(dev, edata.data);
- return 0;
-}
-
static int ethtool_nway_reset(struct net_device *dev)
{
if (!dev->ethtool_ops->nway_reset)
@@ -308,20 +280,6 @@ static int ethtool_nway_reset(struct net_device *dev)
return dev->ethtool_ops->nway_reset(dev);
}
-static int ethtool_get_link(struct net_device *dev, void __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GLINK };
-
- if (!dev->ethtool_ops->get_link)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_link(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
{
struct ethtool_eeprom eeprom;
@@ -489,48 +447,6 @@ static int ethtool_set_pauseparam(struct net_device *dev, void __user *useraddr)
return dev->ethtool_ops->set_pauseparam(dev, &pauseparam);
}
-static int ethtool_get_rx_csum(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GRXCSUM };
-
- if (!dev->ethtool_ops->get_rx_csum)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_rx_csum(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
-static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata;
-
- if (!dev->ethtool_ops->set_rx_csum)
- return -EOPNOTSUPP;
-
- if (copy_from_user(&edata, useraddr, sizeof(edata)))
- return -EFAULT;
-
- dev->ethtool_ops->set_rx_csum(dev, edata.data);
- return 0;
-}
-
-static int ethtool_get_tx_csum(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GTXCSUM };
-
- if (!dev->ethtool_ops->get_tx_csum)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_tx_csum(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
static int __ethtool_set_sg(struct net_device *dev, u32 data)
{
int err;
@@ -569,20 +485,6 @@ static int ethtool_set_tx_csum(struct net_device *dev, char __user *useraddr)
return dev->ethtool_ops->set_tx_csum(dev, edata.data);
}
-static int ethtool_get_sg(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GSG };
-
- if (!dev->ethtool_ops->get_sg)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_sg(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
static int ethtool_set_sg(struct net_device *dev, char __user *useraddr)
{
struct ethtool_value edata;
@@ -600,20 +502,6 @@ static int ethtool_set_sg(struct net_device *dev, char __user *useraddr)
return __ethtool_set_sg(dev, edata.data);
}
-static int ethtool_get_tso(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GTSO };
-
- if (!dev->ethtool_ops->get_tso)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_tso(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
static int ethtool_set_tso(struct net_device *dev, char __user *useraddr)
{
struct ethtool_value edata;
@@ -630,18 +518,6 @@ static int ethtool_set_tso(struct net_device *dev, char __user *useraddr)
return dev->ethtool_ops->set_tso(dev, edata.data);
}
-static int ethtool_get_ufo(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GUFO };
-
- if (!dev->ethtool_ops->get_ufo)
- return -EOPNOTSUPP;
- edata.data = dev->ethtool_ops->get_ufo(dev);
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
- return 0;
-}
-
static int ethtool_set_ufo(struct net_device *dev, char __user *useraddr)
{
struct ethtool_value edata;
@@ -857,58 +733,48 @@ static int ethtool_get_perm_addr(struct net_device *dev, void __user *useraddr)
return 0;
}
-static int ethtool_get_flags(struct net_device *dev, char __user *useraddr)
+static int ethtool_get_value(struct net_device *dev, char __user *useraddr,
+ u32 cmd, u32 (*actor)(struct net_device *))
{
- struct ethtool_value edata = { ETHTOOL_GFLAGS };
+ struct ethtool_value edata = { cmd };
- if (!dev->ethtool_ops->get_flags)
+ if (!actor)
return -EOPNOTSUPP;
- edata.data = dev->ethtool_ops->get_flags(dev);
+ edata.data = actor(dev);
if (copy_to_user(useraddr, &edata, sizeof(edata)))
return -EFAULT;
return 0;
}
-static int ethtool_set_flags(struct net_device *dev, char __user *useraddr)
+static int ethtool_set_value_void(struct net_device *dev, char __user *useraddr,
+ void (*actor)(struct net_device *, u32))
{
struct ethtool_value edata;
- if (!dev->ethtool_ops->set_flags)
+ if (!actor)
return -EOPNOTSUPP;
if (copy_from_user(&edata, useraddr, sizeof(edata)))
return -EFAULT;
- return dev->ethtool_ops->set_flags(dev, edata.data);
-}
-
-static int ethtool_get_priv_flags(struct net_device *dev, char __user *useraddr)
-{
- struct ethtool_value edata = { ETHTOOL_GPFLAGS };
-
- if (!dev->ethtool_ops->get_priv_flags)
- return -EOPNOTSUPP;
-
- edata.data = dev->ethtool_ops->get_priv_flags(dev);
-
- if (copy_to_user(useraddr, &edata, sizeof(edata)))
- return -EFAULT;
+ actor(dev, edata.data);
return 0;
}
-static int ethtool_set_priv_flags(struct net_device *dev, char __user *useraddr)
+static int ethtool_set_value(struct net_device *dev, char __user *useraddr,
+ int (*actor)(struct net_device *, u32))
{
struct ethtool_value edata;
- if (!dev->ethtool_ops->set_priv_flags)
+ if (!actor)
return -EOPNOTSUPP;
if (copy_from_user(&edata, useraddr, sizeof(edata)))
return -EFAULT;
- return dev->ethtool_ops->set_priv_flags(dev, edata.data);
+ return actor(dev, edata.data);
}
/* The main entry point in this file. Called from net/core/dev.c */
@@ -979,16 +845,19 @@ int dev_ethtool(struct ifreq *ifr)
rc = ethtool_set_wol(dev, useraddr);
break;
case ETHTOOL_GMSGLVL:
- rc = ethtool_get_msglevel(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_msglevel);
break;
case ETHTOOL_SMSGLVL:
- rc = ethtool_set_msglevel(dev, useraddr);
+ rc = ethtool_set_value_void(dev, useraddr,
+ dev->ethtool_ops->set_msglevel);
break;
case ETHTOOL_NWAY_RST:
rc = ethtool_nway_reset(dev);
break;
case ETHTOOL_GLINK:
- rc = ethtool_get_link(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_link);
break;
case ETHTOOL_GEEPROM:
rc = ethtool_get_eeprom(dev, useraddr);
@@ -1015,25 +884,30 @@ int dev_ethtool(struct ifreq *ifr)
rc = ethtool_set_pauseparam(dev, useraddr);
break;
case ETHTOOL_GRXCSUM:
- rc = ethtool_get_rx_csum(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_rx_csum);
break;
case ETHTOOL_SRXCSUM:
- rc = ethtool_set_rx_csum(dev, useraddr);
+ rc = ethtool_set_value(dev, useraddr,
+ dev->ethtool_ops->set_rx_csum);
break;
case ETHTOOL_GTXCSUM:
- rc = ethtool_get_tx_csum(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_tx_csum);
break;
case ETHTOOL_STXCSUM:
rc = ethtool_set_tx_csum(dev, useraddr);
break;
case ETHTOOL_GSG:
- rc = ethtool_get_sg(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_sg);
break;
case ETHTOOL_SSG:
rc = ethtool_set_sg(dev, useraddr);
break;
case ETHTOOL_GTSO:
- rc = ethtool_get_tso(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_tso);
break;
case ETHTOOL_STSO:
rc = ethtool_set_tso(dev, useraddr);
@@ -1054,7 +928,8 @@ int dev_ethtool(struct ifreq *ifr)
rc = ethtool_get_perm_addr(dev, useraddr);
break;
case ETHTOOL_GUFO:
- rc = ethtool_get_ufo(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_ufo);
break;
case ETHTOOL_SUFO:
rc = ethtool_set_ufo(dev, useraddr);
@@ -1066,16 +941,20 @@ int dev_ethtool(struct ifreq *ifr)
rc = ethtool_set_gso(dev, useraddr);
break;
case ETHTOOL_GFLAGS:
- rc = ethtool_get_flags(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_flags);
break;
case ETHTOOL_SFLAGS:
- rc = ethtool_set_flags(dev, useraddr);
+ rc = ethtool_set_value(dev, useraddr,
+ dev->ethtool_ops->set_flags);
break;
case ETHTOOL_GPFLAGS:
- rc = ethtool_get_priv_flags(dev, useraddr);
+ rc = ethtool_get_value(dev, useraddr, ethcmd,
+ dev->ethtool_ops->get_priv_flags);
break;
case ETHTOOL_SPFLAGS:
- rc = ethtool_set_priv_flags(dev, useraddr);
+ rc = ethtool_set_value(dev, useraddr,
+ dev->ethtool_ops->set_priv_flags);
break;
default:
rc = -EOPNOTSUPP;
Jeff Garzik wrote:
> commit 4901236cec047029b970261b95e47d6be60f523e
> Author: Jeff Garzik <[email protected]>
> Date: Fri Aug 10 15:52:06 2007 -0400
>
> [ETHTOOL] Introduce ->{get,set}_priv_flags, ETHTOOL_[GS]PFLAGS
>
> Signed-off-by: Jeff Garzik <[email protected]>
>
> include/linux/ethtool.h | 8 +++++++-
> net/core/ethtool.c | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+), 1 deletion(-)
This change permits driver writers to allow userspace to enable/disable
driver-specific boolean options on a per-interface basis. This is best
illustrated by describing how userland uses this:
(this is very similar to how the get-stats and self-test operations
currently work)
1) Userland issues ETHTOOL_GDRVINFO, to obtain the n_priv_flags value.
2) Userland issues ETHTOOL_GSTRINGS (ETH_SS_PRIV_FLAGS) to obtain an
array of strings. Each element in this array maps to a bit in the list
of driver-private flags. Array element #0 is the tag (name) for bit #0.
Array element #5 is the tag for bit #5. etc.
If we are getting (retrieving) flags:
3) Userland issues ETHTOOL_GPFLAGS, to obtain a 32-bit bitmap
4) Userland prints out a tag returned from ETHTOOL_GSTRINGS
for each bit set to one in the bitmap. If a bit is set,
but there is no string to describe it, that bit is ignored.
(i.e. a list of 5 strings is returned, but bit 24 is set)
If we are setting flags:
3) Userland issues ETHTOOL_GPFLAGS, to obtain 32-bit bitmap
4) Userland parses command line, which has a series of strings
in the format of "+option" (set flag) and "-option" (clear
flag).
5) Userland sets and clears bits in the 32-bit bitmap, by
matching the command line-supplied data with strings returned
by ETHTOOL_GSTRINGS.
6) If the bitmap has changed, send it back to the driver using
ETHTOOL_SPFLAGS.
In this way, you can see that the actual bit numbers are irrelevant, and
need not be a fixed part of the ABI. Everything is named-based.
This name-based stuff only applies to the private flags. The
ETHTOOL_[GS]FLAGS operations /do/ fix the bit numbers in stone, in the
ABI. There is no name list associated with ETHTOOL_[GS]FLAGS]. The
userland interface does, however, use names rather than bit numbers, for
ease of use.
Another attribute is worth pointing out at this point: ETHTOOL_SFLAGS
and ETHTOOL_SPFLAGS both permit atomic retrieval/setting of groups of
features all at once. IMO this is a nice addition to ethtool as well,
where previously you had to issue separate ioctls for each feature,
which might result in repeated NIC resets [depending on driver
implementation].
Get-flags and get-priv-flags are unpriveleged operations. Set-flags and
set-priv-flags require CAP_NET_ADMIN like other ethtool sub-ioctls.
Drivers should return -EINVAL if userland attempts to set an invariant
(read-only) flag to something other than its constant value.
All this is currently checked into the 'eflags' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
But when everybody is happy with it, IMO we should get it into
net-2.6.24.git, as it enables LRO.
Jeff
Jeff Garzik wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 4a616d7..559a4dc 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -341,6 +341,7 @@ struct net_device
> #define NETIF_F_GSO 2048 /* Enable software GSO. */
> #define NETIF_F_LLTX 4096 /* LockLess TX */
> #define NETIF_F_MULTI_QUEUE 16384 /* Has multiple TX/RX queues */
> +#define NETIF_F_LRO 32768 /* large receive offload */
>
> /* Segmentation offload features */
> #define NETIF_F_GSO_SHIFT 16
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 2ab0a60..6e8563e 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -109,6 +109,32 @@ int ethtool_op_set_ufo(struct net_device *dev, u32 data)
> return 0;
> }
>
> +/* the following list of flags are the same as their associated
> + * NETIF_F_xxx values in include/linux/netdevice.h
> + */
> +static const u32 flags_dup_features =
> + ETH_FLAG_LRO;
> +
> +u32 ethtool_op_get_flags(struct net_device *dev)
> +{
> + /* in the future, this function will probably contain additional
> + * handling for flags which are not so easily handled
> + * by a simple masking operation
> + */
> +
> + return dev->features & flags_dup_features;
> +}
> +
> +int ethtool_op_set_flags(struct net_device *dev, u32 data)
> +{
> + if (data & ETH_FLAG_LRO)
> + dev->features |= NETIF_F_LRO;
> + else
> + dev->features &= ~NETIF_F_LRO;
> +
And, a side discussion:
This patch copies Auke in adding NETIF_F_LRO. Is that just for
temporary merging, or does the net core really not touch it at all?
Because, logically, if NETIF_F_LRO exists nowhere else but this patch,
we should not add it to dev->features. LRO knowledge can be contained
entirely within the driver, if the net core never tests NETIF_F_LRO.
I haven't reviewed the other NETIF_F_XXX flags, but, that logic can be
applied to any other NETIF_F_XXX flag: if the net stack isn't using it,
it's a piece of information specific to that driver.
Jeff
Rick Jones wrote:
>> If we are getting (retrieving) flags:
>>
>> 3) Userland issues ETHTOOL_GPFLAGS, to obtain a 32-bit bitmap
>>
>> 4) Userland prints out a tag returned from ETHTOOL_GSTRINGS
>> for each bit set to one in the bitmap. If a bit is set,
>> but there is no string to describe it, that bit is ignored.
>> (i.e. a list of 5 strings is returned, but bit 24 is set)
>
> Is that to enable "hidden" bits? If not I'd think that emitting some
> sort of "UNKNOWN_FLAG" might help flush-out little oopses like
> forgetting a string.
No purpose other than attempting to define sane edge case behavior.
The bitmap interface is completely invisible to the user (who sees UTF8
strings on the command line), and given the interface I felt the easiest
thing to do would be to define an "ignore any garbage" default policy.
Due to the way the interface is designed, each userland user must be
aware of the precise number of valid bits in the bitmap /anyway/, thus
making clipping/ignoring garbage bits almost a trivial side effect.
Jeff
Jeff Garzik wrote:
> This patch copies Auke in adding NETIF_F_LRO. Is that just for
> temporary merging, or does the net core really not touch it at all?
>
> Because, logically, if NETIF_F_LRO exists nowhere else but this patch,
> we should not add it to dev->features. LRO knowledge can be contained
> entirely within the driver, if the net core never tests NETIF_F_LRO.
>
> I haven't reviewed the other NETIF_F_XXX flags, but, that logic can be
> applied to any other NETIF_F_XXX flag: if the net stack isn't using it,
> it's a piece of information specific to that driver.
I believe LRO is going to have to be disabled for routing/bridging,
so the stack will probably need to become aware of it at some point...
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
> If we are getting (retrieving) flags:
>
> 3) Userland issues ETHTOOL_GPFLAGS, to obtain a 32-bit bitmap
>
> 4) Userland prints out a tag returned from ETHTOOL_GSTRINGS
> for each bit set to one in the bitmap. If a bit is set,
> but there is no string to describe it, that bit is ignored.
> (i.e. a list of 5 strings is returned, but bit 24 is set)
Is that to enable "hidden" bits? If not I'd think that emitting some
sort of "UNKNOWN_FLAG" might help flush-out little oopses like
forgetting a string.
rick jones
From: Ben Greear <[email protected]>
Date: Fri, 10 Aug 2007 14:11:24 -0700
> Jeff Garzik wrote:
>
> > This patch copies Auke in adding NETIF_F_LRO. Is that just for
> > temporary merging, or does the net core really not touch it at all?
> >
> > Because, logically, if NETIF_F_LRO exists nowhere else but this patch,
> > we should not add it to dev->features. LRO knowledge can be contained
> > entirely within the driver, if the net core never tests NETIF_F_LRO.
> >
> > I haven't reviewed the other NETIF_F_XXX flags, but, that logic can be
> > applied to any other NETIF_F_XXX flag: if the net stack isn't using it,
> > it's a piece of information specific to that driver.
>
> I believe LRO is going to have to be disabled for routing/bridging,
> so the stack will probably need to become aware of it at some point...
The packet will be GSO'd on output I believe, so it won't
break anything.
Alternatively, we could make the driver only LRO accumulate if the
packet is unicast and matches one of the MAC's programmed into the
chip.
David Miller wrote:
> From: Ben Greear <[email protected]>
>> I believe LRO is going to have to be disabled for routing/bridging,
>> so the stack will probably need to become aware of it at some point...
>
> The packet will be GSO'd on output I believe, so it won't
> break anything.
>
> Alternatively, we could make the driver only LRO accumulate if the
> packet is unicast and matches one of the MAC's programmed into the
> chip.
I think even this would fail if you are doing something clever with
NAT or other iptables stuff. Probably we're going to have to put this
in the hands of the users..who hopefully can determine whether they
can allow LRO or not...
For GSO on output, is there a generic fallback for any driver that
does not specifically implement GSO?
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
From: Ben Greear <[email protected]>
Date: Fri, 10 Aug 2007 15:40:02 -0700
> For GSO on output, is there a generic fallback for any driver that
> does not specifically implement GSO?
Absolutely, in fact that's mainly what it's there for.
I don't think there is any issue. The knob is there via
ethtool for people who really want to disable it.
David Miller wrote:
> From: Ben Greear <[email protected]>
> Date: Fri, 10 Aug 2007 15:40:02 -0700
>
>
>>For GSO on output, is there a generic fallback for any driver that
>>does not specifically implement GSO?
>
>
> Absolutely, in fact that's mainly what it's there for.
>
> I don't think there is any issue. The knob is there via
> ethtool for people who really want to disable it.
Just to be paranoid (who me?) we are then at a point where what happened
a couple months ago with forwarding between 10G and IPoIB won't happen
again - where things failed because a 10G NIC had LRO enabled by default?
rick jones
Rick Jones wrote:
> David Miller wrote:
>> From: Ben Greear <[email protected]>
>> Date: Fri, 10 Aug 2007 15:40:02 -0700
>>
>>
>>> For GSO on output, is there a generic fallback for any driver that
>>> does not specifically implement GSO?
>>
>> Absolutely, in fact that's mainly what it's there for.
>>
>> I don't think there is any issue. The knob is there via
>> ethtool for people who really want to disable it.
>
> Just to be paranoid (who me?) we are then at a point where what happened
> a couple months ago with forwarding between 10G and IPoIB won't happen
> again - where things failed because a 10G NIC had LRO enabled by default?
we still have the NETIF_F_LRO flag which Jeff will keep around. Perhaps the
IPoIB code can force this to _off_ when setting it up? (or at least warn about it).
Auke
From: Jeff Garzik <[email protected]>
Date: Fri, 10 Aug 2007 16:56:17 -0400
> All this is currently checked into the 'eflags' branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
>
> But when everybody is happy with it, IMO we should get it into
> net-2.6.24.git, as it enables LRO.
I've backed out the ETHTOOL LRO stuff from Auke, and applied your
patches to net-2.6.24
We can get rid of that NETIF_F_LRO thing, since as you observed
it is indeed superfluous.