On Tue, Oct 29, 2013 at 10:24 AM, Andy Zhou <az...@nicira.com> wrote:
> This is not a review, but it would be nice to add some code comments in
> proper data structure layout, so that the same logic can be followed in the
> future.
>
ok, I will add general guideline in datapath.h as follows:
/*
* data structure members should be arranged according to frequency of access
* in ovs fast-path. So that most frequently accessed members are in
first cache-line
* for a datapath object.
*/
>
> On Tue, Oct 29, 2013 at 10:12 AM, Pravin B Shelar <pshe...@nicira.com>
> wrote:
>>
>> Following patch rearranges various ovs structures which
>> are accessed in datapath for optimal cacheline access.
>>
>> Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
>> ---
>> datapath/datapath.h | 19 +++++++++----------
>> datapath/flow.h | 13 ++++++-------
>> datapath/flow_table.c | 2 +-
>> datapath/vport.h | 26 ++++++++++++--------------
>> 4 files changed, 28 insertions(+), 32 deletions(-)
>>
>> 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;
>> };
>>
>> /**
>> * struct datapath - datapath for flow-based packet switching
>> - * @rcu: RCU callback head for deferred destruction.
>> - * @list_node: Element in global 'dps' list.
>> + * @stats_percpu: Per-CPU datapath statistics.
>> * @table: flow table.
>> * @ports: Hash table for ports. %OVSP_LOCAL port always exists.
>> Protected by
>> * ovs_mutex and RCU.
>> - * @stats_percpu: Per-CPU datapath statistics.
>> * @net: Reference to net namespace.
>> + * @list_node: Element in global 'dps' list.
>> + * @rcu: RCU callback head for deferred destruction.
>> *
>> * Context: See the comment on locking at the top of datapath.c for
>> additional
>> * locking information.
>> */
>> struct datapath {
>> - struct rcu_head rcu;
>> - struct list_head list_node;
>> + /* Stats. */
>> + struct dp_stats_percpu __percpu *stats_percpu;
>>
>> /* Flow table. */
>> struct flow_table table;
>> @@ -83,13 +83,12 @@ struct datapath {
>> /* Switch ports. */
>> struct hlist_head *ports;
>>
>> - /* Stats. */
>> - struct dp_stats_percpu __percpu *stats_percpu;
>> -
>> #ifdef CONFIG_NET_NS
>> /* Network namespace ref. */
>> struct net *net;
>> #endif
>> + struct list_head list_node;
>> + struct rcu_head rcu;
>> };
>>
>> /**
>> diff --git a/datapath/flow.h b/datapath/flow.h
>> index d1ac85a..bab87c3 100644
>> --- a/datapath/flow.h
>> +++ b/datapath/flow.h
>> @@ -128,11 +128,11 @@ struct sw_flow_key_range {
>> };
>>
>> struct sw_flow_mask {
>> - int ref_count;
>> - struct rcu_head rcu;
>> struct list_head list;
>> struct sw_flow_key_range range;
>> struct sw_flow_key key;
>> + int ref_count;
>> + struct rcu_head rcu;
>> };
>>
>> struct sw_flow_match {
>> @@ -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[];
>> };
>>
>> diff --git a/datapath/flow_table.c b/datapath/flow_table.c
>> index c2a7aa5..9c828d8 100644
>> --- a/datapath/flow_table.c
>> +++ b/datapath/flow_table.c
>> @@ -50,7 +50,7 @@
>> #define TBL_MIN_BUCKETS 1024
>> #define REHASH_INTERVAL (10 * 60 * HZ)
>>
>> -static struct kmem_cache *flow_cache;
>> +static struct kmem_cache *flow_cache __read_mostly;
>>
>> static u16 range_n_bytes(const struct sw_flow_key_range *range)
>> {
>> diff --git a/datapath/vport.h b/datapath/vport.h
>> index 995889c..761e638 100644
>> --- a/datapath/vport.h
>> +++ b/datapath/vport.h
>> @@ -63,35 +63,34 @@ struct vport_err_stats {
>>
>> /**
>> * struct vport - one port within a datapath
>> - * @rcu: RCU callback head for deferred destruction.
>> + * @percpu_stats: Points to per-CPU statistics used and maintained by
>> vport
>> * @dp: Datapath to which this port belongs.
>> + * @ops: Class structure.
>> + * @dp_hash_node: Element in @datapath->ports hash table in datapath.c.
>> * @upcall_portid: The Netlink port to use for packets received on this
>> port that
>> * miss the flow table.
>> * @port_no: Index into @dp's @ports array.
>> * @hash_node: Element in @dev_table hash table in vport.c.
>> - * @dp_hash_node: Element in @datapath->ports hash table in datapath.c.
>> - * @ops: Class structure.
>> - * @percpu_stats: Points to per-CPU statistics used and maintained by
>> vport
>> * @stats_lock: Protects @err_stats and @offset_stats.
>> * @err_stats: Points to error statistics used and maintained by vport
>> * @offset_stats: Added to actual statistics as a sop to compatibility
>> with
>> * XAPI for Citrix XenServer. Deprecated.
>> + * @rcu: RCU callback head for deferred destruction.
>> */
>> struct vport {
>> - struct rcu_head rcu;
>> + struct pcpu_tstats __percpu *percpu_stats;
>> struct datapath *dp;
>> + const struct vport_ops *ops;
>> + struct hlist_node dp_hash_node;
>> +
>> u32 upcall_portid;
>> u16 port_no;
>>
>> struct hlist_node hash_node;
>> - struct hlist_node dp_hash_node;
>> - const struct vport_ops *ops;
>> -
>> - struct pcpu_tstats __percpu *percpu_stats;
>> -
>> spinlock_t stats_lock;
>> struct vport_err_stats err_stats;
>> struct ovs_vport_stats offset_stats;
>> + struct rcu_head rcu;
>> };
>>
>> /**
>> @@ -118,6 +117,8 @@ struct vport_parms {
>> /**
>> * struct vport_ops - definition of a type of virtual port
>> *
>> + * @send: Send a packet on the device. Returns the length of the packet
>> sent,
>> + * zero for dropped packets or negative for error.
>> * @type: %OVS_VPORT_TYPE_* value for this type of virtual port.
>> * @create: Create a new vport configured as specified. On success
>> returns
>> * a new vport allocated with ovs_vport_alloc(), otherwise an ERR_PTR()
>> value.
>> @@ -129,10 +130,9 @@ struct vport_parms {
>> * existing vport to a &struct sk_buff. May be %NULL for a vport that
>> does not
>> * have any configuration.
>> * @get_name: Get the device's name.
>> - * @send: Send a packet on the device. Returns the length of the packet
>> sent,
>> - * zero for dropped packets or negative for error.
>> */
>> struct vport_ops {
>> + int (*send)(struct vport *, struct sk_buff *);
>> enum ovs_vport_type type;
>>
>> /* Called with ovs_mutex. */
>> @@ -144,8 +144,6 @@ struct vport_ops {
>>
>> /* Called with rcu_read_lock or ovs_mutex. */
>> const char *(*get_name)(const struct vport *);
>> -
>> - int (*send)(struct vport *, struct sk_buff *);
>> };
>>
>> enum vport_err_type {
>> --
>> 1.7.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
