Removing dependency on lower level drivers for the BGPRouter application.

Also a couple of bug fixes Change-Id: I8f2ec58ed3403ae03cf7c068dabb30ae4272ff85

Removing dependency on lower level drivers for the BGPRouter application.
Also a couple of bug fixes Change-Id: I8f2ec58ed3403ae03cf7c068dabb30ae4272ff85
Saurav Das
Commit 3ea46622c8fcdbbad4c25a62cda4ce9bbee55e20 3ea46622 1 parent e3bfcafa
Showing 7 changed files with 72 additions and 56 deletions
apps/bgprouter/src/main/java/org/onosproject/bgprouter/BgpRouter.java
core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
drivers/src/main/java/org/onosproject/driver/pipeline/OVSCorsaPipeline.java
openflow/drivers/src/main/java/org/onosproject/openflow/drivers/OFCorsaSwitchDriver.java
openflow/drivers/src/main/java/org/onosproject/openflow/drivers/OFOVSSwitchCorsaTTP.java
providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java
--- a/apps/bgprouter/src/main/java/org/onosproject/bgprouter/BgpRouter.java
View file @3ea4662
+++ b/apps/bgprouter/src/main/java/org/onosproject/bgprouter/BgpRouter.java
View file @3ea4662
@@ -252,10 +252,8 @@ public class BgpRouter {
             flowObjectiveService.forward(deviceId,
                                          generateRibFlowRule(fibEntry.prefix(), nextId).add());
-
+            log.trace("Sending flow forwarding objective {}->{}", fibEntry, nextId);
-
         }
-        log.info("Sending flow forwarding objective");
     }
