[ovs-dev] [PATCH] datapath: Optimize datapath struct for cacheline access.

[Pravin Shelar]

On Tue, Oct 29, 2013 at 3:52 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Tue, Oct 29, 2013 at 10:12 AM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> diff --git a/datapath/datapath.h b/datapath/datapath.h
>> index 879a830..5a89e0e 100644
>> --- a/datapath/datapath.h
>> +++ b/datapath/datapath.h
>> @@ -53,29 +53,29 @@
>>   *   up per packet.
>>   */
>>  struct dp_stats_percpu {
>> +       struct u64_stats_sync sync;
>> +       u64 n_mask_hit;
>>         u64 n_hit;
>>         u64 n_missed;
>>         u64 n_lost;
>> -       u64 n_mask_hit;
>> -       struct u64_stats_sync sync;
>
> Can you update the comment to match the struct changes?
>
ok.

>> diff --git a/datapath/flow.h b/datapath/flow.h
>> index d1ac85a..bab87c3 100644
>> --- a/datapath/flow.h
>> +++ b/datapath/flow.h
>> @@ -156,14 +156,13 @@ struct sw_flow_stats {
>>  } ____cacheline_aligned_in_smp;
>>
>>  struct sw_flow {
>> -       struct rcu_head rcu;
>>         struct hlist_node hash_node[2];
>> -       u32 hash;
>> -
>> -       struct sw_flow_key key;
>> -       struct sw_flow_key unmasked_key;
>>         struct sw_flow_mask *mask;
>> +       struct sw_flow_key key;
>>         struct sw_flow_actions __rcu *sf_acts;
>> +       struct sw_flow_key unmasked_key;
>> +       u32 hash;
>> +       struct rcu_head rcu;
>>         struct sw_flow_stats stats[];
>>  };
>
> I was surprised that we don't look at the hash in the fast path to
> avoid doing a comparison in the face of collisions. It seems we don't
> but can you think of a reason why not? I think we used to.
>
I think we can use it, I will send patch for same.

> Should we also sort struct table_instance for consistency? I realize
> that it's currently less than a cache line but it is in the fast path.

ok.