On 6 September 2017 at 15:24, Andy Zhou <[email protected]> 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 <[email protected]>
> Signed-off-by: Andy Zhou <[email protected]>
> ---
This bug was reported on v2.8.0, so will need backport to branch-2.8.
> AUTHORS.rst | 1 +
> vswitchd/bridge.c | 12 +++++++-----
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/AUTHORS.rst b/AUTHORS.rst
> index cb0cd91b21ba..e304609b2b27 100644
> --- a/AUTHORS.rst
> +++ b/AUTHORS.rst
> @@ -389,6 +389,7 @@ Ben Basler [email protected]
> Bhargava Shastry [email protected]
> Bob Ball [email protected]
> Brad Hall [email protected]
> +Brailey Josh [email protected]
> Brandon Heller [email protected]
> Brendan Kelley [email protected]
> Brent Salisbury [email protected]
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index a8cbae78cb23..00f86182c820 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2720,11 +2720,13 @@ 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);
> + }
Prior to commit 85c55772a453f, the following lines were in the alternative case:
ovsrec_controller_set_is_connected(cfg, false);
ovsrec_controller_set_role(cfg, NULL);
ovsrec_controller_set_status(cfg, NULL);
Should we update the records in the else case here like was previously done?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
