netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Beshay, Joseph" <jdb109120@utdallas.edu>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>,
	"netem@lists.linux-foundation.org"
	<netem@lists.linux-foundation.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: [PATCH] netem:  Fixes byte backlog accounting for the first of two chained netem instances
Date: Fri, 3 Apr 2015 21:52:01 +0000	[thread overview]
Message-ID: <DM2PR01MB511AE74EFA517D7A30CD41DD1F10@DM2PR01MB511.prod.exchangelabs.com> (raw)

Fixes byte backlog accounting for the first of two chained netem instances.
Bytes backlog reported now corresponds to the number of queued packets.

When two netem instances are chained, for instance to apply rate and queue
limitation followed by packet delay, the number of backlogged bytes reported
by the first netem instance is wrong. It reports the sum of bytes in the queues
of the first and second netem. The first netem reports the correct number of
backlogged packets but not bytes. This is shown in the example below.

Consider a chain of two netem schedulers created using the following commands:

$ tc -s qdisc replace dev veth2 root handle 1:0 netem rate 10000kbit limit 100
$ tc -s qdisc add dev veth2 parent 1:0 handle 2: netem delay 50ms

Start an iperf session to send packets out on the specified interface and
monitor the backlog using tc:

$ tc -s qdisc show dev veth2

Output using unpatched netem:
	qdisc netem 1: root refcnt 2 limit 100 rate 10000Kbit
	 Sent 98422639 bytes 65434 pkt (dropped 123, overlimits 0 requeues 0)
	 backlog 172694b 73p requeues 0
	qdisc netem 2: parent 1: limit 1000 delay 50.0ms
	 Sent 98422639 bytes 65434 pkt (dropped 0, overlimits 0 requeues 0)
	 backlog 63588b 42p requeues 0
	 
The interface used to produce this output has an MTU of 1500. The output for
backlogged bytes behind netem 1 is 172694b. This value is not correct. Consider
the total number of sent bytes and packets. By dividing the number of sent
bytes by the number of sent packets, we get an average packet size of ~=1504.
If we divide the number of backlogged bytes by packets, we get ~=2365. This is
due to the first netem incorrectly counting the 63588b which are in netem 2's
queue as being in its own queue. To verify this is the case, we subtract them
from the reported value and divide by the number of packets as follows: 
	172694 - 63588 = 109106 bytes actualled backlogged in netem 1
	109106 / 73 packets ~= 1494 bytes (which matches our MTU)
	
The root cause is that the byte accounting is not done at the
same time with packet accounting. The solution is to update the backlog value
every time the packet queue is updated.

Signed-off-by: Joseph D Beshay <joseph.beshay@utdallas.edu>
---
diff --git a/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 struct sk_buff *netem_dequeue(struct Qdisc *sch)
 tfifo_dequeue:
 	skb = __skb_dequeue(&sch->q);
 	if (skb) {
+		/* jbeshay: update byte backlog */
+		qdisc_qstats_backlog_dec(sch, skb);
 deliver:
-		qdisc_qstats_backlog_dec(sch, skb);
 		qdisc_unthrottled(sch);
 		qdisc_bstats_update(sch, skb);
 		return skb;
@@ -578,6 +579,8 @@ deliver:
 			rb_erase(p, &q->t_root);
 
 			sch->q.qlen--;
+			/* jbeshay: update byte backlog */
+			qdisc_qstats_backlog_dec(sch, skb);
 			skb->next = NULL;
 			skb->prev = NULL;
 			skb->tstamp = netem_skb_cb(skb)->tstamp_save;

Signed-off-by: Joseph D Beshay <joseph.beshay@utdallas.edu>

             reply	other threads:[~2015-04-03 22:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-03 21:52 Beshay, Joseph [this message]
2015-04-03 22:21 ` [PATCH] netem: Fixes byte backlog accounting for the first of two chained netem instances Hagen Paul Pfeifer
2015-04-03 22:47   ` Beshay, Joseph
  -- strict thread matches above, loose matches on Subject: below --
2015-04-06 18:00 Beshay, Joseph
2015-04-07 22:34 ` David Miller

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=DM2PR01MB511AE74EFA517D7A30CD41DD1F10@DM2PR01MB511.prod.exchangelabs.com \
    --to=jdb109120@utdallas.edu \
    --cc=netdev@vger.kernel.org \
    --cc=netem@lists.linux-foundation.org \
    --cc=stephen@networkplumber.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).