From 0ac8877c3539d82e55ffe49dc0a2c8bcb0c82b19 Mon Sep 17 00:00:00 2001 From: Tomas Slusny Date: Tue, 24 Jan 2017 17:57:13 +0100 Subject: [PATCH] Fix comparison between port numbers in match Properly extract port number from NodeConnectorId when comparing port numbers, as NodeConnectorId is basically string, and sometimes when going back from device, it do not contains datapath id or prefix (for example in new serialization/deserialization path). See also: bug 7139 Change-Id: I89999b71dba2e834ea1f37f42b5cddf1082bdd12 Signed-off-by: Tomas Slusny --- .../impl/device/DeviceContextImpl.java | 4 +- .../registry/flow/DeviceFlowRegistryImpl.java | 31 ++++----- .../registry/flow/FlowRegistryKeyFactory.java | 10 +-- .../impl/services/SalFlowServiceImpl.java | 10 +-- .../statistics/StatisticsGatheringUtils.java | 2 +- .../direct/FlowDirectStatisticsService.java | 2 +- .../impl/util/MatchComparatorFactory.java | 66 +++++++++++++------ .../impl/util/SimpleComparator.java | 4 +- .../impl/device/DeviceContextImplTest.java | 2 +- .../flow/DeviceFlowRegistryImplTest.java | 13 ++-- .../flow/FlowRegistryKeyFactoryTest.java | 24 +++---- .../impl/services/ServiceMocking.java | 2 +- .../statistics/StatisticsManagerImplTest.java | 3 +- 13 files changed, 103 insertions(+), 70 deletions(-) diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java index 26d13b9e18..515d3ec041 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java @@ -322,7 +322,7 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi final ItemLifecycleListener itemLifecycleListener = flowLifeCycleKeeper.getItemLifecycleListener(); if (itemLifecycleListener != null) { //2. create registry key - final FlowRegistryKey flowRegKey = FlowRegistryKeyFactory.create(flowRemovedNotification); + final FlowRegistryKey flowRegKey = FlowRegistryKeyFactory.create(getDeviceInfo().getVersion(), flowRemovedNotification); //3. lookup flowId final FlowDescriptor flowDescriptor = deviceFlowRegistry.retrieveIdForFlow(flowRegKey); //4. if flowId present: @@ -692,7 +692,7 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi LOG.debug("Transaction chain manager for node {} created", deviceInfo.getLOGValue()); } this.transactionChainManager = new TransactionChainManager(dataBroker, deviceInfo); - this.deviceFlowRegistry = new DeviceFlowRegistryImpl(dataBroker, deviceInfo.getNodeInstanceIdentifier()); + this.deviceFlowRegistry = new DeviceFlowRegistryImpl(deviceInfo.getVersion(), dataBroker, deviceInfo.getNodeInstanceIdentifier()); this.deviceGroupRegistry = new DeviceGroupRegistryImpl(); this.deviceMeterRegistry = new DeviceMeterRegistryImpl(); this.initialized = true; diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java index 01f7178e2f..01a92d00f4 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java @@ -51,23 +51,24 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { private final DataBroker dataBroker; private final KeyedInstanceIdentifier instanceIdentifier; private final List>>> lastFillFutures = new ArrayList<>(); + private final Consumer flowConsumer; - // Specifies what to do with flow read from datastore - private final Consumer flowConsumer = flow -> { - // Create flow registry key from flow - final FlowRegistryKey key = FlowRegistryKeyFactory.create(flow); - - // Now, we will update the registry, but we will also try to prevent duplicate entries - if (!flowRegistry.containsKey(key)) { - LOG.trace("Found flow with table ID : {} and flow ID : {}", flow.getTableId(), flow.getId().getValue()); - final FlowDescriptor descriptor = FlowDescriptorFactory.create(flow.getTableId(), flow.getId()); - store(key, descriptor); - } - }; - - public DeviceFlowRegistryImpl(final DataBroker dataBroker, final KeyedInstanceIdentifier instanceIdentifier) { + public DeviceFlowRegistryImpl(final short version, final DataBroker dataBroker, final KeyedInstanceIdentifier instanceIdentifier) { this.dataBroker = dataBroker; this.instanceIdentifier = instanceIdentifier; + + // Specifies what to do with flow read from datastore + flowConsumer = flow -> { + // Create flow registry key from flow + final FlowRegistryKey key = FlowRegistryKeyFactory.create(version, flow); + + // Now, we will update the registry, but we will also try to prevent duplicate entries + if (!flowRegistry.containsKey(key)) { + LOG.trace("Found flow with table ID : {} and flow ID : {}", flow.getTableId(), flow.getId().getValue()); + final FlowDescriptor descriptor = FlowDescriptorFactory.create(flow.getTableId(), flow.getId()); + store(key, descriptor); + } + }; } @Override @@ -235,4 +236,4 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry { final String alienId = ALIEN_SYSTEM_FLOW_ID + tableId + '-' + UNACCOUNTED_FLOWS_COUNTER.incrementAndGet(); return new FlowId(alienId); } -} \ No newline at end of file +} diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactory.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactory.java index 889e30a2c2..a691b60ba2 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactory.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactory.java @@ -27,8 +27,8 @@ public class FlowRegistryKeyFactory { // Hide implicit constructor } - public static FlowRegistryKey create(final Flow flow) { - return new FlowRegistryKeyDto(flow); + public static FlowRegistryKey create(final short version, final Flow flow) { + return new FlowRegistryKeyDto(version, flow); } private static final class FlowRegistryKeyDto implements FlowRegistryKey { @@ -36,13 +36,15 @@ public class FlowRegistryKeyFactory { private final int priority; private final BigInteger cookie; private final Match match; + private final short version; - private FlowRegistryKeyDto(final Flow flow) { + private FlowRegistryKeyDto(final short version, final Flow flow) { //TODO: mandatory flow input values (or default values) should be specified via yang model tableId = Preconditions.checkNotNull(flow.getTableId(), "flow tableId must not be null"); priority = MoreObjects.firstNonNull(flow.getPriority(), OFConstants.DEFAULT_FLOW_PRIORITY); match = MoreObjects.firstNonNull(flow.getMatch(), OFConstants.EMPTY_MATCH); cookie = MoreObjects.firstNonNull(flow.getCookie(), OFConstants.DEFAULT_FLOW_COOKIE).getValue(); + this.version = version; } @Override @@ -60,7 +62,7 @@ public class FlowRegistryKeyFactory { return getPriority() == that.getPriority() && getTableId() == that.getTableId() && getCookie().equals(that.getCookie()) && - MatchComparatorFactory.createMatch().areObjectsEqual(getMatch(), that.getMatch()); + MatchComparatorFactory.createMatch().areObjectsEqual(version, getMatch(), that.getMatch()); } @Override diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java index 5f032a7043..5a4ed71f75 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/SalFlowServiceImpl.java @@ -75,7 +75,7 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource { @Override public Future> addFlow(final AddFlowInput input) { - final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(input); + final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(deviceContext.getDeviceInfo().getVersion(), input); final ListenableFuture> future = flowAdd.processFlowModInputBuilders(flowAdd.toFlowModInputs(input)); Futures.addCallback(future, new AddFlowCallback(input, flowRegistryKey)); @@ -187,7 +187,7 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource { if (LOG.isDebugEnabled()) { LOG.debug("Flow remove finished without error for flow={}", input); } - FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(input); + FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(deviceContext.getDeviceInfo().getVersion(), input); deviceContext.getDeviceFlowRegistry().removeDescriptor(flowRegistryKey); if (itemLifecycleListener != null) { @@ -226,8 +226,8 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource { final UpdatedFlow updated = input.getUpdatedFlow(); final OriginalFlow original = input.getOriginalFlow(); - final FlowRegistryKey origFlowRegistryKey = FlowRegistryKeyFactory.create(original); - final FlowRegistryKey updatedFlowRegistryKey = FlowRegistryKeyFactory.create(updated); + final FlowRegistryKey origFlowRegistryKey = FlowRegistryKeyFactory.create(deviceContext.getDeviceInfo().getVersion(), original); + final FlowRegistryKey updatedFlowRegistryKey = FlowRegistryKeyFactory.create(deviceContext.getDeviceInfo().getVersion(), updated); final FlowDescriptor origFlowDescriptor = deviceFlowRegistry.retrieveIdForFlow(origFlowRegistryKey); final boolean isUpdate = Objects.nonNull(origFlowDescriptor); @@ -263,4 +263,4 @@ public class SalFlowServiceImpl implements SalFlowService, ItemLifeCycleSource { LOG.warn("Service call for updating flow={} failed, reason: {}", input, throwable); } } -} \ No newline at end of file +} diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java index ac14caeca6..e0dd990709 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsGatheringUtils.java @@ -284,7 +284,7 @@ public final class StatisticsGatheringUtils { flowBuilder.addAugmentation(FlowStatisticsData.class, refineFlowStatisticsAugmentation(flowStat).build()); final short tableId = flowStat.getTableId(); - final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(flowBuilder.build()); + final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowBuilder.build()); final FlowId flowId = registry.storeIfNecessary(flowRegistryKey); final FlowKey flowKey = new FlowKey(flowId); diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java index adbd1e9736..a106ad93e5 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/services/direct/FlowDirectStatisticsService.java @@ -167,7 +167,7 @@ public class FlowDirectStatisticsService extends AbstractDirectStatisticsService final FlowBuilder flowBuilder = new FlowBuilder(flowStatistics) .addAugmentation(FlowStatisticsData.class, flowStatisticsDataBld.build()); - final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(flowBuilder.build()); + final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(getVersion(), flowBuilder.build()); return getDeviceRegistry().getDeviceFlowRegistry().storeIfNecessary(flowRegistryKey); } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MatchComparatorFactory.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MatchComparatorFactory.java index 8c3b48e0fd..7b005a7ae3 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MatchComparatorFactory.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MatchComparatorFactory.java @@ -8,8 +8,10 @@ package org.opendaylight.openflowplugin.impl.util; +import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion; +import org.opendaylight.openflowplugin.openflow.md.util.InventoryDataServiceUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.flow.Match; - +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorId; import java.util.ArrayList; import java.util.Collection; @@ -45,7 +47,7 @@ public final class MatchComparatorFactory { * Comparation by whole object */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { return (statsMatch == null) == (storedMatch == null); } }; @@ -57,7 +59,7 @@ public final class MatchComparatorFactory { * Comparation by VLAN */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -79,7 +81,7 @@ public final class MatchComparatorFactory { * Comparation by tunnel */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -101,7 +103,7 @@ public final class MatchComparatorFactory { * Comparation by protocol fields */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -123,7 +125,7 @@ public final class MatchComparatorFactory { * Comparation by metadata */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -145,7 +147,7 @@ public final class MatchComparatorFactory { * Comparation by layer4 */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -167,7 +169,7 @@ public final class MatchComparatorFactory { * Comparation by layer3 */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -189,7 +191,7 @@ public final class MatchComparatorFactory { * Comparation by Ip */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -205,13 +207,39 @@ public final class MatchComparatorFactory { }; } + + /** + * Converts both ports in node connector id format to number format and compare them + * + * @param version openflow version + * @param left first object to compare + * @param right second object to compare + * @return true if equal + */ + private static boolean arePortNumbersEqual(short version, NodeConnectorId left, NodeConnectorId right) { + final OpenflowVersion ofVersion = OpenflowVersion.get(version); + + final Long leftPort = InventoryDataServiceUtil.portNumberfromNodeConnectorId(ofVersion, left); + final Long rightPort = InventoryDataServiceUtil.portNumberfromNodeConnectorId(ofVersion, right); + + if (leftPort == null) { + if (rightPort != null) { + return false; + } + } else if (!leftPort.equals(rightPort)) { + return false; + } + + return true; + } + public static SimpleComparator createInPort() { return new SimpleComparator() { /** * Comparation by InPort */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -219,7 +247,7 @@ public final class MatchComparatorFactory { if (statsMatch.getInPort() != null) { return false; } - } else if (!storedMatch.getInPort().equals(statsMatch.getInPort())) { + } else if (!arePortNumbersEqual(version, storedMatch.getInPort(), statsMatch.getInPort())) { return false; } return true; @@ -233,7 +261,7 @@ public final class MatchComparatorFactory { * Comparation by InPhyPort */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -241,7 +269,7 @@ public final class MatchComparatorFactory { if (statsMatch.getInPhyPort() != null) { return false; } - } else if (!storedMatch.getInPhyPort().equals(statsMatch.getInPhyPort())) { + } else if (!arePortNumbersEqual(version, storedMatch.getInPhyPort(), statsMatch.getInPhyPort())) { return false; } return true; @@ -255,7 +283,7 @@ public final class MatchComparatorFactory { * Comparation by Ethernet */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -277,7 +305,7 @@ public final class MatchComparatorFactory { * Comparation by Icmpv4 */ @Override - public boolean areObjectsEqual(Match statsMatch, Match storedMatch) { + public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { if (storedMatch == null) { return false; } @@ -299,12 +327,12 @@ public final class MatchComparatorFactory { * Compares flows by whole match */ @Override - public boolean areObjectsEqual(final Match statsFlow, final Match storedFlow) { + public boolean areObjectsEqual(short version, final Match statsFlow, final Match storedFlow) { if (statsFlow == null) { if (storedFlow != null) { return false; } - } else if (!compareMatches(statsFlow, storedFlow)) { + } else if (!compareMatches(version, statsFlow, storedFlow)) { return false; } return true; @@ -337,13 +365,13 @@ public final class MatchComparatorFactory { * @param storedMatch * @return */ - private static boolean compareMatches(final Match statsMatch, final Match storedMatch) { + private static boolean compareMatches(final short version, final Match statsMatch, final Match storedMatch) { if (statsMatch == storedMatch) { return true; } for (SimpleComparator matchComp : MATCH_COMPARATORS) { - if (!matchComp.areObjectsEqual(statsMatch, storedMatch)) { + if (!matchComp.areObjectsEqual(version, statsMatch, storedMatch)) { return false; } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/SimpleComparator.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/SimpleComparator.java index 55583e9c54..dfcea9b277 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/SimpleComparator.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/SimpleComparator.java @@ -10,6 +10,6 @@ package org.opendaylight.openflowplugin.impl.util; public interface SimpleComparator { - boolean areObjectsEqual(T obj1, T obj2); + boolean areObjectsEqual(short version, T obj1, T obj2); -} \ No newline at end of file +} diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java index 2af535ac93..e5a3167bc5 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java @@ -476,7 +476,7 @@ public class DeviceContextImplTest { .thenReturn(flowRemovedMdsalBld.build()); // insert flow+flowId into local registry - final FlowRegistryKey flowRegKey = FlowRegistryKeyFactory.create(flowRemovedMdsalBld.build()); + final FlowRegistryKey flowRegKey = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowRemovedMdsalBld.build()); final FlowDescriptor flowDescriptor = FlowDescriptorFactory.create((short) 0, new FlowId("ut-ofp:f456")); deviceContext.getDeviceFlowRegistry().store(flowRegKey, flowDescriptor); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java index dafa7ca27e..250d13615c 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImplTest.java @@ -33,6 +33,7 @@ import org.mockito.runners.MockitoJUnitRunner; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowDescriptor; import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowRegistryKey; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode; @@ -73,9 +74,9 @@ public class DeviceFlowRegistryImplTest { public void setUp() throws Exception { nodeInstanceIdentifier = InstanceIdentifier.create(Nodes.class).child(Node.class, new NodeKey(new NodeId(NODE_ID))); when(dataBroker.newReadOnlyTransaction()).thenReturn(readOnlyTransaction); - deviceFlowRegistry = new DeviceFlowRegistryImpl(dataBroker, nodeInstanceIdentifier); + deviceFlowRegistry = new DeviceFlowRegistryImpl(OFConstants.OFP_VERSION_1_3, dataBroker, nodeInstanceIdentifier); final FlowAndStatisticsMapList flowStats = TestFlowHelper.createFlowAndStatisticsMapListBuilder(1).build(); - key = FlowRegistryKeyFactory.create(flowStats); + key = FlowRegistryKeyFactory.create(OFConstants.OFP_VERSION_1_3, flowStats); descriptor = FlowDescriptorFactory.create(key.getTableId(), new FlowId("ut:1")); Assert.assertEquals(0, deviceFlowRegistry.getAllFlowDescriptors().size()); @@ -103,7 +104,7 @@ public class DeviceFlowRegistryImplTest { .build(); final Map allFlowDescriptors = testFill(path, flowCapableNode); - final FlowRegistryKey key = FlowRegistryKeyFactory.create(flow); + final FlowRegistryKey key = FlowRegistryKeyFactory.create(OFConstants.OFP_VERSION_1_3, flow); InOrder order = inOrder(dataBroker, readOnlyTransaction); order.verify(dataBroker).newReadOnlyTransaction(); @@ -178,7 +179,7 @@ public class DeviceFlowRegistryImplTest { // store new key with old value final FlowAndStatisticsMapList flowStats = TestFlowHelper.createFlowAndStatisticsMapListBuilder(2).build(); - final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(flowStats); + final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(OFConstants.OFP_VERSION_1_3, flowStats); deviceFlowRegistry.store(key2, descriptor); Assert.assertEquals(2, deviceFlowRegistry.getAllFlowDescriptors().size()); Assert.assertEquals("ut:1", deviceFlowRegistry.retrieveIdForFlow(key2).getFlowId().getValue()); @@ -197,7 +198,7 @@ public class DeviceFlowRegistryImplTest { //store new key final String alienPrefix = "#UF$TABLE*2-"; - final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(TestFlowHelper.createFlowAndStatisticsMapListBuilder(2).build()); + final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(OFConstants.OFP_VERSION_1_3, TestFlowHelper.createFlowAndStatisticsMapListBuilder(2).build()); newFlowId = deviceFlowRegistry.storeIfNecessary(key2); Assert.assertTrue(newFlowId.getValue().startsWith(alienPrefix)); @@ -238,4 +239,4 @@ public class DeviceFlowRegistryImplTest { return null; } -} \ No newline at end of file +} diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactoryTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactoryTest.java index 26313fc95f..35e08e6cdf 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactoryTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/registry/flow/FlowRegistryKeyFactoryTest.java @@ -71,8 +71,8 @@ public class FlowRegistryKeyFactoryTest { HashSet flowRegistryKeys = new HashSet<>(); for (FlowAndStatisticsMapList item : flowStats.getFlowAndStatisticsMapList()) { - final FlowRegistryKey key1 = FlowRegistryKeyFactory.create(item); - final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(item); + final FlowRegistryKey key1 = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), item); + final FlowRegistryKey key2 = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), item); flowRegistryKeys.add(key1); flowRegistryKeys.add(key1); flowRegistryKeys.add(key2); @@ -83,7 +83,7 @@ public class FlowRegistryKeyFactoryTest { @Test public void testEqualsNegative() throws Exception { final FlowAndStatisticsMapList flowStatisticsMapList1 = TestFlowHelper.createFlowAndStatisticsMapListBuilder(1).build(); - final FlowRegistryKey key1 = FlowRegistryKeyFactory.create(flowStatisticsMapList1); + final FlowRegistryKey key1 = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowStatisticsMapList1); FlowRegistryKey key2; FlowAndStatisticsMapListBuilder flowStatisticsMapListBld2; @@ -91,19 +91,19 @@ public class FlowRegistryKeyFactoryTest { // different priority flowStatisticsMapListBld2 = new FlowAndStatisticsMapListBuilder(flowStatisticsMapList1); flowStatisticsMapListBld2.setPriority(flowStatisticsMapListBld2.getPriority() + 1); - key2 = FlowRegistryKeyFactory.create(flowStatisticsMapListBld2.build()); + key2 = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowStatisticsMapListBld2.build()); Assert.assertFalse(key1.equals(key2)); // different match flowStatisticsMapListBld2 = new FlowAndStatisticsMapListBuilder(flowStatisticsMapList1); flowStatisticsMapListBld2.setMatch(new MatchBuilder().build()); - key2 = FlowRegistryKeyFactory.create(flowStatisticsMapListBld2.build()); + key2 = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowStatisticsMapListBld2.build()); Assert.assertFalse(key1.equals(key2)); // different tableId flowStatisticsMapListBld2 = new FlowAndStatisticsMapListBuilder(flowStatisticsMapList1); flowStatisticsMapListBld2.setTableId((short) (flowStatisticsMapListBld2.getTableId() + 1)); - key2 = FlowRegistryKeyFactory.create(flowStatisticsMapListBld2.build()); + key2 = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flowStatisticsMapListBld2.build()); Assert.assertFalse(key1.equals(key2)); Assert.assertFalse(key1.equals(null)); @@ -119,7 +119,7 @@ public class FlowRegistryKeyFactoryTest { .setPriority(2) .setTableId((short) 0); - FlowRegistryKey flow1Hash = FlowRegistryKeyFactory.create(flow1Builder.build()); + FlowRegistryKey flow1Hash = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flow1Builder.build()); LOG.info("flowHash1: {}", flow1Hash.hashCode()); @@ -129,7 +129,7 @@ public class FlowRegistryKeyFactoryTest { .setCookie(new FlowCookie(BigInteger.valueOf(148))) .setMatch(match2Builder.build()); - FlowRegistryKey flow2Hash = FlowRegistryKeyFactory.create(flow2Builder.build()); + FlowRegistryKey flow2Hash = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), flow2Builder.build()); LOG.info("flowHash2: {}", flow2Hash.hashCode()); Assert.assertNotSame(flow1Hash, flow2Hash); @@ -148,7 +148,7 @@ public class FlowRegistryKeyFactoryTest { FlowBuilder fb1 = new FlowBuilder(flow1Builder.build()); fb1.setTableId(null); try { - FlowRegistryKeyFactory.create(fb1.build()); + FlowRegistryKeyFactory.create(deviceInfo.getVersion(), fb1.build()); Assert.fail("hash creation should have failed because of NPE"); } catch (Exception e) { // expected @@ -158,7 +158,7 @@ public class FlowRegistryKeyFactoryTest { FlowBuilder fb2 = new FlowBuilder(flow1Builder.build()); fb2.setPriority(null); try { - FlowRegistryKeyFactory.create(fb2.build()); + FlowRegistryKeyFactory.create(deviceInfo.getVersion(), fb2.build()); } catch (Exception e) { // not expected Assert.fail("no exception was expected while hash was creating."); @@ -166,7 +166,7 @@ public class FlowRegistryKeyFactoryTest { FlowBuilder fb3 = new FlowBuilder(flow1Builder.build()); fb3.setCookie(null); - FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(fb3.build()); + FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), fb3.build()); Assert.assertNotNull(flowRegistryKey.getCookie()); Assert.assertEquals(OFConstants.DEFAULT_COOKIE, flowRegistryKey.getCookie()); } @@ -176,7 +176,7 @@ public class FlowRegistryKeyFactoryTest { FlowsStatisticsUpdate flowStats = FLOWS_STATISTICS_UPDATE_BUILDER.build(); for (FlowAndStatisticsMapList item : flowStats.getFlowAndStatisticsMapList()) { - FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(item); + FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(deviceInfo.getVersion(), item); FlowRegistryKey lastHash = null; if (null != lastHash) { assertNotEquals(lastHash, flowRegistryKey); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/ServiceMocking.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/ServiceMocking.java index fafa4df3f8..92180d798a 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/ServiceMocking.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/ServiceMocking.java @@ -106,7 +106,7 @@ public abstract class ServiceMocking { when(mockedDeviceContext.getPrimaryConnectionContext()).thenReturn(mockedPrimConnectionContext); when(mockedDeviceContext.getMessageSpy()).thenReturn(mockedMessagSpy); - when(mockedDeviceContext.getDeviceFlowRegistry()).thenReturn(new DeviceFlowRegistryImpl(dataBroker, NODE_II)); + when(mockedDeviceContext.getDeviceFlowRegistry()).thenReturn(new DeviceFlowRegistryImpl(DUMMY_VERSION, dataBroker, NODE_II)); when(mockedDeviceContext.getDeviceState()).thenReturn(mockedDeviceState); when(mockedDeviceContext.getDeviceInfo()).thenReturn(mockedDeviceInfo); when(mockedDeviceContext.getMultiMsgCollector(Matchers.any())).thenReturn(multiMessageCollector); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java index 873a84e293..77a3eb04e3 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java @@ -36,6 +36,7 @@ import org.opendaylight.controller.sal.binding.api.BindingAwareBroker; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; import org.opendaylight.openflowjava.protocol.api.connection.ConnectionAdapter; import org.opendaylight.openflowjava.protocol.api.connection.OutboundQueue; +import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; @@ -162,7 +163,7 @@ public class StatisticsManagerImplTest { when(mockedDeviceContext.getDeviceInfo()).thenReturn(mockedDeviceInfo); when(mockedDeviceContext.getPrimaryConnectionContext()).thenReturn(mockedPrimConnectionContext); when(mockedDeviceContext.getMessageSpy()).thenReturn(mockedMessagSpy); - when(mockedDeviceContext.getDeviceFlowRegistry()).thenReturn(new DeviceFlowRegistryImpl(dataBroker, nodePath)); + when(mockedDeviceContext.getDeviceFlowRegistry()).thenReturn(new DeviceFlowRegistryImpl(OFConstants.OFP_VERSION_1_3, dataBroker, nodePath)); when(mockedDeviceContext.getDeviceState()).thenReturn(mockedDeviceState); when(mockedDeviceContext.getMultiMsgCollector( Matchers.>>any())).thenAnswer( -- 2.36.6