netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] netem: update backlog after drop
@ 2013-10-06 22:15 Stephen Hemminger
  2013-10-06 22:16 ` [PATCH net] netem: free skb's in tree on reset Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Stephen Hemminger @ 2013-10-06 22:15 UTC (permalink / raw)
  To: David Miller, Eric Dumazet; +Cc: netdev

When packet is dropped from rb-tree netem the backlog statistic should
also be updated.

Reported-by: Сергеев Сергей <adron@yapic.net>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

---
Should be reviewed by Eric (he added the rb-tree stuff), and added to stable
as well.

--- a/net/sched/sch_netem.c	2013-10-06 15:06:50.675250833 -0700
+++ b/net/sched/sch_netem.c	2013-10-06 15:10:03.957219532 -0700
@@ -520,6 +520,7 @@ static unsigned int netem_drop(struct Qd
 			skb->next = NULL;
 			skb->prev = NULL;
 			len = qdisc_pkt_len(skb);
+			sch->qstats.backlog -= len;
 			kfree_skb(skb);
 		}
 	}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] netem: free skb's in tree on reset
  2013-10-06 22:15 [PATCH net] netem: update backlog after drop Stephen Hemminger
@ 2013-10-06 22:16 ` Stephen Hemminger
  2013-10-11 21:31   ` David Miller
  2013-10-11 18:30 ` [PATCH net] netem: update backlog after drop David Miller
  2013-10-11 21:31 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2013-10-06 22:16 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Eric Dumazet, netdev

Netem can leak memory because packets get stored in red-black
tree and it is not cleared on reset.

Reported by: Сергеев Сергей <adron@yapic.net>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

--- a/net/sched/sch_netem.c	2013-10-06 15:10:03.957219532 -0700
+++ b/net/sched/sch_netem.c	2013-10-06 15:10:10.521150202 -0700
@@ -358,6 +358,21 @@ static psched_time_t packet_len_2_sched_
 	return PSCHED_NS2TICKS(ticks);
 }
 
+static void tfifo_reset(struct Qdisc *sch)
+{
+	struct netem_sched_data *q = qdisc_priv(sch);
+	struct rb_node *p;
+
+	while ((p = rb_first(&q->t_root)) {
+		struct sk_buff *skb = netem_rb_to_skb(p);
+
+		rb_erase(p, &q->t_root);
+		skb->next = NULL;
+		skb->prev = NULL;
+		kfree_skb(skb);
+	}
+}
+
 static void tfifo_enqueue(struct sk_buff *nskb, struct Qdisc *sch)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
@@ -610,6 +625,7 @@ static void netem_reset(struct Qdisc *sc
 	struct netem_sched_data *q = qdisc_priv(sch);
 
 	qdisc_reset_queue(sch);
+	tfifo_reset(sch);
 	if (q->qdisc)
 		qdisc_reset(q->qdisc);
 	qdisc_watchdog_cancel(&q->watchdog);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] netem: update backlog after drop
  2013-10-06 22:15 [PATCH net] netem: update backlog after drop Stephen Hemminger
  2013-10-06 22:16 ` [PATCH net] netem: free skb's in tree on reset Stephen Hemminger
@ 2013-10-11 18:30 ` David Miller
  2013-10-11 18:38   ` Eric Dumazet
  2013-10-11 21:31 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2013-10-11 18:30 UTC (permalink / raw)
  To: stephen; +Cc: eric.dumazet, netdev

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 6 Oct 2013 15:15:33 -0700

> When packet is dropped from rb-tree netem the backlog statistic should
> also be updated.
> 
> Reported-by: Сергеев Сергей <adron@yapic.net>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> ---
> Should be reviewed by Eric (he added the rb-tree stuff), and added to stable
> as well.

Eric please review this patch, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] netem: update backlog after drop
  2013-10-11 18:30 ` [PATCH net] netem: update backlog after drop David Miller
@ 2013-10-11 18:38   ` Eric Dumazet
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2013-10-11 18:38 UTC (permalink / raw)
  To: David Miller; +Cc: stephen, netdev

On Fri, 2013-10-11 at 14:30 -0400, David Miller wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Sun, 6 Oct 2013 15:15:33 -0700
> 
> > When packet is dropped from rb-tree netem the backlog statistic should
> > also be updated.
> > 
> > Reported-by: Сергеев Сергей <adron@yapic.net>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > ---
> > Should be reviewed by Eric (he added the rb-tree stuff), and added to stable
> > as well.
> 
> Eric please review this patch, thanks.

It somehow escaped from my radar ;)

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] netem: update backlog after drop
  2013-10-06 22:15 [PATCH net] netem: update backlog after drop Stephen Hemminger
  2013-10-06 22:16 ` [PATCH net] netem: free skb's in tree on reset Stephen Hemminger
  2013-10-11 18:30 ` [PATCH net] netem: update backlog after drop David Miller
@ 2013-10-11 21:31 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-10-11 21:31 UTC (permalink / raw)
  To: stephen; +Cc: eric.dumazet, netdev

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 6 Oct 2013 15:15:33 -0700

> When packet is dropped from rb-tree netem the backlog statistic should
> also be updated.
> 
> Reported-by: Сергеев Сергей <adron@yapic.net>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] netem: free skb's in tree on reset
  2013-10-06 22:16 ` [PATCH net] netem: free skb's in tree on reset Stephen Hemminger
@ 2013-10-11 21:31   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-10-11 21:31 UTC (permalink / raw)
  To: stephen; +Cc: eric.dumazet, netdev

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 6 Oct 2013 15:16:49 -0700

> Netem can leak memory because packets get stored in red-black
> tree and it is not cleared on reset.
> 
> Reported by: Сергеев Сергей <adron@yapic.net>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-11 21:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-06 22:15 [PATCH net] netem: update backlog after drop Stephen Hemminger
2013-10-06 22:16 ` [PATCH net] netem: free skb's in tree on reset Stephen Hemminger
2013-10-11 21:31   ` David Miller
2013-10-11 18:30 ` [PATCH net] netem: update backlog after drop David Miller
2013-10-11 18:38   ` Eric Dumazet
2013-10-11 21:31 ` David Miller

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).