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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 02 Nov 2015 13:20:57 -0800 From: Eric Dumazet <eric.dumazet@...il.com> To: Cong Wang <cwang@...pensource.com> Cc: Cong Wang <xiyou.wangcong@...il.com>, netdev <netdev@...r.kernel.org>, Jamal Hadi Salim <jhs@...atatu.com> Subject: Re: [Patch net-next v2 2/4] net_sched: update hierarchical backlog too On Mon, 2015-11-02 at 13:09 -0800, Cong Wang wrote: > (Sorry for the delay) > > On Fri, Oct 30, 2015 at 12:30 PM, Eric Dumazet <eric.dumazet@...il.com> wrote: > > On Fri, 2015-10-30 at 11:22 -0700, Cong Wang wrote: > >> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > >> index 3abab53..498f0a2 100644 > >> --- a/net/sched/sch_sfq.c > >> +++ b/net/sched/sch_sfq.c > >> @@ -346,7 +346,7 @@ static int > >> sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > >> { > >> struct sfq_sched_data *q = qdisc_priv(sch); > >> - unsigned int hash; > >> + unsigned int hash, dropped; > >> sfq_index x, qlen; > >> struct sfq_slot *slot; > >> int uninitialized_var(ret); > >> @@ -461,7 +461,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > >> return NET_XMIT_SUCCESS; > >> > >> qlen = slot->qlen; > >> - sfq_drop(sch); > >> + dropped = sfq_drop(sch); > >> /* Return Congestion Notification only if we dropped a packet > >> * from this flow. > >> */ > >> @@ -469,7 +469,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > >> return NET_XMIT_CN; > > > > > > I believe you missed the NET_XMIT_CN cases. > > > > SFQ can drop a prior packet, and queue current packet. > > > > qdisc_tree_reduce_backlog() wont be called to update parents. > > > > Not sure about other qdisc(s) > > Are you saying some qdisc_tree_reduce_backlog() is missing? It could be, > since I don't audit that, if so, I'd do that in a separated patch, because this > patch is supposed to fix existing qdisc_tree_reduce_backlog(). > > Does this make sense for you? Well, before your changes, the qdisc_tree_decrease_qlen(sch, 0) would have been useless, since qdisc_tree_decrease_qlen() does nothing in this case [1] But after your changes, we need to change the backlog, even if the qlen does not change. Maybe I missed something, but your patch is not really obvious ;) [1] void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n) { ... if (n == 0) return; -- 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
Powered by blists - more mailing lists