From e7c7d0586fc0eee51a5781ea8500e27704953da2 Mon Sep 17 00:00:00 2001 From: Charles Chan Date: Mon, 9 Apr 2018 11:52:08 -0400 Subject: [PATCH] Ignore more than two next hops Change-Id: Ie42365a3a8b9e7f763c21e3f9be9e9abfc35dbf7 (cherry picked from commit e800860179913ce9c37bb378ff27ecb1c68d81dd) --- .../segmentrouting/RouteHandler.java | 19 +++ .../segmentrouting/RouteHandlerTest.java | 113 +++++++++++++++--- 2 files changed, 118 insertions(+), 14 deletions(-) diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java index 4b95bf31c7..6d77da414b 100644 --- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java +++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java @@ -111,6 +111,11 @@ public class RouteHandler { log.info("processRouteAddedInternal. routes={}", routes); + if (routes.size() > 2) { + log.info("Route {} has more than two next hops. Do not process route change", routes); + return; + } + Set allLocations = Sets.newHashSet(); Set allPrefixes = Sets.newHashSet(); routes.forEach(route -> { @@ -152,12 +157,26 @@ public class RouteHandler { log.info("processRouteUpdatedInternal. routes={}, oldRoutes={}", routes, oldRoutes); + if (routes.size() > 2) { + log.info("Route {} has more than two next hops. Do not process route change", routes); + return; + } + Set allLocations = Sets.newHashSet(); Set allPrefixes = Sets.newHashSet(); routes.forEach(route -> { allLocations.addAll(srManager.nextHopLocations(route)); allPrefixes.add(route.prefix()); }); + + // Just come back from an invalid next hop count + // Revoke subnet from all locations and reset oldRoutes such that system will be reprogrammed from scratch + if (oldRoutes.size() > 2) { + log.info("Revoke subnet {} and reset oldRoutes"); + srManager.defaultRoutingHandler.revokeSubnet(allPrefixes); + oldRoutes = Sets.newHashSet(); + } + log.debug("RouteUpdated. populateSubnet {}, {}", allLocations, allPrefixes); srManager.defaultRoutingHandler.populateSubnet(allLocations, allPrefixes); diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java index a7388d15b3..ef4c2aa2f3 100644 --- a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java +++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java @@ -101,6 +101,14 @@ public class RouteHandlerTest { private static final Route R3 = new Route(Route.Source.STATIC, P1, N3); private static final ResolvedRoute RR3 = new ResolvedRoute(R3, M3, V3); + // Single homed router 3 + private static final IpAddress N4 = IpAddress.valueOf("10.0.4.254"); + private static final MacAddress M4 = MacAddress.valueOf("00:00:00:00:00:04"); + private static final VlanId V4 = VlanId.vlanId((short) 4); + private static final ConnectPoint CP4 = ConnectPoint.deviceConnectPoint("of:0000000000000004/4"); + private static final Route R4 = new Route(Route.Source.STATIC, P1, N4); + private static final ResolvedRoute RR4 = new ResolvedRoute(R4, M4, V4); + // Hosts private static final Host H1 = new DefaultHost(ProviderId.NONE, HostId.hostId(M1, V1), M1, V1, Sets.newHashSet(new HostLocation(CP1, 0)), Sets.newHashSet(N1), false); @@ -110,12 +118,14 @@ public class RouteHandlerTest { Sets.newHashSet(new HostLocation(CP1, 0), new HostLocation(CP2, 0)), Sets.newHashSet(N3), false); private static final Host H3S = new DefaultHost(ProviderId.NONE, HostId.hostId(M3, V3), M3, V3, Sets.newHashSet(new HostLocation(CP1, 0)), Sets.newHashSet(N3), false); + private static final Host H4 = new DefaultHost(ProviderId.NONE, HostId.hostId(M4, V4), M4, V4, + Sets.newHashSet(new HostLocation(CP4, 0)), Sets.newHashSet(N4), false); // Pair Local Port private static final PortNumber P9 = PortNumber.portNumber(9); // A set of hosts - private static final Set HOSTS = Sets.newHashSet(H1, H2, H3D); + private static final Set HOSTS = Sets.newHashSet(H1, H2, H3D, H4); private static final Set HOSTS_ONE_FAIL = Sets.newHashSet(H1, H2, H3S); private static final Set HOSTS_BOTH_FAIL = Sets.newHashSet(H1, H2); // A set of devices of which we have mastership @@ -124,7 +134,7 @@ public class RouteHandlerTest { private static final Set INTERFACES = Sets.newHashSet(); @Before - public void setUp() throws Exception { + public void setUp() { ObjectMapper mapper = new ObjectMapper(); ConfigApplyDelegate delegate = config -> { }; @@ -173,7 +183,7 @@ public class RouteHandlerTest { } @Test - public void init() throws Exception { + public void init() { MockRoutingTableKey rtk = new MockRoutingTableKey(CP1.deviceId(), P1); MockRoutingTableValue rtv = new MockRoutingTableValue(CP1.port(), M1, V1); ROUTING_TABLE.put(rtk, rtv); @@ -191,7 +201,7 @@ public class RouteHandlerTest { } @Test - public void processRouteAdded() throws Exception { + public void processRouteAdded() { reset(srManager.deviceConfiguration); srManager.deviceConfiguration.addSubnet(CP1, P1); expectLastCall().once(); @@ -213,7 +223,7 @@ public class RouteHandlerTest { } @Test - public void processRouteUpdated() throws Exception { + public void processRouteUpdated() { processRouteAdded(); reset(srManager.deviceConfiguration); @@ -242,7 +252,7 @@ public class RouteHandlerTest { } @Test - public void processRouteRemoved() throws Exception { + public void processRouteRemoved() { processRouteAdded(); reset(srManager.deviceConfiguration); @@ -260,7 +270,7 @@ public class RouteHandlerTest { } @Test - public void testTwoSingleHomedAdded() throws Exception { + public void testTwoSingleHomedAdded() { reset(srManager.deviceConfiguration); srManager.deviceConfiguration.addSubnet(CP1, P1); expectLastCall().once(); @@ -289,7 +299,7 @@ public class RouteHandlerTest { } @Test - public void testOneDualHomedAdded() throws Exception { + public void testOneDualHomedAdded() { reset(srManager.deviceConfiguration); srManager.deviceConfiguration.addSubnet(CP1, P1); expectLastCall().once(); @@ -318,7 +328,7 @@ public class RouteHandlerTest { } @Test - public void testOneSingleHomedToTwoSingleHomed() throws Exception { + public void testOneSingleHomedToTwoSingleHomed() { processRouteAdded(); reset(srManager.deviceConfiguration); @@ -348,7 +358,7 @@ public class RouteHandlerTest { } @Test - public void testTwoSingleHomedToOneSingleHomed() throws Exception { + public void testTwoSingleHomedToOneSingleHomed() { testTwoSingleHomedAdded(); reset(srManager.deviceConfiguration); @@ -377,7 +387,7 @@ public class RouteHandlerTest { // TODO Add test cases for two single homed next hop at same location @Test - public void testDualHomedSingleLocationFail() throws Exception { + public void testDualHomedSingleLocationFail() { testOneDualHomedAdded(); HostEvent he = new HostEvent(HostEvent.Type.HOST_MOVED, H3S, H3D); @@ -400,7 +410,7 @@ public class RouteHandlerTest { } @Test - public void testDualHomedBothLocationFail() throws Exception { + public void testDualHomedBothLocationFail() { testDualHomedSingleLocationFail(); hostService = new MockHostService(HOSTS_ONE_FAIL); @@ -422,7 +432,7 @@ public class RouteHandlerTest { } @Test - public void testTwoSingleHomedRemoved() throws Exception { + public void testTwoSingleHomedRemoved() { testTwoSingleHomedAdded(); hostService = new MockHostService(HOSTS_BOTH_FAIL); @@ -444,7 +454,7 @@ public class RouteHandlerTest { } @Test - public void testOneDualHomeRemoved() throws Exception { + public void testOneDualHomeRemoved() { testOneDualHomedAdded(); reset(srManager.deviceConfiguration); @@ -462,4 +472,79 @@ public class RouteHandlerTest { verify(srManager.deviceConfiguration); } + + @Test + public void testMoreThanTwoNextHop() { + // next hop = CP1, CP2 + reset(srManager.deviceConfiguration); + srManager.deviceConfiguration.addSubnet(CP1, P1); + srManager.deviceConfiguration.addSubnet(CP2, P1); + expectLastCall().once(); + replay(srManager.deviceConfiguration); + + RouteEvent re = new RouteEvent(RouteEvent.Type.ROUTE_ADDED, RR1, Sets.newHashSet(RR1, RR2)); + routeHandler.processRouteAdded(re); + + assertEquals(2, ROUTING_TABLE.size()); + MockRoutingTableValue rtv1 = ROUTING_TABLE.get(new MockRoutingTableKey(CP1.deviceId(), P1)); + assertEquals(M1, rtv1.macAddress); + assertEquals(V1, rtv1.vlanId); + assertEquals(CP1.port(), rtv1.portNumber); + MockRoutingTableValue rtv2 = ROUTING_TABLE.get(new MockRoutingTableKey(CP2.deviceId(), P1)); + assertEquals(M2, rtv2.macAddress); + assertEquals(V2, rtv2.vlanId); + assertEquals(CP2.port(), rtv2.portNumber); + + assertEquals(2, SUBNET_TABLE.size()); + assertTrue(SUBNET_TABLE.get(CP1).contains(P1)); + assertTrue(SUBNET_TABLE.get(CP2).contains(P1)); + + verify(srManager.deviceConfiguration); + + // next hop = CP1, CP2, CP4 (invalid) + re = new RouteEvent(RouteEvent.Type.ALTERNATIVE_ROUTES_CHANGED, RR1, null, + Sets.newHashSet(RR1, RR2, RR4), Sets.newHashSet(RR1, RR2)); + routeHandler.processAlternativeRoutesChanged(re); + + assertEquals(2, ROUTING_TABLE.size()); + rtv1 = ROUTING_TABLE.get(new MockRoutingTableKey(CP1.deviceId(), P1)); + assertEquals(M1, rtv1.macAddress); + assertEquals(V1, rtv1.vlanId); + assertEquals(CP1.port(), rtv1.portNumber); + rtv2 = ROUTING_TABLE.get(new MockRoutingTableKey(CP2.deviceId(), P1)); + assertEquals(M2, rtv2.macAddress); + assertEquals(V2, rtv2.vlanId); + assertEquals(CP2.port(), rtv2.portNumber); + + assertEquals(2, SUBNET_TABLE.size()); + assertTrue(SUBNET_TABLE.get(CP1).contains(P1)); + assertTrue(SUBNET_TABLE.get(CP2).contains(P1)); + + // next hop = CP2, CP4 + reset(srManager.deviceConfiguration); + srManager.deviceConfiguration.addSubnet(CP2, P1); + srManager.deviceConfiguration.addSubnet(CP4, P1); + expectLastCall().once(); + replay(srManager.deviceConfiguration); + + re = new RouteEvent(RouteEvent.Type.ALTERNATIVE_ROUTES_CHANGED, RR1, null, + Sets.newHashSet(RR2, RR4), Sets.newHashSet(RR1, RR2, RR4)); + routeHandler.processAlternativeRoutesChanged(re); + + assertEquals(2, ROUTING_TABLE.size()); + rtv1 = ROUTING_TABLE.get(new MockRoutingTableKey(CP2.deviceId(), P1)); + assertEquals(M2, rtv1.macAddress); + assertEquals(V2, rtv1.vlanId); + assertEquals(CP2.port(), rtv1.portNumber); + rtv2 = ROUTING_TABLE.get(new MockRoutingTableKey(CP4.deviceId(), P1)); + assertEquals(M4, rtv2.macAddress); + assertEquals(V4, rtv2.vlanId); + assertEquals(CP4.port(), rtv2.portNumber); + + assertEquals(2, SUBNET_TABLE.size()); + assertTrue(SUBNET_TABLE.get(CP2).contains(P1)); + assertTrue(SUBNET_TABLE.get(CP4).contains(P1)); + + verify(srManager.deviceConfiguration); + } } \ No newline at end of file