* [PATCH net v4 0/2] net: sched: fix some issues @ 2013-12-03 3:26 Yang Yingliang 2013-12-03 3:26 ` [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size Yang Yingliang 2013-12-03 3:26 ` [PATCH net v4 2/2] net: sched: htb: fix calculation of quantum Yang Yingliang 0 siblings, 2 replies; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 3:26 UTC (permalink / raw) To: davem, netdev; +Cc: eric.dumazet, brouer, jpirko, jbrouer fix calculation of max_size in tbf fix quantum calculation introduced by 64bit rates. v2 patch 1/2: redescribe the regression. patch 2/2: add Eric's ack. v3 patch 1/2: use psched_l2t_ns to calculate max_size and cleanup exit/done section suggested by Jesper. v4 patch 1/2: 1.Update commit message suggested by Jesper. 2.Use a macro to replace 65535 constant. 3.Add condition that when peakrate is lower than rate, return -EINVAL. 4.Don't use cell_log anymore. Yang Yingliang (2): net: sched: tbf: fix calculation of max_size net: sched: htb: fix calculation of quantum net/sched/sch_htb.c | 18 ++++++----- net/sched/sch_tbf.c | 91 ++++++++++++++++++++++++++++++----------------------- 2 files changed, 61 insertions(+), 48 deletions(-) -- 1.8.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 3:26 [PATCH net v4 0/2] net: sched: fix some issues Yang Yingliang @ 2013-12-03 3:26 ` Yang Yingliang 2013-12-03 4:51 ` Eric Dumazet 2013-12-03 4:59 ` Eric Dumazet 2013-12-03 3:26 ` [PATCH net v4 2/2] net: sched: htb: fix calculation of quantum Yang Yingliang 1 sibling, 2 replies; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 3:26 UTC (permalink / raw) To: davem, netdev; +Cc: eric.dumazet, brouer, jpirko, jbrouer Current max_size is caluated from rate table. Now, the rate table has been replaced and it's wrong to caculate max_size based on this rate table. It can lead wrong calculation of max_size. The burst in kernel may be lower than user asked, because burst may gets some loss when transform it to buffer(E.g. "burst 40kb rate 30mbit/s") and it seems we cannot avoid this loss. Burst's value(max_size) based on rate table may be equal user asked. If a packet's length is max_size, this packet will be stalled in tbf_dequeue() because its length is above the burst in kernel so that it cannot get enough tokens. The max_size guards against enqueuing packet sizes above q->buffer "time" in tbf_enqueue(). To make consistent with the calculation of tokens, this patch fixes the calculation of max_size by using psched_l2t_ns() and q->buffer to recalculate burst(max_size). Suggested-by: Jesper Dangaard Brouer <jbrouer@redhat.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- net/sched/sch_tbf.c | 91 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c index a609005..7883e7f 100644 --- a/net/sched/sch_tbf.c +++ b/net/sched/sch_tbf.c @@ -283,6 +283,8 @@ static const struct nla_policy tbf_policy[TCA_TBF_MAX + 1] = { [TCA_TBF_PRATE64] = { .type = NLA_U64 }, }; +#define MAX_PKT_LEN 65535 + static int tbf_change(struct Qdisc *sch, struct nlattr *opt) { int err; @@ -292,7 +294,7 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt) struct qdisc_rate_table *rtab = NULL; struct qdisc_rate_table *ptab = NULL; struct Qdisc *child = NULL; - int max_size, n; + int max_size; u64 rate64 = 0, prate64 = 0; err = nla_parse_nested(tb, TCA_TBF_MAX, opt, tbf_policy); @@ -304,38 +306,20 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt) goto done; qopt = nla_data(tb[TCA_TBF_PARMS]); - rtab = qdisc_get_rtab(&qopt->rate, tb[TCA_TBF_RTAB]); - if (rtab == NULL) - goto done; - - if (qopt->peakrate.rate) { - if (qopt->peakrate.rate > qopt->rate.rate) - ptab = qdisc_get_rtab(&qopt->peakrate, tb[TCA_TBF_PTAB]); - if (ptab == NULL) - goto done; + if (qopt->rate.linklayer == TC_LINKLAYER_UNAWARE) { + rtab = qdisc_get_rtab(&qopt->rate, tb[TCA_TBF_RTAB]); + if (rtab) { + qdisc_put_rtab(rtab); + rtab = NULL; + } } - - for (n = 0; n < 256; n++) - if (rtab->data[n] > qopt->buffer) - break; - max_size = (n << qopt->rate.cell_log) - 1; - if (ptab) { - int size; - - for (n = 0; n < 256; n++) - if (ptab->data[n] > qopt->mtu) - break; - size = (n << qopt->peakrate.cell_log) - 1; - if (size < max_size) - max_size = size; + if (qopt->peakrate.linklayer == TC_LINKLAYER_UNAWARE) { + ptab = qdisc_get_rtab(&qopt->peakrate, tb[TCA_TBF_PTAB]); + if (ptab) { + qdisc_put_rtab(ptab); + ptab = NULL; + } } - if (max_size < 0) - goto done; - - if (max_size < psched_mtu(qdisc_dev(sch))) - pr_warn_ratelimited("sch_tbf: burst %u is lower than device %s mtu (%u) !\n", - max_size, qdisc_dev(sch)->name, - psched_mtu(qdisc_dev(sch))); if (q->qdisc != &noop_qdisc) { err = fifo_set_limit(q->qdisc, qopt->limit); @@ -357,30 +341,57 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt) } q->limit = qopt->limit; q->mtu = PSCHED_TICKS2NS(qopt->mtu); - q->max_size = max_size; q->buffer = PSCHED_TICKS2NS(qopt->buffer); q->tokens = q->buffer; q->ptokens = q->mtu; if (tb[TCA_TBF_RATE64]) rate64 = nla_get_u64(tb[TCA_TBF_RATE64]); - psched_ratecfg_precompute(&q->rate, &rtab->rate, rate64); - if (ptab) { + psched_ratecfg_precompute(&q->rate, &qopt->rate, rate64); + if (!q->rate.rate_bytes_ps) + goto unlock_done; + + for (max_size = 0; max_size < MAX_PKT_LEN; max_size++) + if (psched_l2t_ns(&q->rate, max_size) > q->buffer) + break; + if (--max_size <= 0) + goto unlock_done; + + if (qopt->peakrate.rate) { + int size = 0; if (tb[TCA_TBF_PRATE64]) prate64 = nla_get_u64(tb[TCA_TBF_PRATE64]); - psched_ratecfg_precompute(&q->peak, &ptab->rate, prate64); + psched_ratecfg_precompute(&q->peak, &qopt->peakrate, prate64); + if (q->peak.rate_bytes_ps <= q->rate.rate_bytes_ps) { + pr_err("Rate must be lower than peakrate!\n"); + goto unlock_done; + } + + for (size = 0; size < MAX_PKT_LEN; size++) + if (psched_l2t_ns(&q->peak, size) > q->mtu) + break; + if (--size <= 0) + goto unlock_done; + max_size = min_t(int, max_size, size); q->peak_present = true; } else { q->peak_present = false; } + if (max_size < psched_mtu(qdisc_dev(sch))) + pr_warn_ratelimited("sch_tbf: burst %u is lower than device %s mtu (%u) !\n", + max_size, qdisc_dev(sch)->name, + psched_mtu(qdisc_dev(sch))); + + q->max_size = max_size; + sch_tree_unlock(sch); - err = 0; + return 0; + +unlock_done: + sch_tree_unlock(sch); + err = -EINVAL; done: - if (rtab) - qdisc_put_rtab(rtab); - if (ptab) - qdisc_put_rtab(ptab); return err; } -- 1.8.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 3:26 ` [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size Yang Yingliang @ 2013-12-03 4:51 ` Eric Dumazet 2013-12-03 6:09 ` Yang Yingliang 2013-12-03 4:59 ` Eric Dumazet 1 sibling, 1 reply; 12+ messages in thread From: Eric Dumazet @ 2013-12-03 4:51 UTC (permalink / raw) To: Yang Yingliang; +Cc: davem, netdev, brouer, jpirko, jbrouer On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: > + if (qopt->rate.linklayer == TC_LINKLAYER_UNAWARE) { > + rtab = qdisc_get_rtab(&qopt->rate, tb[TCA_TBF_RTAB]); > + if (rtab) { > + qdisc_put_rtab(rtab); > + rtab = NULL; > + } > } Please make remove rtab use to make sure you dont leak a pointer. if (qopt->rate.linklayer == TC_LINKLAYER_UNAWARE) qdisc_put_rtab(qdisc_get_rtab(&qopt->rate, tb[TCA_TBF_RTAB])); > + if (qopt->peakrate.linklayer == TC_LINKLAYER_UNAWARE) { > + ptab = qdisc_get_rtab(&qopt->peakrate, tb[TCA_TBF_PTAB]); > + if (ptab) { > + qdisc_put_rtab(ptab); > + ptab = NULL; > + } Same here for ptab ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 4:51 ` Eric Dumazet @ 2013-12-03 6:09 ` Yang Yingliang 0 siblings, 0 replies; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 6:09 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, netdev, brouer, jpirko, jbrouer On 2013/12/3 12:51, Eric Dumazet wrote: > On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: > >> + if (qopt->rate.linklayer == TC_LINKLAYER_UNAWARE) { >> + rtab = qdisc_get_rtab(&qopt->rate, tb[TCA_TBF_RTAB]); >> + if (rtab) { >> + qdisc_put_rtab(rtab); >> + rtab = NULL; >> + } >> } > > Please make remove rtab use to make sure you dont leak a pointer. > > if (qopt->rate.linklayer == TC_LINKLAYER_UNAWARE) > qdisc_put_rtab(qdisc_get_rtab(&qopt->rate, > tb[TCA_TBF_RTAB])); > > >> + if (qopt->peakrate.linklayer == TC_LINKLAYER_UNAWARE) { >> + ptab = qdisc_get_rtab(&qopt->peakrate, tb[TCA_TBF_PTAB]); >> + if (ptab) { >> + qdisc_put_rtab(ptab); >> + ptab = NULL; >> + } > > Same here for ptab > OK, Thanks! Regards, Yang ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 3:26 ` [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size Yang Yingliang 2013-12-03 4:51 ` Eric Dumazet @ 2013-12-03 4:59 ` Eric Dumazet 2013-12-03 7:44 ` Yang Yingliang 1 sibling, 1 reply; 12+ messages in thread From: Eric Dumazet @ 2013-12-03 4:59 UTC (permalink / raw) To: Yang Yingliang; +Cc: davem, netdev, brouer, jpirko, jbrouer On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: > + for (max_size = 0; max_size < MAX_PKT_LEN; max_size++) > + if (psched_l2t_ns(&q->rate, max_size) > q->buffer) > + break; > + if (--max_size <= 0) > + goto unlock_done; > + This seems dubious. With your new code, max_size < 65536 Prior code had : for (n = 0; n < 256; n++) if (rtab->data[n] > qopt->buffer) break; max_size = (n << qopt->rate.cell_log) - 1; So we could have much bigger max_size. The reason I ask is that its possible to have qdisc_pkt_len(skb) being bigger than 65536, for TCP packets with low MSS value. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 4:59 ` Eric Dumazet @ 2013-12-03 7:44 ` Yang Yingliang 2013-12-03 8:12 ` Eric Dumazet 0 siblings, 1 reply; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 7:44 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, netdev, brouer, jpirko, jbrouer On 2013/12/3 12:59, Eric Dumazet wrote: > On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: > >> + for (max_size = 0; max_size < MAX_PKT_LEN; max_size++) >> + if (psched_l2t_ns(&q->rate, max_size) > q->buffer) >> + break; >> + if (--max_size <= 0) >> + goto unlock_done; >> + > > This seems dubious. With your new code, max_size < 65536 > > Prior code had : > > for (n = 0; n < 256; n++) > if (rtab->data[n] > qopt->buffer) > break; > max_size = (n << qopt->rate.cell_log) - 1; > > So we could have much bigger max_size. > > The reason I ask is that its possible to have qdisc_pkt_len(skb) being > bigger than 65536, for TCP packets with low MSS value. > Hmmm, if qdisc_pkt_len(skb) is bigger than 65536, skb_is_gso(skb) is true, it will go into tbf_segment(). If I am wrong, please point me out, thanks! BTW, 65536 is suggested by Jesper, I'm a little uncertain about it. He is, too. Do you or some other developers have stronger opinions on this? Thanks! Yang ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 7:44 ` Yang Yingliang @ 2013-12-03 8:12 ` Eric Dumazet 2013-12-03 9:47 ` Yang Yingliang 0 siblings, 1 reply; 12+ messages in thread From: Eric Dumazet @ 2013-12-03 8:12 UTC (permalink / raw) To: Yang Yingliang; +Cc: davem, netdev, brouer, jpirko, jbrouer On Tue, 2013-12-03 at 15:44 +0800, Yang Yingliang wrote: > On 2013/12/3 12:59, Eric Dumazet wrote: > > On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: > > > >> + for (max_size = 0; max_size < MAX_PKT_LEN; max_size++) > >> + if (psched_l2t_ns(&q->rate, max_size) > q->buffer) > >> + break; > >> + if (--max_size <= 0) > >> + goto unlock_done; > >> + > > > > This seems dubious. With your new code, max_size < 65536 > > > > Prior code had : > > > > for (n = 0; n < 256; n++) > > if (rtab->data[n] > qopt->buffer) > > break; > > max_size = (n << qopt->rate.cell_log) - 1; > > > > So we could have much bigger max_size. > > > > The reason I ask is that its possible to have qdisc_pkt_len(skb) being > > bigger than 65536, for TCP packets with low MSS value. > > > > Hmmm, if qdisc_pkt_len(skb) is bigger than 65536, skb_is_gso(skb) is true, > it will go into tbf_segment(). If I am wrong, please point me out, thanks! > > BTW, 65536 is suggested by Jesper, I'm a little uncertain about it. He is, too. > Do you or some other developers have stronger opinions on this? We do not want to go to tbf_segment() if we programmed tbf to allow TSO packets of 68.000 bytes being sent without being segmented. TSO packet of 64KB -> about 45 frames if MSS=1448, 45*1514 = 68130 bytes Have you tried to use TBF on a 10 Gbps link, say with one TCP flow ? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 8:12 ` Eric Dumazet @ 2013-12-03 9:47 ` Yang Yingliang 2013-12-03 11:32 ` Yang Yingliang 0 siblings, 1 reply; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 9:47 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, netdev, brouer, jpirko, jbrouer On 2013/12/3 16:12, Eric Dumazet wrote: > On Tue, 2013-12-03 at 15:44 +0800, Yang Yingliang wrote: >> On 2013/12/3 12:59, Eric Dumazet wrote: >>> On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: >>> >>>> + for (max_size = 0; max_size < MAX_PKT_LEN; max_size++) >>>> + if (psched_l2t_ns(&q->rate, max_size) > q->buffer) >>>> + break; >>>> + if (--max_size <= 0) >>>> + goto unlock_done; >>>> + >>> >>> This seems dubious. With your new code, max_size < 65536 >>> >>> Prior code had : >>> >>> for (n = 0; n < 256; n++) >>> if (rtab->data[n] > qopt->buffer) >>> break; >>> max_size = (n << qopt->rate.cell_log) - 1; >>> >>> So we could have much bigger max_size. >>> >>> The reason I ask is that its possible to have qdisc_pkt_len(skb) being >>> bigger than 65536, for TCP packets with low MSS value. >>> >> >> Hmmm, if qdisc_pkt_len(skb) is bigger than 65536, skb_is_gso(skb) is true, >> it will go into tbf_segment(). If I am wrong, please point me out, thanks! >> >> BTW, 65536 is suggested by Jesper, I'm a little uncertain about it. He is, too. >> Do you or some other developers have stronger opinions on this? > > We do not want to go to tbf_segment() if we programmed tbf to allow TSO > packets of 68.000 bytes being sent without being segmented. I mean a 64KB TSO packet's "qdisc_pkt_len(skb)" is bigger than 65536, but it will go to tbf_segment(), so the TSO packet of 64KB can be enqueued. > > TSO packet of 64KB -> about 45 frames if MSS=1448, 45*1514 = 68130 bytes > > Have you tried to use TBF on a 10 Gbps link, say with one TCP flow ? Yes, I had tried it with intel82599 nic. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 9:47 ` Yang Yingliang @ 2013-12-03 11:32 ` Yang Yingliang 2013-12-03 14:51 ` Eric Dumazet 0 siblings, 1 reply; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 11:32 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, netdev, brouer, jpirko, jbrouer On 2013/12/3 17:47, Yang Yingliang wrote: > On 2013/12/3 16:12, Eric Dumazet wrote: >> On Tue, 2013-12-03 at 15:44 +0800, Yang Yingliang wrote: >>> On 2013/12/3 12:59, Eric Dumazet wrote: >>>> On Tue, 2013-12-03 at 11:26 +0800, Yang Yingliang wrote: >>>> >>>>> + for (max_size = 0; max_size < MAX_PKT_LEN; max_size++) >>>>> + if (psched_l2t_ns(&q->rate, max_size) > q->buffer) >>>>> + break; >>>>> + if (--max_size <= 0) >>>>> + goto unlock_done; >>>>> + >>>> >>>> This seems dubious. With your new code, max_size < 65536 >>>> >>>> Prior code had : >>>> >>>> for (n = 0; n < 256; n++) >>>> if (rtab->data[n] > qopt->buffer) >>>> break; >>>> max_size = (n << qopt->rate.cell_log) - 1; >>>> >>>> So we could have much bigger max_size. >>>> >>>> The reason I ask is that its possible to have qdisc_pkt_len(skb) being >>>> bigger than 65536, for TCP packets with low MSS value. >>>> >>> >>> Hmmm, if qdisc_pkt_len(skb) is bigger than 65536, skb_is_gso(skb) is true, >>> it will go into tbf_segment(). If I am wrong, please point me out, thanks! >>> >>> BTW, 65536 is suggested by Jesper, I'm a little uncertain about it. He is, too. >>> Do you or some other developers have stronger opinions on this? >> >> We do not want to go to tbf_segment() if we programmed tbf to allow TSO >> packets of 68.000 bytes being sent without being segmented. > > I mean a 64KB TSO packet's "qdisc_pkt_len(skb)" is bigger than 65536, but it will go > to tbf_segment(), so the TSO packet of 64KB can be enqueued. I misunderstood 68.000 bytes as 68 bytes, ignore it.:) > >> >> TSO packet of 64KB -> about 45 frames if MSS=1448, 45*1514 = 68130 bytes Maybe MAX_PKT_LEN should be much bigger. Hmm, I'm uncertain how big is the proper value. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 11:32 ` Yang Yingliang @ 2013-12-03 14:51 ` Eric Dumazet 2013-12-04 1:53 ` Yang Yingliang 0 siblings, 1 reply; 12+ messages in thread From: Eric Dumazet @ 2013-12-03 14:51 UTC (permalink / raw) To: Yang Yingliang; +Cc: davem, netdev, brouer, jpirko, jbrouer On Tue, 2013-12-03 at 19:32 +0800, Yang Yingliang wrote: > > > >> > >> TSO packet of 64KB -> about 45 frames if MSS=1448, 45*1514 = 68130 bytes > > Maybe MAX_PKT_LEN should be much bigger. Hmm, I'm uncertain how big is the proper value. > Thats the thing : When user is able to compute the appropriate burst and give precise instructions to the kernel, why tbf would reduce it to whatever fixed threshold ? If I set 200000 as a burst, I do not want tbf use 65536 or even less. If tbf rounds my 200000 to 199800, its fine. You had problems to set burst to appropriate values, but most other users did that fine. tbf_segment() is an attempt to help for very low rates, to not overcome the small bursts (as low as 2000 bytes) programmed on atbf qdisc, but for high rates, we _definitely_ want to avoid segmentation as hell. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size 2013-12-03 14:51 ` Eric Dumazet @ 2013-12-04 1:53 ` Yang Yingliang 0 siblings, 0 replies; 12+ messages in thread From: Yang Yingliang @ 2013-12-04 1:53 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, netdev, brouer, jpirko, jbrouer On 2013/12/3 22:51, Eric Dumazet wrote: > On Tue, 2013-12-03 at 19:32 +0800, Yang Yingliang wrote: > >>> >>>> >>>> TSO packet of 64KB -> about 45 frames if MSS=1448, 45*1514 = 68130 bytes >> >> Maybe MAX_PKT_LEN should be much bigger. Hmm, I'm uncertain how big is the proper value. >> > > Thats the thing : When user is able to compute the appropriate burst and > give precise instructions to the kernel, why tbf would reduce it to > whatever fixed threshold ? > > If I set 200000 as a burst, I do not want tbf use 65536 or even less. > > If tbf rounds my 200000 to 199800, its fine. > > You had problems to set burst to appropriate values, but most other > users did that fine. > > tbf_segment() is an attempt to help for very low rates, to not overcome > the small bursts (as low as 2000 bytes) programmed on atbf qdisc, but > for high rates, we _definitely_ want to avoid segmentation as hell. > Thanks for your opinions! >From your opinions, how about calculating burst directly with q->buffer and psched_ns_t2l(). I did it in my v1 patch: /* Time to Length, convert time in ns to length in bytes * to determinate how many bytes can be sent in given time. */ static inline u64 psched_ns_t2l(const struct psched_ratecfg *r, u64 time_in_ns) { u64 len = time_in_ns; u8 shift = r->shift; bool is_div = false; /* The formula is : * len = (time_in_ns << shift) / mult * when time_in_ns does shift, it would overflow. * If overflow happens first time, do division. * Then do shift. If it happens again, * set lenth to ~0ULL. */ while (shift) { if (len & (1ULL << 63)) { if (!is_div) { len = div64_u64(len, r->mult); is_div = true; } else { /* overflow happens */ len = ~0ULL; is_div = true; break; } } len <<= 1; shift--; } if (!is_div) len = div64_u64(len, r->mult); if (unlikely(r->linklayer == TC_LINKLAYER_ATM)) len = (len / 53) * 48; if (len > r->overhead) len -= r->overhead; else len = 0; return len; } max_size = min_t(u64, psched_ns_t2l(&q->rate, q->buffer), ~0); Regards, Yang ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net v4 2/2] net: sched: htb: fix calculation of quantum 2013-12-03 3:26 [PATCH net v4 0/2] net: sched: fix some issues Yang Yingliang 2013-12-03 3:26 ` [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size Yang Yingliang @ 2013-12-03 3:26 ` Yang Yingliang 1 sibling, 0 replies; 12+ messages in thread From: Yang Yingliang @ 2013-12-03 3:26 UTC (permalink / raw) To: davem, netdev; +Cc: eric.dumazet, brouer, jpirko, jbrouer Now, 32bit rates may be not the true rate. So use rate_bytes_ps which is from max(rate32, rate64) to calcualte quantum. Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Acked-by: Eric Dumazet <edumazet@google.com> --- net/sched/sch_htb.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 0e1e38b..57c6678 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -1477,11 +1477,20 @@ static int htb_change_class(struct Qdisc *sch, u32 classid, sch_tree_lock(sch); } + rate64 = tb[TCA_HTB_RATE64] ? nla_get_u64(tb[TCA_HTB_RATE64]) : 0; + + ceil64 = tb[TCA_HTB_CEIL64] ? nla_get_u64(tb[TCA_HTB_CEIL64]) : 0; + + psched_ratecfg_precompute(&cl->rate, &hopt->rate, rate64); + psched_ratecfg_precompute(&cl->ceil, &hopt->ceil, ceil64); + /* it used to be a nasty bug here, we have to check that node * is really leaf before changing cl->un.leaf ! */ if (!cl->level) { - cl->quantum = hopt->rate.rate / q->rate2quantum; + u64 quantum = div64_u64(cl->rate.rate_bytes_ps, + q->rate2quantum); + cl->quantum = min_t(u64, quantum, INT_MAX); if (!hopt->quantum && cl->quantum < 1000) { pr_warning( "HTB: quantum of class %X is small. Consider r2q change.\n", @@ -1500,13 +1509,6 @@ static int htb_change_class(struct Qdisc *sch, u32 classid, cl->prio = TC_HTB_NUMPRIO - 1; } - rate64 = tb[TCA_HTB_RATE64] ? nla_get_u64(tb[TCA_HTB_RATE64]) : 0; - - ceil64 = tb[TCA_HTB_CEIL64] ? nla_get_u64(tb[TCA_HTB_CEIL64]) : 0; - - psched_ratecfg_precompute(&cl->rate, &hopt->rate, rate64); - psched_ratecfg_precompute(&cl->ceil, &hopt->ceil, ceil64); - cl->buffer = PSCHED_TICKS2NS(hopt->buffer); cl->cbuffer = PSCHED_TICKS2NS(hopt->cbuffer); -- 1.8.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-12-04 1:53 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-03 3:26 [PATCH net v4 0/2] net: sched: fix some issues Yang Yingliang 2013-12-03 3:26 ` [PATCH net v4 1/2] net: sched: tbf: fix calculation of max_size Yang Yingliang 2013-12-03 4:51 ` Eric Dumazet 2013-12-03 6:09 ` Yang Yingliang 2013-12-03 4:59 ` Eric Dumazet 2013-12-03 7:44 ` Yang Yingliang 2013-12-03 8:12 ` Eric Dumazet 2013-12-03 9:47 ` Yang Yingliang 2013-12-03 11:32 ` Yang Yingliang 2013-12-03 14:51 ` Eric Dumazet 2013-12-04 1:53 ` Yang Yingliang 2013-12-03 3:26 ` [PATCH net v4 2/2] net: sched: htb: fix calculation of quantum Yang Yingliang
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).