From ec6ac42337261990c4eb98dee282dad53e71a1de Mon Sep 17 00:00:00 2001 From: Pier Luigi Date: Mon, 29 Jan 2018 10:30:59 +0100 Subject: [PATCH] [CORD-2634] Fix verify operation in OFDPA pipeliner. Current code does not detect properly a mismatch between nextobj and flowobjstore. In particular it is able to detect only missing chains and duplicated chains, while falls if allactivekeys has more chains with respect to the next we want to verify. I think flapping links or subsequent events (for example several mcast-join follow by mcast-delete) can create easily this problem. Imagine we send a next with two treatments Next(x,y) where x and y are the output ports. AllActiveKeys has three chains AllActiveKeys(x,y,z) where x, y and z are the output ports. existingPortAndLabel is not able to detect the extraneous chain related to the port z Change-Id: I41fa47347a8c1d4188d990d96f48a898a4df59e1 --- .../pipeline/ofdpa/Ofdpa2GroupHandler.java | 19 ++++++ .../ofdpa/OfdpaGroupHandlerUtility.java | 61 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java index bb215ae50b..a3670e6cab 100644 --- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java +++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java @@ -1597,6 +1597,9 @@ public class Ofdpa2GroupHandler { List> allActiveKeys = appKryo.deserialize(next.data()); List bucketsToCreate = Lists.newArrayList(); List indicesToRemove = Lists.newArrayList(); + + // Iterating over the treatments of the next objective allows + // to detect missing buckets and/or duplicate buckets (to be removed) for (TrafficTreatment bkt : nextObjective.next()) { PortNumber portNumber = readOutPortFromTreatment(bkt); int label = readLabelFromTreatment(bkt); @@ -1621,6 +1624,22 @@ public class Ofdpa2GroupHandler { } } + // Detect situation where the next data has more buckets + // (not duplicates) respect to the next objective + if (allActiveKeys.size() > nextObjective.next().size()) { + log.warn("Mismatch detected between next and flowobjstore for device {}: " + + "nextId:{}, nextObjective-size:{} next-size:{} .. correcting", + deviceId, nextObjective.id(), nextObjective.next().size(), allActiveKeys.size()); + List otherIndices = indicesToRemoveFromNextGroup(allActiveKeys, nextObjective, + groupService, deviceId); + // Filter out the indices not present + otherIndices = otherIndices.stream() + .filter(index -> !indicesToRemove.contains(index)) + .collect(Collectors.toList()); + // Add all to the final list + indicesToRemove.addAll(otherIndices); + } + if (!bucketsToCreate.isEmpty()) { log.info("creating {} buckets as part of nextId: {} verification", bucketsToCreate.size(), nextObjective.id()); diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/OfdpaGroupHandlerUtility.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/OfdpaGroupHandlerUtility.java index df03656a61..e9c97bc61f 100644 --- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/OfdpaGroupHandlerUtility.java +++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/OfdpaGroupHandlerUtility.java @@ -263,6 +263,67 @@ public final class OfdpaGroupHandlerUtility { return indices; } + + /** + * Get indices to remove comparing next group with next objective. + * + * @param allActiveKeys the representation of the group + * @param nextObjective the next objective to verify + * @param groupService groups service for querying group information + * @param deviceId the device id for the device that contains the group + * @return a list of indexes in the allActiveKeys to remove. + */ + public static List indicesToRemoveFromNextGroup(List> allActiveKeys, + NextObjective nextObjective, + GroupService groupService, + DeviceId deviceId) { + List indicesToRemove = Lists.newArrayList(); + int index = 0; + // Iterate over the chain in the next data + for (Deque keyChain : allActiveKeys) { + // Valid chain should have at least two elements + if (keyChain.size() >= 2) { + // Get last group (l2if) and retrieve port number + GroupKey ifaceGroupKey = keyChain.peekLast(); + Group ifaceGroup = groupService.getGroup(deviceId, ifaceGroupKey); + if (ifaceGroup != null && !ifaceGroup.buckets().buckets().isEmpty()) { + PortNumber portNumber = readOutPortFromTreatment( + ifaceGroup.buckets().buckets().iterator().next().treatment()); + // If there is not a port number continue + if (portNumber != null) { + // check for label in the 2nd group of this chain + GroupKey secondKey = (GroupKey) keyChain.toArray()[1]; + Group secondGroup = groupService.getGroup(deviceId, secondKey); + // If there is not a second group or there are no buckets continue + if (secondGroup != null && !secondGroup.buckets().buckets().isEmpty()) { + // Get label or -1 + int label = readLabelFromTreatment( + secondGroup.buckets().buckets() + .iterator().next().treatment()); + // Iterate over the next treatments looking for the port and the label + boolean matches = false; + for (TrafficTreatment t : nextObjective.next()) { + PortNumber tPort = readOutPortFromTreatment(t); + int tLabel = readLabelFromTreatment(t); + if (tPort != null && tPort.equals(portNumber) && tLabel == label) { + // We found it, exit + matches = true; + break; + } + } + // Not found, we have to remove it + if (!matches) { + indicesToRemove.add(index); + } + } + } + } + } + index++; + } + return indicesToRemove; + } + /** * The purpose of this function is to verify if the hashed next * objective is supported by the current pipeline.