| Message ID | 1505333152-11087-1-git-send-email-azhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] bridge: Fix controller status update to passive connections | expand |
On 13 September 2017 at 13:05, Andy Zhou <azhou@ovn.org> wrote: > The bug can cause ovs-vswitchd to crash (due to assert) when it is > set up with a passive controller connection. Since only active > connections are kept, the passive connection status update should be > ignored and not trigger asserts. > > Fixes: 85c55772a453 ("bridge: Fix controller status update") > Reported-by: Josh Bailey <josh@faucet.nz> > Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
On Wed, Sep 13, 2017 at 8:30 PM, Joe Stringer <joe@ovn.org> wrote: > On 13 September 2017 at 13:05, Andy Zhou <azhou@ovn.org> wrote: >> The bug can cause ovs-vswitchd to crash (due to assert) when it is >> set up with a passive controller connection. Since only active >> connections are kept, the passive connection status update should be >> ignored and not trigger asserts. >> >> Fixes: 85c55772a453 ("bridge: Fix controller status update") >> Reported-by: Josh Bailey <josh@faucet.nz> >> Signed-off-by: Andy Zhou <azhou@ovn.org> > > Acked-by: Joe Stringer <joe@ovn.org> Thanks for the review. Pushed to master and branch-2.8.
diff --git a/AUTHORS.rst b/AUTHORS.rst index 4b9e78f07f87..5d8b723f62e6 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -390,6 +390,7 @@ Ben Basler bbasler@nicira.com Bhargava Shastry bshastry@sec.t-labs.tu-berlin.de Bob Ball bob.ball@citrix.com Brad Hall brad@nicira.com +Brailey Josh josh@faucet.nz Brandon Heller brandonh@stanford.edu Brendan Kelley bkelley@nicira.com Brent Salisbury brent.salisbury@gmail.com diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index a8cbae78cb23..eec9689e6dbd 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2720,11 +2720,17 @@ refresh_controller_status(void) struct ofproto_controller_info *cinfo = shash_find_data(&info, cfg->target); - ovs_assert(cinfo); - ovsrec_controller_set_is_connected(cfg, cinfo->is_connected); - const char *role = ofp12_controller_role_to_str(cinfo->role); - ovsrec_controller_set_role(cfg, role); - ovsrec_controller_set_status(cfg, &cinfo->pairs); + /* cinfo is NULL when 'cfg->target' is a passive connection. */ + if (cinfo) { + ovsrec_controller_set_is_connected(cfg, cinfo->is_connected); + const char *role = ofp12_controller_role_to_str(cinfo->role); + ovsrec_controller_set_role(cfg, role); + ovsrec_controller_set_status(cfg, &cinfo->pairs); + } else { + ovsrec_controller_set_is_connected(cfg, false); + ovsrec_controller_set_role(cfg, NULL); + ovsrec_controller_set_status(cfg, NULL); + } } ofproto_free_ofproto_controller_info(&info);
The bug can cause ovs-vswitchd to crash (due to assert) when it is set up with a passive controller connection. Since only active connections are kept, the passive connection status update should be ignored and not trigger asserts. Fixes: 85c55772a453 ("bridge: Fix controller status update") Reported-by: Josh Bailey <josh@faucet.nz> Signed-off-by: Andy Zhou <azhou@ovn.org> --- v1->v2: Set defaults when cinfo is NULL. --- AUTHORS.rst | 1 + vswitchd/bridge.c | 16 +++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-)