Hi,
Philippe Reynes sent in a patch [1] to convert the get_settings stuff to the new ethtool link_ksettings api. I've therefore asked Marek (original author of the ethtool stubs) whether the code is still needed for something.
He couldn't remember anything about it. This was therefore a good opportunity to completely get rid of the code - instead of converting not useful code to the new api.
Kind regards, Sven
[1] https://patchwork.open-mesh.org/patch/16982/
Sven Eckelmann (4): batman-adv: Use ethtool helper to get link status batman-adv: Remove ethtool msglevel functions batman-adv: Remove ethtool .get_settings stub batman-adv: Group ethtool related code together
net/batman-adv/soft-interface.c | 229 ++++++++++++++++------------------------ 1 file changed, 92 insertions(+), 137 deletions(-)
The ethtool_ops of batman-adv never contained more than a stub for the get_link function pointer. It was always returning that a link exists even when the devices was not yet up and therefore nothing resampling a link could have been available.
Instead use the ethtool helper which returns the current carrier state.
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/soft-interface.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index c5940882..583fcfca 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -69,7 +69,6 @@ static void batadv_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info); static u32 batadv_get_msglevel(struct net_device *dev); static void batadv_set_msglevel(struct net_device *dev, u32 value); -static u32 batadv_get_link(struct net_device *dev); static void batadv_get_strings(struct net_device *dev, u32 stringset, u8 *data); static void batadv_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *stats, u64 *data); @@ -80,7 +79,7 @@ static const struct ethtool_ops batadv_ethtool_ops = { .get_drvinfo = batadv_get_drvinfo, .get_msglevel = batadv_get_msglevel, .set_msglevel = batadv_set_msglevel, - .get_link = batadv_get_link, + .get_link = ethtool_op_get_link, .get_strings = batadv_get_strings, .get_ethtool_stats = batadv_get_ethtool_stats, .get_sset_count = batadv_get_sset_count, @@ -1118,11 +1117,6 @@ static void batadv_set_msglevel(struct net_device *dev, u32 value) { }
-static u32 batadv_get_link(struct net_device *dev) -{ - return 1; -} - /* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 * Declare each description string in struct.name[] to get fixed sized buffer * and compile time checking for strings longer than ETH_GSTRING_LEN.
batadv devices don't support msglevel. The ethtool stubs therefore returned that it isn't supported. But instead, the complete function can be dropped to avoid that bogus values are shown in ethtool.
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/soft-interface.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index 583fcfca..4e888c5e 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -67,8 +67,6 @@ static int batadv_get_settings(struct net_device *dev, struct ethtool_cmd *cmd); static void batadv_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info); -static u32 batadv_get_msglevel(struct net_device *dev); -static void batadv_set_msglevel(struct net_device *dev, u32 value); static void batadv_get_strings(struct net_device *dev, u32 stringset, u8 *data); static void batadv_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *stats, u64 *data); @@ -77,8 +75,6 @@ static int batadv_get_sset_count(struct net_device *dev, int stringset); static const struct ethtool_ops batadv_ethtool_ops = { .get_settings = batadv_get_settings, .get_drvinfo = batadv_get_drvinfo, - .get_msglevel = batadv_get_msglevel, - .set_msglevel = batadv_set_msglevel, .get_link = ethtool_op_get_link, .get_strings = batadv_get_strings, .get_ethtool_stats = batadv_get_ethtool_stats, @@ -1108,15 +1104,6 @@ static void batadv_get_drvinfo(struct net_device *dev, strlcpy(info->bus_info, "batman", sizeof(info->bus_info)); }
-static u32 batadv_get_msglevel(struct net_device *dev) -{ - return -EOPNOTSUPP; -} - -static void batadv_set_msglevel(struct net_device *dev, u32 value) -{ -} - /* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 * Declare each description string in struct.name[] to get fixed sized buffer * and compile time checking for strings longer than ETH_GSTRING_LEN.
The .get_settings function pointer and the related API was deprecated. Fortunately, batman-adv is a virtual interface and never provided any useful information via .get_settings. The stub can therefore be removed.
This also avoids that incorrect information is shown in ethtool about the batadv interface.
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/soft-interface.c | 19 ------------------- 1 file changed, 19 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index 4e888c5e..c2a00342 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -64,7 +64,6 @@ #include "sysfs.h" #include "translation-table.h"
-static int batadv_get_settings(struct net_device *dev, struct ethtool_cmd *cmd); static void batadv_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info); static void batadv_get_strings(struct net_device *dev, u32 stringset, u8 *data); @@ -73,7 +72,6 @@ static void batadv_get_ethtool_stats(struct net_device *dev, static int batadv_get_sset_count(struct net_device *dev, int stringset);
static const struct ethtool_ops batadv_ethtool_ops = { - .get_settings = batadv_get_settings, .get_drvinfo = batadv_get_drvinfo, .get_link = ethtool_op_get_link, .get_strings = batadv_get_strings, @@ -1078,23 +1076,6 @@ struct rtnl_link_ops batadv_link_ops __read_mostly = { .dellink = batadv_softif_destroy_netlink, };
-/* ethtool */ -static int batadv_get_settings(struct net_device *dev, struct ethtool_cmd *cmd) -{ - cmd->supported = 0; - cmd->advertising = 0; - ethtool_cmd_speed_set(cmd, SPEED_10); - 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 0; -} - static void batadv_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info) {
The ethtool code was spread in soft-interface.c. This makes reading the code and working on it unnecessary complicated. Having everything in a common place next to the other code which references it, makes it slightly easier.
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/soft-interface.c | 191 +++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 99 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index c2a00342..7f6a4d34 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -64,21 +64,6 @@ #include "sysfs.h" #include "translation-table.h"
-static void batadv_get_drvinfo(struct net_device *dev, - struct ethtool_drvinfo *info); -static void batadv_get_strings(struct net_device *dev, u32 stringset, u8 *data); -static void batadv_get_ethtool_stats(struct net_device *dev, - struct ethtool_stats *stats, u64 *data); -static int batadv_get_sset_count(struct net_device *dev, int stringset); - -static const struct ethtool_ops batadv_ethtool_ops = { - .get_drvinfo = batadv_get_drvinfo, - .get_link = ethtool_op_get_link, - .get_strings = batadv_get_strings, - .get_ethtool_stats = batadv_get_ethtool_stats, - .get_sset_count = batadv_get_sset_count, -}; - int batadv_skb_head_push(struct sk_buff *skb, unsigned int len) { int result; @@ -943,6 +928,98 @@ static const struct net_device_ops batadv_netdev_ops = { .ndo_del_slave = batadv_softif_slave_del, };
+static void batadv_get_drvinfo(struct net_device *dev, + struct ethtool_drvinfo *info) +{ + strlcpy(info->driver, "B.A.T.M.A.N. advanced", sizeof(info->driver)); + strlcpy(info->version, BATADV_SOURCE_VERSION, sizeof(info->version)); + strlcpy(info->fw_version, "N/A", sizeof(info->fw_version)); + strlcpy(info->bus_info, "batman", sizeof(info->bus_info)); +} + +/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 + * Declare each description string in struct.name[] to get fixed sized buffer + * and compile time checking for strings longer than ETH_GSTRING_LEN. + */ +static const struct { + const char name[ETH_GSTRING_LEN]; +} batadv_counters_strings[] = { + { "tx" }, + { "tx_bytes" }, + { "tx_dropped" }, + { "rx" }, + { "rx_bytes" }, + { "forward" }, + { "forward_bytes" }, + { "mgmt_tx" }, + { "mgmt_tx_bytes" }, + { "mgmt_rx" }, + { "mgmt_rx_bytes" }, + { "frag_tx" }, + { "frag_tx_bytes" }, + { "frag_rx" }, + { "frag_rx_bytes" }, + { "frag_fwd" }, + { "frag_fwd_bytes" }, + { "tt_request_tx" }, + { "tt_request_rx" }, + { "tt_response_tx" }, + { "tt_response_rx" }, + { "tt_roam_adv_tx" }, + { "tt_roam_adv_rx" }, +#ifdef CONFIG_BATMAN_ADV_DAT + { "dat_get_tx" }, + { "dat_get_rx" }, + { "dat_put_tx" }, + { "dat_put_rx" }, + { "dat_cached_reply_tx" }, +#endif +#ifdef CONFIG_BATMAN_ADV_NC + { "nc_code" }, + { "nc_code_bytes" }, + { "nc_recode" }, + { "nc_recode_bytes" }, + { "nc_buffer" }, + { "nc_decode" }, + { "nc_decode_bytes" }, + { "nc_decode_failed" }, + { "nc_sniffed" }, +#endif +}; + +static void batadv_get_strings(struct net_device *dev, u32 stringset, u8 *data) +{ + if (stringset == ETH_SS_STATS) + memcpy(data, batadv_counters_strings, + sizeof(batadv_counters_strings)); +} + +static void batadv_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data) +{ + struct batadv_priv *bat_priv = netdev_priv(dev); + int i; + + for (i = 0; i < BATADV_CNT_NUM; i++) + data[i] = batadv_sum_counter(bat_priv, i); +} + +static int batadv_get_sset_count(struct net_device *dev, int stringset) +{ + if (stringset == ETH_SS_STATS) + return BATADV_CNT_NUM; + + return -EOPNOTSUPP; +} + +static const struct ethtool_ops batadv_ethtool_ops = { + .get_drvinfo = batadv_get_drvinfo, + .get_link = ethtool_op_get_link, + .get_strings = batadv_get_strings, + .get_ethtool_stats = batadv_get_ethtool_stats, + .get_sset_count = batadv_get_sset_count, +}; + /** * batadv_softif_free - Deconstructor of batadv_soft_interface * @dev: Device to cleanup and remove @@ -1075,87 +1152,3 @@ struct rtnl_link_ops batadv_link_ops __read_mostly = { .setup = batadv_softif_init_early, .dellink = batadv_softif_destroy_netlink, }; - -static void batadv_get_drvinfo(struct net_device *dev, - struct ethtool_drvinfo *info) -{ - strlcpy(info->driver, "B.A.T.M.A.N. advanced", sizeof(info->driver)); - strlcpy(info->version, BATADV_SOURCE_VERSION, sizeof(info->version)); - strlcpy(info->fw_version, "N/A", sizeof(info->fw_version)); - strlcpy(info->bus_info, "batman", sizeof(info->bus_info)); -} - -/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 - * Declare each description string in struct.name[] to get fixed sized buffer - * and compile time checking for strings longer than ETH_GSTRING_LEN. - */ -static const struct { - const char name[ETH_GSTRING_LEN]; -} batadv_counters_strings[] = { - { "tx" }, - { "tx_bytes" }, - { "tx_dropped" }, - { "rx" }, - { "rx_bytes" }, - { "forward" }, - { "forward_bytes" }, - { "mgmt_tx" }, - { "mgmt_tx_bytes" }, - { "mgmt_rx" }, - { "mgmt_rx_bytes" }, - { "frag_tx" }, - { "frag_tx_bytes" }, - { "frag_rx" }, - { "frag_rx_bytes" }, - { "frag_fwd" }, - { "frag_fwd_bytes" }, - { "tt_request_tx" }, - { "tt_request_rx" }, - { "tt_response_tx" }, - { "tt_response_rx" }, - { "tt_roam_adv_tx" }, - { "tt_roam_adv_rx" }, -#ifdef CONFIG_BATMAN_ADV_DAT - { "dat_get_tx" }, - { "dat_get_rx" }, - { "dat_put_tx" }, - { "dat_put_rx" }, - { "dat_cached_reply_tx" }, -#endif -#ifdef CONFIG_BATMAN_ADV_NC - { "nc_code" }, - { "nc_code_bytes" }, - { "nc_recode" }, - { "nc_recode_bytes" }, - { "nc_buffer" }, - { "nc_decode" }, - { "nc_decode_bytes" }, - { "nc_decode_failed" }, - { "nc_sniffed" }, -#endif -}; - -static void batadv_get_strings(struct net_device *dev, u32 stringset, u8 *data) -{ - if (stringset == ETH_SS_STATS) - memcpy(data, batadv_counters_strings, - sizeof(batadv_counters_strings)); -} - -static void batadv_get_ethtool_stats(struct net_device *dev, - struct ethtool_stats *stats, u64 *data) -{ - struct batadv_priv *bat_priv = netdev_priv(dev); - int i; - - for (i = 0; i < BATADV_CNT_NUM; i++) - data[i] = batadv_sum_counter(bat_priv, i); -} - -static int batadv_get_sset_count(struct net_device *dev, int stringset) -{ - if (stringset == ETH_SS_STATS) - return BATADV_CNT_NUM; - - return -EOPNOTSUPP; -}
On Saturday, April 1, 2017 2:47:02 PM HKT Sven Eckelmann wrote:
Sven Eckelmann (4): batman-adv: Use ethtool helper to get link status batman-adv: Remove ethtool msglevel functions batman-adv: Remove ethtool .get_settings stub batman-adv: Group ethtool related code together
For this patch series:
Acked-by: Marek Lindner mareklindner@neomailbox.ch
On Saturday, April 1, 2017 2:47:02 PM CEST Sven Eckelmann wrote:
Hi,
Philippe Reynes sent in a patch [1] to convert the get_settings stuff to the new ethtool link_ksettings api. I've therefore asked Marek (original author of the ethtool stubs) whether the code is still needed for something.
He couldn't remember anything about it. This was therefore a good opportunity to completely get rid of the code - instead of converting not useful code to the new api.
Merged in 9e8d507d..080d75e5.
Thanks!! Simon
b.a.t.m.a.n@lists.open-mesh.org
-
Marek Lindner -
Simon Wunderlich -
Sven Eckelmann