Fixes bug where driver gets initialized only when device is available.

More explict handling of versatile forwarding flows in corsa driver. Moving TunnelConnectivityManager to use flowObjectives instead of flowRules. Change-Id: If43023f30a6e7a028dfdefbe1ffbcc710a1c7be3

Fixes bug where driver gets initialized only when device is available.
More explict handling of versatile forwarding flows in corsa driver. Moving TunnelConnectivityManager to use flowObjectives instead of flowRules. Change-Id: If43023f30a6e7a028dfdefbe1ffbcc710a1c7be3
Saurav Das · Gerrit Code Review
Commit 3d0382657855da48341ba9d0e5e827e7d57ad2d6 3d038265 1 parent e9c8bbcf
Showing 5 changed files with 90 additions and 22 deletions
apps/bgprouter/src/main/java/org/onosproject/bgprouter/BgpRouter.java
apps/bgprouter/src/main/java/org/onosproject/bgprouter/TunnellingConnectivityManager.java
core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
drivers/src/main/java/org/onosproject/driver/pipeline/OVSCorsaPipeline.java
--- a/apps/bgprouter/src/main/java/org/onosproject/bgprouter/BgpRouter.java
View file @3d03826
+++ b/apps/bgprouter/src/main/java/org/onosproject/bgprouter/BgpRouter.java
View file @3d03826
@@ -161,7 +161,7 @@ public class BgpRouter {
         connectivityManager = new TunnellingConnectivityManager(appId,
                                                                 configService,
                                                                 packetService,
-                                                                flowService);
+                                                                flowObjectiveService);
         icmpHandler = new IcmpHandler(configService, packetService);
@@ -252,7 +252,7 @@ public class BgpRouter {
             flowObjectiveService.forward(deviceId,
                                          generateRibFlowRule(fibEntry.prefix(), nextId).add());
-            log.trace("Sending flow forwarding objective {}->{}", fibEntry, nextId);
+            log.trace("Sending forwarding objective {} -> nextId:{}", fibEntry, nextId);
         }
     }
