On 7/24/24 20:07, Mike Pattrick wrote:
> Currently it is possible to call netdev-dpdk/get-mempool-info before a
> mempool as been created. This can happen because a device is added to
> the netdev_shash before a mempool is allocated for it, which results in
> a segmentation fault.
>
> Now we check for a NULL value before attempting to dereference it.
>
> Fixes: be4817331071 ("netdev-dpdk: Add debug appctl to get mempool
> information.")
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
> v2: Added a test for this issue.
> ---
> lib/netdev-dpdk.c | 12 ++++++++----
> tests/system-dpdk.at | 5 +++++
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 02cef6e45..6afa6da88 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -4664,10 +4664,14 @@ netdev_dpdk_get_mempool_info(struct unixctl_conn
> *conn,
> ovs_mutex_lock(&dev->mutex);
> ovs_mutex_lock(&dpdk_mp_mutex);
>
> - rte_mempool_dump(stream, dev->dpdk_mp->mp);
> - fprintf(stream, " count: avail (%u), in use (%u)\n",
> - rte_mempool_avail_count(dev->dpdk_mp->mp),
> - rte_mempool_in_use_count(dev->dpdk_mp->mp));
> + if (dev->dpdk_mp) {
> + rte_mempool_dump(stream, dev->dpdk_mp->mp);
> + fprintf(stream, " count: avail (%u), in use (%u)\n",
> + rte_mempool_avail_count(dev->dpdk_mp->mp),
> + rte_mempool_in_use_count(dev->dpdk_mp->mp));
> + } else {
> + fprintf(stream, " Not allocated\n");
Hi, Mike. Do we actually need these spaces at the beginning?
The other fprintf has them because this is the way rte_mempool_dump()
formats mempool statistics and we're trying to blend in, but it seems
unnecessary for the no-mempool case.
Best regards, Ilya Maixmets.
> + }
>
> ovs_mutex_unlock(&dpdk_mp_mutex);
> ovs_mutex_unlock(&dev->mutex);
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index 1c97bf777..ecf4c7496 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -88,6 +88,11 @@ ADD_VHOST_USER_CLIENT_PORT([br10], [dpdkvhostuserclient0],
> [$OVS_RUNDIR/dpdkvhos
> AT_CHECK([ovs-vsctl show], [], [stdout])
> sleep 2
>
> +dnl Check that no mempool was allocated.
> +AT_CHECK([ovs-appctl netdev-dpdk/get-mempool-info dpdkvhostuserclient0],
> [0], [dnl
> + Not allocated
> +])
> +
> dnl Clean up
> AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout],
> [stderr])
> OVS_DPDK_STOP_VSWITCHD(["dnl
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
