From: John Fastabend <john.r.fastabend@intel.com>
To: Eric Dumazet <eric.dumazet@gmail.com>,
John Fastabend <john.fastabend@gmail.com>
Cc: xiyou.wangcong@gmail.com, jhs@mojatatu.com, davem@davemloft.net,
netdev@vger.kernel.org
Subject: Re: [net-next PATCH 2/4] net: sched: implement qstat helper routines
Date: Fri, 26 Sep 2014 11:41:56 -0700 [thread overview]
Message-ID: <5425B374.2040304@intel.com> (raw)
In-Reply-To: <1411756601.16953.136.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/26/2014 11:36 AM, Eric Dumazet wrote:
> On Fri, 2014-09-26 at 11:14 -0700, John Fastabend wrote:
>> This adds helpers to manipulate qstats logic and replaces locations
>> that touch the counters directly. This simplifies future patches
>> to push qstats onto per cpu counters.
>>
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>> ---
>> include/net/sch_generic.h | 43 +++++++++++++++++++++++++++++++++++--------
>> net/sched/sch_api.c | 2 +-
>> net/sched/sch_atm.c | 2 +-
>> net/sched/sch_cbq.c | 10 +++++-----
>> net/sched/sch_choke.c | 14 +++++++-------
>> net/sched/sch_codel.c | 2 +-
>> net/sched/sch_drr.c | 4 ++--
>> net/sched/sch_dsmark.c | 2 +-
>> net/sched/sch_fifo.c | 2 +-
>> net/sched/sch_fq.c | 4 ++--
>> net/sched/sch_fq_codel.c | 8 ++++----
>> net/sched/sch_gred.c | 4 ++--
>> net/sched/sch_hfsc.c | 8 ++++----
>> net/sched/sch_hhf.c | 8 ++++----
>> net/sched/sch_htb.c | 6 +++---
>> net/sched/sch_ingress.c | 2 +-
>> net/sched/sch_multiq.c | 4 ++--
>> net/sched/sch_netem.c | 15 +++++++--------
>> net/sched/sch_pie.c | 2 +-
>> net/sched/sch_prio.c | 4 ++--
>> net/sched/sch_qfq.c | 4 ++--
>> net/sched/sch_red.c | 8 ++++----
>> net/sched/sch_sfb.c | 10 +++++-----
>> net/sched/sch_sfq.c | 17 +++++++++--------
>> net/sched/sch_tbf.c | 8 ++++----
>> 25 files changed, 110 insertions(+), 83 deletions(-)
>>
>> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>> index 4b93511..5609844 100644
>> --- a/include/net/sch_generic.h
>> +++ b/include/net/sch_generic.h
>> @@ -521,11 +521,39 @@ static inline void qdisc_bstats_update(struct Qdisc *sch,
>> bstats_update(&sch->bstats, skb);
>> }
>>
>> +static inline void qdisc_qstats_backlog(struct Qdisc *sch,
>> + const struct sk_buff *skb)
>
> qdisc_qstats_backlog_dec() to be consistent with
> qdisc_qstats_backlog_dec() ?
>
>> +{
>> + sch->qstats.backlog -= qdisc_pkt_len(skb);
>> +}
>> +
>> +static inline void qdisc_qstats_backlog_inc(struct Qdisc *sch,
>> + const struct sk_buff *skb)
>> +{
>> +
>> + sch->qstats.backlog += qdisc_pkt_len(skb);
>> +}
>> +
>> +static inline void __qdisc_qstats_drop(struct Qdisc *sch, int count)
>> +{
>> + sch->qstats.drops += count;
>> +}
>> +
>> +static inline void qdisc_qstats_drop(struct Qdisc *sch)
>> +{
>> + sch->qstats.drops++;
>> +}
>> +
>> +static inline void qdisc_qstats_overlimit(struct Qdisc *sch)
>> +{
>> + sch->qstats.overlimits++;
>> +}
>> +
>> static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
>> struct sk_buff_head *list)
>> {
>> __skb_queue_tail(list, skb);
>> - sch->qstats.backlog += qdisc_pkt_len(skb);
>> + qdisc_qstats_backlog_inc(sch, skb);
>>
>> return NET_XMIT_SUCCESS;
>> }
>> @@ -541,7 +569,7 @@ static inline struct sk_buff *__qdisc_dequeue_head(struct Qdisc *sch,
>> struct sk_buff *skb = __skb_dequeue(list);
>>
>> if (likely(skb != NULL)) {
>> - sch->qstats.backlog -= qdisc_pkt_len(skb);
>> + qdisc_qstats_backlog(sch, skb);
>> qdisc_bstats_update(sch, skb);
>> }
>>
>> @@ -559,10 +587,9 @@ static inline unsigned int __qdisc_queue_drop_head(struct Qdisc *sch,
>> struct sk_buff *skb = __skb_dequeue(list);
>>
>> if (likely(skb != NULL)) {
>> - unsigned int len = qdisc_pkt_len(skb);
>> - sch->qstats.backlog -= len;
>> + qdisc_qstats_backlog(sch, skb);
>> kfree_skb(skb);
>> - return len;
>> + return qdisc_pkt_len(skb);
>
> Well, you just added a use after free.
hmm should have held off on the v2 I guess. I'll fix this
and add a test for the only drop_head consumer pfifo_head_drop.
Thanks!
>
>> }
>>
>> return 0;
>> @@ -579,7 +606,7 @@ static inline struct sk_buff *__qdisc_dequeue_tail(struct Qdisc *sch,
>> struct sk_buff *skb = __skb_dequeue_tail(list);
>>
>> if (likely(skb != NULL))
>
>
> Rest of the patch seems fine, 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
>
next prev parent reply other threads:[~2014-09-26 18:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-26 18:13 [net-next PATCH 1/4] net: sched: make bstats per cpu and estimator RCU safe John Fastabend
2014-09-26 18:14 ` [net-next PATCH 2/4] net: sched: implement qstat helper routines John Fastabend
2014-09-26 18:36 ` Eric Dumazet
2014-09-26 18:41 ` John Fastabend [this message]
2014-09-26 18:15 ` [net-next PATCH 3/4] net: sched: restrict use of qstats qlen John Fastabend
2014-09-26 18:15 ` [net-next PATCH 4/4] net: sched: enable per cpu qstats John Fastabend
2014-09-26 18:21 ` [net-next PATCH 1/4] net: sched: make bstats per cpu and estimator RCU safe John Fastabend
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=5425B374.2040304@intel.com \
--to=john.r.fastabend@intel.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=jhs@mojatatu.com \
--cc=john.fastabend@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=xiyou.wangcong@gmail.com \
/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).