From 3495256c2cab615fd3119ee88538e248ed6f2a63 Mon Sep 17 00:00:00 2001 From: Matej Sramcik Date: Thu, 21 Sep 2023 09:53:44 +0200 Subject: [PATCH] Do not use NotificationListener in LISP components Migrated usage of of NotificationListener to Listener migration for LISP components. Also fixes MapServer not relinquishing its listener registration. JIRA: LISPMAP-184 Change-Id: I8f2d22a7c1bad8d7e3bf23f2cc1580e9b4639701 Signed-off-by: Matej Sramcik Signed-off-by: Robert Varga --- .../MappingServiceIntegrationTest.java | 60 +++++-------------- .../implementation/LispMappingService.java | 54 ++++++++++------- .../implementation/lisp/MapServer.java | 32 +++++----- .../LispMappingServiceTest.java | 4 +- 4 files changed, 65 insertions(+), 85 deletions(-) diff --git a/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java b/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java index a03edb0c9..72ea2154e 100644 --- a/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java +++ b/integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java @@ -73,6 +73,7 @@ import org.opendaylight.lispflowmapping.lisp.util.LispAddressUtil; import org.opendaylight.lispflowmapping.lisp.util.MappingRecordUtil; import org.opendaylight.lispflowmapping.lisp.util.MaskUtil; import org.opendaylight.lispflowmapping.type.sbplugin.IConfigLispSouthboundPlugin; +import org.opendaylight.mdsal.binding.api.NotificationService.Listener; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpPrefix; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address; @@ -106,18 +107,11 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types. import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.inet.binary.types.rev160303.Ipv4AddressBinary; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.binary.address.types.rev160504.Ipv4PrefixBinaryAfi; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.binary.address.types.rev160504.augmented.lisp.address.address.Ipv4PrefixBinaryBuilder; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.AddMapping; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.GotMapNotify; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.GotMapReply; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapNotify; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapRegister; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapReply; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapRequest; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MappingKeepAlive; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MessageType; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.OdlLispProtoListener; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.RequestMapping; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrReplyMapping; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrRequestMapping; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.Eid; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.EidBuilder; @@ -167,7 +161,6 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { private byte[] mapRegisterPacketWithoutNotify; String lispBindAddress = "127.0.0.1"; static final String ourAddress = "127.0.0.2"; - private Rloc locatorEid; private DatagramSocket socket; private byte[] mapRegisterPacketWithAuthenticationAndMapNotify; @@ -2900,36 +2893,22 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { } - private class XtrRequestMappingListener implements OdlLispProtoListener { + private class XtrRequestMappingListener implements Listener { + private final String eid; + private final MapRequest mapRequest; - @Override - public void onGotMapReply(GotMapReply notification) { - } - - @Override - public void onAddMapping(AddMapping notification) { - } - - @Override - public void onXtrReplyMapping(XtrReplyMapping notification) { - } - - @Override - public void onRequestMapping(RequestMapping notification) { + private XtrRequestMappingListener(String eid, MapRequest mapRequest) { + this.eid = eid; + this.mapRequest = mapRequest; } @Override - public void onGotMapNotify(GotMapNotify notification) { + public void onNotification(XtrRequestMapping notification) { + assertEquals(((Ipv4Prefix) mapRequest.getEidItem().get(0).getEid().getAddress()) + .getIpv4Prefix().getValue(), eid); + notificationCalled = true; + LOG.warn("notification arrived"); } - - @Override - public void onXtrRequestMapping(XtrRequestMapping notification) { - } - - @Override - public void onMappingKeepAlive(MappingKeepAlive notification) { - } - } public void testRecievingNonProxyOnXtrPort() throws SocketTimeoutException, SocketException, Throwable { @@ -2950,17 +2929,10 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase { Rloc adLcaf = rb.build(); final MapRequest mapRequest = createNonProxyMapRequest(eid, adLcaf); - ((LispMappingService) lms).getNotificationService().registerNotificationListener( - new XtrRequestMappingListener() { - - @Override - public void onXtrRequestMapping(XtrRequestMapping notification) { - assertEquals(((Ipv4Prefix) mapRequest.getEidItem().get(0).getEid().getAddress()) - .getIpv4Prefix().getValue(), eid); - notificationCalled = true; - LOG.warn("notification arrived"); - } - }); + final var handler = new XtrRequestMappingListener(eid, mapRequest); + + ((LispMappingService) lms).getNotificationService().registerListener(XtrRequestMapping.class, handler); + sendMapRequest(mapRequest, port); for (int i = 0; i < MAX_NOTIFICATION_RETRYS; i++) { if (notificationCalled) { diff --git a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/LispMappingService.java b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/LispMappingService.java index bec9825fe..968b26408 100644 --- a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/LispMappingService.java +++ b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/LispMappingService.java @@ -8,9 +8,11 @@ package org.opendaylight.lispflowmapping.implementation; +import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import java.util.List; +import java.util.Set; import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Singleton; @@ -24,12 +26,12 @@ import org.opendaylight.lispflowmapping.interfaces.lisp.IFlowMapping; import org.opendaylight.lispflowmapping.interfaces.lisp.IMapNotifyHandler; import org.opendaylight.lispflowmapping.interfaces.lisp.IMapRequestResultHandler; import org.opendaylight.lispflowmapping.interfaces.lisp.IMapResolverAsync; -import org.opendaylight.lispflowmapping.interfaces.lisp.IMapServerAsync; import org.opendaylight.lispflowmapping.interfaces.lisp.ISmrNotificationListener; import org.opendaylight.lispflowmapping.interfaces.mappingservice.IMappingService; import org.opendaylight.lispflowmapping.lisp.type.LispMessage; import org.opendaylight.lispflowmapping.lisp.util.LispAddressStringifier; import org.opendaylight.mdsal.binding.api.NotificationService; +import org.opendaylight.mdsal.binding.api.NotificationService.CompositeListener; import org.opendaylight.mdsal.binding.api.RpcProviderService; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider; @@ -43,7 +45,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.Ma import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapReply; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapRequest; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MappingKeepAlive; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.OdlLispProtoListener; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.RequestMapping; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrId; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrReplyMapping; @@ -73,7 +74,7 @@ import org.slf4j.LoggerFactory; @Component(service = {IFlowMapping.class, IMapRequestResultHandler.class, IMapNotifyHandler.class}, immediate = true, property = "type=default") public class LispMappingService implements IFlowMapping, IMapRequestResultHandler, - IMapNotifyHandler, OdlLispProtoListener, AutoCloseable, ClusterSingletonService { + IMapNotifyHandler, AutoCloseable, ClusterSingletonService { private static final String LISPFLOWMAPPING_ENTITY_NAME = "lispflowmapping"; private static final ServiceGroupIdentifier SERVICE_GROUP_IDENTIFIER = ServiceGroupIdentifier.create( LISPFLOWMAPPING_ENTITY_NAME); @@ -88,12 +89,11 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle private ThreadLocal> tlsMapRequest = new ThreadLocal<>(); private IMapResolverAsync mapResolver; - private IMapServerAsync mapServer; + private MapServer mapServer; private final IMappingService mapService; private final OdlLispSbService lispSB; private final ClusterSingletonServiceProvider clusterSingletonService; - private final RpcProviderService rpcProviderService; private final NotificationService notificationService; private final Registration rpcRegistration; private final Registration listenerRegistration; @@ -108,11 +108,17 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle this.mapService = mappingService; this.lispSB = odlLispService; this.clusterSingletonService = clusterSingletonService; - this.rpcProviderService = rpcProviderService; this.notificationService = notificationService; // initialize - listenerRegistration = notificationService.registerNotificationListener(this); + listenerRegistration = notificationService.registerCompositeListener(new CompositeListener(Set.of( + new CompositeListener.Component<>(AddMapping.class, this::onAddMapping), + new CompositeListener.Component<>(GotMapNotify.class, this::onGotMapNotify), + new CompositeListener.Component<>(RequestMapping.class, this::onRequestMapping), + new CompositeListener.Component<>(GotMapReply.class, this::onGotMapReply), + new CompositeListener.Component<>(XtrRequestMapping.class, this::onXtrRequestMapping), + new CompositeListener.Component<>(XtrReplyMapping.class, this::onXtrReplyMapping), + new CompositeListener.Component<>(MappingKeepAlive.class, this::onMappingKeepAlive)))); rpcRegistration = rpcProviderService.registerRpcImplementation(OdlLispSbService.class, lispSB); mapResolver = new MapResolver(mapService, smr, elpPolicy, this); @@ -189,8 +195,8 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle getLispSB().sendMapNotify(smnib.build()); } - @Override - public void onAddMapping(AddMapping mapRegisterNotification) { + @VisibleForTesting + void onAddMapping(AddMapping mapRegisterNotification) { Pair> result = handleMapRegister(mapRegisterNotification.getMapRegister()); if (result != null && result.getLeft() != null) { MapNotify mapNotify = result.getLeft(); @@ -210,8 +216,8 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle } } - @Override - public void onRequestMapping(RequestMapping mapRequestNotification) { + @VisibleForTesting + void onRequestMapping(RequestMapping mapRequestNotification) { MapReply mapReply = handleMapRequest(mapRequestNotification.getMapRequest()); if (mapReply != null) { SendMapReplyInputBuilder smrib = new SendMapReplyInputBuilder(); @@ -223,28 +229,28 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle } } - @Override - public void onGotMapReply(GotMapReply notification) { + @VisibleForTesting + void onGotMapReply(GotMapReply notification) { LOG.debug("Received GotMapReply notification, ignoring"); } - @Override - public void onGotMapNotify(GotMapNotify notification) { + @VisibleForTesting + void onGotMapNotify(GotMapNotify notification) { LOG.debug("Received GotMapNotify notification, ignoring"); } - @Override - public void onXtrRequestMapping(XtrRequestMapping notification) { + @VisibleForTesting + void onXtrRequestMapping(XtrRequestMapping notification) { LOG.debug("Received XtrRequestMapping notification, ignoring"); } - @Override - public void onXtrReplyMapping(XtrReplyMapping notification) { + @VisibleForTesting + void onXtrReplyMapping(XtrReplyMapping notification) { LOG.debug("Received XtrReplyMapping notification, ignoring"); } - @Override - public void onMappingKeepAlive(MappingKeepAlive notification) { + @VisibleForTesting + void onMappingKeepAlive(MappingKeepAlive notification) { final MapRegisterCacheMetadata cacheMetadata = notification.getMapRegisterCacheMetadata(); for (EidLispAddress eidLispAddress : cacheMetadata.nonnullEidLispAddress().values()) { final Eid eid = eidLispAddress.getEid(); @@ -293,7 +299,10 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle private void destroy() { LOG.info("LISP (RFC6830) Mapping Service is destroyed!"); mapResolver = null; - mapServer = null; + if (mapServer != null) { + mapServer.close(); + mapServer = null; + } } @Deactivate @@ -324,4 +333,3 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle return SERVICE_GROUP_IDENTIFIER; } } - diff --git a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java index 1735e73cd..50efa073f 100644 --- a/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java +++ b/mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java @@ -50,6 +50,7 @@ import org.opendaylight.lispflowmapping.lisp.util.MappingRecordUtil; import org.opendaylight.lispflowmapping.lisp.util.MaskUtil; import org.opendaylight.lispflowmapping.lisp.util.SourceDestKeyHelper; import org.opendaylight.mdsal.binding.api.NotificationService; +import org.opendaylight.mdsal.binding.api.NotificationService.Listener; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.lisp.address.types.rev151105.lisp.address.Address; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.lisp.address.types.rev151105.lisp.address.address.SourceDestKey; @@ -62,7 +63,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.Ma import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.SiteId; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.Eid; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.EidBuilder; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.list.EidItem; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.list.EidItemBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapnotifymessage.MapNotifyBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapping._record.container.MappingRecord; @@ -75,32 +75,32 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.tr import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.transport.address.TransportAddressBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.MappingChanged; import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.MappingOrigin; -import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.OdlMappingserviceListener; -import org.opendaylight.yangtools.concepts.ListenerRegistration; +import org.opendaylight.yangtools.concepts.Registration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class MapServer implements IMapServerAsync, OdlMappingserviceListener, ISmrNotificationListener { - +public class MapServer implements IMapServerAsync, ISmrNotificationListener, Listener, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(MapServer.class); private static final byte[] ALL_ZEROES_XTR_ID = new byte[] {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ,0}; + + private final SmrScheduler scheduler = new SmrScheduler(); private final IMappingService mapService; - private boolean subscriptionService; private final IMapNotifyHandler notifyHandler; - private final NotificationService notificationService; - private ListenerRegistration mapServerListenerRegistration; - private final SmrScheduler scheduler; + private final Registration listenerRegistration; + + private boolean subscriptionService; public MapServer(IMappingService mapService, boolean subscriptionService, IMapNotifyHandler notifyHandler, NotificationService notificationService) { this.mapService = requireNonNull(mapService); this.subscriptionService = subscriptionService; this.notifyHandler = notifyHandler; - this.notificationService = notificationService; - if (notificationService != null) { - notificationService.registerNotificationListener(this); - } - scheduler = new SmrScheduler(); + listenerRegistration = notificationService.registerListener(MappingChanged.class, this); + } + + @Override + public void close() { + listenerRegistration.close(); } @Override @@ -203,7 +203,7 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS } @Override - public void onMappingChanged(MappingChanged notification) { + public void onNotification(MappingChanged notification) { if (subscriptionService) { Eid eid = notification.getEid(); if (eid == null) { @@ -411,7 +411,7 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS // a given mapping. if (executionCount <= ConfigIni.getInstance().getSmrRetryCount()) { synchronized (mrb) { - mrb.setEidItem(new ArrayList()); + mrb.setEidItem(new ArrayList<>()); mrb.getEidItem().add(new EidItemBuilder() .setEidItemId(LispAddressStringifier.getString(subscriber.getSrcEid())) .setEid(subscriber.getSrcEid()).build()); diff --git a/mappingservice/implementation/src/test/java/org/opendaylight/lispflowmapping/implementation/LispMappingServiceTest.java b/mappingservice/implementation/src/test/java/org/opendaylight/lispflowmapping/implementation/LispMappingServiceTest.java index 021da330b..f022e5096 100644 --- a/mappingservice/implementation/src/test/java/org/opendaylight/lispflowmapping/implementation/LispMappingServiceTest.java +++ b/mappingservice/implementation/src/test/java/org/opendaylight/lispflowmapping/implementation/LispMappingServiceTest.java @@ -24,8 +24,8 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import org.opendaylight.lispflowmapping.implementation.lisp.MapServer; import org.opendaylight.lispflowmapping.interfaces.lisp.IMapResolverAsync; -import org.opendaylight.lispflowmapping.interfaces.lisp.IMapServerAsync; import org.opendaylight.lispflowmapping.interfaces.mappingservice.IMappingService; import org.opendaylight.lispflowmapping.lisp.type.LispMessage; import org.opendaylight.lispflowmapping.lisp.util.LispAddressUtil; @@ -73,7 +73,7 @@ import org.opendaylight.yangtools.yang.common.Uint16; @RunWith(MockitoJUnitRunner.class) public class LispMappingServiceTest { @Mock private static MapRegister mapRegisterMock; - @Mock(name = "mapServer") private static IMapServerAsync mapServerMock; + @Mock(name = "mapServer") private static MapServer mapServerMock; @Mock(name = "mapResolver") private static IMapResolverAsync mapResolverMock; @Mock(name = "tlsMapReply") private static ThreadLocal tlsMapReplyMock; @Mock(name = "tlsMapNotify") private static ThreadLocal>> tlsMapNotifyMock; -- 2.36.6