netdev - Re: [PATCH bpf-next 3/3] bpf: tcp: Add bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7b8770e1-6cac-2e67-fb79-2feb2a35a0e5@gmail.com>
Date:   Tue, 21 Jan 2020 12:26:07 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Martin KaFai Lau <kafai@...com>, bpf@...r.kernel.org
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, kernel-team@...com,
        netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 3/3] bpf: tcp: Add bpf_cubic example



On 1/21/20 11:54 AM, Martin KaFai Lau wrote:
> This patch adds a bpf_cubic example.  Some highlights:
> 1. CONFIG_HZ kconfig is used.  For example, CONFIG_HZ is used in the usecs
>    to jiffies conversion in usecs_to_jiffies().
> 2. In bitctcp_update() [under tcp_friendliness], the original
>    "while (ca->ack_cnt > delta)" loop is changed to the equivalent
>    "ca->ack_cnt / delta" operation


...

> +	/* cubic function - calc*/
> +	/* calculate c * time^3 / rtt,
> +	 *  while considering overflow in calculation of time^3
> +	 * (so time^3 is done by using 64 bit)
> +	 * and without the support of division of 64bit numbers
> +	 * (so all divisions are done by using 32 bit)
> +	 *  also NOTE the unit of those veriables
> +	 *	  time  = (t - K) / 2^bictcp_HZ
> +	 *	  c = bic_scale >> 10
> +	 * rtt  = (srtt >> 3) / HZ
> +	 * !!! The following code does not have overflow problems,
> +	 * if the cwnd < 1 million packets !!!
> +	 */
> +
> +	t = (__s32)(tcp_jiffies32 - ca->epoch_start);
> +	t += usecs_to_jiffies(ca->delay_min);
> +	/* change the unit from HZ to bictcp_HZ */
> +	t <<= BICTCP_HZ;
> +	t /= HZ;
>

Note that this part could use usec resolution instead of jiffies
to avoid all these inlines for {u|n}secs_to_jiffies() 

	t = (__s32)(tcp_jiffies32 - ca->epoch_start) * (USEC_PER_JIFFY);
	t += ca->delay_min;
	/* change the unit from usec to bictcp_HZ */
	t <<= BICTCP_HZ;
	t /= USEC_PER_SEC;

ie :

diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 8f8eefd3a3ce116aa8fa2b7ef85c7eb503fa8da7..9ba58e95dbe6b15098bcfd045e1d0bb8874d713f 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -271,11 +271,11 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd, u32 acked)
         * if the cwnd < 1 million packets !!!
         */
 
-       t = (s32)(tcp_jiffies32 - ca->epoch_start);
-       t += usecs_to_jiffies(ca->delay_min);
-       /* change the unit from HZ to bictcp_HZ */
+       t = (s32)(tcp_jiffies32 - ca->epoch_start) * (USEC_PER_SEC / HZ);
+       t += ca->delay_min;
+       /* change the unit from usec to bictcp_HZ */
        t <<= BICTCP_HZ;
-       do_div(t, HZ);
+       do_div(t, USEC_PER_SEC);
 
        if (t < ca->bic_K)              /* t - K */
                offs = ca->bic_K - t;


But this is a minor detail.