On Thu, Nov 14, 2013 at 03:28:26PM -0800, Joe Stringer wrote: > Currently, as part of ofproto-dpif run() processing, we loop through all > ports and poll corresponding devices for changes in carrier, cfm and bfd > status. This allows us to determine how it may affect bundles. For the > average case where devices are not going up or down constantly, this is > a large amount of unnecessary processing. > > This patch gets the bfd, cfm, lacp and stp modules to update the global > netdev change_seq when changes in port/device status occur. We can then > use this global change_seq to check if anything has changed before > looping through the ports in ofproto-dpif. In a test environment of 5000 > internal ports and 50 tunnel ports with bfd, this reduces CPU usage from > about 35% to about 25%. > > Signed-off-by: Joe Stringer <joestrin...@nicira.com>
I think that this introduces a strict but easy to miss dependency on the order in which ofproto_run() calls the provider's 'run' function and that it updates the ofproto's change_seq: it works with the current order but if the order were reversed then it would break. I think it would be better to avoid this dependency. (One way would be for ofproto_dpif to maintain its own change sequence number.) I think that it is a little surprising to use a netdev sequence number to track changes to things that are not network devices. If we are going to do that, then I think we need to document it somewhere to make it harder to surprise developers. Another alternative would be to add a sequence number for each object type (cfm, bfd, lacp, stp?). (If we did that, then it would also solve the ordering dependency issue I mentioned above.) > --- > v2: Fixed mutex safety in netdev_vport_changed() > Track STP changes > > NB: I'm tracking STP changes in ofproto_port_set_state(), as the testsuite > would fail when I tracked them in stp_set_port_state(). The latter makes more > sense to me, but I clearly don't understand some interaction there because it > makes STP stop working. I tried a few combinations in lib/stp.c, but I > couldn't > get any of them to work. Hmm. I agree that's odd. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
