[ONOS-6627] Revise adding and removing packet processor for virtual network
At present, we have to use requestPackets to trigger adding packet processor for virtual network and use cancelPackets to trigger removing the packet process for the virtual network.
But if we call cancelPackets more then one time in the deactivate() method when the application is deactivated, if will throw a NullPoint exception.
Furthermore, if a user does not requestPackets() in the application, the packet processor will never be added.
It may be a confusing trouble for a tenant user.
As a result, I think the packet processor should be created when the virtual network is added and be removed when no virtual network exists.
Soultions:
Listen to the network event to add and remove packet processor for virtual network.
Change-Id: I583d453219bef2f271b4a1e96f9869a28b4f0250
diff --git a/incubator/api/src/main/java/org/onosproject/incubator/net/virtual/provider/VirtualPacketProvider.java b/incubator/api/src/main/java/org/onosproject/incubator/net/virtual/provider/VirtualPacketProvider.java
index abd1d7d..9338e18 100644
--- a/incubator/api/src/main/java/org/onosproject/incubator/net/virtual/provider/VirtualPacketProvider.java
+++ b/incubator/api/src/main/java/org/onosproject/incubator/net/virtual/provider/VirtualPacketProvider.java
@@ -32,18 +32,4 @@
* @param packet outbound packet in the context of virtual network
*/
void emit(NetworkId networkId, OutboundPacket packet);
-
- /**
- * Starts to deliver packets to virtual packet managers.
- *
- * @param networkId the network identifier
- */
- void startPacketHandling(NetworkId networkId);
-
- /**
- * Stops to deliver packets to virtual packet managers.
- *
- * @param networkId the network identifier
- */
- void stopPacketHandling(NetworkId networkId);
}
diff --git a/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManager.java b/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManager.java
index abe089b..fc870b1 100644
--- a/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManager.java
+++ b/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManager.java
@@ -280,8 +280,6 @@
} else {
pushToAllDevices(request);
}
-
- providerService.provider().startPacketHandling(networkId);
}
@Override
@@ -293,8 +291,6 @@
} else {
removeFromAllDevices(request);
}
-
- providerService.provider().stopPacketHandling(networkId);
}
}
diff --git a/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProvider.java b/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProvider.java
index ad81063..e587fd1 100644
--- a/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProvider.java
+++ b/incubator/net/src/main/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProvider.java
@@ -31,6 +31,8 @@
import org.onosproject.incubator.net.virtual.VirtualDevice;
import org.onosproject.incubator.net.virtual.VirtualNetwork;
import org.onosproject.incubator.net.virtual.VirtualNetworkAdminService;
+import org.onosproject.incubator.net.virtual.VirtualNetworkEvent;
+import org.onosproject.incubator.net.virtual.VirtualNetworkListener;
import org.onosproject.incubator.net.virtual.VirtualPacketContext;
import org.onosproject.incubator.net.virtual.VirtualPort;
import org.onosproject.incubator.net.virtual.provider.AbstractVirtualProvider;
@@ -86,10 +88,13 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected VirtualProviderRegistryService providerRegistryService;
- private ApplicationId appId;
- private InternalPacketProcessor processor;
+ private final VirtualNetworkListener virtualNetListener = new InternalVirtualNetworkListener();
- private Set<NetworkId> requestsSet = Sets.newHashSet();
+ private InternalPacketProcessor processor = null;
+
+ private Set<NetworkId> networkIdSet = Sets.newConcurrentHashSet();
+
+ private ApplicationId appId;
/**
* Creates a provider with the supplied identifier.
@@ -102,16 +107,17 @@
public void activate() {
appId = coreService.registerApplication("org.onosproject.virtual.virtual-packet");
providerRegistryService.registerProvider(this);
+ vnaService.addListener(virtualNetListener);
log.info("Started");
}
@Deactivate
public void deactivate() {
- if (processor != null) {
- packetService.removeProcessor(processor);
- }
+
providerRegistryService.unregisterProvider(this);
+ vnaService.removeListener(virtualNetListener);
+
log.info("Stopped");
}
@@ -127,22 +133,12 @@
.forEach(outboundPacket -> packetService.emit(outboundPacket));
}
- @Override
- public void startPacketHandling(NetworkId networkId) {
- requestsSet.add(networkId);
- if (processor == null) {
- processor = new InternalPacketProcessor();
- packetService.addProcessor(processor, PACKET_PROCESSOR_PRIORITY);
- }
- }
-
- @Override
- public void stopPacketHandling(NetworkId networkId) {
- requestsSet.remove(networkId);
- if (requestsSet.isEmpty()) {
- packetService.removeProcessor(processor);
- processor = null;
- }
+ /**
+ * Just for test.
+ */
+ protected void startPacketHandling() {
+ processor = new InternalPacketProcessor();
+ packetService.addProcessor(processor, PACKET_PROCESSOR_PRIORITY);
}
/**
@@ -187,7 +183,7 @@
VirtualPacketContext vContext =
new DefaultVirtualPacketContext(context.time(), inPacket, outPkt,
- context.isHandled(), vPort.networkId(),
+ false, vPort.networkId(),
this);
return vContext;
@@ -373,6 +369,9 @@
@Override
public void process(PacketContext context) {
+ if (context.isHandled()) {
+ return;
+ }
VirtualPacketContext vContexts = virtualize(context);
if (vContexts == null) {
@@ -388,4 +387,35 @@
}
}
}
+
+ private class InternalVirtualNetworkListener implements VirtualNetworkListener {
+
+ @Override
+ public void event(VirtualNetworkEvent event) {
+ switch (event.type()) {
+ case NETWORK_ADDED:
+ if (networkIdSet.isEmpty()) {
+ processor = new InternalPacketProcessor();
+ packetService.addProcessor(processor, PACKET_PROCESSOR_PRIORITY);
+ log.info("Packet processor {} for virtual network is added.", processor.getClass().getName());
+ }
+ networkIdSet.add(event.subject());
+ break;
+
+ case NETWORK_REMOVED:
+ networkIdSet.remove(event.subject());
+ if (networkIdSet.isEmpty()) {
+ packetService.removeProcessor(processor);
+ log.info("Packet processor {} for virtual network is removed.", processor.getClass().getName());
+ processor = null;
+ }
+ break;
+
+ default:
+ // do nothing
+ break;
+ }
+ }
+ }
+
}
diff --git a/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManagerTest.java b/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManagerTest.java
index 05e5fc5..3143a9e 100644
--- a/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManagerTest.java
+++ b/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/VirtualNetworkPacketManagerTest.java
@@ -333,15 +333,6 @@
emittedPacket = packet;
}
- @Override
- public void startPacketHandling(NetworkId networkId) {
-
- }
-
- @Override
- public void stopPacketHandling(NetworkId networkId) {
-
- }
}
private class TestProcessor implements PacketProcessor {
diff --git a/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProviderTest.java b/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProviderTest.java
index 42a3645..a979a05 100644
--- a/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProviderTest.java
+++ b/incubator/net/src/test/java/org/onosproject/incubator/net/virtual/impl/provider/DefaultVirtualPacketProviderTest.java
@@ -154,7 +154,7 @@
TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
selector.matchEthType(Ethernet.TYPE_IPV4);
- virtualProvider.startPacketHandling(VNET_ID);
+ virtualProvider.startPacketHandling();
}
@After
diff --git a/incubator/store/src/main/java/org/onosproject/incubator/store/virtual/impl/DistributedVirtualNetworkStore.java b/incubator/store/src/main/java/org/onosproject/incubator/store/virtual/impl/DistributedVirtualNetworkStore.java
index c938814..c5f303b 100644
--- a/incubator/store/src/main/java/org/onosproject/incubator/store/virtual/impl/DistributedVirtualNetworkStore.java
+++ b/incubator/store/src/main/java/org/onosproject/incubator/store/virtual/impl/DistributedVirtualNetworkStore.java
@@ -696,7 +696,11 @@
Set<NetworkId> networkIdSet = tenantIdNetworkIdSetMap.get(tenantId);
Set<VirtualNetwork> virtualNetworkSet = new HashSet<>();
if (networkIdSet != null) {
- networkIdSet.forEach(networkId -> virtualNetworkSet.add(networkIdVirtualNetworkMap.get(networkId)));
+ networkIdSet.forEach(networkId -> {
+ if (networkIdVirtualNetworkMap.get(networkId) != null) {
+ virtualNetworkSet.add(networkIdVirtualNetworkMap.get(networkId));
+ }
+ });
}
return ImmutableSet.copyOf(virtualNetworkSet);
}