From 21e90a45f25b53e162458f330ad1c80bfe0b6aef Mon Sep 17 00:00:00 2001 From: Tomas Slusny Date: Tue, 14 Feb 2017 10:01:58 +0100 Subject: [PATCH] Fix comparison of match extensions Add comparison between match extension lists to MatchComparatorFactory to treat matches with different extensions as unique. Change-Id: Ia43322d5e3ca3ee62e8313fd1286a6a9c68f7423 Signed-off-by: Tomas Slusny --- .../extension/api/GroupingLooseResolver.java | 22 ++--- .../serialization/match/MatchSerializer.java | 18 ++-- .../impl/util/MatchComparatorFactory.java | 93 ++++++++++++------- .../convertor/match/MatchConvertorImpl.java | 4 +- 4 files changed, 83 insertions(+), 54 deletions(-) diff --git a/extension/openflowplugin-extension-api/src/main/java/org/opendaylight/openflowplugin/extension/api/GroupingLooseResolver.java b/extension/openflowplugin-extension-api/src/main/java/org/opendaylight/openflowplugin/extension/api/GroupingLooseResolver.java index 18480befd8..037715d34e 100644 --- a/extension/openflowplugin-extension-api/src/main/java/org/opendaylight/openflowplugin/extension/api/GroupingLooseResolver.java +++ b/extension/openflowplugin-extension-api/src/main/java/org/opendaylight/openflowplugin/extension/api/GroupingLooseResolver.java @@ -1,39 +1,37 @@ /** * Copyright (c) 2014 Cisco Systems, Inc. and others. All rights reserved. - * + * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ package org.opendaylight.openflowplugin.extension.api; +import com.google.common.base.Preconditions; import java.util.HashSet; +import java.util.Optional; import java.util.Set; - import org.opendaylight.yangtools.yang.binding.Augmentable; import org.opendaylight.yangtools.yang.binding.Augmentation; import org.opendaylight.yangtools.yang.binding.DataObject; -import com.google.common.base.Optional; -import com.google.common.base.Preconditions; - /** - * Provides augmentation resolving upon given {@link Augmentable}. + * Provides augmentation resolving upon given {@link Augmentable}. * Used {@link Augmentation}s do not share {@link Augmentable}. *
* Usage: in case there are multiple {@link Augmentable} classes which might contain - * corresponding {@link Augmentation}s (1:1..n binding). And those {@link Augmentation}s + * corresponding {@link Augmentation}s (1:1..n binding). And those {@link Augmentation}s * are sharing the same grouping so that they could be processed in the same way. - * + * * @param grouping */ public class GroupingLooseResolver { - Class commonInterface; - Set>> classes; + private Class commonInterface; + private Set>> classes; /** - * @param commonInterface + * @param commonInterface common interface */ public GroupingLooseResolver(Class commonInterface) { this.commonInterface = commonInterface; @@ -67,6 +65,6 @@ public class GroupingLooseResolver { } } - return Optional.absent(); + return Optional.empty(); } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/match/MatchSerializer.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/match/MatchSerializer.java index 7712dbecff..bb7454f33c 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/match/MatchSerializer.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/match/MatchSerializer.java @@ -87,24 +87,24 @@ public class MatchSerializer implements OFSerializer, HeaderSerializer { + ExtensionResolvers.getMatchExtensionResolver().getExtension(match).map(extensions -> { if (Objects.nonNull(extensions)) { - extensions.getExtensionList().forEach(extension-> { + extensions.getExtensionList().forEach(extension -> { // TODO: Remove also extension converters final MatchEntry entry = OFSessionUtil - .getExtensionConvertorProvider() - .getConverter(new ConverterExtensionKey<>( - extension.getExtensionKey(), - OFConstants.OFP_VERSION_1_3)) - .convert(extension.getExtension()); + .getExtensionConvertorProvider() + .getConverter(new ConverterExtensionKey<>( + extension.getExtensionKey(), + OFConstants.OFP_VERSION_1_3)) + .convert(extension.getExtension()); final MatchEntrySerializerKey key = new MatchEntrySerializerKey<>( - EncodeConstants.OF13_VERSION_ID, entry.getOxmClass(), entry.getOxmMatchField()); + EncodeConstants.OF13_VERSION_ID, entry.getOxmClass(), entry.getOxmMatchField()); // If entry is experimenter, set experimenter ID to key if (entry.getOxmClass().equals(ExperimenterClass.class)) { key.setExperimenterId(ExperimenterIdCase.class.cast(entry.getMatchEntryValue()) - .getExperimenter().getExperimenter().getValue()); + .getExperimenter().getExperimenter().getValue()); } final OFSerializer entrySerializer = registry.getSerializer(key); 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 7b005a7ae3..dc77ec61c9 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,12 +8,16 @@ package org.opendaylight.openflowplugin.impl.util; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Optional; import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion; +import org.opendaylight.openflowplugin.openflow.md.core.extension.ExtensionResolvers; 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; +import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.list.grouping.ExtensionList; /** * Provides comparator for comparing according to various {@link Match} attributes @@ -39,12 +43,44 @@ public final class MatchComparatorFactory { MATCH_COMPARATORS.add(MatchComparatorFactory.createNull()); MATCH_COMPARATORS.add(MatchComparatorFactory.createTunnel()); MATCH_COMPARATORS.add(MatchComparatorFactory.createVlan()); + MATCH_COMPARATORS.add(MatchComparatorFactory.createExtension()); + } + + private static SimpleComparator createExtension() { + return new SimpleComparator() { + /** + * Compare extension lists + */ + @Override + public boolean areObjectsEqual(final short version, final Match statsMatch, final Match storedMatch) { + return ExtensionResolvers + .getMatchExtensionResolver() + .getExtension(statsMatch) + .flatMap(statExt -> Optional.ofNullable(statExt.getExtensionList())) + .map(statList -> ExtensionResolvers + .getMatchExtensionResolver() + .getExtension(storedMatch) + .flatMap(storedExt -> Optional.ofNullable(storedExt.getExtensionList())) + .filter(storedList -> statList.size() == storedList.size()) + .map(storedList -> { + final Collection difference = new HashSet<>(statList); + difference.removeAll(storedList); + return difference.isEmpty(); + }) + .orElse(false)) + .orElse(!ExtensionResolvers + .getMatchExtensionResolver() + .getExtension(storedMatch) + .flatMap(storedExt -> Optional.ofNullable(storedExt.getExtensionList())) + .isPresent()); + } + }; } - public static SimpleComparator createNull() { + private static SimpleComparator createNull() { return new SimpleComparator() { /** - * Comparation by whole object + * Compares by whole object */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -53,10 +89,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createVlan() { + private static SimpleComparator createVlan() { return new SimpleComparator() { /** - * Comparation by VLAN + * Compares by VLAN */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -75,10 +111,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createTunnel() { + private static SimpleComparator createTunnel() { return new SimpleComparator() { /** - * Comparation by tunnel + * Compares by tunnel */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -97,10 +133,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createProtocolMatchFields() { + private static SimpleComparator createProtocolMatchFields() { return new SimpleComparator() { /** - * Comparation by protocol fields + * Compares by protocol fields */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -119,10 +155,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createMetadata() { + private static SimpleComparator createMetadata() { return new SimpleComparator() { /** - * Comparation by metadata + * Compares by metadata */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -141,10 +177,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createL4() { + private static SimpleComparator createL4() { return new SimpleComparator() { /** - * Comparation by layer4 + * Compares by layer4 */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -163,10 +199,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createL3() { + private static SimpleComparator createL3() { return new SimpleComparator() { /** - * Comparation by layer3 + * Compares by layer3 */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -185,10 +221,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createIp() { + private static SimpleComparator createIp() { return new SimpleComparator() { /** - * Comparation by Ip + * Compares by Ip */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -233,10 +269,10 @@ public final class MatchComparatorFactory { return true; } - public static SimpleComparator createInPort() { + private static SimpleComparator createInPort() { return new SimpleComparator() { /** - * Comparation by InPort + * Compares by InPort */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -255,10 +291,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createInPhyPort() { + private static SimpleComparator createInPhyPort() { return new SimpleComparator() { /** - * Comparation by InPhyPort + * Compares by InPhyPort */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -277,10 +313,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createEthernet() { + private static SimpleComparator createEthernet() { return new SimpleComparator() { /** - * Comparation by Ethernet + * Compares by Ethernet */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -299,10 +335,10 @@ public final class MatchComparatorFactory { }; } - public static SimpleComparator createIcmpv4() { + private static SimpleComparator createIcmpv4() { return new SimpleComparator() { /** - * Comparation by Icmpv4 + * Compares by Icmpv4 */ @Override public boolean areObjectsEqual(short version, Match statsMatch, Match storedMatch) { @@ -359,11 +395,6 @@ public final class MatchComparatorFactory { * request to the switch. So when statistics manager gets flow statistics, it gets the default value. * But the flow stored in config data store don't have those defaults value. I included those checks * in the customer flow/match equal function. - * - * - * @param statsMatch - * @param storedMatch - * @return */ private static boolean compareMatches(final short version, final Match statsMatch, final Match storedMatch) { if (statsMatch == storedMatch) { diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java index 9f273ed729..bd68916026 100644 --- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java +++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/match/MatchConvertorImpl.java @@ -9,17 +9,17 @@ package org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.match; -import com.google.common.base.Optional; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import org.opendaylight.openflowjava.util.ByteBufUtils; import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion; import org.opendaylight.openflowplugin.extension.api.ConverterExtensionKey; import org.opendaylight.openflowplugin.extension.api.ConvertorToOFJava; import org.opendaylight.openflowplugin.openflow.md.core.extension.ExtensionResolvers; -import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.OFApprovedExperimenterIds; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.ConvertorExecutor; +import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.OFApprovedExperimenterIds; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.common.ConvertorProcessor; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.VersionConvertorData; import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.match.cases.SalToOfArpMatchCase; -- 2.36.6