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 <[email protected]>
...
> @@ -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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
