On Mon, Apr 23, 2012 at 01:45:24AM -0700, Ethan Jackson wrote: > > This looks more or less good-to-go to me. Â Thank you! > > > > (How much have you tested it? Â What kinds of tests did you do?) > > I mostly did basic functional testing. Created a bunch of ports, destroyed > them. Created bridges named for ports on other bridges. All the testing I > did > used valgrind to look for leaks. I also tested the conf.db which caused > vswitchd to infinite loop to verify that it doesn't. > > > The ofpp_garbage list could just be a dynamic array of "uint16_t"s, > > although the current representation is OK too. > > I thought about that when implementing it. I think lists are a bit easier to > deal with, at this point I'd rather leave it cause I want to get this in. > > > It would work out more cleanly if we changed the "no port yet" > > ofp_port parameter value to be OFPP_NONE, or if we changed the > > semantics of the ofproto_port_add() output parameter to use -1. Â If > > you don't make this change, though, please do notice that both the > > function parameter and the inner block variable are named 'ofp_port' > > currently. > > I find the current code a bit easier to reason about as all of the iface > initialization is done in one place. I changed the name in the block to > new_ofp_port. > > The following is an incremental:
Thanks, it looks good. The one thing that I thought about verifying, but didn't (because it isn't convenient at the moment), was that iface_create() didn't use if_cfg after freeing it. Please push this when you are satisfied with it. Thanks again, Ben. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
