홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Thomas Vachuska
Committed by Gerrit Code Review
b5f6f526 1 parent 3fad9bcf

Moving LLDP provider configuration processing off the event thread. 

Change-Id: Ief44d354081107c037870d9a7ede60e8a6447113
Inline Side-by-side
Showing 2 changed files with 85 additions and 71 deletions
...@@ -784,44 +784,46 @@ public class LldpLinkProvider extends AbstractProvider implements LinkProvider { ...@@ -784,44 +784,46 @@ public class LldpLinkProvider extends AbstractProvider implements LinkProvider {
784 784
785 @Override 785 @Override
786 public void event(NetworkConfigEvent event) { 786 public void event(NetworkConfigEvent event) {
787 - if (event.configClass() == LinkDiscoveryFromDevice.class && 787 + executor.submit(() -> {
788 - CONFIG_CHANGED.contains(event.type())) { 788 + if (event.configClass() == LinkDiscoveryFromDevice.class &&
789 + CONFIG_CHANGED.contains(event.type())) {
789 790
790 - if (event.subject() instanceof DeviceId) { 791 + if (event.subject() instanceof DeviceId) {
791 - final DeviceId did = (DeviceId) event.subject(); 792 + final DeviceId did = (DeviceId) event.subject();
792 - Device device = deviceService.getDevice(did); 793 + Device device = deviceService.getDevice(did);
793 - updateDevice(device).ifPresent(ld -> updatePorts(ld, did)); 794 + updateDevice(device).ifPresent(ld -> updatePorts(ld, did));
794 - } 795 + }
796 +
797 + } else if (event.configClass() == LinkDiscoveryFromPort.class &&
798 + CONFIG_CHANGED.contains(event.type())) {
799 +
800 + if (event.subject() instanceof ConnectPoint) {
801 + ConnectPoint cp = (ConnectPoint) event.subject();
802 + if (cp.elementId() instanceof DeviceId) {
803 + final DeviceId did = (DeviceId) cp.elementId();
804 + Device device = deviceService.getDevice(did);
805 + Port port = deviceService.getPort(did, cp.port());
806 + updateDevice(device).ifPresent(ld -> updatePort(ld, port));
807 + }
808 + }
795 809
796 - } else if (event.configClass() == LinkDiscoveryFromPort.class && 810 + } else if (event.configClass() == FingerprintProbeFromDevice.class &&
797 - CONFIG_CHANGED.contains(event.type())) { 811 + CONFIG_CHANGED.contains(event.type())) {
798 812
799 - if (event.subject() instanceof ConnectPoint) { 813 + if (event.subject() instanceof DeviceId) {
800 - ConnectPoint cp = (ConnectPoint) event.subject(); 814 + final DeviceId did = (DeviceId) event.subject();
801 - if (cp.elementId() instanceof DeviceId) {
802 - final DeviceId did = (DeviceId) cp.elementId();
803 Device device = deviceService.getDevice(did); 815 Device device = deviceService.getDevice(did);
804 - Port port = deviceService.getPort(did, cp.port()); 816 + updateDevice(device);
805 - updateDevice(device).ifPresent(ld -> updatePort(ld, port));
806 } 817 }
807 - }
808 -
809 - } else if (event.configClass() == FingerprintProbeFromDevice.class &&
810 - CONFIG_CHANGED.contains(event.type())) {
811 818
812 - if (event.subject() instanceof DeviceId) { 819 + } else if (event.configClass().equals(SuppressionConfig.class) &&
813 - final DeviceId did = (DeviceId) event.subject(); 820 + (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
814 - Device device = deviceService.getDevice(did); 821 + event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)) {
815 - updateDevice(device); 822 + SuppressionConfig cfg = cfgRegistry.getConfig(appId, SuppressionConfig.class);
823 + reconfigureSuppressionRules(cfg);
824 + log.trace("Network config reconfigured");
816 } 825 }
817 - 826 + });
818 - } else if (event.configClass().equals(SuppressionConfig.class) &&
819 - (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
820 - event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)) {
821 - SuppressionConfig cfg = cfgRegistry.getConfig(appId, SuppressionConfig.class);
822 - reconfigureSuppressionRules(cfg);
823 - log.trace("Network config reconfigured");
824 - }
825 } 827 }
826 } 828 }
827 } 829 }
......
...@@ -93,6 +93,7 @@ import static org.junit.Assert.assertFalse; ...@@ -93,6 +93,7 @@ import static org.junit.Assert.assertFalse;
93 import static org.junit.Assert.assertNotNull; 93 import static org.junit.Assert.assertNotNull;
94 import static org.junit.Assert.assertNull; 94 import static org.junit.Assert.assertNull;
95 import static org.junit.Assert.assertTrue; 95 import static org.junit.Assert.assertTrue;
96 +import static org.onlab.junit.TestTools.assertAfter;
96 import static org.onosproject.provider.lldp.impl.LldpLinkProvider.DEFAULT_RULES; 97 import static org.onosproject.provider.lldp.impl.LldpLinkProvider.DEFAULT_RULES;
97 98
98 99
...@@ -101,6 +102,7 @@ public class LldpLinkProviderTest { ...@@ -101,6 +102,7 @@ public class LldpLinkProviderTest {
101 private static final DeviceId DID1 = DeviceId.deviceId("of:0000000000000001"); 102 private static final DeviceId DID1 = DeviceId.deviceId("of:0000000000000001");
102 private static final DeviceId DID2 = DeviceId.deviceId("of:0000000000000002"); 103 private static final DeviceId DID2 = DeviceId.deviceId("of:0000000000000002");
103 private static final DeviceId DID3 = DeviceId.deviceId("of:0000000000000003"); 104 private static final DeviceId DID3 = DeviceId.deviceId("of:0000000000000003");
105 + private static final int EVENT_MS = 500;
104 106
105 private static Port pd1; 107 private static Port pd1;
106 private static Port pd2; 108 private static Port pd2;
...@@ -138,7 +140,7 @@ public class LldpLinkProviderTest { ...@@ -138,7 +140,7 @@ public class LldpLinkProviderTest {
138 cfg = new TestSuppressionConfig(); 140 cfg = new TestSuppressionConfig();
139 coreService = createMock(CoreService.class); 141 coreService = createMock(CoreService.class);
140 expect(coreService.registerApplication(appId.name())) 142 expect(coreService.registerApplication(appId.name()))
141 - .andReturn(appId).anyTimes(); 143 + .andReturn(appId).anyTimes();
142 replay(coreService); 144 replay(coreService);
143 145
144 provider.cfgService = new ComponentConfigAdapter(); 146 provider.cfgService = new ComponentConfigAdapter();
...@@ -199,8 +201,8 @@ public class LldpLinkProviderTest { ...@@ -199,8 +201,8 @@ public class LldpLinkProviderTest {
199 201
200 // update device in stub DeviceService with suppression config 202 // update device in stub DeviceService with suppression config
201 deviceService.putDevice(device(DID3, DefaultAnnotations.builder() 203 deviceService.putDevice(device(DID3, DefaultAnnotations.builder()
202 - .set(LldpLinkProvider.NO_LLDP, "true") 204 + .set(LldpLinkProvider.NO_LLDP, "true")
203 - .build())); 205 + .build()));
204 deviceListener.event(deviceEvent(DeviceEvent.Type.DEVICE_UPDATED, DID3)); 206 deviceListener.event(deviceEvent(DeviceEvent.Type.DEVICE_UPDATED, DID3));
205 207
206 // discovery on device is expected to be gone or stopped 208 // discovery on device is expected to be gone or stopped
...@@ -222,12 +224,13 @@ public class LldpLinkProviderTest { ...@@ -222,12 +224,13 @@ public class LldpLinkProviderTest {
222 DID3, 224 DID3,
223 LinkDiscoveryFromDevice.class)); 225 LinkDiscoveryFromDevice.class));
224 226
225 - // discovery helper for device is expected to be gone or stopped 227 + assertAfter(EVENT_MS, () -> {
226 - LinkDiscovery linkDiscovery = provider.discoverers.get(DID3); 228 + // discovery helper for device is expected to be gone or stopped
227 - if (linkDiscovery != null) { 229 + LinkDiscovery linkDiscovery = provider.discoverers.get(DID3);
228 - assertTrue("Discovery expected to be stopped", linkDiscovery.isStopped()); 230 + if (linkDiscovery != null) {
229 - } 231 + assertTrue("Discovery expected to be stopped", linkDiscovery.isStopped());
230 - 232 + }
233 + });
231 } 234 }
232 235
233 @Test 236 @Test
...@@ -258,7 +261,7 @@ public class LldpLinkProviderTest { ...@@ -258,7 +261,7 @@ public class LldpLinkProviderTest {
258 261
259 assertTrue("Port is not gone.", vanishedPort(3L)); 262 assertTrue("Port is not gone.", vanishedPort(3L));
260 assertFalse("Port was not removed from discoverer", 263 assertFalse("Port was not removed from discoverer",
261 - provider.discoverers.get(DID1).containsPort(3L)); 264 + provider.discoverers.get(DID1).containsPort(3L));
262 } 265 }
263 266
264 /** 267 /**
...@@ -271,8 +274,8 @@ public class LldpLinkProviderTest { ...@@ -271,8 +274,8 @@ public class LldpLinkProviderTest {
271 274
272 // add device in stub DeviceService with suppression configured 275 // add device in stub DeviceService with suppression configured
273 deviceService.putDevice(device(DID3, DefaultAnnotations.builder() 276 deviceService.putDevice(device(DID3, DefaultAnnotations.builder()
274 - .set(LldpLinkProvider.NO_LLDP, "true") 277 + .set(LldpLinkProvider.NO_LLDP, "true")
275 - .build())); 278 + .build()));
276 deviceListener.event(deviceEvent(DeviceEvent.Type.DEVICE_ADDED, DID3)); 279 deviceListener.event(deviceEvent(DeviceEvent.Type.DEVICE_ADDED, DID3));
277 280
278 // non-suppressed port added to suppressed device 281 // non-suppressed port added to suppressed device
...@@ -395,9 +398,9 @@ public class LldpLinkProviderTest { ...@@ -395,9 +398,9 @@ public class LldpLinkProviderTest {
395 // suppressed port added to non-suppressed device 398 // suppressed port added to non-suppressed device
396 final long portno3 = 3L; 399 final long portno3 = 3L;
397 final Port port3 = port(DID3, portno3, true, 400 final Port port3 = port(DID3, portno3, true,
398 - DefaultAnnotations.builder() 401 + DefaultAnnotations.builder()
399 - .set(LldpLinkProvider.NO_LLDP, "true") 402 + .set(LldpLinkProvider.NO_LLDP, "true")
400 - .build()); 403 + .build());
401 deviceService.putPorts(DID3, port3); 404 deviceService.putPorts(DID3, port3);
402 deviceListener.event(portEvent(DeviceEvent.Type.PORT_ADDED, DID3, port3)); 405 deviceListener.event(portEvent(DeviceEvent.Type.PORT_ADDED, DID3, port3));
403 406
...@@ -486,27 +489,27 @@ public class LldpLinkProviderTest { ...@@ -486,27 +489,27 @@ public class LldpLinkProviderTest {
486 489
487 private DefaultDevice device(DeviceId did) { 490 private DefaultDevice device(DeviceId did) {
488 return new DefaultDevice(ProviderId.NONE, did, Device.Type.SWITCH, 491 return new DefaultDevice(ProviderId.NONE, did, Device.Type.SWITCH,
489 - "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId()); 492 + "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
490 } 493 }
491 494
492 private DefaultDevice device(DeviceId did, Device.Type type) { 495 private DefaultDevice device(DeviceId did, Device.Type type) {
493 return new DefaultDevice(ProviderId.NONE, did, type, 496 return new DefaultDevice(ProviderId.NONE, did, type,
494 - "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId()); 497 + "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
495 } 498 }
496 499
497 private DefaultDevice device(DeviceId did, Annotations annotations) { 500 private DefaultDevice device(DeviceId did, Annotations annotations) {
498 return new DefaultDevice(ProviderId.NONE, did, Device.Type.SWITCH, 501 return new DefaultDevice(ProviderId.NONE, did, Device.Type.SWITCH,
499 - "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId(), annotations); 502 + "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId(), annotations);
500 } 503 }
501 504
502 - @SuppressWarnings(value = { "unused" }) 505 + @SuppressWarnings(value = {"unused"})
503 private DeviceEvent portEvent(DeviceEvent.Type type, DeviceId did, PortNumber port) { 506 private DeviceEvent portEvent(DeviceEvent.Type type, DeviceId did, PortNumber port) {
504 - return new DeviceEvent(type, deviceService.getDevice(did), 507 + return new DeviceEvent(type, deviceService.getDevice(did),
505 - deviceService.getPort(did, port)); 508 + deviceService.getPort(did, port));
506 } 509 }
507 510
508 private DeviceEvent portEvent(DeviceEvent.Type type, DeviceId did, Port port) { 511 private DeviceEvent portEvent(DeviceEvent.Type type, DeviceId did, Port port) {
509 - return new DeviceEvent(type, deviceService.getDevice(did), port); 512 + return new DeviceEvent(type, deviceService.getDevice(did), port);
510 } 513 }
511 514
512 private Port port(DeviceId did, long port, boolean enabled) { 515 private Port port(DeviceId did, long port, boolean enabled) {
...@@ -578,8 +581,10 @@ public class LldpLinkProviderTest { ...@@ -578,8 +581,10 @@ public class LldpLinkProviderTest {
578 581
579 configEvent(NetworkConfigEvent.Type.CONFIG_UPDATED); 582 configEvent(NetworkConfigEvent.Type.CONFIG_UPDATED);
580 583
581 - assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType1)); 584 + assertAfter(EVENT_MS, () -> {
582 - assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType2)); 585 + assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType1));
586 + assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType2));
587 + });
583 } 588 }
584 589
585 @Test 590 @Test
...@@ -594,9 +599,11 @@ public class LldpLinkProviderTest { ...@@ -594,9 +599,11 @@ public class LldpLinkProviderTest {
594 599
595 configEvent(NetworkConfigEvent.Type.CONFIG_ADDED); 600 configEvent(NetworkConfigEvent.Type.CONFIG_ADDED);
596 601
597 - assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1)); 602 + assertAfter(EVENT_MS, () -> {
598 - assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1)); 603 + assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
599 - assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2)); 604 + assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
605 + assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2));
606 + });
600 } 607 }
601 608
602 @Test 609 @Test
...@@ -610,25 +617,29 @@ public class LldpLinkProviderTest { ...@@ -610,25 +617,29 @@ public class LldpLinkProviderTest {
610 617
611 configEvent(NetworkConfigEvent.Type.CONFIG_ADDED); 618 configEvent(NetworkConfigEvent.Type.CONFIG_ADDED);
612 619
613 - assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1)); 620 + assertAfter(EVENT_MS, () -> {
614 - assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1)); 621 + assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
615 - assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2)); 622 + assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
623 + assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2));
624 + });
616 625
617 annotation.put(key2, value2); 626 annotation.put(key2, value2);
618 cfg.annotation(annotation); 627 cfg.annotation(annotation);
619 628
620 configEvent(NetworkConfigEvent.Type.CONFIG_UPDATED); 629 configEvent(NetworkConfigEvent.Type.CONFIG_UPDATED);
621 630
622 - assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1)); 631 + assertAfter(EVENT_MS, () -> {
623 - assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1)); 632 + assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
624 - assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key2)); 633 + assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
625 - assertEquals(value2, provider.rules().getSuppressedAnnotation().get(key2)); 634 + assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key2));
635 + assertEquals(value2, provider.rules().getSuppressedAnnotation().get(key2));
636 + });
626 } 637 }
627 638
628 private void configEvent(NetworkConfigEvent.Type evType) { 639 private void configEvent(NetworkConfigEvent.Type evType) {
629 configListener.event(new NetworkConfigEvent(evType, 640 configListener.event(new NetworkConfigEvent(evType,
630 - appId, 641 + appId,
631 - SuppressionConfig.class)); 642 + SuppressionConfig.class));
632 } 643 }
633 644
634 private class TestPacketContext implements PacketContext { 645 private class TestPacketContext implements PacketContext {
...@@ -706,6 +717,7 @@ public class LldpLinkProviderTest { ...@@ -706,6 +717,7 @@ public class LldpLinkProviderTest {
706 private final Map<DeviceId, Device> devices = new HashMap<>(); 717 private final Map<DeviceId, Device> devices = new HashMap<>();
707 private final ArrayListMultimap<DeviceId, Port> ports = 718 private final ArrayListMultimap<DeviceId, Port> ports =
708 ArrayListMultimap.create(); 719 ArrayListMultimap.create();
720 +
709 public TestDeviceService() { 721 public TestDeviceService() {
710 Device d1 = new DefaultDevice(ProviderId.NONE, DID1, Device.Type.SWITCH, 722 Device d1 = new DefaultDevice(ProviderId.NONE, DID1, Device.Type.SWITCH,
711 "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId()); 723 "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
...@@ -727,7 +739,7 @@ public class LldpLinkProviderTest { ...@@ -727,7 +739,7 @@ public class LldpLinkProviderTest {
727 devices.put(deviceId, device); 739 devices.put(deviceId, device);
728 } 740 }
729 741
730 - private void putPorts(DeviceId did, Port...ports) { 742 + private void putPorts(DeviceId did, Port... ports) {
731 this.ports.putAll(did, Lists.newArrayList(ports)); 743 this.ports.putAll(did, Lists.newArrayList(ports));
732 } 744 }
733 745
...@@ -831,7 +843,7 @@ public class LldpLinkProviderTest { ...@@ -831,7 +843,7 @@ public class LldpLinkProviderTest {
831 } 843 }
832 844
833 private final class TestNetworkConfigRegistry 845 private final class TestNetworkConfigRegistry
834 - extends NetworkConfigRegistryAdapter { 846 + extends NetworkConfigRegistryAdapter {
835 @SuppressWarnings("unchecked") 847 @SuppressWarnings("unchecked")
836 @Override 848 @Override
837 public <S, C extends Config<S>> C getConfig(S subj, Class<C> configClass) { 849 public <S, C extends Config<S>> C getConfig(S subj, Class<C> configClass) {
...@@ -896,8 +908,8 @@ public class LldpLinkProviderTest { ...@@ -896,8 +908,8 @@ public class LldpLinkProviderTest {
896 final IpAddress addr = IpAddress.valueOf(0); 908 final IpAddress addr = IpAddress.valueOf(0);
897 final Partition p = new DefaultPartition(PartitionId.from(1), Sets.newHashSet(nid)); 909 final Partition p = new DefaultPartition(PartitionId.from(1), Sets.newHashSet(nid));
898 return new ClusterMetadata("test-cluster", 910 return new ClusterMetadata("test-cluster",
899 - Sets.newHashSet(new DefaultControllerNode(nid, addr)), 911 + Sets.newHashSet(new DefaultControllerNode(nid, addr)),
900 - Sets.newHashSet(p)); 912 + Sets.newHashSet(p));
901 } 913 }
902 914
903 @Override 915 @Override
......