@@ -325,8 +323,10 @@ public class BgpRouter {
                     .setOutput(egressIntf.connectPoint().port())
                     .build();
+            int nextId = flowObjectiveService.allocateNextId();
+
             NextObjective nextObjective = DefaultNextObjective.builder()
-                    .withId(entry.hashCode())
+                    .withId(nextId)
                     .addTreatment(treatment)
                     .withType(NextObjective.Type.SIMPLE)
                     .fromApp(appId)
@@ -348,7 +348,7 @@ public class BgpRouter {
             groupService.addGroup(groupDescription);
             */
-            nextHops.put(nextHop.ip(), flowObjectiveService.allocateNextId());
+            nextHops.put(nextHop.ip(), nextId);
         }
--- a/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
View file @3ea4662
+++ b/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
View file @3ea4662
@@ -365,7 +365,7 @@ public class DefaultFlowRule implements FlowRule {
                     "a timeout or be permanent");
             checkNotNull(deviceId != null, "Must refer to a device");
             checkNotNull(priority != null, "Priority cannot be null");
-            checkArgument(priority < MIN_PRIORITY, "Priority cannot be less than " +
+            checkArgument(priority >= MIN_PRIORITY, "Priority cannot be less than " +
                     MIN_PRIORITY);
             return new DefaultFlowRule(deviceId, selector, treatment, priority,
--- a/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
View file @3ea4662
+++ b/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
View file @3ea4662
@@ -178,7 +178,7 @@ public class FlowObjectiveManager implements FlowObjectiveService {
     private boolean queueObjective(DeviceId deviceId, ForwardingObjective fwd) {
         if (fwd.nextId() != null &&
                 flowObjectiveStore.getNextGroup(fwd.nextId()) == null) {
-            log.warn("Queuing forwarding objective.");
+            log.trace("Queuing forwarding objective for nextId {}", fwd.nextId());
             if (pendingForwards.putIfAbsent(fwd.nextId(),
                                 Sets.newHashSet(new PendingNext(deviceId, fwd))) != null) {
                 Set<PendingNext> pending = pendingForwards.get(fwd.nextId());
@@ -226,10 +226,11 @@ public class FlowObjectiveManager implements FlowObjectiveService {
             }
             // Always (re)initialize the pipeline behaviour
+            log.info("Driver {} bound to device {} ... initializing driver",
+                     handler.driver().name(), deviceId);
             Pipeliner pipeliner = handler.behaviour(Pipeliner.class);
             pipeliner.init(deviceId, context);
             pipeliners.putIfAbsent(deviceId, pipeliner);
-            log.info("Driver {} bound to device {}", handler.driver().name(), deviceId);
         }
     }
@@ -241,6 +242,7 @@ public class FlowObjectiveManager implements FlowObjectiveService {
                 case MASTER_CHANGED:
                     if (event.roleInfo().master() != null) {
                         setupPipelineHandler(event.subject());
+                        log.info("mastership changed on device {}", event.subject());
                     }
                     break;
                 case BACKUPS_CHANGED:
@@ -258,7 +260,10 @@ public class FlowObjectiveManager implements FlowObjectiveService {
             switch (event.type()) {
                 case DEVICE_ADDED:
                 case DEVICE_AVAILABILITY_CHANGED:
+                    log.info("Device either added or availability changed {}",
+                             event.subject().id());
                     if (deviceService.isAvailable(event.subject().id())) {
+                        log.info("Device is now available {}", event.subject().id());
                         setupPipelineHandler(event.subject().id());
                         processPendingObjectives(event.subject().id());
                     }
@@ -281,6 +286,8 @@ public class FlowObjectiveManager implements FlowObjectiveService {
         }
         private void processPendingObjectives(DeviceId deviceId) {
+            log.debug("Processing pending objectives for device {}", deviceId);
+
             pendingObjectives.getOrDefault(deviceId,
                                            Collections.emptySet()).forEach(obj -> {
                 if (obj instanceof NextObjective) {
@@ -313,13 +320,15 @@ public class FlowObjectiveManager implements FlowObjectiveService {
     private class InternalStoreDelegate implements FlowObjectiveStoreDelegate {
         @Override
         public void notify(ObjectiveEvent event) {
+            log.debug("Received notification of obj event {}", event);
             Set<PendingNext> pending = pendingForwards.remove(event.subject());
             if (pending == null) {
+                log.debug("Nothing pending for this obj event");
                 return;
             }
-            log.info("Processing pending objectives {}", pending.size());
+            log.debug("Processing pending forwarding objectives {}", pending.size());
             pending.forEach(p -> getDevicePipeliner(p.deviceId())
                     .forward(p.forwardingObjective()));
--- a/drivers/src/main/java/org/onosproject/driver/pipeline/OVSCorsaPipeline.java
View file @3ea4662
+++ b/drivers/src/main/java/org/onosproject/driver/pipeline/OVSCorsaPipeline.java
View file @3ea4662
@@ -79,7 +79,7 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
-    protected static final int FIRST_TABLE = 0;
+    protected static final int MAC_TABLE = 0;
     protected static final int VLAN_MPLS_TABLE = 1;
     protected static final int VLAN_TABLE = 2;
     //protected static final int MPLS_TABLE = 3;
@@ -329,7 +329,7 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
                         .withPriority(CONTROLLER_PRIORITY)
                         .fromApp(applicationId)
                         .makePermanent()
-                        .forTable(FIRST_TABLE).build();
+                        .forTable(MAC_TABLE).build();
                 ops =  install ? ops.add(rule) : ops.remove(rule);
             } else if (c.type() == Criterion.Type.VLAN_VID) {
                 Criteria.VlanIdCriterion v = (Criteria.VlanIdCriterion) c;
@@ -378,13 +378,13 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
             @Override
             public void onSuccess(FlowRuleOperations ops) {
                 pass(filt);
-                log.info("Provisioned default table for bgp router");
+                log.info("Applied filtering rules");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
                 fail(filt, ObjectiveError.FLOWINSTALLATIONFAILED);
-                log.info("Failed to provision default table for bgp router");
+                log.info("Failed to apply filtering rules");
             }
         }));
     }
@@ -402,16 +402,16 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
     }
     private void pushDefaultRules() {
-        processTableZero(true);
+        processMacTable(true);
-        processTableOne(true);
+        processVlanMplsTable(true);
-        processTableTwo(true);
+        processVlanTable(true);
-        processTableFour(true);
+        processEtherTable(true);
-        processTableFive(true);
+        processCosTable(true);
-        processTableSix(true);
+        processFibTable(true);
-        processTableNine(true);
+        processLocalTable(true);
     }
-    private void processTableZero(boolean install) {
+    private void processMacTable(boolean install) {
         TrafficSelector.Builder selector;
         TrafficTreatment.Builder treatment;
@@ -429,7 +429,7 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
                 .withPriority(CONTROLLER_PRIORITY)
                 .fromApp(appId)
                 .makePermanent()
-                .forTable(FIRST_TABLE).build();
+                .forTable(MAC_TABLE).build();
         FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
@@ -450,7 +450,7 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
                 .withPriority(DROP_PRIORITY)
                 .fromApp(appId)
                 .makePermanent()
-                .forTable(FIRST_TABLE).build();
+                .forTable(MAC_TABLE).build();
         ops = install ? ops.add(rule) : ops.remove(rule);
@@ -458,18 +458,18 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned default table for bgp router");
+                log.info("Provisioned mac table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
-                log.info("Failed to provision default table for bgp router");
+                log.info("Failed to provision mac table");
             }
         }));
     }
-    private void processTableOne(boolean install) {
+    private void processVlanMplsTable(boolean install) {
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         TrafficTreatment.Builder treatment = DefaultTrafficTreatment
                 .builder();
@@ -494,19 +494,19 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned vlan/mpls table for bgp router");
+                log.info("Provisioned vlan/mpls table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
                 log.info(
-                        "Failed to provision vlan/mpls table for bgp router");
+                        "Failed to provision vlan/mpls table");
             }
         }));
     }
-    private void processTableTwo(boolean install) {
+    private void processVlanTable(boolean install) {
         TrafficSelector.Builder selector;
         TrafficTreatment.Builder treatment;
         FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
@@ -533,17 +533,17 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned vlan table for bgp router");
+                log.info("Provisioned vlan table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
-                log.info("Failed to provision vlan table for bgp router");
+                log.info("Failed to provision vlan table");
             }
         }));
     }
-    private void processTableFour(boolean install) {
+    private void processEtherTable(boolean install) {
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         TrafficTreatment.Builder treatment = DefaultTrafficTreatment
                 .builder();
@@ -602,18 +602,18 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned ether table for bgp router");
+                log.info("Provisioned ether table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
-                log.info("Failed to provision ether table for bgp router");
+                log.info("Failed to provision ether table");
             }
         }));
     }
-    private void processTableFive(boolean install) {
+    private void processCosTable(boolean install) {
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         TrafficTreatment.Builder treatment = DefaultTrafficTreatment
                 .builder();
@@ -636,18 +636,18 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned cos table for bgp router");
+                log.info("Provisioned cos table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
-                log.info("Failed to provision cos table for bgp router");
+                log.info("Failed to provision cos table");
             }
         }));
     }
