Following is Ethan's reply from a similar RFC patch I sent just to him earlier,
Repost it here, so that all following comments can be made on this thread. """ At a high level, this looks much more in the direction I was hoping for. I've got a couple of structural comments, some of which are directly contradictory to things I told you earlier =) (sorry bout that). I'd ditch struct monitor and just deal with the hmap directly. It's not giving us anything. I don't really like that we're reusing the xlate_rwlock. I'd prefer that we instead just create our own rwlock internal to the module so things stay clean and separate. Instead of doing everything in terms of xports, I'd do them in terms of ofports. You aren't allowed to dereference an ofport because that isn't thread safe, but you are allowed to use their pointers as indexes into hash tables. We've got a ton of code which does something like this. If you ditch monitor you can use the HMAP_INITIALIZER and ditch ofproto_dpif_monitor_init(). I actually changed my mind about what I said earlier with respect to maintaining a CFM and BFD pointer in monitor. Having xlate deal with this is more ugly than I had hoped for, but I do like that the monitor module only has to deal with sending packets and we don't need a bunch of wrappers. How about we take a middle way, something like this: We rename mport_create to something like ofporot_dpif_monitor_register(), and pass it a pointer to the ofport, bfd, cfm, and hwaddr like before. We do an mport_find() like the current code does, if the mport doesn't exist, we increment ref counts and set it's bfd, cfm, and hwaddrs. If it does exist, we replace it's current bfd, cfm, and hwaddr with the new ones. The caller would be responsible for calling this function whenever any of these things change, or realistically just whenever the revalidate loop happens. This has the advantage of not requiring us to maintain a pointer to xport in the monitor module. Thoughts? """
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
