From 3a4f5033bfa1d048f435478f9db89b61a9b70723 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 27 Jan 2019 22:54:15 +0100 Subject: [PATCH] Rework table modification application This reworks the way a table modification is applied so that we examine and handle the attributes first -- hence we can understand that a table has become LLGR_STALE and perform an overwrite. This also cleans up some the code paths, so they can be optimized in future, for example the logic for handling route targets. JIRA: BGPCEP-495 Change-Id: Ic20f55e2fe985b41e971d084ae683eb574b9da33 Signed-off-by: Robert Varga --- .../bgp/rib/impl/EffectiveRibInWriter.java | 156 ++++++++---------- 1 file changed, 70 insertions(+), 86 deletions(-) diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java index f52155d099..460e2c9aa8 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/EffectiveRibInWriter.java @@ -67,6 +67,7 @@ import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; +import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; @@ -76,6 +77,7 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType; +import org.opendaylight.yangtools.yang.data.impl.schema.Builders; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -107,6 +109,7 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn .xml.ns.yang.bgp.message.rev180329.path.attributes.AttributesBuilder() .setCommunities(STALE_LLGR_COMMUNUTIES) .build(); + private static final ChoiceNode EMPTY_ROUTES = Builders.choiceBuilder().withNodeIdentifier(ROUTES).build(); private final RIBSupportContextRegistry registry; private final YangInstanceIdentifier peerIId; @@ -308,7 +311,47 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn private void modifyTable(final DOMDataWriteTransaction tx, final RIBSupportContext ribContext, final YangInstanceIdentifier effectiveTablePath, final DataTreeCandidateNode table) { LOG.debug("Modify Effective Table {}", effectiveTablePath); - processTableChildren(tx, ribContext.getRibSupport(), effectiveTablePath, table.getChildNodes()); + + final DataTreeCandidateNode modifiedAttrs = table.getModifiedChild(ATTRIBUTES); + if (modifiedAttrs != null) { + final Optional> attrsAfter = modifiedAttrs.getDataAfter(); + final YangInstanceIdentifier effAttrsPath = effectiveTablePath.node(ATTRIBUTES); + if (attrsAfter.isPresent()) { + tx.put(LogicalDatastoreType.OPERATIONAL, effAttrsPath, attrsAfter.get()); + } else { + tx.delete(LogicalDatastoreType.OPERATIONAL, effAttrsPath); + } + } + + final DataTreeCandidateNode modifiedRoutes = table.getModifiedChild(ROUTES); + if (modifiedRoutes != null) { + final RIBSupport ribSupport = ribContext.getRibSupport(); + switch (modifiedRoutes.getModificationType()) { + case APPEARED: + case WRITE: + deleteRoutesBefore(tx, ribSupport, effectiveTablePath, modifiedRoutes); + // XXX: YANG Tools seems to have an issue stacking DELETE with child WRITE + tx.put(LogicalDatastoreType.OPERATIONAL, effectiveTablePath.node(ROUTES), EMPTY_ROUTES); + writeRoutesAfter(tx, ribSupport, effectiveTablePath, modifiedRoutes.getDataAfter()); + break; + case DELETE: + case DISAPPEARED: + deleteRoutesBefore(tx, ribSupport, effectiveTablePath, modifiedRoutes); + tx.delete(LogicalDatastoreType.OPERATIONAL, effectiveTablePath.node(ROUTES)); + break; + case SUBTREE_MODIFIED: + for (DataTreeCandidateNode modifiedRoute : ribSupport.changedRoutes(modifiedRoutes)) { + processRoute(tx, ribSupport, effectiveTablePath, modifiedRoute); + } + break; + case UNMODIFIED: + // No-op + return; + default: + LOG.warn("Ignoring modified routes {}", modifiedRoutes); + break; + } + } } private void writeTable(final DOMDataWriteTransaction tx, final RIBSupportContext ribContext, @@ -327,16 +370,8 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn tx.put(LogicalDatastoreType.OPERATIONAL, effectiveTablePath.node(ATTRIBUTES), attrsAfter); } - final RIBSupport ribSupport = ribContext.getRibSupport(); - final Optional> maybeRoutesAfter = findRoutesMap(ribSupport, NormalizedNodes.findNode( - NormalizedNodes.findNode(tableAfter, ROUTES))); - if (maybeRoutesAfter.isPresent()) { - final YangInstanceIdentifier routesPath = routeMapPath(ribSupport, effectiveTablePath); - for (final MapEntryNode routeAfter : extractMap(maybeRoutesAfter).getValue()) { - writeRoute(tx, ribSupport, routesPath.node(routeAfter.getIdentifier()), Optional.empty(), - routeAfter); - } - } + writeRoutesAfter(tx, ribContext.getRibSupport(), effectiveTablePath, + NormalizedNodes.findNode(tableAfter, ROUTES)); } } @@ -347,94 +382,43 @@ final class EffectiveRibInWriter implements PrefixesReceivedCounters, PrefixesIn final Optional> maybeRoutesBefore = findRoutesMap(ribSupport, NormalizedNodes.findNode(tableBefore, ROUTES)); if (maybeRoutesBefore.isPresent()) { - final Collection deletedRoutes = extractMap(maybeRoutesBefore).getValue(); - if (ribSupport.getSafi() == RouteTargetConstrainSubsequentAddressFamily.class) { - final YangInstanceIdentifier routesPath = routeMapPath(ribSupport, effectiveTablePath); - for (final MapEntryNode routeBefore : deletedRoutes) { - deleteRouteTarget(ribSupport, routesPath.node(routeBefore.getIdentifier()), routeBefore); - } - this.rtMembershipsUpdated = true; - } - - final TablesKey tablesKey = ribSupport.getTablesKey(); - CountersUtil.add(prefixesInstalled.get(tablesKey), tablesKey, -deletedRoutes.size()); + onRoutesDeleted(ribSupport, effectiveTablePath, extractMap(maybeRoutesBefore).getValue()); } } - private void processTableChildren(final DOMDataWriteTransaction tx, final RIBSupport ribSupport, - final YangInstanceIdentifier effectiveTablePath, final Collection children) { - for (final DataTreeCandidateNode child : children) { - final PathArgument childIdentifier = child.getIdentifier(); - final ModificationType childModType = child.getModificationType(); - final Optional> childDataAfter = child.getDataAfter(); - LOG.debug("Process table {} type {}, dataAfter {}, dataBefore {}", childIdentifier, childModType, - childDataAfter, child.getDataBefore()); - final YangInstanceIdentifier routesPath = effectiveTablePath.node(childIdentifier); - switch (childModType) { - case DELETE: - case DISAPPEARED: - processTableChildrenDelete(child, childIdentifier, tx, ribSupport, routesPath); - break; - case UNMODIFIED: - // No-op - break; - case SUBTREE_MODIFIED: - processModifiedRouteTables(child, childIdentifier, tx, ribSupport, routesPath, childDataAfter); - break; - case APPEARED: - case WRITE: - writeRouteTables(child, childIdentifier, tx, ribSupport, routesPath, childDataAfter); - break; - default: - LOG.warn("Ignoring unhandled child {}", child); - break; - } + private void deleteRoutesBefore(final DOMDataWriteTransaction tx, final RIBSupport ribSupport, + final YangInstanceIdentifier effectiveTablePath, final DataTreeCandidateNode modifiedRoutes) { + final Optional> maybeRoutesBefore = NormalizedNodes.findNode( + modifiedRoutes.getDataBefore(), ribSupport.relativeRoutesPath()); + if (maybeRoutesBefore.isPresent()) { + onRoutesDeleted(ribSupport, effectiveTablePath, extractMap(maybeRoutesBefore).getValue()); } } - private void processTableChildrenDelete(final DataTreeCandidateNode child, final PathArgument childIdentifier, - final DOMDataWriteTransaction tx, final RIBSupport ribSupport, - final YangInstanceIdentifier routesPath) { - if (ROUTES.equals(childIdentifier)) { - for (final DataTreeCandidateNode route : ribSupport.changedRoutes(child)) { - final Optional> routeBefore = route.getDataBefore(); - if (routeBefore.isPresent()) { - final YangInstanceIdentifier routePath = ribSupport.routePath(routesPath, route.getIdentifier()); - handleRouteTarget(ModificationType.DELETE, ribSupport, routePath, routeBefore.get()); - final TablesKey tablesKey = ribSupport.getTablesKey(); - CountersUtil.decrement(this.prefixesInstalled.get(tablesKey), tablesKey); - } + private void writeRoutesAfter(final DOMDataWriteTransaction tx, final RIBSupport ribSupport, + final YangInstanceIdentifier effectiveTablePath, final Optional> routesAfter) { + final Optional> maybeRoutesAfter = NormalizedNodes.findNode(routesAfter, + ribSupport.relativeRoutesPath()); + if (maybeRoutesAfter.isPresent()) { + final YangInstanceIdentifier routesPath = routeMapPath(ribSupport, effectiveTablePath); + for (MapEntryNode routeAfter : extractMap(maybeRoutesAfter).getValue()) { + writeRoute(tx, ribSupport, routesPath.node(routeAfter.getIdentifier()), Optional.empty(), routeAfter); } } - tx.delete(LogicalDatastoreType.OPERATIONAL, routesPath); } - private void processModifiedRouteTables(final DataTreeCandidateNode child, final PathArgument childIdentifier, - final DOMDataWriteTransaction tx, final RIBSupport ribSupport, - final YangInstanceIdentifier routesPath, final Optional> childDataAfter) { - if (ROUTES.equals(childIdentifier)) { - for (final DataTreeCandidateNode route : ribSupport.changedRoutes(child)) { - processRoute(tx, ribSupport, routesPath.getParent(), route); + private void onRoutesDeleted(final RIBSupport ribSupport, + final YangInstanceIdentifier effectiveTablePath, final Collection deletedRoutes) { + if (ribSupport.getSafi() == RouteTargetConstrainSubsequentAddressFamily.class) { + final YangInstanceIdentifier routesPath = routeMapPath(ribSupport, effectiveTablePath); + for (final MapEntryNode routeBefore : deletedRoutes) { + deleteRouteTarget(ribSupport, routesPath.node(routeBefore.getIdentifier()), routeBefore); } - } else { - tx.put(LogicalDatastoreType.OPERATIONAL, routesPath, childDataAfter.get()); + this.rtMembershipsUpdated = true; } - } - private void writeRouteTables(final DataTreeCandidateNode child, final PathArgument childIdentifier, - final DOMDataWriteTransaction tx, final RIBSupport ribSupport, - final YangInstanceIdentifier routesPath, final Optional> childDataAfter) { - if (ROUTES.equals(childIdentifier)) { - final Collection changedRoutes = ribSupport.changedRoutes(child); - if (!changedRoutes.isEmpty()) { - tx.put(LogicalDatastoreType.OPERATIONAL, routesPath, childDataAfter.get()); - // Routes are special, as they may end up being filtered. The previous put conveniently - // ensured that we have them in at target, so a subsequent delete will not fail :) - for (final DataTreeCandidateNode route : changedRoutes) { - processRoute(tx, ribSupport, routesPath.getParent(), route); - } - } - } + final TablesKey tablesKey = ribSupport.getTablesKey(); + CountersUtil.add(prefixesInstalled.get(tablesKey), tablesKey, -deletedRoutes.size()); } private void processRoute(final DOMDataWriteTransaction tx, final RIBSupport ribSupport, -- 2.36.6