Thanks Ethan. I did zeroed out stack and ttl bits in my diffs earlier while debugging. It didn't help so I removed them. I will take a closer look one more time.
Thanks Ravi -----Original Message----- From: dev-boun...@openvswitch.org [mailto:dev-boun...@openvswitch.org] On Behalf Of Ethan Jackson Sent: Tuesday, February 21, 2012 3:20 PM To: Kerur, Ravi Cc: dev@openvswitch.org Subject: Re: [ovs-dev] Q on FLOW_SIG_SIZE and hashing > 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:et...@nicira.com] > Sent: Tuesday, February 21, 2012 2:31 PM > To: Kerur, Ravi > Cc: dev@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
