Ok, quite some things to do, I will fix the things you mentioned after sorting out the tc_load problem.
Regarding tc_load: See the comment in: tc_parse_qdisc at 4414 to 4420: * To avoid the OOPS, we must not make a request that would attempt to dump * a "built-in" qdisc, that is, the default pfifo_fast qdisc or one of a * few others. There are a few ways that I can see to do this, but most of * them seem to be racy (and if you lose the race the kernel OOPSes). The * technique chosen here is to assume that any non-default qdisc that we * create will have a class with handle 1:0. The built-in qdiscs only have * a class with handle 0:0. I presume this means that in order for tc_load to be called, the qdisc *must* have one class, which classless qdiscs cannot have. Please correct me if I'm wrong. -----Original Message----- From: Ben Pfaff <[email protected]> To: Jonathan Vestin <[email protected]> Cc: [email protected] Date: Tue, 10 Mar 2015 10:54:35 -0700 Subject: Re: [ovs-dev] Patch adding additional QoS strategies to OVS On Tue, Mar 10, 2015 at 04:22:12PM +0100, Jonathan Vestin wrote: > Hello, I finally made my changes to OVS into a patch. I am a beginner in doing this kind of low-level programming, and it is my first patch to any open source project, so before submitting a patch, it would be nice with some code review. > > This patch adds support for SFQ, CoDel and FQ_CoDel classless qdiscs to Open vSwitch. It also removes the requirement for a QoS to have at least one Queue (as this makes no sense when using classless qdiscs). I have not implemented tc_load it is never run on qdiscs without classes. I have also not implemented class_{get,set,delete,get_stats,dump_stats} because they are meant for qdiscs with classes. > > The only problem now is that tc_load does not work unless the qdisc has at least one class, which is beyond my skill to fix. > > Any advice would be appreciated. The code is tested with 'make', 'make check', 'make testcheck' and manually. Hi Jonathan! Thanks for the contribution. We can always use support for new qdiscs. Your code builds without warnings on Clang and GCC, and "sparse" does not complain, which is very good. One coding style violation I noticed is that even single statements should be enclosed in {}, e.g.: if (a > b) { return a; } else { return b; } This patch should also update vswitch.xml to document the new qdisc support. It should also add an item to NEWS that mentions the new qdisc support. The problem you report with tc_load isn't obvious to me from the caller in tc_query_qdisc. Can you explain it a little further? To get this committed, you'll need to "sign off" on it. CONTRIBUTING.md describes how to do this and what it means. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
