diff --git a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java index 5b661175de..aa3a24fb62 100644 --- a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java +++ b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java @@ -105,17 +105,17 @@ public class ReactiveForwarding { @Property(name = "packetOutOfppTable", boolValue = false, label = "Enable first packet forwarding using OFPP_TABLE port " + - "instead of PacketOut with actual port; default is false") + "instead of PacketOut with actual port; default is false") private boolean packetOutOfppTable = false; @Property(name = "flowTimeout", intValue = DEFAULT_TIMEOUT, label = "Configure Flow Timeout for installed flow rules; " + - "default is 10 sec") + "default is 10 sec") private int flowTimeout = DEFAULT_TIMEOUT; @Property(name = "flowPriority", intValue = DEFAULT_PRIORITY, label = "Configure Flow Priority for installed flow rules; " + - "default is 10") + "default is 10") private int flowPriority = DEFAULT_PRIORITY; @Property(name = "ipv6Forwarding", boolValue = false, @@ -152,7 +152,7 @@ public class ReactiveForwarding { @Property(name = "matchIcmpFields", boolValue = false, label = "Enable matching ICMPv4 and ICMPv6 fields; " + - "default is false") + "default is false") private boolean matchIcmpFields = false; @@ -163,15 +163,15 @@ public class ReactiveForwarding { packetService.addProcessor(processor, PacketProcessor.ADVISOR_MAX + 2); readComponentConfiguration(context); - requestPackests(); + requestIntercepts(); log.info("Started with Application ID {}", appId.id()); } @Deactivate public void deactivate() { - // TODO revoke all packet requests when deactivate cfgService.unregisterProperties(getClass(), false); + withdrawIntercepts(); flowRuleService.removeFlowRulesById(appId); packetService.removeProcessor(processor); processor = null; @@ -180,30 +180,41 @@ public class ReactiveForwarding { @Modified public void modified(ComponentContext context) { - // TODO revoke unnecessary packet requests when config being modified readComponentConfiguration(context); - requestPackests(); + requestIntercepts(); } /** * Request packet in via PacketService. */ - private void requestPackests() { + private void requestIntercepts() { TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); selector.matchEthType(Ethernet.TYPE_IPV4); - packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, - appId); + packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, appId); selector.matchEthType(Ethernet.TYPE_ARP); - packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, - appId); + packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, appId); + selector.matchEthType(Ethernet.TYPE_IPV6); if (ipv6Forwarding) { - selector.matchEthType(Ethernet.TYPE_IPV6); - packetService.requestPackets(selector.build(), - PacketPriority.REACTIVE, appId); + packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, appId); + } else { + packetService.cancelPackets(selector.build(), PacketPriority.REACTIVE, appId); } } + /** + * Request packet in via PacketService. + */ + private void withdrawIntercepts() { + TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); + selector.matchEthType(Ethernet.TYPE_IPV4); + packetService.cancelPackets(selector.build(), PacketPriority.REACTIVE, appId); + selector.matchEthType(Ethernet.TYPE_ARP); + packetService.cancelPackets(selector.build(), PacketPriority.REACTIVE, appId); + selector.matchEthType(Ethernet.TYPE_IPV6); + packetService.cancelPackets(selector.build(), PacketPriority.REACTIVE, appId); + } + /** * Extracts properties from the component configuration context. * @@ -212,84 +223,84 @@ public class ReactiveForwarding { private void readComponentConfiguration(ComponentContext context) { Dictionary properties = context.getProperties(); boolean packetOutOnlyEnabled = - isPropertyEnabled(properties, "packetOutOnly"); + isPropertyEnabled(properties, "packetOutOnly"); if (packetOutOnly != packetOutOnlyEnabled) { packetOutOnly = packetOutOnlyEnabled; log.info("Configured. Packet-out only forwarding is {}", - packetOutOnly ? "enabled" : "disabled"); + packetOutOnly ? "enabled" : "disabled"); } boolean packetOutOfppTableEnabled = - isPropertyEnabled(properties, "packetOutOfppTable"); + isPropertyEnabled(properties, "packetOutOfppTable"); if (packetOutOfppTable != packetOutOfppTableEnabled) { packetOutOfppTable = packetOutOfppTableEnabled; log.info("Configured. Forwarding using OFPP_TABLE port is {}", - packetOutOfppTable ? "enabled" : "disabled"); + packetOutOfppTable ? "enabled" : "disabled"); } boolean ipv6ForwardingEnabled = - isPropertyEnabled(properties, "ipv6Forwarding"); + isPropertyEnabled(properties, "ipv6Forwarding"); if (ipv6Forwarding != ipv6ForwardingEnabled) { ipv6Forwarding = ipv6ForwardingEnabled; log.info("Configured. IPv6 forwarding is {}", - ipv6Forwarding ? "enabled" : "disabled"); + ipv6Forwarding ? "enabled" : "disabled"); } boolean matchDstMacOnlyEnabled = - isPropertyEnabled(properties, "matchDstMacOnly"); + isPropertyEnabled(properties, "matchDstMacOnly"); if (matchDstMacOnly != matchDstMacOnlyEnabled) { matchDstMacOnly = matchDstMacOnlyEnabled; log.info("Configured. Match Dst MAC Only is {}", - matchDstMacOnly ? "enabled" : "disabled"); + matchDstMacOnly ? "enabled" : "disabled"); } boolean matchVlanIdEnabled = - isPropertyEnabled(properties, "matchVlanId"); + isPropertyEnabled(properties, "matchVlanId"); if (matchVlanId != matchVlanIdEnabled) { matchVlanId = matchVlanIdEnabled; log.info("Configured. Matching Vlan ID is {}", - matchVlanId ? "enabled" : "disabled"); + matchVlanId ? "enabled" : "disabled"); } boolean matchIpv4AddressEnabled = - isPropertyEnabled(properties, "matchIpv4Address"); + isPropertyEnabled(properties, "matchIpv4Address"); if (matchIpv4Address != matchIpv4AddressEnabled) { matchIpv4Address = matchIpv4AddressEnabled; log.info("Configured. Matching IPv4 Addresses is {}", - matchIpv4Address ? "enabled" : "disabled"); + matchIpv4Address ? "enabled" : "disabled"); } boolean matchIpv4DscpEnabled = - isPropertyEnabled(properties, "matchIpv4Dscp"); + isPropertyEnabled(properties, "matchIpv4Dscp"); if (matchIpv4Dscp != matchIpv4DscpEnabled) { matchIpv4Dscp = matchIpv4DscpEnabled; log.info("Configured. Matching IPv4 DSCP and ECN is {}", - matchIpv4Dscp ? "enabled" : "disabled"); + matchIpv4Dscp ? "enabled" : "disabled"); } boolean matchIpv6AddressEnabled = - isPropertyEnabled(properties, "matchIpv6Address"); + isPropertyEnabled(properties, "matchIpv6Address"); if (matchIpv6Address != matchIpv6AddressEnabled) { matchIpv6Address = matchIpv6AddressEnabled; log.info("Configured. Matching IPv6 Addresses is {}", - matchIpv6Address ? "enabled" : "disabled"); + matchIpv6Address ? "enabled" : "disabled"); } boolean matchIpv6FlowLabelEnabled = - isPropertyEnabled(properties, "matchIpv6FlowLabel"); + isPropertyEnabled(properties, "matchIpv6FlowLabel"); if (matchIpv6FlowLabel != matchIpv6FlowLabelEnabled) { matchIpv6FlowLabel = matchIpv6FlowLabelEnabled; log.info("Configured. Matching IPv6 FlowLabel is {}", - matchIpv6FlowLabel ? "enabled" : "disabled"); + matchIpv6FlowLabel ? "enabled" : "disabled"); } boolean matchTcpUdpPortsEnabled = - isPropertyEnabled(properties, "matchTcpUdpPorts"); + isPropertyEnabled(properties, "matchTcpUdpPorts"); if (matchTcpUdpPorts != matchTcpUdpPortsEnabled) { matchTcpUdpPorts = matchTcpUdpPortsEnabled; log.info("Configured. Matching TCP/UDP fields is {}", - matchTcpUdpPorts ? "enabled" : "disabled"); + matchTcpUdpPorts ? "enabled" : "disabled"); } boolean matchIcmpFieldsEnabled = - isPropertyEnabled(properties, "matchIcmpFields"); + isPropertyEnabled(properties, "matchIcmpFields"); if (matchIcmpFields != matchIcmpFieldsEnabled) { matchIcmpFields = matchIcmpFieldsEnabled; log.info("Configured. Matching ICMP (v4 and v6) fields is {}", - matchIcmpFields ? "enabled" : "disabled"); + matchIcmpFields ? "enabled" : "disabled"); } Integer flowTimeoutConfigured = - getIntegerProperty(properties, "flowTimeout"); + getIntegerProperty(properties, "flowTimeout"); if (flowTimeoutConfigured == null) { log.info("Flow Timeout is not configured, default value is {}", flowTimeout); @@ -299,7 +310,7 @@ public class ReactiveForwarding { flowTimeout, " seconds"); } Integer flowPriorityConfigured = - getIntegerProperty(properties, "flowPriority"); + getIntegerProperty(properties, "flowPriority"); if (flowPriorityConfigured == null) { log.info("Flow Priority is not configured, default value is {}", flowPriority); @@ -314,7 +325,7 @@ public class ReactiveForwarding { * Get Integer property from the propertyName * Return null if propertyName is not found. * - * @param properties properties to be looked up + * @param properties properties to be looked up * @param propertyName the name of the property to look up * @return value when the propertyName is defined or return null */ @@ -333,7 +344,7 @@ public class ReactiveForwarding { /** * Check property name is defined and set to true. * - * @param properties properties to be looked up + * @param properties properties to be looked up * @param propertyName the name of the property to look up * @return true when the propertyName is defined and set to true */ @@ -408,9 +419,9 @@ public class ReactiveForwarding { // Otherwise, get a set of paths that lead from here to the // destination edge switch. Set paths = - topologyService.getPaths(topologyService.currentTopology(), - pkt.receivedFrom().deviceId(), - dst.location().deviceId()); + topologyService.getPaths(topologyService.currentTopology(), + pkt.receivedFrom().deviceId(), + dst.location().deviceId()); if (paths.isEmpty()) { // If there are no paths, flood and bail. flood(context); @@ -513,11 +524,11 @@ public class ReactiveForwarding { IPv4 ipv4Packet = (IPv4) inPkt.getPayload(); byte ipv4Protocol = ipv4Packet.getProtocol(); Ip4Prefix matchIp4SrcPrefix = - Ip4Prefix.valueOf(ipv4Packet.getSourceAddress(), - Ip4Prefix.MAX_MASK_LENGTH); + Ip4Prefix.valueOf(ipv4Packet.getSourceAddress(), + Ip4Prefix.MAX_MASK_LENGTH); Ip4Prefix matchIp4DstPrefix = - Ip4Prefix.valueOf(ipv4Packet.getDestinationAddress(), - Ip4Prefix.MAX_MASK_LENGTH); + Ip4Prefix.valueOf(ipv4Packet.getDestinationAddress(), + Ip4Prefix.MAX_MASK_LENGTH); selectorBuilder.matchEthType(Ethernet.TYPE_IPV4) .matchIPSrc(matchIp4SrcPrefix) .matchIPDst(matchIp4DstPrefix); @@ -556,11 +567,11 @@ public class ReactiveForwarding { IPv6 ipv6Packet = (IPv6) inPkt.getPayload(); byte ipv6NextHeader = ipv6Packet.getNextHeader(); Ip6Prefix matchIp6SrcPrefix = - Ip6Prefix.valueOf(ipv6Packet.getSourceAddress(), - Ip6Prefix.MAX_MASK_LENGTH); + Ip6Prefix.valueOf(ipv6Packet.getSourceAddress(), + Ip6Prefix.MAX_MASK_LENGTH); Ip6Prefix matchIp6DstPrefix = - Ip6Prefix.valueOf(ipv6Packet.getDestinationAddress(), - Ip6Prefix.MAX_MASK_LENGTH); + Ip6Prefix.valueOf(ipv6Packet.getDestinationAddress(), + Ip6Prefix.MAX_MASK_LENGTH); selectorBuilder.matchEthType(Ethernet.TYPE_IPV6) .matchIPv6Src(matchIp6SrcPrefix) .matchIPv6Dst(matchIp6DstPrefix); diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/ObjectiveContext.java b/core/api/src/main/java/org/onosproject/net/flowobjective/ObjectiveContext.java index 5bb2bdceae..c6db17fe69 100644 --- a/core/api/src/main/java/org/onosproject/net/flowobjective/ObjectiveContext.java +++ b/core/api/src/main/java/org/onosproject/net/flowobjective/ObjectiveContext.java @@ -20,14 +20,27 @@ import com.google.common.annotations.Beta; /** * The context of a objective that will become the subject of * the notification. - * + *

* Implementations of this class must be serializable. + *

*/ @Beta public interface ObjectiveContext { - default void onSuccess(Objective objective) {} + /** + * Invoked on successful execution of the flow objective. + * + * @param objective objective to execute + */ + default void onSuccess(Objective objective) { + } - default void onError(Objective objective, ObjectiveError error) {} + /** + * Invoked when error is encountered while executing the flow objective. + * + * @param objective objective to execute + */ + default void onError(Objective objective, ObjectiveError error) { + } } diff --git a/core/api/src/main/java/org/onosproject/net/packet/DefaultPacketRequest.java b/core/api/src/main/java/org/onosproject/net/packet/DefaultPacketRequest.java index 0efcc7f5c5..ce2eb118ee 100644 --- a/core/api/src/main/java/org/onosproject/net/packet/DefaultPacketRequest.java +++ b/core/api/src/main/java/org/onosproject/net/packet/DefaultPacketRequest.java @@ -17,9 +17,10 @@ package org.onosproject.net.packet; import com.google.common.base.MoreObjects; import org.onosproject.core.ApplicationId; -import org.onosproject.net.flow.FlowRule; import org.onosproject.net.flow.TrafficSelector; +import java.util.Objects; + /** * Default implementation of a packet request. */ @@ -27,14 +28,19 @@ public final class DefaultPacketRequest implements PacketRequest { private final TrafficSelector selector; private final PacketPriority priority; private final ApplicationId appId; - private final FlowRule.Type tableType; + /** + * Creates a new packet request. + * + * @param selector traffic selector + * @param priority intercept priority + * @param appId application id + */ public DefaultPacketRequest(TrafficSelector selector, PacketPriority priority, - ApplicationId appId, FlowRule.Type tableType) { + ApplicationId appId) { this.selector = selector; this.priority = priority; this.appId = appId; - this.tableType = tableType; } public TrafficSelector selector() { @@ -49,39 +55,23 @@ public final class DefaultPacketRequest implements PacketRequest { return appId; } - public FlowRule.Type tableType() { - return tableType; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - DefaultPacketRequest that = (DefaultPacketRequest) o; - - if (priority != that.priority) { - return false; - } - if (!selector.equals(that.selector)) { - return false; - } - if (!tableType.equals(that.tableType)) { - return false; - } - - return true; - } - @Override public int hashCode() { - int result = selector.hashCode(); - result = 31 * result + priority.hashCode(); - return result; + return Objects.hash(selector, priority, appId); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + final DefaultPacketRequest other = (DefaultPacketRequest) obj; + return Objects.equals(this.selector, other.selector) + && Objects.equals(this.priority, other.priority) + && Objects.equals(this.appId, other.appId); } @Override @@ -89,7 +79,6 @@ public final class DefaultPacketRequest implements PacketRequest { return MoreObjects.toStringHelper(this.getClass()) .add("selector", selector) .add("priority", priority) - .add("appId", appId) - .add("table-type", tableType).toString(); + .add("appId", appId).toString(); } } \ No newline at end of file diff --git a/core/api/src/main/java/org/onosproject/net/packet/PacketRequest.java b/core/api/src/main/java/org/onosproject/net/packet/PacketRequest.java index a4e45ac870..dc09219a61 100644 --- a/core/api/src/main/java/org/onosproject/net/packet/PacketRequest.java +++ b/core/api/src/main/java/org/onosproject/net/packet/PacketRequest.java @@ -16,7 +16,6 @@ package org.onosproject.net.packet; import org.onosproject.core.ApplicationId; -import org.onosproject.net.flow.FlowRule; import org.onosproject.net.flow.TrafficSelector; /** @@ -26,26 +25,23 @@ public interface PacketRequest { /** * Obtain the traffic selector. + * * @return a traffic selector */ TrafficSelector selector(); /** * Obtain the priority. + * * @return a PacketPriority */ PacketPriority priority(); /** * Obtain the application id. + * * @return an application id */ ApplicationId appId(); - /** - * Obtain the table type. - * @return a table type - */ - FlowRule.Type tableType(); - } diff --git a/core/api/src/main/java/org/onosproject/net/packet/PacketService.java b/core/api/src/main/java/org/onosproject/net/packet/PacketService.java index be5a50589b..06c416ec94 100644 --- a/core/api/src/main/java/org/onosproject/net/packet/PacketService.java +++ b/core/api/src/main/java/org/onosproject/net/packet/PacketService.java @@ -16,7 +16,6 @@ package org.onosproject.net.packet; import org.onosproject.core.ApplicationId; -import org.onosproject.net.flow.FlowRule; import org.onosproject.net.flow.TrafficSelector; /** @@ -54,28 +53,21 @@ public interface PacketService { * * @param selector the traffic selector used to match packets * @param priority the priority of the rule - * @param appId the application ID of the requester + * @param appId the application ID of the requester */ void requestPackets(TrafficSelector selector, PacketPriority priority, ApplicationId appId); /** - * Requests that packets matching the given selector are punted from the - * dataplane to the controller. Clients of the PacketService should use - * this call to hint at the tableType in the dataplane valid for the selector. + * Cancels previous packet requests for packets matching the given + * selector to be punted from the dataplane to the controller. * * @param selector the traffic selector used to match packets * @param priority the priority of the rule - * @param appId the application ID of the requester - * @param tableType the abstract table Type in the dataplane where flowrules - * should be inserted to punt the selector packets to the - * control plane + * @param appId the application ID of the requester */ - void requestPackets(TrafficSelector selector, PacketPriority priority, - ApplicationId appId, FlowRule.Type tableType); - - - // TODO add API to allow applications to revoke requests when they deactivate + void cancelPackets(TrafficSelector selector, PacketPriority priority, + ApplicationId appId); /** * Emits the specified outbound packet onto the network. diff --git a/core/api/src/main/java/org/onosproject/net/packet/PacketStore.java b/core/api/src/main/java/org/onosproject/net/packet/PacketStore.java index 450c23b400..ff45cc0c00 100644 --- a/core/api/src/main/java/org/onosproject/net/packet/PacketStore.java +++ b/core/api/src/main/java/org/onosproject/net/packet/PacketStore.java @@ -34,15 +34,21 @@ public interface PacketStore extends Store { void emit(OutboundPacket packet); /** - * Register a request for packets. If the registration - * is successful the manager can proceed, otherwise it should - * consider these packet already available in the system. + * Requests intercept of packets that match the given selector. * * @param request a packet request - * @return a boolean indicating registration state. + * @return true if the first time the given selector was requested */ boolean requestPackets(PacketRequest request); + /** + * Cancels intercept of packets that match the given selector. + * + * @param request a packet request + * @return true if there is no other application requesting the given selector + */ + boolean cancelPackets(PacketRequest request); + /** * Obtains all existing requests in the system. * diff --git a/core/api/src/test/java/org/onosproject/net/packet/PacketServiceAdapter.java b/core/api/src/test/java/org/onosproject/net/packet/PacketServiceAdapter.java new file mode 100644 index 0000000000..afe936b751 --- /dev/null +++ b/core/api/src/test/java/org/onosproject/net/packet/PacketServiceAdapter.java @@ -0,0 +1,44 @@ +/* + * Copyright 2015 Open Networking Laboratory + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.onosproject.net.packet; + +import org.onosproject.core.ApplicationId; +import org.onosproject.net.flow.TrafficSelector; + +/** + * Test adapter for packet service. + */ +public class PacketServiceAdapter implements PacketService { + @Override + public void addProcessor(PacketProcessor processor, int priority) { + } + + @Override + public void removeProcessor(PacketProcessor processor) { + } + + @Override + public void requestPackets(TrafficSelector selector, PacketPriority priority, ApplicationId appId) { + } + + @Override + public void cancelPackets(TrafficSelector selector, PacketPriority priority, ApplicationId appId) { + } + + @Override + public void emit(OutboundPacket packet) { + } +} diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimplePacketStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimplePacketStore.java index 81cef49e20..4345abaf27 100644 --- a/core/common/src/test/java/org/onosproject/store/trivial/SimplePacketStore.java +++ b/core/common/src/test/java/org/onosproject/store/trivial/SimplePacketStore.java @@ -51,6 +51,11 @@ public class SimplePacketStore return requests.add(request); } + @Override + public boolean cancelPackets(PacketRequest request) { + return requests.remove(request); + } + @Override public Set existingRequests() { return Collections.unmodifiableSet(requests); diff --git a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java index d5b12b3f78..b92067fc7e 100644 --- a/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java +++ b/core/net/src/main/java/org/onosproject/net/flow/impl/FlowRuleManager.java @@ -498,10 +498,10 @@ public class FlowRuleManager FlowRuleBatchOperation batchOperation = request.asBatchOperation(deviceId); - FlowRuleProvider flowRuleProvider = - getProvider(deviceId); - - flowRuleProvider.executeBatch(batchOperation); + FlowRuleProvider flowRuleProvider = getProvider(deviceId); + if (flowRuleProvider != null) { + flowRuleProvider.executeBatch(batchOperation); + } break; diff --git a/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java b/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java index 28f1df0c16..d7ed927ec4 100644 --- a/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java +++ b/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java @@ -29,10 +29,8 @@ import org.onosproject.net.device.DeviceEvent; import org.onosproject.net.device.DeviceListener; import org.onosproject.net.device.DeviceService; import org.onosproject.net.flow.DefaultTrafficTreatment; -import org.onosproject.net.flow.FlowRule; import org.onosproject.net.flow.FlowRuleService; import org.onosproject.net.flow.TrafficSelector; -import org.onosproject.net.flow.TrafficTreatment; import org.onosproject.net.flowobjective.DefaultForwardingObjective; import org.onosproject.net.flowobjective.FlowObjectiveService; import org.onosproject.net.flowobjective.ForwardingObjective; @@ -62,9 +60,9 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import static com.google.common.base.Preconditions.checkNotNull; -import static org.slf4j.LoggerFactory.getLogger; import static org.onlab.util.Tools.groupedThreads; import static org.onosproject.security.AppGuard.checkPermission; +import static org.slf4j.LoggerFactory.getLogger; /** @@ -78,6 +76,10 @@ public class PacketManager private final Logger log = getLogger(getClass()); + private static final String TABLE_TYPE_MSG = + "Table Type cannot be null. For requesting packets without " + + "table hints, use other methods in the packetService API"; + private final PacketStoreDelegate delegate = new InternalStoreDelegate(); @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) @@ -125,7 +127,6 @@ public class PacketManager @Override public void addProcessor(PacketProcessor processor, int priority) { checkPermission(Permission.PACKET_EVENT); - checkNotNull(processor, "Processor cannot be null"); processors.put(priority, processor); } @@ -133,7 +134,6 @@ public class PacketManager @Override public void removeProcessor(PacketProcessor processor) { checkPermission(Permission.PACKET_EVENT); - checkNotNull(processor, "Processor cannot be null"); processors.values().remove(processor); } @@ -142,35 +142,26 @@ public class PacketManager public void requestPackets(TrafficSelector selector, PacketPriority priority, ApplicationId appId) { checkPermission(Permission.PACKET_READ); - checkNotNull(selector, "Selector cannot be null"); checkNotNull(appId, "Application ID cannot be null"); - PacketRequest request = - new DefaultPacketRequest(selector, priority, appId, FlowRule.Type.DEFAULT); - + PacketRequest request = new DefaultPacketRequest(selector, priority, appId); if (store.requestPackets(request)) { pushToAllDevices(request); } } @Override - public void requestPackets(TrafficSelector selector, PacketPriority priority, - ApplicationId appId, FlowRule.Type tableType) { + public void cancelPackets(TrafficSelector selector, PacketPriority priority, + ApplicationId appId) { checkPermission(Permission.PACKET_READ); - checkNotNull(selector, "Selector cannot be null"); checkNotNull(appId, "Application ID cannot be null"); - checkNotNull(tableType, "Table Type cannot be null. For requesting packets +" - + "without table hints, use other methods in the packetService API"); - PacketRequest request = - new DefaultPacketRequest(selector, priority, appId, tableType); - - if (store.requestPackets(request)) { - pushToAllDevices(request); + PacketRequest request = new DefaultPacketRequest(selector, priority, appId); + if (store.cancelPackets(request)) { + removeFromAllDevices(request); } - } /** @@ -184,9 +175,20 @@ public class PacketManager } } + /** - * Pushes flow rules to the device to request packets be sent to the - * controller. + * Removes packet request flow rule from all devices. + * + * @param request the packet request + */ + private void removeFromAllDevices(PacketRequest request) { + for (Device device : deviceService.getDevices()) { + removeRule(device, request); + } + } + + /** + * Pushes packet intercept flow rules to the device. * * @param device the device to push the rules to * @param request the packet request @@ -197,37 +199,54 @@ public class PacketManager return; } - TrafficTreatment treatment = DefaultTrafficTreatment.builder() - .punt() - .build(); - - ForwardingObjective forwarding = DefaultForwardingObjective.builder() - .withPriority(request.priority().priorityValue()) - .withSelector(request.selector()) - .fromApp(appId) - .withFlag(ForwardingObjective.Flag.VERSATILE) - .withTreatment(treatment) - .makePermanent() + ForwardingObjective forwarding = createBuilder(request) .add(new ObjectiveContext() { - @Override - public void onSuccess(Objective objective) { } - @Override public void onError(Objective objective, ObjectiveError error) { - log.warn("Failed to install packet request {}: {}", - request, error); + log.warn("Failed to install packet request {}: {}", request, error); } }); objectiveService.forward(device.id(), forwarding); } + /** + * Removes packet intercept flow rules from the device. + * + * @param device the device to remove the rules deom + * @param request the packet request + */ + private void removeRule(Device device, PacketRequest request) { + // Everything is pre-provisioned on ROADMs + if (device.type().equals(Device.Type.ROADM)) { + return; + } + + ForwardingObjective forwarding = createBuilder(request) + .remove(new ObjectiveContext() { + @Override + public void onError(Objective objective, ObjectiveError error) { + log.warn("Failed to withdraw packet request {}: {}", request, error); + } + }); + + objectiveService.forward(device.id(), forwarding); + } + + private DefaultForwardingObjective.Builder createBuilder(PacketRequest request) { + return DefaultForwardingObjective.builder() + .withPriority(request.priority().priorityValue()) + .withSelector(request.selector()) + .fromApp(appId) + .withFlag(ForwardingObjective.Flag.VERSATILE) + .withTreatment(DefaultTrafficTreatment.builder().punt().build()) + .makePermanent(); + } + @Override public void emit(OutboundPacket packet) { checkPermission(Permission.PACKET_WRITE); - checkNotNull(packet, "Packet cannot be null"); - store.emit(packet); } @@ -238,8 +257,7 @@ public class PacketManager return; } - final PacketProvider packetProvider = getProvider(device.providerId()); - + PacketProvider packetProvider = getProvider(device.providerId()); if (packetProvider != null) { packetProvider.emit(packet); } @@ -250,7 +268,7 @@ public class PacketManager return new InternalPacketProviderService(provider); } - // Personalized link provider service issued to the supplied provider. + // Personalized packet provider service issued to the supplied provider. private class InternalPacketProviderService extends AbstractProviderService implements PacketProviderService { diff --git a/core/net/src/test/java/org/onosproject/net/host/impl/HostMonitorTest.java b/core/net/src/test/java/org/onosproject/net/host/impl/HostMonitorTest.java index 1028ddc88f..679a888eeb 100644 --- a/core/net/src/test/java/org/onosproject/net/host/impl/HostMonitorTest.java +++ b/core/net/src/test/java/org/onosproject/net/host/impl/HostMonitorTest.java @@ -15,20 +15,9 @@ */ package org.onosproject.net.host.impl; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Set; - +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; import org.junit.After; import org.junit.Test; import org.onlab.packet.ARP; @@ -36,7 +25,6 @@ import org.onlab.packet.Ethernet; import org.onlab.packet.IpAddress; import org.onlab.packet.IpPrefix; import org.onlab.packet.MacAddress; -import org.onosproject.core.ApplicationId; import org.onlab.packet.VlanId; import org.onosproject.net.ConnectPoint; import org.onosproject.net.Device; @@ -47,31 +35,31 @@ import org.onosproject.net.Port; import org.onosproject.net.PortNumber; import org.onosproject.net.device.DeviceListener; import org.onosproject.net.device.DeviceServiceAdapter; -import org.onosproject.net.flow.FlowRule; -import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.instructions.Instruction; import org.onosproject.net.flow.instructions.Instructions.OutputInstruction; import org.onosproject.net.host.HostProvider; import org.onosproject.net.host.InterfaceIpAddress; import org.onosproject.net.host.PortAddresses; import org.onosproject.net.packet.OutboundPacket; -import org.onosproject.net.packet.PacketPriority; -import org.onosproject.net.packet.PacketProcessor; -import org.onosproject.net.packet.PacketService; +import org.onosproject.net.packet.PacketServiceAdapter; import org.onosproject.net.provider.ProviderId; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.Lists; -import com.google.common.collect.Multimap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import static org.easymock.EasyMock.*; +import static org.junit.Assert.*; public class HostMonitorTest { private static final IpAddress TARGET_IP_ADDR = - IpAddress.valueOf("10.0.0.1"); + IpAddress.valueOf("10.0.0.1"); private static final IpAddress SOURCE_ADDR = - IpAddress.valueOf("10.0.0.99"); + IpAddress.valueOf("10.0.0.99"); private static final InterfaceIpAddress IA1 = - new InterfaceIpAddress(SOURCE_ADDR, IpPrefix.valueOf("10.0.0.0/24")); + new InterfaceIpAddress(SOURCE_ADDR, IpPrefix.valueOf("10.0.0.0/24")); private MacAddress sourceMac = MacAddress.valueOf(1L); private HostMonitor hostMonitor; @@ -132,7 +120,7 @@ public class HostMonitorTest { ConnectPoint cp = new ConnectPoint(devId, portNum); PortAddresses pa = - new PortAddresses(cp, Collections.singleton(IA1), sourceMac, VlanId.NONE); + new PortAddresses(cp, Collections.singleton(IA1), sourceMac, VlanId.NONE); expect(hostManager.getHostsByIp(TARGET_IP_ADDR)) .andReturn(Collections.emptySet()).anyTimes(); @@ -200,8 +188,8 @@ public class HostMonitorTest { ConnectPoint cp = new ConnectPoint(devId, portNum); PortAddresses pa = - new PortAddresses(cp, Collections.singleton(IA1), sourceMac, - VlanId.vlanId(vlan)); + new PortAddresses(cp, Collections.singleton(IA1), sourceMac, + VlanId.vlanId(vlan)); expect(hostManager.getHostsByIp(TARGET_IP_ADDR)) .andReturn(Collections.emptySet()).anyTimes(); @@ -246,33 +234,14 @@ public class HostMonitorTest { arp.getTargetProtocolAddress()); } - class TestPacketService implements PacketService { + class TestPacketService extends PacketServiceAdapter { List packets = new ArrayList<>(); - @Override - public void addProcessor(PacketProcessor processor, int priority) { - } - - @Override - public void removeProcessor(PacketProcessor processor) { - } - @Override public void emit(OutboundPacket packet) { packets.add(packet); } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId) { - } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId, - FlowRule.Type tableType) { - } } class TestDeviceService extends DeviceServiceAdapter { diff --git a/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java b/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java index 9e45a34133..c79d44c131 100644 --- a/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java +++ b/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java @@ -15,21 +15,7 @@ */ package org.onosproject.net.proxyarp.impl; -import static org.easymock.EasyMock.anyObject; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; -import java.util.Set; - +import com.google.common.collect.Sets; import org.junit.Before; import org.junit.Test; import org.onlab.packet.ARP; @@ -38,7 +24,6 @@ import org.onlab.packet.Ip4Address; import org.onlab.packet.Ip4Prefix; import org.onlab.packet.MacAddress; import org.onlab.packet.VlanId; -import org.onosproject.core.ApplicationId; import org.onosproject.net.ConnectPoint; import org.onosproject.net.DefaultHost; import org.onosproject.net.Device; @@ -51,8 +36,6 @@ import org.onosproject.net.Port; import org.onosproject.net.PortNumber; import org.onosproject.net.device.DeviceListener; import org.onosproject.net.device.DeviceService; -import org.onosproject.net.flow.FlowRule; -import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.instructions.Instruction; import org.onosproject.net.flow.instructions.Instructions.OutputInstruction; import org.onosproject.net.host.HostService; @@ -61,12 +44,17 @@ import org.onosproject.net.host.PortAddresses; import org.onosproject.net.link.LinkListener; import org.onosproject.net.link.LinkService; import org.onosproject.net.packet.OutboundPacket; -import org.onosproject.net.packet.PacketPriority; -import org.onosproject.net.packet.PacketProcessor; -import org.onosproject.net.packet.PacketService; +import org.onosproject.net.packet.PacketServiceAdapter; import org.onosproject.net.provider.ProviderId; -import com.google.common.collect.Sets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Set; + +import static org.easymock.EasyMock.*; +import static org.junit.Assert.*; /** * Tests for the {@link ProxyArpManager} class. @@ -208,17 +196,17 @@ public class ProxyArpManagerTest { for (int i = 1; i <= NUM_ADDRESS_PORTS; i++) { ConnectPoint cp = new ConnectPoint(getDeviceId(i), P1); Ip4Prefix prefix1 = - Ip4Prefix.valueOf("10.0." + (2 * i - 1) + ".0/24"); + Ip4Prefix.valueOf("10.0." + (2 * i - 1) + ".0/24"); Ip4Address addr1 = - Ip4Address.valueOf("10.0." + (2 * i - 1) + ".1"); + Ip4Address.valueOf("10.0." + (2 * i - 1) + ".1"); Ip4Prefix prefix2 = Ip4Prefix.valueOf("10.0." + (2 * i) + ".0/24"); Ip4Address addr2 = Ip4Address.valueOf("10.0." + (2 * i) + ".1"); InterfaceIpAddress ia1 = new InterfaceIpAddress(addr1, prefix1); InterfaceIpAddress ia2 = new InterfaceIpAddress(addr2, prefix2); PortAddresses pa1 = - new PortAddresses(cp, Sets.newHashSet(ia1), - MacAddress.valueOf(2 * i - 1), - VlanId.vlanId((short) 1)); + new PortAddresses(cp, Sets.newHashSet(ia1), + MacAddress.valueOf(2 * i - 1), + VlanId.vlanId((short) 1)); PortAddresses pa2 = new PortAddresses(cp, Sets.newHashSet(ia2), MacAddress.valueOf(2 * i), @@ -235,7 +223,7 @@ public class ProxyArpManagerTest { for (int i = 1; i <= NUM_FLOOD_PORTS; i++) { ConnectPoint cp = new ConnectPoint(getDeviceId(i + NUM_ADDRESS_PORTS), - P1); + P1); expect(hostService.getAddressBindingsForPort(cp)) .andReturn(Collections.emptySet()).anyTimes(); } @@ -279,13 +267,13 @@ public class ProxyArpManagerTest { @Test public void testReplyKnown() { Host replyer = new DefaultHost(PID, HID1, MAC1, VLAN1, getLocation(4), - Collections.singleton(IP1)); + Collections.singleton(IP1)); Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, getLocation(5), - Collections.singleton(IP2)); + Collections.singleton(IP2)); expect(hostService.getHostsByIp(IP1)) - .andReturn(Collections.singleton(replyer)); + .andReturn(Collections.singleton(replyer)); expect(hostService.getHost(HID2)).andReturn(requestor); replay(hostService); @@ -307,7 +295,7 @@ public class ProxyArpManagerTest { @Test public void testReplyUnknown() { Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, getLocation(5), - Collections.singleton(IP2)); + Collections.singleton(IP2)); expect(hostService.getHostsByIp(IP1)) .andReturn(Collections.emptySet()); @@ -331,10 +319,10 @@ public class ProxyArpManagerTest { @Test public void testReplyDifferentVlan() { Host replyer = new DefaultHost(PID, HID1, MAC1, VLAN2, getLocation(4), - Collections.singleton(IP1)); + Collections.singleton(IP1)); Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, getLocation(5), - Collections.singleton(IP2)); + Collections.singleton(IP2)); expect(hostService.getHostsByIp(IP1)) .andReturn(Collections.singleton(replyer)); @@ -358,7 +346,7 @@ public class ProxyArpManagerTest { MacAddress secondMac = MacAddress.valueOf(2L); Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1, - Collections.singleton(theirIp)); + Collections.singleton(theirIp)); expect(hostService.getHost(HID2)).andReturn(requestor); replay(hostService); @@ -390,7 +378,7 @@ public class ProxyArpManagerTest { // Request for a valid external IP address but coming in the wrong port Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC1, null, theirIp, - Ip4Address.valueOf("10.0.3.1")); + Ip4Address.valueOf("10.0.3.1")); proxyArp.reply(arpRequest, LOC1); assertEquals(0, packetService.packets.size()); @@ -433,7 +421,7 @@ public class ProxyArpManagerTest { @Test public void testForwardToHost() { Host host1 = new DefaultHost(PID, HID1, MAC1, VLAN1, LOC1, - Collections.singleton(IP1)); + Collections.singleton(IP1)); expect(hostService.getHost(HID1)).andReturn(host1); replay(hostService); @@ -476,17 +464,17 @@ public class ProxyArpManagerTest { assertEquals(NUM_FLOOD_PORTS - 1, packetService.packets.size()); Collections.sort(packetService.packets, - new Comparator() { - @Override - public int compare(OutboundPacket o1, OutboundPacket o2) { - return o1.sendThrough().uri().compareTo(o2.sendThrough().uri()); - } - }); + new Comparator() { + @Override + public int compare(OutboundPacket o1, OutboundPacket o2) { + return o1.sendThrough().uri().compareTo(o2.sendThrough().uri()); + } + }); for (int i = 0; i < NUM_FLOOD_PORTS - 1; i++) { ConnectPoint cp = new ConnectPoint(getDeviceId(NUM_ADDRESS_PORTS + i + 1), - PortNumber.portNumber(1)); + PortNumber.portNumber(1)); OutboundPacket outboundPacket = packetService.packets.get(i); verifyPacketOut(packet, cp, outboundPacket); @@ -497,11 +485,11 @@ public class ProxyArpManagerTest { * Verifies the given packet was sent out the given port. * * @param expected the packet that was expected to be sent - * @param outPort the port the packet was expected to be sent out - * @param actual the actual OutboundPacket to verify + * @param outPort the port the packet was expected to be sent out + * @param actual the actual OutboundPacket to verify */ private void verifyPacketOut(Ethernet expected, ConnectPoint outPort, - OutboundPacket actual) { + OutboundPacket actual) { assertArrayEquals(expected.serialize(), actual.data().array()); assertEquals(1, actual.treatment().immediate().size()); assertEquals(outPort.deviceId(), actual.sendThrough()); @@ -530,12 +518,12 @@ public class ProxyArpManagerTest { * @param opcode opcode of the ARP packet * @param srcMac source MAC address * @param dstMac destination MAC address, or null if this is a request - * @param srcIp source IP address - * @param dstIp destination IP address + * @param srcIp source IP address + * @param dstIp destination IP address * @return the ARP packet */ private Ethernet buildArp(short opcode, MacAddress srcMac, MacAddress dstMac, - Ip4Address srcIp, Ip4Address dstIp) { + Ip4Address srcIp, Ip4Address dstIp) { Ethernet eth = new Ethernet(); if (dstMac == null) { @@ -574,32 +562,14 @@ public class ProxyArpManagerTest { * Test PacketService implementation that simply stores OutboundPackets * passed to {@link #emit(OutboundPacket)} for later verification. */ - class TestPacketService implements PacketService { + class TestPacketService extends PacketServiceAdapter { List packets = new ArrayList<>(); - @Override - public void addProcessor(PacketProcessor processor, int priority) { - } - - @Override - public void removeProcessor(PacketProcessor processor) { - } - @Override public void emit(OutboundPacket packet) { packets.add(packet); } - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId) { - } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId, - FlowRule.Type tableType) { - } } } diff --git a/core/store/dist/src/main/java/org/onosproject/store/packet/impl/DistributedPacketStore.java b/core/store/dist/src/main/java/org/onosproject/store/packet/impl/DistributedPacketStore.java index 5357fa8c1a..027378aaa8 100644 --- a/core/store/dist/src/main/java/org/onosproject/store/packet/impl/DistributedPacketStore.java +++ b/core/store/dist/src/main/java/org/onosproject/store/packet/impl/DistributedPacketStore.java @@ -15,6 +15,7 @@ */ package org.onosproject.store.packet.impl; +import com.google.common.collect.ImmutableSet; import org.apache.felix.scr.annotations.Activate; import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Deactivate; @@ -25,6 +26,7 @@ import org.onlab.util.KryoNamespace; import org.onosproject.cluster.ClusterService; import org.onosproject.cluster.NodeId; import org.onosproject.mastership.MastershipService; +import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.packet.OutboundPacket; import org.onosproject.net.packet.PacketEvent; import org.onosproject.net.packet.PacketEvent.Type; @@ -41,8 +43,10 @@ import org.onosproject.store.serializers.KryoSerializer; import org.onosproject.store.service.ConsistentMap; import org.onosproject.store.service.Serializer; import org.onosproject.store.service.StorageService; +import org.onosproject.store.service.Versioned; import org.slf4j.Logger; +import java.util.HashSet; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -96,7 +100,7 @@ public class DistributedPacketStore @Activate public void activate() { - messageHandlingExecutor = Executors.newFixedThreadPool( + messageHandlingExecutor = Executors.newFixedThreadPool( MESSAGE_HANDLER_THREAD_POOL_SIZE, groupedThreads("onos/store/packet", "message-handlers")); @@ -104,7 +108,7 @@ public class DistributedPacketStore new InternalClusterMessageHandler(), messageHandlingExecutor); - tracker = new PacketRequestTracker(); + tracker = new PacketRequestTracker(); log.info("Started"); } @@ -140,6 +144,11 @@ public class DistributedPacketStore return tracker.add(request); } + @Override + public boolean cancelPackets(PacketRequest request) { + return tracker.remove(request); + } + @Override public Set existingRequests() { return tracker.requests(); @@ -162,47 +171,49 @@ public class DistributedPacketStore private class PacketRequestTracker { - private ConsistentMap requests; + private ConsistentMap> requests; public PacketRequestTracker() { - requests = storageService.consistentMapBuilder() - .withName("packet-requests") + requests = storageService.>consistentMapBuilder() + .withName("onos-packet-requests") .withPartitionsDisabled() - .withSerializer(Serializer.using( - new KryoNamespace.Builder().register(KryoNamespaces.API).build())) - .withSerializer(new Serializer() { - KryoNamespace kryo = new KryoNamespace.Builder() - .register(KryoNamespaces.API) - .build(); - - @Override - public byte[] encode(T object) { - return kryo.serialize(object); - } - - @Override - public T decode(byte[] bytes) { - return kryo.deserialize(bytes); - } - }).build(); + .withSerializer(Serializer.using(KryoNamespaces.API)) + .build(); } public boolean add(PacketRequest request) { - if (requests.putIfAbsent(request, true) == null) { - return true; + Versioned> old = requests.get(request.selector()); + if (old != null && old.value().contains(request)) { + return false; } - return false; + // FIXME: add retry logic using a random delay + Set newSet = new HashSet<>(); + newSet.add(request); + if (old == null) { + return requests.putIfAbsent(request.selector(), newSet) == null; + } + newSet.addAll(old.value()); + return requests.replace(request.selector(), old.version(), newSet); } public boolean remove(PacketRequest request) { - if (requests.remove(request) == null) { + Versioned> old = requests.get(request.selector()); + if (old == null || !old.value().contains(request)) { return false; } - return true; + // FIXME: add retry logic using a random delay + Set newSet = new HashSet<>(old.value()); + newSet.remove(request); + if (newSet.isEmpty()) { + return requests.remove(request.selector(), old.version()); + } + return requests.replace(request.selector(), old.version(), newSet); } public Set requests() { - return requests.keySet(); + ImmutableSet.Builder builder = ImmutableSet.builder(); + requests.values().forEach(v -> builder.addAll(v.value())); + return builder.build(); } } diff --git a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java index b7762805a6..0da1ae3139 100644 --- a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java +++ b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java @@ -113,8 +113,8 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid private boolean hostRemovalEnabled = true; @Property(name = "ipv6NeighborDiscovery", boolValue = false, - label = "Enable using IPv6 Neighbor Discovery by the " + - "Host Location Provider; default is false") + label = "Enable using IPv6 Neighbor Discovery by the " + + "Host Location Provider; default is false") private boolean ipv6NeighborDiscovery = false; /** @@ -133,15 +133,17 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid packetService.addProcessor(processor, 1); deviceService.addListener(deviceListener); readComponentConfiguration(context); - requestPackests(); + requestIntercepts(); log.info("Started with Application ID {}", appId.id()); } @Deactivate public void deactivate() { - // TODO revoke all packet requests when deactivate cfgService.unregisterProperties(getClass(), false); + + withdrawIntercepts(); + providerRegistry.unregister(this); packetService.removeProcessor(processor); deviceService.removeListener(deviceListener); @@ -151,38 +153,54 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid @Modified public void modified(ComponentContext context) { - // TODO revoke unnecessary packet requests when config being modified readComponentConfiguration(context); - requestPackests(); + requestIntercepts(); } /** - * Request packet in via PacketService. + * Request packet intercepts. */ - private void requestPackests() { - TrafficSelector.Builder selectorBuilder = - DefaultTrafficSelector.builder(); - selectorBuilder.matchEthType(Ethernet.TYPE_ARP); - packetService.requestPackets(selectorBuilder.build(), - PacketPriority.CONTROL, appId); + private void requestIntercepts() { + TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); + selector.matchEthType(Ethernet.TYPE_ARP); + packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId); + // IPv6 Neighbor Solicitation packet. + selector.matchEthType(Ethernet.TYPE_IPV6); + selector.matchIPProtocol(IPv6.PROTOCOL_ICMP6); + selector.matchIcmpv6Type(ICMP6.NEIGHBOR_SOLICITATION); if (ipv6NeighborDiscovery) { - // IPv6 Neighbor Solicitation packet. - selectorBuilder = DefaultTrafficSelector.builder(); - selectorBuilder.matchEthType(Ethernet.TYPE_IPV6); - selectorBuilder.matchIPProtocol(IPv6.PROTOCOL_ICMP6); - selectorBuilder.matchIcmpv6Type(ICMP6.NEIGHBOR_SOLICITATION); - packetService.requestPackets(selectorBuilder.build(), - PacketPriority.CONTROL, appId); - - // IPv6 Neighbor Advertisement packet. - selectorBuilder = DefaultTrafficSelector.builder(); - selectorBuilder.matchEthType(Ethernet.TYPE_IPV6); - selectorBuilder.matchIPProtocol(IPv6.PROTOCOL_ICMP6); - selectorBuilder.matchIcmpv6Type(ICMP6.NEIGHBOR_ADVERTISEMENT); - packetService.requestPackets(selectorBuilder.build(), - PacketPriority.CONTROL, appId); + packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId); + } else { + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); } + + // IPv6 Neighbor Advertisement packet. + selector.matchIcmpv6Type(ICMP6.NEIGHBOR_ADVERTISEMENT); + if (ipv6NeighborDiscovery) { + packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId); + } else { + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); + } + } + + /** + * Withdraw packet intercepts. + */ + private void withdrawIntercepts() { + TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); + selector.matchEthType(Ethernet.TYPE_ARP); + packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId); + + // IPv6 Neighbor Solicitation packet. + selector.matchEthType(Ethernet.TYPE_IPV6); + selector.matchIPProtocol(IPv6.PROTOCOL_ICMP6); + selector.matchIcmpv6Type(ICMP6.NEIGHBOR_SOLICITATION); + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); + + // IPv6 Neighbor Advertisement packet. + selector.matchIcmpv6Type(ICMP6.NEIGHBOR_ADVERTISEMENT); + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); } /** @@ -197,7 +215,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid flag = isPropertyEnabled(properties, "hostRemovalEnabled"); if (flag == null) { log.info("Host removal on port/device down events is not configured, " + - "using current value of {}", hostRemovalEnabled); + "using current value of {}", hostRemovalEnabled); } else { hostRemovalEnabled = flag; log.info("Configured. Host removal on port/device down events is {}", @@ -207,7 +225,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid flag = isPropertyEnabled(properties, "ipv6NeighborDiscovery"); if (flag == null) { log.info("Using IPv6 Neighbor Discovery is not configured, " + - "using current value of {}", ipv6NeighborDiscovery); + "using current value of {}", ipv6NeighborDiscovery); } else { ipv6NeighborDiscovery = flag; log.info("Configured. Using IPv6 Neighbor Discovery is {}", @@ -218,7 +236,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid /** * Check property name is defined and set to true. * - * @param properties properties to be looked up + * @param properties properties to be looked up * @param propertyName the name of the property to look up * @return value when the propertyName is defined or return null */ @@ -244,24 +262,25 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid /** * Update host location only. * - * @param hid host ID - * @param mac source Mac address + * @param hid host ID + * @param mac source Mac address * @param vlan VLAN ID * @param hloc host location */ private void updateLocation(HostId hid, MacAddress mac, - VlanId vlan, HostLocation hloc) { + VlanId vlan, HostLocation hloc) { HostDescription desc = new DefaultHostDescription(mac, vlan, hloc); providerService.hostDetected(hid, desc); } + /** * Update host location and IP address. * - * @param hid host ID - * @param mac source Mac address + * @param hid host ID + * @param mac source Mac address * @param vlan VLAN ID * @param hloc host location - * @param ip source IP address + * @param ip source IP address */ private void updateLocationIP(HostId hid, MacAddress mac, VlanId vlan, HostLocation hloc, @@ -297,7 +316,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid } HostLocation hloc = - new HostLocation(heardOn, System.currentTimeMillis()); + new HostLocation(heardOn, System.currentTimeMillis()); HostId hid = HostId.hostId(eth.getSourceMAC(), vlan); @@ -308,19 +327,19 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid arp.getSenderProtocolAddress()); updateLocationIP(hid, srcMac, vlan, hloc, ip); - // IPv4: update location only + // IPv4: update location only } else if (eth.getEtherType() == Ethernet.TYPE_IPV4) { updateLocation(hid, srcMac, vlan, hloc); - // - // NeighborAdvertisement and NeighborSolicitation: possible - // new hosts, update both location and IP. - // - // IPv6: update location only + // + // NeighborAdvertisement and NeighborSolicitation: possible + // new hosts, update both location and IP. + // + // IPv6: update location only } else if (eth.getEtherType() == Ethernet.TYPE_IPV6) { IPv6 ipv6 = (IPv6) eth.getPayload(); IpAddress ip = IpAddress.valueOf(IpAddress.Version.INET6, - ipv6.getSourceAddress()); + ipv6.getSourceAddress()); // skip extension headers IPacket pkt = ipv6; @@ -335,11 +354,11 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid pkt = pkt.getPayload(); // RouterSolicitation, RouterAdvertisement if (pkt != null && (pkt instanceof RouterAdvertisement || - pkt instanceof RouterSolicitation)) { + pkt instanceof RouterSolicitation)) { return; } if (pkt != null && (pkt instanceof NeighborSolicitation || - pkt instanceof NeighborAdvertisement)) { + pkt instanceof NeighborAdvertisement)) { // Duplicate Address Detection if (ip.isZero()) { return; @@ -367,37 +386,37 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid public void event(DeviceEvent event) { Device device = event.subject(); switch (event.type()) { - case DEVICE_ADDED: - break; - case DEVICE_AVAILABILITY_CHANGED: - if (hostRemovalEnabled && - !deviceService.isAvailable(device.id())) { - removeHosts(hostService.getConnectedHosts(device.id())); - } - break; - case DEVICE_SUSPENDED: - case DEVICE_UPDATED: - // Nothing to do? - break; - case DEVICE_REMOVED: - if (hostRemovalEnabled) { - removeHosts(hostService.getConnectedHosts(device.id())); - } - break; - case PORT_ADDED: - break; - case PORT_UPDATED: - if (hostRemovalEnabled) { - ConnectPoint point = - new ConnectPoint(device.id(), event.port().number()); - removeHosts(hostService.getConnectedHosts(point)); - } - break; - case PORT_REMOVED: - // Nothing to do? - break; - default: - break; + case DEVICE_ADDED: + break; + case DEVICE_AVAILABILITY_CHANGED: + if (hostRemovalEnabled && + !deviceService.isAvailable(device.id())) { + removeHosts(hostService.getConnectedHosts(device.id())); + } + break; + case DEVICE_SUSPENDED: + case DEVICE_UPDATED: + // Nothing to do? + break; + case DEVICE_REMOVED: + if (hostRemovalEnabled) { + removeHosts(hostService.getConnectedHosts(device.id())); + } + break; + case PORT_ADDED: + break; + case PORT_UPDATED: + if (hostRemovalEnabled) { + ConnectPoint point = + new ConnectPoint(device.id(), event.port().number()); + removeHosts(hostService.getConnectedHosts(point)); + } + break; + case PORT_REMOVED: + // Nothing to do? + break; + default: + break; } } } diff --git a/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java b/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java index 9f9c3b452a..b9d90976e9 100644 --- a/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java +++ b/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java @@ -15,27 +15,7 @@ */ package org.onosproject.provider.host.impl; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.onlab.packet.VlanId.vlanId; -import static org.onosproject.net.Device.Type.SWITCH; -import static org.onosproject.net.DeviceId.deviceId; -import static org.onosproject.net.HostId.hostId; -import static org.onosproject.net.PortNumber.portNumber; -import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED; -import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_REMOVED; -import static org.onosproject.net.device.DeviceEvent.Type.PORT_UPDATED; - -import java.nio.ByteBuffer; -import java.util.Collections; -import java.util.Dictionary; -import java.util.Hashtable; -import java.util.Set; - +import com.google.common.collect.ImmutableSet; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -62,8 +42,6 @@ import org.onosproject.net.HostLocation; import org.onosproject.net.device.DeviceEvent; import org.onosproject.net.device.DeviceListener; import org.onosproject.net.device.DeviceServiceAdapter; -import org.onosproject.net.flow.FlowRule; -import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.TrafficTreatment; import org.onosproject.net.host.HostDescription; import org.onosproject.net.host.HostProvider; @@ -74,15 +52,27 @@ import org.onosproject.net.packet.DefaultInboundPacket; import org.onosproject.net.packet.InboundPacket; import org.onosproject.net.packet.OutboundPacket; import org.onosproject.net.packet.PacketContext; -import org.onosproject.net.packet.PacketPriority; import org.onosproject.net.packet.PacketProcessor; -import org.onosproject.net.packet.PacketService; +import org.onosproject.net.packet.PacketServiceAdapter; import org.onosproject.net.provider.AbstractProviderService; import org.onosproject.net.provider.ProviderId; import org.onosproject.net.topology.Topology; import org.onosproject.net.topology.TopologyServiceAdapter; -import com.google.common.collect.ImmutableSet; +import java.nio.ByteBuffer; +import java.util.Collections; +import java.util.Dictionary; +import java.util.Hashtable; +import java.util.Set; + +import static org.easymock.EasyMock.*; +import static org.junit.Assert.*; +import static org.onlab.packet.VlanId.vlanId; +import static org.onosproject.net.Device.Type.SWITCH; +import static org.onosproject.net.DeviceId.deviceId; +import static org.onosproject.net.HostId.hostId; +import static org.onosproject.net.PortNumber.portNumber; +import static org.onosproject.net.device.DeviceEvent.Type.*; public class HostLocationProviderTest { @@ -143,7 +133,7 @@ public class HostLocationProviderTest { coreService = createMock(CoreService.class); expect(coreService.registerApplication(appId.name())) - .andReturn(appId).anyTimes(); + .andReturn(appId).anyTimes(); replay(coreService); provider.cfgService = new ComponentConfigAdapter(); @@ -271,31 +261,11 @@ public class HostLocationProviderTest { } - private class TestPacketService implements PacketService { - + private class TestPacketService extends PacketServiceAdapter { @Override public void addProcessor(PacketProcessor processor, int priority) { testProcessor = processor; } - - @Override - public void removeProcessor(PacketProcessor processor) { - } - - @Override - public void emit(OutboundPacket packet) { - } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId) { - } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId, - FlowRule.Type tableType) { - } } diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java index 6a69af891d..ec4a3ffd67 100644 --- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java +++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java @@ -178,18 +178,20 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { executor = newSingleThreadScheduledExecutor(groupedThreads("onos/device", "sync-%d")); executor.scheduleAtFixedRate(new SyncDeviceInfoTask(), INIT_DELAY, DELAY, SECONDS); - requestPackets(); + requestIntercepts(); log.info("Started"); } @Deactivate public void deactivate() { - // TODO revoke all packet requests when deactivate cfgService.unregisterProperties(getClass(), false); if (disableLinkDiscovery) { return; } + + withdrawIntercepts(); + providerRegistry.unregister(this); deviceService.removeListener(listener); packetService.removeProcessor(listener); @@ -205,7 +207,6 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { @Modified public void modified(ComponentContext context) { - // TODO revoke unnecessary packet requests when config being modified if (context == null) { loadSuppressionRules(); return; @@ -225,7 +226,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { if (!Strings.isNullOrEmpty(s)) { lldpSuppression = s; } - + requestIntercepts(); loadSuppressionRules(); } @@ -246,22 +247,33 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider { } /** - * Request packet in via PacketService. + * Request packet intercepts. */ - private void requestPackets() { - TrafficSelector.Builder lldpSelector = DefaultTrafficSelector.builder(); - lldpSelector.matchEthType(Ethernet.TYPE_LLDP); - packetService.requestPackets(lldpSelector.build(), - PacketPriority.CONTROL, appId); + private void requestIntercepts() { + TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); + selector.matchEthType(Ethernet.TYPE_LLDP); + packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId); + selector.matchEthType(Ethernet.TYPE_BSN); if (useBDDP) { - TrafficSelector.Builder bddpSelector = DefaultTrafficSelector.builder(); - bddpSelector.matchEthType(Ethernet.TYPE_BSN); - packetService.requestPackets(bddpSelector.build(), - PacketPriority.CONTROL, appId); + packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId); + } else { + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); } } + /** + * Withdraw packet intercepts. + */ + private void withdrawIntercepts() { + TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); + selector.matchEthType(Ethernet.TYPE_LLDP); + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); + selector.matchEthType(Ethernet.TYPE_BSN); + packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId); + } + + private class InternalRoleListener implements MastershipListener { @Override diff --git a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java index f6627af08b..1d63a15dd2 100644 --- a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java +++ b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java @@ -15,23 +15,9 @@ */ package org.onosproject.provider.lldp.impl; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import java.nio.ByteBuffer; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.CompletableFuture; - +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -57,8 +43,6 @@ import org.onosproject.net.PortNumber; import org.onosproject.net.device.DeviceEvent; import org.onosproject.net.device.DeviceListener; import org.onosproject.net.device.DeviceServiceAdapter; -import org.onosproject.net.flow.FlowRule; -import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.TrafficTreatment; import org.onosproject.net.link.LinkDescription; import org.onosproject.net.link.LinkProvider; @@ -68,15 +52,21 @@ import org.onosproject.net.packet.DefaultInboundPacket; import org.onosproject.net.packet.InboundPacket; import org.onosproject.net.packet.OutboundPacket; import org.onosproject.net.packet.PacketContext; -import org.onosproject.net.packet.PacketPriority; import org.onosproject.net.packet.PacketProcessor; -import org.onosproject.net.packet.PacketService; +import org.onosproject.net.packet.PacketServiceAdapter; import org.onosproject.net.provider.AbstractProviderService; import org.onosproject.net.provider.ProviderId; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; +import java.nio.ByteBuffer; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletableFuture; + +import static org.easymock.EasyMock.*; +import static org.junit.Assert.*; public class LLDPLinkProviderTest { @@ -383,33 +373,11 @@ public class LLDPLinkProviderTest { } - private class TestPacketService implements PacketService { - + private class TestPacketService extends PacketServiceAdapter { @Override public void addProcessor(PacketProcessor processor, int priority) { testProcessor = processor; } - - @Override - public void removeProcessor(PacketProcessor processor) { - - } - - @Override - public void emit(OutboundPacket packet) { - - } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId) { - } - - @Override - public void requestPackets(TrafficSelector selector, - PacketPriority priority, ApplicationId appId, - FlowRule.Type tableType) { - } } private class TestDeviceService extends DeviceServiceAdapter { @@ -433,8 +401,6 @@ public class LLDPLinkProviderTest { ports.putAll(DID1, Lists.newArrayList(pd1, pd2)); ports.putAll(DID2, Lists.newArrayList(pd3, pd4)); - - } @Override