>-----Original Message----- >From: Jarno Rajahalme [mailto:ja...@ovn.org] >Sent: Monday, October 10, 2016 9:01 PM >To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com> >Cc: dev@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.bodire...@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:ja...@ovn.org] >>> Sent: Friday, October 7, 2016 10:11 PM >>> To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com> >>> Cc: dev@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.bodire...@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.bodire...@intel.com> >>>> Signed-off-by: Antonio Fischetti <antonio.fische...@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@openvswitch.org >>>> http://openvswitch.org/mailman/listinfo/dev >> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev