Re: [B.A.T.M.A.N.] [PATCH v2 6/7] batman-adv: ELP - use tp meter to estimate the throughput if otherwise not available

["Linus Lüssing" <linus.luessing@c0d3.blue>]

On Fri, May 18, 2018 at 09:47:53AM +0800, Marek Lindner wrote:
> @@ -251,6 +253,21 @@ static void batadv_tp_caller_notify(struct batadv_priv *bat_priv,
>  
>  		break;
>  	case BATADV_TP_ELP:
> +		if (reason_is_error) {
> +			batadv_v_elp_tp_fail(tp_vars->hardif_neigh);
> +			return;
> +		}
> +
> +		test_time = jiffies_to_msecs(jiffies - tp_vars->start_time);
> +		total_bytes = atomic64_read(&tp_vars->tot_sent);
> +
> +		/* The following calculation includes these steps:
> +		 * - convert bytes to bits
> +		 * - divide bits by the test length (msecs)
> +		 * - convert result from bits/ms to 0.1Mb/s (* 1024 * 10 / 1000)
> +		 */
> +		throughput = total_bytes * 8 >> ilog2(test_time) / 10;
> +		batadv_v_elp_tp_finish(tp_vars->hardif_neigh, throughput);

I find the throughput calculation quite hard to read here, would
it be possible to put this into an extra (inline?) function?

Also the comment for the "convert result..." seems wrong,
"[bits/ms]*1024*10/1000" would be 0.01Mb/s, not 0.1Mb/s?

What is the advantage of using the ilog2 and shift operator here
compared to plain multiplications and divisions?

Also, when trying this in a small C program I get weird results:

-----
#include <stdio.h>
#include <math.h>

int main()
{
        unsigned long test_time = 10000;        // 10s
        unsigned long total_bytes = 20000000;   // 16MBit/s
        unsigned long throughput, throughput2;
        unsigned long log_test_time = log(test_time) / log(2);

        throughput = total_bytes * 8 >> log_test_time / 10;

	// Straightforward approach?
        throughput2 = total_bytes * 8 / test_time * 1000 / 1024 / 100;

        printf("Result: %lu (log_test_time: %lu)\n", throughput, log_test_time);
        printf("Result2: %lu\n", throughput2);

        return 0;
}
-----
$ ./test
Result: 80000000 (log_test_time: 13)
Result2: 156
$ file ./test
./test: ELF 32-bit LSB pie executable ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, BuildID[sha1]=d18f32829cdd2bc42cf744cdcafde7cdbd315cb0, not stripped
-----


Regards, Linus