-    private void processTableSix(boolean install) {
+    private void processFibTable(boolean install) {
         TrafficSelector.Builder selector;
         TrafficTreatment.Builder treatment;
         FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
@@ -673,17 +673,17 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned FIB table for bgp router");
+                log.info("Provisioned FIB table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
-                log.info("Failed to provision FIB table for bgp router");
+                log.info("Failed to provision FIB table");
             }
         }));
     }
-    private void processTableNine(boolean install) {
+    private void processLocalTable(boolean install) {
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         TrafficTreatment.Builder treatment = DefaultTrafficTreatment
                 .builder();
@@ -706,12 +706,12 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
         flowRuleService.apply(ops.build(new FlowRuleOperationsContext() {
             @Override
             public void onSuccess(FlowRuleOperations ops) {
-                log.info("Provisioned Local table for bgp router");
+                log.info("Provisioned Local table");
             }
             @Override
             public void onError(FlowRuleOperations ops) {
-                log.info("Failed to provision Local table for bgp router");
+                log.info("Failed to provision Local table");
             }
         }));
     }
@@ -748,6 +748,7 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
                 }
                 pass(obj);
                 pendingGroups.invalidate(key);
+                log.info("Heard back from group service for group {}", obj.id());
                 flowObjectiveStore.putNextGroup(obj.id(), new CorsaGroup(key));
             });
         }
--- a/openflow/drivers/src/main/java/org/onosproject/openflow/drivers/OFCorsaSwitchDriver.java
View file @3ea4662
+++ b/openflow/drivers/src/main/java/org/onosproject/openflow/drivers/OFCorsaSwitchDriver.java
View file @3ea4662
@@ -38,7 +38,7 @@ import org.projectfloodlight.openflow.types.OFGroup;
 import org.projectfloodlight.openflow.types.OFVlanVidMatch;
 import org.projectfloodlight.openflow.types.TableId;
