From: Alessandro Boch Date: Thu, 8 Aug 2013 17:28:11 +0000 (+0000) Subject: Merge "Fix race condition when registering services" X-Git-Tag: releasepom-0.1.0~218 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=89f53da1dd72537642e2901ffb3be57cd28b1397;hp=0d1d688ac55acc55c3909c52c2cdb940cfb3764f;p=controller.git Merge "Fix race condition when registering services" --- diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java index 1ac7cb2f51..5db7d32745 100644 --- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java +++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java @@ -647,7 +647,7 @@ public class FlowConfig implements Serializable { public boolean isTpPortValid(String tpPort) { int port = Integer.decode(tpPort); - return ((port > 0) && (port <= 0xffff)); + return ((port >= 0) && (port <= 0xffff)); } public boolean isTimeoutValid(String timeout) { diff --git a/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java b/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java index fc22ee7ddd..176d8e9ebd 100644 --- a/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java +++ b/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java @@ -611,9 +611,15 @@ public class frmTest { Assert.assertFalse(status.isSuccess()); Assert.assertTrue(status.getDescription().contains("Transport source port")); + fc.setSrcPort("0"); + Assert.assertTrue(fc.validate(null).isSuccess()); + fc.setSrcPort("0x00ff"); Assert.assertTrue(fc.validate(null).isSuccess()); + fc.setSrcPort("0xffff"); + Assert.assertTrue(fc.validate(null).isSuccess()); + fc.setDstPort("-1"); status = fc.validate(null); Assert.assertFalse(status.isSuccess()); @@ -624,9 +630,15 @@ public class frmTest { Assert.assertFalse(status.isSuccess()); Assert.assertTrue(status.getDescription().contains("Transport destination port")); + fc.setDstPort("0"); + Assert.assertTrue(fc.validate(null).isSuccess()); + fc.setDstPort("0x00ff"); Assert.assertTrue(fc.validate(null).isSuccess()); + fc.setDstPort("0xffff"); + Assert.assertTrue(fc.validate(null).isSuccess()); + fc.setSrcMac("abc"); status = fc.validate(null); Assert.assertFalse(status.isSuccess()); diff --git a/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java b/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java index e3c2189bf9..17780fc70d 100644 --- a/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java +++ b/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java @@ -7,9 +7,7 @@ */ package org.opendaylight.controller.subnets.northbound; -import java.util.ArrayList; import java.util.HashSet; -import java.util.List; import java.util.Set; import javax.ws.rs.Consumes; @@ -159,7 +157,7 @@ public class SubnetsNorthboundJAXRS { throw new ResourceNotFoundException(RestMessages.NOCONTAINER.toString()); } - SubnetConfig cfgObject = new SubnetConfig(subnetName, subnet, new ArrayList(0)); + SubnetConfig cfgObject = new SubnetConfig(subnetName, subnet, new HashSet(0)); Status status = switchManager.addSubnet(cfgObject); if (status.isSuccess()) { return Response.status(Response.Status.CREATED).build(); @@ -387,7 +385,7 @@ public class SubnetsNorthboundJAXRS { } else { Status st; boolean successful = true; - List ports = subnetConf.getNodePorts(); + Set ports = subnetConf.getNodePorts(); if (action.equals("add")) { // add new ports diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/MatchType.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/MatchType.java index 1c964267b1..f4c8f4ae80 100644 --- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/MatchType.java +++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/MatchType.java @@ -34,8 +34,8 @@ public enum MatchType { NW_PROTO("nwProto", 1 << 9, Byte.class, 0, 0xff), // 1 byte NW_SRC("nwSrc", 1 << 10, InetAddress.class, 0, 0), NW_DST("nwDst", 1 << 11, InetAddress.class, 0, 0), - TP_SRC("tpSrc", 1 << 12, Short.class, 1, 0xffff), // 2 bytes - TP_DST("tpDst", 1 << 13, Short.class, 1, 0xffff); // 2 bytes + TP_SRC("tpSrc", 1 << 12, Short.class, 0, 0xffff), // 2 bytes + TP_DST("tpDst", 1 << 13, Short.class, 0, 0xffff); // 2 bytes // Used to indicate that no VLAN ID is set. public static final short DL_VLAN_NONE = (short) 0; diff --git a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java index 55b5cabf11..ae78e1acc8 100644 --- a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java +++ b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java @@ -137,6 +137,44 @@ public class MatchTest { Assert.assertTrue(o.equals(proto)); } + @Test + public void testSetTpSrc() { + // Minimum value validation. + Match match = new Match(); + short tp_src = 0; + match.setField(MatchType.TP_SRC, tp_src); + + Object o = match.getField(MatchType.TP_SRC).getValue(); + Assert.assertTrue(o.equals(tp_src)); + + // Maximum value validation. + match = new Match(); + tp_src = (short) 0xffff; + match.setField(MatchType.TP_SRC, tp_src); + + o = match.getField(MatchType.TP_SRC).getValue(); + Assert.assertTrue(o.equals(tp_src)); + } + + @Test + public void testSetTpDst() { + // Minimum value validation. + Match match = new Match(); + short tp_dst = 0; + match.setField(MatchType.TP_DST, tp_dst); + + Object o = match.getField(MatchType.TP_DST).getValue(); + Assert.assertTrue(o.equals(tp_dst)); + + // Maximum value validation. + match = new Match(); + tp_dst = (short) 0xffff; + match.setField(MatchType.TP_DST, tp_dst); + + o = match.getField(MatchType.TP_DST).getValue(); + Assert.assertTrue(o.equals(tp_dst)); + } + @Test public void testMatchMask() { Match x = new Match(); diff --git a/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java b/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java index eec183d595..57dfa91b96 100644 --- a/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java +++ b/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java @@ -170,13 +170,25 @@ public class StatisticsManager implements IStatisticsManager, IReadServiceListen // Retrieve current statistics so we don't have to wait for next refresh ISwitchManager switchManager = (ISwitchManager) ServiceHelper.getInstance( ISwitchManager.class, container.getName(), this); - if (reader != null && switchManager != null) { + if ((reader != null) && (switchManager != null)) { Set nodeSet = switchManager.getNodes(); for (Node node : nodeSet) { - flowStatistics.put(node, reader.readAllFlows(node)); - descriptionStatistics.put(node, reader.readDescription(node)); - tableStatistics.put(node, reader.readNodeTable(node)); - nodeConnectorStatistics.put(node, reader.readNodeConnectors(node)); + List flows = reader.readAllFlows(node); + if (flows != null) { + flowStatistics.put(node, flows); + } + NodeDescription descr = reader.readDescription(node); + if (descr != null) { + descriptionStatistics.put(node, descr); + } + List tableStats = reader.readNodeTable(node); + if (tableStats != null) { + tableStatistics.put(node, tableStats); + } + List ncStats = reader.readNodeConnectors(node); + if (ncStats != null) { + nodeConnectorStatistics.put(node, ncStats); + } } } else { diff --git a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java index 6a8eea2d0c..55a7ecb9cd 100644 --- a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java +++ b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java @@ -56,12 +56,8 @@ public class Subnet implements Cloneable, Serializable { * @param sp Set of NodeConnectors to add to the subnet */ public void addNodeConnectors(Set sp) { - if (sp == null) { - return; - } - - for (NodeConnector p : sp) { - this.nodeConnectors.add(p); + if (sp != null) { + this.nodeConnectors.addAll(sp); } } diff --git a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java index 52b5f5255d..4ed9934b27 100644 --- a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java +++ b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java @@ -48,13 +48,13 @@ public class SubnetConfig implements Cloneable, Serializable { private String subnet; // A.B.C.D/MM Where A.B.C.D is the Default // Gateway IP (L3) or ARP Querier IP (L2 @XmlElement - private List nodePorts; // datapath ID/port list: + private Set nodePorts; // datapath ID/port list: // xx:xx:xx:xx:xx:xx:xx:xx/a,b,c-m,r-t,y public SubnetConfig() { } - public SubnetConfig(String desc, String sub, List sp) { + public SubnetConfig(String desc, String sub, Set sp) { name = desc; subnet = sub; nodePorts = sp; @@ -63,14 +63,14 @@ public class SubnetConfig implements Cloneable, Serializable { public SubnetConfig(SubnetConfig subnetConfig) { name = subnetConfig.name; subnet = subnetConfig.subnet; - nodePorts = new ArrayList(subnetConfig.nodePorts); + nodePorts = new HashSet(subnetConfig.nodePorts); } public String getName() { return name; } - public List getNodePorts() { + public Set getNodePorts() { return nodePorts; } diff --git a/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java b/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java index f50f303a3f..4c4adf0478 100644 --- a/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java +++ b/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java @@ -92,7 +92,7 @@ CommandProvider { private static final String SAVE = "Save"; private String subnetFileName, spanFileName, switchConfigFileName; private final List spanNodeConnectors = new CopyOnWriteArrayList(); - // set of Subnets keyed by the InetAddress + // Collection of Subnets keyed by the InetAddress private ConcurrentMap subnets; private ConcurrentMap subnetsConfigList; private ConcurrentMap spanConfigList; @@ -401,19 +401,21 @@ CommandProvider { Set sp = conf.getSubnetNodeConnectors(); subnet.addNodeConnectors(sp); } - boolean result = false; + boolean putNewSubnet = false; if(subnetCurr == null) { if(subnets.putIfAbsent(conf.getIPnum(), subnet) == null) { - result = true; + putNewSubnet = true; } } else { - result = subnets.replace(conf.getIPnum(), subnetCurr, subnet); + putNewSubnet = subnets.replace(conf.getIPnum(), subnetCurr, subnet); } - if(!result) { + if(!putNewSubnet) { String msg = "Cluster conflict: Conflict while adding the subnet " + conf.getIPnum(); return new Status(StatusCode.CONFLICT, msg); } - } else { // This is the deletion of the whole subnet + + // Subnet removal case + } else { subnets.remove(conf.getIPnum()); } return new Status(StatusCode.SUCCESS); @@ -435,7 +437,7 @@ CommandProvider { return new Status(StatusCode.SUCCESS); } - private Status addRemoveSubnet(SubnetConfig conf, boolean add) { + private Status addRemoveSubnet(SubnetConfig conf, boolean isAdding) { // Valid config check if (!conf.isValidConfig()) { String msg = "Invalid Subnet configuration"; @@ -443,13 +445,13 @@ CommandProvider { return new Status(StatusCode.BADREQUEST, msg); } - if (add) { + if (isAdding) { // Presence check if (subnetsConfigList.containsKey(conf.getName())) { return new Status(StatusCode.CONFLICT, - "Same subnet config already exists"); + "Subnet with the specified name already configured."); } - // Semantyc check + // Semantic check Status rc = semanticCheck(conf); if (!rc.isSuccess()) { return rc; @@ -457,13 +459,13 @@ CommandProvider { } // Update Database - Status rc = updateDatabase(conf, add); + Status rc = updateDatabase(conf, isAdding); if (rc.isSuccess()) { // Update Configuration - rc = updateConfig(conf, add); + rc = updateConfig(conf, isAdding); if(!rc.isSuccess()) { - updateDatabase(conf, (!add)); + updateDatabase(conf, (!isAdding)); } } @@ -512,8 +514,8 @@ CommandProvider { Subnet sub = subCurr.clone(); Set sp = confCurr.getNodeConnectors(switchPorts); sub.addNodeConnectors(sp); - boolean subnetsReplace = subnets.replace(confCurr.getIPnum(), subCurr, sub); - if (!subnetsReplace) { + boolean subnetsReplaced = subnets.replace(confCurr.getIPnum(), subCurr, sub); + if (!subnetsReplaced) { String msg = "Cluster conflict: Conflict while adding ports to the subnet " + name; return new Status(StatusCode.CONFLICT, msg); } @@ -521,8 +523,8 @@ CommandProvider { // Update Configuration SubnetConfig conf = confCurr.clone(); conf.addNodeConnectors(switchPorts); - boolean result = subnetsConfigList.replace(name, confCurr, conf); - if (!result) { + boolean configReplaced = subnetsConfigList.replace(name, confCurr, conf); + if (!configReplaced) { // TODO: recovery using Transactionality String msg = "Cluster conflict: Conflict while adding ports to the subnet " + name; return new Status(StatusCode.CONFLICT, msg); diff --git a/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java b/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java index a233ad4b3e..d74c3d3630 100644 --- a/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java +++ b/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.switchmanager.internal; -import java.util.ArrayList; import java.util.HashSet; import java.util.Set; @@ -33,7 +32,7 @@ public class SwitchManagerTest { SwitchManager switchmgr = new SwitchManager(); switchmgr.startUp(); - ArrayList portList = new ArrayList(); + Set portList = new HashSet(); portList.add("1/1"); portList.add("1/2"); portList.add("1/3"); diff --git a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java index ca1dd97137..37798ac137 100644 --- a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java +++ b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.devices.web; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -438,7 +439,7 @@ public class Devices implements IDaylightWeb { ISwitchManager switchManager = (ISwitchManager) ServiceHelper .getInstance(ISwitchManager.class, containerName, this); SubnetConfig cfgObject = new SubnetConfig(gatewayName, - gatewayIPAddress, new ArrayList()); + gatewayIPAddress, new HashSet()); Status result = switchManager.addSubnet(cfgObject); if (result.isSuccess()) { resultBean.setStatus(true); diff --git a/opendaylight/web/flows/src/main/resources/js/page.js b/opendaylight/web/flows/src/main/resources/js/page.js index 59557dccd2..7ff54ed4dc 100644 --- a/opendaylight/web/flows/src/main/resources/js/page.js +++ b/opendaylight/web/flows/src/main/resources/js/page.js @@ -715,13 +715,13 @@ one.f.flows = { var $label = one.lib.form.label("Source Port"); var $input = one.lib.form.input("Source Port"); $input.attr('id', one.f.flows.id.modal.form.srcPort); - var $help = one.lib.form.help("Range: 1 - 65535"); + var $help = one.lib.form.help("Range: 0 - 65535"); $fieldset.append($label).append($input).append($help); // dstPort var $label = one.lib.form.label("Destination Port"); var $input = one.lib.form.input("Destination Port"); $input.attr('id', one.f.flows.id.modal.form.dstPort); - var $help = one.lib.form.help("Range: 1 - 65535"); + var $help = one.lib.form.help("Range: 0 - 65535"); $fieldset.append($label).append($input).append($help); // protocol var $label = one.lib.form.label("Protocol");