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 AAD18A00C2; Thu, 6 Oct 2022 11:42:37 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 497DE42BD8; Thu, 6 Oct 2022 11:42:37 +0200 (CEST) Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on2081.outbound.protection.outlook.com [40.107.94.81]) by mails.dpdk.org (Postfix) with ESMTP id 3696D42BCC; Thu, 6 Oct 2022 11:42:36 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mI9CQBYJUndUCQsRLOL2/5iKL8yfBVUKB/M/eGVjwxyHxgRQT6NU/9xVq9PXX2jkBUYdRsWY5CiYZ51/CVLDQzgy387efEJZ3nX0s9UdgInYWtWezH5j8TQ/1IGtMGeJB/AVUR+TxJVAt8Pm/Bii+gb2YMbLcq7k9myL+oGTfNUlGmJH5gVlbKM47oGDTKPWtbHEeC2Uj2n1EfjZ95RNkqZRv5R0AF/vXobYdaoctL2qaNWnavEkZQ7vE77a3vBzeAmY80EtC9aw6gZ/lw7ek6J+JxbqNvFoFC8yb9ipI0ffiJXlFl/3/UfujWdV1IOUGpezRTg54h8KL7zAFb9zmg== 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=VPzdN5848gcM3OfngEdA/gfY7ZMILPVegPsVBlDNcBo=; b=f5tZCPEG4bga3+x3KOQrC0lwzhIqqj8p1FhO8GxO2KL71Z6Asq2Wxiya4GCP+ZZYUeCn4zV+UkQzG1CugRF0WvUqQqyQTXm7EQyFlWzjLU9iMzZR+e0+rgj2Rc+kVPkI/vE3XbzoPGGMbR15AZVa0ehF93cqMkN0ofIY6rrl3IM1fTtTiDgDCwUA1rNKz4yEDWZagACGmyXL8DiKj0e0JrngL4g94CtqHGUU7l4stsR+CRJI0+BpO8fPNESlV3thJqNdyUD27uGKNlHS5DSPiP45dAhSmJF2ac40Wo+uiSgH75gZsIa2Cl2o4lSxeyHHaVsrphMIFUBnPyKd3GTRew== 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=VPzdN5848gcM3OfngEdA/gfY7ZMILPVegPsVBlDNcBo=; b=A4KRNI+a8N4iZ/tBRScy11R0tRzkQTwVFhDUrzm6HJEZa8RMbpQVARqBY5RnvGqHFqCgPGQ/g3H/Xlm+BADyc7KA6Dt3M7T4U4sH43pu8p+XofY234BnyCrBzfrmLJUCMs2u1MoKFiUmt4N4hYVwPXGSnJ/kbVk2WoNtg2kVVfM= 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 SA1PR12MB6946.namprd12.prod.outlook.com (2603:10b6:806:24d::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5676.20; Thu, 6 Oct 2022 09:42:34 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::d07a:463f:6f93:337f]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::d07a:463f:6f93:337f%7]) with mapi id 15.20.5676.036; Thu, 6 Oct 2022 09:42:34 +0000 Message-ID: <3a635d69-ce9c-3240-ee3b-dccd6b3d0880@amd.com> Date: Thu, 6 Oct 2022 10:42:29 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH 15/15] net/dpaa: fix buffer free in slow path Content-Language: en-US To: Gagandeep Singh , "dev@dpdk.org" Cc: "stable@dpdk.org" References: <20220928052516.1279442-1-g.singh@nxp.com> <20220928052516.1279442-16-g.singh@nxp.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0143.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:9f::35) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|SA1PR12MB6946:EE_ X-MS-Office365-Filtering-Correlation-Id: a7150816-7aa6-478c-8969-08daa77f188a X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: KnxTf0/C0OCHJtCp1R5z1yyBIc9ov4VnJjIRk0kaAvNtxuBR+PkptD8sAYpjHMI+syjN5dCWj2PrRgCrcPMA1Z0nFSMmxGfU3zjD+18uUNWkCl2tlO8i3pWD689+OEokxnFDLlQJ4RTqB7jZsfPWVB2AOK1QC7IJBgjlBPGQMAr2Hp+t5XmxALSWpgJxhdERm2Aa9FXSiCKU8jVBp68x6k5VPtmrFAzZDEwKu/YIvARQRIU886J++Vjiy6q6kVxFkRmP733W8Y82ywRALHWWsjx3h2PGbjHWdz1EKgCIkBOI7rbvghx3izkca8FZfUjsxBS5q0ISiTNcJa5dJTDeA3G5h/2iX1DMzq12cSRiZhzk5RXwnAyHZKw+ICDhCGcokUxGmMTKb15K2V8ckVKhpXJYdEzkvQWroPcl2YPUN9m08lO8d3X1jUKAJosavLy7jEKM/sEJywtfPG75Kr6344iN50izYiNZLCIqbxM6bc1+QfwhzXfUZqoAVcRFGVUwTHPAMGU9YISIUuILNAGvn1TEzcO/V5Z6IxsXEjepu2heXsIvOQX5ghT+CxOJMTaZgFvhKWDoQlCSI3GhUkhgE/Xp2uKqdJvNgXna/twwBAC1r0uhZKDoLlBuAss0lo8loocByAZJ/+fVghm+NOzGJnMT/vFoSCgyMsRk5krk3jTOLCTO5yGUnfl6q6Iwhb6P3ioMrgLfhylo64+rwpooOLBFhOuAOmIJNGPFmJp/uVZDbvngGZxo88Dm4+GE2FrZa0jegr0t0FUst9p8x+gTj36sSWY8zcYJXDN6VW9BO64= 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:(13230022)(4636009)(396003)(366004)(376002)(136003)(39860400002)(346002)(451199015)(38100700002)(31696002)(86362001)(5660300002)(66946007)(66556008)(4326008)(8676002)(66476007)(316002)(110136005)(44832011)(2906002)(2616005)(41300700001)(8936002)(186003)(83380400001)(6486002)(478600001)(6666004)(6512007)(26005)(6506007)(31686004)(53546011)(36756003)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OVZSVFowamJ4a09ub3BUUUp1eXJCUkpHWnlOaURrU1AxdldXdTdObDlZcVF5?= =?utf-8?B?TGlteXoxSGpURmQ1UFU3cTl4ZGNkNmt1VHM2VEZMYytlTFpQa3EweDhYNmNO?= =?utf-8?B?a25jaVVraytkWVp4U1I0UFZNTmw5aTBycWk2Z1MycVNFYXMxT05PZ2Z6QVdm?= =?utf-8?B?bUo0S21lSE1NVVNHSFFZSGI5eEJpMFFPdFp4SEZTREF5c3NJczdxNTFDSEtU?= =?utf-8?B?YXZFYlpwQmt0RmpTT2NSKzRVbEJSazdlNXRZSUM0bFg2emtQYkIwS2pKYWlm?= =?utf-8?B?TFNYeFpqZWx1WnhZSFpLdUdaSXBadS9rRndQN2czdno1eTV3QzRUalRUQ3VX?= =?utf-8?B?VXpXTStGOEppSFptRURkb0hidU5xNjZ5enN5dGdlME5UeXlTd0t2cW90aDVC?= =?utf-8?B?bGxIa1UvSWhzS1Nsa3RSbXo2enlWdlc0SFhzNWpkSmpwcHZMWHc5TzhiUUlM?= =?utf-8?B?U2hWY053YXdFYStBRUl0ZjZtVFlLVUgvTStHSmNQQnJCZ3IvVjRqOHorUitT?= =?utf-8?B?U0lxWGpiZzlkVVlnUGVSRkc3RmlxSWZRNzlmblY4bVBDRHkrR2ZXVVlNTllW?= =?utf-8?B?V0wvWXNDRzVLSFJUaG4zblFSdUtNY1JZaDl5bXM0akNuaUJWaDA0NXlwa05z?= =?utf-8?B?RDFhcGQ4YjZaZkpOVzNBdzVGQW1aY1N0N2ZyQVFINElnVHBDZEFnRndiT2Qz?= =?utf-8?B?U0ZsSG5HYWhSdzRFRElPM3BITzVsd0tTQityVVhjbDdNTVpVUVZOOThsV0dL?= =?utf-8?B?ZHpWSkRXb1lUVjluMTBmUzFSMlhHMVJPSll2QXgwOTNUSkZiNm40OU9zVUk3?= =?utf-8?B?RGtTYzV4S2hBTzZkVTJjaFdMK1FDSjZtSzIzWlZQRGxhT1dCemM0NWQxNW1K?= =?utf-8?B?V3NVRHBjejdoZEVhNVFCdUMwdUdyb2RhV2hsN1lPUUR4ZTJnSHVITnB0bHlm?= =?utf-8?B?WHgwUDJQM3Q0MFZvbzl3ajByVUZhT3NIL0VXVmFWS0sxZjFwdlNhMkgrK25M?= =?utf-8?B?eFJGNHRKOWpMemR4b2ZIZEd6QWNoSHlVeTJQTVRhVmpoZUZ1dkhxaTlOcGk5?= =?utf-8?B?aytFRW8xczlOclNnSnoyQUJ4Tko5T1ZzSW9tM2doZUVhMGp6b3RWMTRmRXJE?= =?utf-8?B?dFNVRG5kd2pDT0srRzhEeEFoWmlCSjRVZXNJZ09Bd1p5dThIQUxvcDNTVjdR?= =?utf-8?B?ckV6L25nL0pQbUU0WXpQcFdlZGY1NzFsRzJXRjF5ajBJaGZEREFnTHFvelRq?= =?utf-8?B?R2d1STR2L3FnVjBEZDYralF5S2svaUE5M29IbHBuSjZyNGl4djFWcjFhMUFU?= =?utf-8?B?ZThiV05RcVhYN3VHNjNWU0pMTFFjMEtIUUlsNFA5L1BydVhCKytzcmNDakVI?= =?utf-8?B?S3JRZG1qamZpUm03WW45Vk9jcnFRUDF0Rmd2Wm03ZjdhUUMzMmVTaUZ6cjNh?= =?utf-8?B?M0g2RzZraEwzWmpqYXpyMjFYZFRuSVBhRzYwVnh0RE02dk00M0VTcTNGT1Bz?= =?utf-8?B?SVZSS2tWVzkwdFkreGtLU3hrSkluT21zc1VTOTRhb0p5ampTdDhIUGhSejdj?= =?utf-8?B?N0FmSUZPQkxJVytmb3RwTk5obWQ4V1hIVU1OOG83TmdxTE1ra202S0RWWEha?= =?utf-8?B?KzVHblZYamNTV3RvcGJJdEFGTXNicGoxUHB1NGkxWTVDWnBrQm1rMHlXN1pS?= =?utf-8?B?K3VHbDRuQ0x6U1J2Vy9NcmFZazNMNGtvczJxNUh4bGVlSTZWczBhbC81MzZP?= =?utf-8?B?ekxyRSsrWjQ2eEJSQXloZFRuUSt1K1NBbENlblBzNThEOE1XODl6cDBWRFpN?= =?utf-8?B?UDdTM1ljcDhKQUpGQXppOENIanZGMG9wSU1hTlNKdmZZdTNmQlk0dk9Kczd1?= =?utf-8?B?TG0vVVBYMzRSVENOMyt1cFk3MlFqVFBZK283WUZacTNvczJDaVBnL3FJSkc1?= =?utf-8?B?OGZkS2ptZUF4RVZNVmkvVkhOS0piM0FiMTlGUjhESFgydFYvT3kvT2IrMGRk?= =?utf-8?B?dlozcVB1U3dQUEY4VEJ4Y3NIMXMvTUdmRFQ5SmVTYVhOZTJEN0FlOWVVV1A3?= =?utf-8?B?bEgyWXQ4TXhNa2ZhOWpBYTdTYmE3cC9SSkxYR1NJcjlOL1lzbUpSa2w1Nk1J?= =?utf-8?Q?qEmRQuZFwGOtKT9+34B+zQhw3?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: a7150816-7aa6-478c-8969-08daa77f188a X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2022 09:42:34.2951 (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: 8QBPhEriO0B4cYXBVVK9Js3spwpPphetoo3m4z8p1t4yXdQqwkaNQfQidBsYvFJo X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB6946 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 10/6/2022 9:51 AM, Gagandeep Singh wrote: > Hi, > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Wednesday, October 5, 2022 7:52 PM >> To: Gagandeep Singh ; dev@dpdk.org >> Cc: stable@dpdk.org >> Subject: Re: [PATCH 15/15] net/dpaa: fix buffer free in slow path >> >> On 9/28/2022 6:25 AM, Gagandeep Singh wrote: >>> Adding a check in slow path to free those buffers which are not >>> external. >>> >> >> Can you please explain what was the error before fix, what was happening >> when you try to free all mbufs? >> >> Also it seems previous logic was different, with 'prev_seg' etc, can you >> explain what/why changed there? >> > Actually, there were two issues, this function was converting all the segments present in HW frame > descriptor to mbuf SG list by doing while on segments in FD (HW descriptor) and in the end > it frees only one segment by calling the API rte_pktmbuf_free_seg(), so for other segments > memory will be leaked. > ack > Now in this change, doing the loop on each segment in FD and if the segment has a valid > buffer pool id (HW pool id), freeing that segment in the loop itself without converting to a mbuf list. > if we free all the buffers even those with invalid HW bpid (which will only be the external buffer case), > then there can be double free because all the external buffer free handling is being done by the > Xmit function. > Got it, can you please give more information in the commit log as above, and can you please elaborate impact of possible double free, will it crash etc? >>> Fixes: 9124e65dd3eb ("net/dpaa: enable Tx queue taildrop") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Gagandeep Singh >>> --- >>> drivers/net/dpaa/dpaa_rxtx.c | 23 ++++++++--------------- >>> 1 file changed, 8 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/net/dpaa/dpaa_rxtx.c >>> b/drivers/net/dpaa/dpaa_rxtx.c index 4d285b4f38..ce4f3d6c85 100644 >>> --- a/drivers/net/dpaa/dpaa_rxtx.c >>> +++ b/drivers/net/dpaa/dpaa_rxtx.c >>> @@ -455,7 +455,7 @@ dpaa_free_mbuf(const struct qm_fd *fd) >>> bp_info = DPAA_BPID_TO_POOL_INFO(fd->bpid); >>> format = (fd->opaque & DPAA_FD_FORMAT_MASK) >> >> DPAA_FD_FORMAT_SHIFT; >>> if (unlikely(format == qm_fd_sg)) { >>> - struct rte_mbuf *first_seg, *prev_seg, *cur_seg, *temp; >>> + struct rte_mbuf *first_seg, *cur_seg; >>> struct qm_sg_entry *sgt, *sg_temp; >>> void *vaddr, *sg_vaddr; >>> int i = 0; >>> @@ -469,32 +469,25 @@ dpaa_free_mbuf(const struct qm_fd *fd) >>> sgt = vaddr + fd_offset; >>> sg_temp = &sgt[i++]; >>> hw_sg_to_cpu(sg_temp); >>> - temp = (struct rte_mbuf *) >>> - ((char *)vaddr - bp_info->meta_data_size); >>> sg_vaddr = DPAA_MEMPOOL_PTOV(bp_info, >>> >> qm_sg_entry_get64(sg_temp)); >>> - >>> first_seg = (struct rte_mbuf *)((char *)sg_vaddr - >>> bp_info->meta_data_size); >>> first_seg->nb_segs = 1; >>> - prev_seg = first_seg; >>> while (i < DPAA_SGT_MAX_ENTRIES) { >>> sg_temp = &sgt[i++]; >>> hw_sg_to_cpu(sg_temp); >>> - sg_vaddr = DPAA_MEMPOOL_PTOV(bp_info, >>> + if (sg_temp->bpid != 0xFF) { >>> + bp_info = >> DPAA_BPID_TO_POOL_INFO(sg_temp->bpid); >>> + sg_vaddr = DPAA_MEMPOOL_PTOV(bp_info, >>> >> qm_sg_entry_get64(sg_temp)); >>> - cur_seg = (struct rte_mbuf *)((char *)sg_vaddr - >>> + cur_seg = (struct rte_mbuf *)((char >> *)sg_vaddr - >>> bp_info- >>> meta_data_size); >>> - first_seg->nb_segs += 1; >>> - prev_seg->next = cur_seg; >>> - if (sg_temp->final) { >>> - cur_seg->next = NULL; >>> - break; >>> + rte_pktmbuf_free_seg(cur_seg); >>> } >>> - prev_seg = cur_seg; >>> + if (sg_temp->final) >>> + break; >>> } >>> - >>> - rte_pktmbuf_free_seg(temp); >>> rte_pktmbuf_free_seg(first_seg); >>> return 0; >>> } >