[ovs-dev,2/4] netdev-dpdk: if mempool already exists don't reinit packet areas.

Message ID 1506438251-16537-2-git-send-email-antonio.fischetti@intel.com
State Superseded
Headers show
Series
  • [ovs-dev,1/4] netdev-dpdk: fix mempool management with vhu client.
Related show

Commit Message

Fischetti, Antonio Sept. 26, 2017, 3:04 p.m.
Skip initialization of mempool objects if this was already
done in a previous call to dpdk_mp_create.

CC: Ciara Loftus <ciara.loftus@intel.com>
CC: Kevin Traynor <ktraynor@redhat.com>
CC: Aaron Conole <aconole@redhat.com>
Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>
---
 lib/netdev-dpdk.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Darrell Ball Sept. 26, 2017, 8:01 p.m. | #1
On 9/26/17, 8:05 AM, "ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of antonio.fischetti@intel.com> wrote:

    Skip initialization of mempool objects if this was already
    done in a previous call to dpdk_mp_create.
    
    CC: Ciara Loftus <ciara.loftus@intel.com>
    CC: Kevin Traynor <ktraynor@redhat.com>
    CC: Aaron Conole <aconole@redhat.com>
    Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>

    ---
     lib/netdev-dpdk.c | 15 +++++++++------
     1 file changed, 9 insertions(+), 6 deletions(-)
    
    diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    index 2f5ec71..f3f42ee 100644
    --- a/lib/netdev-dpdk.c
    +++ b/lib/netdev-dpdk.c
    @@ -566,12 +566,15 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
             }
             free(mp_name);
             if (dmp->mp) {
    -            /* rte_pktmbuf_pool_create has done some initialization of the
    -             * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
    -             * initializes some OVS specific fields of dp_packet.
    -             */
    -            rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
    -
    +            /* If the current mp was already created by a previous call
    +             * we don't need to init again all its elements. */
    +            if (!mp_exists) {
    +                /* rte_pktmbuf_pool_create has done some initialization of the
    +                 * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
    +                 * initializes some OVS specific fields of dp_packet.
    +                 */
    +                rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);


[Darrell] Can this be moved inside 
        if (dmp->mp) {
            VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
                     dmp->mp_size);
        }….. 

for clarity?


    +            }
                 return dmp;
             }
         } while (!mp_exists &&
    -- 
    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=Bf45eQULq41ut2PpTtt6Dah9xN86c0suku7rL1WVaTs&s=2HhdsumV1sIAkn7BT6u3jzjIPghy-48d2IgkqXemk8c&e=
Fischetti, Antonio Sept. 28, 2017, 12:40 p.m. | #2
> -----Original Message-----

> From: Darrell Ball [mailto:dball@vmware.com]

> Sent: Tuesday, September 26, 2017 9:02 PM

> To: Fischetti, Antonio <antonio.fischetti@intel.com>; dev@openvswitch.org

> Subject: Re: [ovs-dev] [PATCH 2/4] netdev-dpdk: if mempool already exists don't

> reinit packet areas.

> 

> 

> 

> On 9/26/17, 8:05 AM, "ovs-dev-bounces@openvswitch.org on behalf of

> antonio.fischetti@intel.com" <ovs-dev-bounces@openvswitch.org on behalf of

> antonio.fischetti@intel.com> wrote:

> 

>     Skip initialization of mempool objects if this was already

>     done in a previous call to dpdk_mp_create.

> 

>     CC: Ciara Loftus <ciara.loftus@intel.com>

>     CC: Kevin Traynor <ktraynor@redhat.com>

>     CC: Aaron Conole <aconole@redhat.com>

>     Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com>

>     ---

>      lib/netdev-dpdk.c | 15 +++++++++------

>      1 file changed, 9 insertions(+), 6 deletions(-)

> 

>     diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c

>     index 2f5ec71..f3f42ee 100644

>     --- a/lib/netdev-dpdk.c

>     +++ b/lib/netdev-dpdk.c

>     @@ -566,12 +566,15 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)

>              }

>              free(mp_name);

>              if (dmp->mp) {

>     -            /* rte_pktmbuf_pool_create has done some initialization of the

>     -             * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init

>     -             * initializes some OVS specific fields of dp_packet.

>     -             */

>     -            rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);

>     -

>     +            /* If the current mp was already created by a previous call

>     +             * we don't need to init again all its elements. */

>     +            if (!mp_exists) {

>     +                /* rte_pktmbuf_pool_create has done some initialization of

> the

>     +                 * rte_mbuf part of each dp_packet, while

> ovs_rte_pktmbuf_init

>     +                 * initializes some OVS specific fields of dp_packet.

>     +                 */

>     +                rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);

> 

> 

> [Darrell] Can this be moved inside

>         if (dmp->mp) {

>             VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,

>                      dmp->mp_size);

>         }…..

> 

> for clarity?


[Antonio] Thanks, that makes the code more readable too.


> 

> 

>     +            }

>                  return dmp;

>              }

>          } while (!mp_exists &&

>     --

>     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=Bf45eQULq41ut2PpTtt6Dah9xN86c0suku7rL1WVaTs&s=2HhdsumV1sIAkn7BT6u3jzjIP

> ghy-48d2IgkqXemk8c&e=

>

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2f5ec71..f3f42ee 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -566,12 +566,15 @@  dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
         }
         free(mp_name);
         if (dmp->mp) {
-            /* rte_pktmbuf_pool_create has done some initialization of the
-             * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
-             * initializes some OVS specific fields of dp_packet.
-             */
-            rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
-
+            /* If the current mp was already created by a previous call
+             * we don't need to init again all its elements. */
+            if (!mp_exists) {
+                /* rte_pktmbuf_pool_create has done some initialization of the
+                 * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
+                 * initializes some OVS specific fields of dp_packet.
+                 */
+                rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
+            }
             return dmp;
         }
     } while (!mp_exists &&