-import java.util.ArrayList;
+//import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -79,7 +79,7 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch {
      */
     @Override
     public void write(OFMessage msg) {
-        if (msg.getType() == OFType.FLOW_MOD) {
+/*        if (msg.getType() == OFType.FLOW_MOD) {
             OFFlowMod flowMod = (OFFlowMod) msg;
             OFFlowMod.Builder builder = flowMod.createBuilder();
             builder.setTableId(TableId.of(LOCAL_TABLE));
@@ -87,11 +87,13 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch {
         } else {
             channel.write(Collections.singletonList(msg));
         }
+*/
+        channel.write(Collections.singletonList(msg));
     }
     @Override
     public void write(List<OFMessage> msgs) {
-        List<OFMessage> newMsgs = new ArrayList<OFMessage>();
+/*        List<OFMessage> newMsgs = new ArrayList<OFMessage>();
         for (OFMessage msg : msgs) {
             if (msg.getType() == OFType.FLOW_MOD) {
                 OFFlowMod flowMod = (OFFlowMod) msg;
@@ -103,6 +105,8 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch {
             }
         }
         channel.write(newMsgs);
+*/
+        channel.write(msgs);
     }
     @Override
--- a/openflow/drivers/src/main/java/org/onosproject/openflow/drivers/OFOVSSwitchCorsaTTP.java
View file @3ea4662
+++ b/openflow/drivers/src/main/java/org/onosproject/openflow/drivers/OFOVSSwitchCorsaTTP.java
View file @3ea4662
@@ -15,18 +15,18 @@
  */
 package org.onosproject.openflow.drivers;
-import com.google.common.collect.Lists;
 import org.onosproject.openflow.controller.Dpid;
 import org.projectfloodlight.openflow.protocol.OFDescStatsReply;
-import org.projectfloodlight.openflow.protocol.OFFlowMod;
 import org.projectfloodlight.openflow.protocol.OFMessage;
+/*import com.google.common.collect.Lists;
+import org.projectfloodlight.openflow.protocol.OFFlowMod;
 import org.projectfloodlight.openflow.protocol.OFType;
 import org.projectfloodlight.openflow.protocol.instruction.OFInstruction;
 import org.projectfloodlight.openflow.protocol.instruction.OFInstructionGotoTable;
 import org.projectfloodlight.openflow.types.TableId;
-
-import java.util.Collections;
 import java.util.List;
+*/
+import java.util.Collections;
 public class OFOVSSwitchCorsaTTP extends OFCorsaSwitchDriver {
@@ -36,7 +36,7 @@ public class OFOVSSwitchCorsaTTP extends OFCorsaSwitchDriver {
     @Override
     public void transformAndSendMsg(OFMessage msg, TableType type) {
-        log.trace("Trying to send {} of TableType {}", msg, type);
+        /*log.trace("Trying to send {} of TableType {}", msg, type);
         if (msg.getType() == OFType.FLOW_MOD) {
             OFFlowMod flowMod = (OFFlowMod) msg;
             OFFlowMod.Builder builder = flowMod.createBuilder();
@@ -84,10 +84,10 @@ public class OFOVSSwitchCorsaTTP extends OFCorsaSwitchDriver {
                             break;
                         case NONE:
                             log.error("Should never have to go to Table 0");
-                            /*newInstructions.add(
+                            newInstructions.add(
                                     gotoTable.createBuilder()
                                             .setTableId(TableId.of(0)).build());
-                            */
+
                             break;
                         default:
                             log.warn("Unknown table type: {}", tid);
@@ -134,6 +134,7 @@ public class OFOVSSwitchCorsaTTP extends OFCorsaSwitchDriver {
         } else {
             channel.write(Collections.singletonList(msg));
-        }
+        }*/
+        channel.write(Collections.singletonList(msg));
     }
 }
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java
View file @3ea4662
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/OpenFlowRuleProvider.java
View file @3ea4662
@@ -211,11 +211,12 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
                               fbe.operator(), fbe);
                     continue;
                 }
-            if (fbe.target().tableId() == 0) {
+            /*if (fbe.target().tableId() == 0) {
                 sw.sendMsg(mod);
             } else {
                 sw.transformAndSendMsg(mod, getTableType(fbe.target().tableId()));
-            }
+            }*/
+            sw.sendMsg(mod);
         }
         OFBarrierRequest.Builder builder = sw.factory()
                 .buildBarrierRequest()