CORD-73 Change the way we configure host learning in segment routing

hostLearning config
    - true: enable host learning
    - false or not provided: disable host learning
suppressHost config
    - all connect points listed here will be ignored
    - no effect if hostLearning is false
    - accept all hosts if this config is not provided

Change-Id: Id4a60bd47cac1f226ab8ba5391931ad2fb798529

@@ -5,6 +5,7 @@ COMPILE_DEPS = [
     '//cli:onos-cli',
     '//core/store/serializers:onos-core-serializers',
     '//incubator/api:onos-incubator-api',
+    '//providers/netcfghost:onos-providers-netcfghost',
     '//utils/rest:onlab-rest',
 ]
 

@@ -76,6 +76,11 @@
             <version>${project.version}</version>
         </dependency>
         <dependency>
+            <groupId>org.onosproject</groupId>
+            <artifactId>onos-netcfg-host-provider</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
             <groupId>javax.ws.rs</groupId>
             <artifactId>javax.ws.rs-api</artifactId>
             <version>2.0.1</version>

@@ -24,6 +24,7 @@ import org.onlab.packet.MacAddress;
 import org.onlab.packet.VlanId;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.Host;
 import org.onosproject.net.HostLocation;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.DefaultTrafficSelector;
@@ -37,6 +38,8 @@ import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.net.flowobjective.ObjectiveContext;
 import org.onosproject.net.host.HostEvent;
 import org.onosproject.net.host.HostService;
+import org.onosproject.provider.netcfghost.NetworkConfigHostProvider;
+import org.onosproject.segmentrouting.config.SegmentRoutingAppConfig;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -69,23 +72,24 @@ public class HostHandler {
             if (!deviceId.equals(devId)) {
                 return;
             }
-            processHostAddedEventInternal(host.mac(), host.vlan(),
-                    host.location(), host.ipAddresses());
+            processHostAddedEventInternal(host);
         });
     }
 
     protected void processHostAddedEvent(HostEvent event) {
-        processHostAddedEventInternal(event.subject().mac(), event.subject().vlan(),
-                event.subject().location(), event.subject().ipAddresses());
+        processHostAddedEventInternal(event.subject());
     }
 
