Re: [ovs-dev] [PATCH v6 2/5] netdev-dpdk: skip init for existing mempools.
>From: Fischetti, Antonio >Sent: Tuesday, October 17, 2017 6:18 PM >To: Kavanagh, Mark B; d...@openvswitch.org >Cc: Darrell Ball ; Loftus, Ciara ; >Kevin Traynor ; Aaron Conole >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 ; d...@openvswitch.org >> Cc: Darrell Ball ; Loftus, Ciara ; >> Kevin Traynor ; Aaron Conole >> 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: d...@openvswitch.org >> >Cc: Kavanagh, Mark B ; Darrell Ball >> > ; Loftus, Ciara ; Kevin Traynor >> > ; Aaron Conole ; Fischetti, >Antonio >> > >> >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 Thanks, Mark > >> >> Other than that, LGTM. >> >> Thanks, >> Mark >> >> > >> >CC: Mark B Kavanagh >> >CC: Darrell Ball >> >CC: Ciara Loftus >> >CC: Kevin Traynor >> >CC: Aaron Conole >> >Signed-off-by: Antonio Fischetti >> >--- >> > 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [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; d...@openvswitch.org > Cc: Darrell Ball ; Loftus, Ciara ; > Kevin Traynor ; Aaron Conole > 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: d...@openvswitch.org > >Cc: Kavanagh, Mark B ; Darrell Ball > > ; Loftus, Ciara ; Kevin Traynor > > ; Aaron Conole ; Fischetti, Antonio > > > >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. > > Other than that, LGTM. > > Thanks, > Mark > > > > >CC: Mark B Kavanagh > >CC: Darrell Ball > >CC: Ciara Loftus > >CC: Kevin Traynor > >CC: Aaron Conole > >Signed-off-by: Antonio Fischetti > >--- > > 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [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. CC: Mark B KavanaghCC: Darrell Ball CC: Ciara Loftus CC: Kevin Traynor CC: Aaron Conole Signed-off-by: Antonio Fischetti --- 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev