On Oct 17, 2013, at 12:44 PM, Ben Pfaff <b...@nicira.com> wrote: > On Thu, Oct 17, 2013 at 12:37:27PM -0700, Ben Pfaff wrote: >> On Wed, Oct 16, 2013 at 04:16:07PM -0700, Jarno Rajahalme wrote: >>> This is a little bit simpler and marginally more efficient, but also makes >>> following changes easier. >>> >>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> >> >> I had to read the patch before I understood what it did. Could you >> elaborate the change log slightly? >> >> Any particular reason to inline mf_from_id()? I don't really object, I >> think, but rationale would be nice. Also I don't see yet how this is >> related to the rest of the patch. Could it be separated? >> >> Because ofpacts are padded to a multiple of 8 bytes, and union mf_value >> is 16 bytes long, it looks to me like the trickery around struct >> ofpact_set_field could save at most 8 bytes, so that a struct >> ofpact_set_field is either 16 bytes or 24 bytes long when included in a >> series of struct ofpacts. I doubt that's worth it. > > Also: I'd be inclined to put the functionality of > ofputil_set_field_to_openflow() inside ofp-actions.c. Any particular > reason this adds it to ofp-util.c?
Good point. The only possible reason is that this was where the old set field initialization function was, if I recall correctly. Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