-    private void processHostAddedEventInternal(MacAddress mac, VlanId vlanId,
-            HostLocation location, Set<IpAddress> ips) {
+    private void processHostAddedEventInternal(Host host) {
+        MacAddress mac = host.mac();
+        VlanId vlanId = host.vlan();
+        HostLocation location = host.location();
         DeviceId deviceId = location.deviceId();
         PortNumber port = location.port();
-        log.info("Host {}/{} is added at {}:{}", mac, vlanId, deviceId, port);
+        Set<IpAddress> ips = host.ipAddresses();
+        log.debug("Host {}/{} is added at {}:{}", mac, vlanId, deviceId, port);
 
-        if (!srManager.deviceConfiguration.suppressHost().contains(location)) {
+        if (accepted(host)) {
             // Populate bridging table entry
             log.debug("Populate L2 table entry for host {} at {}:{}",
                     mac, deviceId, port);
@@ -121,8 +125,7 @@ public class HostHandler {
         Set<IpAddress> ips = event.subject().ipAddresses();
         log.debug("Host {}/{} is removed from {}:{}", mac, vlanId, deviceId, port);
 
-        if (!srManager.deviceConfiguration.suppressHost()
-                .contains(new ConnectPoint(deviceId, port))) {
+        if (accepted(event.subject())) {
             // Revoke bridging table entry
             ForwardingObjective.Builder fob =
                     hostFwdObjBuilder(deviceId, mac, vlanId, port);
@@ -161,8 +164,7 @@ public class HostHandler {
         log.debug("Host {}/{} is moved from {}:{} to {}:{}",
                 mac, vlanId, prevDeviceId, prevPort, newDeviceId, newPort);
 
-        if (!srManager.deviceConfiguration.suppressHost()
-                .contains(new ConnectPoint(prevDeviceId, prevPort))) {
+        if (accepted(event.prevSubject())) {
             // Revoke previous bridging table entry
             ForwardingObjective.Builder prevFob =
                     hostFwdObjBuilder(prevDeviceId, mac, vlanId, prevPort);
@@ -186,8 +188,7 @@ public class HostHandler {
             });
         }
 
-        if (!srManager.deviceConfiguration.suppressHost()
-                .contains(new ConnectPoint(newDeviceId, newPort))) {
+        if (accepted(event.subject())) {
             // Populate new bridging table entry
             ForwardingObjective.Builder newFob =
                     hostFwdObjBuilder(newDeviceId, mac, vlanId, newPort);
@@ -225,8 +226,7 @@ public class HostHandler {
         Set<IpAddress> newIps = event.subject().ipAddresses();
         log.debug("Host {}/{} is updated", mac, vlanId);
 
-        if (!srManager.deviceConfiguration.suppressHost()
-                .contains(new ConnectPoint(prevDeviceId, prevPort))) {
+        if (accepted(event.prevSubject())) {
             // Revoke previous IP table entry
             prevIps.forEach(ip -> {
                 if (ip.isIp4()) {
@@ -237,8 +237,7 @@ public class HostHandler {
             });
         }
 
-        if (!srManager.deviceConfiguration.suppressHost()
-                .contains(new ConnectPoint(newDeviceId, newPort))) {
+        if (accepted(event.subject())) {
             // Populate new IP table entry
             newIps.forEach(ip -> {
                 if (ip.isIp4()) {
@@ -344,4 +343,27 @@ public class HostHandler {
             srManager.defaultRoutingHandler.revokeSubnet(ImmutableSet.of(ip4Prefix));
         }
     }
+
+    /**
+     * Check if a host is accepted or not.
+     *
+     * @param host host to be checked
+     * @return true if segment routing accepts the host
+     */
+    private boolean accepted(Host host) {
+        // Always accept configured hosts
+        if (host.providerId().equals(NetworkConfigHostProvider.PROVIDER_ID)) {
+            return true;
+        }
+
+        SegmentRoutingAppConfig appConfig = srManager.cfgService
+                .getConfig(srManager.appId, SegmentRoutingAppConfig.class);
+        boolean accepted = appConfig != null &&
+                appConfig.hostLearning() &&
+                !appConfig.suppressHost().contains(host.location());
+        if (!accepted) {
+            log.info("Ignore suppressed host {}", host.id());
+        }
+        return accepted;
+    }
 }

@@ -51,6 +51,7 @@ import org.onosproject.net.flowobjective.ObjectiveContext;
 import org.onosproject.net.mcast.McastEvent;
 import org.onosproject.net.mcast.McastRouteInfo;
 import org.onosproject.net.topology.TopologyService;
+import org.onosproject.segmentrouting.config.SegmentRoutingAppConfig;
 import org.onosproject.segmentrouting.storekey.McastStoreKey;
 import org.onosproject.store.serializers.KryoNamespaces;
 import org.onosproject.store.service.ConsistentMap;
@@ -345,11 +346,11 @@ public class McastHandler {
      */
     private void addFilterToDevice(DeviceId deviceId, PortNumber port, VlanId assignedVlan) {
         // Do nothing if the port is configured as suppressed
-        ConnectPoint connectPt = new ConnectPoint(deviceId, port);
-        if (srManager.deviceConfiguration == null ||
-                srManager.deviceConfiguration.suppressSubnet().contains(connectPt) ||
-                srManager.deviceConfiguration.suppressHost().contains(connectPt)) {
-            log.info("Ignore suppressed port {}", connectPt);
+        ConnectPoint connectPoint = new ConnectPoint(deviceId, port);
+        SegmentRoutingAppConfig appConfig = srManager.cfgService
+                .getConfig(srManager.appId, SegmentRoutingAppConfig.class);
+        if (appConfig != null && appConfig.suppressSubnet().contains(connectPoint)) {
+            log.info("Ignore suppressed port {}", connectPoint);
             return;
         }
 

@@ -28,6 +28,7 @@ import org.onosproject.net.flowobjective.DefaultObjectiveContext;
 import org.onosproject.net.flowobjective.ObjectiveContext;
 import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
 import org.onosproject.segmentrouting.config.DeviceConfiguration;
+import org.onosproject.segmentrouting.config.SegmentRoutingAppConfig;
 import org.onosproject.segmentrouting.grouphandler.NeighborSet;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Port;
@@ -540,7 +541,9 @@ public class RoutingRulePopulator {
         for (Port port : devPorts) {
             ConnectPoint connectPoint = new ConnectPoint(deviceId, port.number());
             // TODO: Handles dynamic port events when we are ready for dynamic config
-            if (!srManager.deviceConfiguration.suppressSubnet().contains(connectPoint) &&
+            SegmentRoutingAppConfig appConfig = srManager.cfgService
+                    .getConfig(srManager.appId, SegmentRoutingAppConfig.class);
+            if ((appConfig == null || !appConfig.suppressSubnet().contains(connectPoint)) &&
                     port.isEnabled()) {
                 Ip4Prefix portSubnet = config.getPortSubnet(deviceId, port.number());
                 VlanId assignedVlan = (portSubnet == null)

@@ -113,7 +113,10 @@ public class DeviceConfiguration implements DeviceProperties {
             cfgService.getSubjects(ConnectPoint.class, InterfaceConfig.class);
         portSubjects.forEach(subject -> {
             // Do not process excluded ports
-            if (suppressSubnet().contains(subject)) {
+            SegmentRoutingAppConfig appConfig =
+                    cfgService.getConfig(appId, SegmentRoutingAppConfig.class);
+            if (appConfig != null && appConfig.suppressSubnet().contains(subject)) {
+                log.info("Ignore suppressed port {}", subject);
                 return;
             }
 
@@ -498,28 +501,6 @@ public class DeviceConfiguration implements DeviceProperties {
     }
 
     /**
-     * Gets connect points for which segment routing does not install subnet rules.
-     *
-     * @return set of connect points
-     */
-    public Set<ConnectPoint> suppressSubnet() {
-        SegmentRoutingAppConfig appConfig =
-                cfgService.getConfig(appId, SegmentRoutingAppConfig.class);
-        return (appConfig != null) ? appConfig.suppressSubnet() : ImmutableSet.of();
-    }
-
-    /**
-     * Gets connect points for which segment routing does not install host rules.
-     *
-     * @return set of connect points
-     */
-    public Set<ConnectPoint> suppressHost() {
-        SegmentRoutingAppConfig appConfig =
-                cfgService.getConfig(appId, SegmentRoutingAppConfig.class);
-        return (appConfig != null) ? appConfig.suppressHost() : ImmutableSet.of();
-    }
-
-    /**
      * Add subnet to specific connect point.
      *
      * @param cp connect point

@@ -38,10 +38,12 @@ public class SegmentRoutingAppConfig extends Config<ApplicationId> {
     private static final String VROUTER_ID = "vRouterId";
     private static final String SUPPRESS_SUBNET = "suppressSubnet";
     private static final String SUPPRESS_HOST = "suppressHost";
+    private static final String HOST_LEARNING = "hostLearning";
 
     @Override
     public boolean isValid() {
-        return hasOnlyFields(VROUTER_MACS, VROUTER_ID, SUPPRESS_SUBNET, SUPPRESS_HOST) &&
+        return hasOnlyFields(VROUTER_MACS, VROUTER_ID, SUPPRESS_SUBNET,
+                SUPPRESS_HOST, HOST_LEARNING) &&
                 vRouterMacs() != null && vRouterId() != null &&
                 suppressSubnet() != null && suppressHost() != null;
     }
@@ -225,6 +227,26 @@ public class SegmentRoutingAppConfig extends Config<ApplicationId> {
         return this;
     }
 
+    /**
+     * Gets whether host learning is enabled or not.
+     *
+     * @return true if enabled. false if disabled or not configured
+     */
+    public boolean hostLearning() {
+        return object.has(HOST_LEARNING) && object.path(HOST_LEARNING).asBoolean();
+    }
+
+    /**
+     * Sets whether host learning is enabled or not.
+     *
+     * @param enabled true if enabled
+     * @return this {@link SegmentRoutingAppConfig}
+     */
+    public SegmentRoutingAppConfig setHostLearning(boolean enabled) {
+        object.put(HOST_LEARNING, enabled);
+        return this;
+    }
+
     @Override
     public String toString() {
         return toStringHelper(this)
@@ -232,6 +254,7 @@ public class SegmentRoutingAppConfig extends Config<ApplicationId> {
                 .add("vRouterId", vRouterId())
                 .add("suppressSubnet", suppressSubnet())
                 .add("suppressHost", suppressHost())
+                .add("hostLearning", hostLearning())
                 .toString();
     }
 }

@@ -210,6 +210,27 @@ public class SegmentRoutingAppConfigTest {
         assertTrue(suppressHost.contains(PORT_3));
     }
 
+    /**
+     * Tests hostLearning getter.
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testHostLearning() throws Exception {
+        assertFalse(config.hostLearning());
+    }
+
+    /**
+     * Tests hostLearning setter.
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testSetHostLearning() throws Exception {
+        config.setHostLearning(true);
+        assertTrue(config.hostLearning());
+    }
+
     private class MockDelegate implements ConfigApplyDelegate {
         @Override
         public void onApply(Config config) {

@@ -11,5 +11,6 @@
   "suppressHost" : [
       "of:1/1",
       "wrongPort"
-  ]
+  ],
+  "hostLearning" : false
 }

@@ -11,5 +11,6 @@
   "suppressHost" : [
       "of:1/1",
       "of:1/2"
-  ]
+  ],
+  "hostLearning" : false
 }

@@ -59,8 +59,9 @@ public class NetworkConfigHostProvider extends AbstractProvider implements HostP
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected NetworkConfigRegistry networkConfigRegistry;
 
-    private static final String APP_NAME = "org.onosproject.provider.netcfghost";
     private ApplicationId appId;
+    private static final String APP_NAME = "org.onosproject.netcfghost";
+    public static final ProviderId PROVIDER_ID = new ProviderId("host", APP_NAME);
     protected HostProviderService providerService;
 
     private final Logger log = LoggerFactory.getLogger(getClass());
@@ -71,7 +72,7 @@ public class NetworkConfigHostProvider extends AbstractProvider implements HostP
      * Creates an network config host location provider.
      */
     public NetworkConfigHostProvider() {
-        super(new ProviderId("host", APP_NAME));
+        super(PROVIDER_ID);
     }
 
     @Activate

@@ -15,6 +15,7 @@
     p.p6 {margin: 0.0px 0.0px 0.0px 0.0px; line-height: 14.0px; font: 12.0px Menlo; color: #ff2600; -webkit-text-stroke: #ff2600}
     p.p7 {margin: 0.0px 0.0px 0.0px 0.0px; line-height: 14.0px; font: 12.0px Menlo; color: #000000; -webkit-text-stroke: #000000}
     p.p8 {margin: 0.0px 0.0px 0.0px 0.0px; line-height: 14.0px; font: 12.0px Menlo; color: #00c7fc; -webkit-text-stroke: #000000}
+    p.p9 {margin: 0.0px 0.0px 0.0px 0.0px; line-height: 14.0px; font: 12.0px Menlo; color: #78ba5b; -webkit-text-stroke: #353535}
     span.s1 {font-kerning: none}
     span.s2 {font-kerning: none; color: #0433ff; -webkit-text-stroke: 0px #0433ff}
     span.s3 {font-kerning: none; color: #000000; -webkit-text-stroke: 0px #000000}
@@ -24,7 +25,8 @@
     span.s7 {font-kerning: none; color: #ff40ff; -webkit-text-stroke: 0px #ff40ff}
     span.s8 {font-kerning: none; color: #ff2600; -webkit-text-stroke: 0px #ff2600}
     span.s9 {font-kerning: none; color: #000000}
-    span.s10 {font-kerning: none; color: #669c35; -webkit-text-stroke: 0px #669c35}
+    span.s10 {font-kerning: none; -webkit-text-stroke: 0px #000000}
+    span.s11 {font-kerning: none; color: #669c35; -webkit-text-stroke: 0px #669c35}
     span.Apple-tab-span {white-space:pre}
   </style>
 </head>
@@ -225,7 +227,8 @@
 <p class="p4"><span class="s3"><span class="Apple-converted-space">                 </span>"suppressSubnet" : [ </span><span class="s1">// Do not push subnet rules for these ports</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                    </span>"of:0000000000000002/31", "of:0000000000000002/32"</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>],</span></p>
-<p class="p4"><span class="s3"><span class="Apple-converted-space">                </span>"suppressHost" : [ </span><span class="s1">// Do not push host rules for these ports</span></p>
+<p class="p9"><span class="s3"><span class="Apple-converted-space">                </span>"hostLearning" : true, </span><span class="s10">// </span><span class="s1">Host learning is enabled if true. Host learning is disabled if false or the config is not provided</span></p>
+<p class="p4"><span class="s3"><span class="Apple-converted-space">                </span>"suppressHost" : [ </span><span class="s1">// Hosts on these ports will be ignored. Only takes effect when hostLearning is enabled</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                    </span>"of:0000000000000001/65", "of:0000000000000001/73",</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                    </span>"of:0000000000000002/31", "of:0000000000000002/32"</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>]</span></p>
@@ -236,7 +239,7 @@
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>"controlPlaneConnectPoint" : "of:0000000000000002/31", </span><span class="s5">// location of Quagga</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>"ospfEnabled" : "true", </span><span class="s5">// enable OSPF</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>"pimEnabled" : "true", </span><span class="s6">// enable PIM</span></p>
-<p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>"interfaces" : [ "external-quagga" ] </span><span class="s10">// </span><span class="s5">VR only handles peers on these ports</span></p>
+<p class="p7"><span class="s1"><span class="Apple-converted-space">                </span>"interfaces" : [ "external-quagga" ] </span><span class="s11">// </span><span class="s5">VR only handles peers on these ports</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">            </span>}</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">        </span>}</span></p>
 <p class="p7"><span class="s1"><span class="Apple-converted-space">    </span>}</span></p>

@@ -234,6 +234,7 @@
                 "suppressSubnet" : [
                     "of:0000000000000002/31", "of:0000000000000002/32"
                 ],
+                "hostLearning" : true,
                 "suppressHost" : [
                     "of:0000000000000001/65", "of:0000000000000001/73",
                     "of:0000000000000002/31", "of:0000000000000002/32"