[next,3/3] batman-adv: fix misleading default throughput warning

Message ID 1454257280-19649-3-git-send-email-mareklindner@neomailbox.ch (mailing list archive)
State Superseded, archived
Commit 41d1e618f6afb197599689097363e8114786de55
Headers

Commit Message

Marek Lindner Jan. 31, 2016, 4:21 p.m. UTC
  The default throughput value represents Mbps and not kbps.

Fixes: 5c324517 ("ELP - compute the metric based on the estimated throughput")

Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/bat_v_elp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Antonio Quartulli Feb. 1, 2016, 12:48 a.m. UTC | #1
On Mon, Feb 01, 2016 at 12:21:20AM +0800, Marek Lindner wrote:
> The default throughput value represents Mbps and not kbps.

Isn't this a bit confusing? The default throughput is expressed in multiples of
100kbps, not Mbps.

the rest looks good:
Acked-by: Antonio Quartulli <a@unstable.cc>
  
Marek Lindner Feb. 1, 2016, 1:44 a.m. UTC | #2
On Monday, February 01, 2016 08:48:26 Antonio Quartulli wrote:
> On Mon, Feb 01, 2016 at 12:21:20AM +0800, Marek Lindner wrote:
> > The default throughput value represents Mbps and not kbps.
> 
> Isn't this a bit confusing? The default throughput is expressed in multiples
> of 100kbps, not Mbps.

Prior to my patch the debug message said:
"[..] therefore defaulting to hardcoded throughput values of 1 kbit/s."

With the patch it says:
"[..] therefore defaulting to hardcoded throughput values of 1.0 Mbps."

FYI, I changed the text to "Mbps" because that is what batman-adv prints in 
the neighbor table as well in the originator table.

Cheers,
Marek
  
Antonio Quartulli Feb. 1, 2016, 2:46 a.m. UTC | #3
On Mon, Feb 01, 2016 at 09:44:39AM +0800, Marek Lindner wrote:
> On Monday, February 01, 2016 08:48:26 Antonio Quartulli wrote:
> > On Mon, Feb 01, 2016 at 12:21:20AM +0800, Marek Lindner wrote:
> > > The default throughput value represents Mbps and not kbps.
> > 
> > Isn't this a bit confusing? The default throughput is expressed in multiples
> > of 100kbps, not Mbps.
> 
> Prior to my patch the debug message said:
> "[..] therefore defaulting to hardcoded throughput values of 1 kbit/s."
> 
> With the patch it says:
> "[..] therefore defaulting to hardcoded throughput values of 1.0 Mbps."
> 
> FYI, I changed the text to "Mbps" because that is what batman-adv prints in 
> the neighbor table as well in the originator table.

The patch is fixing the way you interpret the BATADV_THROUGHPUT_DEFAULT_VALUE
constant, therefore I expected that with "The default throughput value
represents Mbps" you were stating how to properly interpret such constant (and
this wouldn't be correct).


Cheers,
  

Patch

diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 461a765..2a6a9a2 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -129,9 +129,10 @@  static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
 default_throughput:
 	if (!(hard_iface->bat_v.flags & BATADV_WARNING_DEFAULT)) {
 		batadv_info(hard_iface->soft_iface,
-			    "WiFi driver or ethtool info does not provide information about link speeds on interface %s, therefore defaulting to hardcoded throughput values of %d kbit/s. Consider overriding the throughput manually or checking your driver.\n",
+			    "WiFi driver or ethtool info does not provide information about link speeds on interface %s, therefore defaulting to hardcoded throughput values of %u.%1u Mbps. Consider overriding the throughput manually or checking your driver.\n",
 			    hard_iface->net_dev->name,
-			    BATADV_THROUGHPUT_DEFAULT_VALUE / 10);
+			    BATADV_THROUGHPUT_DEFAULT_VALUE / 10,
+			    BATADV_THROUGHPUT_DEFAULT_VALUE % 10);
 		hard_iface->bat_v.flags |= BATADV_WARNING_DEFAULT;
 	}