Committed by
Gerrit Code Review
[ONOS-2885] Resolve flow installation failure bug
There is a time gap between controller is elected as the master of a switch and controller actually becomes the master of a switch (receives ROLE_REPLY). During this time gap, OF messages destined for the switch will be sent to this controller. However, the OF message can only be sent to the switch after controller receives the ROLE_REPLY. Change-Id: I32d68384226e272658a0c9de2d32ae9e1cc39b6a
Showing
2 changed files
with
30 additions
and
4 deletions
... | @@ -30,6 +30,14 @@ public interface OpenFlowSwitch { | ... | @@ -30,6 +30,14 @@ public interface OpenFlowSwitch { |
30 | /** | 30 | /** |
31 | * Writes the message to the driver. | 31 | * Writes the message to the driver. |
32 | * | 32 | * |
33 | + * Note: | ||
34 | + * Calling {@link #sendMsg(OFMessage)} does NOT guarantee the messages to be | ||
35 | + * transmitted on the wire in order, especially during role transition. | ||
36 | + * The messages may be reordered at the switch side. | ||
37 | + * | ||
38 | + * Calling {@link #sendMsg(List)} guarantee the messages inside the list | ||
39 | + * to be transmitted on the wire in order. | ||
40 | + * | ||
33 | * @param msg the message to write | 41 | * @param msg the message to write |
34 | */ | 42 | */ |
35 | void sendMsg(OFMessage msg); | 43 | void sendMsg(OFMessage msg); | ... | ... |
... | @@ -79,6 +79,8 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour | ... | @@ -79,6 +79,8 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour |
79 | protected OFFeaturesReply features; | 79 | protected OFFeaturesReply features; |
80 | protected OFDescStatsReply desc; | 80 | protected OFDescStatsReply desc; |
81 | 81 | ||
82 | + List<OFMessage> messagesPendingMastership; | ||
83 | + | ||
82 | @Override | 84 | @Override |
83 | public void init(Dpid dpid, OFDescStatsReply desc, OFVersion ofv) { | 85 | public void init(Dpid dpid, OFDescStatsReply desc, OFVersion ofv) { |
84 | this.dpid = dpid; | 86 | this.dpid = dpid; |
... | @@ -96,16 +98,21 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour | ... | @@ -96,16 +98,21 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour |
96 | } | 98 | } |
97 | 99 | ||
98 | @Override | 100 | @Override |
99 | - public void sendMsg(OFMessage m) { | 101 | + public void sendMsg(OFMessage msg) { |
100 | - if (role == RoleState.MASTER && channel.isConnected()) { | 102 | + this.sendMsg(Collections.singletonList(msg)); |
101 | - channel.write(Collections.singletonList(m)); | ||
102 | - } | ||
103 | } | 103 | } |
104 | 104 | ||
105 | @Override | 105 | @Override |
106 | public final void sendMsg(List<OFMessage> msgs) { | 106 | public final void sendMsg(List<OFMessage> msgs) { |
107 | if (role == RoleState.MASTER && channel.isConnected()) { | 107 | if (role == RoleState.MASTER && channel.isConnected()) { |
108 | channel.write(msgs); | 108 | channel.write(msgs); |
109 | + } else if (messagesPendingMastership != null) { | ||
110 | + messagesPendingMastership.addAll(msgs); | ||
111 | + log.debug("Enqueue message for switch {}. queue size after is {}", | ||
112 | + dpid, messagesPendingMastership.size()); | ||
113 | + } else { | ||
114 | + log.warn("Dropping message for switch {} (role: {}, connected: {}): {}", | ||
115 | + dpid, role, channel.isConnected(), msgs); | ||
109 | } | 116 | } |
110 | } | 117 | } |
111 | 118 | ||
... | @@ -232,6 +239,12 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour | ... | @@ -232,6 +239,12 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour |
232 | @Override | 239 | @Override |
233 | public final void transitionToMasterSwitch() { | 240 | public final void transitionToMasterSwitch() { |
234 | this.agent.transitionToMasterSwitch(dpid); | 241 | this.agent.transitionToMasterSwitch(dpid); |
242 | + if (messagesPendingMastership != null) { | ||
243 | + this.sendMsg(messagesPendingMastership); | ||
244 | + log.debug("Sending {} pending messages to switch {}", | ||
245 | + messagesPendingMastership.size(), dpid); | ||
246 | + messagesPendingMastership = null; | ||
247 | + } | ||
235 | } | 248 | } |
236 | 249 | ||
237 | @Override | 250 | @Override |
... | @@ -278,6 +291,11 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour | ... | @@ -278,6 +291,11 @@ public abstract class AbstractOpenFlowSwitch extends AbstractHandlerBehaviour |
278 | log.debug("Sending role {} to switch {}", role, getStringId()); | 291 | log.debug("Sending role {} to switch {}", role, getStringId()); |
279 | if (role == RoleState.SLAVE || role == RoleState.EQUAL) { | 292 | if (role == RoleState.SLAVE || role == RoleState.EQUAL) { |
280 | this.role = role; | 293 | this.role = role; |
294 | + } else { | ||
295 | + if (messagesPendingMastership == null) { | ||
296 | + log.debug("Initializing new queue for switch {}", dpid); | ||
297 | + messagesPendingMastership = new ArrayList<>(); | ||
298 | + } | ||
281 | } | 299 | } |
282 | } else { | 300 | } else { |
283 | this.role = role; | 301 | this.role = role; | ... | ... |
-
Please register or login to post a comment