From 521874598e3bea3866471f528416dde70e901183 Mon Sep 17 00:00:00 2001 From: Michal Cmarada Date: Tue, 12 Sep 2017 10:09:16 +0200 Subject: [PATCH] Optimize DHCP relay processing for VPP Change-Id: Ibec2b8fec3aea330885d7cc915e7536e256512a8 Signed-off-by: Michal Cmarada --- .../config/vpp_provider/impl/VppRenderer.java | 2 +- .../vpp/commands/DhcpRelayCommand.java | 50 +++++++++++++ .../renderer/vpp/dhcp/DhcpRelayHandler.java | 72 +++++++++---------- .../vpp/policy/ForwardingManager.java | 49 ++++++++----- .../vpp/policy/VppRendererPolicyManager.java | 21 +++--- .../vpp/util/GbpNetconfTransaction.java | 16 +++++ .../vpp/dhcp/DhcpRelayHandlerTest.java | 21 ++++-- .../policy/VppRendererPolicyManagerTest.java | 2 +- 8 files changed, 163 insertions(+), 70 deletions(-) diff --git a/renderers/vpp/src/main/java/org/opendaylight/controller/config/yang/config/vpp_provider/impl/VppRenderer.java b/renderers/vpp/src/main/java/org/opendaylight/controller/config/yang/config/vpp_provider/impl/VppRenderer.java index 5e4e99596..31a461d99 100644 --- a/renderers/vpp/src/main/java/org/opendaylight/controller/config/yang/config/vpp_provider/impl/VppRenderer.java +++ b/renderers/vpp/src/main/java/org/opendaylight/controller/config/yang/config/vpp_provider/impl/VppRenderer.java @@ -158,7 +158,7 @@ public class VppRenderer implements AutoCloseable, BindingAwareProvider { dtoEventBus.register(interfaceManager); dtoEventBus.register(subnetEventManager); RoutingManager routingManager = new RoutingManager(dataBroker, mountDataProvider); - DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker, mountDataProvider); + DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker); bdManager = new BridgeDomainManagerImpl(dataBroker); ForwardingManager fwManager = new ForwardingManager(interfaceManager, aclManager, natManager, routingManager, bdManager, diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/commands/DhcpRelayCommand.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/commands/DhcpRelayCommand.java index 531b55628..0582402d7 100644 --- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/commands/DhcpRelayCommand.java +++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/commands/DhcpRelayCommand.java @@ -18,6 +18,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.dhcp import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.dhcp.rev170315.dhcp.attributes.relays.RelayBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.dhcp.rev170315.dhcp.attributes.relays.RelayKey; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.dhcp.rev170315.relay.attributes.Server; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,6 +34,7 @@ public class DhcpRelayCommand extends AbstractConfigCommand { private IpAddress gatewayIpAddress; private List serverIpAddresses; private Class addressType; + private NodeId vppNodeId; private DhcpRelayCommand(DhcpRelayBuilder builder) { operation = builder.getOperation(); @@ -40,6 +42,7 @@ public class DhcpRelayCommand extends AbstractConfigCommand { gatewayIpAddress = builder.getGatewayIpAddress(); serverIpAddresses = builder.getServerIpAddresses(); addressType = builder.getAddressType(); + vppNodeId = builder.getVppNodeId(); } public static DhcpRelayBuilder builder() { @@ -66,6 +69,10 @@ public class DhcpRelayCommand extends AbstractConfigCommand { return VppIidFactory.getDhcpRelayIid(getDhcpBuilder().getKey()); } + public NodeId getVppNodeId() { + return vppNodeId; + } + void put(ReadWriteTransaction rwTx) { rwTx.put(LogicalDatastoreType.CONFIGURATION, getIid(), getDhcpBuilder().build(), true); } @@ -88,6 +95,39 @@ public class DhcpRelayCommand extends AbstractConfigCommand { + ", operations=" + operation + "]"; } + /** + * Compares two DhcpRelayCommands without checking operation status. + * @param compareTo DhcpRelayCommand to compare with. + * @return true if commands match, false otherwise. + */ + @Override public boolean equals(Object compareTo) { + if (compareTo == null || !compareTo.getClass().equals(this.getClass())) { + return false; + } + + DhcpRelayCommand command = (DhcpRelayCommand) compareTo; + + if (!this.getVppNodeId().equals(command.getVppNodeId())) { + return false; + } else if (!this.getAddressType().equals(command.getAddressType())) { + return false; + } else if (!this.getGatewayIpAddress().equals(command.getGatewayIpAddress())) { + return false; + } else if (!this.getIid().equals(command.getIid())) { + return false; + } else if (!this.getRxVrfId().equals(command.getRxVrfId())) { + return false; + } else if (this.getServerIpAddresses() != null && !this.getServerIpAddresses() + .containsAll(command.getServerIpAddresses())) { + return false; + } else if (command.getServerIpAddresses() != null && !command.getServerIpAddresses() + .containsAll(this.getServerIpAddresses())) { + return false; + } + + return true; + } + public RelayBuilder getDhcpBuilder() { return new RelayBuilder() .setAddressType(addressType) @@ -104,6 +144,7 @@ public class DhcpRelayCommand extends AbstractConfigCommand { private IpAddress gatewayIpAddress; private List serverIpAddress; private Class addressType; + private NodeId VppNodeId; public General.Operations getOperation() { return operation; @@ -150,6 +191,15 @@ public class DhcpRelayCommand extends AbstractConfigCommand { return this; } + public NodeId getVppNodeId() { + return VppNodeId; + } + + public DhcpRelayBuilder setVppNodeId(NodeId vppNodeId) { + VppNodeId = vppNodeId; + return this; + } + /** * RoutingCommand build method. * diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandler.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandler.java index 9ee97fd67..92ac7d75c 100644 --- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandler.java +++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandler.java @@ -10,7 +10,9 @@ package org.opendaylight.groupbasedpolicy.renderer.vpp.dhcp; import static org.opendaylight.groupbasedpolicy.renderer.vpp.util.VppIidFactory.getVppRendererConfig; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.stream.Stream; @@ -19,7 +21,6 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.groupbasedpolicy.renderer.vpp.commands.DhcpRelayCommand; import org.opendaylight.groupbasedpolicy.renderer.vpp.util.GbpNetconfTransaction; import org.opendaylight.groupbasedpolicy.renderer.vpp.util.General; -import org.opendaylight.groupbasedpolicy.renderer.vpp.util.MountedDataBrokerProvider; import org.opendaylight.groupbasedpolicy.renderer.vpp.util.VppIidFactory; import org.opendaylight.groupbasedpolicy.util.DataStoreHelper; import org.opendaylight.groupbasedpolicy.util.NetUtils; @@ -43,16 +44,15 @@ public class DhcpRelayHandler { private static final Logger LOG = LoggerFactory.getLogger(DhcpRelayHandler.class); private final DataBroker dataBroker; - // TODO remove argument - public DhcpRelayHandler(DataBroker dataBroker, MountedDataBrokerProvider mountDataProvider) { + public DhcpRelayHandler(DataBroker dataBroker) { this.dataBroker = dataBroker; } - public void createIpv4DhcpRelay(long vni_vrfid, Subnet subnet, SetMultimap vppNodesByL2Fd) { + public List getCreatedIpv4DhcpRelays(long vni_vrfid, Subnet subnet, + SetMultimap vppNodesByL2Fd) { + List dhcpRelayCommandsCreated = new ArrayList<>(); if (subnet.getDefaultSubnetGatewayIp() == null) { - LOG.trace("Subnet GW IP is null, skipping processing DhcpRelay for vrfid: {}, subnet: {}, VPP nodes: {}", - vni_vrfid, subnet, vppNodesByL2Fd); - return; + return dhcpRelayCommandsCreated; } for (String bd : vppNodesByL2Fd.keySet()) { @@ -60,23 +60,19 @@ public class DhcpRelayHandler { for (NodeId vppNode : vppNodes) { IpAddress ipAddress = resolveDhcpIpAddress(vppNode, subnet); if (ipAddress != null) { - DhcpRelayCommand dhcpRelayCommand = - getDhcpRelayBuilder(vni_vrfid, subnet, ipAddress, General.Operations.PUT).build(); - - if (!submitDhcpRelay(dhcpRelayCommand, vppNode)) { - LOG.warn("DHCP Relay was not configured: {}", dhcpRelayCommand); - } - } else { - LOG.trace("DHCP server IP address was not found for node: {}. Skipping processing", vppNode); + dhcpRelayCommandsCreated.add( + getDhcpRelayBuilder(vni_vrfid, subnet, ipAddress, General.Operations.PUT, vppNode).build()); } } } + return dhcpRelayCommandsCreated; } private DhcpRelayCommand.DhcpRelayBuilder getDhcpRelayBuilder(long vni_vrfid, Subnet subnet, IpAddress ipAddress, - General.Operations operations) { + General.Operations operations, NodeId nodeId) { return DhcpRelayCommand.builder() + .setVppNodeId(nodeId) .setRxVrfId(vni_vrfid) .setOperation(operations) .setAddressType(Ipv4.class) @@ -115,42 +111,46 @@ public class DhcpRelayHandler { return null; } - public void deleteIpv4DhcpRelay(long vni_vrfid, Subnet subnet, SetMultimap vppNodesByL2Fd) { + public List getDeletedIpv4DhcpRelays(long vni_vrfid, Subnet subnet, + SetMultimap vppNodesByL2Fd) { if (subnet.getDefaultSubnetGatewayIp() == null) { - LOG.trace("Subnet GW IP is null, skipping processing DhcpRelay for vrfid: {}, subnet: {}, VPP nodes: {}", - vni_vrfid, subnet, vppNodesByL2Fd); - return; + return new ArrayList<>(); } + List dhcpRelayCommandsDeleted = new ArrayList<>(); + for (String bd : vppNodesByL2Fd.keySet()) { Set vppNodes = vppNodesByL2Fd.get(bd); for (NodeId vppNode : vppNodes) { IpAddress ipAddress = resolveDhcpIpAddress(vppNode, subnet); if (ipAddress != null) { - DhcpRelayCommand dhcpRelayCommand = - getDhcpRelayBuilder(vni_vrfid, subnet, ipAddress, General.Operations.DELETE).build(); - - if (!submitDhcpRelay(dhcpRelayCommand, vppNode)) { - LOG.warn("DHCP Relay was not deleted: {}", dhcpRelayCommand); - } - } else { - LOG.trace("DHCP server IP address was not found for node: {}. Skipping processing.", vppNode); + dhcpRelayCommandsDeleted.add( + getDhcpRelayBuilder(vni_vrfid, subnet, ipAddress, General.Operations.DELETE, vppNode).build()); } - } } + return dhcpRelayCommandsDeleted; } - private boolean submitDhcpRelay(DhcpRelayCommand dhcpRelayCommand, NodeId nodeIid) { - LOG.trace("Submitting DhcpRelay command: {}, nodeId: {}", dhcpRelayCommand, nodeIid); - if (dhcpRelayCommand.getOperation() == General.Operations.PUT) { - return GbpNetconfTransaction.netconfSyncedWrite(VppIidFactory.getNetconfNodeIid(nodeIid), dhcpRelayCommand, + public boolean submitDhcpRelay(DhcpRelayCommand dhcpRelayCommand) { + LOG.trace("Submitting DhcpRelay command: {}, nodeId: {}", dhcpRelayCommand, dhcpRelayCommand.getVppNodeId()); + switch (dhcpRelayCommand.getOperation()){ + case PUT: + return GbpNetconfTransaction.netconfSyncedWrite( + VppIidFactory.getNetconfNodeIid(dhcpRelayCommand.getVppNodeId()), dhcpRelayCommand, GbpNetconfTransaction.RETRY_COUNT); - } else if (dhcpRelayCommand.getOperation() == General.Operations.DELETE) { - return GbpNetconfTransaction.netconfSyncedDelete(VppIidFactory.getNetconfNodeIid(nodeIid), dhcpRelayCommand, + case DELETE: + return GbpNetconfTransaction.netconfSyncedDelete( + VppIidFactory.getNetconfNodeIid(dhcpRelayCommand.getVppNodeId()), dhcpRelayCommand, GbpNetconfTransaction.RETRY_COUNT); + case MERGE: + return GbpNetconfTransaction.netconfSyncedMerge( + VppIidFactory.getNetconfNodeIid(dhcpRelayCommand.getVppNodeId()), dhcpRelayCommand, + GbpNetconfTransaction.RETRY_COUNT); + default: + LOG.warn("Unknown operation for command, cannot submit command {}.", dhcpRelayCommand); + return false; } - return false; } } diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/ForwardingManager.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/ForwardingManager.java index 0d7b2620c..c8100e9ec 100644 --- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/ForwardingManager.java +++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/ForwardingManager.java @@ -24,6 +24,7 @@ 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.groupbasedpolicy.renderer.vpp.api.BridgeDomainManager; +import org.opendaylight.groupbasedpolicy.renderer.vpp.commands.DhcpRelayCommand; import org.opendaylight.groupbasedpolicy.renderer.vpp.commands.RoutingCommand; import org.opendaylight.groupbasedpolicy.renderer.vpp.config.ConfigUtil; import org.opendaylight.groupbasedpolicy.renderer.vpp.dhcp.DhcpRelayHandler; @@ -114,8 +115,9 @@ public final class ForwardingManager { private final DataBroker dataBroker; public ForwardingManager(@Nonnull InterfaceManager ifaceManager, @Nonnull AclManager aclManager, - @Nonnull NatManager natManager, @Nonnull RoutingManager routingManager, @Nonnull BridgeDomainManager bdManager, - @Nonnull LispStateManager lispStateManager, @Nonnull LoopbackManager loopbackManager, @Nonnull FlatOverlayManager flatOverlayManager, + @Nonnull NatManager natManager, @Nonnull RoutingManager routingManager, + @Nonnull BridgeDomainManager bdManager, @Nonnull LispStateManager lispStateManager, + @Nonnull LoopbackManager loopbackManager, @Nonnull FlatOverlayManager flatOverlayManager, @Nonnull DhcpRelayHandler dhcpRelayHandler, @Nonnull DataBroker dataBroker) { this.ifaceManager = Preconditions.checkNotNull(ifaceManager); this.bdManager = Preconditions.checkNotNull(bdManager); @@ -129,7 +131,7 @@ public final class ForwardingManager { this.dhcpRelayHandler = Preconditions.checkNotNull(dhcpRelayHandler); } - public Optional readGbpBridgeDomainConfig(String name) { + Optional readGbpBridgeDomainConfig(String name) { InstanceIdentifier bdIid = InstanceIdentifier.builder(Config.class) .child(GbpBridgeDomain.class, new GbpBridgeDomainKey(name)) .build(); @@ -139,7 +141,7 @@ public final class ForwardingManager { return optBd; } - public void createBridgeDomainOnNodes(SetMultimap vppNodesByBridgeDomain) { + void createBridgeDomainOnNodes(SetMultimap vppNodesByBridgeDomain) { for (String bd : vppNodesByBridgeDomain.keySet()) { Optional bdConfig = readGbpBridgeDomainConfig(bd); Set vppNodes = vppNodesByBridgeDomain.get(bd); @@ -192,7 +194,7 @@ public final class ForwardingManager { } } - public void removeBridgeDomainOnNodes(final SetMultimap vppNodesByBridgeDomain) { + void removeBridgeDomainOnNodes(final SetMultimap vppNodesByBridgeDomain) { for (String bd : vppNodesByBridgeDomain.keySet()) { Set vppNodes = vppNodesByBridgeDomain.get(bd); for (NodeId vppNode : vppNodes) { @@ -210,7 +212,7 @@ public final class ForwardingManager { } } - public void createForwardingForEndpoint(RendererEndpointKey rEpKey, PolicyContext policyCtx) { + void createForwardingForEndpoint(RendererEndpointKey rEpKey, PolicyContext policyCtx) { AddressEndpointWithLocation rEp = policyCtx.getAddrEpByKey().get(KeyFactory.addressEndpointKey(rEpKey)); if (ConfigUtil.getInstance().isLispOverlayEnabled()) { @@ -287,7 +289,7 @@ public final class ForwardingManager { return false; } - public void removeForwardingForEndpoint(RendererEndpointKey rEpKey, PolicyContext policyCtx) { + void removeForwardingForEndpoint(RendererEndpointKey rEpKey, PolicyContext policyCtx) { AddressEndpointWithLocation rEp = policyCtx.getAddrEpByKey().get(KeyFactory.addressEndpointKey(rEpKey)); ExternalLocationCase rEpLoc = resolveAndValidateLocation(rEp); @@ -318,7 +320,7 @@ public final class ForwardingManager { } } - public static ExternalLocationCase resolveAndValidateLocation(AddressEndpointWithLocation addrEpWithLoc) { + static ExternalLocationCase resolveAndValidateLocation(AddressEndpointWithLocation addrEpWithLoc) { if (addrEpWithLoc.getAbsoluteLocation() != null && addrEpWithLoc.getAbsoluteLocation().getLocationType() != null) { LocationType locationType = addrEpWithLoc.getAbsoluteLocation().getLocationType(); @@ -334,7 +336,7 @@ public final class ForwardingManager { return null; } - public static java.util.Optional resolveL2FloodDomain(@Nonnull AddressEndpointWithLocation ep, + static java.util.Optional resolveL2FloodDomain(@Nonnull AddressEndpointWithLocation ep, @Nonnull PolicyContext policyCtx) { NetworkContainment netCont = ep.getNetworkContainment(); if (netCont == null) { @@ -402,7 +404,7 @@ public final class ForwardingManager { } } - public void deleteNatEntries(PolicyContext policyCtx) { + void deleteNatEntries(PolicyContext policyCtx) { Configuration cfg = policyCtx.getPolicy().getConfiguration(); if(cfg != null) { List natEntries = resolveStaticNatTableEntries(cfg.getEndpoints()); @@ -421,8 +423,8 @@ public final class ForwardingManager { } } - public List> resolvePhysicalInterfacesForNat( - List rendNetDomains) { + private List> resolvePhysicalInterfacesForNat( + List rendNetDomains) { List> physIfaces = new ArrayList<>(); for (RendererNetworkDomain rendDomain : rendNetDomains) { Optional resolvedIpPrefix = resolveIpPrefix(rendDomain); @@ -507,7 +509,7 @@ public final class ForwardingManager { WAIT_FOR_BD_PROCESSING = time; } - public void syncRouting(PolicyContext policyCtx) { + void syncRouting(PolicyContext policyCtx) { Configuration cfg = policyCtx.getPolicy().getConfiguration(); if (cfg != null && cfg.getRendererForwarding() != null) { for (RendererForwardingByTenant fwd : cfg.getRendererForwarding().getRendererForwardingByTenant()) { @@ -529,7 +531,7 @@ public final class ForwardingManager { } } - public void deleteRouting(PolicyContext policyCtx) { + void deleteRouting(PolicyContext policyCtx) { Configuration cfg = policyCtx.getPolicy().getConfiguration(); if (cfg != null && cfg.getRendererForwarding() != null) { for (RendererForwardingByTenant fwd : cfg.getRendererForwarding().getRendererForwardingByTenant()) { @@ -550,7 +552,7 @@ public final class ForwardingManager { } } - public void createDhcpRelay(RendererForwarding rendererForwarding, + List createDhcpRelay(RendererForwarding rendererForwarding, SetMultimap vppNodesByL2Fd) { for (RendererForwardingByTenant forwardingByTenant : rendererForwarding.getRendererForwardingByTenant()) { long vni_vrfid = NeutronTenantToVniMapper.getInstance().getVni(forwardingByTenant.getTenantId().getValue()); @@ -560,22 +562,33 @@ public final class ForwardingManager { if (subnet.isIsTenant()) { LOG.trace("Creating Dhcp Relay from subnet: {}, vrfid: {}, vppNodesByL2Fd: {}", subnet, vni_vrfid, vppNodesByL2Fd); - dhcpRelayHandler.createIpv4DhcpRelay(vni_vrfid, subnet, vppNodesByL2Fd); + return dhcpRelayHandler.getCreatedIpv4DhcpRelays(vni_vrfid, subnet, vppNodesByL2Fd); } } } + return new ArrayList<>(); } - public void deleteDhcpRelay(RendererForwarding rendererForwarding, SetMultimap vppNodesByL2Fd) { + List deleteDhcpRelay(RendererForwarding rendererForwarding, SetMultimap vppNodesByL2Fd) { for (RendererForwardingByTenant forwardingByTenant : rendererForwarding.getRendererForwardingByTenant()) { long vni_vrfid = NeutronTenantToVniMapper.getInstance().getVni(forwardingByTenant.getTenantId().getValue()); for (RendererNetworkDomain networkDomain : forwardingByTenant.getRendererNetworkDomain()) { org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.forwarding.l2_l3.rev170511.has.subnet.Subnet subnet = networkDomain.getAugmentation((SubnetAugmentRenderer.class)).getSubnet(); if (subnet.isIsTenant()) { - dhcpRelayHandler.deleteIpv4DhcpRelay(vni_vrfid, subnet, vppNodesByL2Fd); + return dhcpRelayHandler.getDeletedIpv4DhcpRelays(vni_vrfid, subnet, vppNodesByL2Fd); } } } + return new ArrayList<>(); + } + + void syncDhcpRelay(List createdDhcpRelays, List deletedDhcpRelays) { + deletedDhcpRelays.stream().filter(deleteCommand -> !createdDhcpRelays.contains(deleteCommand)) + .forEach(dhcpRelayHandler::submitDhcpRelay); + + createdDhcpRelays.stream().filter(createCommand -> !deletedDhcpRelays.contains(createCommand)) + .forEach(dhcpRelayHandler::submitDhcpRelay); } } diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManager.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManager.java index 3fc2e4845..b380358c5 100644 --- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManager.java +++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManager.java @@ -8,8 +8,10 @@ package org.opendaylight.groupbasedpolicy.renderer.vpp.policy; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -21,6 +23,7 @@ import org.opendaylight.controller.config.yang.config.vpp_provider.impl.VppRende import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.groupbasedpolicy.renderer.vpp.commands.DhcpRelayCommand; import org.opendaylight.groupbasedpolicy.renderer.vpp.config.ConfigUtil; import org.opendaylight.groupbasedpolicy.renderer.vpp.event.NodeOperEvent; import org.opendaylight.groupbasedpolicy.renderer.vpp.event.RendererPolicyConfEvent; @@ -158,15 +161,15 @@ public class VppRendererPolicyManager { LOG.debug("Creating bridge domains on nodes {}", createdVppNodesByL2Fd); fwManager.createBridgeDomainOnNodes(createdVppNodesByL2Fd); } else { + List deletedDhcpRelays = new ArrayList<>(); + List createdDhcpRelays = new ArrayList<>(); if (rPolicyBefore.getConfiguration() != null) { RendererForwarding rendererForwardingBefore = rPolicyBefore.getConfiguration().getRendererForwarding(); SetMultimap vppNodesByL2FdBefore = resolveVppNodesByL2Fd(policyCtxBefore.getPolicyTable().rowKeySet(), policyCtxBefore); if (!vppNodesByL2FdBefore.isEmpty()) { - LOG.debug("Deleting DhcpRelay for forwarding: {}, on VPP nodes: {}", rendererForwardingBefore, - vppNodesByL2FdBefore); - fwManager.deleteDhcpRelay(rendererForwardingBefore, vppNodesByL2FdBefore); + deletedDhcpRelays = fwManager.deleteDhcpRelay(rendererForwardingBefore, vppNodesByL2FdBefore); } } @@ -175,11 +178,11 @@ public class VppRendererPolicyManager { SetMultimap vppNodesByL2FdAfter = resolveVppNodesByL2Fd(policyCtxAfter.getPolicyTable().rowKeySet(), policyCtxAfter); if (!vppNodesByL2FdAfter.isEmpty()) { - LOG.debug("Creating DhcpRelay for forwarding: {}, on VPP nodes: {}", rendererForwardingAfter, - vppNodesByL2FdAfter); - fwManager.createDhcpRelay(rendererForwardingAfter, vppNodesByL2FdAfter); + createdDhcpRelays = fwManager.createDhcpRelay(rendererForwardingAfter, vppNodesByL2FdAfter); } } + + fwManager.syncDhcpRelay(createdDhcpRelays, deletedDhcpRelays); } fwManager.syncNatEntries(policyCtxAfter); @@ -250,7 +253,8 @@ public class VppRendererPolicyManager { fwManager.createBridgeDomainOnNodes(vppNodesByL2Fd); } else { RendererForwarding rendererForwarding = rPolicy.getConfiguration().getRendererForwarding(); - fwManager.createDhcpRelay(rendererForwarding, vppNodesByL2Fd); + List createdDhcpRelays = fwManager.createDhcpRelay(rendererForwarding, vppNodesByL2Fd); + fwManager.syncDhcpRelay(createdDhcpRelays, new ArrayList<>()); } rEpKeys.forEach(rEpKey -> fwManager.createForwardingForEndpoint(rEpKey, policyCtx)); @@ -270,7 +274,8 @@ public class VppRendererPolicyManager { fwManager.removeBridgeDomainOnNodes(vppNodesByL2Fd); } else { RendererForwarding rendererForwarding = rendererPolicy.getConfiguration().getRendererForwarding(); - fwManager.deleteDhcpRelay(rendererForwarding, vppNodesByL2Fd); + List deletedDhcpRelays = fwManager.deleteDhcpRelay(rendererForwarding, vppNodesByL2Fd); + fwManager.syncDhcpRelay(new ArrayList<>(), deletedDhcpRelays); } fwManager.deleteNatEntries(policyCtx); fwManager.deleteRouting(policyCtx); diff --git a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/util/GbpNetconfTransaction.java b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/util/GbpNetconfTransaction.java index c6c461833..178648286 100644 --- a/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/util/GbpNetconfTransaction.java +++ b/renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/util/GbpNetconfTransaction.java @@ -85,6 +85,22 @@ public class GbpNetconfTransaction { return result; } + /*** + * Netconf wrapper for merge operation on a Netconf Device + * @param vppIid destination node + * @param command config command that needs to be executed + * @param retryCounter retry counter, will repeat the operation for specified amount of times if transaction fails + * @return true if transaction is successful, false otherwise + */ + public static boolean netconfSyncedMerge(@Nonnull final InstanceIdentifier vppIid, + @Nonnull final ConfigCommand command, byte retryCounter) { + VbdNetconfTransaction.NODE_DATA_BROKER_MAP.get(vppIid).getValue().lock(); + boolean result = + write(VbdNetconfTransaction.NODE_DATA_BROKER_MAP.get(vppIid).getKey(), command, retryCounter); + VbdNetconfTransaction.NODE_DATA_BROKER_MAP.get(vppIid).getValue().unlock(); + return result; + } + /*** * Netconf wrapper method for synced requests for write operation on a Netconf Device * @param vppIid destination node diff --git a/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandlerTest.java b/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandlerTest.java index 0679d46e9..5f277d10a 100644 --- a/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandlerTest.java +++ b/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/dhcp/DhcpRelayHandlerTest.java @@ -112,9 +112,12 @@ public class DhcpRelayHandlerTest extends VppRendererDataBrokerTest { @Test public void createIpv4DhcpRelayTest() throws ExecutionException, InterruptedException { - DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker, mountedDataProviderMock); + DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker); + + List createdIpv4DhcpRelays = + dhcpRelayHandler.getCreatedIpv4DhcpRelays(RX_VRF_ID, subnet, vppNodesByL2Fd); + createdIpv4DhcpRelays.forEach(dhcpRelayHandler::submitDhcpRelay); - dhcpRelayHandler.createIpv4DhcpRelay(RX_VRF_ID, subnet, vppNodesByL2Fd); Optional relayOptional = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION, VppIidFactory.getDhcpRelayIid(DHCP_RELAY_COMMAND.getDhcpBuilder().getKey()), dataBroker.newReadOnlyTransaction()); @@ -124,11 +127,14 @@ public class DhcpRelayHandlerTest extends VppRendererDataBrokerTest { @Test public void createIpv4DhcpRelayTestWithNullServerIp() throws ExecutionException, InterruptedException { - DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker, mountedDataProviderMock); + DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker); Subnet subnet1 = new SubnetBuilder(subnet).setDefaultSubnetGatewayIp(null).build(); - dhcpRelayHandler.createIpv4DhcpRelay(RX_VRF_ID, subnet1, vppNodesByL2Fd); + List createdIpv4DhcpRelays = + dhcpRelayHandler.getCreatedIpv4DhcpRelays(RX_VRF_ID, subnet1, vppNodesByL2Fd); + createdIpv4DhcpRelays.forEach(dhcpRelayHandler::submitDhcpRelay); + Optional relayOptional = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION, VppIidFactory.getDhcpRelayIid(DHCP_RELAY_COMMAND.getDhcpBuilder().getKey()), dataBroker.newReadOnlyTransaction()); @@ -137,7 +143,7 @@ public class DhcpRelayHandlerTest extends VppRendererDataBrokerTest { @Test public void deleteIpv4DhcpRelayTest() throws ExecutionException, InterruptedException { - DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker, mountedDataProviderMock); + DhcpRelayHandler dhcpRelayHandler = new DhcpRelayHandler(dataBroker); writeBasicRelay(); Optional relayOptional = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION, @@ -146,7 +152,10 @@ public class DhcpRelayHandlerTest extends VppRendererDataBrokerTest { Assert.assertTrue(relayOptional.isPresent()); Assert.assertEquals(RELAY, relayOptional.get()); - dhcpRelayHandler.deleteIpv4DhcpRelay(RX_VRF_ID, subnet, vppNodesByL2Fd); + List deletedIpv4DhcpRelays = + dhcpRelayHandler.getDeletedIpv4DhcpRelays(RX_VRF_ID, subnet, vppNodesByL2Fd); + deletedIpv4DhcpRelays.forEach(dhcpRelayHandler::submitDhcpRelay); + relayOptional = DataStoreHelper.readFromDs(LogicalDatastoreType.CONFIGURATION, VppIidFactory.getDhcpRelayIid(DHCP_RELAY_COMMAND.getDhcpBuilder().getKey()), dataBroker.newReadOnlyTransaction()); diff --git a/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManagerTest.java b/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManagerTest.java index 1e70ecbe0..79770adb8 100644 --- a/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManagerTest.java +++ b/renderers/vpp/src/test/java/org/opendaylight/groupbasedpolicy/renderer/vpp/policy/VppRendererPolicyManagerTest.java @@ -158,7 +158,7 @@ public class VppRendererPolicyManagerTest extends CustomDataBrokerTest { natManager = new NatManager(dataBroker, mountedDataProviderMock); routingManager = new RoutingManager(dataBroker, mountedDataProviderMock); bdManager = new BridgeDomainManagerImpl(mountPointDataBroker); - dhcpRelayHandler = new DhcpRelayHandler(dataBroker, mountedDataProviderMock); + dhcpRelayHandler = new DhcpRelayHandler(dataBroker); fwManager = new ForwardingManager(ifaceManager, aclManager, natManager, routingManager, bdManager, lispStateManager, loopbackManager, flatOverlayManager, dhcpRelayHandler, dataBroker); vppRendererPolicyManager = new VppRendererPolicyManager(fwManager, aclManager, dataBroker); -- 2.36.6