[ovs-dev] Q on FLOW_SIG_SIZE and hashing

Ravi.Kerur at telekom.com Ravi.Kerur at telekom.com
Tue Feb 21 22:14:37 UTC 2012


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...

You can patch changes in test_diffs, compile and install ovs. Later program following flows

ovs-ofctl add-flow br0
priority=100,idle_timeout=50000,dl_vlan=1,dl_vlan_pcp=0,actions=strip_vlan,1
ovs-ofctl add-flow br0
priority=65535,idle_timeout=50000,dl_type=0x0806,actions=NORMAL
ovs-ofctl add-flow br0
priority=65535,idle_timeout=50000,dl_type=0x8847,actions=1

test.tgz is a small test program which sends out vlan packets. After you extract files and compile from test.tgz(ip and mac are hardcoded, you might want to change that), you can run it as "./mpls br0 1 0".

Now with the flows programmed, you expect vlan packets to match flow-1 and expect counters to increment. It doesn't happen. 

I have debugged the problem and found that "classifier_lookup" which is called to get matching rule for a particular flow is NULL and packets later are dropped. I played around placing the new variable/member at different offset within struct flow and adjusting the size accordingly with varying results. Let me know if you need additional information. 

Thanks
Ravi

-----Original Message-----
From: Ethan Jackson [mailto:ethan at nicira.com] 
Sent: Tuesday, February 21, 2012 12:39 PM
To: Kerur, Ravi
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] Q on FLOW_SIG_SIZE and hashing

I can't really figure out what issue you're running into without a
complete patch containing the changes you've made.  The diff you sent
out seems close as an approach.  The only comment I would make is that
we shy away from using __attribute__((packed)).  First thing I would
do is try to get the patch to build without packing the structure.
Once you have that working, if you still see the problem could you
please send a patch and I'll have a look at it?

Ethan

On Tue, Feb 21, 2012 at 11:57,  <Ravi.Kerur at telekom.com> wrote:
>
> I have already done that i.e changed FLOW_WC_SEQ = 9 and modified related changes where ever necessary. The problem I am facing is that when a new field is added to struct flow or struct flow size is increased by rearranging the fields within it, hashing doesn't work(as mentioned earlier).
>
> The diffs I sent out earlier is just experimental to check if there is a problem with the mpls changes I have done? At least it looked like it is not because in the diffs I have just modified to increase the size of struct flow and see if things work, it doesn't.
>
> Thanks
> Ravi
>
> -----Original Message-----
> From: Ethan Jackson [mailto:ethan at nicira.com]
> Sent: Tuesday, February 21, 2012 11:38 AM
> To: Kerur, Ravi
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] Q on FLOW_SIG_SIZE and hashing
>
>> Since the reserved field is used to get full struct flow size to be 64 bits
>> aligned, I believe it can be changed based on FLOW_SIG_SIZE and it doesn't
>> have any other inherent meaning and since it is not used for calculating
>> hash I believe changing reserved size should be ok. Secondly, any other
>> changes I need to do or areas of code to look at to fix this problem?
>
> We included the 'reserved' field because we prefer to have padding in
> structures explicit.  It doesn't have any sort of semantic meaning
> beyond that.  It's totally fine to change it to add additional fields.
>
> Adding an additional field to struct flow is a fairly involved
> process.  You will note at the top of lib/flow.h the 'FLOW_WC_SEQ'
> #define.  This was added to help developers find all the places that
> need to change when either struct flow, or the classifier changes.
> It's not comprehensive, but I would start by incrementing that value
> and seeing where the compiler leads you.
>
> Ethan
>
>
>>
>>
>>
>> Thanks
>>
>> Ravi
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_diffs
Type: application/octet-stream
Size: 1772 bytes
Desc: test_diffs
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20120221/e9dce369/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.tgz
Type: application/x-compressed
Size: 5412 bytes
Desc: test.tgz
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20120221/e9dce369/attachment-0004.bin>


More information about the dev mailing list