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 <b...@nicira.com>
To: Jonathan Vestin <jonav...@kau.se>
Cc: dev@openvswitch.org
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
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
