That's good, thank you.
On Fri, May 13, 2011 at 11:31:42AM -0700, Ethan Jackson wrote: > Thanks for the review. I changed the comment in the code to the following. > > /* According to the 802.1ag specification, reception of a CCM > * with an incorrect ccm_interval should trigger a fault. We > * ignore this requirement for several reasons. > * > * Faults can cause a controller or Open vSwitch to make > * potentially expensive changes to the network topology. It > * seems prudent to trigger them judiciously, especially when > * CFM is used to check slave status of bonds. Furthermore, > * faults can be maliciously triggered by crafting invalid > * CCMs. */ > > On Fri, May 13, 2011 at 10:05, Ben Pfaff <[email protected]> wrote: > > On Thu, May 12, 2011 at 02:44:10PM -0700, Ethan Jackson wrote: > >> According to the 802.1ag specification, when a CCM is received > >> which advertises a misconfigured transmission interval, a fault > >> should be triggered. ?This patch goes against the spec by simply > >> warning when this happens. ?This is done for several reasons. > >> > >> ? - Faults can cause controllers to make potentially expensive > >> ? ? changes in the network topology. > >> ? - Faults can be maliciously triggered by crafting invalid CCMs. > >> ? - Reducing the number of places in the code where rmp->fault and > >> ? ? cfm->fault are changed makes the code easier to debug and > >> ? ? reason about. > > > > I think that the change is OK but the comment in the code itself > > should list at least the first two reasons above, not just the third, > > which isn't nearly as good as rationale. > > > > Thanks, > > > > Ben. > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
