Hi Antonio, I'm not sure that this approach will work. Mainly the memcpy will not take account of any padding that the compiler may insert and the struct. Maybe if the struct was defined as packed it could fix this objection.
Also anyone editing the structure in future would have to be aware that these elements need to be kept contiguous in order for packet_clone to keep working. Or if the relevant fields here were all placed in the their own nested struct then sizeof that nested struct could be used in the memcpy call as the sizeof nested_struct would account for whatever padding the compiler inserted. Does this change give much of a performance increase? Also I commented on 1/4 of this patchset about a cover letter - but if the patchset members are independent of each other then maybe they should just be separate patches. Regards, Billy. > -----Original Message----- > From: [email protected] [mailto:ovs-dev- > [email protected]] On Behalf Of [email protected] > Sent: Monday, June 19, 2017 11:12 AM > To: [email protected] > Subject: [ovs-dev] [PATCH 4/4] dp-packet: Use memcpy to copy dp_packet > fields. > > From: Antonio Fischetti <[email protected]> > > memcpy replaces the single copies inside > dp_packet_clone_with_headroom(). > > Signed-off-by: Antonio Fischetti <[email protected]> > --- > lib/dp-packet.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 67aa406..5b1d416 100644 > --- a/lib/dp-packet.c > +++ b/lib/dp-packet.c > @@ -157,7 +157,7 @@ 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 > +/* 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 +167,14 @@ > 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; > + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size, > + sizeof(new_buffer->l2_pad_size) + > + sizeof(new_buffer->l2_5_ofs) + > + sizeof(new_buffer->l3_ofs) + > + sizeof(new_buffer->l4_ofs) + > + sizeof(new_buffer->cutlen) + > + sizeof(new_buffer->packet_type) + > + sizeof(new_buffer->md)); > #ifdef DPDK_NETDEV > new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; #else > -- > 2.4.11 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
