[ovs-dev] [PATCH v5 4/6] netdev-dpdk: assert mempool names.

Kavanagh, Mark B mark.b.kavanagh at intel.com
Fri Oct 13 14:47:25 UTC 2017


>From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org]
>On Behalf Of antonio.fischetti at intel.com
>Sent: Wednesday, October 11, 2017 5:01 PM
>To: dev at openvswitch.org
>Subject: [ovs-dev] [PATCH v5 4/6] netdev-dpdk: assert mempool names.
>
>Replace if statement with an assert.
>
>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>

Hey Antonio,

As per my previous comments, I believe that this patch should not be included in the patchset.

Other than that, two comments inline below.

Thanks,
Mark


>---
> lib/netdev-dpdk.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 2aa4a55..c451bc9 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -501,9 +501,7 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>     char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
>     int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>                        h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>-    if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>-        return NULL;
>-    }
>+    ovs_assert(ret >= 0 && ret < RTE_MEMPOOL_NAMESIZE);

The behavior of ovs_assert in the event of failure is to abort execution - are you sure that's what you want to happen here?

In the previous implementation, NULL was returned, and execution continued; are there some undesired side effects of same that you're trying to avoid with this change?


>     return mp_name;
> }
>
>--
>2.4.11
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list