From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A2A9E43B73; Thu, 22 Feb 2024 10:54:44 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4BEB2406A2; Thu, 22 Feb 2024 10:54:44 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id D5D2240689 for ; Thu, 22 Feb 2024 10:54:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708595682; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ACG+65ZfRzyWS8VnLZdSY5IPKQh55KF6JrqqUdnxZLY=; b=BRin5paN4M3ykvSXJsEd/k5RnWlISgh6oJDCBc85T8H0xohYhcFU5b31M6cVWGXBguyIas j89/rA/E/AG7pwetOuMvQj1q3oAInon9p0/6lT5FeHrurlbWfVvaDQH5SDfoGeaytYoRSm 0UHQFD39VfRCvNTwc703RZjtFA9pau0= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-304-CJ1SVVIrOPGOrcWI485Jjw-1; Thu, 22 Feb 2024 04:54:41 -0500 X-MC-Unique: CJ1SVVIrOPGOrcWI485Jjw-1 Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-68f75058f07so19979836d6.0 for ; Thu, 22 Feb 2024 01:54:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708595680; x=1709200480; h=in-reply-to:cc:from:references:to:content-language:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=ACG+65ZfRzyWS8VnLZdSY5IPKQh55KF6JrqqUdnxZLY=; b=if4adZa9cAjzGYFQGalJt1yopKWibr2MnBMdMKMSDipgMtkrwWWHJPsct56gRQ+QKI M6J3lNlFXb2Tkrfk1MBheijeFSrrGYA9oqXki3u62JQNPiWBk0hb06HnXa2+/XxsK+V+ tzrnSCH3Jn0BRVsm9By3FS5L+8+2DPJj2oWPfHK8u7oqIDRby1JxHpgK0Ke1bINogG4S aAiYl0ITD5TzD7ExeONokrpR53AlKhaO5ncy2FNBwhEoMcEU+I5AHOWjTqno11UPafVh /7EbtJT/hIHTf+CVIQxlSZW/epFmC7XWDNq0g4ZjG/nbjIVque0LrZjAA6Iy6Y3j++zO 5MOw== X-Gm-Message-State: AOJu0YxTUO0+iVQXc2oTdIEba4aZGM/31BfjP2JN0xhukLIabrew8qoM l9fsDGw4kP5NNefp+hLewJ6VN7m/mEouN0pwcdoQwX+i1tqHGAgYM2MLQac7s5dIJmHrE75ZQEZ hF9LWXSqz6ZTkja12nd1OEhkBHWb8mSTj4gvR0FoHCpvW7H3f X-Received: by 2002:a0c:cb06:0:b0:68f:5322:184e with SMTP id o6-20020a0ccb06000000b0068f5322184emr14426440qvk.26.1708595680006; Thu, 22 Feb 2024 01:54:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IH/wbWlYMtbqixM3NaL9JbU/YrgZiPCjGwvNoHZiOOmr3axVkADIqgzjYmVxdfsbbiqKPQxRQ== X-Received: by 2002:a0c:cb06:0:b0:68f:5322:184e with SMTP id o6-20020a0ccb06000000b0068f5322184emr14426432qvk.26.1708595679688; Thu, 22 Feb 2024 01:54:39 -0800 (PST) Received: from [192.168.0.12] ([78.18.30.42]) by smtp.gmail.com with ESMTPSA id g5-20020a0cf845000000b0068ee3a50585sm6645629qvo.38.2024.02.22.01.54.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 22 Feb 2024 01:54:38 -0800 (PST) Message-ID: <871cc85f-2042-4e1e-a705-1c18fe60ecce@redhat.com> Date: Thu, 22 Feb 2024 09:54:36 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] net/af_xdp: fix resources leak when xsk configure fails To: Yunjian Wang , Ciara Loftus References: <1708571262-48380-1-git-send-email-wangyunjian@huawei.com> From: Maryam Tahhan Cc: "dev@dpdk.org" , "ferruh.yigit@amd.com" , stable@dpdk.org In-Reply-To: <1708571262-48380-1-git-send-email-wangyunjian@huawei.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="------------dSsueHujMpqJhR9iutM8ADmr" Content-Language: en-US X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org This is a multi-part message in MIME format. --------------dSsueHujMpqJhR9iutM8ADmr Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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:stable@dpdk.org > > Signed-off-by: Yunjian Wang > --- > 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? --------------dSsueHujMpqJhR9iutM8ADmr Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
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: stable@dpdk.org

Signed-off-by: Yunjian Wang <wangyunjian@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?

--------------dSsueHujMpqJhR9iutM8ADmr--