On 20 November 2013 11:15, Ben Pfaff <b...@nicira.com> wrote: > 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 understand. I think your suggestion sounds suitable. > 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.) I agree that it is surprising. From a code readability standpoint, the name doesn't really match "something told us that a port has gone up/down, whether locally or remote". A better name and some documentation could fix this. Perhaps "connectivity_change_seq"? And this could be moved to somewhere less netdev-specific, but still accessible to bridge and ofproto. That said, if there's a good reason to split it out then I'm happy to do so. This would provide an alternative implementation to patch #5, with the performance benefit of that. AFAIK, the common case, even under unstable network conditions, is where only one of [carrier, bfd, cfm, lacp, stp] is changing. In this case, there shouldn't be much contention on the global seq. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
