[ovs-dev] connmgr: Do not send asynchronous messages to rconns lacking protocols.

Message ID 20181212202834.22019-1-blp@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev] connmgr: Do not send asynchronous messages to rconns lacking protocols.
Related show

Commit Message

Ben Pfaff Dec. 12, 2018, 8:28 p.m.
There are corner cases in which an rconn might not have a defined OpenFlow
protocol or version.  These happen at connection startup, before the
protocol version has been negotiated, and can also happen when a connection
is being shut down.  It's desirable to avoid these situations entirely,
but so far we haven't managed to do this.  This commit avoids trying to
send messages to such connection, which is what really tends to get OVS in
trouble since there's no way to construct an OpenFlow message without
knowing what version of OpenFlow to use (with a few exceptions that don't
matter here).

Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-December/047876.html
Reported-by: Josh Bailey <joshb@google.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ofproto/connmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Justin Pettit Jan. 10, 2019, 11:12 p.m. | #1
> On Dec 12, 2018, at 12:28 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> There are corner cases in which an rconn might not have a defined OpenFlow
> protocol or version.  These happen at connection startup, before the
> protocol version has been negotiated, and can also happen when a connection
> is being shut down.  It's desirable to avoid these situations entirely,
> but so far we haven't managed to do this.  This commit avoids trying to
> send messages to such connection, which is what really tends to get OVS in
> trouble since there's no way to construct an OpenFlow message without
> knowing what version of OpenFlow to use (with a few exceptions that don't
> matter here).
> 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-December/047876.html
> Reported-by: Josh Bailey <joshb@google.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>

Acked-by: Justin Pettit <jpettit@ovn.org>

--Justin
Ben Pfaff Jan. 18, 2019, 8:58 p.m. | #2
On Thu, Jan 10, 2019 at 03:12:30PM -0800, Justin Pettit wrote:
> 
> > On Dec 12, 2018, at 12:28 PM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > There are corner cases in which an rconn might not have a defined OpenFlow
> > protocol or version.  These happen at connection startup, before the
> > protocol version has been negotiated, and can also happen when a connection
> > is being shut down.  It's desirable to avoid these situations entirely,
> > but so far we haven't managed to do this.  This commit avoids trying to
> > send messages to such connection, which is what really tends to get OVS in
> > trouble since there's no way to construct an OpenFlow message without
> > knowing what version of OpenFlow to use (with a few exceptions that don't
> > matter here).
> > 
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-December/047876.html
> > Reported-by: Josh Bailey <joshb@google.com>
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
> Acked-by: Justin Pettit <jpettit@ovn.org>

Thanks, applied to master.

Patch

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 7a7790c0bdbb..226e493bda01 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1492,7 +1492,7 @@  ofconn_receives_async_msg(const struct ofconn *ofconn,
     ovs_assert(reason < 32);
     ovs_assert((unsigned int) type < OAM_N_TYPES);
 
-    if (!rconn_is_connected(ofconn->rconn)) {
+    if (!rconn_is_connected(ofconn->rconn) || !ofconn_get_protocol(ofconn)) {
         return false;
     }