[dpdk-dev] Memory leak when adding/removing vhost_user ports
Yuanhan Liu
yuanhan.liu at linux.intel.com
Wed Apr 20 07:04:45 CEST 2016
On Tue, Apr 19, 2016 at 06:33:50PM +0200, Christian Ehrhardt wrote:
> Hi,
> thanks for the patch.
> I backported it this way to my DPDK 2.2 based environment for now (see below):
>
> With that applied one (and only one) of my two guests looses connectivity after
> removing the ports the first time.
Yeah, that's should be because I invoked the "->destroy_device()"
callback.
BTW, I'm curious how do you do the test? I saw you added 256 ports, but
with 2 guests only? So, 254 of them are idle, just for testing the
memory leak bug?
And then you remove all of them, without stopping the guest. How that's
gonna work? I mean, the vhost-user connection would be broken, and data
flow would not work.
--yliu
> No traffic seems to pass, setting the device in the guest down/up doesn't get
> anything.
> But It isn't totally broken - stopping/starting the guest gets it working
> again.
> So openvswitch/dpdk is still somewhat working - it just seems the guest lost
> something, after tapping on that vhost_user interface again it works.
>
> I will check tomorrow and let you know:
> - if I'm more lucky with that patch on top of 16.04
> - if it looses connectivity after the first or a certain amount of port removes
>
> If you find issues with my backport adaption let me know.
>
>
> ---
>
> Backport and reasoning:
>
> new fix relies on a lot of new code, vhost_destroy_device looks totally
> different from the former destroy_device.
> History on todays function content:
> 4796ad63 - original code moved from examples to lib
> a90ca1a1 - this replaces ops->destroy_device with vhost_destroy_device
> 71dc571e - simple check against null pointers
> 45ca9c6f - this changed the code from linked list to arrays
>
> New code cleans with:
> notify_ops->destroy_device (callback into the parent)
> cleanup_device was existing before even in 2.2 code
> free_device as well existing before even in 2.2 code
> Old code cleans with:
> notify_ops->destroy_device - still there
> rm_config_ll_entry -> eventually calls cleanup_device and free_device
> (just in the more complex linked list way)
>
> So the "only" adaption for backporting needed is to replace
> vhost_destroy_device
> with ops->destroy_device(ctx)
>
> Index: dpdk/lib/librte_vhost/vhost_user/vhost-net-user.c
> ===================================================================
> --- dpdk.orig/lib/librte_vhost/vhost_user/vhost-net-user.c
> +++ dpdk/lib/librte_vhost/vhost_user/vhost-net-user.c
> @@ -310,6 +310,7 @@ vserver_new_vq_conn(int fd, void *dat, _
> }
>
> vdev_ctx.fh = fh;
> + vserver->fh = fh;
> size = strnlen(vserver->path, PATH_MAX);
> ops->set_ifname(vdev_ctx, vserver->path,
> size);
> @@ -516,6 +517,11 @@ rte_vhost_driver_unregister(const char *
>
> for (i = 0; i < g_vhost_server.vserver_cnt; i++) {
> if (!strcmp(g_vhost_server.server[i]->path, path)) {
> + struct vhost_device_ctx ctx;
> +
> + ctx.fh = g_vhost_server.server[i]->fh;
> + ops->destroy_device(ctx);
> +
> fdset_del(&g_vhost_server.fdset,
> g_vhost_server.server[i]->listenfd);
>
> Index: dpdk/lib/librte_vhost/vhost_user/vhost-net-user.h
> ===================================================================
> --- dpdk.orig/lib/librte_vhost/vhost_user/vhost-net-user.h
> +++ dpdk/lib/librte_vhost/vhost_user/vhost-net-user.h
> @@ -43,6 +43,7 @@
> struct vhost_server {
> char *path; /**< The path the uds is bind to. */
> int listenfd; /**< The listener sockfd. */
> + uint32_t fh;
> };
>
> /* refer to hw/virtio/vhost-user.c */
>
>
>
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
> On Mon, Apr 18, 2016 at 8:14 PM, Yuanhan Liu <yuanhan.liu at linux.intel.com>
> wrote:
>
> On Mon, Apr 18, 2016 at 10:46:50AM -0700, Yuanhan Liu wrote:
> > On Mon, Apr 18, 2016 at 07:18:05PM +0200, Christian Ehrhardt wrote:
> > > I assume there is a leak somewhere on adding/removing vhost_user ports.
> > > Although it could also be "only" a fragmentation issue.
> > >
> > > Reproduction is easy:
> > > I set up a pair of nicely working OVS-DPDK connected KVM Guests.
> > > Then in a loop I
> > > - add up to more 512 ports
> > > - test connectivity between the two guests
> > > - remove up to 512 ports
> > >
> > > Depending on memory and the amount of multiqueue/rxq I use it seems to
> > > slightly change when exactly it breaks. But for my default setup of 4
> > > queues and 5G Hugepages initialized by DPDK it always breaks at the
> sixth
> > > iteration.
> > > Here a link to the stack trace indicating a memory shortage (TBC):
> > > https://launchpadlibrarian.net/253916410/apport-retrace.log
> > >
> > > Known Todos:
> > > - I want to track it down more, and will try to come up with a non
> > > openvswitch based looping testcase that might show it as well to
> simplify
> > > debugging.
> > > - in use were Openvswitch-dpdk 2.5 and DPDK 2.2; Retest with DPDK 16.04
> and
> > > Openvswitch master is planned.
> > >
> > > I will go on debugging this and let you know, but I wanted to give a
> heads
> > > up to everyone.
> >
> > Thanks for the report.
> >
> > > In case this is a known issue for some of you please let me know.
> >
> > Yeah, it might be. I'm wondering that virtio_net struct is not freed.
> > It will be freed only (if I'm not mistaken) when guest quits, by far.
>
> Would you try following diff and to see if it fix your issue?
>
> --yliu
>
> ---
> lib/librte_vhost/vhost_user/vhost-net-user.c | 6 ++++++
> lib/librte_vhost/vhost_user/vhost-net-user.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/
> librte_vhost/vhost_user/vhost-net-user.c
> index df2bd64..8f7ebd7 100644
> --- a/lib/librte_vhost/vhost_user/vhost-net-user.c
> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
> @@ -309,6 +309,7 @@ vserver_new_vq_conn(int fd, void *dat, __rte_unused int
> *remove)
> }
>
> vdev_ctx.fh = fh;
> + vserver->fh = fh;
> size = strnlen(vserver->path, PATH_MAX);
> vhost_set_ifname(vdev_ctx, vserver->path,
> size);
> @@ -501,6 +502,11 @@ rte_vhost_driver_unregister(const char *path)
>
> for (i = 0; i < g_vhost_server.vserver_cnt; i++) {
> if (!strcmp(g_vhost_server.server[i]->path, path)) {
> + struct vhost_device_ctx ctx;
> +
> + ctx.fh = g_vhost_server.server[i]->fh;
> + vhost_destroy_device(ctx);
> +
> fdset_del(&g_vhost_server.fdset,
> g_vhost_server.server[i]->listenfd);
>
> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h b/lib/
> librte_vhost/vhost_user/vhost-net-user.h
> index e3bb413..7cf21db 100644
> --- a/lib/librte_vhost/vhost_user/vhost-net-user.h
> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.h
> @@ -43,6 +43,7 @@
> struct vhost_server {
> char *path; /**< The path the uds is bind to. */
> int listenfd; /**< The listener sockfd. */
> + uint32_t fh;
> };
>
> /* refer to hw/virtio/vhost-user.c */
> --
> 1.9.3
>
>
>
>
More information about the dev
mailing list