Message ID | 1312891483-30034-1-git-send-email-fw@strlen.de |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Le mardi 09 août 2011 à 14:04 +0200, Florian Westphal a écrit : > commit 07bd8df5df4369487812bf85a237322ff3569b77 > (sch_sfq: fix peek() implementation) changed sfq to use generic > peek helper. > > This makes HFSC complain about a non-work-conserving child qdisc, if > prio with sfq child is used within hfsc: > > hfsc peeks into prio qdisc, which will then peek into sfq. > returned skb is stashed in sch->gso_skb. > > Next, hfsc tries to dequeue from prio, but prio will call sfq dequeue > directly, which may return NULL instead of previously peeked-at skb. > > Have prio call qdisc_dequeue_peeked, so sfq->dequeue() is > not called in this case. > > Cc: Eric Dumazet <eric.dumazet@gmail.com> > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > Eric, does this look correct to you? > I am not sure if sfq needs fixing instead of this patch here. > > diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c > index 2a318f2..b5d56a2 100644 > --- a/net/sched/sch_prio.c > +++ b/net/sched/sch_prio.c > @@ -112,7 +112,7 @@ static struct sk_buff *prio_dequeue(struct Qdisc *sch) > > for (prio = 0; prio < q->bands; prio++) { > struct Qdisc *qdisc = q->queues[prio]; > - struct sk_buff *skb = qdisc->dequeue(qdisc); > + struct sk_buff *skb = qdisc_dequeue_peeked(qdisc); > if (skb) { > qdisc_bstats_update(sch, skb); > sch->q.qlen--; Hi Florian Are you sure this patch is still needed, after commit e1738bd9cecc5c867b0e2996470c1ff20f66ba79 (sch_sfq: fix sfq_enqueue()) I am asking before fully reviewing the code, I dont know who should/shouldnt call qdisc_dequeue_peeked() instead of qdisc->dequeue() 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
Eric Dumazet <eric.dumazet@gmail.com> wrote: > > Eric, does this look correct to you? > > I am not sure if sfq needs fixing instead of this patch here. > > > > diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c > > index 2a318f2..b5d56a2 100644 > > --- a/net/sched/sch_prio.c > > +++ b/net/sched/sch_prio.c > > @@ -112,7 +112,7 @@ static struct sk_buff *prio_dequeue(struct Qdisc *sch) > > > > for (prio = 0; prio < q->bands; prio++) { > > struct Qdisc *qdisc = q->queues[prio]; > > - struct sk_buff *skb = qdisc->dequeue(qdisc); > > + struct sk_buff *skb = qdisc_dequeue_peeked(qdisc); > > if (skb) { > > qdisc_bstats_update(sch, skb); > > sch->q.qlen--; > > > Hi Florian > > Are you sure this patch is still needed, after commit > e1738bd9cecc5c867b0e2996470c1ff20f66ba79 > (sch_sfq: fix sfq_enqueue()) Yes, I double checked. This patch is applied, also I see HFSC complaints even without that code path touched by "sch_sfq: fix sfq_enqueue" being hit. -- 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: Florian Westphal <fw@strlen.de> Date: Tue, 9 Aug 2011 14:04:43 +0200 > commit 07bd8df5df4369487812bf85a237322ff3569b77 > (sch_sfq: fix peek() implementation) changed sfq to use generic > peek helper. > > This makes HFSC complain about a non-work-conserving child qdisc, if > prio with sfq child is used within hfsc: > > hfsc peeks into prio qdisc, which will then peek into sfq. > returned skb is stashed in sch->gso_skb. > > Next, hfsc tries to dequeue from prio, but prio will call sfq dequeue > directly, which may return NULL instead of previously peeked-at skb. > > Have prio call qdisc_dequeue_peeked, so sfq->dequeue() is > not called in this case. > > Cc: Eric Dumazet <eric.dumazet@gmail.com> > Signed-off-by: Florian Westphal <fw@strlen.de> Applied and queued up for -stable, 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
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c index 2a318f2..b5d56a2 100644 --- a/net/sched/sch_prio.c +++ b/net/sched/sch_prio.c @@ -112,7 +112,7 @@ static struct sk_buff *prio_dequeue(struct Qdisc *sch) for (prio = 0; prio < q->bands; prio++) { struct Qdisc *qdisc = q->queues[prio]; - struct sk_buff *skb = qdisc->dequeue(qdisc); + struct sk_buff *skb = qdisc_dequeue_peeked(qdisc); if (skb) { qdisc_bstats_update(sch, skb); sch->q.qlen--;
commit 07bd8df5df4369487812bf85a237322ff3569b77 (sch_sfq: fix peek() implementation) changed sfq to use generic peek helper. This makes HFSC complain about a non-work-conserving child qdisc, if prio with sfq child is used within hfsc: hfsc peeks into prio qdisc, which will then peek into sfq. returned skb is stashed in sch->gso_skb. Next, hfsc tries to dequeue from prio, but prio will call sfq dequeue directly, which may return NULL instead of previously peeked-at skb. Have prio call qdisc_dequeue_peeked, so sfq->dequeue() is not called in this case. Cc: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: Florian Westphal <fw@strlen.de> --- Eric, does this look correct to you? I am not sure if sfq needs fixing instead of this patch here.