On Tue, Aug 23, 2016 at 05:51:37PM -0700, Jarno Rajahalme wrote:
> Change the value and mask to be added to the end of the set field
> action without any extra bytes, exept for the usual ofp-actions
> padding to 8 bytes. Together with some structure member packing this
> saves on average about to 256 bytes for each set field and load action
> (set field internal representation is also used for load actions).
>
> On a specific production data set each flow entry uses on average
> about 4.1 load and set field actions. This means that with this patch
> an average of more than 1kb can be saved for each flow with such a
> flow table.
>
> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
Thanks for working on this!
Building on 32-bit I get assertion failures without:
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index 9eaa78c..198eedd 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -479,9 +479,11 @@ struct ofpact_stack {
*
* Used for NXAST_REG_LOAD and OFPAT12_SET_FIELD. */
struct ofpact_set_field {
- struct ofpact ofpact;
- bool flow_has_vlan; /* VLAN present at action validation time. */
- const struct mf_field *field;
+ OFPACT_PADDED_MEMBERS(
+ struct ofpact ofpact;
+ bool flow_has_vlan; /* VLAN present at action validation time. */
+ const struct mf_field *field;
+ );
union mf_value value[]; /* Significant value bytes followed by
* significant mask bytes. */
};
I'd suggest ALIGNED_CAST in ofpact_set_field_mask() to make the
cast-to-void-then-to-real-type trick more greppable.
There's a number of instances of code similar to:
+ struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, mf);
+ memcpy(sf->value, &sf_value, mf->n_bytes);
+ memcpy(ofpact_set_field_mask(sf), &sf_mask, mf->n_bytes);
so that it might be worthwhile to make a function that combines the
three steps (or just the memcpy()s?).
Acked-by: Ben Pfaff <b...@ovn.org>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
