[ovs-dev] [PATCH] bfd: Increase configuration efficiency.

[Ben Pfaff]

I don't understand bfd well enough to understand this comment.  Do you
mean that this patch fixes such a bug or that it introduces such a
bug?

Thanks,

Ben.

On Tue, Aug 13, 2013 at 04:33:42PM -0700, Alex Wang wrote:
> This can be a bug, when we configure bfd:min_rx and bfd:decay_min_rx
> together. (at the time we are in decay)
> 
> 
> 
> On Tue, Aug 13, 2013 at 4:51 PM, Alex Wang <alexw at nicira.com> wrote:
> 
> > Currently, when there are multiple bfd configuration changes,
> > the bfd_poll() will only update one change at a time with the
> > other side. This commit moves the call to bfd_poll() at the
> > end of configuration processing function, so that bfd_poll()
> > will update all configuration changes together.
> >
> > Signed-off-by: Alex Wang <alexw at nicira.com>
> > ---
> >  lib/bfd.c |   11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/bfd.c b/lib/bfd.c
> > index 7c7c0b7..de85c7f 100644
> > --- a/lib/bfd.c
> > +++ b/lib/bfd.c
> > @@ -261,7 +261,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
> > struct smap *cfg)
> >      static atomic_uint16_t udp_src = ATOMIC_VAR_INIT(0);
> >
> >      long long int min_tx, min_rx;
> > -    bool cpath_down;
> > +    bool cpath_down, need_poll = false;
> >      const char *hwaddr;
> >      uint8_t ea[ETH_ADDR_LEN];
> >
> > @@ -315,7 +315,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
> > struct smap *cfg)
> >              || (!bfd_in_poll(bfd) && bfd->cfg_min_tx < bfd->min_tx)) {
> >              bfd->min_tx = bfd->cfg_min_tx;
> >          }
> > -        bfd_poll(bfd);
> > +        need_poll = true;
> >      }
> >
> >      min_rx = smap_get_int(cfg, "min_rx", 1000);
> > @@ -326,7 +326,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
> > struct smap *cfg)
> >              || (!bfd_in_poll(bfd) && bfd->cfg_min_rx > bfd->min_rx)) {
> >              bfd->min_rx = bfd->cfg_min_rx;
> >          }
> > -        bfd_poll(bfd);
> > +        need_poll = true;
> >      }
> >
> >      cpath_down = smap_get_bool(cfg, "cpath_down", false);
> > @@ -335,7 +335,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
> > struct smap *cfg)
> >          if (bfd->diag == DIAG_NONE || bfd->diag == DIAG_CPATH_DOWN) {
> >              bfd_set_state(bfd, bfd->state, DIAG_NONE);
> >          }
> > -        bfd_poll(bfd);
> > +        need_poll = true;
> >      }
> >
> >      hwaddr = smap_get(cfg, "bfd_dst_mac");
> > @@ -347,6 +347,9 @@ bfd_configure(struct bfd *bfd, const char *name, const
> > struct smap *cfg)
> >          bfd->eth_dst_set = false;
> >      }
> >
> > +    if (need_poll) {
> > +        bfd_poll(bfd);
> > +    }
> >      ovs_mutex_unlock(&mutex);
> >      return bfd;
> >  }
> > --
> > 1.7.9.5
> >
> >