No problem. Misunderstandings are inevitable. Thanks,
Ben. On Wed, Aug 14, 2013 at 03:47:44PM -0700, Alex Wang wrote: > Thanks a lot Ben, > > I should be more careful about commenting. > > Kind Regards, > Alex Wang, > > > On Wed, Aug 14, 2013 at 3:39 PM, Ben Pfaff <[email protected]> wrote: > > > On Wed, Aug 14, 2013 at 03:27:07PM -0700, Ben Pfaff wrote: > > > On Wed, Aug 14, 2013 at 02:03:37PM -0700, Alex Wang wrote: > > > > On Wed, Aug 14, 2013 at 1:57 PM, Alex Wang <[email protected]> wrote: > > > > > > > > > > > > > > > > > > > On Wed, Aug 14, 2013 at 1:49 PM, Ben Pfaff <[email protected]> wrote: > > > > > > > > > >> 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? > > > > >> > > > > > > > > > > It will introduce a bug in my bfd patch. Consider if bfd is in > > decay, and > > > > > we set bfd:min_rx and bdf:decay_min_rx=0 (disable bfd decay) > > together, the > > > > > change to bfd:min_rx will trigger the poll and the bfd:decay_min_rx > > > > > configuration will not be included in the poll. So, the decay is not > > > > > stopped. > > > > > > > > > > Also, I didn't see in the BFD protocol that each poll sequence can > > only > > > > > include one parameter change. > > > > > > > > > > > > > > > > > To be more clear, in current implementation, once bfd_poll() is called. > > > > Before the poll sequence finishes, the following calls to bfd_poll() > > will > > > > have no effects. This is why when we configure multiple parameters > > > > together, we will see many poll sequence logs. > > > > > > Let me rephrase my question, because I still don't understand. Should > > > I drop this or should I review it? If I review it, how shall I edit > > > the commit message to reflect this new information? > > > > Alex explained face-to-face that this patch is a prerequisite for his > > other outstanding bfd patch. > > > > I changed the patch to initialize need_poll in a separate declaration, > > because C syntax like "int a, b = 5;" always looks to me as if it > > initializes both 'a' and 'b' to 5. > > > > With that change, I applied this to master. > > > > Thanks Alex! > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
