On 8/7/24 17:48, 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.
> v3: Set command error instead of printing error.
> ---
> lib/netdev-dpdk.c | 19 ++++++++++++++-----
> tests/system-dpdk.at | 6 ++++++
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 02cef6e45..fe80dbb5a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -4639,6 +4639,7 @@ netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
> {
> size_t size;
> FILE *stream;
> + char *error = NULL;
Should be const. And maybe re-order for the reverse x-mass tree?
> char *response = NULL;
> struct netdev *netdev = NULL;
>
> @@ -4664,10 +4665,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 {
> + error = "Not allocated";
> + }
>
> ovs_mutex_unlock(&dpdk_mp_mutex);
> ovs_mutex_unlock(&dev->mutex);
> @@ -4679,7 +4684,11 @@ netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
>
> fclose(stream);
>
> - unixctl_command_reply(conn, response);
> + if (error) {
> + unixctl_command_reply_error(conn, "Not allocated");
We should use the 'error' here and not duplicate the string.
> + } else {
> + unixctl_command_reply(conn, response);
> + }
> out:
> free(response);
> netdev_close(netdev);
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index 1c97bf777..e79c75565 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -88,6 +88,12 @@ 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],
> [2], [], [dnl
> +Not allocated
> +ovs-appctl: ovs-vswitchd: server returned an error
> +])
> +
> 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
