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.
