ONOS-2145 Added ability to withdraw packet intercepts via PacketService::cancelPackets.
Change-Id: Ie41271fa02740560bd67b0faf49f633ee749773c
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 d5b12b3..b92067f 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 @@
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 28f1df0..d7ed927 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.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.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 @@
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 @@
@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 @@
@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 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 @@
}
}
+
/**
- * 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 @@
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 @@
return;
}
- final PacketProvider packetProvider = getProvider(device.providerId());
-
+ PacketProvider packetProvider = getProvider(device.providerId());
if (packetProvider != null) {
packetProvider.emit(packet);
}
@@ -250,7 +268,7 @@
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 {
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 1028ddc..679a888 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.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.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 @@
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.<Host>emptySet()).anyTimes();
@@ -200,8 +188,8 @@
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.<Host>emptySet()).anyTimes();
@@ -246,33 +234,14 @@
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 {
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 9e45a34..c79d44c 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.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.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.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 @@
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 @@
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.<PortAddresses>emptySet()).anyTimes();
}
@@ -279,13 +267,13 @@
@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 @@
@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.<Host>emptySet());
@@ -331,10 +319,10 @@
@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 @@
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 @@
// 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 @@
@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 @@
assertEquals(NUM_FLOOD_PORTS - 1, packetService.packets.size());
Collections.sort(packetService.packets,
- new Comparator<OutboundPacket>() {
- @Override
- public int compare(OutboundPacket o1, OutboundPacket o2) {
- return o1.sendThrough().uri().compareTo(o2.sendThrough().uri());
- }
- });
+ new Comparator<OutboundPacket>() {
+ @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 @@
* 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 @@
* @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 @@
* 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) {
- }
}
}