| Message ID | 1500480297-7530-5-git-send-email-antonio.fischetti@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Darrell Ball |
| Headers | show |
Hi Antonio, > -----Original Message----- > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > bounces@openvswitch.org] On Behalf Of antonio.fischetti@intel.com > Sent: Wednesday, July 19, 2017 5:05 PM > To: dev@openvswitch.org > Subject: [ovs-dev] [PATCH v2 5/5] dp-packet: Use memcpy on dp_packet > elements. > > memcpy replaces the several single copies inside > dp_packet_clone_with_headroom(). > > Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> > --- > lib/dp-packet.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 67aa406..f4dbcb7 100644 > --- a/lib/dp-packet.c > +++ b/lib/dp-packet.c > @@ -157,8 +157,9 @@ dp_packet_clone(const struct dp_packet *buffer) > return dp_packet_clone_with_headroom(buffer, 0); } > > -/* Creates and returns a new dp_packet whose data are copied from > 'buffer'. The > - * returned dp_packet will additionally have 'headroom' bytes of headroom. > */ > +/* Creates and returns a new dp_packet whose data are copied from > 'buffer'. > + * The returned dp_packet will additionally have 'headroom' bytes of > + * headroom. */ > struct dp_packet * > dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t > headroom) { @@ -167,13 +168,12 @@ > dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t > headroom) > new_buffer = > dp_packet_clone_data_with_headroom(dp_packet_data(buffer), > dp_packet_size(buffer), > headroom); > - new_buffer->l2_pad_size = buffer->l2_pad_size; > - new_buffer->l2_5_ofs = buffer->l2_5_ofs; > - new_buffer->l3_ofs = buffer->l3_ofs; > - new_buffer->l4_ofs = buffer->l4_ofs; > - new_buffer->md = buffer->md; > - new_buffer->cutlen = buffer->cutlen; > - new_buffer->packet_type = buffer->packet_type; > + /* Copy the following fields into the returned buffer: l2_pad_size, > + * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */ > + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size, > + sizeof(struct dp_packet) - > + offsetof(struct dp_packet, l2_pad_size)); > + [[BO'M]] Does this change in itself have a perf improvement? A reference/warning in the dp_packet declaration would be a good idea - anyone changing fields from l2_pad_size to the end of the structure needs to be aware of this implementation of dp_packet_clone_data_with_headroom(). > #ifdef DPDK_NETDEV > new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; #else > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> -----Original Message----- > From: O Mahony, Billy > Sent: Tuesday, August 1, 2017 3:15 PM > To: Fischetti, Antonio <antonio.fischetti@intel.com>; dev@openvswitch.org > Subject: RE: [ovs-dev] [PATCH v2 5/5] dp-packet: Use memcpy on dp_packet > elements. > > Hi Antonio, > > > -----Original Message----- > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > > bounces@openvswitch.org] On Behalf Of antonio.fischetti@intel.com > > Sent: Wednesday, July 19, 2017 5:05 PM > > To: dev@openvswitch.org > > Subject: [ovs-dev] [PATCH v2 5/5] dp-packet: Use memcpy on dp_packet > > elements. > > > > memcpy replaces the several single copies inside > > dp_packet_clone_with_headroom(). > > > > Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> > > --- > > lib/dp-packet.c | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 67aa406..f4dbcb7 100644 > > --- a/lib/dp-packet.c > > +++ b/lib/dp-packet.c > > @@ -157,8 +157,9 @@ dp_packet_clone(const struct dp_packet *buffer) > > return dp_packet_clone_with_headroom(buffer, 0); } > > > > -/* Creates and returns a new dp_packet whose data are copied from > > 'buffer'. The > > - * returned dp_packet will additionally have 'headroom' bytes of headroom. > > */ > > +/* Creates and returns a new dp_packet whose data are copied from > > 'buffer'. > > + * The returned dp_packet will additionally have 'headroom' bytes of > > + * headroom. */ > > struct dp_packet * > > dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t > > headroom) { @@ -167,13 +168,12 @@ > > dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t > > headroom) > > new_buffer = > > dp_packet_clone_data_with_headroom(dp_packet_data(buffer), > > dp_packet_size(buffer), > > headroom); > > - new_buffer->l2_pad_size = buffer->l2_pad_size; > > - new_buffer->l2_5_ofs = buffer->l2_5_ofs; > > - new_buffer->l3_ofs = buffer->l3_ofs; > > - new_buffer->l4_ofs = buffer->l4_ofs; > > - new_buffer->md = buffer->md; > > - new_buffer->cutlen = buffer->cutlen; > > - new_buffer->packet_type = buffer->packet_type; > > + /* Copy the following fields into the returned buffer: l2_pad_size, > > + * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */ > > + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size, > > + sizeof(struct dp_packet) - > > + offsetof(struct dp_packet, l2_pad_size)); > > + > [[BO'M]] > Does this change in itself have a perf improvement? [Antonio] I tested this change by comparing the CPU Time over a 60 sec analysis with VTune. In original ovs: dp_packet_clone_with_headroom 4.530s + this changes: dp_packet_clone_with_headroom 3.920s Further details were reported in this reply for v1 https://mail.openvswitch.org/pipermail/ovs-dev/2017-June/334536.html ) > > A reference/warning in the dp_packet declaration would be a good idea - anyone > changing fields from l2_pad_size to the end of the structure needs to be aware > of this implementation of dp_packet_clone_data_with_headroom(). [Antonio] agree, will do that. > > > #ifdef DPDK_NETDEV > > new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; #else > > -- > > 2.4.11 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 67aa406..f4dbcb7 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -157,8 +157,9 @@ dp_packet_clone(const struct dp_packet *buffer) return dp_packet_clone_with_headroom(buffer, 0); } -/* Creates and returns a new dp_packet whose data are copied from 'buffer'. The - * returned dp_packet will additionally have 'headroom' bytes of headroom. */ +/* Creates and returns a new dp_packet whose data are copied from 'buffer'. + * The returned dp_packet will additionally have 'headroom' bytes of + * headroom. */ struct dp_packet * dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom) { @@ -167,13 +168,12 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom) new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), dp_packet_size(buffer), headroom); - new_buffer->l2_pad_size = buffer->l2_pad_size; - new_buffer->l2_5_ofs = buffer->l2_5_ofs; - new_buffer->l3_ofs = buffer->l3_ofs; - new_buffer->l4_ofs = buffer->l4_ofs; - new_buffer->md = buffer->md; - new_buffer->cutlen = buffer->cutlen; - new_buffer->packet_type = buffer->packet_type; + /* Copy the following fields into the returned buffer: l2_pad_size, + * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */ + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size, + sizeof(struct dp_packet) - + offsetof(struct dp_packet, l2_pad_size)); + #ifdef DPDK_NETDEV new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; #else
memcpy replaces the several single copies inside dp_packet_clone_with_headroom(). Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> --- lib/dp-packet.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)