netdev - Re: [DCCP] [RFC] [Patchv3 1/1]: Queuing policies -- reworked version of Tomasz's patch set

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

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

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

Date:	Mon, 28 Apr 2008 22:12:02 +0200
From:	Tomasz Grobelny <tomasz@...belny.oswiecenia.net>
To:	Gerrit Renker <gerrit@....abdn.ac.uk>
Cc:	Arnaldo Carvalho de Melo <acme@...hat.com>, dccp@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [DCCP] [RFC] [Patchv3 1/1]: Queuing policies -- reworked version of Tomasz's patch set

Dnia Monday 28 of April 2008, Gerrit Renker napisał:
> Here is a tidied-up version, the patch sent last week and stored in
>
> 	git://eden-feed.erg.abdn.ac.uk/dccp_exp (tree `qpolicy')
>
> was not very good, the updates are:
>
>  * the skb->priority is now cleared when leaving the DCCP layer;
Ok. I only wonder if this kind of cleanup shouldn't happen in qpolicy specific 
function. Such as already non-existant qpolicy_prio_pop(). Note that 
qpolicy_simple_pop() would not need to clear skb->priority. This kind of 
function could act as skb deinitializer. And I think qpolicy_*_pop() should 
be added again...

The same question applies to skb initialization. In my opinion it should 
happen in qpolicy_*_push(). So that dccp_msghdr_parse code should be moved 
there.

This approach would clearly abstract qpolicy subsystem from the rest of dccp 
code. We would have packet initialization in qpolicy_*_push() and 
deinitialization in qpolicy_*_pop().

I'll try to write a patch that demonstrates it.

>  * there is now documentation/hints about the use of skb->priority in DCCP;
There is something I don't understand in your approach regarding 
skb->priority. I think I could understand it better it you could sketch how 
in your opinion should an interface look like if packet timeout was to be 
added.
Would you:
a) pack this data into this u32 identified by DCCP_SCM_PRIORITY. This is what 
I thought you meant (but I may be wrong). And something I don't really like.
b) add another parameter type eg. DCCP_SCM_TIMEOUT_MS and use another u32 to 
pass this parameter. This is something I like much better,
c) something else. Then what is that?

>  * there is one question I have, and that can be reverted:
>    - qpolicy_prio_push() has been replaced by a combination of
>    - qpolicy_prio_full() [which now does the drop-on-queue-full] and
>      qpolicy_simple_push() [which does the FIFO-queueing]
>    This means fewer routines, but if you don't like it, I am ok to revert
> it.
>
I don't care that much about internal implementation. But I don't really like 
mixing policies. Especially since when prio policy will have more features 
these functions will have to be separated again. When making any choices I 
always try to think how it will look like when more features are added 
(especially packet timeouts and getting statistics).
-- 
Regards,
Tomasz Grobelny
--
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