netdev - Re: [PATCH] sch_htb: ix the deficit overflows

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 30 Nov 2009 12:26:33 +0800
From:	Changli Gao <xiaosuo@...il.com>
To:	Jarek Poplawski <jarkao2@...il.com>
Cc:	Jamal Hadi Salim <hadi@...erus.ca>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] sch_htb: ix the deficit overflows

On Sat, Nov 28, 2009 at 8:04 AM, Jarek Poplawski <jarkao2@...il.com> wrote:
> Changli Gao wrote, On 11/27/2009 09:14 AM:
>
>
> This case of the quantum smaller than the packet size should be treated
> as a broken config, so I don't think it's worth to do such a deep change
> with additional delays and cpu cycles for all to fix it. A warning or
> lower limit should be enough (if necessary at all).
>

I don't think this change is deep. HTB has it own lower limit for
quantum 1000, but the MTU is various, and maybe larger than that. And
if we use IMQ to shape traffic, the skb will be defragmented by
conntrack, and its size will be larger than MTU.

The previous patch indeed introduces some additional CPU cycles.
Review  the new patch bellow please:

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 2e38d1a..d55382b 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -815,6 +815,17 @@ next:
                        goto next;
                }

+               if (unlikely(cl->un.leaf.deficit[level] < 0)) {
+                       cl->un.leaf.deficit[level] += cl->quantum;
+                       htb_next_rb_node((level ? cl->parent->un.inner.ptr :
+                                         q->ptr[0]) + prio);
+                       cl = htb_lookup_leaf(q->row[level] + prio, prio,
+                                            q->ptr[level] + prio,
+                                            q->last_ptr_id[level] + prio);
+                       start = cl;
+                       goto next;
+               }
+
                skb = cl->un.leaf.q->dequeue(cl->un.leaf.q);
                if (likely(skb != NULL))
                        break;

If you think it is acceptable, I'll resubmit it for inclusion.

-- 
Regards，
Changli Gao(xiaosuo@...il.com)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html