netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
	jhs@mojatatu.com, kuznet@ms2.inr.ac.ru, j.vimal@gmail.com
Subject: Re: [patch net-next v5 10/11] tbf: take into account gso skbs
Date: Sun, 17 Feb 2013 17:18:03 +0100	[thread overview]
Message-ID: <20130217161803.GB1931@minipsycho.orion> (raw)
In-Reply-To: <1360687182.6884.5.camel@edumazet-glaptop>

Tue, Feb 12, 2013 at 05:39:42PM CET, eric.dumazet@gmail.com wrote:
>On Tue, 2013-02-12 at 11:12 +0100, Jiri Pirko wrote:
>> Ignore max_size check for gso skbs. This check made bigger packets
>> incorrectly dropped. Remove this limitation for gso skbs.
>> 
>> Also for peaks, ignore mtu for gso skbs.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  net/sched/sch_tbf.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
>> index c8388f3..8973e93 100644
>> --- a/net/sched/sch_tbf.c
>> +++ b/net/sched/sch_tbf.c
>> @@ -121,7 +121,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch)
>>  	struct tbf_sched_data *q = qdisc_priv(sch);
>>  	int ret;
>>  
>> -	if (qdisc_pkt_len(skb) > q->max_size)
>> +	if (qdisc_pkt_len(skb) > q->max_size && !skb_is_gso(skb))
>>  		return qdisc_reshape_fail(skb, sch);
>>  
>>  	ret = qdisc_enqueue(skb, q->qdisc);
>> @@ -165,7 +165,7 @@ static struct sk_buff *tbf_dequeue(struct Qdisc *sch)
>>  
>>  		if (q->peak_present) {
>>  			ptoks = toks + q->ptokens;
>> -			if (ptoks > q->mtu)
>> +			if (ptoks > q->mtu && !skb_is_gso(skb))
>>  				ptoks = q->mtu;
>>  			ptoks -= (s64) psched_l2t_ns(&q->peak, len);
>>  		}
>
>
>I guess this part is wrong.
>
>If we dont cap ptoks to q->mtu we allow bigger bursts.


I'm going through this issue back and front and on the second thought,
I think this patch might not be so wrong after all.

"Accumulating" time in ptoks would effectively cause the skb to be sent
only in case time for whole skb is available (accumulated).

The re-segmenting will only cause the skb fragments sent in each time frame.

I can't see how the bigger bursts you are reffering to can happen.

Or am I missing something?

>
>Ideally we could re-segment the skb if psched_l2t_ns(&q->peak, len) is
>bigger than q->mtu
>
>
>
>
>
>

  parent reply	other threads:[~2013-02-17 16:18 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-12 10:11 [patch net-next v5 00/11] couple of net/sched fixes+improvements Jiri Pirko
2013-02-12 10:11 ` [patch net-next v5 01/11] htb: use PSCHED_TICKS2NS() Jiri Pirko
2013-02-13  0:00   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 02/11] htb: fix values in opt dump Jiri Pirko
2013-02-12 23:51   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 03/11] htb: remove pointless first initialization of buffer and cbuffer Jiri Pirko
2013-02-13  0:00   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 04/11] htb: initialize cl->tokens and cl->ctokens correctly Jiri Pirko
2013-02-13  0:00   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 05/11] sch: make htb_rate_cfg and functions around that generic Jiri Pirko
2013-02-13  0:00   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 06/11] sch_api: introduce qdisc_watchdog_schedule_ns() Jiri Pirko
2013-02-12 16:32   ` Eric Dumazet
2013-02-13  0:00   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 07/11] tbf: improved accuracy at high rates Jiri Pirko
2013-02-12 16:34   ` Eric Dumazet
2013-02-13  0:01   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 08/11] act_police: move struct tcf_police to act_police.c Jiri Pirko
2013-02-12 12:08   ` Jamal Hadi Salim
2013-02-12 16:34   ` Eric Dumazet
2013-02-13  0:01   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 09/11] act_police: improved accuracy at high rates Jiri Pirko
2013-02-12 13:31   ` Jamal Hadi Salim
2013-02-12 13:39     ` Jiri Pirko
2013-02-13  0:01   ` David Miller
2013-02-12 10:12 ` [patch net-next v5 10/11] tbf: take into account gso skbs Jiri Pirko
2013-02-12 16:39   ` Eric Dumazet
2013-02-12 17:31     ` Jiri Pirko
2013-02-12 17:54       ` Eric Dumazet
2013-02-17 16:18     ` Jiri Pirko [this message]
2013-02-17 17:54       ` Eric Dumazet
2013-02-18  9:58         ` Jiri Pirko
2013-02-19 16:15           ` Eric Dumazet
2013-02-19 16:46             ` Jiri Pirko
2013-02-19 17:01               ` Eric Dumazet
2013-03-08 15:23                 ` Jiri Pirko
2013-03-22 10:02                   ` Jiri Pirko
2013-02-12 10:12 ` [patch net-next v5 11/11] act_police: remove <=mtu check for " Jiri Pirko
2013-02-12 16:40   ` Eric Dumazet

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=20130217161803.GB1931@minipsycho.orion \
    --to=jiri@resnulli.us \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=j.vimal@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).