On Wed, Oct 09, 2013 at 01:47:50PM -0700, Jarno Rajahalme wrote: > Use the offset of the last member in struct flow instead of the > struct size to help catch changes in the declaration. > > Add flow_random_hash_fields() used for testing in places where > struct flow was used without zero initialization before. > > With these changes we do not need to keep updating explicit padding > in struct flow as fields are added or deleted. > > Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
... > @@ -603,7 +604,11 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc) > } > > /* Initializes 'wc' as an exact-match set of wildcards; that is, 'wc' does > not > - * wildcard any bits or fields. */ > + * wildcard any bits or fields. > + * Note that also bits that are always zeroes (padding, extra bits in fields > + * like IPv6 flow label), and fields that are mutually exclusive (e.g., IPv4 > + * and IPv6 addresses) are included. > + */ > void > flow_wildcards_init_exact(struct flow_wildcards *wc) > { I don't like this semantic of initializing pads to 0xff very much. But I see that flow_wildcards_init_exact() has only one caller worth considering, which is this code in rule_dpif_lookup_in_table(): } else if (frag && ofproto->up.frag_handling == OFPC_FRAG_DROP) { cls_rule = &ofproto->drop_frags_rule->up.cr; if (wc) { flow_wildcards_init_exact(wc); } } else { I don't understand why we need to use flow_wildcards_init_exact() when dropping fragments. If we didn't use it, then we could probably drop the function entirely. Do you have any idea why we use it here? (Justin?) > @@ -65,7 +63,11 @@ main(int argc, char *argv[]) > struct flow_wildcards wc; > struct flow flow; > > - random_bytes(&flow, sizeof flow); > + /* The generated flows will have random bits where zeroes would > + * normally be required (padding, extra bits in fields like IPv6 > + * flow label, but this test will never use those bits, so it > does > + * not matter. */ > + flow_random_hash_fields(&flow); Is this comment correct? It doesn't look to me like flow_random_hash_fields() puts random bits in the whole flow, only in selected fields. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev