Fix RoundRobin scheduler

Need to change assumptions in ShaperClass about bandwidth and
this goes over to TrafficShaper and RR.

3 files changed, 40 insertions, 45 deletions

diff --git a/lib/Vyatta/Qos/RoundRobin.pm b/lib/Vyatta/Qos/RoundRobin.pm
index b4174d1..a94da2c 100644
--- a/lib/Vyatta/Qos/RoundRobin.pm
+++ b/lib/Vyatta/Qos/RoundRobin.pm
@@ -67,9 +67,8 @@ sub commands {
     my $classes = $self->{_classes};
     my $parent  = 1;
 
-    printf "qdisc add dev %s s handle %x: drr", $dev, 'root', $parent;
-    my $quantum = $self->{_quantum};
-    print " quantum $quantum" if ($quantum);
+    printf "qdisc add dev %s root  handle %x: drr", $dev, $parent;
+    print " quantum $self->{_quantum}" if ( $self->{_quantum} );
     print "\n";
 
     foreach my $class (@$classes) {
diff --git a/lib/Vyatta/Qos/ShaperClass.pm b/lib/Vyatta/Qos/ShaperClass.pm
index 9a0ba00..1dbfd98 100644
--- a/lib/Vyatta/Qos/ShaperClass.pm
+++ b/lib/Vyatta/Qos/ShaperClass.pm
@@ -23,52 +23,45 @@ require Vyatta::Config;
 use Vyatta::Qos::Match;
 use Vyatta::Qos::Util qw/getDsfield getRate/;
 
-my %fields = (
-    id        => undef,
-    dsmark    => undef,
-    _priority => undef,
-    _rate     => undef,
-    _ceiling  => undef,
-    _burst    => undef,
-    _match    => undef,
-    _limit    => undef,
-    _qdisc    => undef,
-);
 
 sub new {
     my ( $that, $config, $id ) = @_;
-    my $class = ref($that) || $that;
-    my $self = {%fields};
+    my $class   = ref($that) || $that;
+    my $self    = { };
 
     $self->{id} = $id;
 
     bless $self, $class;
-    $self->_define($config) if ($config);
+    
+    if ($config) {
+	my $level   = $config->setLevel();
+
+	$self->{level}     = $level;
+	$self->{_rate}     = $config->returnValue("bandwidth");
+	$self->{_priority} = $config->returnValue("priority");
+	$self->{_ceiling}  = $config->returnValue("ceiling");
+	$self->{_burst}    = $config->returnValue("burst");
+	$self->{_limit}    = $config->returnValue("queue-limit");
+	$self->{_qdisc}    = $config->returnValue("queue-type");
+
+	$self->{dsmark} = getDsfield( $config->returnValue("set-dscp") );
+	my @matches = _getMatch("$level match");
+	$self->{_match} = \@matches;
+    }
 
     return $self;
 }
 
-sub _define {
-    my ( $self, $config ) = @_;
-    my $level   = $config->setLevel();
-    my @matches = ();
-
-    $self->{_rate} = $config->returnValue("bandwidth");
-    defined $self->{_rate} or die "$level bandwidth not defined\n";
-
-    $self->{_priority} = $config->returnValue("priority");
-    $self->{_ceiling}  = $config->returnValue("ceiling");
-    $self->{_burst}    = $config->returnValue("burst");
-    $self->{_limit}    = $config->returnValue("queue-limit");
-    $self->{_qdisc}    = $config->returnValue("queue-type");
+sub _getMatch {
+    my $level = shift;
+    my @matches;
+    my $config = new Vyatta::Config;
 
-    $self->{dsmark} = getDsfield( $config->returnValue("set-dscp") );
-
-    foreach my $match ( $config->listNodes("match") ) {
-        $config->setLevel("$level match $match");
+    foreach my $match ( $config->listNodes($level) ) {
+        $config->setLevel("$level $match");
         push @matches, new Vyatta::Qos::Match($config);
     }
-    $self->{_match} = \@matches;
+    return @matches;
 }
 
 sub matchRules {
@@ -210,11 +203,11 @@ sub gen_class {
 
 sub gen_leaf {
     my ( $self, $dev, $parent, $rate ) = @_;
-    my $q = $qdiscOptions{ $self->{_qdisc} };
-    die "Unknown queue type $self->{_qdisc}\n" unless $q;
 
     printf "qdisc add dev %s parent %x:%x ", $dev, $parent, $self->{id};
-    $q->( $self, $dev, $rate );
+
+    my $q = $qdiscOptions{ $self->{_qdisc} };
+    $q->( $self, $dev, $rate ) if ($q);
 }
 
 sub dsmarkClass {
diff --git a/lib/Vyatta/Qos/TrafficShaper.pm b/lib/Vyatta/Qos/TrafficShaper.pm
index 721dac9..114bba7 100644
--- a/lib/Vyatta/Qos/TrafficShaper.pm
+++ b/lib/Vyatta/Qos/TrafficShaper.pm
@@ -33,8 +33,7 @@ sub new {
     my $level   = $config->setLevel();
     my @classes = _getClasses($level);
 
-    _checkClasses( $level, getRate($rate), @classes )
-      if ( $rate ne "auto" );
+    _checkClasses( $level, $rate, @classes );
 
     my $self = {};
     my $class = ref($that) || $that;
@@ -86,15 +85,19 @@ sub _getClasses {
     return @classes;
 }
 
+# Check constraints on class bandwidth values
 sub _checkClasses {
-    my $level   = shift;
-    my $rate    = shift;
+    my $level = shift;
+    my $rate = shift;
     my $default = shift;
-
-    $default->rateCheck( $rate, "$level default" );
+    
+    # if auto, can't check at create must wait for policy to be applied
+    $rate = ( $rate eq "auto") ? undef : getRate($rate);
+    $default->rateCheck( $rate, "$level default" ) if $rate;
 
     foreach my $class (@_) {
-        $class->rateCheck( $rate, "$level class $class->{id}" );
+	die "$class->{level} bandwidth not defined\n" unless $class->{_rate};
+        $class->rateCheck( $rate, "$level class $class->{id}" ) if $rate;
     }
 }