[ovs-dev,v3,4/4] dp-packet: Use memcpy on dp_packet elements.

Submitted by Fischetti, Antonio on Aug. 11, 2017, 3:52 p.m.

Details

Message ID 1502466766-17370-5-git-send-email-antonio.fischetti@intel.com
State New
Headers show

Commit Message

Fischetti, Antonio Aug. 11, 2017, 3:52 p.m.
memcpy replaces the several single copies inside
dp_packet_clone_with_headroom().

Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
---
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
---
 lib/dp-packet.c | 18 +++++++++---------
 lib/dp-packet.h |  6 +++++-
 2 files changed, 14 insertions(+), 10 deletions(-)

Comments

Darrell Ball Aug. 14, 2017, 6:34 a.m.
Could you show some throughput results for a particular tests ?

-----Original Message-----
From: <ovs-dev-bounces@openvswitch.org> on behalf of "antonio.fischetti@intel.com" <antonio.fischetti@intel.com>
Date: Friday, August 11, 2017 at 8:52 AM
To: "dev@openvswitch.org" <dev@openvswitch.org>
Subject: [ovs-dev] [PATCH v3 4/4] 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>
    ---
    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://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DJune_334536.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6qGelsac4wBCA5cHYalnmiB0QySdkGCItzNJ_-twiJg&s=Wc0BH6ff_CG15qMnkGq8VDI5YmNby5Eo3JxRdZslOsI&e= 
    ---
     lib/dp-packet.c | 18 +++++++++---------
     lib/dp-packet.h |  6 +++++-
     2 files changed, 14 insertions(+), 10 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));
    +
     #ifdef DPDK_NETDEV
         new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
     #else
    diff --git a/lib/dp-packet.h b/lib/dp-packet.h
    index 9dbb611..9cab7c7 100644
    --- a/lib/dp-packet.h
    +++ b/lib/dp-packet.h
    @@ -40,7 +40,8 @@ enum OVS_PACKED_ENUM dp_packet_source {
         DPBUF_STACK,               /* Un-movable stack space or static buffer. */
         DPBUF_STUB,                /* Starts on stack, may expand into heap. */
         DPBUF_DPDK,                /* buffer data is from DPDK allocated memory.
    -                                * ref to dp_packet_init_dpdk() in dp-packet.c. */
    +                                * ref to dp_packet_init_dpdk() in dp-packet.c.
    +                                */
     };
     
     #define DP_PACKET_CONTEXT_SIZE 64
    @@ -61,6 +62,9 @@ struct dp_packet {
         bool rss_hash_valid;        /* Is the 'rss_hash' valid? */
     #endif
         enum dp_packet_source source;  /* Source of memory allocated as 'base'. */
    +
    +    /* All the following elements of this struct are copied by a single call
    +     * to memcpy in dp_packet_clone_with_headroom. */
         uint8_t l2_pad_size;           /* Detected l2 padding size.
                                         * Padding is non-pullable. */
         uint16_t l2_5_ofs;             /* MPLS label stack offset, or UINT16_MAX */
    -- 
    2.4.11
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6qGelsac4wBCA5cHYalnmiB0QySdkGCItzNJ_-twiJg&s=LREI681tchx40jdHLb94j5YDPgqXZnbI9_Yv4QNnqyA&e=

Patch hide | download patch | download mbox

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
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 9dbb611..9cab7c7 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -40,7 +40,8 @@  enum OVS_PACKED_ENUM dp_packet_source {
     DPBUF_STACK,               /* Un-movable stack space or static buffer. */
     DPBUF_STUB,                /* Starts on stack, may expand into heap. */
     DPBUF_DPDK,                /* buffer data is from DPDK allocated memory.
-                                * ref to dp_packet_init_dpdk() in dp-packet.c. */
+                                * ref to dp_packet_init_dpdk() in dp-packet.c.
+                                */
 };
 
 #define DP_PACKET_CONTEXT_SIZE 64
@@ -61,6 +62,9 @@  struct dp_packet {
     bool rss_hash_valid;        /* Is the 'rss_hash' valid? */
 #endif
     enum dp_packet_source source;  /* Source of memory allocated as 'base'. */
+
+    /* All the following elements of this struct are copied by a single call
+     * to memcpy in dp_packet_clone_with_headroom. */
     uint8_t l2_pad_size;           /* Detected l2 padding size.
                                     * Padding is non-pullable. */
     uint16_t l2_5_ofs;             /* MPLS label stack offset, or UINT16_MAX */