Jonathan Hart
Committed by Gerrit Code Review

Compute the flow ID at build time rather than when fromApp() is called.

Depending on the order that the application calls the builder methods,
there may not be any selector, priority, etc. added at the time fromApp() is
called. This means there's no entropy to the hash function so all flows end
up with the same flow ID.

Change-Id: I9badedc343fdf6f3ee7f73547955cd84302d9226
...@@ -291,6 +291,7 @@ public class DefaultFlowRule implements FlowRule { ...@@ -291,6 +291,7 @@ public class DefaultFlowRule implements FlowRule {
291 public static final class Builder implements FlowRule.Builder { 291 public static final class Builder implements FlowRule.Builder {
292 292
293 private FlowId flowId; 293 private FlowId flowId;
294 + private ApplicationId appId;
294 private Integer priority; 295 private Integer priority;
295 private DeviceId deviceId; 296 private DeviceId deviceId;
296 private Integer tableId = 0; 297 private Integer tableId = 0;
...@@ -307,7 +308,7 @@ public class DefaultFlowRule implements FlowRule { ...@@ -307,7 +308,7 @@ public class DefaultFlowRule implements FlowRule {
307 308
308 @Override 309 @Override
309 public FlowRule.Builder fromApp(ApplicationId appId) { 310 public FlowRule.Builder fromApp(ApplicationId appId) {
310 - this.flowId = computeFlowId(appId); 311 + this.appId = appId;
311 return this; 312 return this;
312 } 313 }
313 314
...@@ -357,16 +358,22 @@ public class DefaultFlowRule implements FlowRule { ...@@ -357,16 +358,22 @@ public class DefaultFlowRule implements FlowRule {
357 358
358 @Override 359 @Override
359 public FlowRule build() { 360 public FlowRule build() {
360 - checkNotNull(flowId != null, "Either an application" + 361 + checkArgument(flowId != null || appId != null, "Either an application" +
361 " id or a cookie must be supplied"); 362 " id or a cookie must be supplied");
362 - checkNotNull(selector != null, "Traffic selector cannot be null"); 363 + checkNotNull(selector, "Traffic selector cannot be null");
363 - checkNotNull(timeout != null || permanent != null, "Must either have " + 364 + checkArgument(timeout != null || permanent != null, "Must either have " +
364 "a timeout or be permanent"); 365 "a timeout or be permanent");
365 - checkNotNull(deviceId != null, "Must refer to a device"); 366 + checkNotNull(deviceId, "Must refer to a device");
366 - checkNotNull(priority != null, "Priority cannot be null"); 367 + checkNotNull(priority, "Priority cannot be null");
367 checkArgument(priority >= MIN_PRIORITY, "Priority cannot be less than " + 368 checkArgument(priority >= MIN_PRIORITY, "Priority cannot be less than " +
368 MIN_PRIORITY); 369 MIN_PRIORITY);
369 370
371 + // Computing a flow ID based on appId takes precedence over setting
372 + // the flow ID directly
373 + if (appId != null) {
374 + flowId = computeFlowId(appId);
375 + }
376 +
370 return new DefaultFlowRule(deviceId, selector, treatment, priority, 377 return new DefaultFlowRule(deviceId, selector, treatment, priority,
371 flowId, permanent, timeout, tableId); 378 flowId, permanent, timeout, tableId);
372 } 379 }
......