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 dccp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
