diff mbox

[ovs-dev] ovn-controller: avoid crash when vswitchd connection is lost

Message ID 20170628185447.30240-1-lrichard@redhat.com
State Accepted
Headers show

Commit Message

Lance Richardson June 28, 2017, 6:54 p.m. UTC
When ovs-vswitchd has dropped its connection to ovn-controller,
rconn_get_version() will return -1. OpenFlow messages built by
ofctrl_put() in this condition will have an invalid OpenFlow version
value of 255, which eventually leads to ovn-controller crashing
due to an assertion failure in raw_instance_get().

Avoid this crash by improving the ofctrl_can_put() test to ensure
that the negotiated version is available. (Note that checking
rconn_is_connected() would not be sufficient since rconn S_IDLE
state is considered "connected" but version negotiation is not
necessarily complete).

Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 ovn/controller/ofctrl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Ben Pfaff July 12, 2017, 5:02 a.m. UTC | #1
On Wed, Jun 28, 2017 at 02:54:47PM -0400, Lance Richardson wrote:
> When ovs-vswitchd has dropped its connection to ovn-controller,
> rconn_get_version() will return -1. OpenFlow messages built by
> ofctrl_put() in this condition will have an invalid OpenFlow version
> value of 255, which eventually leads to ovn-controller crashing
> due to an assertion failure in raw_instance_get().
> 
> Avoid this crash by improving the ofctrl_can_put() test to ensure
> that the negotiated version is available. (Note that checking
> rconn_is_connected() would not be sufficient since rconn S_IDLE
> state is considered "connected" but version negotiation is not
> necessarily complete).
> 
> Signed-off-by: Lance Richardson <lrichard@redhat.com>

Ouch.  This is nasty.  I want a less surprising implementation here.

Anyway, I applied this to master and backported to 2.7 and 2.6, for now.
Thanks a lot!
diff mbox

Patch

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 277d3d7..5aff230 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -817,7 +817,8 @@  bool
 ofctrl_can_put(void)
 {
     if (state != S_UPDATE_FLOWS
-        || rconn_packet_counter_n_packets(tx_counter)) {
+        || rconn_packet_counter_n_packets(tx_counter)
+        || rconn_get_version(swconn) < 0) {
         return false;
     }
     return true;