> """ > 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. >
Okay, I'll adjust accordingly. So, for the 'seq', 'latch' structs, added for multithreading, I'll just make them static global variable. > 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. > I see, I'll make the change. 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. > Sounds reasonable to me, I'll take this 'middle way'. > """ >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
