ONOS-2145 Added ability to withdraw packet intercepts via PacketService::cancelPackets.

Change-Id: Ie41271fa02740560bd67b0faf49f633ee749773c

apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java

@@ -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,
+        packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, appId);
-                                     appId);
         selector.matchEthType(Ethernet.TYPE_ARP);
-        packetService.requestPackets(selector.build(), PacketPriority.REACTIVE,
+        packetService.requestPackets(selector.build(), PacketPriority.REACTIVE, appId);
-                                     appId);
 
-        if (ipv6Forwarding) {
         selector.matchEthType(Ethernet.TYPE_IPV6);
-            packetService.requestPackets(selector.build(),
+        if (ipv6Forwarding) {
-                                         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.
      *

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.
- *
+ * <p>
  * Implementations of this class must be serializable.
+ * </p>
  */
 @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) {
+    }
 
 }

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();
+        return Objects.hash(selector, priority, appId);
-        result = 31 * result + priority.hashCode();
+    }
-        return result;
+
+    @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("appId", appId).toString();
-                .add("table-type", tableType).toString();
     }
 }

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();
-
 }

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;
 
 /**
@@ -60,22 +59,15 @@ public interface PacketService {
                         ApplicationId appId);
 
     /**
-     * Requests that packets matching the given selector are punted from the
+     * Cancels previous packet requests for packets matching the given
-     * dataplane to the controller. Clients of the PacketService should use
+     * selector to be punted from the dataplane to the controller.
-     * this call to hint at the tableType in the dataplane valid for the selector.
      *
      * @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
      */
