On Tue, Aug 6, 2024 at 9:29 AM Ilya Maximets <[email protected]> wrote: > > 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.
I don't know what is best here. The spaces are similar to the current formatting, but maybe nothing should be printed at all instead? -M > > 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
