홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Saurav Das
Committed by Jonathan Hart
cbe6de3b 1 parent dd91b84e

Fixing a couple of bugs in default entries for corsa pipeline. 

Also fixing flowRule table-awareness with changes reflected in flow identification (hashing)
and the karaf CLI.

Change-Id: I2fac83db8e0b54b802fb765ef9d82033f7478b99
Inline Side-by-side
Showing 6 changed files with 45 additions and 26 deletions
...@@ -285,7 +285,7 @@ public class BgpRouter { ...@@ -285,7 +285,7 @@ public class BgpRouter {
285 processTableZero(install); 285 processTableZero(install);
286 processTableOne(install); 286 processTableOne(install);
287 processTableTwo(install); 287 processTableTwo(install);
288 - processTableThree(install); 288 + processTableFour(install);
289 processTableFive(install); 289 processTableFive(install);
290 processTableSix(install); 290 processTableSix(install);
291 processTableNine(install); 291 processTableNine(install);
...@@ -305,7 +305,7 @@ public class BgpRouter { ...@@ -305,7 +305,7 @@ public class BgpRouter {
305 FlowRule rule = new DefaultFlowRule(deviceId, selector.build(), 305 FlowRule rule = new DefaultFlowRule(deviceId, selector.build(),
306 treatment.build(), 306 treatment.build(),
307 CONTROLLER_PRIORITY, appId, 0, 307 CONTROLLER_PRIORITY, appId, 0,
308 - true); 308 + true, FlowRule.Type.FIRST);
309 309
310 FlowRuleOperations.Builder ops = FlowRuleOperations.builder(); 310 FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
311 311
...@@ -319,7 +319,7 @@ public class BgpRouter { ...@@ -319,7 +319,7 @@ public class BgpRouter {
319 319
320 rule = new DefaultFlowRule(deviceId, selector.build(), 320 rule = new DefaultFlowRule(deviceId, selector.build(),
321 treatment.build(), DROP_PRIORITY, appId, 321 treatment.build(), DROP_PRIORITY, appId,
322 - 0, true, FlowRule.Type.VLAN_MPLS); 322 + 0, true, FlowRule.Type.FIRST);
323 323
324 ops = install ? ops.add(rule) : ops.remove(rule); 324 ops = install ? ops.add(rule) : ops.remove(rule);
325 325
...@@ -433,7 +433,7 @@ public class BgpRouter { ...@@ -433,7 +433,7 @@ public class BgpRouter {
433 })); 433 }));
434 } 434 }
435 435
436 - private void processTableThree(boolean install) { 436 + private void processTableFour(boolean install) {
437 TrafficSelector.Builder selector = DefaultTrafficSelector.builder(); 437 TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
438 TrafficTreatment.Builder treatment = DefaultTrafficTreatment 438 TrafficTreatment.Builder treatment = DefaultTrafficTreatment
439 .builder(); 439 .builder();
...@@ -469,7 +469,7 @@ public class BgpRouter { ...@@ -469,7 +469,7 @@ public class BgpRouter {
469 469
470 rule = new DefaultFlowRule(deviceId, selector.build(), 470 rule = new DefaultFlowRule(deviceId, selector.build(),
471 treatment.build(), DROP_PRIORITY, appId, 471 treatment.build(), DROP_PRIORITY, appId,
472 - 0, true, FlowRule.Type.VLAN_MPLS); 472 + 0, true, FlowRule.Type.ETHER);
473 473
474 ops = install ? ops.add(rule) : ops.remove(rule); 474 ops = install ? ops.add(rule) : ops.remove(rule);
475 475
......
...@@ -52,7 +52,7 @@ public class FlowsListCommand extends AbstractShellCommand { ...@@ -52,7 +52,7 @@ public class FlowsListCommand extends AbstractShellCommand {
52 public static final String ANY = "any"; 52 public static final String ANY = "any";
53 53
54 private static final String FMT = 54 private static final String FMT =
55 - " id=%s, state=%s, bytes=%s, packets=%s, duration=%s, priority=%s, appId=%s"; 55 + " id=%s, state=%s, bytes=%s, packets=%s, duration=%s, priority=%s, tableId=%s appId=%s";
56 private static final String TFMT = " treatment=%s"; 56 private static final String TFMT = " treatment=%s";
57 private static final String SFMT = " selector=%s"; 57 private static final String SFMT = " selector=%s";
58 58
...@@ -131,6 +131,7 @@ public class FlowsListCommand extends AbstractShellCommand { ...@@ -131,6 +131,7 @@ public class FlowsListCommand extends AbstractShellCommand {
131 .put("bytes", flow.bytes()) 131 .put("bytes", flow.bytes())
132 .put("packets", flow.packets()) 132 .put("packets", flow.packets())
133 .put("life", flow.life()) 133 .put("life", flow.life())
134 + .put("tableId", flow.type().toString())
134 .put("appId", coreService.getAppId(flow.appId()).name()); 135 .put("appId", coreService.getAppId(flow.appId()).name());
135 result.set("selector", crit); 136 result.set("selector", crit);
136 result.set("treatment", instr); 137 result.set("treatment", instr);
...@@ -185,7 +186,7 @@ public class FlowsListCommand extends AbstractShellCommand { ...@@ -185,7 +186,7 @@ public class FlowsListCommand extends AbstractShellCommand {
185 if (!empty) { 186 if (!empty) {
186 for (FlowEntry f : flows) { 187 for (FlowEntry f : flows) {
187 print(FMT, Long.toHexString(f.id().value()), f.state(), 188 print(FMT, Long.toHexString(f.id().value()), f.state(),
188 - f.bytes(), f.packets(), f.life(), f.priority(), 189 + f.bytes(), f.packets(), f.life(), f.priority(), f.type(),
189 coreService.getAppId(f.appId()).name()); 190 coreService.getAppId(f.appId()).name());
190 print(SFMT, f.selector().criteria()); 191 print(SFMT, f.selector().criteria());
191 print(TFMT, f.treatment().instructions()); 192 print(TFMT, f.treatment().instructions());
......
...@@ -180,11 +180,11 @@ public class DefaultFlowRule implements FlowRule { ...@@ -180,11 +180,11 @@ public class DefaultFlowRule implements FlowRule {
180 * @see java.lang.Object#equals(java.lang.Object) 180 * @see java.lang.Object#equals(java.lang.Object)
181 */ 181 */
182 public int hashCode() { 182 public int hashCode() {
183 - return Objects.hash(deviceId, selector, priority); 183 + return Objects.hash(deviceId, selector, priority, type);
184 } 184 }
185 185
186 public int hash() { 186 public int hash() {
187 - return Objects.hash(deviceId, selector, treatment); 187 + return Objects.hash(deviceId, selector, treatment, type);
188 } 188 }
189 189
190 @Override 190 @Override
...@@ -202,7 +202,8 @@ public class DefaultFlowRule implements FlowRule { ...@@ -202,7 +202,8 @@ public class DefaultFlowRule implements FlowRule {
202 DefaultFlowRule that = (DefaultFlowRule) obj; 202 DefaultFlowRule that = (DefaultFlowRule) obj;
203 return Objects.equals(deviceId, that.deviceId) && 203 return Objects.equals(deviceId, that.deviceId) &&
204 Objects.equals(priority, that.priority) && 204 Objects.equals(priority, that.priority) &&
205 - Objects.equals(selector, that.selector); 205 + Objects.equals(selector, that.selector) &&
206 + Objects.equals(type, that.type);
206 207
207 } 208 }
208 return false; 209 return false;
...@@ -216,6 +217,7 @@ public class DefaultFlowRule implements FlowRule { ...@@ -216,6 +217,7 @@ public class DefaultFlowRule implements FlowRule {
216 .add("priority", priority) 217 .add("priority", priority)
217 .add("selector", selector.criteria()) 218 .add("selector", selector.criteria())
218 .add("treatment", treatment == null ? "N/A" : treatment.instructions()) 219 .add("treatment", treatment == null ? "N/A" : treatment.instructions())
220 + .add("table type", type)
219 .add("created", created) 221 .add("created", created)
220 .toString(); 222 .toString();
221 } 223 }
......
...@@ -33,7 +33,8 @@ public interface FlowRule { ...@@ -33,7 +33,8 @@ public interface FlowRule {
33 * For single table switch, Default is used. 33 * For single table switch, Default is used.
34 */ 34 */
35 public static enum Type { 35 public static enum Type {
36 - /* Default type - used in flow rule for single table switch */ 36 + /* Default type - used in flow rule for single table switch
37 + * NOTE: this setting should not be used as Table 0 in a multi-table pipeline*/
37 DEFAULT, 38 DEFAULT,
38 /* Used in flow entry for IP table */ 39 /* Used in flow entry for IP table */
39 IP, 40 IP,
...@@ -48,11 +49,14 @@ public interface FlowRule { ...@@ -48,11 +49,14 @@ public interface FlowRule {
48 /* VLAN table */ 49 /* VLAN table */
49 VLAN, 50 VLAN,
50 51
51 - /* L2 table */ 52 + /* Ethtype table */
52 ETHER, 53 ETHER,
53 54
54 /* Class of Service table */ 55 /* Class of Service table */
55 COS, 56 COS,
57 +
58 + /* Table 0 in a multi-table pipeline */
59 + FIRST,
56 } 60 }
57 61
58 //TODO: build cookie value 62 //TODO: build cookie value
......
...@@ -63,7 +63,7 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch { ...@@ -63,7 +63,7 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch {
63 if (msg.getType() == OFType.FLOW_MOD) { 63 if (msg.getType() == OFType.FLOW_MOD) {
64 OFFlowMod flowMod = (OFFlowMod) msg; 64 OFFlowMod flowMod = (OFFlowMod) msg;
65 OFFlowMod.Builder builder = flowMod.createBuilder(); 65 OFFlowMod.Builder builder = flowMod.createBuilder();
66 - List<OFInstruction> instructions = builder.getInstructions(); 66 + List<OFInstruction> instructions = flowMod.getInstructions();
67 List<OFInstruction> newInstructions = Lists.newArrayList(); 67 List<OFInstruction> newInstructions = Lists.newArrayList();
68 for (OFInstruction i : instructions) { 68 for (OFInstruction i : instructions) {
69 if (i instanceof OFInstructionGotoTable) { 69 if (i instanceof OFInstructionGotoTable) {
...@@ -106,9 +106,11 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch { ...@@ -106,9 +106,11 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch {
106 .setTableId(TableId.of(LOCAL_TABLE)).build()); 106 .setTableId(TableId.of(LOCAL_TABLE)).build());
107 break; 107 break;
108 case NONE: 108 case NONE:
109 - newInstructions.add( 109 + log.error("Should never have to go to Table 0");
110 + /*newInstructions.add(
110 gotoTable.createBuilder() 111 gotoTable.createBuilder()
111 .setTableId(TableId.of(0)).build()); 112 .setTableId(TableId.of(0)).build());
113 + */
112 break; 114 break;
113 default: 115 default:
114 log.warn("Unknown table type: {}", tid); 116 log.warn("Unknown table type: {}", tid);
...@@ -147,8 +149,10 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch { ...@@ -147,8 +149,10 @@ public class OFCorsaSwitchDriver extends AbstractOpenFlowSwitch {
147 log.warn("Unknown table type: {}", type); 149 log.warn("Unknown table type: {}", type);
148 } 150 }
149 builder.setInstructions(newInstructions); 151 builder.setInstructions(newInstructions);
150 - this.write(builder.build()); 152 + OFMessage msgnew = builder.build();
151 - log.info("Installed {}", builder.build()); 153 + this.write(msgnew);
154 + log.debug("Installed {}", msgnew);
155 +
152 } else { 156 } else {
153 this.write(msg); 157 this.write(msg);
154 } 158 }
......
...@@ -97,19 +97,20 @@ public class FlowModBuilderVer13 extends FlowModBuilder { ...@@ -97,19 +97,20 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
97 Match match = buildMatch(); 97 Match match = buildMatch();
98 List<OFAction> actions = buildActions(); 98 List<OFAction> actions = buildActions();
99 List<OFInstruction> instructions = buildInstructions(); 99 List<OFInstruction> instructions = buildInstructions();
100 +
100 // FIXME had to revert back to using apply-actions instead of 101 // FIXME had to revert back to using apply-actions instead of
101 // write-actions because LINC-OE apparently doesn't support 102 // write-actions because LINC-OE apparently doesn't support
102 // write-actions. I would prefer to change this back in the future 103 // write-actions. I would prefer to change this back in the future
103 // because apply-actions is an optional instruction in OF 1.3. 104 // because apply-actions is an optional instruction in OF 1.3.
104 105
105 - OFInstruction applyActions = 106 + if (actions != null) {
106 - factory().instructions().applyActions(actions); 107 + OFInstruction applyActions =
107 - 108 + factory().instructions().applyActions(actions);
108 - instructions.add(applyActions); 109 + instructions.add(applyActions);
110 + }
109 111
110 long cookie = flowRule().id().value(); 112 long cookie = flowRule().id().value();
111 113
112 -
113 OFFlowAdd fm = factory().buildFlowAdd() 114 OFFlowAdd fm = factory().buildFlowAdd()
114 .setXid(xid) 115 .setXid(xid)
115 .setCookie(U64.of(cookie)) 116 .setCookie(U64.of(cookie))
...@@ -129,14 +130,15 @@ public class FlowModBuilderVer13 extends FlowModBuilder { ...@@ -129,14 +130,15 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
129 Match match = buildMatch(); 130 Match match = buildMatch();
130 List<OFAction> actions = buildActions(); 131 List<OFAction> actions = buildActions();
131 List<OFInstruction> instructions = buildInstructions(); 132 List<OFInstruction> instructions = buildInstructions();
132 - OFInstruction applyActions =
133 - factory().instructions().applyActions(actions);
134 133
135 - instructions.add(applyActions); 134 + if (actions != null) {
135 + OFInstruction applyActions =
136 + factory().instructions().applyActions(actions);
137 + instructions.add(applyActions);
138 + }
136 139
137 long cookie = flowRule().id().value(); 140 long cookie = flowRule().id().value();
138 141
139 -
140 OFFlowMod fm = factory().buildFlowModify() 142 OFFlowMod fm = factory().buildFlowModify()
141 .setXid(xid) 143 .setXid(xid)
142 .setCookie(U64.of(cookie)) 144 .setCookie(U64.of(cookie))
...@@ -189,6 +191,7 @@ public class FlowModBuilderVer13 extends FlowModBuilder { ...@@ -189,6 +191,7 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
189 191
190 private List<OFAction> buildActions() { 192 private List<OFAction> buildActions() {
191 List<OFAction> actions = new LinkedList<>(); 193 List<OFAction> actions = new LinkedList<>();
194 + boolean tableFound = false;
192 if (treatment == null) { 195 if (treatment == null) {
193 return actions; 196 return actions;
194 } 197 }
...@@ -223,12 +226,17 @@ public class FlowModBuilderVer13 extends FlowModBuilder { ...@@ -223,12 +226,17 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
223 break; 226 break;
224 case TABLE: 227 case TABLE:
225 //FIXME: should not occur here. 228 //FIXME: should not occur here.
229 + tableFound = true;
226 break; 230 break;
227 default: 231 default:
228 log.warn("Instruction type {} not yet implemented.", i.type()); 232 log.warn("Instruction type {} not yet implemented.", i.type());
229 } 233 }
230 } 234 }
231 - 235 + if (tableFound && actions.isEmpty()) {
236 + // handles the case where there are no actions, but there is
237 + // a goto instruction for the next table
238 + return null;
239 + }
232 return actions; 240 return actions;
233 } 241 }
234 242
......