| Message ID | 1465389093-29674-1-git-send-email-fw@strlen.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2016-06-08 at 14:31 +0200, Florian Westphal wrote: > When we need to create a new aggregate to enqueue the skb we need > to kzalloc the new struct. > > If that fails we returned ENOBUFS without freeing the skb. > > Spotted during code review. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > net/sched/sch_qfq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > index 8d2d8d9..435b970 100644 > --- a/net/sched/sch_qfq.c > +++ b/net/sched/sch_qfq.c > @@ -1236,7 +1236,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > err = qfq_change_agg(sch, cl, cl->agg->class_weight, > qdisc_pkt_len(skb)); > if (err) > - return err; > + return qdisc_drop(skb, sch); > } > > err = qdisc_enqueue(skb, cl->qdisc); Good catch, but it looks like you forgot : cl->qstats.drops++;
Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Wed, 2016-06-08 at 14:31 +0200, Florian Westphal wrote: > > When we need to create a new aggregate to enqueue the skb we need > > to kzalloc the new struct. > > > > If that fails we returned ENOBUFS without freeing the skb. > > > > Spotted during code review. > > > > Signed-off-by: Florian Westphal <fw@strlen.de> > > --- > > net/sched/sch_qfq.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > > index 8d2d8d9..435b970 100644 > > --- a/net/sched/sch_qfq.c > > +++ b/net/sched/sch_qfq.c > > @@ -1236,7 +1236,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > > err = qfq_change_agg(sch, cl, cl->agg->class_weight, > > qdisc_pkt_len(skb)); > > if (err) > > - return err; > > + return qdisc_drop(skb, sch); > > } > > > > err = qdisc_enqueue(skb, cl->qdisc); > > Good catch, but it looks like you forgot : > > cl->qstats.drops++; Thanks, we have too many stats :-/ I'll send a v2, thanks!
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c index 8d2d8d9..435b970 100644 --- a/net/sched/sch_qfq.c +++ b/net/sched/sch_qfq.c @@ -1236,7 +1236,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch) err = qfq_change_agg(sch, cl, cl->agg->class_weight, qdisc_pkt_len(skb)); if (err) - return err; + return qdisc_drop(skb, sch); } err = qdisc_enqueue(skb, cl->qdisc);
When we need to create a new aggregate to enqueue the skb we need to kzalloc the new struct. If that fails we returned ENOBUFS without freeing the skb. Spotted during code review. Signed-off-by: Florian Westphal <fw@strlen.de> --- net/sched/sch_qfq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)