On Tue, Feb 21, 2012 at 06:16:36PM -0800, Ethan Jackson wrote: > nicira-ext.h: > I'm surprised the zero padding is put before the 'controller_id' field > in struct nx_controller_id. Typically we've done it the other way > around, though I don't think it matters in particular. Was there a > specific reason for doing it this way?
Yes, it is odd. I chose to do it that way in case, in the future, we decide that we need longer controller IDs. If we do that, then we can keep using the same struct here, just changing the ovs_be16 to an ovs_be32 or ovs_be64. Admittedly it's strange, but I didn't see a reason that it was a bad idea. > ofp-errors.h: > I like the addition of OFPERR_NXBRC_MUST_BE_ZERO. This isn't the > appropriate place to do it, but I think we have a lot of other actions > which could benefit from returning this error. Is it worth taking the > time to find the places which could use it, i.e. sense it doesn't > actually get sent on the wire, are there other benefits? I'm not sure what you mean by saying that it doesn't actually get sent on the wire. If a function returns that error, then the error will get sent to the controller. Can you explain further? > ofp-util.def: > I think it may be cleaner to follow the precedent of > NXAST_RESUBMIT_TABLE and NXAST_OUTPUT_REG and create an OFPAT_ACTION > for controller, and an NXAST_ACTION for controller which uses NULL as > it's string. This may require some minor tweaks in the rest of the > patch. I don't follow here. Can you explain further? Thanks, Ben. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