-    void requestPackets(TrafficSelector selector, PacketPriority priority,
+    void cancelPackets(TrafficSelector selector, PacketPriority priority,
-                        ApplicationId appId, FlowRule.Type tableType);
+                       ApplicationId appId);
-
-
-    // TODO add API to allow applications to revoke requests when they deactivate
 
     /**
      * Emits the specified outbound packet onto the network.

core/api/src/main/java/org/onosproject/net/packet/PacketStore.java

@@ -34,15 +34,21 @@ public interface PacketStore extends Store<PacketEvent, PacketStoreDelegate> {
     void emit(OutboundPacket packet);
 
     /**
-     * Register a request for packets. If the registration
+     * Requests intercept of packets that match the given selector.
-     * is successful the manager can proceed, otherwise it should
-     * consider these packet already available in the system.
      *
      * @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.
      *

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) {
+    }
+}

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<PacketRequest> existingRequests() {
         return Collections.unmodifiableSet(requests);

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 =
+                FlowRuleProvider flowRuleProvider = getProvider(deviceId);
-                        getProvider(deviceId);
+                if (flowRuleProvider != null) {
-
                     flowRuleProvider.executeBatch(batchOperation);
+                }
 
                 break;
 

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 =
+        PacketRequest request = new DefaultPacketRequest(selector, priority, appId);
-                new DefaultPacketRequest(selector, priority, appId, FlowRule.Type.DEFAULT);
-
         if (store.requestPackets(request)) {
             pushToAllDevices(request);
         }
     }
 
     @Override
-    public void requestPackets(TrafficSelector selector, PacketPriority priority,
+    public void cancelPackets(TrafficSelector selector, PacketPriority priority,
-                               ApplicationId appId, FlowRule.Type tableType) {
+                              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 =
+        PacketRequest request = new DefaultPacketRequest(selector, priority, appId);
-                new DefaultPacketRequest(selector, priority, appId, tableType);
+        if (store.cancelPackets(request)) {
-
+            removeFromAllDevices(request);
-        if (store.requestPackets(request)) {
-            pushToAllDevices(request);
         }
-
     }
 
     /**
@@ -184,9 +175,20 @@ public class PacketManager
         }
     }
 
+
     /**
-     * Pushes flow rules to the device to request packets be sent to the
+     * Removes packet request flow rule from all devices.
-     * controller.
+     *
+     * @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()
+        ForwardingObjective forwarding = createBuilder(request)
-                                                            .punt()
-                                                            .build();
-
-        ForwardingObjective forwarding = DefaultForwardingObjective.builder()
-                .withPriority(request.priority().priorityValue())
-                .withSelector(request.selector())
-                .fromApp(appId)
-                .withFlag(ForwardingObjective.Flag.VERSATILE)
-                .withTreatment(treatment)
-                .makePermanent()
                 .add(new ObjectiveContext() {
-                    @Override
-                    public void onSuccess(Objective objective) { }
-
                     @Override
                     public void onError(Objective objective, ObjectiveError error) {
-                        log.warn("Failed to install packet request {}: {}",
+                        log.warn("Failed to install packet request {}: {}", request, error);
-                                 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<PacketProvider>
             implements PacketProviderService {

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 com.google.common.collect.HashMultimap;
-import static org.easymock.EasyMock.expect;
+import com.google.common.collect.Lists;
-import static org.easymock.EasyMock.expectLastCall;
+import com.google.common.collect.Multimap;
-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 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,22 +35,22 @@ 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.PacketServiceAdapter;
-import org.onosproject.net.packet.PacketProcessor;
-import org.onosproject.net.packet.PacketService;
 import org.onosproject.net.provider.ProviderId;
 
-import com.google.common.collect.HashMultimap;
+import java.util.ArrayList;
-import com.google.common.collect.Lists;
+import java.util.Collections;
-import com.google.common.collect.Multimap;
+import java.util.List;
+import java.util.Set;
+
+import static org.easymock.EasyMock.*;
+import static org.junit.Assert.*;
 
 public class HostMonitorTest {
 
@@ -246,33 +234,14 @@ public class HostMonitorTest {
                           arp.getTargetProtocolAddress());
     }
 
-    class TestPacketService implements PacketService {
+    class TestPacketService extends PacketServiceAdapter {
 
         List<OutboundPacket> 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 {

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 com.google.common.collect.Sets;
-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 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.PacketServiceAdapter;
-import org.onosproject.net.packet.PacketProcessor;
-import org.onosproject.net.packet.PacketService;
 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.
@@ -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<OutboundPacket> 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) {
-        }
     }
 }

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;
@@ -140,6 +144,11 @@ public class DistributedPacketStore
         return tracker.add(request);
     }
 
+    @Override
+    public boolean cancelPackets(PacketRequest request) {
+        return tracker.remove(request);
+    }
+
     @Override
     public Set<PacketRequest> existingRequests() {
         return tracker.requests();
@@ -162,47 +171,49 @@ public class DistributedPacketStore
 
     private class PacketRequestTracker {
 
-        private ConsistentMap<PacketRequest, Boolean> requests;
+        private ConsistentMap<TrafficSelector, Set<PacketRequest>> requests;
 
         public PacketRequestTracker() {
-            requests = storageService.<PacketRequest, Boolean>consistentMapBuilder()
+            requests = storageService.<TrafficSelector, Set<PacketRequest>>consistentMapBuilder()
-                    .withName("packet-requests")
+                    .withName("onos-packet-requests")
                     .withPartitionsDisabled()
-                    .withSerializer(Serializer.using(
+                    .withSerializer(Serializer.using(KryoNamespaces.API))
-                            new KryoNamespace.Builder().register(KryoNamespaces.API).build()))
-                    .withSerializer(new Serializer() {
-                        KryoNamespace kryo = new KryoNamespace.Builder()
-                                .register(KryoNamespaces.API)
                     .build();
-
-                        @Override
-                        public <T> byte[] encode(T object) {
-                            return kryo.serialize(object);
-                        }
-
-                        @Override
-                        public <T> T decode(byte[] bytes) {
-                            return kryo.deserialize(bytes);
-                        }
-                    }).build();
         }
 
         public boolean add(PacketRequest request) {
-            if (requests.putIfAbsent(request, true) == null) {
+            Versioned<Set<PacketRequest>> old = requests.get(request.selector());
-                return true;
+            if (old != null && old.value().contains(request)) {
-            }
                 return false;
             }
+            // FIXME: add retry logic using a random delay
+            Set<PacketRequest> 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<Set<PacketRequest>> 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<PacketRequest> 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<PacketRequest> requests() {
-            return requests.keySet();
+            ImmutableSet.Builder<PacketRequest> builder = ImmutableSet.builder();
+            requests.values().forEach(v -> builder.addAll(v.value()));
+            return builder.build();
         }
 
     }

providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java

@@ -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,40 +153,56 @@ 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() {
+    private void requestIntercepts() {
-        TrafficSelector.Builder selectorBuilder =
+        TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
-                DefaultTrafficSelector.builder();
+        selector.matchEthType(Ethernet.TYPE_ARP);
-        selectorBuilder.matchEthType(Ethernet.TYPE_ARP);
+        packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId);
-        packetService.requestPackets(selectorBuilder.build(),
-                                     PacketPriority.CONTROL, appId);
 
-        if (ipv6NeighborDiscovery) {
         // IPv6 Neighbor Solicitation packet.
-            selectorBuilder = DefaultTrafficSelector.builder();
+        selector.matchEthType(Ethernet.TYPE_IPV6);
-            selectorBuilder.matchEthType(Ethernet.TYPE_IPV6);
+        selector.matchIPProtocol(IPv6.PROTOCOL_ICMP6);
-            selectorBuilder.matchIPProtocol(IPv6.PROTOCOL_ICMP6);
+        selector.matchIcmpv6Type(ICMP6.NEIGHBOR_SOLICITATION);
-            selectorBuilder.matchIcmpv6Type(ICMP6.NEIGHBOR_SOLICITATION);
+        if (ipv6NeighborDiscovery) {
-            packetService.requestPackets(selectorBuilder.build(),
+            packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId);
-                                         PacketPriority.CONTROL, appId);
+        } else {
+            packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId);
+        }
 
         // IPv6 Neighbor Advertisement packet.
-            selectorBuilder = DefaultTrafficSelector.builder();
+        selector.matchIcmpv6Type(ICMP6.NEIGHBOR_ADVERTISEMENT);
-            selectorBuilder.matchEthType(Ethernet.TYPE_IPV6);
+        if (ipv6NeighborDiscovery) {
-            selectorBuilder.matchIPProtocol(IPv6.PROTOCOL_ICMP6);
+            packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId);
-            selectorBuilder.matchIcmpv6Type(ICMP6.NEIGHBOR_ADVERTISEMENT);
+        } else {
-            packetService.requestPackets(selectorBuilder.build(),
+            packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId);
-                                         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);
+    }
+
     /**
      * Extracts properties from the component configuration context.
      *
@@ -254,6 +272,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
             HostDescription desc = new DefaultHostDescription(mac, vlan, hloc);
             providerService.hostDetected(hid, desc);
         }
+
         /**
          * Update host location and IP address.
          *

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 com.google.common.collect.ImmutableSet;
-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 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 {
 
@@ -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) {
-        }
     }
 
 

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() {
+    private void requestIntercepts() {
-        TrafficSelector.Builder lldpSelector = DefaultTrafficSelector.builder();
+        TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
-        lldpSelector.matchEthType(Ethernet.TYPE_LLDP);
+        selector.matchEthType(Ethernet.TYPE_LLDP);
-        packetService.requestPackets(lldpSelector.build(),
+        packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId);
-                                     PacketPriority.CONTROL, appId);
 
+        selector.matchEthType(Ethernet.TYPE_BSN);
         if (useBDDP) {
-            TrafficSelector.Builder bddpSelector = DefaultTrafficSelector.builder();
+            packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId);
-            bddpSelector.matchEthType(Ethernet.TYPE_BSN);
+        } else {
-            packetService.requestPackets(bddpSelector.build(),
+            packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId);
-                                         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

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 com.google.common.collect.ArrayListMultimap;
-import static org.easymock.EasyMock.expect;
+import com.google.common.collect.Lists;
-import static org.easymock.EasyMock.replay;
+import com.google.common.collect.Maps;
-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 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 java.nio.ByteBuffer;
-import com.google.common.collect.Lists;
+import java.util.Collections;
-import com.google.common.collect.Maps;
+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