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);
diff --git a/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java b/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java
index eb27cf9..41f7d55 100755
--- a/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java
+++ b/src/main/java/net/onrc/onos/registry/controller/IControllerRegistryService.java
@@ -135,5 +135,12 @@
* Get next unique id and retrieve a new range of ids if needed.
*/
public IdBlock allocateUniqueIdBlock(long range);
-
+
+
+ /**
+ * Get a globally unique ID.
+ *
+ * @return a globally unique ID.
+ */
+ public long getNextUniqueId();
}
diff --git a/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java b/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java
index 1e2934b..d581590 100755
--- a/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java
+++ b/src/main/java/net/onrc/onos/registry/controller/StandaloneRegistry.java
@@ -5,6 +5,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicLong;
import net.floodlightcontroller.core.IFloodlightProviderService;
import net.floodlightcontroller.core.module.FloodlightModuleContext;
@@ -33,7 +34,11 @@
protected String controllerId = null;
protected Map<String, ControlChangeCallback> switchCallbacks;
-
+
+ //
+ // Unique ID generation state
+ //
+ private static AtomicLong nextUniqueId = new AtomicLong(0);
@Override
public void requestControl(long dpid, ControlChangeCallback cb)
@@ -141,6 +146,16 @@
return block;
}
+ /**
+ * Get a globally unique ID.
+ *
+ * @return a globally unique ID.
+ */
+ @Override
+ public long getNextUniqueId() {
+ return nextUniqueId.incrementAndGet();
+ }
+
@Override
public Collection<Class<? extends IFloodlightService>> getModuleServices() {
Collection<Class<? extends IFloodlightService>> l =
diff --git a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
index b8a1021..ef45c64 100755
--- a/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
+++ b/src/main/java/net/onrc/onos/registry/controller/ZookeeperRegistry.java
@@ -8,6 +8,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Random;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
@@ -88,7 +89,16 @@
//Zookeeper performance-related configuration
protected static final int sessionTimeout = 5000;
protected static final int connectionTimeout = 7000;
-
+
+ //
+ // Unique ID generation state
+ // TODO: The implementation must be updated to use the Zookeeper
+ // instead of a ramdon generator.
+ //
+ private static Random randomGenerator = new Random();
+ private static int nextUniqueIdPrefix = 0;
+ private static int nextUniqueIdSuffix = 0;
+
private final BlockingQueue<SwitchLeaderEvent> switchLeadershipEvents =
new LinkedBlockingQueue<SwitchLeaderEvent>();
@@ -474,7 +484,33 @@
public IdBlock allocateUniqueIdBlock(){
return allocateUniqueIdBlock(ID_BLOCK_SIZE);
}
-
+
+ /**
+ * Get a globally unique ID.
+ *
+ * @return a globally unique ID.
+ */
+ @Override
+ public synchronized long getNextUniqueId() {
+ //
+ // Generate the next Unique ID.
+ //
+ // TODO: For now, the higher 32 bits are random, and
+ // the lower 32 bits are sequential.
+ // The implementation must be updated to use the Zookeeper
+ // to allocate the higher 32 bits (globally unique).
+ //
+ if ((nextUniqueIdSuffix & 0xffffffffL) == 0xffffffffL) {
+ nextUniqueIdPrefix = randomGenerator.nextInt();
+ nextUniqueIdSuffix = 0;
+ } else {
+ nextUniqueIdSuffix++;
+ }
+ long result = (long)nextUniqueIdPrefix << 32;
+ result = result | (0xffffffffL & nextUniqueIdSuffix);
+ return result;
+ }
+
/*
* IFloodlightModule
*/
@@ -517,7 +553,13 @@
this.connectionString = connectionString;
}
log.info("Setting Zookeeper connection string to {}", this.connectionString);
-
+
+ //
+ // Initialize the Unique ID generator
+ // TODO: This must be replaced by Zookeeper-based allocation
+ //
+ nextUniqueIdPrefix = randomGenerator.nextInt();
+
restApi = context.getServiceImpl(IRestApiService.class);
switches = new ConcurrentHashMap<String, SwitchLeadershipData>();