From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B5C41A0519; Mon, 22 Jun 2020 11:59:21 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 332B91D5DD; Mon, 22 Jun 2020 11:59:21 +0200 (CEST) Received: from FRA01-PR2-obe.outbound.protection.outlook.com (mail-eopbgr120124.outbound.protection.outlook.com [40.107.12.124]) by dpdk.org (Postfix) with ESMTP id 2A2D01D5DC for ; Mon, 22 Jun 2020 11:59:20 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Oa49iCCEkINutdacTM7YEAmPm249m11simn8MJp+LhD9lLf64J1zqg2zfIa+yOzAEoe2R3f/d7PgRT0DsyXCTS9Cdv+I8QIsj6+GA5FHQ+1QxNXmdQVH5O0N4UOwTUy9IelKNKgnziytSLWdx/hYtWnvLWRGf2evN/Uvc3iteHMroyZ1NxOPAKqhNp6V0oB2qJ+IpZUNcgg/D3ZSsYztXOPjZOIhZHGrhyia153owVLvJKPn7ClBRlfddqRikKyPmPb7k8a9ttIIIkHANBntHAHbRP7tSMWiu0YuFUMxoav3UNqVWduz0BMLLyXtZ6FvtFaHxxMSg8ZpgRGjambb5A== 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-SenderADCheck; bh=D+nek4BItZLpLrjQJ6SAjTgOKVHPKk87z0a4PFTOLvo=; b=GnjdR+mQhuIkNaS1sVjxhdwL+hOfz1LJvAQbo38pYZK3WI3xAIRC3hBeth6u42xahRND6hf3UGtTw1tuh5P+cvpdz/hkgjskogsWTQuqijvTJqEwN5FDZsS56Zb0Be+LRkVQXIg3q5W87xmMmAZeHj7dyG2/1JPEl0WJjlWjLGSPVN6WWQmr+bajCM25PQqEQ+SCfg8MFI5iug6piDEPCnHGWlcw5URWgRd7uUMirPD8dIb2A3lDz0VmZ5MoF7XA3OxxSHGaXqecT6ip7u/2FDPZ6lG8sVb9z6215KFHPH+IJemqz6WbTsX2nuYZ5OguaRctfnon7ZO3Ly4WHrFo9g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=ekinops.com; dmarc=pass action=none header.from=ekinops.com; dkim=pass header.d=ekinops.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ekinops.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=D+nek4BItZLpLrjQJ6SAjTgOKVHPKk87z0a4PFTOLvo=; b=OHPFhPkVj6pz67dnOx66gM/ZwuFb52ZmlKplvAPg+TU/+J3G8Gt+l2MVyisnvJAwBWnZH57IZI8Ix6dJZrkbftfr4oLv6q7/ybdg6oaQvLjaWPsKeJZ3vrKRTa+jTEceEA03m5Hj1uqSH+m5dT4Q02I51ESuX6/2WgbjfDiHiMk= Authentication-Results: tuxdriver.com; dkim=none (message not signed) header.d=none;tuxdriver.com; dmarc=none action=none header.from=ekinops.com; Received: from MRXP264MB0120.FRAP264.PROD.OUTLOOK.COM (2603:10a6:500:1b::10) by MRXP264MB0072.FRAP264.PROD.OUTLOOK.COM (2603:10a6:500:24::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3109.23; Mon, 22 Jun 2020 09:59:19 +0000 Received: from MRXP264MB0120.FRAP264.PROD.OUTLOOK.COM ([fe80::b9c3:a77c:6837:2548]) by MRXP264MB0120.FRAP264.PROD.OUTLOOK.COM ([fe80::b9c3:a77c:6837:2548%6]) with mapi id 15.20.3109.027; Mon, 22 Jun 2020 09:59:19 +0000 To: Ferruh Yigit , "dev@dpdk.org" Cc: Anatoly Burakov , Thomas Monjalon , Neil Horman References: <20200513131425.27817-1-Renata.Saiakhova@ekinops.com> <7e3ef1e4-cca6-c113-3d15-648265d3072f@intel.com> From: Renata Saiakhova Message-ID: <39f6e7d4-1228-3b51-f659-75fd168e17a0@ekinops.com> Date: Mon, 22 Jun 2020 11:59:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-ClientProxiedBy: AM3PR07CA0140.eurprd07.prod.outlook.com (2603:10a6:207:8::26) To MRXP264MB0120.FRAP264.PROD.OUTLOOK.COM (2603:10a6:500:1b::10) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [10.0.21.135] (91.183.184.98) by AM3PR07CA0140.eurprd07.prod.outlook.com (2603:10a6:207:8::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3131.11 via Frontend Transport; Mon, 22 Jun 2020 09:59:18 +0000 X-Originating-IP: [91.183.184.98] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 10f962fd-84dc-4e7a-8c21-08d81692ee27 X-MS-TrafficTypeDiagnostic: MRXP264MB0072: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-Forefront-PRVS: 0442E569BC X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: kqnifCSxTk2mHo65ASYWGlfK03897K8tBi8Yyht1765zXAFWF35y9ZfUAXm4oFwerZq5xL7neiSFgGDM0PrHEivFhW41UjrKa+qPW2vjMwj2WA5ovvSDKHR4mAcIfOL3QQbJfCX89REINkK3kruWkk5/KyTDZzzG5HcM00LQSnUdz9ZX77NERolleZqsnH6tr1+/pvLx67HyJ7QctiOrm0NW7SV2SdpL91CLvBu5v4rKVTv55j4PzODUqBWY18WVegUyvFGMyE6G1L8GzaZRuXLqdiA/JHWazxJfzoPaV+31Y0yowActVD8hIeBHdiNRg/5LmEgWNtwHhPpZagrAAToCUMLlxz35AD0DnjZPgesgbYEKjB4rCF9JKqGBnFFA X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MRXP264MB0120.FRAP264.PROD.OUTLOOK.COM; PTR:; CAT:NONE; SFTY:; SFS:(376002)(366004)(39840400004)(136003)(396003)(346002)(316002)(186003)(2906002)(478600001)(31686004)(8936002)(8676002)(2616005)(36756003)(956004)(66946007)(66476007)(66556008)(16576012)(26005)(16526019)(110136005)(54906003)(83380400001)(31696002)(4326008)(53546011)(6486002)(86362001)(5660300002)(52116002)(43740500002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData: vT4omuENBxF07sBbjE6F5KVIe1bA8l9F1OllsSZamy6tvNrtDB5kKRtTJ3gB4b1cmZInnpEUKNxWS028NNyBUr9Csm9L98AU4cObC16rFgG/bWUP9/HwTDD17h8VWEky1Cs29evykPjciv2Dw8UOnITx806YjxGDHMN2hPdjZkpMEKO9RdfwUgx02uq3rbIDrI+oCbNuX6EHFXLrrNVMUdNVhY+RTAKeUBIgH67hfDrVTx8OfJhN/ryvYls8jYA3JmJkHU1OKrDW6xv4Vvscve0KcOhFmkIeS7P6Xyjz8QC1qL9BMn+OmCGT2gNdS9RknrRr07pmtE0ce/nDIpZ3Fiznu7Q1lFKmozcL8BXxoVwdIVRxtVQtKLbqc8co6UlbFh4omwH8HRnrDhG/RN2XwfQ8CaA6HWO0OurHIyo1nB2LwqhdhDRZCqgHwj/2ssR15ngIkgS3w/js6V+TQSf3jcYgtpV4ozS3JQ+i7wQ0M+4= X-OriginatorOrg: ekinops.com X-MS-Exchange-CrossTenant-Network-Message-Id: 10f962fd-84dc-4e7a-8c21-08d81692ee27 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jun 2020 09:59:19.2379 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f57b78a6-c654-4771-a72f-837275f46179 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: zg0/wLv7rEbCD0xcUUfzZFSm11G6d7mqWvyCmfOiSrNJHZ59dC+IeL1Z6ES8lTJs81HPf1evaJSHhnh+nLcVqt6A3qfNh5U0I1LhZi+CKEU= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MRXP264MB0072 Subject: Re: [dpdk-dev] [PATCH v3 0/4] Memory corruption due to HW rings allocation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Ferruh, I've just send a new corrected version of the patchset, which takes into account the comments of Wie and Jeff, hope it is OK now. Kind regards and thank you, Renata On 06/19/2020 06:54 PM, Ferruh Yigit wrote: > On 5/18/2020 10:48 AM, Renata Saiakhova wrote: >> Hi Ferruh, >> >> thanks for comments, >> >> are the rte_eth_dma_zone_reserve() calls always used to allocate HW rings? It is >> not totally clear to me. That was partly the reason I don't do the fix for every >> driver which uses this API. I made fixes in the drivers which uses the same >> pattern to allocate / release queues, for other drivers I was not sure but >> anyway I couldn't spend more time for further investigations. In the company I >> work for we use dpdk for our project and maintain it in separate tree, and the >> vulnerability with HW rings is a real issue for igb and ixgbe drivers and needs >> to be fixed. Therefore I would like this patch to be accepted in order to not >> maintain the fix ourselves. But unfortunately I don't have resources (e.g. time) >> to fix the issue for all the drivers, because, as I mentioned, they are not >> following the same pattern to release their queues. So my proposal is that I fix >> it in this patch in a number of drivers (including igb, ixgbe and i40e) and >> others can take over and improve other drivers, if they see the same issue. This >> is also a reason why the drivers' changes are not in one commit for all the drivers. >> >> For the proposal adding pmd name as prefix to queue memzone name or update >> the 'rte_eth_dma_zone_reserve()' to check the size & alignment instead of just a >> name, I don't know, as an external person, how sensitive dpdk project to change >> an internal API and existing code (the call should be changed in all the >> drivers). But anyway, I think the real problem is more an absence of memzone >> pointer, and in long term it should be solved in this way, rather than search by >> name. > Hi Renata, > > 'rte_eth_dma_zone_reserve()' returning existing memzone just based on 'name' > without checking the size, alignment, socket_id seems wrong to me, let me check > it, it shouldn't be hard to update. > > But even 'rte_eth_dma_zone_reserve()' checks won't be enough (what to do if > check fails and not returns a memzone?), the proper solution is PMD free the > memzone as they removed, as done in this patch. > > Also I suggested prefixing the memzone name as workaround but instead we can go > with this patchset with the implemented PMDs and other PMD owners can implement > the free when they need instead of workaround. > > There is a comment from Wie to add more frees to i40e, and there is another from > Jeff on 'rte_eth_dma_zone_free()' return type, can you please check them? So we > can proceed with this patch for the release. > > Thanks, > ferruh > >> Kind regards, >> Renata >> -------------------------------------------------------------------------------- >> *From:* Ferruh Yigit >> *Sent:* Wednesday, May 13, 2020 5:22 PM >> *To:* Renata Saiakhova ; dev@dpdk.org >> *Cc:* Anatoly Burakov ; Thomas Monjalon >> ; Neil Horman >> *Subject:* Re: [dpdk-dev] [PATCH v3 0/4] Memory corruption due to HW rings >> allocation >> >> On 5/13/2020 2:14 PM, Renata Saiakhova wrote: >>> igb and ixgbe and some other drivers allocate HW rings using rte_eth_dma_zone_reserve(), >>> which checks first if the memzone exists for a given name, consisting of port >>> id, queue_id, rx/tx direction, but not for the size, alignment, and socket_id. >>> If the memzone with a given name exists it is returned, otherwise it is >>> allocated. >>> Disconnecting dpdk port from one type of interface (igb) and connecting it >>> to another type of interface (ixgbe) for the same port id, potentially creates >>> memory overlap and corruption, because it may require memzone of bigger size. >>> That's what is happening from switching from igb to ixgbe having the same port >>> id. >>> >>> v2->v3: Remove #undef ETH_DMA_MZONE_NAME and minor changes in code standard >>> v1->v2: Minor changes on code standard and additional fixes in i40e em and ice drivers >>> >>> Renata Saiakhova (4): >>>    librte_ethdev: Introduce a function to release HW rings >>>    drivers/net: Fix in igb and ixgbe HW rings memory >>>    drivers/net: Fix in i40e HW rings memory overlap >>>    drivers/net: Fix in em and ice HW rings memory overlap >> I think all driver patches can be squashed into single patch, overall they are >> implementing same logic. >> >> But as mentioned before, there are multiple other drivers allocating HW rings >> with exact same name. At least I can see: >> iavf >> nfp >> fm10k >> axgbe >> >> Or how can we know if a new PMD won't cause exact same behavior? What to you >> think adding pmd name as prefix to queue memzone name for all PMDs? This can >> help new PMDs using existing code as sample. >> >> I don't know if it has been discussed before, but wouldn't update the >> 'rte_eth_dma_zone_reserve()' to check the size & alignment instead of just name >> fix the issue for all drivers without needing to update them?