Hi Xueming & Maxime, > -----Original Message----- > From: Xueming(Steven) Li <[email protected]> > Sent: Wednesday, November 11, 2020 2:02 PM > To: Maxime Coquelin <[email protected]>; [email protected]; Ding, Xuan > <[email protected]>; [email protected]; NBU-Contact-Thomas > Monjalon <[email protected]>; [email protected]; Xia, Chenbo > <[email protected]> > Subject: RE: [dpdk-dev] [PATCH v3 3/3] vhost: fix fd leak in kick setup > > Hi Maxime, > > Near end of this function, if vhost_check_queue_inflights_packed() and > vhost_check_queue_inflights_split() return with error, is the fd expected > to be > closed by closing vq?
I thought about this before. In theory, it will not cause fd leak because the fd is saved in vq. It will be closed upon next kick msg or vhost device destroy. But thinking it again, maybe it's better to close it now since anyway it's useless now😊 What do you think? Thanks, Chenbo > > >-----Original Message----- > >From: dev <[email protected]> On Behalf Of Maxime Coquelin > >Sent: Monday, November 9, 2020 8:17 PM > >To: [email protected]; [email protected]; [email protected]; > >NBU-Contact-Thomas Monjalon <[email protected]>; [email protected]; > >[email protected] > >Cc: Maxime Coquelin <[email protected]> > >Subject: [dpdk-dev] [PATCH v3 3/3] vhost: fix fd leak in kick setup > > > >This patch fixes a file descriptor leak which happens in the error path > of > >vhost_user_set_vring_kick(). > > > >Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application") > >Cc: [email protected] > > > >Signed-off-by: Maxime Coquelin <[email protected]> > >Reviewed-by: Chenbo Xia <[email protected]> > >--- > > lib/librte_vhost/vhost_user.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > >diff --git a/lib/librte_vhost/vhost_user.c > b/lib/librte_vhost/vhost_user.c index > >94b066f0b9..f3b2adabac 100644 > >--- a/lib/librte_vhost/vhost_user.c > >+++ b/lib/librte_vhost/vhost_user.c > >@@ -1855,8 +1855,12 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, > >struct VhostUserMsg *msg, > > > > /* Interpret ring addresses only when ring is started. */ > > dev = translate_ring_addresses(dev, file.index); > >- if (!dev) > >+ if (!dev) { > >+ if (file.fd != VIRTIO_INVALID_EVENTFD) > >+ close(file.fd); > >+ > > return RTE_VHOST_MSG_RESULT_ERR; > >+ } > > > > *pdev = dev; > > > >-- > >2.26.2
