[ovs-dev] [PATCH 09/12] dpif: Reorder elements in dpif_upcall structure.
Bodireddy, Bhanuprakash
bhanuprakash.bodireddy at intel.com
Tue Oct 11 16:00:10 UTC 2016
>-----Original Message-----
>From: Jarno Rajahalme [mailto:jarno at ovn.org]
>Sent: Monday, October 10, 2016 9:01 PM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy at intel.com>
>Cc: dev at openvswitch.org
>Subject: Re: [ovs-dev] [PATCH 09/12] dpif: Reorder elements in dpif_upcall
>structure.
>
>How about making the ‘dp_packet’ member the first member and adding a
>comment that this should be first?
This makes sense and by doing so we can avoid the hole, will make this change in V2.
- Bhanu Prakash.
>
> Jarno
>
>> On Oct 10, 2016, at 8:42 AM, Bodireddy, Bhanuprakash
><bhanuprakash.bodireddy at intel.com> wrote:
>>
>> Hello jarno,
>>
>> Thanks for the feedback, while reordering the members of dpif_upcall, I had
>to deviate from standards due to below reason.
>> The dp_packet member has mbuf as first member that starts at new cache
>line creating hole of size 60 bytes between dpif_upcall_type and dp_packet as
>pointed below.
>>
>> struct dpif_upcall {
>> enum dpif_upcall_type type;
>>
>> --> 60 bytes hole
>>
>> /* --- cacheline 1 boundary (64 bytes) --- */
>> struct dp_packet {
>> struct rte_mbuf {
>> /* typedef MARKER */ void * cacheline0[0]; /* 64 0 */
>>
>> }
>> struct nlattr * key;
>> .
>> .
>> }
>>
>> I tried to pack this hole by moving other members in to this space.
>>
>> Regards,
>> Bhanu Prakash.
>>
>>> -----Original Message-----
>>> From: Jarno Rajahalme [mailto:jarno at ovn.org]
>>> Sent: Friday, October 7, 2016 10:11 PM
>>> To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy at intel.com>
>>> Cc: dev at openvswitch.org
>>> Subject: Re: [ovs-dev] [PATCH 09/12] dpif: Reorder elements in
>>> dpif_upcall structure.
>>>
>>> CodingStyle.md instructs to group struct members into related groups.
>>> Also, changing the relative order of pointers should not make any
>>> difference. Could you achieve the same by reordering just the members
>>> above the ‘DPIF_UC_ACTION only.’ comment?
>>>
>>> Jarno
>>>
>>>> On Oct 7, 2016, at 9:17 AM, Bhanuprakash Bodireddy
>>> <bhanuprakash.bodireddy at intel.com> wrote:
>>>>
>>>> By reordering the data elements in dpif_upcall structure, pad bytes
>>>> can be reduced and also a cache line.
>>>>
>>>> Before: structure size:768, holes:1, sum padbytes:60, cachelines:12
>>>> After: structure size:656, holes:1, sum padbytes:4, cachelines:11
>>>>
>>>> Signed-off-by: Bhanuprakash Bodireddy
>>>> <bhanuprakash.bodireddy at intel.com>
>>>> Signed-off-by: Antonio Fischetti <antonio.fischetti at intel.com>
>>>> ---
>>>> lib/dpif.h | 17 +++++++++--------
>>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/lib/dpif.h b/lib/dpif.h index a7c5097..4a4bb3d 100644
>>>> --- a/lib/dpif.h
>>>> +++ b/lib/dpif.h
>>>> @@ -779,17 +779,18 @@ const char *dpif_upcall_type_to_string(enum
>>>> dpif_upcall_type); struct dpif_upcall {
>>>> /* All types. */
>>>> enum dpif_upcall_type type;
>>>> - struct dp_packet packet; /* Packet data. */
>>>> - struct nlattr *key; /* Flow key. */
>>>> - size_t key_len; /* Length of 'key' in bytes. */
>>>> - ovs_u128 ufid; /* Unique flow identifier for 'key'. */
>>>> - struct nlattr *mru; /* Maximum receive unit. */
>>>> - struct nlattr *cutlen; /* Number of bytes shrink from the end. */
>>>>
>>>> /* DPIF_UC_ACTION only. */
>>>> - struct nlattr *userdata; /* Argument to
>>> OVS_ACTION_ATTR_USERSPACE. */
>>>> - struct nlattr *out_tun_key; /* Output tunnel key. */
>>>> struct nlattr *actions; /* Argument to
>OVS_ACTION_ATTR_USERSPACE.
>>> */
>>>> + struct nlattr *out_tun_key; /* Output tunnel key. */
>>>> + struct nlattr *userdata; /* Argument to
>>> OVS_ACTION_ATTR_USERSPACE. */
>>>> +
>>>> + struct nlattr *cutlen; /* Number of bytes shrink from the end. */
>>>> + struct nlattr *mru; /* Maximum receive unit. */
>>>> + ovs_u128 ufid; /* Unique flow identifier for 'key'. */
>>>> + struct dp_packet packet; /* Packet data. */
>>>> + struct nlattr *key; /* Flow key. */
>>>> + size_t key_len; /* Length of 'key' in bytes. */
>>>> };
>>>>
>>>> /* A callback to notify higher layer of dpif about to be purged, so
>>>> that
>>>> --
>>>> 2.4.11
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev at openvswitch.org
>>>> http://openvswitch.org/mailman/listinfo/dev
>>
More information about the dev
mailing list