Work toward fixing ONOS-1031: Moved method FlowManager.getNextFlowEntryId()
Moved method FlowManager.getNextFlowEntryId() to
IControllerRegistryService.getNextUniqueId()
NOTE: For now the implementation of ZookeeperRegistry.getNextUniqueId()
is still using the random generator for the higher 32-bits allocation.
In the (near) future this should be replaced by Zookeeper mechanism that
allocates globally unique 32 bits.
NOTE2: For now the ZookeeperRegistry.getNextUniqueId() is using "synchronized"
because it is simpler. After its implementation is refactored to uze
the Zookeper for the allocation of the higher 32 bits, we can try
to avoid using "synchronized".
Change-Id: Iafff0168de9aec2fe4705e3f0a6690b510b0bb7d
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
index 7eac308..a3cb17b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
@@ -560,8 +560,6 @@
}
// Create flowPath FlowId
- //FlowId returnByRefFlowId = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId);
flowPath.setFlowId(new FlowId());
// Create DataPath object: srcSwitchPort
@@ -766,8 +764,6 @@
}
// Create flowPath FlowId
- //FlowId returnByRefFlowId = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId);
flowPath.setFlowId(new FlowId());
// Create the DataPath object: srcSwitchPort
@@ -821,11 +817,6 @@
Interface peerInterface = interfaces.get(bgpPeer.getInterfaceName());
- // set flowPath FlowId
- //FlowId returnByRefFlowId = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId);
-
-
// Create the Flow Path Match condition(s)
FlowEntryMatch flowEntryMatch = new FlowEntryMatch();
flowEntryMatch.enableEthernetFrameType(Ethernet.TYPE_IPv4);
@@ -877,8 +868,6 @@
flowPath.setFlowEntryMatch(flowEntryMatch);
// Create a new FlowId
- //FlowId returnByRefFlowId2 = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId2);
flowPath.setFlowId(new FlowId());
if (flowManagerService.addFlow(flowPath) == null) {
@@ -894,8 +883,6 @@
* Create the DataPath: BGP <-BGP peer
*/
// Reversed BGP flow path for src-TCP-port
- //FlowId returnByRefFlowId3 = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId3);
flowPath.setFlowId(new FlowId());
DataPath reverse_dataPath = new DataPath();
@@ -929,8 +916,6 @@
}
// Reversed BGP flow path for dst-TCP-port
- //FlowId returnByRefFlowId4 = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId4);
flowPath.setFlowId(new FlowId());
// Disable src-TCP-port, and set the dst-TCP-port
@@ -953,8 +938,6 @@
* ICMP paths between BGPd and its peers
*/
//match ICMP protocol BGP <- Peer
- //FlowId returnByRefFlowId5 = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId5);
flowPath.setFlowId(new FlowId());
flowEntryMatch.enableIpProto(IPv4.PROTOCOL_ICMP);
@@ -978,8 +961,6 @@
}
//match ICMP protocol BGP -> Peer
- //FlowId returnByRefFlowId6 = new FlowId(flowManagerService.getNextFlowEntryId());
- //flowPath.setFlowId(returnByRefFlowId6);
flowPath.setFlowId(new FlowId());
flowEntryMatch.enableDstIPv4Net(dstIPv4Net);
diff --git a/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowEventHandler.java b/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowEventHandler.java
index 116af1f..fd1a8b1 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowEventHandler.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowEventHandler.java
@@ -36,6 +36,7 @@
import net.onrc.onos.ofcontroller.util.Pair;
import net.onrc.onos.ofcontroller.util.Port;
import net.onrc.onos.ofcontroller.util.serializers.KryoFactory;
+import net.onrc.onos.registry.controller.IControllerRegistryService;
import com.esotericsoftware.kryo.Kryo;
import org.slf4j.Logger;
@@ -63,6 +64,7 @@
private FlowManager flowManager; // The Flow Manager to use
private IDatagridService datagridService; // The Datagrid Service to use
+ private IControllerRegistryService registryService; // The Registry Service
private Topology topology; // The network topology
private KryoFactory kryoFactory = new KryoFactory();
@@ -109,11 +111,14 @@
*
* @param flowManager the Flow Manager to use.
* @param datagridService the Datagrid Service to use.
+ * @param registryService the Registry Service to use.
*/
FlowEventHandler(FlowManager flowManager,
- IDatagridService datagridService) {
+ IDatagridService datagridService,
+ IControllerRegistryService registryService) {
this.flowManager = flowManager;
this.datagridService = datagridService;
+ this.registryService = registryService;
this.topology = new Topology();
}
@@ -323,7 +328,7 @@
for (FlowPath flowPath : modifiedFlowPaths.values()) {
for (FlowEntry flowEntry : flowPath.flowEntries()) {
if (! flowEntry.isValidFlowEntryId()) {
- long id = flowManager.getNextFlowEntryId();
+ long id = registryService.getNextUniqueId();
flowEntry.setFlowEntryId(new FlowEntryId(id));
}
}
@@ -543,7 +548,7 @@
if (mySwitch == null)
continue;
if (! flowEntry.isValidFlowEntryId()) {
- long id = flowManager.getNextFlowEntryId();
+ long id = registryService.getNextUniqueId();
flowEntry.setFlowEntryId(new FlowEntryId(id));
}
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowManager.java b/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowManager.java
index 6c09977..353997b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/flowmanager/FlowManager.java
@@ -6,7 +6,6 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
-import java.util.Random;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.concurrent.BlockingQueue;
@@ -69,11 +68,6 @@
private KryoFactory kryoFactory = new KryoFactory();
- // Flow Entry ID generation state
- private static Random randomGenerator = new Random();
- private static int nextFlowEntryIdPrefix = 0;
- private static int nextFlowEntryIdSuffix = 0;
-
/** The logger. */
private final static Logger log = LoggerFactory.getLogger(FlowManager.class);
@@ -185,30 +179,6 @@
}
/**
- * Get the next Flow Entry ID to use.
- *
- * @return the next Flow Entry ID to use.
- */
- @Override
- public synchronized long getNextFlowEntryId() {
- //
- // Generate the next Flow Entry ID.
- // NOTE: For now, the higher 32 bits are random, and
- // the lower 32 bits are sequential.
- // In the future, we need a better allocation mechanism.
- //
- if ((nextFlowEntryIdSuffix & 0xffffffffL) == 0xffffffffL) {
- nextFlowEntryIdPrefix = randomGenerator.nextInt();
- nextFlowEntryIdSuffix = 0;
- } else {
- nextFlowEntryIdSuffix++;
- }
- long result = (long)nextFlowEntryIdPrefix << 32;
- result = result | (0xffffffffL & nextFlowEntryIdSuffix);
- return result;
- }
-
- /**
* Startup module operation.
*
* @param context the module context to use for the startup.
@@ -217,9 +187,6 @@
public void startUp(FloodlightModuleContext context) {
restApi.addRestletRoutable(new FlowWebRoutable());
- // Initialize the Flow Entry ID generator
- nextFlowEntryIdPrefix = randomGenerator.nextInt();
-
//
// The thread to write to the database
//
@@ -233,7 +200,8 @@
// - register with the Datagrid Service
// - startup
//
- flowEventHandler = new FlowEventHandler(this, datagridService);
+ flowEventHandler = new FlowEventHandler(this, datagridService,
+ registryService);
floodlightProvider.addOFSwitchListener(flowEventHandler);
datagridService.registerFlowEventHandlerService(flowEventHandler);
flowEventHandler.start();
@@ -293,7 +261,7 @@
// Allocate the Flow ID if necessary
if (! flowPath.isValidFlowId()) {
- long id = getNextFlowEntryId();
+ long id = registryService.getNextUniqueId();
flowPath.setFlowId(new FlowId(id));
}
@@ -309,7 +277,7 @@
}
// The Flow Entry ID
if (! flowEntry.isValidFlowEntryId()) {
- long id = getNextFlowEntryId();
+ long id = registryService.getNextUniqueId();
flowEntry.setFlowEntryId(new FlowEntryId(id));
}
// The Flow ID
@@ -674,7 +642,7 @@
// assignments by the caller).
//
if (! flowEntry.isValidFlowEntryId()) {
- long id = getNextFlowEntryId();
+ long id = registryService.getNextUniqueId();
flowEntry.setFlowEntryId(new FlowEntryId(id));
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/flowmanager/IFlowService.java b/src/main/java/net/onrc/onos/ofcontroller/flowmanager/IFlowService.java
index 549a0fc..d6a5db1 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/flowmanager/IFlowService.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/flowmanager/IFlowService.java
@@ -71,14 +71,6 @@
Topology getTopology();
/**
- * Get a globally unique flow ID from the flow service.
- * NOTE: Not currently guaranteed to be globally unique.
- *
- * @return unique flow ID
- */
- public long getNextFlowEntryId();
-
- /**
* Inform the Flow Manager that a Flow Entry on switch expired.
*
* @param sw the switch the Flow Entry expired on.
diff --git a/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java b/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java
index 9c3b5ee..5ddee15 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java
@@ -42,6 +42,7 @@
import net.onrc.onos.ofcontroller.util.FlowPathUserState;
import net.onrc.onos.ofcontroller.util.Port;
import net.onrc.onos.ofcontroller.util.SwitchPort;
+import net.onrc.onos.registry.controller.IControllerRegistryService;
import org.openflow.protocol.OFMessage;
import org.openflow.protocol.OFPacketIn;
@@ -74,6 +75,7 @@
private IFlowService flowService;
private IFlowPusherService flowPusher;
private IDatagridService datagrid;
+ private IControllerRegistryService controllerRegistryService;
private IDeviceStorage deviceStorage;
private TopologyManager topologyService;
@@ -163,6 +165,7 @@
dependencies.add(IFlowService.class);
dependencies.add(IFlowPusherService.class);
dependencies.add(IOnosDeviceService.class);
+ dependencies.add(IControllerRegistryService.class);
// We don't use the IProxyArpService directly, but reactive forwarding
// requires it to be loaded and answering ARP requests
dependencies.add(IProxyArpService.class);
@@ -176,6 +179,7 @@
flowService = context.getServiceImpl(IFlowService.class);
flowPusher = context.getServiceImpl(IFlowPusherService.class);
datagrid = context.getServiceImpl(IDatagridService.class);
+ controllerRegistryService = context.getServiceImpl(IControllerRegistryService.class);
floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
@@ -411,7 +415,7 @@
flowPath.flowEntryMatch().enableEthernetFrameType(Ethernet.TYPE_IPv4);
flowPath.setDataPath(datapath);
- FlowId flowId = new FlowId(flowService.getNextFlowEntryId());
+ FlowId flowId = new FlowId(controllerRegistryService.getNextUniqueId());
flowPath.setFlowId(flowId);