From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamal Hadi Salim Subject: Re: [PATCH net-next] net:sched: add gkprio scheduler Date: Wed, 9 May 2018 10:43:46 -0400 Message-ID: References: <20180507093626.GA5794@gmail.com> <20180508101210.GB4383@gmail.com> <273f91db-7a2f-acda-b306-5a78dd948478@digirati.com.br> <2855ce67-f1bc-c04c-81ae-70ae3fdc6b17@mojatatu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Michel Machado , Nishanth Devarajan , Jiri Pirko , David Miller , Linux Kernel Network Developers , Cody Doucette To: Cong Wang Return-path: Received: from mail-qk0-f171.google.com ([209.85.220.171]:32832 "EHLO mail-qk0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752098AbeEIOny (ORCPT ); Wed, 9 May 2018 10:43:54 -0400 Received: by mail-qk0-f171.google.com with SMTP id c11so4034528qkm.0 for ; Wed, 09 May 2018 07:43:54 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 08/05/18 10:27 PM, Cong Wang wrote: > On Tue, May 8, 2018 at 6:29 AM, Jamal Hadi Salim wrote: >> Have you considered using skb->prio instead of peeking into the packet >> header. >> Also have you looked at the dsmark qdisc? >> > > dsmark modifies ds fields, while this one just maps ds fields into > different queues. > Yeah, I was thinking more of re-using it for the purpose of mapping to queues - but would require a lot more work. once skbprio is set by something[1] then this qdisc could be used by other subsystems (8021q, sockets etc); so i would argue for removal of the embedded classification and instead maybe writing a simple extension to skbmod to mark skbprio based on ds. I find the cleverness in changing the highest/low prios confusing. It looks error-prone (I guess that is why there is a BUG check) To the authors: Is there a document/paper on the theory of this thing as to why no explicit queues are "faster"? Some other feedback: 1) I agree that using multiple queues as in prio qdisc would make it more manageable; does not necessarily need to be classful if you use implicit skbprio classification. i.e on equeue use a priority map to select a queue; on dequeue always dequeu from highest prio until it has no more packets to send. 2) Dropping already enqueued packets will not work well for local feedback (__NET_XMIT_BYPASS return code is about the packet that has been dropped from earlier enqueueing because it is lower priority - it does not signify anything with current skb to which actually just got enqueud). Perhaps (off top of my head) is to always enqueue packets on high priority when their limit is exceeded as long as lower prio has some space. Means youd have to increment low prio accounting if their space is used. cheers, jamal [1] something like: tc filter add match all ip action skbmod inheritdsfield tc filter add match all ip6 action skbmod inheritdsfield inheritdsfield maps ds to skb->prioriry