> On Oct 11, 2013, at 1:36 PM, Ben Pfaff <b...@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 <b...@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 <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?)
>> 
>> 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?

  Jarno

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to