[ovs-dev] Q on FLOW_SIG_SIZE and hashing

[Ethan Jackson]

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