> -----Original Message----- > From: Loftus, Ciara [mailto:ciara.lof...@intel.com] > Sent: Thursday, February 22, 2024 7:06 PM > To: Tahhan, Maryam <mtah...@redhat.com>; wangyunjian > <wangyunj...@huawei.com> > Cc: dev@dpdk.org; ferruh.yi...@amd.com; sta...@dpdk.org > Subject: RE: [PATCH] net/af_xdp: fix resources leak when xsk configure fails > > > > > On 22/02/2024 03:07, Yunjian Wang wrote: > > In xdp_umem_configure() allocated some resources for the xsk umem, we > > should delete them when xsk configure fails, otherwise it will lead to > > resources leak. > > > > Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD") > > Cc: mailto:sta...@dpdk.org > > > > Signed-off-by: Yunjian Wang mailto:wangyunj...@huawei.com > > --- > > drivers/net/af_xdp/rte_eth_af_xdp.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c > > b/drivers/net/af_xdp/rte_eth_af_xdp.c > > index 2d151e45c7..8b8b2cff9f 100644 > > --- a/drivers/net/af_xdp/rte_eth_af_xdp.c > > +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c > > @@ -1723,8 +1723,10 @@ xsk_configure(struct pmd_internals *internals, > > struct pkt_rx_queue *rxq, > > out_xsk: > > xsk_socket__delete(rxq->xsk); > > out_umem: > > - if (__atomic_fetch_sub(&rxq->umem->refcnt, 1, > > __ATOMIC_ACQUIRE) - 1 == 0) > > + if (__atomic_fetch_sub(&rxq->umem->refcnt, 1, > > __ATOMIC_ACQUIRE) - 1 == 0) { > > + (void)xsk_umem__delete(rxq->umem->umem); > > xdp_umem_destroy(rxq->umem); > > + } > > > > return ret; > > } > > > > Does it make sense to: move `xsk_umem__delete()` inside > > `xdp_umem_destroy()` to be invoked after a NULL check for `umem->umem` > > and then fixup the places where both functions are called to only > > invoke `xdp_umem_destroy()`? (Keeping all the umem cleanup code in one > > place) @Yunjian WDYT? > > > > @Ciara WDYT? > > Thanks for the patch Yunjian. > > @Maryam +1 for the suggestion I think it would be a good optimisation for the > cleanup code.
OK, I will update it in next version. > > Thanks, > Ciara > > >
