Added a mechanism to configure the following timeout values.
Those are needed for the Internet2 deployment. * FlowRuleManager Timeout per Flow Operation (default to 500ms) This can be set in the following configuration file and variable: tools/package/etc/org.onosproject.net.flow.impl.FlowRuleManager.cfg timeoutPerFlowOpMsec = 500 * IntentManager Timeout per Intent Operation (default to 500ms) This can be set in the following configuration file and variable: tools/package/etc/org.onosproject.net.intent.impl.IntentManager.cfg timeoutPerIntentOpMsec = 500 For the Intentet2 deployment the above two parameters should be configured to 15000 (i.e., 15 seconds). The reason is because of recent modifications to the Internet2 firewall: it could take up to 10s for the reported FlowStats to reflect added FlowMods. Also, sometimes a single FlowAdd FLOW_MOD operation with the Internet2 firewall in the middle could take close to 1s to complete. * Added sample configuration files: tools/package/etc/samples/org.onosproject.*.cfg Those sample configuration files should be copied to the following directory so they can be used: tools/package/etc/ Change-Id: I699d93d4210fd7033234fd71be9b64c1f1d86117
Showing
6 changed files
with
150 additions
and
10 deletions
| ... | @@ -25,6 +25,8 @@ import com.google.common.collect.Sets; | ... | @@ -25,6 +25,8 @@ import com.google.common.collect.Sets; |
| 25 | import org.apache.felix.scr.annotations.Activate; | 25 | import org.apache.felix.scr.annotations.Activate; |
| 26 | import org.apache.felix.scr.annotations.Component; | 26 | import org.apache.felix.scr.annotations.Component; |
| 27 | import org.apache.felix.scr.annotations.Deactivate; | 27 | import org.apache.felix.scr.annotations.Deactivate; |
| 28 | +import org.apache.felix.scr.annotations.Modified; | ||
| 29 | +import org.apache.felix.scr.annotations.Property; | ||
| 28 | import org.apache.felix.scr.annotations.Reference; | 30 | import org.apache.felix.scr.annotations.Reference; |
| 29 | import org.apache.felix.scr.annotations.ReferenceCardinality; | 31 | import org.apache.felix.scr.annotations.ReferenceCardinality; |
| 30 | import org.apache.felix.scr.annotations.Service; | 32 | import org.apache.felix.scr.annotations.Service; |
| ... | @@ -53,8 +55,10 @@ import org.onosproject.net.flow.FlowRuleStore; | ... | @@ -53,8 +55,10 @@ import org.onosproject.net.flow.FlowRuleStore; |
| 53 | import org.onosproject.net.flow.FlowRuleStoreDelegate; | 55 | import org.onosproject.net.flow.FlowRuleStoreDelegate; |
| 54 | import org.onosproject.net.provider.AbstractProviderRegistry; | 56 | import org.onosproject.net.provider.AbstractProviderRegistry; |
| 55 | import org.onosproject.net.provider.AbstractProviderService; | 57 | import org.onosproject.net.provider.AbstractProviderService; |
| 58 | +import org.osgi.service.component.ComponentContext; | ||
| 56 | import org.slf4j.Logger; | 59 | import org.slf4j.Logger; |
| 57 | 60 | ||
| 61 | +import java.util.Dictionary; | ||
| 58 | import java.util.HashSet; | 62 | import java.util.HashSet; |
| 59 | import java.util.List; | 63 | import java.util.List; |
| 60 | import java.util.Map; | 64 | import java.util.Map; |
| ... | @@ -68,6 +72,7 @@ import java.util.concurrent.TimeUnit; | ... | @@ -68,6 +72,7 @@ import java.util.concurrent.TimeUnit; |
| 68 | import java.util.concurrent.TimeoutException; | 72 | import java.util.concurrent.TimeoutException; |
| 69 | import java.util.concurrent.atomic.AtomicReference; | 73 | import java.util.concurrent.atomic.AtomicReference; |
| 70 | import static com.google.common.base.Preconditions.checkNotNull; | 74 | import static com.google.common.base.Preconditions.checkNotNull; |
| 75 | +import static com.google.common.base.Strings.isNullOrEmpty; | ||
| 71 | import static org.onlab.util.Tools.namedThreads; | 76 | import static org.onlab.util.Tools.namedThreads; |
| 72 | import static org.slf4j.LoggerFactory.getLogger; | 77 | import static org.slf4j.LoggerFactory.getLogger; |
| 73 | 78 | ||
| ... | @@ -82,6 +87,8 @@ public class FlowRuleManager | ... | @@ -82,6 +87,8 @@ public class FlowRuleManager |
| 82 | 87 | ||
| 83 | enum BatchState { STARTED, FINISHED, CANCELLED }; | 88 | enum BatchState { STARTED, FINISHED, CANCELLED }; |
| 84 | 89 | ||
| 90 | + private static final int DEFAULT_TIMEOUT_PER_FLOW_OP_MSEC = 500; // ms | ||
| 91 | + | ||
| 85 | public static final String FLOW_RULE_NULL = "FlowRule cannot be null"; | 92 | public static final String FLOW_RULE_NULL = "FlowRule cannot be null"; |
| 86 | private final Logger log = getLogger(getClass()); | 93 | private final Logger log = getLogger(getClass()); |
| 87 | 94 | ||
| ... | @@ -101,8 +108,16 @@ public class FlowRuleManager | ... | @@ -101,8 +108,16 @@ public class FlowRuleManager |
| 101 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 108 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| 102 | protected DeviceService deviceService; | 109 | protected DeviceService deviceService; |
| 103 | 110 | ||
| 111 | + @Property(name = "timeoutPerFlowOpMsec", intValue = DEFAULT_TIMEOUT_PER_FLOW_OP_MSEC, | ||
| 112 | + label = "Configure Timeout per Flow Operation; " + | ||
| 113 | + "default is 500 msec") | ||
| 114 | + private int timeoutPerFlowOpMsec = DEFAULT_TIMEOUT_PER_FLOW_OP_MSEC; | ||
| 115 | + | ||
| 116 | + | ||
| 104 | @Activate | 117 | @Activate |
| 105 | - public void activate() { | 118 | + public void activate(ComponentContext context) { |
| 119 | + readComponentConfiguration(context); | ||
| 120 | + | ||
| 106 | futureService = | 121 | futureService = |
| 107 | Executors.newFixedThreadPool(32, namedThreads("provider-future-listeners-%d")); | 122 | Executors.newFixedThreadPool(32, namedThreads("provider-future-listeners-%d")); |
| 108 | store.setDelegate(delegate); | 123 | store.setDelegate(delegate); |
| ... | @@ -119,6 +134,54 @@ public class FlowRuleManager | ... | @@ -119,6 +134,54 @@ public class FlowRuleManager |
| 119 | log.info("Stopped"); | 134 | log.info("Stopped"); |
| 120 | } | 135 | } |
| 121 | 136 | ||
| 137 | + @Modified | ||
| 138 | + public void modified(ComponentContext context) { | ||
| 139 | + readComponentConfiguration(context); | ||
| 140 | + } | ||
| 141 | + | ||
| 142 | + /** | ||
| 143 | + * Extracts properties from the component configuration context. | ||
| 144 | + * | ||
| 145 | + * @param context the component context | ||
| 146 | + */ | ||
| 147 | + private void readComponentConfiguration(ComponentContext context) { | ||
| 148 | + if (context == null) { | ||
| 149 | + return; // Nothing to do | ||
| 150 | + } | ||
| 151 | + Dictionary<?, ?> properties = context.getProperties(); | ||
| 152 | + | ||
| 153 | + Integer timeoutPerFlowOpMsecConfigured = | ||
| 154 | + getIntegerProperty(properties, "timeoutPerFlowOpMsec"); | ||
| 155 | + if (timeoutPerFlowOpMsecConfigured == null) { | ||
| 156 | + log.info("Timeout per Flow Operation is not configured, " + | ||
| 157 | + "using current value of {} ms", timeoutPerFlowOpMsec); | ||
| 158 | + } else { | ||
| 159 | + timeoutPerFlowOpMsec = timeoutPerFlowOpMsecConfigured; | ||
| 160 | + log.info("Configured. Timeout per Flow Operation is " + | ||
| 161 | + "configured to {} ms", timeoutPerFlowOpMsec); | ||
| 162 | + } | ||
| 163 | + } | ||
| 164 | + | ||
| 165 | + /** | ||
| 166 | + * Get Integer property from the propertyName | ||
| 167 | + * Return null if propertyName is not found. | ||
| 168 | + * | ||
| 169 | + * @param properties properties to be looked up | ||
| 170 | + * @param propertyName the name of the property to look up | ||
| 171 | + * @return value when the propertyName is defined or return null | ||
| 172 | + */ | ||
| 173 | + private static Integer getIntegerProperty(Dictionary<?, ?> properties, | ||
| 174 | + String propertyName) { | ||
| 175 | + Integer value = null; | ||
| 176 | + try { | ||
| 177 | + String s = (String) properties.get(propertyName); | ||
| 178 | + value = isNullOrEmpty(s) ? null : Integer.parseInt(s.trim()); | ||
| 179 | + } catch (NumberFormatException | ClassCastException e) { | ||
| 180 | + value = null; | ||
| 181 | + } | ||
| 182 | + return value; | ||
| 183 | + } | ||
| 184 | + | ||
| 122 | @Override | 185 | @Override |
| 123 | public int getFlowRuleCount() { | 186 | public int getFlowRuleCount() { |
| 124 | return store.getFlowRuleCount(); | 187 | return store.getFlowRuleCount(); |
| ... | @@ -378,9 +441,6 @@ public class FlowRuleManager | ... | @@ -378,9 +441,6 @@ public class FlowRuleManager |
| 378 | // Store delegate to re-post events emitted from the store. | 441 | // Store delegate to re-post events emitted from the store. |
| 379 | private class InternalStoreDelegate implements FlowRuleStoreDelegate { | 442 | private class InternalStoreDelegate implements FlowRuleStoreDelegate { |
| 380 | 443 | ||
| 381 | - // FIXME set appropriate default and make it configurable | ||
| 382 | - private static final int TIMEOUT_PER_OP = 500; // ms | ||
| 383 | - | ||
| 384 | // TODO: Right now we only dispatch events at individual flowEntry level. | 444 | // TODO: Right now we only dispatch events at individual flowEntry level. |
| 385 | // It may be more efficient for also dispatch events as a batch. | 445 | // It may be more efficient for also dispatch events as a batch. |
| 386 | @Override | 446 | @Override |
| ... | @@ -408,7 +468,8 @@ public class FlowRuleManager | ... | @@ -408,7 +468,8 @@ public class FlowRuleManager |
| 408 | public void run() { | 468 | public void run() { |
| 409 | CompletedBatchOperation res; | 469 | CompletedBatchOperation res; |
| 410 | try { | 470 | try { |
| 411 | - res = result.get(TIMEOUT_PER_OP * batchOperation.size(), TimeUnit.MILLISECONDS); | 471 | + res = result.get(timeoutPerFlowOpMsec * batchOperation.size(), |
| 472 | + TimeUnit.MILLISECONDS); | ||
| 412 | store.batchOperationComplete(FlowRuleBatchEvent.completed(request, res)); | 473 | store.batchOperationComplete(FlowRuleBatchEvent.completed(request, res)); |
| 413 | } catch (TimeoutException | InterruptedException | ExecutionException e) { | 474 | } catch (TimeoutException | InterruptedException | ExecutionException e) { |
| 414 | log.warn("Something went wrong with the batch operation {}", | 475 | log.warn("Something went wrong with the batch operation {}", | ... | ... |
| ... | @@ -22,6 +22,8 @@ import com.google.common.collect.Maps; | ... | @@ -22,6 +22,8 @@ import com.google.common.collect.Maps; |
| 22 | import org.apache.felix.scr.annotations.Activate; | 22 | import org.apache.felix.scr.annotations.Activate; |
| 23 | import org.apache.felix.scr.annotations.Component; | 23 | import org.apache.felix.scr.annotations.Component; |
| 24 | import org.apache.felix.scr.annotations.Deactivate; | 24 | import org.apache.felix.scr.annotations.Deactivate; |
| 25 | +import org.apache.felix.scr.annotations.Modified; | ||
| 26 | +import org.apache.felix.scr.annotations.Property; | ||
| 25 | import org.apache.felix.scr.annotations.Reference; | 27 | import org.apache.felix.scr.annotations.Reference; |
| 26 | import org.apache.felix.scr.annotations.ReferenceCardinality; | 28 | import org.apache.felix.scr.annotations.ReferenceCardinality; |
| 27 | import org.apache.felix.scr.annotations.Service; | 29 | import org.apache.felix.scr.annotations.Service; |
| ... | @@ -50,10 +52,12 @@ import org.onosproject.net.intent.IntentState; | ... | @@ -50,10 +52,12 @@ import org.onosproject.net.intent.IntentState; |
| 50 | import org.onosproject.net.intent.IntentStore; | 52 | import org.onosproject.net.intent.IntentStore; |
| 51 | import org.onosproject.net.intent.BatchWrite; | 53 | import org.onosproject.net.intent.BatchWrite; |
| 52 | import org.onosproject.net.intent.IntentStoreDelegate; | 54 | import org.onosproject.net.intent.IntentStoreDelegate; |
| 55 | +import org.osgi.service.component.ComponentContext; | ||
| 53 | import org.slf4j.Logger; | 56 | import org.slf4j.Logger; |
| 54 | 57 | ||
| 55 | import java.util.ArrayList; | 58 | import java.util.ArrayList; |
| 56 | import java.util.Collections; | 59 | import java.util.Collections; |
| 60 | +import java.util.Dictionary; | ||
| 57 | import java.util.EnumSet; | 61 | import java.util.EnumSet; |
| 58 | import java.util.List; | 62 | import java.util.List; |
| 59 | import java.util.Map; | 63 | import java.util.Map; |
| ... | @@ -67,6 +71,7 @@ import java.util.concurrent.TimeoutException; | ... | @@ -67,6 +71,7 @@ import java.util.concurrent.TimeoutException; |
| 67 | 71 | ||
| 68 | import static com.google.common.base.Preconditions.checkArgument; | 72 | import static com.google.common.base.Preconditions.checkArgument; |
| 69 | import static com.google.common.base.Preconditions.checkNotNull; | 73 | import static com.google.common.base.Preconditions.checkNotNull; |
| 74 | +import static com.google.common.base.Strings.isNullOrEmpty; | ||
| 70 | import static java.util.concurrent.Executors.newFixedThreadPool; | 75 | import static java.util.concurrent.Executors.newFixedThreadPool; |
| 71 | import static org.onosproject.net.intent.IntentState.*; | 76 | import static org.onosproject.net.intent.IntentState.*; |
| 72 | import static org.onlab.util.Tools.namedThreads; | 77 | import static org.onlab.util.Tools.namedThreads; |
| ... | @@ -79,6 +84,9 @@ import static org.slf4j.LoggerFactory.getLogger; | ... | @@ -79,6 +84,9 @@ import static org.slf4j.LoggerFactory.getLogger; |
| 79 | @Service | 84 | @Service |
| 80 | public class IntentManager | 85 | public class IntentManager |
| 81 | implements IntentService, IntentExtensionService { | 86 | implements IntentService, IntentExtensionService { |
| 87 | + | ||
| 88 | + private static final int DEFAULT_TIMEOUT_PER_INTENT_OP_MSEC = 500; // ms | ||
| 89 | + | ||
| 82 | private static final Logger log = getLogger(IntentManager.class); | 90 | private static final Logger log = getLogger(IntentManager.class); |
| 83 | 91 | ||
| 84 | public static final String INTENT_NULL = "Intent cannot be null"; | 92 | public static final String INTENT_NULL = "Intent cannot be null"; |
| ... | @@ -119,6 +127,10 @@ public class IntentManager | ... | @@ -119,6 +127,10 @@ public class IntentManager |
| 119 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 127 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| 120 | protected FlowRuleService flowRuleService; | 128 | protected FlowRuleService flowRuleService; |
| 121 | 129 | ||
| 130 | + @Property(name = "timeoutPerIntentOpMsec", intValue = DEFAULT_TIMEOUT_PER_INTENT_OP_MSEC, | ||
| 131 | + label = "Configure Timeout per Intent Operation; " + | ||
| 132 | + "default is 500 msec") | ||
| 133 | + private int timeoutPerIntentOpMsec = DEFAULT_TIMEOUT_PER_INTENT_OP_MSEC; | ||
| 122 | 134 | ||
| 123 | private ExecutorService executor; | 135 | private ExecutorService executor; |
| 124 | 136 | ||
| ... | @@ -128,7 +140,9 @@ public class IntentManager | ... | @@ -128,7 +140,9 @@ public class IntentManager |
| 128 | private IdGenerator idGenerator; | 140 | private IdGenerator idGenerator; |
| 129 | 141 | ||
| 130 | @Activate | 142 | @Activate |
| 131 | - public void activate() { | 143 | + public void activate(ComponentContext context) { |
| 144 | + readComponentConfiguration(context); | ||
| 145 | + | ||
| 132 | store.setDelegate(delegate); | 146 | store.setDelegate(delegate); |
| 133 | trackerService.setDelegate(topoDelegate); | 147 | trackerService.setDelegate(topoDelegate); |
| 134 | batchService.setDelegate(batchDelegate); | 148 | batchService.setDelegate(batchDelegate); |
| ... | @@ -150,6 +164,54 @@ public class IntentManager | ... | @@ -150,6 +164,54 @@ public class IntentManager |
| 150 | log.info("Stopped"); | 164 | log.info("Stopped"); |
| 151 | } | 165 | } |
| 152 | 166 | ||
| 167 | + @Modified | ||
| 168 | + public void modified(ComponentContext context) { | ||
| 169 | + readComponentConfiguration(context); | ||
| 170 | + } | ||
| 171 | + | ||
| 172 | + /** | ||
| 173 | + * Extracts properties from the component configuration context. | ||
| 174 | + * | ||
| 175 | + * @param context the component context | ||
| 176 | + */ | ||
| 177 | + private void readComponentConfiguration(ComponentContext context) { | ||
| 178 | + if (context == null) { | ||
| 179 | + return; // Nothing to do | ||
| 180 | + } | ||
| 181 | + Dictionary<?, ?> properties = context.getProperties(); | ||
| 182 | + | ||
| 183 | + Integer timeoutPerIntentOpMsecConfigured = | ||
| 184 | + getIntegerProperty(properties, "timeoutPerIntentOpMsec"); | ||
| 185 | + if (timeoutPerIntentOpMsecConfigured == null) { | ||
| 186 | + log.info("Timeout per Intent Operation is not configured, " + | ||
| 187 | + "using current value of {} ms", timeoutPerIntentOpMsec); | ||
| 188 | + } else { | ||
| 189 | + timeoutPerIntentOpMsec = timeoutPerIntentOpMsecConfigured; | ||
| 190 | + log.info("Configured. Timeout per Intent Operation is " + | ||
| 191 | + "configured to {} ms", timeoutPerIntentOpMsec); | ||
| 192 | + } | ||
| 193 | + } | ||
| 194 | + | ||
| 195 | + /** | ||
| 196 | + * Get Integer property from the propertyName | ||
| 197 | + * Return null if propertyName is not found. | ||
| 198 | + * | ||
| 199 | + * @param properties properties to be looked up | ||
| 200 | + * @param propertyName the name of the property to look up | ||
| 201 | + * @return value when the propertyName is defined or return null | ||
| 202 | + */ | ||
| 203 | + private static Integer getIntegerProperty(Dictionary<?, ?> properties, | ||
| 204 | + String propertyName) { | ||
| 205 | + Integer value = null; | ||
| 206 | + try { | ||
| 207 | + String s = (String) properties.get(propertyName); | ||
| 208 | + value = isNullOrEmpty(s) ? null : Integer.parseInt(s.trim()); | ||
| 209 | + } catch (NumberFormatException | ClassCastException e) { | ||
| 210 | + value = null; | ||
| 211 | + } | ||
| 212 | + return value; | ||
| 213 | + } | ||
| 214 | + | ||
| 153 | @Override | 215 | @Override |
| 154 | public void submit(Intent intent) { | 216 | public void submit(Intent intent) { |
| 155 | checkNotNull(intent, INTENT_NULL); | 217 | checkNotNull(intent, INTENT_NULL); |
| ... | @@ -744,7 +806,6 @@ public class IntentManager | ... | @@ -744,7 +806,6 @@ public class IntentManager |
| 744 | private class IntentInstallMonitor implements Runnable { | 806 | private class IntentInstallMonitor implements Runnable { |
| 745 | 807 | ||
| 746 | // TODO make this configurable | 808 | // TODO make this configurable |
| 747 | - private static final int TIMEOUT_PER_OP = 500; // ms | ||
| 748 | private static final int MAX_ATTEMPTS = 3; | 809 | private static final int MAX_ATTEMPTS = 3; |
| 749 | 810 | ||
| 750 | private final IntentOperations ops; | 811 | private final IntentOperations ops; |
| ... | @@ -764,7 +825,7 @@ public class IntentManager | ... | @@ -764,7 +825,7 @@ public class IntentManager |
| 764 | private void resetTimeoutLimit() { | 825 | private void resetTimeoutLimit() { |
| 765 | // FIXME compute reasonable timeouts | 826 | // FIXME compute reasonable timeouts |
| 766 | this.endTime = System.currentTimeMillis() | 827 | this.endTime = System.currentTimeMillis() |
| 767 | - + ops.operations().size() * TIMEOUT_PER_OP; | 828 | + + ops.operations().size() * timeoutPerIntentOpMsec; |
| 768 | } | 829 | } |
| 769 | 830 | ||
| 770 | private void buildIntentUpdates() { | 831 | private void buildIntentUpdates() { | ... | ... |
| ... | @@ -106,7 +106,7 @@ public class FlowRuleManagerTest { | ... | @@ -106,7 +106,7 @@ public class FlowRuleManagerTest { |
| 106 | service = mgr; | 106 | service = mgr; |
| 107 | registry = mgr; | 107 | registry = mgr; |
| 108 | 108 | ||
| 109 | - mgr.activate(); | 109 | + mgr.activate(null); |
| 110 | mgr.addListener(listener); | 110 | mgr.addListener(listener); |
| 111 | provider = new TestProvider(PID); | 111 | provider = new TestProvider(PID); |
| 112 | providerService = registry.register(provider); | 112 | providerService = registry.register(provider); | ... | ... |
| ... | @@ -285,7 +285,7 @@ public class IntentManagerTest { | ... | @@ -285,7 +285,7 @@ public class IntentManagerTest { |
| 285 | service = manager; | 285 | service = manager; |
| 286 | extensionService = manager; | 286 | extensionService = manager; |
| 287 | 287 | ||
| 288 | - manager.activate(); | 288 | + manager.activate(null); |
| 289 | service.addListener(listener); | 289 | service.addListener(listener); |
| 290 | extensionService.registerCompiler(MockIntent.class, compiler); | 290 | extensionService.registerCompiler(MockIntent.class, compiler); |
| 291 | extensionService.registerInstaller(MockInstallableIntent.class, installer); | 291 | extensionService.registerInstaller(MockInstallableIntent.class, installer); | ... | ... |
-
Please register or login to post a comment