[ovs-dev] [PATCH] lib/flow: Always zero init before use.

[Ben Pfaff]

On Fri, Oct 11, 2013 at 04:40:38PM -0700, Jarno Rajahalme wrote:
> > On Oct 11, 2013, at 1:36 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> >> On Fri, Oct 11, 2013 at 12:51:35PM -0700, Jarno Rajahalme wrote:
> >> 
> >>> On Oct 11, 2013, at 9:02 AM, Ben Pfaff <blp at nicira.com> wrote:
> >>> 
> >>>> 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 <jrajahalme at 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?)
> >> 
> >> Maybe that's just to be on the safe side? If the policy of the
> >> ofproto is to drop all frags, then maybe one mega-flow for that
> >> effect would be enough? It seems that instead of forcing an exact
> >> match, looking at the in_port and nw_frag would be sufficient?
> > 
> > Justin confirmed off-list that he doesn't think this is necessary
> > here.
> 
> You mean full mask, rather than just the frag bits?

I mean that Justin says it isn't necessary to match on everything, so
that we could easily get rid of the call to
flow_wildcards_init_exact().