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: <20200121220020.mg6iekrxywnh2nhq@kafai-mbp.dhcp.thefacebook.com>
Date:   Tue, 21 Jan 2020 22:00:25 +0000
From:   Martin Lau <kafai@...com>
To:     Eric Dumazet <eric.dumazet@...il.com>
CC:     "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Kernel Team <Kernel-team@...com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next 3/3] bpf: tcp: Add bpf_cubic example

On Tue, Jan 21, 2020 at 12:26:07PM -0800, Eric Dumazet wrote:
> 
> 
> 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.
Thanks for the suggestion and the corresponding explanation in tcp_cubic.c

USEC_PER_JIFFY is simpler than my current approach.  I will give it a spin.