Date:   Wed, 9 May 2018 13:37:05 -0400
From:   Michel Machado <michel@...irati.com.br>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>
Cc:     Nishanth Devarajan <ndev2021@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Cody Doucette <doucette@...edu>
Subject: Re: [PATCH net-next] net:sched: add gkprio scheduler

On 05/09/2018 10:43 AM, Jamal Hadi Salim wrote:
> On 08/05/18 10:27 PM, Cong Wang wrote:
>> On Tue, May 8, 2018 at 6:29 AM, Jamal Hadi Salim <jhs@...atatu.com> 
>> 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 like the suggestion of extending skbmod to mark skbprio based on ds. 
Given that DSprio would no longer depend on the DS field, would you have 
a name suggestion for this new queue discipline since the name "prio" is 
currently in use?

What should be the range of priorities that this new queue discipline 
would accept? skb->prioriry is of type __u32, but supporting 2^32 
priorities would require too large of an array to index packets by 
priority; the DS field is only 6 bits long. Do you have a use case in 
mind to guide us here?

> 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"?

The priority orientation in GKprio is due to two factors: failing safe 
and elegance. If zero were the highest priority, any operational mistake 
that leads not-classified packets through GKprio would potentially 
disrupt the system. We are humans, we'll make mistakes. The elegance 
aspect comes from the fact that the assigned priority is not massaged to 
fit the DS field. We find it helpful while inspecting packets on the wire.

The reason for us to avoid explicit queues in GKprio, which could change 
the behavior within a given priority, is to closely abide to the 
expected behavior assumed to prove Theorem 4.1 in the paper "Portcullis: 
Protecting Connection Setup from Denial-of-Capability Attacks":

https://dl.acm.org/citation.cfm?id=1282413

> 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.

In my reply to Cong, I point out that there is a technical limitation 
in the interface of queue disciplines that forbids GKprio to have 
explicit sub-queues:

https://www.mail-archive.com/netdev@vger.kernel.org/msg234201.html

> 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.

I don't understand the point you are making here. Could you develop it 
further?

[ ]'s
Michel Machado

Powered by blists - more mailing lists