Message ID | 20170628185447.30240-1-lrichard@redhat.com |
---|---|
State | Accepted |
Headers | show |
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 --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;
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(-)