From 6a2d95e329324ddae3e747b9f872d35e66bb271f Mon Sep 17 00:00:00 2001 From: Daniel Park Date: Mon, 5 Nov 2018 18:50:16 +0900 Subject: [PATCH] Fix to add icmp reply flow rule in default. Change-Id: Ia32073715834b6e9c0fe7ad73076980bff3bdc92 --- .../impl/OpenstackRoutingHandler.java | 29 -------- .../impl/OpenstackRoutingIcmpHandler.java | 72 +++++++++++++++++++ .../impl/OpenstackSwitchingArpHandler.java | 3 + .../impl/OpenstackRoutingIcmpHandlerTest.java | 24 +++++++ 4 files changed, 99 insertions(+), 29 deletions(-) diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java index 89b50dfda5..7f1ecd6300 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java @@ -26,7 +26,6 @@ import org.apache.felix.scr.annotations.Property; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; import org.onlab.packet.Ethernet; -import org.onlab.packet.ICMP; import org.onlab.packet.IPv4; import org.onlab.packet.IpAddress; import org.onlab.packet.IpPrefix; @@ -880,34 +879,6 @@ public class OpenstackRoutingHandler { PRIORITY_EXTERNAL_ROUTING_RULE, GW_COMMON_TABLE, install); - - // TODO: we do not remove the IcmpReplyMatchRules with false installation flag - // need to find a better way to remove this rule - if (install) { - setIcmpReplyRules(deviceId, install); - } - } - - private void setIcmpReplyRules(DeviceId deviceId, boolean install) { - // Sends ICMP response to controller for SNATing ingress traffic - TrafficSelector selector = DefaultTrafficSelector.builder() - .matchEthType(Ethernet.TYPE_IPV4) - .matchIPProtocol(IPv4.PROTOCOL_ICMP) - .matchIcmpType(ICMP.TYPE_ECHO_REPLY) - .build(); - - TrafficTreatment treatment = DefaultTrafficTreatment.builder() - .punt() - .build(); - - osFlowRuleService.setRule( - appId, - deviceId, - selector, - treatment, - PRIORITY_INTERNAL_ROUTING_RULE, - GW_COMMON_TABLE, - install); } private void setRouterAdminRules(String segmentId, NetworkType networkType, boolean install) { diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java index 91970823ba..4417873319 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java @@ -29,10 +29,15 @@ import org.onlab.packet.IpAddress; import org.onlab.packet.MacAddress; import org.onlab.packet.VlanId; import org.onlab.util.KryoNamespace; +import org.onosproject.cluster.ClusterService; +import org.onosproject.cluster.LeadershipService; +import org.onosproject.cluster.NodeId; import org.onosproject.core.ApplicationId; import org.onosproject.core.CoreService; import org.onosproject.net.DeviceId; +import org.onosproject.net.flow.DefaultTrafficSelector; import org.onosproject.net.flow.DefaultTrafficTreatment; +import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.TrafficTreatment; import org.onosproject.net.packet.DefaultOutboundPacket; import org.onosproject.net.packet.InboundPacket; @@ -44,9 +49,12 @@ import org.onosproject.openstacknetworking.api.Constants; import org.onosproject.openstacknetworking.api.ExternalPeerRouter; import org.onosproject.openstacknetworking.api.InstancePort; import org.onosproject.openstacknetworking.api.InstancePortService; +import org.onosproject.openstacknetworking.api.OpenstackFlowRuleService; import org.onosproject.openstacknetworking.api.OpenstackNetworkService; import org.onosproject.openstacknetworking.api.OpenstackRouterService; import org.onosproject.openstacknode.api.OpenstackNode; +import org.onosproject.openstacknode.api.OpenstackNodeEvent; +import org.onosproject.openstacknode.api.OpenstackNodeListener; import org.onosproject.openstacknode.api.OpenstackNodeService; import org.onosproject.store.serializers.KryoNamespaces; import org.onosproject.store.service.ConsistentMap; @@ -74,7 +82,9 @@ import static org.onlab.packet.ICMP.TYPE_ECHO_REPLY; import static org.onlab.packet.ICMP.TYPE_ECHO_REQUEST; import static org.onlab.util.Tools.groupedThreads; import static org.onosproject.openstacknetworking.api.Constants.DEFAULT_GATEWAY_MAC; +import static org.onosproject.openstacknetworking.api.Constants.GW_COMMON_TABLE; import static org.onosproject.openstacknetworking.api.Constants.OPENSTACK_NETWORKING_APP_ID; +import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_INTERNAL_ROUTING_RULE; import static org.onosproject.openstacknode.api.OpenstackNode.NodeType.GATEWAY; import static org.slf4j.LoggerFactory.getLogger; @@ -117,10 +127,20 @@ public class OpenstackRoutingIcmpHandler { @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) protected OpenstackRouterService osRouterService; + @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) + protected LeadershipService leadershipService; + + @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) + protected OpenstackFlowRuleService osFlowRuleService; + + @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) + protected ClusterService clusterService; + private final ExecutorService eventExecutor = newSingleThreadExecutor( groupedThreads(this.getClass().getSimpleName(), "event-handler", log)); private final InternalPacketProcessor packetProcessor = new InternalPacketProcessor(); private ConsistentMap icmpInfoMap; + private final OpenstackNodeListener osNodeListener = new InternalNodeEventListener(); private static final KryoNamespace SERIALIZER_ICMP_MAP = KryoNamespace.newBuilder() .register(KryoNamespaces.API) @@ -130,11 +150,15 @@ public class OpenstackRoutingIcmpHandler { .build(); private ApplicationId appId; + private NodeId localNodeId; @Activate protected void activate() { appId = coreService.registerApplication(OPENSTACK_NETWORKING_APP_ID); packetService.addProcessor(packetProcessor, PacketProcessor.director(1)); + localNodeId = clusterService.getLocalNode().id(); + leadershipService.runForLeadership(appId.name()); + osNodeService.addListener(osNodeListener); icmpInfoMap = storageService.consistentMapBuilder() .withSerializer(Serializer.using(SERIALIZER_ICMP_MAP)) @@ -149,6 +173,8 @@ public class OpenstackRoutingIcmpHandler { protected void deactivate() { packetService.removeProcessor(packetProcessor); eventExecutor.shutdown(); + leadershipService.withdraw(appId.name()); + osNodeService.removeListener(osNodeListener); log.info("Stopped"); } @@ -497,4 +523,50 @@ public class OpenstackRoutingIcmpHandler { } } } + + private class InternalNodeEventListener implements OpenstackNodeListener { + @Override + public boolean isRelevant(OpenstackNodeEvent event) { + // do not allow to proceed without leadership + NodeId leader = leadershipService.getLeader(appId.name()); + return Objects.equals(localNodeId, leader) && event.subject().type() == GATEWAY; + } + + @Override + public void event(OpenstackNodeEvent event) { + OpenstackNode osNode = event.subject(); + switch (event.type()) { + case OPENSTACK_NODE_COMPLETE: + eventExecutor.execute(() -> setIcmpReplyRules(osNode.intgBridge(), true)); + break; + case OPENSTACK_NODE_INCOMPLETE: + eventExecutor.execute(() -> setIcmpReplyRules(osNode.intgBridge(), false)); + break; + default: + break; + } + } + + private void setIcmpReplyRules(DeviceId deviceId, boolean install) { + // Sends ICMP response to controller for SNATing ingress traffic + TrafficSelector selector = DefaultTrafficSelector.builder() + .matchEthType(Ethernet.TYPE_IPV4) + .matchIPProtocol(IPv4.PROTOCOL_ICMP) + .matchIcmpType(ICMP.TYPE_ECHO_REPLY) + .build(); + + TrafficTreatment treatment = DefaultTrafficTreatment.builder() + .punt() + .build(); + + osFlowRuleService.setRule( + appId, + deviceId, + selector, + treatment, + PRIORITY_INTERNAL_ROUTING_RULE, + GW_COMMON_TABLE, + install); + } + } } diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java index 8cbec2a04f..95661a3fe7 100644 --- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java +++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingArpHandler.java @@ -302,6 +302,9 @@ public final class OpenstackSwitchingArpHandler { } String gateway = osSubnet.getGateway(); + if (gateway == null) { + return; + } TrafficSelector selector = DefaultTrafficSelector.builder() .matchEthType(EthType.EtherType.ARP.ethType().toShort()) diff --git a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandlerTest.java b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandlerTest.java index d5227c3b7f..b7c4e7bc7f 100644 --- a/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandlerTest.java +++ b/apps/openstacknetworking/app/src/test/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandlerTest.java @@ -30,6 +30,8 @@ import org.onlab.packet.IPv4; import org.onlab.packet.IpAddress; import org.onlab.packet.MacAddress; import org.onlab.packet.VlanId; +import org.onosproject.cluster.ClusterServiceAdapter; +import org.onosproject.cluster.LeadershipServiceAdapter; import org.onosproject.core.ApplicationId; import org.onosproject.core.CoreServiceAdapter; import org.onosproject.core.DefaultApplicationId; @@ -117,6 +119,10 @@ public class OpenstackRoutingIcmpHandlerTest { icmpHandler.instancePortService = new TestInstancePortService(); icmpHandler.osNetworkService = new TestOpenstackNetworkService(); icmpHandler.osRouterService = new TestOpenstackRouterService(); + icmpHandler.leadershipService = new TestLeadershipService(); + icmpHandler.osFlowRuleService = new TestOpenstackFlowRuleService(); + icmpHandler.clusterService = new TestClusterService(); + TestUtils.setField(icmpHandler, "eventExecutor", MoreExecutors.newDirectExecutorService()); icmpHandler.activate(); @@ -511,4 +517,22 @@ public class OpenstackRoutingIcmpHandlerTest { return PortNumber.portNumber(1); } } + + /** + * Mocks the LeadershipService. + */ + private class TestLeadershipService extends LeadershipServiceAdapter { + } + + /** + * Mocks the OpenstackFlowRuleService. + */ + private class TestOpenstackFlowRuleService extends OpenstackFlowRuleServiceAdapter { + } + + /** + * Mocks the ClusterService. + */ + private class TestClusterService extends ClusterServiceAdapter { + } }