[ovs-dev] Patch adding additional QoS strategies to OVS

Ben Pfaff blp at nicira.com
Tue Mar 10 17:54:35 UTC 2015


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.



More information about the dev mailing list