* net_sched: dsmark: use qdisc_dequeue_peeked()
@ 2016-03-02 4:10 Kyeong Yoo
2016-03-02 14:56 ` Sergei Shtylyov
2016-03-02 18:26 ` Cong Wang
0 siblings, 2 replies; 6+ messages in thread
From: Kyeong Yoo @ 2016-03-02 4:10 UTC (permalink / raw)
To: netdev@vger.kernel.org
This fix is for dsmark similar to 3557619f0f6f7496ed453d4825e24958ab1884e0,
and makes use of qdisc_dequeue_peeked() instead of direct dequeue() call.
First time, wrr peeks dsmark, which will then peek into sfq.
sfq dequeues an skb and it's stored in sch->gso_skb.
Next time, wrr tries to dequeue from dsmark, which will call sfq dequeue
directly. This results skipping the previously peeked skb.
So changed dsmark dequeue to call qdisc_dequeue_peeked() instead to use
peeked skb if exists.
Also replaced dsmark peek to use qdisc_peek_dequeued() which works correctly
if previously peeked skb still exists.
Signed-off-by: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index f357f34..8fa3a4f 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -281,7 +281,7 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch)
pr_debug("%s(sch %p,[qdisc %p])\n", __func__, sch, p);
- skb = p->q->ops->dequeue(p->q);
+ skb = qdisc_dequeue_peeked(p->q);
if (skb == NULL)
return NULL;
@@ -315,15 +315,6 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch)
return skb;
}
-static struct sk_buff *dsmark_peek(struct Qdisc *sch)
-{
- struct dsmark_qdisc_data *p = qdisc_priv(sch);
-
- pr_debug("%s(sch %p,[qdisc %p])\n", __func__, sch, p);
-
- return p->q->ops->peek(p->q);
-}
-
static unsigned int dsmark_drop(struct Qdisc *sch)
{
struct dsmark_qdisc_data *p = qdisc_priv(sch);
@@ -491,7 +482,7 @@ static struct Qdisc_ops dsmark_qdisc_ops __read_mostly = {
.priv_size = sizeof(struct dsmark_qdisc_data),
.enqueue = dsmark_enqueue,
.dequeue = dsmark_dequeue,
- .peek = dsmark_peek,
+ .peek = qdisc_peek_dequeued,
.drop = dsmark_drop,
.init = dsmark_init,
.reset = dsmark_reset,
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: net_sched: dsmark: use qdisc_dequeue_peeked()
2016-03-02 4:10 net_sched: dsmark: use qdisc_dequeue_peeked() Kyeong Yoo
@ 2016-03-02 14:56 ` Sergei Shtylyov
2016-03-02 18:26 ` Cong Wang
1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2016-03-02 14:56 UTC (permalink / raw)
To: Kyeong Yoo, netdev@vger.kernel.org
Hello.
On 3/2/2016 7:10 AM, Kyeong Yoo wrote:
> This fix is for dsmark similar to 3557619f0f6f7496ed453d4825e24958ab1884e0,
scripts/checkpatch.pl now enforces certain commit citing format, yours
doesn't match it.
> and makes use of qdisc_dequeue_peeked() instead of direct dequeue() call.
>
> First time, wrr peeks dsmark, which will then peek into sfq.
> sfq dequeues an skb and it's stored in sch->gso_skb.
> Next time, wrr tries to dequeue from dsmark, which will call sfq dequeue
> directly. This results skipping the previously peeked skb.
>
> So changed dsmark dequeue to call qdisc_dequeue_peeked() instead to use
> peeked skb if exists.
>
> Also replaced dsmark peek to use qdisc_peek_dequeued() which works correctly
> if previously peeked skb still exists.
>
> Signed-off-by: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: net_sched: dsmark: use qdisc_dequeue_peeked()
2016-03-02 4:10 net_sched: dsmark: use qdisc_dequeue_peeked() Kyeong Yoo
2016-03-02 14:56 ` Sergei Shtylyov
@ 2016-03-02 18:26 ` Cong Wang
2016-03-02 21:19 ` Kyeong Yoo
1 sibling, 1 reply; 6+ messages in thread
From: Cong Wang @ 2016-03-02 18:26 UTC (permalink / raw)
To: Kyeong Yoo; +Cc: netdev@vger.kernel.org, Jamal Hadi Salim
On Tue, Mar 1, 2016 at 8:10 PM, Kyeong Yoo
<Kyeong.Yoo@alliedtelesis.co.nz> wrote:
> This fix is for dsmark similar to 3557619f0f6f7496ed453d4825e24958ab1884e0,
> and makes use of qdisc_dequeue_peeked() instead of direct dequeue() call.
>
> First time, wrr peeks dsmark, which will then peek into sfq.
> sfq dequeues an skb and it's stored in sch->gso_skb.
> Next time, wrr tries to dequeue from dsmark, which will call sfq dequeue
> directly. This results skipping the previously peeked skb.
>
> So changed dsmark dequeue to call qdisc_dequeue_peeked() instead to use
> peeked skb if exists.
This part looks reasonable to me.
>
> Also replaced dsmark peek to use qdisc_peek_dequeued() which works correctly
> if previously peeked skb still exists.
But not this part, since sfq already calls qdisc_peek_dequeued()?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: net_sched: dsmark: use qdisc_dequeue_peeked()
2016-03-02 18:26 ` Cong Wang
@ 2016-03-02 21:19 ` Kyeong Yoo
2016-03-02 22:01 ` Cong Wang
2016-03-03 5:52 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Kyeong Yoo @ 2016-03-02 21:19 UTC (permalink / raw)
To: xiyou.wangcong@gmail.com; +Cc: jhs@mojatatu.com, netdev@vger.kernel.org
Author: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
net_sched: dsmark: use qdisc_dequeue_peeked()
This fix is for dsmark similar to commit 3557619f0f6f7496ed453d4825e249
("net_sched: prio: use qdisc_dequeue_peeked")
and makes use of qdisc_dequeue_peeked() instead of direct dequeue() call.
First time, wrr peeks dsmark, which will then peek into sfq.
sfq dequeues an skb and it's stored in sch->gso_skb.
Next time, wrr tries to dequeue from dsmark, which will call sfq dequeue
directly. This results skipping the previously peeked skb.
So changed dsmark dequeue to call qdisc_dequeue_peeked() instead to use
peeked skb if exists.
Signed-off-by: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index f357f34..b204472 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -281,7 +281,7 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch)
pr_debug("%s(sch %p,[qdisc %p])\n", __func__, sch, p);
- skb = p->q->ops->dequeue(p->q);
+ skb = qdisc_dequeue_peeked(p->q);
if (skb == NULL)
return NULL;
On Wed, 2016-03-02 at 10:26 -0800, Cong Wang wrote:
> On Tue, Mar 1, 2016 at 8:10 PM, Kyeong Yoo
> <Kyeong.Yoo@alliedtelesis.co.nz> wrote:
> > This fix is for dsmark similar to 3557619f0f6f7496ed453d4825e24958ab1884e0,
> > and makes use of qdisc_dequeue_peeked() instead of direct dequeue() call.
> >
> > First time, wrr peeks dsmark, which will then peek into sfq.
> > sfq dequeues an skb and it's stored in sch->gso_skb.
> > Next time, wrr tries to dequeue from dsmark, which will call sfq dequeue
> > directly. This results skipping the previously peeked skb.
> >
> > So changed dsmark dequeue to call qdisc_dequeue_peeked() instead to use
> > peeked skb if exists.
>
>
> This part looks reasonable to me.
>
> >
> > Also replaced dsmark peek to use qdisc_peek_dequeued() which works correctly
> > if previously peeked skb still exists.
>
>
> But not this part, since sfq already calls qdisc_peek_dequeued()?
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: net_sched: dsmark: use qdisc_dequeue_peeked()
2016-03-02 21:19 ` Kyeong Yoo
@ 2016-03-02 22:01 ` Cong Wang
2016-03-03 5:52 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Cong Wang @ 2016-03-02 22:01 UTC (permalink / raw)
To: Kyeong Yoo; +Cc: jhs@mojatatu.com, netdev@vger.kernel.org
On Wed, Mar 2, 2016 at 1:19 PM, Kyeong Yoo
<Kyeong.Yoo@alliedtelesis.co.nz> wrote:
> Author: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
>
> net_sched: dsmark: use qdisc_dequeue_peeked()
>
> This fix is for dsmark similar to commit 3557619f0f6f7496ed453d4825e249
> ("net_sched: prio: use qdisc_dequeue_peeked")
> and makes use of qdisc_dequeue_peeked() instead of direct dequeue() call.
>
> First time, wrr peeks dsmark, which will then peek into sfq.
> sfq dequeues an skb and it's stored in sch->gso_skb.
> Next time, wrr tries to dequeue from dsmark, which will call sfq dequeue
> directly. This results skipping the previously peeked skb.
>
> So changed dsmark dequeue to call qdisc_dequeue_peeked() instead to use
> peeked skb if exists.
>
> Signed-off-by: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks!
>
> diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
> index f357f34..b204472 100644
> --- a/net/sched/sch_dsmark.c
> +++ b/net/sched/sch_dsmark.c
> @@ -281,7 +281,7 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch)
>
> pr_debug("%s(sch %p,[qdisc %p])\n", __func__, sch, p);
>
> - skb = p->q->ops->dequeue(p->q);
> + skb = qdisc_dequeue_peeked(p->q);
> if (skb == NULL)
> return NULL;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: net_sched: dsmark: use qdisc_dequeue_peeked()
2016-03-02 21:19 ` Kyeong Yoo
2016-03-02 22:01 ` Cong Wang
@ 2016-03-03 5:52 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2016-03-03 5:52 UTC (permalink / raw)
To: Kyeong.Yoo; +Cc: xiyou.wangcong, jhs, netdev
From: Kyeong Yoo <Kyeong.Yoo@alliedtelesis.co.nz>
Date: Wed, 2 Mar 2016 21:19:51 +0000
> Author: Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>
>
> net_sched: dsmark: use qdisc_dequeue_peeked()
...
> On Wed, 2016-03-02 at 10:26 -0800, Cong Wang wrote:
>> On Tue, Mar 1, 2016 at 8:10 PM, Kyeong Yoo
>> <Kyeong.Yoo@alliedtelesis.co.nz> wrote:
Please do not submit patches like this.
Specifically, do not reply to feedback you are given in order
to submit a new version of a patch.
Instead, make a new, fresh, list posting for the new version of
your patch. Indicate that this is a new version of the patch
by putting something like "[net-next v2] net_sched: dsmark: ..."
in your Subject line.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-03 5:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-02 4:10 net_sched: dsmark: use qdisc_dequeue_peeked() Kyeong Yoo
2016-03-02 14:56 ` Sergei Shtylyov
2016-03-02 18:26 ` Cong Wang
2016-03-02 21:19 ` Kyeong Yoo
2016-03-02 22:01 ` Cong Wang
2016-03-03 5:52 ` 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).