| Message ID | 1292437116.3427.386.camel@edumazet-laptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Le mercredi 15 décembre 2010 à 19:18 +0100, Eric Dumazet a écrit : > We currently return for each active SFQ slot the number of packets in > queue. We can also give number of bytes accounted for these packets. > > tc -s class show dev ifb0 > > Before patch : > > class sfq 11:3d9 parent 11: > (dropped 0, overlimits 0 requeues 0) > backlog 0b 3p requeues 0 > allot 1266 > > After patch : > > class sfq 11:3e4 parent 11: > (dropped 0, overlimits 0 requeues 0) > backlog 4380b 3p requeues 0 > allot 1212 > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > net/sched/sch_sfq.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > index 3cf478d..cb331de 100644 > --- a/net/sched/sch_sfq.c > +++ b/net/sched/sch_sfq.c > @@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl, > { > struct sfq_sched_data *q = qdisc_priv(sch); > sfq_index idx = q->ht[cl-1]; > - struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen }; > + struct sk_buff_head *list = &q->qs[idx]; > + struct gnet_stats_queue qs = { .qlen = list->qlen }; > struct tc_sfq_xstats xstats = { .allot = q->allot[idx] }; > + struct sk_buff *skb; > + > + skb_queue_walk(list, skb) > + qs.backlog += qdisc_pkt_len(skb); > > if (gnet_stats_copy_queue(d, &qs) < 0) > return -1; > By the way, I could not find out how to make "tc -s class show dev ifb0" reports correct backlog information on parent class itself : Here we can see 126p, and 0b in backlog : class cbq 1:11 parent 1:1 leaf 11: rate 50000Kbit cell 8b mpu 64b (bounded) prio 2/2 weight 50000Kbit allot 2250b level 0 ewma 5 avpkt 1500b maxidle 0us Sent 330418440 bytes 226314 pkt (dropped 280142, overlimits 689534 requeues 0) rate 52200Kbit 4469pps backlog 0b 126p requeues 0 borrowed 0 overactions 182814 avgidle -3363 undertime 2812 qdisc is OK : qdisc sfq 11: parent 1:11 limit 127p quantum 1514b flows 127/1024 perturb 60sec Sent 1712372746 bytes 1172859 pkt (dropped 1451945, overlimits 0 requeues 0) rate 52287Kbit 4477pps backlog 185420b 127p requeues 0 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2010-12-15 19:18, Eric Dumazet wrote: > We currently return for each active SFQ slot the number of packets in > queue. We can also give number of bytes accounted for these packets. > > tc -s class show dev ifb0 > > Before patch : > > class sfq 11:3d9 parent 11: > (dropped 0, overlimits 0 requeues 0) > backlog 0b 3p requeues 0 > allot 1266 > > After patch : > > class sfq 11:3e4 parent 11: > (dropped 0, overlimits 0 requeues 0) > backlog 4380b 3p requeues 0 > allot 1212 > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > net/sched/sch_sfq.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > index 3cf478d..cb331de 100644 > --- a/net/sched/sch_sfq.c > +++ b/net/sched/sch_sfq.c > @@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl, > { > struct sfq_sched_data *q = qdisc_priv(sch); > sfq_index idx = q->ht[cl-1]; > - struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen }; > + struct sk_buff_head *list = &q->qs[idx]; > + struct gnet_stats_queue qs = { .qlen = list->qlen }; > struct tc_sfq_xstats xstats = { .allot = q->allot[idx] }; > + struct sk_buff *skb; > + > + skb_queue_walk(list, skb) > + qs.backlog += qdisc_pkt_len(skb); I don't think you can walk this list without the qdisc lock. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le jeudi 16 décembre 2010 à 08:16 +0000, Jarek Poplawski a écrit : > On 2010-12-15 19:18, Eric Dumazet wrote: > > We currently return for each active SFQ slot the number of packets in > > queue. We can also give number of bytes accounted for these packets. > > > > tc -s class show dev ifb0 > > > > Before patch : > > > > class sfq 11:3d9 parent 11: > > (dropped 0, overlimits 0 requeues 0) > > backlog 0b 3p requeues 0 > > allot 1266 > > > > After patch : > > > > class sfq 11:3e4 parent 11: > > (dropped 0, overlimits 0 requeues 0) > > backlog 4380b 3p requeues 0 > > allot 1212 > > > > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > --- > > net/sched/sch_sfq.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > > index 3cf478d..cb331de 100644 > > --- a/net/sched/sch_sfq.c > > +++ b/net/sched/sch_sfq.c > > @@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl, > > { > > struct sfq_sched_data *q = qdisc_priv(sch); > > sfq_index idx = q->ht[cl-1]; > > - struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen }; > > + struct sk_buff_head *list = &q->qs[idx]; > > + struct gnet_stats_queue qs = { .qlen = list->qlen }; > > struct tc_sfq_xstats xstats = { .allot = q->allot[idx] }; > > + struct sk_buff *skb; > > + > > + skb_queue_walk(list, skb) > > + qs.backlog += qdisc_pkt_len(skb); > > I don't think you can walk this list without the qdisc lock. So after checks, I confirm qdisc lock is held at this point, patch is valid. tc_fill_tclass() calls gnet_stats_start_copy_compat() (and locks qdisc_root_sleeping_lock()), before calling cl_ops->dump_stats(q, cl, &d) Thanks ! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 16, 2010 at 12:03:04PM +0100, Eric Dumazet wrote: > Le jeudi 16 décembre 2010 ?? 08:16 +0000, Jarek Poplawski a écrit : > > On 2010-12-15 19:18, Eric Dumazet wrote: > > > We currently return for each active SFQ slot the number of packets in > > > queue. We can also give number of bytes accounted for these packets. ... > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > > --- > > > net/sched/sch_sfq.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > > > index 3cf478d..cb331de 100644 > > > --- a/net/sched/sch_sfq.c > > > +++ b/net/sched/sch_sfq.c > > > @@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl, > > > { > > > struct sfq_sched_data *q = qdisc_priv(sch); > > > sfq_index idx = q->ht[cl-1]; > > > - struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen }; > > > + struct sk_buff_head *list = &q->qs[idx]; > > > + struct gnet_stats_queue qs = { .qlen = list->qlen }; > > > struct tc_sfq_xstats xstats = { .allot = q->allot[idx] }; > > > + struct sk_buff *skb; > > > + > > > + skb_queue_walk(list, skb) > > > + qs.backlog += qdisc_pkt_len(skb); > > > > I don't think you can walk this list without the qdisc lock. > > So after checks, I confirm qdisc lock is held at this point, patch is > valid. > > tc_fill_tclass() calls gnet_stats_start_copy_compat() (and locks > qdisc_root_sleeping_lock()), before calling > cl_ops->dump_stats(q, cl, &d) > > Thanks ! You are right. Sorry for misleading. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 15 Dec 2010 19:18:36 +0100 > We currently return for each active SFQ slot the number of packets in > queue. We can also give number of bytes accounted for these packets. > > tc -s class show dev ifb0 > > Before patch : > > class sfq 11:3d9 parent 11: > (dropped 0, overlimits 0 requeues 0) > backlog 0b 3p requeues 0 > allot 1266 > > After patch : > > class sfq 11:3e4 parent 11: > (dropped 0, overlimits 0 requeues 0) > backlog 4380b 3p requeues 0 > allot 1212 > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied, thanks Eric. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index 3cf478d..cb331de 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c @@ -548,8 +548,13 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl, { struct sfq_sched_data *q = qdisc_priv(sch); sfq_index idx = q->ht[cl-1]; - struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen }; + struct sk_buff_head *list = &q->qs[idx]; + struct gnet_stats_queue qs = { .qlen = list->qlen }; struct tc_sfq_xstats xstats = { .allot = q->allot[idx] }; + struct sk_buff *skb; + + skb_queue_walk(list, skb) + qs.backlog += qdisc_pkt_len(skb); if (gnet_stats_copy_queue(d, &qs) < 0) return -1;
We currently return for each active SFQ slot the number of packets in queue. We can also give number of bytes accounted for these packets. tc -s class show dev ifb0 Before patch : class sfq 11:3d9 parent 11: (dropped 0, overlimits 0 requeues 0) backlog 0b 3p requeues 0 allot 1266 After patch : class sfq 11:3e4 parent 11: (dropped 0, overlimits 0 requeues 0) backlog 4380b 3p requeues 0 allot 1212 Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- net/sched/sch_sfq.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html