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 EAFB741CF1; Tue, 21 Feb 2023 00:13:24 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A4D4D42686; Tue, 21 Feb 2023 00:13:24 +0100 (CET) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2047.outbound.protection.outlook.com [40.107.92.47]) by mails.dpdk.org (Postfix) with ESMTP id E7AE84021D for ; Tue, 21 Feb 2023 00:13:22 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RMSVhgVzSO3vJKnv9wfU8Tb57khLRL+UrEEjPVCSjllq3kB9JKh5Feuv6Fn5pmE2ajF8LW88m9oLQt8wNq7Ite670cLBWS5HeVdbCCWOkPw+tKZwnM9+Z6gtORdEEOinxOBtm2rTRi/d129jQRPlK5RyfdAsl8jxQ0CFjzsv5ayJbylWwPYteWA9bNFdHlnplIRrsU6zTzF98GUhzNTgwpjEKLSzn1T85/u+m3tewxNOG76oWv+R+73EStoskgB+hXy+IUMDo/NAWgd1kIIdHRHgbmJPB5mswlgEbKWcjzzfHBMZ1B5oWC8pueGoNgzyDGL9TASwLjiY81KDy2kRMQ== 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=AHp82iLwgjyyiPsV5BWcqo3Vo6j2+qA02UzajhqpAxc=; b=khI5HzP/4MLhu7OzdVDcAdmyFASC2pllUM2RohrwoqbuD3n3tRG6c/zfVnIQd1Pkmxb/NJ7Uyiay4uWAYC8enHjlyKTlbfkMILDn8EC+MdqHZuqo2FCvdRjSu2LufvKymkB6oARv9vfZu3h/+0H8JQNXE4OVF+v55gNOQELS6Eevk82CaukICpGID+zjGViOVMEU6OVMA3N1LHldvd1d9iuIUSa36y/PjCUYABITb0Q4JdQxyxcxRTZD+Z0nvAOSBef+GXe158ezYPTyOpP6sEXxdpUxyfiFCkj2sYKzzA+6eL80NXB2/nWervSvU6qO5V/fRMrbHJQDyDZPj7JrJA== 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=AHp82iLwgjyyiPsV5BWcqo3Vo6j2+qA02UzajhqpAxc=; b=gdMdzH5aS5/YGrHBbhdVwKcOAD5boJt5u9kws5O4YoxpJW9gfcJzWntzAyPg+pUJvZo7zejcxm6b82Thjxftjuv7BMe3Q7qb1l/pnEp4o5rStFo/Gpew2B4jSUPWmtEhrYIA2aWp7+IueUrzQrSiQ4iTc+nb+njWWIPNVciegpM= 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 PH8PR12MB7025.namprd12.prod.outlook.com (2603:10b6:510:1bc::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6111.19; Mon, 20 Feb 2023 23:13:20 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48%8]) with mapi id 15.20.6111.020; Mon, 20 Feb 2023 23:13:20 +0000 Message-ID: <9af27b72-0216-7860-e52a-9b984bda3037@amd.com> Date: Mon, 20 Feb 2023 23:13:14 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Content-Language: en-US To: Ed Czeck Cc: dev@dpdk.org, stephen@networkplumber.org, John Miller , Shepard Siegel , Anatoly Burakov References: <20230217160039.2487085-1-ed.czeck@atomicrules.com> <20230217215923.2561685-1-ed.czeck@atomicrules.com> From: Ferruh Yigit Subject: Re: [PATCH v2 1/3] net/ark: support secondary process In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P265CA0371.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a3::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_|PH8PR12MB7025:EE_ X-MS-Office365-Filtering-Correlation-Id: 6c9b5c9c-db3b-4e99-37a3-08db13980e2a X-LD-Processed: 3dd8961f-e488-4e60-8e11-a82d994e183d,ExtAddr X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: X1PPfDN7zltoIE4Pn6j92YlrV+wmrr1WNKE0QiDdZEKhp3klbwGC1M7aIzQZmUoZ56YMJUExafWT1NxARhUPHOD41UtkifjhD/UjPlTG68ULPSACD/yM14q+ySENmrj5vFpeghu+Z3F+6dgsqkVWXdNUlB4YRgZT8AtKe9YbQT88EqS6WoqVR2S1lxIbonFVHrqB4JFehjUx1BujY4rF/v0p4LfjEaVervyQVfJqkkBHuhQYEJ1NEbv5teeydP3OUHZfbLL0mTjAwDJVVuECShoFeeXa7U5MC27u0Ih+kuNQOExLUHyQWplndJVUNdIAyS7pO9oWqbWGL9a78sf7I/PiVBLPNJa1p8gHgWzFo+4dmpZ1xG5yZHDqqd61aNAyfwJYaDrqqFZoLRbgsNrImxheBILQoiQu6e6MNSYkIhzfL9ASBX77Kdp7+Q4zLbZJxu8pIYml/J8KkCEI+PSKSzoBZCxaHSmBT1PrOCoStsDkqGSC8eaGCnUxmgA7/F04CAZEdxcje4RLOwo/j+DOhA+IQ8grhYxtjxATvCghRLHYHHRW3B87YxYYR7UgkqsEEKte4fgE238K9z+v4rVDzsScSy+2lZJ25X0fZ5mlyT5L9BMjcJ/wyjwHaG08HZn9PlkT9C1wx3b0zIuCu2vsdBA/rgDN7hgAmy+mVyYs6kGyGZWxjHo9ahQSGDNbsB7aO7OoFQ6Cp60ApB5ysUKtxoon/o39m+tsnv01cO89pmZBnbM4i9Zq8KnXxfiVQvQh 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:(13230025)(4636009)(136003)(376002)(396003)(366004)(346002)(39860400002)(451199018)(31686004)(4326008)(6916009)(8676002)(66556008)(66476007)(316002)(66946007)(6486002)(54906003)(44832011)(8936002)(41300700001)(5660300002)(36756003)(86362001)(31696002)(478600001)(6506007)(53546011)(6512007)(38100700002)(186003)(26005)(6666004)(2616005)(2906002)(83380400001)(21314003)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dEdOUHlTWWJ5ZUxVREtHalVzYnRKREwvaTlSUXNLbHczRnE0TWVQTXExaHFz?= =?utf-8?B?bW9NeHlXWTNKU3BOc2hYa1N0ekdIOEdEd2p0Y3FpeHZDRlZBREl6ZzZwTlB6?= =?utf-8?B?RUcrbmpUckp2UEg5aXc4Nm00eVBGQk5YM2dhTnZMUGRHL25mSUtwc01XK2JP?= =?utf-8?B?dXg3clpXMDNRWlhGcFBzbUdzb0pJcklicFZCY1NqcjJjcjIvOGROM3o1Vm5J?= =?utf-8?B?WFNvaldPbUpyWGQvNXBnaUU5S2c3ODhGMUZIZHdHb2g4RUIrcWxJTnpBQitm?= =?utf-8?B?dVczdldybkZCS2txeDdmM0JwV1hPU2dOUkhieVhBaGpsSFgxYUFFbktTZSsr?= =?utf-8?B?enJCc2FKNnhkYlZZTUlwbU5Hdld1bXA2eUJ0NjJ5UEpFNkJuSFRFZUtENVpK?= =?utf-8?B?K2R3NC83Y0xnZ25zTW8rTnJnbWxkVW9rc2lESW90TWkveEhlS2NwM2loYTM0?= =?utf-8?B?NHFYVUJsRWhTTjJHMC9TdnBLSU9HcG1xVTcwM0JSdU82cWd0NFQ1TXoyWDZQ?= =?utf-8?B?UTFLSm1xb2syZEt3amp6WEl2UUQ1b2NrVy91c2FNcVBOeVkxL0E0WWtHaGlG?= =?utf-8?B?bXdlNkdqdXZQQXFmYjlMRWlwcEwwQldSeVVhcUhrV0p1VzdKMU9NRkdBQko3?= =?utf-8?B?S0tCZ25NZTVPUG1GaUZNWHgybTYwQjFMUDd4R01WZjV5QU83UnV5dnFBNmdD?= =?utf-8?B?djFUeENtQ2V3WEtvSUZVdFgwcjM2a3pvMkRUYXNmcDBXZWZpS2JEamJMQ2dK?= =?utf-8?B?L3ZDdmJSVUdrR3RDeG1ObTBQRjg5cHB3aThKeGYzZmJUUVMxdXFyeC92emVo?= =?utf-8?B?MDFRdXNHUUdlWS9vWWs0U2ZnNnFYU2gzREdWcy9UWmlXTE9tdFV2SDFKbVZy?= =?utf-8?B?Z0NFZ1BnRVZUVGtQWUpNbk1RVy8yeWx0NnFDNnVCVWRrTC9JV3RmQjM3Q0tJ?= =?utf-8?B?TFRKTFFqdXVkWWxRUFcwSXhWQXVrTmJBNEhIREo1aHpqWnBWMHVydCtGdlAw?= =?utf-8?B?MGFtME9EVzM3aGZzeWhOMW9FYy96WFJGM2ZVckVsNDlLVS9DS3J3Mk9QV1B0?= =?utf-8?B?Qmo0b0VRY0JGMHAzVFJRdkhJVXJ2L2pOeVVGQ3JGaDFGY3dUMnVsQTZzbGVF?= =?utf-8?B?THc4dHFlbGQ0Y3U4UFZxOHZTOFBxR0xUaUJzdHAyUnNvekhaLzNKUVJBbWhL?= =?utf-8?B?VVoydk90dnlYdmMxMnhzVVR3TDJSRWdpdnBCSDJ4VkV3RlQrNEl2Qm5US3dk?= =?utf-8?B?MTRkanhxeHdvNitVUXJmcjdyOVlNK2RXdzVyWVhUaUhMdWViUnZjRmZFWkVW?= =?utf-8?B?Yms3OXBBejdLWGZQZHZBMFhuVW0vUTV0ZzFCUFNaVHZQdWRjZnFjc0hhd3Fm?= =?utf-8?B?eThteWdiN1FEQ2ZHNCsvVTg1RHdybHRnM2ZDaW4wYUFLMGFhWEcvbU5HNXVY?= =?utf-8?B?QXhtNDZ5d1ladERJU1dWZUVNKy90c2ZaemJRNzg4Ym9VRlkrdHcvUFVzSklJ?= =?utf-8?B?TEEzdkc1NDBVN2F3V0VQd0dVZk9xbUVyYmhIZnUzdk5iNlQ1blIwVys3WmhB?= =?utf-8?B?bGpaWmsyZFhUMHZuQjZuanRrM1FIN3RwM0tiYXJ4Z1FPREJwOTFsdzFIL1Rv?= =?utf-8?B?QTFNNVpUSlZCa3lNZ25rRlI0ZVExY0hNcDdBbmVrSmZDQjF6Vm9YbW50WGQ4?= =?utf-8?B?VThXcnJaY1p2U1NsUDFUUHNBTk9QbFVIakthc05zRmh4c25JWEhCNDFXcElY?= =?utf-8?B?VG15ZnEyUmc4b040dTd6UnpYRFY4UmhTb1JrdERRZnZrOUpRanhxYVVSZDl2?= =?utf-8?B?QnE1cEplZC9GdG9yb202QU1SY1ZaeG05dlRZOFpyRzI5cHdOTWNCZ3dqQStH?= =?utf-8?B?YUQwcHByeStTVHlxWnZNS0lnaVBZZUZYZkxmUG0wekZqUTZ5RjhRMVBDelRG?= =?utf-8?B?QUcvYUhHbFdTTzNTTE9LVk9mS1ZXZTU2WTJBR1dPdXJLelRJcWNHUHhRaDI0?= =?utf-8?B?N1JLTFRDRGltaWw1RzJaYUs4bkxCS1lkTS9vK0pSNFpMM25ZZzNxK3A4Z0FL?= =?utf-8?B?YlptZVR4M3lLMFZhdEpSZVVOOSs3NWY1TXhMZzVMekxhVnYvQm85ZEZJK1FL?= =?utf-8?Q?s1Eo8AoBCN5sJW8zqgaDXhZj7?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6c9b5c9c-db3b-4e99-37a3-08db13980e2a X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2023 23:13:20.0892 (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: j9SKJDXfRA8mS/FXzXhJH2nemAQDfhqpAx+4QowPomt9S2B0UyGJNHqlKMw9Gb0I X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB7025 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 2/20/2023 10:04 PM, Ed Czeck wrote: > Hi ferruh, > We have limited support for secondary processes.  This patch simply > avoids corrupting the FPGA state if a secondary process attaches. > Improved support for secondary processes is on our list, but we need a > strong customer driver for this feature. > An update patch is following soon. Hi Ed, I see new version just updates commit log, mentioning this is minimal secondary process support, but current implementation is wrong, it is not about minimal support or full support. Like just below secondary process, there is following code: ``dev->data->mac_addrs = rte_zmalloc()`` so you are allocating memory and set it to exact same pointer for primary and each secondaries. This is probably leaking memory and secondaries overwriting existing mac config by pointing it to new memory location. And there are more `dev->data->xxx` modifications in the function, which is cause unexpected result. As mentioned in the previous review, you need to find proper location to return from function for secondary processes. Please try to fix the implementation, instead of trying to push it with known errors. > Thanks for the review. > Ed. > > On Mon, Feb 20, 2023 at 9:17 AM Ferruh Yigit > wrote: > > On 2/17/2023 9:59 PM, Ed Czeck wrote: > > From: John Miller > > > > > disable device configuration for secondary processes > > > > Signed-off-by: John Miller > > > --- > > v2: > > * Use standard logging > > --- > >  drivers/net/ark/ark_ethdev.c | 11 ++++++++--- > >  1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ark/ark_ethdev.c > b/drivers/net/ark/ark_ethdev.c > > index b2995427c8..d237e80cf4 100644 > > --- a/drivers/net/ark/ark_ethdev.c > > +++ b/drivers/net/ark/ark_ethdev.c > > @@ -147,6 +147,9 @@ eth_ark_pci_probe(struct rte_pci_driver > *pci_drv __rte_unused, > >       struct rte_eth_dev *eth_dev; > >       int ret; > >  > > +     if (rte_eal_process_type() == RTE_PROC_SECONDARY) > > +             ARK_PMD_LOG(DEBUG, "ARK probed by secondary process\n"); > > + > >       eth_dev = rte_eth_dev_pci_allocate(pci_dev, sizeof(struct > ark_adapter)); > >  > >       if (eth_dev == NULL) > > @@ -385,9 +388,11 @@ eth_ark_dev_init(struct rte_eth_dev *dev) > >                   0xcafef00d, ark->sysctrl.t32[4], __func__); > >  > >       /* We are a single function multi-port device. */ > > -     ret = ark_config_device(dev); > > -     if (ret) > > -             return -1; > > +     if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > > +             ret = ark_config_device(dev); > > +             if (ret) > > +                     return -1; > > +     } > > > Hi Ed, > > As far as I can see both primary and secondary process continues to run > after this point, and below there are a few places that updates > 'eth_dev->data'. > > 'eth_dev->data' is shared between primary and secondaries, so each > secondary will be overwriting the shared data. > Better usage is shared data only updated by primary process and > secondary processes use available values. > But 'eth_dev' is process specific and all primary and shared processes > must set fields of this struct. > > You may need to re-order calls in function to make secondary quit after > 'eth_dev' fields updated and before 'eth_dev->data' updated, to make > sure secondaries don't update shared data. > > >  > >       dev->dev_ops = &ark_eth_dev_ops; > >       dev->rx_queue_count = eth_ark_dev_rx_queue_count; >