@@ -282,8 +282,6 @@ public class BgpRouter {
                 .matchIPDst(prefix)
                 .build();
-
-
         int priority = prefix.prefixLength() * PRIORITY_MULTIPLIER + PRIORITY_OFFSET;
         ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective.builder()
--- a/apps/bgprouter/src/main/java/org/onosproject/bgprouter/TunnellingConnectivityManager.java
View file @3d03826
+++ b/apps/bgprouter/src/main/java/org/onosproject/bgprouter/TunnellingConnectivityManager.java
View file @3d03826
@@ -15,19 +15,21 @@
  */
 package org.onosproject.bgprouter;
+import static org.slf4j.LoggerFactory.getLogger;
+
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IPv4;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.TCP;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.flow.DefaultFlowRule;
 import org.onosproject.net.flow.DefaultTrafficSelector;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
-import org.onosproject.net.flow.FlowRuleOperations;
-import org.onosproject.net.flow.FlowRuleService;
 import org.onosproject.net.flow.TrafficSelector;
 import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.flowobjective.DefaultForwardingObjective;
+import org.onosproject.net.flowobjective.FlowObjectiveService;
+import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.net.packet.DefaultOutboundPacket;
 import org.onosproject.net.packet.OutboundPacket;
 import org.onosproject.net.packet.PacketContext;
@@ -37,6 +39,7 @@ import org.onosproject.routing.config.BgpPeer;
 import org.onosproject.routing.config.BgpSpeaker;
 import org.onosproject.routing.config.InterfaceAddress;
 import org.onosproject.routing.config.RoutingConfigurationService;
+import org.slf4j.Logger;
 /**
@@ -46,23 +49,25 @@ import org.onosproject.routing.config.RoutingConfigurationService;
 public class TunnellingConnectivityManager {
     private static final short BGP_PORT = 179;
-
+    private final Logger log = getLogger(getClass());
     private final ApplicationId appId;
     private final BgpSpeaker bgpSpeaker;
     private final PacketService packetService;
     private final RoutingConfigurationService configService;
+    private final FlowObjectiveService flowObjectiveService;
     private final BgpProcessor processor = new BgpProcessor();
     public TunnellingConnectivityManager(ApplicationId appId,
                                          RoutingConfigurationService configService,
                                          PacketService packetService,
-                                         FlowRuleService flowService) {
+                                         FlowObjectiveService flowObjectiveService) {
         this.appId = appId;
         this.configService = configService;
         this.packetService = packetService;
+        this.flowObjectiveService = flowObjectiveService;
         BgpSpeaker bgpSpeaker = null;
         for (BgpSpeaker speaker : configService.getBgpSpeakers().values()) {
@@ -92,12 +97,27 @@ public class TunnellingConnectivityManager {
                 .punt()
                 .build();
-        FlowRuleOperations.Builder builder = FlowRuleOperations.builder();
+        ForwardingObjective puntSrc = DefaultForwardingObjective.builder()
-        builder.add(new DefaultFlowRule(bgpSpeaker.connectPoint().deviceId(),
+                .fromApp(appId)
-                                        selectorSrc, treatment, 0, appId, 0, true));
+                .makePermanent()
-        builder.add(new DefaultFlowRule(bgpSpeaker.connectPoint().deviceId(),
+                .withSelector(selectorSrc)
-                                        selectorDst, treatment, 0, appId, 0, true));
+                .withTreatment(treatment)
-        flowService.apply(builder.build());
+                .withFlag(ForwardingObjective.Flag.VERSATILE)
+                .add();
+        flowObjectiveService.forward(bgpSpeaker.connectPoint().deviceId(),
+                                     puntSrc);
+
+        ForwardingObjective puntDst = DefaultForwardingObjective.builder()
+                .fromApp(appId)
+                .makePermanent()
+                .withSelector(selectorDst)
+                .withTreatment(treatment)
+                .withFlag(ForwardingObjective.Flag.VERSATILE)
+                .add();
+        flowObjectiveService.forward(bgpSpeaker.connectPoint().deviceId(),
+                                     puntDst);
+        log.info("Sent punt forwarding objective to {}", bgpSpeaker.connectPoint().deviceId());
+
     }
     public void start() {
--- a/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
View file @3d03826
+++ b/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
View file @3d03826
@@ -67,7 +67,7 @@ import static org.onlab.util.Tools.groupedThreads;
 @Service
 public class FlowObjectiveManager implements FlowObjectiveService {
-    public static final int INSTALL_RETRY_ATTEMPTS = 5;
+    public static final int INSTALL_RETRY_ATTEMPTS = 10;
     public static final long INSTALL_RETRY_INTERVAL = 1000; // ms
     private final Logger log = LoggerFactory.getLogger(getClass());
@@ -167,7 +167,7 @@ public class FlowObjectiveManager implements FlowObjectiveService {
                         pipeliner.filter((FilteringObjective) objective);
                     }
                 } else if (numAttempts < INSTALL_RETRY_ATTEMPTS) {
-                    Thread.currentThread().sleep(INSTALL_RETRY_INTERVAL);
+                    Thread.sleep(INSTALL_RETRY_INTERVAL);
                     executorService.submit(this);
                 } else {
                     // Otherwise we've tried a few times and failed, report an
@@ -262,7 +262,9 @@ public class FlowObjectiveManager implements FlowObjectiveService {
             switch (event.type()) {
                 case MASTER_CHANGED:
                     log.info("mastership changed on device {}", event.subject());
-                    setupPipelineHandler(event.subject());
+                    if (deviceService.isAvailable(event.subject())) {
+                        setupPipelineHandler(event.subject());
+                    }
                     break;
                 case BACKUPS_CHANGED:
                     break;
@@ -278,8 +280,6 @@ public class FlowObjectiveManager implements FlowObjectiveService {
         public void event(DeviceEvent event) {
             switch (event.type()) {
                 case DEVICE_ADDED:
-                    setupPipelineHandler(event.subject().id());
-                    break;
                 case DEVICE_AVAILABILITY_CHANGED:
                     log.info("Device either added or availability changed {}",
                              event.subject().id());
--- a/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
View file @3d03826
+++ b/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
View file @3d03826
@@ -194,7 +194,8 @@ public class PacketManager
                     @Override
                     public void onError(Objective objective, ObjectiveError error) {
-                        log.warn("Failed to install packet request flow: {}", error);
+                        log.warn("Failed to install packet request {}: {}",
+                                 request, error);
                     }
                 });
--- a/drivers/src/main/java/org/onosproject/driver/pipeline/OVSCorsaPipeline.java
View file @3d03826
+++ b/drivers/src/main/java/org/onosproject/driver/pipeline/OVSCorsaPipeline.java
View file @3d03826
@@ -19,8 +19,10 @@ import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.RemovalCause;
 import com.google.common.cache.RemovalNotification;
+
 import org.onlab.osgi.ServiceDirectory;
 import org.onlab.packet.Ethernet;
+import org.onlab.packet.IPv4;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.VlanId;
 import org.onlab.util.KryoNamespace;
@@ -240,12 +242,59 @@ public class OVSCorsaPipeline extends AbstractHandlerBehaviour implements Pipeli
     }
     private Collection<FlowRule> processVersatile(ForwardingObjective fwd) {
+        log.debug("Processing versatile forwarding objective");
+        TrafficSelector selector = fwd.selector();
+
+        Criteria.EthTypeCriterion ethType =
+                (Criteria.EthTypeCriterion) selector.getCriterion(Criterion.Type.ETH_TYPE);
+        if (ethType == null) {
+            log.error("Versatile forwarding objective must include ethType");
+            fail(fwd, ObjectiveError.UNKNOWN);
+            return Collections.emptySet();
+        }
+        if (ethType.ethType() == Ethernet.TYPE_ARP) {
+            log.warn("Driver automatically handles ARP packets by punting to controller "
+                    + " from ETHER table");
+            pass(fwd);
+            return Collections.emptySet();
+        } else if (ethType.ethType() == Ethernet.TYPE_LLDP ||
+                ethType.ethType() == Ethernet.TYPE_BSN) {
+            log.warn("Driver currently does not currently handle LLDP packets");
+            fail(fwd, ObjectiveError.UNSUPPORTED);
+            return Collections.emptySet();
+        } else if (ethType.ethType() == Ethernet.TYPE_IPV4) {
+            Criteria.IPCriterion ipSrc = (Criteria.IPCriterion) selector
+                    .getCriterion(Criterion.Type.IPV4_SRC);
+            Criteria.IPCriterion ipDst = (Criteria.IPCriterion) selector
+                    .getCriterion(Criterion.Type.IPV4_DST);
+            Criteria.IPProtocolCriterion ipProto = (Criteria.IPProtocolCriterion) selector
+                    .getCriterion(Criterion.Type.IP_PROTO);
+            if (ipSrc != null) {
+                log.warn("Driver currently does not currently handle matching Src IP");
+                fail(fwd, ObjectiveError.UNSUPPORTED);
+                return Collections.emptySet();
+            }
+            if (ipDst != null) {
+                log.error("Driver handles Dst IP matching as specific forwarding "
+                        + "objective, not versatile");
+                fail(fwd, ObjectiveError.UNSUPPORTED);
+                return Collections.emptySet();
+            }
+            if (ipProto != null && ipProto.protocol() == IPv4.PROTOCOL_TCP) {
+                log.warn("Driver automatically punts all packets reaching the "
+                        + "LOCAL table to the controller");
+                pass(fwd);
+                return Collections.emptySet();
+            }
+        }
+
+        log.warn("Driver does not support given versatile forwarding objective");
         fail(fwd, ObjectiveError.UNSUPPORTED);
         return Collections.emptySet();
     }
     private Collection<FlowRule> processSpecific(ForwardingObjective fwd) {
-        log.warn("Processing specific");
+        log.debug("Processing specific forwarding objective");
         TrafficSelector selector = fwd.selector();
         Criteria.EthTypeCriterion ethType =
                 (Criteria.EthTypeCriterion) selector.getCriterion(Criterion.Type.ETH_TYPE);