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 5E1C742B2E; Wed, 17 May 2023 20:26:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F314240EE1; Wed, 17 May 2023 20:26:03 +0200 (CEST) Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2045.outbound.protection.outlook.com [40.107.223.45]) by mails.dpdk.org (Postfix) with ESMTP id 3CAE5406B7; Wed, 17 May 2023 20:26:02 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A3fIfpbny3eYWwNvE8OnVQ4Nn6aatBNtQktaTOpcZxMNZS1pKOwN3aBFHfarwhnSDPbAx9FZdD6KtCV4vunf4tfDHicafyUCwNtnbl2bbG1u5+qYiZyfwaUaRptLiH9Ia30rwuCbsbgRou3g6aAj3Pj78/XiUUOu2x5k6gZmhykj2Ix48I7I4AFVz4gcDseVxEmm3nnFo+OQKSubwY/MXrWt9QgR7q1pQcdQx2nZtnRHCMH+TtGZiBpDC3WPc8ZxiR8VX1Zl0Q0hN2uVkxO6PTvmGYP0VX+llwS0mR26PCk+h716CZ3m00X/P/SjPs53A/o6LTyfFzSQ6fldgnxT3A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3sVxj7nroRD+zWWgKmHkwAQI01uPJE/z5kE77OFBSQk=; b=mY+qEINzggrWDiOjdtRS1eLDGQ33gwXCmjDxaP4qbGczcraU9OBNk2w0VCO98sicnk9UzxVL2B6WCrY1fczyx6UsI1YooM+k9O+Ar6KemH+7RobKHmFjG+moyoYlcIoW7UCAhEo1OgYPnsn+2nxw9Y5mmkvqrijXhTaKy1Tj9Z+cdJBIaNYx7c8xnzYv9WAJjhuO+MdQIjzvqbNFIKLEkSQIxFyFd9kw61bqXuKFhDVsvQ4i+EhxFgxbHBkel6zrvns1RocTFMvQDApDfTB0CH06TtOiriX/impxiIXfS6tzCXAXVkhiTJYa35ttvFQynkqyEQ+495ogQ/uYVOVDZg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3sVxj7nroRD+zWWgKmHkwAQI01uPJE/z5kE77OFBSQk=; b=TdqwozNP1EwCJBNccMu96CH+C92zkswVokTl/aKmK0ZQzYyTEw2hZC1Zi/ysZvNyj3DgslMJ9yUJRRpJcC8fVXFBov27IkKhXWs9zT4CZqBNAD6elFz2/7k7mk1QGCoSkt4KP10Rgj3ZaomVuPc4phSBgfQw2MtgVQLXPsBhmtQ= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by DM4PR12MB5914.namprd12.prod.outlook.com (2603:10b6:8:67::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.33; Wed, 17 May 2023 18:26:00 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::7957:641d:6aba:3f9a]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::7957:641d:6aba:3f9a%4]) with mapi id 15.20.6387.033; Wed, 17 May 2023 18:26:00 +0000 Message-ID: Date: Wed, 17 May 2023 19:25:54 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Content-Language: en-US To: Yasin CANER , dev@dpdk.org Cc: stable@dpdk.org, stephen@networkplumber.org References: <20230516104853.479297-1-yasinncaner@gmail.com> From: Ferruh Yigit Subject: Re: [PATCH] lib/kni : fix memory-leak on rte_kni_rx_burst In-Reply-To: <20230516104853.479297-1-yasinncaner@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0131.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:9f::23) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM4PR12MB5914:EE_ X-MS-Office365-Filtering-Correlation-Id: 68bb18f9-5306-41cc-73ca-08db57042a03 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 3WxtGdE2YyasKK47UcL5//XFUtIflOYJnYf8m5gShl3QqC0LPNXNPJPRKXw35MkmKNGd5LkSzv8LWWBe9pu2fDng4x8GLSqoPMwpeGir/SW/y8+vUQkF+pvgyg2BeedrsMmo+P8lhP7Vb3yY7w/zYX999h4lAPN9T29ijyvV8sHgC5SFfpIgurqz/AgXgGKDwfT97DE+iP7fSVPQwqcL9/krSiZcGMziRRAIMdJW+bK4d7P1bE5uO/U7OlkGuwtc0dIUHNMwv9d0HSaMHHoxjeU2AbnS4hJJh142ZGRx0P8hSOduMifXgIcXzVqW3y8NB4GX2adEDQ+35BABk1yInQcASAKC9PwAHdO1K4OSSNCXkCwu/sh8G9kFW3Qd0SAL4VIx4ILJA2lhSY6/8O034L75X7MHM/bLAHm4Bs8oT0ilnAECLBUH5MFL3FRu7sdXV/xAQ8TA7XssCvAgcxSSJFIa3DZmzQZlQIxVw+PqGc/RnlW9jKXPQGpPg30v3gCFbcCbgBso5cClnMgYuEnOFWzrDfCMxCBlOBjQlnbipFlljJ8eyWLOoX1PXzYfHQKW1jYIiKdgT1Q5yXK10HimlBPU9qS4Kj7ihS2drCYh6/CZ9xOSIqkmdqUltINC0YanNx+MgV+leOg7NGtpR8SDRA== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(4636009)(396003)(136003)(376002)(346002)(39860400002)(366004)(451199021)(8936002)(38100700002)(66476007)(6486002)(8676002)(44832011)(66946007)(36756003)(4326008)(6666004)(66556008)(478600001)(2616005)(186003)(41300700001)(31696002)(5660300002)(86362001)(83380400001)(2906002)(31686004)(53546011)(6512007)(316002)(26005)(6506007)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bm1VYmwvbG8yZGJDa0JsVkpMUW10YUlFV0tNelpBUHlURWdOaFVqY016ZnFX?= =?utf-8?B?UVdxS1ZBK2lpdmpvQURtMWJ1eWxDRXhuSnFIS1Rqck1Pc0pGdE4reDZBa3Z6?= =?utf-8?B?TWo1TzJNMDI0US85ak1GVDdnZkNjVUgyK3hSa01aQVAxSHRDNkRVanh5ekk1?= =?utf-8?B?M1JUMC9JcmdENEEvWklGTldUQzBpSUNaTVkzVzlQSDg1WWpoSkF4ekhOTHJH?= =?utf-8?B?Z2ZkOUJaMlJCV0lYa01OSXlRMjlpOEYwMDZQaHpQcUJac0pIM1BIQjZiWUt6?= =?utf-8?B?T0FkZ3ZIMkpFTitKTnpUbzhuVVdwRmZiQ0VBMGpxNG13MlFBd3ZSbWJPR0J4?= =?utf-8?B?c3Bla210MUs2bDhHcUlpQ2pvUHZCSnVaU0gxSk5UMjMyWEFKZlZTSldERGs1?= =?utf-8?B?aXZqTi9vNzllanRZU0VLMUZBWmkwTm5Fa3FKWjZ2V1F4U1VrWVpEWG9ha2RU?= =?utf-8?B?WXhuNXc2Y0JwZURmSkMxVHAvQmtLaVpDaHFEY0xpTWZJUWk5RUdzR0xKQm9J?= =?utf-8?B?Z2xtOXZPT0Q3c3E1UUFDU1lzbDVQR3ZXdTRjU3R2dFNLUHdCazhJeDlrZURw?= =?utf-8?B?R3g0NjhSUlJwdEZWRWtSZ3hxTDZ5b21ib1JoLzdzL3M3Y1VlblI0M0FTNjg5?= =?utf-8?B?UE44T1JpY0ZSZkQra1MzRStWeENXSzRpSjJOWXBNR0RHNWxjMit6NkNENlVo?= =?utf-8?B?WWRlTDgybnBCYVAyYlhLVXh6azNvZk5nOTBaZU84eXl5N3QwdnZYR3pBU016?= =?utf-8?B?YThqc1YrRklaWW9SSjQwN2JrT1R6L3FMc2svaUNCR3ZBRmtEclVmV29rUUYz?= =?utf-8?B?V2pMOWE3S1NaL3hUOEJ4amVsTW5EaFlTN2pCOC8wSGpKaGU0M01iR1R3ZlFx?= =?utf-8?B?Z2RCakxDWitvcmFFWnFTZmtBV3hieEdBT0xwUTh4d3BWbndSbExzbjdSNjIr?= =?utf-8?B?WkRWekJMUXNPTlFSa2R5RXVVQ0RvRDUzM1hubWh6cTRNYWt2VUlhdXFYMHo4?= =?utf-8?B?K3JGVldYWjRrdTJZRUtja0QxMHdibTFuT3hnbmpob2FKbjlOem1qM3c2Ullv?= =?utf-8?B?Rko0NEF0RFRldkJoOS9YeStXL1VWbVVxc2Y2TFd0UDcwd2hVdDJLaUVhYmdB?= =?utf-8?B?d2pYNHBJeTU1YXN4Z0MxQzlIaEluaXg1K0FuNDliVXJ5aThaaDl5UkdLR2cy?= =?utf-8?B?Ykh5N3ZyNEZhVXczOGtYbGU2aURrZFBobG1PWEJsN0VSeUxxM2xkYjlENmdI?= =?utf-8?B?Y2hkaFVVMGNHZVZGWG93aWI5Mmh6bHFsTTlIVGdLUHpsQ3BEZ0p3ZElpUkpC?= =?utf-8?B?WUZvUjcvdkFxdm9nRW1UT240SlVlUFBPWkRhYzBhandZbHByVC9oMkNRa0lK?= =?utf-8?B?cm9CK3ZGWmowbkhQRXRVZnBJaGpXaU1PZ2NLT0dlY2xhaDM1T2F0YXA0Nkc0?= =?utf-8?B?dXYyNElyNnlVY3htS0kxRnNtNWdMZE1jODlJSjVUSlorMGd2OG1TMnBnbzRw?= =?utf-8?B?eVZGNTgwbS9leHROT1crTUd6d2I0ZzYzcDJRRVM3Qk9aOGhyVm9UdDE3aVJK?= =?utf-8?B?aXJLUDh6L2RVcWQrQk91RGVnOU9rdkpNbUJhQUxzWi9VNWJuQWdIQTVsN2Fn?= =?utf-8?B?UEZhaDlJRHVsWTlmUGoxOWRXNnZEaUUyUkpkaGRTOXdqSW14MGlEZzc2YWlJ?= =?utf-8?B?ay9Ccm9KTENrMHh4cjV3WG5vSTNkaytqTlFOL043MnFidGM3WUhCR1ZpSlJJ?= =?utf-8?B?UUR1WVRCbEhiSGZDYlVDV1d6WmFxbjJZZXJVdHk2enhNTkwreEk2Vncvakcx?= =?utf-8?B?OVdQSFZoNG5IWUllaUgrQ1psU3A2VHptTWVBSHZ3WWJreE9PQjBndjByMDAr?= =?utf-8?B?WmtuTzQyZXpqZXc4akdIaStHKzdiQkJvcFZwTzQ4Qy9nSkQzN0liM1NZZ2x6?= =?utf-8?B?QzVoTjhteHRIdGRHTFNYMi8zT1ZuUDlPOVJjUitEVEVKUFFnNDBDUkUzZHFE?= =?utf-8?B?cEl4Z05jclE0UlR0SzRFaFJpSWVhU0diRlNTRGFORzNTYkpoSDdRRkF5M1J4?= =?utf-8?B?NThvTGhTT2oybXFjNkxwZmhNcmhGZTNmNTU1QngvclpGZUNhS2FIaXA2U0h0?= =?utf-8?Q?1RW7tO7RRuSgMsFKi4mvkt7LX?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 68bb18f9-5306-41cc-73ca-08db57042a03 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2023 18:26:00.2459 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: TZCBxjOmzV2pxC6JX7y/pJ9+SI4aNiazmmYbWikgsSYHZmk1q2/ulGXKyK5roq+R X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5914 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 On 5/16/2023 11:48 AM, Yasin CANER wrote: > Coverity issue: > Bugzilla ID: 1227 > Fixes: > Cc: stable@dpdk.org > Cc: stephen@networkplumber.org > > Adding new condition to check buffer is removed or not. > it prevent allocation each time when rte_kni_rx_burst function called > that cause memory-leak. > > Signed-off-by: Yasin CANER > --- > lib/kni/rte_kni.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c > index bfa6a001ff..2244892aae 100644 > --- a/lib/kni/rte_kni.c > +++ b/lib/kni/rte_kni.c > @@ -660,7 +660,8 @@ kni_allocate_mbufs(struct rte_kni *kni) > int i, ret; > struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM]; > void *phys[MAX_MBUF_BURST_NUM]; > - int allocq_free; > + int allocq_free, allocq_count; > + uint32_t allocq; > > RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, pool) != > offsetof(struct rte_kni_mbuf, pool)); > @@ -682,10 +683,26 @@ kni_allocate_mbufs(struct rte_kni *kni) > RTE_LOG(ERR, KNI, "No valid mempool for allocating mbufs\n"); > return; > } > - > + /* First, getting allocation count from alloc_q. alloc_q is allocated in this function > + * and/or kni_alloc function from mempool. > + * If alloc_q is completely removed, it shall be allocated again. > + * */ > + allocq = kni_fifo_count(kni->alloc_q); > + /* How many free allocation is possible from mempool. */ > allocq_free = kni_fifo_free_count(kni->alloc_q); > - allocq_free = (allocq_free > MAX_MBUF_BURST_NUM) ? > - MAX_MBUF_BURST_NUM : allocq_free; > + /* Allocated alloc_q count shall be max MAX_MBUF_BURST_NUM. */ > + allocq_count = MAX_MBUF_BURST_NUM - (int)allocq; > + /* Try to figure out how many allocation is possible. allocq_free is max possible.*/ > + allocq_free = (allocq_free > MAX_MBUF_BURST_NUM )? MAX_MBUF_BURST_NUM : allocq_free; > + /* Buffer is not removed so no need re-allocate*/ > + > + if(!allocq_count) { > + /* Buffer is not removed so no need re-allocation*/ > + return; > + } else if (allocq_free > allocq_count) { > + allocq_free = allocq_count; > + } > + > for (i = 0; i < allocq_free; i++) { > pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); > if (unlikely(pkts[i] == NULL)) { Nack, Above logic caps number of mbufs can be stored in 'kni->alloc_q' to MAX_MBUF_BURST_NUM. I can see from Bugzilla this is done based on a memory leak concern but that concern is not valid. Original logic is to keep 'kni->alloc_q' as full as possible to prevent buffer underflow for kernel side. And 'kni->alloc_q' freed when kni released, so there shouldn't be any memory leak. But some mbufs can stay in the 'kni->alloc_q' for a longer period, this needs to taken into account. I believe it is known, but let me briefly describe mbuf flow in KNI, there are four fifos shared between userspace and kernel: alloc_q, free_q, rx_q & tx_q. Userspace manages (allocs and frees) buffers, but kernel needs to able to access them that is why: Rx path: 1- userspace allocates mbufs and stores in 'alloc_q' 2- kernel gets mbuf from 'alloc_q', stores packet to mbuf and stores mbuf to 'tx_q' 3- userspace consumes mbuf from 'tx_q' Tx path: 1- userspace stores mbuf to 'rx_q' 2- kernel consumes mbuf form 'rx_q' and stores empty mbuf to 'free_q' 3- userspace gets mbuf from 'free_q' and frees it That is why userspace target is to keep 'alloc_q' fifo full and 'free_q' fifo empty to not block the kernel side. If above explanation makes sense, can you also close the Bugzilla defect please? Thanks, ferruh