One problem with this, is that if some other code later calls bfd_set_state() with something other than DIAG_NONE, we lose our cpath_down status. I think instead, we should change bfd_set_state() to always override with cpath_down instead of just when DIAG_NONE. Then whenever cpath_down changes we could force it to update by calling bfd_set_state(bfd, bfd->state, bfd->diag), retaining our original diagnostic when cpath_down is off.
Ethan On Wed, Jan 8, 2014 at 6:32 PM, Alex Wang <[email protected]> wrote: > Commit ccc09689 (bfd: Implement Bidirectional Forwarding Detection.) > set the bfd local diagnostic to "Concatenated Path Down" in response > to the set of cpath_down only when the current local diagnostic is > "None". However, since the bfd local diagnostic is not reset when > the bfd state is restored from last erroneous state, the set of > cpath_down will not update the local diagnostic in that case. > > This commit fixes the bug by always checking for local diagnostic > change when cpath_down is set or reset. > > Bug #22625 > > Signed-off-by: Alex Wang <[email protected]> > --- > lib/bfd.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/lib/bfd.c b/lib/bfd.c > index b8574d5..3deeab1 100644 > --- a/lib/bfd.c > +++ b/lib/bfd.c > @@ -403,9 +403,7 @@ bfd_configure(struct bfd *bfd, const char *name, const > struct smap *cfg, > cpath_down = smap_get_bool(cfg, "cpath_down", false); > if (bfd->cpath_down != cpath_down) { > bfd->cpath_down = cpath_down; > - if (bfd->diag == DIAG_NONE || bfd->diag == DIAG_CPATH_DOWN) { > - bfd_set_state(bfd, bfd->state, DIAG_NONE); > - } > + bfd_set_state(bfd, bfd->state, DIAG_NONE); > need_poll = true; > } > > -- > 1.7.9.5 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
