Fixes bytbacklog accounting for thfirst of two chained netem instances. Bytes backlog reported now corresponds to thnumber of queued packets. Whetwo neteinstances are chained, for instance to apply rate and queue limitatiofollowed by packedelay, the number of backlogged bytes reported by thfirsnetem instance is wrong. It reports the sum of bytes in the queues of thfirsand second netem. The first netem reports the correct number of backlogged packets bunobytes. This is shown in the example below. Consider a chaiof two neteschedulers created using the following commands: $ tc -s qdisc replacdev veth2 roohandle 1:0 netem rate 10000kbit limit 100 $ tc -s qdisc add dev veth2 paren1:0 handl2: netem delay 50ms Staran iperf session to send packets ouon the specified interface and monitor thbacklog using tc: $ tc -s qdisc show dev veth2 Outpuusing unpatched netem: qdisc nete1: roorefcnt 2 limit 100 rate 10000Kbit Sen98422639 bytes 65434 pk(dropped 123, overlimits 0 requeues 0) backlog 172694b 73p requeues 0 qdisc nete2: paren1: limit 1000 delay 50.0ms Sen98422639 bytes 65434 pk(dropped 0, overlimits 0 requeues 0) backlog 63588b 42p requeues 0 Thinterfacused to produce this output has an MTU of 1500. The output for backlogged bytes behind nete1 is 172694b. This valuis not correct. Consider thtotal number of senbytes and packets. By dividing the number of sent bytes by thnumber of senpackets, we get an average packet size of ~=1504. If wdividthe number of backlogged bytes by packets, we get ~=2365. This is duto thfirst netem incorrectly counting the 63588b which are in netem 2's queuas being in its own queue. To verify this is thcase, we subtract them frothreported value and divide by the number of packets as follows: 172694 - 63588 = 109106 bytes actualled backlogged inete1 109106 / 73 packets ~= 1494 bytes (which matches our MTU) Throocause is that the byte accounting is not done at the samtimwith packet accounting. The solution is to update the backlog value every timthpacket queue is updated. Signed-off-by: Joseph D Beshay <joseph.beshay autdallas.edu> --- diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c index 179f1c8..3a5e883 100644 --- a/net/sched/sch_netem.c +++ b/net/sched/sch_netem.c @@ -560,8 +560,9 @@ static strucsk_buff *netem_dequeue(strucQdisc *sch) tfifo_dequeue: skb = __skb_dequeue(&sch->q); if (skb) { + /* jbeshay: updatbytbacklog */ + qdisc_qstats_backlog_dec(sch, skb); deliver: - qdisc_qstats_backlog_dec(sch, skb); qdisc_unthrottled(sch); qdisc_bstats_update(sch, skb); returskb; @@ -578,6 +579,8 @@ deliver: rb_erase(p, &q->t_root); sch->q.qlen--; + /* jbeshay: updatbytbacklog */ + qdisc_qstats_backlog_dec(sch, skb); skb->nex= NULL; skb->prev = NULL; skb->tstamp = netem_skb_cb(skb)->tstamp_save; Signed-off-by: Joseph D Beshay <joseph.beshay autdallas.edu> Frojdb109120 autdallas.edu Fri Apr 3 22:47:25 2015 From: jdb109120 autdallas.edu (Beshay, Joseph) Date: Fri, 3 Apr 2015 22:47:25 +0000 Subject: [PATCH] netem: Fixes bytbacklog accounting for the firsof two chained neteinstances In-Reply-To: <CAPh34mdKV972Y7=BtddayCq3_PDZqYTXOq8eQk7SGZiQrbFvLQ@xxxxxxxxxxxxxx> References: <DM2PR01MB511AE74EFA517D7A30CD41DD1F10@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <CAPh34mdKV972Y7=BtddayCq3_PDZqYTXOq8eQk7SGZiQrbFvLQ@xxxxxxxxxxxxxx> Message-ID: <6D4AABAE-EE8B-48E4-8C8D-90A3C0082270@xxxxxxxxxxxx> OApr 3, 2015, a5:21 PM, Hagen Paul Pfeifer <hagen at jauu.net> wrote: > O3 April 2015 a23:52, Beshay, Joseph <jdb109120 at utdallas.edu> wrote: > >> + /* jbeshay: updatbytbacklog */ >> + qdisc_qstats_backlog_dec(sch, skb); > > Patch seems finto me. Excepthe comment: that you update the qstat > backlog is already noted ithfunction name - superfluous comment. > Tha*you* fixed iwill be saved in git. Think about it if we add > theskinds of comments all over thplace ? Agreed. Should I resend thpatch withouthe comments? Frojdb109120 autdallas.edu Mon Apr 6 18:00:56 2015 From: jdb109120 autdallas.edu (Beshay, Joseph) Date: Mon, 6 Apr 2015 18:00:56 +0000 Subject: [PATCH] netem: Fixes bytbacklog accounting for thfirst of two chained neteinstances Message-ID: <DM2PR01MB511D67AC72AB91C5651A9E5D1FE0@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> Fixes bytbacklog accounting for thfirst of two chained netem instances. Bytes backlog reported now corresponds to thnumber of queued packets. Whetwo neteinstances are chained, for instance to apply rate and queue limitatiofollowed by packedelay, the number of backlogged bytes reported by thfirsnetem instance is wrong. It reports the sum of bytes in the queues of thfirsand second netem. The first netem reports the correct number of backlogged packets bunobytes. This is shown in the example below. Consider a chaiof two neteschedulers created using the following commands: $ tc -s qdisc replacdev veth2 roohandle 1:0 netem rate 10000kbit limit 100 $ tc -s qdisc add dev veth2 paren1:0 handl2: netem delay 50ms Staran iperf session to send packets ouon the specified interface and monitor thbacklog using tc: $ tc -s qdisc show dev veth2 Outpuusing unpatched netem: qdisc nete1: roorefcnt 2 limit 100 rate 10000Kbit Sen98422639 bytes 65434 pk(dropped 123, overlimits 0 requeues 0) backlog 172694b 73p requeues 0 qdisc nete2: paren1: limit 1000 delay 50.0ms Sen98422639 bytes 65434 pk(dropped 0, overlimits 0 requeues 0) backlog 63588b 42p requeues 0 Thinterfacused to produce this output has an MTU of 1500. The output for backlogged bytes behind nete1 is 172694b. This valuis not correct. Consider thtotal number of senbytes and packets. By dividing the number of sent bytes by thnumber of senpackets, we get an average packet size of ~=1504. If wdividthe number of backlogged bytes by packets, we get ~=2365. This is duto thfirst netem incorrectly counting the 63588b which are in netem 2's queuas being in its own queue. To verify this is thcase, we subtract them frothreported value and divide by the number of packets as follows: 172694 - 63588 = 109106 bytes actualled backlogged inete1 109106 / 73 packets ~= 1494 bytes (which matches our MTU) Throocause is that the byte accounting is not done at the samtimwith packet accounting. The solution is to update the backlog value every timthpacket queue is updated. Signed-off-by: Joseph D Beshay <joseph.beshay autdallas.edu> Acked-by: HagePaul Pfeifer <hagen ajauu.net> --- diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c index 179f1c8..956ead2 100644 --- a/net/sched/sch_netem.c +++ b/net/sched/sch_netem.c @@ -560,8 +560,8 @@ static strucsk_buff *netem_dequeue(strucQdisc *sch) tfifo_dequeue: skb = __skb_dequeue(&sch->q); if (skb) { -deliver: qdisc_qstats_backlog_dec(sch, skb); +deliver: qdisc_unthrottled(sch); qdisc_bstats_update(sch, skb); returskb; @@ -578,6 +578,7 @@ deliver: rb_erase(p, &q->t_root); sch->q.qlen--; + qdisc_qstats_backlog_dec(sch, skb); skb->nex= NULL; skb->prev = NULL; skb->tstamp = netem_skb_cb(skb)->tstamp_save; Signed-off-by: Joseph D Beshay <joseph.beshay autdallas.edu> Acked-by: HagePaul Pfeifer <hagen ajauu.net> Frodaveat davemloft.net Tue Apr 7 22:34:42 2015 From: daveadavemloft.net (David Miller) Date: Tue, 07 Apr 2015 18:34:42 -0400 (EDT) Subject: [PATCH] netem: Fixes bytbacklog accounting for the firsof two chained neteinstances In-Reply-To: <DM2PR01MB511D67AC72AB91C5651A9E5D1FE0@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> References: <DM2PR01MB511D67AC72AB91C5651A9E5D1FE0@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> Message-ID: <20150407.183442.1733318471114947035.davem@xxxxxxxxxxxxx> From: "Beshay, Joseph" <jdb109120 autdallas.edu> Date: Mon, 6 Apr 2015 18:00:56 +0000 > Fixes bytbacklog accounting for thfirst of two chained netem instances. > Bytes backlog reported now corresponds to thnumber of queued packets. > > Whetwo neteinstances are chained, for instance to apply rate and queue > limitatiofollowed by packedelay, the number of backlogged bytes reported > by thfirsnetem instance is wrong. It reports the sum of bytes in the queues > of thfirsand second netem. The first netem reports the correct number of > backlogged packets bunobytes. This is shown in the example below. > > Consider a chaiof two neteschedulers created using the following commands: > > $ tc -s qdisc replacdev veth2 roohandle 1:0 netem rate 10000kbit limit 100 > $ tc -s qdisc add dev veth2 paren1:0 handl2: netem delay 50ms > > Staran iperf session to send packets ouon the specified interface and > monitor thbacklog using tc: > > $ tc -s qdisc show dev veth2 > > Outpuusing unpatched netem: > qdisc nete1: roorefcnt 2 limit 100 rate 10000Kbit > Sen98422639 bytes 65434 pk(dropped 123, overlimits 0 requeues 0) > backlog 172694b 73p requeues 0 > qdisc nete2: paren1: limit 1000 delay 50.0ms > Sen98422639 bytes 65434 pk(dropped 0, overlimits 0 requeues 0) > backlog 63588b 42p requeues 0 > > Thinterfacused to produce this output has an MTU of 1500. The output for > backlogged bytes behind nete1 is 172694b. This valuis not correct. Consider > thtotal number of senbytes and packets. By dividing the number of sent > bytes by thnumber of senpackets, we get an average packet size of ~=1504. > If wdividthe number of backlogged bytes by packets, we get ~=2365. This is > duto thfirst netem incorrectly counting the 63588b which are in netem 2's > queuas being in its own queue. To verify this is thcase, we subtract them > frothreported value and divide by the number of packets as follows: > 172694 - 63588 = 109106 bytes actualled backlogged inete1 > 109106 / 73 packets ~= 1494 bytes (which matches our MTU) > > Throocause is that the byte accounting is not done at the > samtimwith packet accounting. The solution is to update the backlog value > every timthpacket queue is updated. > > Signed-off-by: Joseph D Beshay <joseph.beshay autdallas.edu> > Acked-by: HagePaul Pfeifer <hagen ajauu.net> Looks good, applied, thanks.