[ovs-dev] Q on FLOW_SIG_SIZE and hashing

Ethan Jackson ethan at nicira.com
Tue Feb 21 23:19:33 UTC 2012


> Yes I do. Attached complete diffs. The debugging information I mentioned earlier are with these diffs as well. I shifted to just flow struct diffs after I went through FLOW_WC_SEQ changes I had and thought it might not have an impact, as most of the checks are in ".c" and with build_assert_decl and compilation should have failed if I had missed something. Anyways let me know your inputs.

OK great.  So typically bugs like this come around when you have
uninitialized data in the flow or in the flow wildcards which could
cause cls_rule_hash() to return unpredictable results.  This
completely breaks the classifier.  I haven't read the diff you've sent
carefully, but it sounds to me like that's the issue you're running
into.  One place to look:  in flow_zero_wildcards() it looks to me
like you are only initializing the MPLS_LABEL_MASK and MPLS_TC_MASK
pits of the 'mpls_lse' field.  If you aren't planning to match on the
TTL or STACk flags, you'll need to zero them out so the uninitialized
memory doesn't break the hash.  That's just a hunch though, I know
very little about MPLS and I haven't read the diff carefully.

Ethan


>
> Thanks
> Ravi
>
> -----Original Message-----
> From: Ethan Jackson [mailto:ethan at nicira.com]
> Sent: Tuesday, February 21, 2012 2:31 PM
> To: Kerur, Ravi
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] Q on FLOW_SIG_SIZE and hashing
>
>> I have attached diffs which includes adding a member to struct flow and adjusting FLOW_SIG_SIZE accordingly. This is experimental so I haven't bothered to change FLOW_WC_SEQ...
>
> Oh I'm sorry for the confusion, I thought you had a more involved
> patch which makes the necessary changes demanded by FLOW_WC_SEQ.  The
> code really does require those changes to work, simply adding the
> field to the structure is insufficient.  The behavior you're seeing is
> what I'd expect to see without the FLOW_WC_SEQ changes.  There may be
> other changes necessary as well.
>
> Ethan



More information about the dev mailing list