* [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full
@ 2005-04-25 19:15 Asim Shankar
2005-04-28 19:04 ` David S. Miller
0 siblings, 1 reply; 4+ messages in thread
From: Asim Shankar @ 2005-04-25 19:15 UTC (permalink / raw)
To: netdev; +Cc: davem
htb_enqueue(): Free skb and return NET_XMIT_DROP if a packet is destined
for the direct_queue but the direct_queue is full. (Before patch:
Erroneously returned NET_XMIT_SUCCESS even though packet was not enqueued)
Signed-off-by: Asim Shankar <asimshankar@gmail.com>
--- linux-2.6.11.7/net/sched/sch_htb.c.orig 2005-04-21 17:40:05.305709014 -0500
+++ linux-2.6.11.7/net/sched/sch_htb.c 2005-04-21 17:35:27.872624173 -0500
@@ -717,6 +717,10 @@ static int htb_enqueue(struct sk_buff *s
if (q->direct_queue.qlen < q->direct_qlen) {
__skb_queue_tail(&q->direct_queue, skb);
q->direct_pkts++;
+ } else {
+ kfree_skb(skb);
+ sch->qstats.drops++;
+ return NET_XMIT_DROP;
}
#ifdef CONFIG_NET_CLS_ACT
} else if (!cl) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full
2005-04-25 19:15 [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full Asim Shankar
@ 2005-04-28 19:04 ` David S. Miller
2005-04-29 17:35 ` Asim Shankar
0 siblings, 1 reply; 4+ messages in thread
From: David S. Miller @ 2005-04-28 19:04 UTC (permalink / raw)
To: Asim Shankar; +Cc: netdev
On Mon, 25 Apr 2005 14:15:05 -0500 (CDT)
Asim Shankar <asimshankar@gmail.com> wrote:
> htb_enqueue(): Free skb and return NET_XMIT_DROP if a packet is destined
> for the direct_queue but the direct_queue is full. (Before patch:
> Erroneously returned NET_XMIT_SUCCESS even though packet was not enqueued)
>
> Signed-off-by: Asim Shankar <asimshankar@gmail.com>
Your patch is mangled by your email client. The non-changing lines
in the patch have two leading spaces, which is wrong. So the patch
will not apply.
^ permalink raw reply [flat|nested] 4+ messages in thread
* BUG: HTB?
@ 2005-04-21 19:21 Asim Shankar
2005-04-21 19:56 ` Thomas Graf
0 siblings, 1 reply; 4+ messages in thread
From: Asim Shankar @ 2005-04-21 19:21 UTC (permalink / raw)
To: netdev
Hi,
I think there is a bug in htb_enqueue() (net/sched/sch_htb.c)
Specifically, in the lines:
if (cl == HTB_DIRECT) {
/* enqueue to helper queue */
if (q->direct_queue.qlen < q->direct_qlen) {
__skb_queue_tail(&q->direct_queue, skb);
q->direct_pkts++;
}
}
If a packet is classified as HTB_DIRECT but the direct_queue is
already full, then the packet doesn't get enqueued but sch->q.qlen++
will happen a few lines later. Overflowing of the direct_queue
probably rarely happens in practice, but I was playing around and
noticed it happen in some corner cases of my testing.
Should the packet be dropped instead? Like:
if (cl == HTB_DIRECT) {
/* enqueue to helper queue */
if (q->direct_queue.qlen < q->direct_qlen) {
__skb_queue_tail(&q->direct_queue, skb);
q->direct_pkts++;
} else {
sch->qstats.drops++;
kfree_skb(skb);
return NET_XMIT_DROP;
}
}
Thanks,
-- Asim
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: BUG: HTB?
2005-04-21 19:21 BUG: HTB? Asim Shankar
@ 2005-04-21 19:56 ` Thomas Graf
2005-04-21 20:41 ` [PATCH] - sch_htb: Drop packet when direct queue overflows Asim Shankar
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Graf @ 2005-04-21 19:56 UTC (permalink / raw)
To: Asim Shankar; +Cc: netdev
Hi,
* Asim Shankar <7bca1cb50504211221655fd54c@mail.gmail.com> 2005-04-21 14:21
> Specifically, in the lines:
> if (cl == HTB_DIRECT) {
> /* enqueue to helper queue */
> if (q->direct_queue.qlen < q->direct_qlen) {
> __skb_queue_tail(&q->direct_queue, skb);
> q->direct_pkts++;
> }
> }
>
> If a packet is classified as HTB_DIRECT but the direct_queue is
> already full, then the packet doesn't get enqueued but sch->q.qlen++
> will happen a few lines later. Overflowing of the direct_queue
> probably rarely happens in practice, but I was playing around and
> noticed it happen in some corner cases of my testing.
Yes, that's a bug.
> Should the packet be dropped instead? Like:
>
> if (cl == HTB_DIRECT) {
> /* enqueue to helper queue */
> if (q->direct_queue.qlen < q->direct_qlen) {
> __skb_queue_tail(&q->direct_queue, skb);
> q->direct_pkts++;
> } else {
> sch->qstats.drops++;
> kfree_skb(skb);
> return NET_XMIT_DROP;
> }
> }
Can you send a patch?
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] - sch_htb: Drop packet when direct queue overflows
2005-04-21 19:56 ` Thomas Graf
@ 2005-04-21 20:41 ` Asim Shankar
2005-04-21 21:54 ` Thomas Graf
0 siblings, 1 reply; 4+ messages in thread
From: Asim Shankar @ 2005-04-21 20:41 UTC (permalink / raw)
To: Thomas Graf; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]
> > If a packet is classified as HTB_DIRECT but the direct_queue is
> > already full, then the packet doesn't get enqueued but sch->q.qlen++
> > will happen a few lines later. Overflowing of the direct_queue
> > probably rarely happens in practice, but I was playing around and
> > noticed it happen in some corner cases of my testing.
>
> Yes, that's a bug.
>
> > Should the packet be dropped instead? Like:
> >
> > if (cl == HTB_DIRECT) {
> > /* enqueue to helper queue */
> > if (q->direct_queue.qlen < q->direct_qlen) {
> > __skb_queue_tail(&q->direct_queue, skb);
> > q->direct_pkts++;
> > } else {
> > sch->qstats.drops++;
> > kfree_skb(skb);
> > return NET_XMIT_DROP;
> > }
> > }
>
> Can you send a patch?
>
Patched pasted and attached:
--- linux-2.6.11.7/net/sched/sch_htb.c 2005-04-07 13:57:45.000000000 -0500
+++ linux-2.6.11.7-new/net/sched/sch_htb.c 2005-04-21 14:17:36.272065816 -0500
@@ -717,6 +717,10 @@
if (q->direct_queue.qlen < q->direct_qlen) {
__skb_queue_tail(&q->direct_queue, skb);
q->direct_pkts++;
+ } else {
+ kfree_skb (skb);
+ sch->qstats.drops++;
+ return NET_XMIT_DROP;
}
#ifdef CONFIG_NET_CLS_ACT
} else if (!cl) {
[-- Attachment #2: patch-htb-2.6.11.7 --]
[-- Type: text/plain, Size: 436 bytes --]
--- linux-2.6.11.7/net/sched/sch_htb.c 2005-04-07 13:57:45.000000000 -0500
+++ linux-2.6.11.7-new/net/sched/sch_htb.c 2005-04-21 14:17:36.272065816 -0500
@@ -717,6 +717,10 @@
if (q->direct_queue.qlen < q->direct_qlen) {
__skb_queue_tail(&q->direct_queue, skb);
q->direct_pkts++;
+ } else {
+ kfree_skb (skb);
+ sch->qstats.drops++;
+ return NET_XMIT_DROP;
}
#ifdef CONFIG_NET_CLS_ACT
} else if (!cl) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] - sch_htb: Drop packet when direct queue overflows
2005-04-21 20:41 ` [PATCH] - sch_htb: Drop packet when direct queue overflows Asim Shankar
@ 2005-04-21 21:54 ` Thomas Graf
2005-04-21 23:02 ` [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full Asim Shankar
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Graf @ 2005-04-21 21:54 UTC (permalink / raw)
To: Asim Shankar; +Cc: netdev, devik
[CCed Martin Devera]
* Asim Shankar <7bca1cb505042113417a5d9f59@mail.gmail.com> 2005-04-21 15:41
> Patched pasted and attached:
For patch formatting details see http://linux.yyz.us/patch-format.html
A short description of the patch would be helpful.
Besides of that...
Signed-off-by: Thomas Graf <tgraf@suug.ch>
> --- linux-2.6.11.7/net/sched/sch_htb.c 2005-04-07 13:57:45.000000000 -0500
> +++ linux-2.6.11.7-new/net/sched/sch_htb.c 2005-04-21 14:17:36.272065816 -0500
> @@ -717,6 +717,10 @@
> if (q->direct_queue.qlen < q->direct_qlen) {
> __skb_queue_tail(&q->direct_queue, skb);
> q->direct_pkts++;
> + } else {
> + kfree_skb (skb);
Could you write this without the whitespace?
> + sch->qstats.drops++;
> + return NET_XMIT_DROP;
> }
> #ifdef CONFIG_NET_CLS_ACT
> } else if (!cl) {
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full
2005-04-21 21:54 ` Thomas Graf
@ 2005-04-21 23:02 ` Asim Shankar
0 siblings, 0 replies; 4+ messages in thread
From: Asim Shankar @ 2005-04-21 23:02 UTC (permalink / raw)
To: netdev; +Cc: devik, Thomas Graf
htb_enqueue(): Free skb and return NET_XMIT_DROP if a packet is
destined for the direct_queue but the direct_queue is full. (Before
this: erroneously returned NET_XMIT_SUCCESS even though the packet was
not enqueued)
Signed-off-by: Asim Shankar <asimshankar@gmail.com>
--- linux-2.6.11.7/net/sched/sch_htb.c.orig 2005-04-21 17:40:05.305709014 -0500
+++ linux-2.6.11.7/net/sched/sch_htb.c 2005-04-21 17:35:27.872624173 -0500
@@ -717,6 +717,10 @@ static int htb_enqueue(struct sk_buff *s
if (q->direct_queue.qlen < q->direct_qlen) {
__skb_queue_tail(&q->direct_queue, skb);
q->direct_pkts++;
+ } else {
+ kfree_skb(skb);
+ sch->qstats.drops++;
+ return NET_XMIT_DROP;
}
#ifdef CONFIG_NET_CLS_ACT
} else if (!cl) {
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2005-04-29 17:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-04-25 19:15 [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full Asim Shankar
2005-04-28 19:04 ` David S. Miller
2005-04-29 17:35 ` Asim Shankar
-- strict thread matches above, loose matches on Subject: below --
2005-04-21 19:21 BUG: HTB? Asim Shankar
2005-04-21 19:56 ` Thomas Graf
2005-04-21 20:41 ` [PATCH] - sch_htb: Drop packet when direct queue overflows Asim Shankar
2005-04-21 21:54 ` Thomas Graf
2005-04-21 23:02 ` [PATCH 2.6.11.7] sch_htb: Drop packet when direct queue is full Asim Shankar
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).