* [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup
@ 2017-04-01 12:47 Sven Eckelmann
2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: Use ethtool helper to get link status Sven Eckelmann
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Sven Eckelmann @ 2017-04-01 12:47 UTC (permalink / raw)
To: b.a.t.m.a.n
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(-)
--
2.11.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: Use ethtool helper to get link status 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann @ 2017-04-01 12:47 ` Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Remove ethtool msglevel functions Sven Eckelmann ` (4 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Sven Eckelmann @ 2017-04-01 12:47 UTC (permalink / raw) To: b.a.t.m.a.n 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. -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Remove ethtool msglevel functions 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: Use ethtool helper to get link status Sven Eckelmann @ 2017-04-01 12:47 ` Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Remove ethtool .get_settings stub Sven Eckelmann ` (3 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Sven Eckelmann @ 2017-04-01 12:47 UTC (permalink / raw) To: b.a.t.m.a.n 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. -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Remove ethtool .get_settings stub 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: Use ethtool helper to get link status Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Remove ethtool msglevel functions Sven Eckelmann @ 2017-04-01 12:47 ` Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: Group ethtool related code together Sven Eckelmann ` (2 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Sven Eckelmann @ 2017-04-01 12:47 UTC (permalink / raw) To: b.a.t.m.a.n 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) { -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: Group ethtool related code together 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann ` (2 preceding siblings ...) 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Remove ethtool .get_settings stub Sven Eckelmann @ 2017-04-01 12:47 ` Sven Eckelmann 2017-04-01 15:35 ` [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Marek Lindner 2017-04-03 16:12 ` Simon Wunderlich 5 siblings, 0 replies; 7+ messages in thread From: Sven Eckelmann @ 2017-04-01 12:47 UTC (permalink / raw) To: b.a.t.m.a.n 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; -} -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann ` (3 preceding siblings ...) 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: Group ethtool related code together Sven Eckelmann @ 2017-04-01 15:35 ` Marek Lindner 2017-04-03 16:12 ` Simon Wunderlich 5 siblings, 0 replies; 7+ messages in thread From: Marek Lindner @ 2017-04-01 15:35 UTC (permalink / raw) To: b.a.t.m.a.n [-- Attachment #1: Type: text/plain, Size: 370 bytes --] 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> [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann ` (4 preceding siblings ...) 2017-04-01 15:35 ` [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Marek Lindner @ 2017-04-03 16:12 ` Simon Wunderlich 5 siblings, 0 replies; 7+ messages in thread From: Simon Wunderlich @ 2017-04-03 16:12 UTC (permalink / raw) To: b.a.t.m.a.n [-- Attachment #1: Type: text/plain, Size: 534 bytes --] 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 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-03 16:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-01 12:47 [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 1/4] batman-adv: Use ethtool helper to get link status Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 2/4] batman-adv: Remove ethtool msglevel functions Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 3/4] batman-adv: Remove ethtool .get_settings stub Sven Eckelmann 2017-04-01 12:47 ` [B.A.T.M.A.N.] [PATCH 4/4] batman-adv: Group ethtool related code together Sven Eckelmann 2017-04-01 15:35 ` [B.A.T.M.A.N.] [PATCH 0/4] batman-adv: ethtool code cleanup Marek Lindner 2017-04-03 16:12 ` Simon Wunderlich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox