public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Yang Yingliang <yangyingliang@huawei•com>
To: David Laight <David.Laight@ACULAB•COM>, <davem@davemloft•net>,
	<netdev@vger•kernel.org>
Cc: <eric.dumazet@gmail•com>, <brouer@redhat•com>,
	<jpirko@redhat•com>, <jbrouer@redhat•com>
Subject: Re: [PATCH net v6 1/2] net: sched: tbf: fix the calculation of max_size
Date: Mon, 9 Dec 2013 10:26:37 +0800	[thread overview]
Message-ID: <52A52A5D.5060601@huawei.com> (raw)
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7468@saturn3.aculab.com>

On 2013/12/6 18:56, David Laight wrote:
>> From: Yang Yingliang
> ...
>>
>> +/* Time to Length, convert time in ns to length in bytes
>> + * to determinate how many bytes can be sent in given time.
>> + */
>> +static u64 psched_ns_t2l(const struct psched_ratecfg *r,
>> +			 u64 time_in_ns)
>> +{
>> +	/* The formula is :
>> +	 * len = (time_in_ns * r->rate_bytes_ps) / NSEC_PER_SEC
>> +	 */
>> +	u64 len = time_in_ns * r->rate_bytes_ps;
>> +
>> +	do_div(len, NSEC_PER_SEC);
> 
> You are multiplying two values then dividing by 10**9
> I'd guess that the intermediate value might exceed 2**64.

I thought the max value of len is burst which is a type of u32 sent
userland, so the max value of (time_in_ns * r->rate_bytes_ps)
should be 64K*(10**9).
Hmm, maybe I should do something in this helper to avoid overflow.

> 
>> +	if (unlikely(r->linklayer == TC_LINKLAYER_ATM))
>> +		len = (len / 53) * 48;
> 
> You probably want to do the multiply first.
> But why not scale rate_bytes_ps instead.
> 
>> +	if (len > r->overhead)
>> +		len -= r->overhead;
>> +	else
>> +		len = 0;
>> +
>> +	return len;
>> +}
> 
> Personally I'd work out how much time you have to send each byte.
> So if you want a rate of 1MB/sec you have a 'time cost' per byte of 1000ns.
> The cost of sending a packet is simply the length multiplied by this cost.
> To work out whether a packet can be sent you have a credit variable that
> tracks current time.
> If 'credit > now' the packet can't be sent, queue and schedule a wakeup.
> if 'credit + backlog < now' set credit = now - backlog.
> if 'credit <= now' send the packet and add the packet's 'cost' to 'credit'.
> 
> In the non-ratelimited case this is almost no work.
> 
> You'd probably need to work in 1/1024ns time units and/or blocks of 16 bytes.
> 
> 	David
> 
> 
> 
> 
> .
> 

  reply	other threads:[~2013-12-09  2:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06  7:00 [PATCH net v6 0/2] net: sched: fix two issues Yang Yingliang
2013-12-06  7:00 ` [PATCH net v6 1/2] net: sched: tbf: fix the calculation of max_size Yang Yingliang
2013-12-06 10:56   ` David Laight
2013-12-09  2:26     ` Yang Yingliang [this message]
2013-12-09  3:26     ` Yang Yingliang
2013-12-09 10:07       ` David Laight
2013-12-09 12:21         ` Yang Yingliang
2013-12-09 13:10           ` [PATCH RFC ] " Yang Yingliang
2013-12-09 15:12             ` Eric Dumazet
2013-12-10  2:29               ` Yang Yingliang
2013-12-10  2:39                 ` Eric Dumazet
2013-12-09 13:11           ` [PATCH net v6 1/2] " David Laight
2013-12-10  2:04             ` Yang Yingliang
2013-12-06  7:00 ` [PATCH net v6 2/2] net: sched: htb: fix the calculation of quantum Yang Yingliang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A52A5D.5060601@huawei.com \
    --to=yangyingliang@huawei$(echo .)com \
    --cc=David.Laight@ACULAB$(echo .)COM \
    --cc=brouer@redhat$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=jbrouer@redhat$(echo .)com \
    --cc=jpirko@redhat$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox