[ovs-dev] [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.

[Kavanagh, Mark B]

>From: Fischetti, Antonio
>Sent: Tuesday, October 17, 2017 6:18 PM
>To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; dev at openvswitch.org
>Cc: Darrell Ball <dlu998 at gmail.com>; Loftus, Ciara <ciara.loftus at intel.com>;
>Kevin Traynor <ktraynor at redhat.com>; Aaron Conole <aconole at redhat.com>
>Subject: RE: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>
>
>
>> -----Original Message-----
>> From: Kavanagh, Mark B
>> Sent: Tuesday, October 17, 2017 2:34 PM
>> To: Fischetti, Antonio <antonio.fischetti at intel.com>; dev at openvswitch.org
>> Cc: Darrell Ball <dlu998 at gmail.com>; Loftus, Ciara <ciara.loftus at intel.com>;
>> Kevin Traynor <ktraynor at redhat.com>; Aaron Conole <aconole at redhat.com>
>> Subject: RE: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>>
>> >From: Fischetti, Antonio
>> >Sent: Monday, October 16, 2017 2:15 PM
>> >To: dev at openvswitch.org
>> >Cc: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; Darrell Ball
>> ><dlu998 at gmail.com>; Loftus, Ciara <ciara.loftus at intel.com>; Kevin Traynor
>> ><ktraynor at redhat.com>; Aaron Conole <aconole at redhat.com>; Fischetti,
>Antonio
>> ><antonio.fischetti at intel.com>
>> >Subject: [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>> >
>> >Skip initialization of mempool packet areas if this was already
>> >done in a previous call to dpdk_mp_create.
>>
>> Hi Antonio,
>>
>> As stated in my previous review, I believe that this could probably be
>folded
>> into patch 1 of the series (it was patch 3 of v5).
>> However, I don't object strongly to this patch, so I'll leave it to your
>> discretion.
>
>[Antonio]
>I'm keeping this change in a separate patch because it is not related
>to the fixes for the mempool management.
>It's a small improvement, not so much to save CPU cycles, it's actually
>a clean up of the code.

Sure thing.

In that case, Acked-by: Mark Kavanagh <mark.b.kavanagh at intel.com>

Thanks,
Mark

>
>>
>> Other than that, LGTM.
>>
>> Thanks,
>> Mark
>>
>> >
>> >CC: Mark B Kavanagh <mark.b.kavanagh at intel.com>
>> >CC: Darrell Ball <dlu998 at gmail.com>
>> >CC: Ciara Loftus <ciara.loftus at intel.com>
>> >CC: Kevin Traynor <ktraynor at redhat.com>
>> >CC: Aaron Conole <aconole at redhat.com>
>> >Signed-off-by: Antonio Fischetti <antonio.fischetti at intel.com>
>> >---
>> > lib/netdev-dpdk.c | 10 +++++-----
>> > 1 file changed, 5 insertions(+), 5 deletions(-)
>> >
>> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> >index 7f2d7ed..07c438a 100644
>> >--- a/lib/netdev-dpdk.c
>> >+++ b/lib/netdev-dpdk.c
>> >@@ -550,6 +550,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>> >*mp_exists)
>> >         if (dmp->mp) {
>> >             VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
>> >                      dmp->mp_size);
>> >+            /* rte_pktmbuf_pool_create has done some initialization of the
>> >+             * rte_mbuf part of each dp_packet. Some OvS specific fields
>> >+             * of the packet still need to be initialized by
>> >+             * ovs_rte_pktmbuf_init. */
>> >+            rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
>> >         } else if (rte_errno == EEXIST) {
>> >             /* A mempool with the same name already exists.  We just
>> >              * retrieve its pointer to be returned to the caller. */
>> >@@ -566,11 +571,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>> >*mp_exists)
>> >         }
>> >         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);
>> >             return dmp;
>> >         }
>> >     } while (!(*mp_exists) &&
>> >--
>> >2.4.11