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 41EADA04BC for ; Fri, 9 Oct 2020 22:43:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 10B521D534; Fri, 9 Oct 2020 22:43:26 +0200 (CEST) Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id C739E1D534 for ; Fri, 9 Oct 2020 22:43:24 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20201009204312euoutp01b9b1f6284a4bdfeb6d821e8bb02c7b18~8bbVPyWyr2895428954euoutp01L for ; Fri, 9 Oct 2020 20:43:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20201009204312euoutp01b9b1f6284a4bdfeb6d821e8bb02c7b18~8bbVPyWyr2895428954euoutp01L DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1602276192; bh=CWEPvf5KRwMRBxTuJhIhpwPIMZJj4YqR8WtBQx3BvOU=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=XWhrBRMNOvZ7t2t2U3jEA96RvKzKP00zOVWuuB8BPP9ZBgnWPdbSFEusEYBpNiCEi M9F7ZHmC1eRvrYqM9nu+rbREW7sn0SNvapwA6UY/TH6nJIbIjYIO1ukeIO7vrPfPHe xRytKYU2eJN0WhvNVfHiFl+QhIcP8yMU3mOEtddM= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20201009204307eucas1p1f70646a061482dc2477e98e5c0f2af5f~8bbQFokIy1160511605eucas1p1Q; Fri, 9 Oct 2020 20:43:07 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 61.B1.06318.B5BC08F5; Fri, 9 Oct 2020 21:43:07 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20201009204307eucas1p2a600f16fa3a428d9935856f4b5e1542f~8bbP1FLk-0533205332eucas1p29; Fri, 9 Oct 2020 20:43:07 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20201009204307eusmtrp157fd5125c38341208d1515818efcc2f3~8bbP0gGe20757907579eusmtrp1U; Fri, 9 Oct 2020 20:43:07 +0000 (GMT) X-AuditID: cbfec7f5-38bff700000018ae-3c-5f80cb5bff80 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 0C.12.06314.B5BC08F5; Fri, 9 Oct 2020 21:43:07 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20201009204306eusmtip160e31265848d595539b2b89e66430a63~8bbPDkjJM0412204122eusmtip1f; Fri, 9 Oct 2020 20:43:06 +0000 (GMT) To: David Hunt , Bruce Richardson Cc: dev@dpdk.org, stable@dpdk.org, "\"'Lukasz Wojciechowski'\"," From: Lukasz Wojciechowski Message-ID: <062f3b1c-3604-8d73-2cd0-be511c6dbdce@partner.samsung.com> Date: Fri, 9 Oct 2020 22:43:05 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: <3fd65112-5230-68d9-ecf3-e6c91df4fcf9@intel.com> Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprGKsWRmVeSWpSXmKPExsWy7djP87rRpxviDd4eErW4screom/SRyaL d5+2M1k861nHaPGv4w+7A6vHrwVLWT0W73nJ5HHw3R6mAOYoLpuU1JzMstQifbsErozpn+6x FhxWqHhwdhpzA+MRqS5GTg4JAROJuz8esnQxcnEICaxglPg9+SwzhPOFUWL/vgeMEM5nRonL 3eeBHA6wlo+LfCDiyxklrs+dxAThvGWUuLrgGiPIXGGBQImVF/8xg9giAmESzc17WUCamQVS Jbb/dgcJswnYShyZ+ZUVxOYVcJO4uHYuO4jNIqAisfXjNhYQW1QgTmLCxhYWiBpBiZMzn4DZ nEC975f8ZgOxmQXkJZq3zmaGsMUlbj2ZD3aPhMBkdolz16awQvzpItGw8icjhC0s8er4FnYI W0bi9OQeFoiGbUAP/P7JCOHsB3qtdwVUlbXE4X8g60A+0JRYv0sfIuwocXvuTWZIqPBJ3Hgr CHEEn8SkbdOhwrwSHW1CENV6Ek97pjLCrP2z9gnLBEalWUhem4XknVlI3pmFsHcBI8sqRvHU 0uLc9NRi47zUcr3ixNzi0rx0veT83E2MwNRy+t/xrzsY9/1JOsQowMGoxMPbkNwQL8SaWFZc mXuIUYKDWUmE1+ns6Tgh3pTEyqrUovz4otKc1OJDjNIcLErivMaLXsYKCaQnlqRmp6YWpBbB ZJk4OKUaGK9qb7D9dLZZuMbWJ3tNwXVWe4OSlxYr2G5dck7YvWaT3cV5S1+/SNhj9PnXrm3z jr/5vk75GKtBx8xj4v+O7c2Mqfh+belTd7Fihqb5S8P33riSXVm+4ECesdrLSawvjD1Wfz0/ PZOdZ27Qyarw1qyWxa7zFZoMNvV27z5pu/Grq3vG4f+rbiopsRRnJBpqMRcVJwIAz5/dxikD AAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrOIsWRmVeSWpSXmKPExsVy+t/xu7rRpxviDR7uYrW4screom/SRyaL d5+2M1k861nHaPGv4w+7A6vHrwVLWT0W73nJ5HHw3R6mAOYoPZui/NKSVIWM/OISW6VoQwsj PUNLCz0jE0s9Q2PzWCsjUyV9O5uU1JzMstQifbsEvYzpn+6xFhxWqHhwdhpzA+MRqS5GDg4J AROJj4t8uhi5OIQEljJKTD/dxQIRl5H4cEmgi5ETyBSW+HOtiw3EFhJ4zShxuDEAxBYWCJRY efEfM4gtIhAmcf3rfXYQm1kgVeJZx1lmiJlbmCV6bzxgAUmwCdhKHJn5lRXE5hVwk7i4di5Y A4uAisTWj9vAakQF4iR+TOxlg6gRlDg58wlYnBOo9/2S32wQC8wk5m1+yAxhy0s0b50NZYtL 3Hoyn2kCo9AsJO2zkLTMQtIyC0nLAkaWVYwiqaXFuem5xYZ6xYm5xaV56XrJ+bmbGIGRtO3Y z807GC9tDD7EKMDBqMTDq5HYEC/EmlhWXJl7iFGCg1lJhNfp7Ok4Id6UxMqq1KL8+KLSnNTi Q4ymQM9NZJYSTc4HRnleSbyhqaG5haWhubG5sZmFkjhvh8DBGCGB9MSS1OzU1ILUIpg+Jg5O qQbG+H2XCm8zhz2O4YvinvHm7dzGn95vZqzaf6co87b9tkmfbB6dZZT0UQj1X7H3gMrfbQe9 7j7ev82fS1OqToNjWZlEoPb6pIsNE2/EPvtYumHiZxeui9O95tu9ElJ6/GNG2bSnXD9W7Atz MJRLP+PV8iBoyWW9IIffU7W1/rKHHtzvtDbZet6hNiWW4oxEQy3mouJEABTNE/a6AgAA X-CMS-MailID: 20201009204307eucas1p2a600f16fa3a428d9935856f4b5e1542f X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20201008052339eucas1p15697f457b8b96809d04f737e041af08a X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20201008052339eucas1p15697f457b8b96809d04f737e041af08a References: <20200925224209.12173-1-l.wojciechow@partner.samsung.com> <20201008052323.11547-1-l.wojciechow@partner.samsung.com> <20201008052323.11547-5-l.wojciechow@partner.samsung.com> <6acff92e-213d-4565-d2ea-4cb71ba97172@intel.com> <43b64806-aa26-68a2-5757-d5a6ec7a8174@partner.samsung.com> <3fd65112-5230-68d9-ecf3-e6c91df4fcf9@intel.com> Subject: Re: [dpdk-stable] [PATCH v5 04/15] distributor: handle worker shutdown in burst mode X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" W dniu 09.10.2020 o 14:13, David Hunt pisze: > > On 8/10/2020 10:07 PM, Lukasz Wojciechowski wrote: >> W dniu 08.10.2020 o 16:26, David Hunt pisze: >>> On 8/10/2020 6:23 AM, Lukasz Wojciechowski wrote: >>>> The burst version of distributor implementation was missing proper >>>> handling of worker shutdown. A worker processing packets received >>>> from distributor can call rte_distributor_return_pkt() function >>>> informing distributor that it want no more packets. Further calls to >>>> rte_distributor_request_pkt() or rte_distributor_get_pkt() however >>>> should inform distributor that new packets are requested again. >>>> >>>> Lack of the proper implementation has caused that even after worker >>>> informed about returning last packets, new packets were still sent >>>> from distributor causing deadlocks as no one could get them on worker >>>> side. >>>> >>>> This patch adds handling shutdown of the worker in following way: >>>> 1) It fixes usage of RTE_DISTRIB_VALID_BUF handshake flag. This flag >>>> was formerly unused in burst implementation and now it is used >>>> for marking valid packets in retptr64 replacing invalid use >>>> of RTE_DISTRIB_RETURN_BUF flag. >>>> 2) Uses RTE_DISTRIB_RETURN_BUF as a worker to distributor handshake >>>> in retptr64 to indicate that worker has shutdown. >>>> 3) Worker that shuts down blocks also bufptr for itself with >>>> RTE_DISTRIB_RETURN_BUF flag allowing distributor to retrieve any >>>> in flight packets. >>>> 4) When distributor receives information about shutdown of a worker, >>>> it: marks worker as not active; retrieves any in flight and backlog >>>> packets and process them to different workers; unlocks bufptr64 >>>> by clearing RTE_DISTRIB_RETURN_BUF flag and allowing use in >>>> the future if worker requests any new packages. >>>> 5) Do not allow to: send or add to backlog any packets for not >>>> active workers. Such workers are also ignored if matched. >>>> 6) Adjust calls to handle_returns() and tags matching procedure >>>> to react for possible activation deactivation of workers. >>>> >>>> Fixes: 775003ad2f96 ("distributor: add new burst-capable library") >>>> Cc: david.hunt@intel.com >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Lukasz Wojciechowski >>>> --- >>> >>> Hi Lukasz, >> Hi David, >> Many thanks for your review. >>>     I spent the most amount of time going through this particular >>> patch, and it looks good to me (even the bit where >>> rte_distributor_process is called recursively) :) >> That's the same trick that was used in the legacy single version. :) >>> I'll try and get some time to run through some more testing, but for >>> now: >>> >>> Acked-by: David Hunt >> Thanks and if you'll run the test, please take a look at the >> performance. I think it has dropped because of these additional >> synchronizations and actions on activation/deactivation. >> >> However the quality has increased much. With v5 version , I ran tests >> over 100000 times and didn't get a single failure! >> >> Let me know about your results. >> > > Going back through the patch set and running performance on each one, > I see a 10% drop in performance at patch 2 in the series, which adds > an extra handle_returns() call in the busy loop. Which avoids possible > deadlock. > > I played around with that patch for a while, only calling > handle_returns() every x times aroudn the loop, but the performance > was worse again, probably because of the extra branch I added. > > However, it's more important to have stable performance than so it's > still a good idea to have that fix applied, IMO. I agree > > Maybe we can get back some lost performance in future optimisation > patches. That would be really nice. If i have some time, I would like to try some ideas I came with during work in the series. > > Thanks, > Dave. > > > > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com