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 0B09E41DCA; Fri, 3 Mar 2023 18:19:18 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9DC8E40EE3; Fri, 3 Mar 2023 18:19:18 +0100 (CET) Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2050.outbound.protection.outlook.com [40.107.223.50]) by mails.dpdk.org (Postfix) with ESMTP id 1184E40ED9 for ; Fri, 3 Mar 2023 18:19:17 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CGg9z2hz/yYJ9BiVHW2W52O3s3keqWeJAXAwYITaMUxjf1VnQPFj3wA31L/8vAe4umXREqfw274TWYqExJPeqFiJMv+KxrkyLf5AJmj3EqAL6O+7O1o7umtuxepbYIde/Y0k4CWDLDBQWUVQtqLoNtjCKtYgggNnek4l08bmAlnu95aBPJT8helwfa2SJX9F84reLcBq6Up43EFKrmeJIRTPJWZDVSmMmulQZlT/AXAVzVPDMHSWJ4ZLaOE0x4fzw7ezlk3ldt/FgYlHerszuasr6gSmkx+31tJPx/SiEJgDnnFf96ipkA+ozjoU28rgoyUtoXFQuZJp+2wDsfmGFQ== 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=XdixImpoxAVCeawD1L5aREHUiq/0B5wKgD66eHeGYzM=; b=QVZVmxMiUtp87cEBV810SDkP5E86GaSH3ENAWSqivXZUANiwaMSCB+2sLAFe80qUEaPMjerO6wSc4ampLJf42YuBPoCsUhZlYFA2E5UTyUxbg5NsA7aLmlDAAODmZv+NGqyEvBPu8X91SPOzXxpghtRzt5FNr9An6uHKJApsprlSmF3nDeZOsyhZMFOd5kHKz1j15H2uh4/Lu3hZvkKd0LonJ+hmMIU+kg+9JKkk8/BA7j8SIZoMIb/5iHJgVBZ/sU9ibkpOPbJrTEf/rOFecVv1z732Pwq5qAK33sxdPs5tF8cW30BzXcF6Bc83Rp7LxWyFvoJPDXa1tmxYnQdoaA== 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=XdixImpoxAVCeawD1L5aREHUiq/0B5wKgD66eHeGYzM=; b=aSE1jEcoRgBtyVciyWsNyslO0uSXW0xDFhwKbavZe71OZHMHXNd4TlVavMpQ2d7AU/3GE+WMK+F9ehL3lGfjtNPdBPoQQieiCexD+abzUS7vSR/dhYPHn1dIUVieZ+qM1k8eRn9J+Pc7RwGPWSOd5AY14Z4d9IglIJaqIv+ZSeI= 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 SA1PR12MB6797.namprd12.prod.outlook.com (2603:10b6:806:259::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6156.22; Fri, 3 Mar 2023 17:19:14 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640%4]) with mapi id 15.20.6156.022; Fri, 3 Mar 2023 17:19:14 +0000 Message-ID: <52296fe2-d9f6-1a24-e577-e5271a69a053@amd.com> Date: Fri, 3 Mar 2023 17:19:08 +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: Konstantin Ananyev , dev@dpdk.org, fengchengwen , Konstantin Ananyev , Honnappa Nagarahalli , Stephen Hemminger , Ruifeng Wang , "Ajit Khaparde (ajit.khaparde@broadcom.com)" References: <20230220060839.1267349-1-ashok.k.kaladi@intel.com> <20230220060839.1267349-2-ashok.k.kaladi@intel.com> <4786db4b-63dc-5329-522d-77eb58d4cff4@huawei.com> <20230221090053.14d653bf@hermes.local> <3cd97a71-b32f-b33b-dce1-46fabad182f6@huawei.com> <54fbf4e55cd44477b1e956f98a7a3c50@huawei.com> <3dc3b3d3-c80f-a361-8780-b1b3e48d843e@yandex.ru> From: Ferruh Yigit Subject: Re: [PATCH 2/2] ethdev: fix race condition in fast-path ops setup In-Reply-To: <3dc3b3d3-c80f-a361-8780-b1b3e48d843e@yandex.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P265CA0074.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:8::14) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|SA1PR12MB6797:EE_ X-MS-Office365-Filtering-Correlation-Id: 776fef8a-ec93-4020-3f9b-08db1c0b6984 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: oSMbknbATY2BHeFQzn3qTPl5CERaS6LejVYL4zRHk8utqnWRhHIFiQspN2NNuoJxH1QrYDXxVVHg0nWPVIbSKBhjX4AEUIQvsmU2Dh1rVBNF0tjjg6CzYW5lrXsb8wX/6XEXGnAoQ+0JekPaG4QZIV039rVIvbbpkiCP8LGxC/JFaJG3rrFeZoVp0KfflcRpsub4xduF2kATuxVH2zwO3cnPDe2TMgA06SPTaXJNoggh/mDqOI9GDM0z9X+mhuJlYrYVF8iolyTIf5wk/xk2l+tk1JGY4Q7aKgDjYaof1AgGVr8KMDf0PDc65NQzUkbIu64hVSn70mwp3aJ6HJtOVo/o5k0lPzHvwIyWrZzWqhHuBuLJNdSQHcBYelUCS3Rbbdy9qvItFFlQBKgHI1MXU853dHjkbDjWRhsXJW5XrnF5V1Jq2EegaDJZYTT/bnbZq2j0k3sUkZbE7Q/TTJM29+tyNjJ4Es1RfPuexGOF/C8t+OSFMLPNoWlJVpKtsl+bkEd/5Cs1iTVYGRH3Mz+hqGFi/kDPOmY6aQiNMUkRMniW0MOzjVOpFW+81WCW2OWex7QQD+nJY2mg+/1nK7uXpMEvj80d/Yt1Ri3l5RWsa9V+psetlswppWMXSniGTJtXoMcWK/Q9SgH6Lke/b/nfGjfFIcTUME3+7W2yUqF838kl1OrI52VN4PsrozMckIaaT4cNxYHB9tTZmwx1OHoS+2EqCuIKgLjWTGvnu/lhiGg= 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)(396003)(376002)(136003)(39860400002)(366004)(346002)(451199018)(36756003)(38100700002)(31686004)(6506007)(6512007)(26005)(53546011)(6666004)(2616005)(186003)(6486002)(8676002)(110136005)(316002)(478600001)(66946007)(66476007)(66556008)(5660300002)(41300700001)(31696002)(30864003)(44832011)(2906002)(83380400001)(8936002)(86362001)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SEVoVDFjOTVMQ1NUbDZBZ1ErenZKYkdVU1hkSG1yQy9lRG9jOTl1L3E4QWxj?= =?utf-8?B?Y2lPcXUvMjY2bU5kVXRMVUcwYU4wYTJVTHh0aFEyUzJySWNMUkwvVkpXT1l4?= =?utf-8?B?ZUNhSmNHN3RzTTBUdFliN2tZR2dQVWo4cWFaSm5qc2h4dHN4Q0lGcWF0Qjhj?= =?utf-8?B?MTNYVUVVeXpWRmhoMWtRWDlsZElxYzVVSHRvV3BkOVZ0cHg0ZEltRjM0Y2tY?= =?utf-8?B?Sk9EU2J4VDBPV1ROaXBzODFEdjRQajBkbU9vUGNIc2o1QkxETXRrNjQzVkxa?= =?utf-8?B?WGViTTVVYjdBNm5OTllzd09EUUcxOGdRSmxzNHJFTGdUQUpWMmtRb1Jpejlv?= =?utf-8?B?VHkxK3Z6VVlTS1RFZHg5eHhuNmhSRjVyZHJPMUhNYmpmQUF3bEFDL0Yxcy8x?= =?utf-8?B?WlJGTlhTdElndS95akpBcTZoeWNqNlpJRE9QM0dJNkpncC8yQ2ZvOTVwNk01?= =?utf-8?B?K1l4OW9PSVZWcXF6Y1ByV0VCSmV3c09XWUFzL3ZIYzF2MUdHV1NvajJ3SFRY?= =?utf-8?B?UTZEYW1XakdxU0hQTkV2b2hLQXBlaXZ6QmZIejZGQkErV201Um5nT1BGT2Ji?= =?utf-8?B?QytoZTR5NjNLcW1FM0pyNExpV1pWeUhLUmsyZUFYT1RoMUowak9NNUVybUR5?= =?utf-8?B?Mk5jVTRaVTRudm04QnBSQ2cwbFBzbDlyYXBmVFZrbUxrS1M5aUxkc0tPcDIx?= =?utf-8?B?dUJuYVMzWDZhWnlLMThTRFNxZEhvajBiK0dNYzlCaEx3TEhkY2lWMUtwYm1O?= =?utf-8?B?ZVQ2d3BjbzltSi9obERJSUN6a1gySElYclU3YkxNOGdwclhJbGZ3U1RUQWQv?= =?utf-8?B?aDRVTDBtWitmSWNRN3Y3K2J0cG4weHlqeG05QXJjNjZrSk5TQkx5TU9jbXVU?= =?utf-8?B?aUlHNU9pMUx3SXZRb3VVNy8rYUFOWTdVV3ZudDZCL1hFN2ZzSDJpTWVXRS9t?= =?utf-8?B?M2R4RFNGZEZycDFuNGU1WFBNYVU4VzIzL3Z5MWd6VGpHOUEwc2NoWHE2Q3du?= =?utf-8?B?N2lLZXFZZmVXeW05aUQ3SnpMU0NRdFdQY3REaXdCVm1qNGNOM3BZRTZiaWsr?= =?utf-8?B?S250eWlQUmo0a3JSdDNLRS9lSkR2R09NTDY3TmFiOCt2TDlXSm1MenBRb0Nl?= =?utf-8?B?aTkrNXA2U2xhUGlsTGZpNjVrUUh4Mk1mblI1ZzZ4VVhza0xySWpTMUMzM2Q3?= =?utf-8?B?OHoyYUNPeWFYMlBZdmx3Sm1JUHBrdFhRemJIN0tHS0EvOXZHRnZjU1dvOEFX?= =?utf-8?B?c3pKeCtLV0dBd1JOWmtvYlhZZ0dZYm9XaXRZWXNINm9TNTVuYURHVTdlWjY0?= =?utf-8?B?T3c2UEFHbVUvcnBOSTA1N2Z3ZElwd3luT0lvMDJxaFRVQ2FSMHhsYkJDeXIx?= =?utf-8?B?RFU3QXp4R2hhSWo3VU5JNE9UZWgvbmRHemZrd2NnVmtaaUcvSlZBSmFIQ2Vv?= =?utf-8?B?dlMzN29kL2NQWDE2ZVhHUVlTdlBRbUdDbFl4ZnFnRDVwRzQ1ZEN2V3dpRE1i?= =?utf-8?B?ZUhneGtBQVovVHFrK2Zmc3QzeXVUTFFUYkx3cmNoZEZaTHhPcnM5V2l4UWNM?= =?utf-8?B?dW52RzUwcnplVUUyTzJrbVpSb1NlbGJOQ0RWYUFnWkFSZWFpN1UyVDk5UGVS?= =?utf-8?B?a01GRk44TXdGT3ZvOWQ0K2ZnWFBxYlp1MmUrakFHV29uNmUvSFJEbms3SUZH?= =?utf-8?B?eFZ0NEYyUVhDNXlYMExYaEMvMEJLMi9pSFVZU3BDR3E3Ui9BQTFhVmZGVzlC?= =?utf-8?B?WlFzZGFEVHR6NEZpd21qeEo3NnlYWFdPMjJGYlc1R2Jpbk9qdVBKZ2VjbjhQ?= =?utf-8?B?UzIreXlvQ21XMVNQNkpuZGVhak1DV1VFVVJ3WWZSMFJ6T3hUYjNWL1M5Q3pW?= =?utf-8?B?ZVNBY0dKSDd0QWR6Mjg4ZTBIZlAyNjJxakJKdXl4c3VaeVQ5M2hJYmJ4cXlN?= =?utf-8?B?SXFxczhiZGkrcnZSK0d4TGlTOUh5d2xKeDVudFBrRFF5djRNcXpVMTFyRHNX?= =?utf-8?B?L2JFbDhERDRNbDhaMU16aFpSbXVyNENXUTJ3eEUyLzRMUWV4SWYzZ0VrQWJK?= =?utf-8?B?RjU1TmhFVWZiam5sTHRid2FXN0ErTmFuUkhhQlVLTVltWUVzNjdDNVJzMVFJ?= =?utf-8?Q?sUwKfyZa2OZ6toDUyzr55MgU6?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 776fef8a-ec93-4020-3f9b-08db1c0b6984 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Mar 2023 17:19:14.5893 (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: mZ2qD5UNAjVS7jlPVyvBtU39ESnpxQhAaDfdYBRIy83VC6DaGVR8RFyMfQ4oBsMt X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB6797 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/26/2023 5:22 PM, Konstantin Ananyev wrote: > >>>>>>>>>>> If ethdev enqueue or dequeue function is called during >>>>>>>>>>> eth_dev_fp_ops_setup(), it may get pre-empted after setting the >>>>>>>>>>> function pointers, but before setting the pointer to port data. >>>>>>>>>>> In this case the newly registered enqueue/dequeue function will >>>>>>>>>>> use dummy port data and end up in seg fault. >>>>>>>>>>> >>>>>>>>>>> This patch moves the updation of each data pointers before >>>>>>>>>>> updating corresponding function pointers. >>>>>>>>>>> >>>>>>>>>>> Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate >>>>>>>>>>> structure") >>>>>>>>>>> Cc: stable@dpdk.org >>>>>>>> >>>>>>>> Why is something calling enqueue/dequeue when device is not fully >>>>>> started. >>>>>>>> A correctly written application would not call rx/tx burst until >>>>>>>> after ethdev start had finished. >>>>>>> >>>>>>> Please refer the eb0d471a894 (ethdev: add proactive error handling >>>>>>> mode), when driver recover itself, the application may still invoke >>>>>> enqueue/dequeue API. >>>>>> >>>>>> Right now DPDK ethdev layer *does not* provide synchronization >>>>>> mechanisms between data-path and control-path functions. >>>>>> That was a deliberate deisgn choice. If we want to change that >>>>>> rule, then I >>>>>> suppose we need a community consensus for it. >>>>>> I think that if the driver wants to provide some sort of error >>>>>> recovery >>>>>> procedure, then it has to provide some synchronization mechanism >>>>>> inside it >>>>>> between data-path and control-path functions. >>>>>> Actually looking at eb0d471a894 (ethdev: add proactive error handling >>>>>> mode), and following patches I wonder how it creeped in? >>>>>> It seems we just introduced a loophole for race condition with this >>>>>> approach... >>>> >>>> Could you try to describe the specific scenario of loophole ? >>> >>> Ok, as I understand the existing mechanism: >>> >>> When PMD wants to start a recovery it has to: >>>   - invoke  rte_eth_dev_callback_process(RTE_ETH_EVENT_ERR_RECOVERING); >>>     That supposed to call user provided callback. After callback is >>> finished PMD assumes >>>     that user is aware that recovery is about to start and should >>> make some precautions. >>> - when recovery is finished it invokes another callback: >>>    RTE_ETH_EVENT_RECOVERY_(SUCCESS/FAILED). After that user either >>> can continue to >>>    use port or have to treat is as faulty. >>> >>> The idea is ok in principle, but there is a problem. >>> >>> lib/ethdev/rte_ethdev.h: >>>             /** Port recovering from a hardware or firmware error. >>>           * If PMD supports proactive error recovery, >>>           * it should trigger this event to notify application >>>           * that it detected an error and the recovery is being started. >>> >>> <<< !!!!! >>>           * Upon receiving the event, the application should not >>> invoke any control path API >>>           * (such as rte_eth_dev_configure/rte_eth_dev_stop...) until >>> receiving >>>           * RTE_ETH_EVENT_RECOVERY_SUCCESS or >>> RTE_ETH_EVENT_RECOVERY_FAILED event. >>>           * The PMD will set the data path pointers to dummy functions, >>>           * and re-set the data path pointers to non-dummy functions >>>           * before reporting RTE_ETH_EVENT_RECOVERY_SUCCESS event. >>> <<< !!!!! >>> >>> That part is just wrong I believe. >>> It should be: >>> Upon receiving the event, the application should not invoke any *both >>> control and data-path* API >>> until receiving  RTE_ETH_EVENT_RECOVERY_SUCCESS or >>> RTE_ETH_EVENT_RECOVERY_FAILED event. >>> Resetting data path pointers to dummy functions by PMD *before* invoking >>> rte_eth_dev_callback_process(RTE_ETH_EVENT_ERR_RECOVERING); >>> introduces a race-condition with data-path threads, as such thread >>> could already be inside RX/TX function >>> or can already read RX/TX function/data pointers and be about to use >>> them. >> >> Current practices: the PMDs already add some delay after set Rx/Tx >> callback to dummy, and plus the DPDK >> worker thread is busypolling, the probability of occurence in reality >> is zero. But in theoretically exist >> the above race-condition. > > > Adding delay might make a problem a bit less reproducible, > but it doesn't fix it. > The bug is still there. > > >> >>> And right now rte_ethdev layer doesn't provide any mechanism to check >>> it or wait when they'll finish, etc. >> >> Yes >> >>> >>> So, probably the simplest way to fix it with existing DPDK design: >>> - user level callback  RTE_ETH_EVENT_ERR_RECOVERING should return >>> only after it ensures that *all* >>>    application threads (and processes) stopped using either control >>> or data-path functions for that port >> >> Agree >> >>>    (yes it means that application that wants to use this feature has >>> to provide its own synchronization mechanism >>>    around data-path functions (RX/TX) that it is going to use). >>> - after that PMD is safe to reset rte_eth_fp_ops[] values to dummy ones. >>> >>> And message to all PMD developers: >>> *please stop updating rte_eth_fp_ops[] on your own*. >>> That's a bad practice and it is not supposed to do things that way. >>> There is a special API provided for these purposes: >>> eth_dev_fp_ops_reset(), eth_dev_fp_ops_setup(), so use it. >> >> This two function is in private.h, so it should be expose to public >> header file. > > You mean we need to move these functions declarations into ethdev_driver.h? > If so, then yes, I think we probably do. > > What about making slightly different version available to drivers, which only updates function pointers, but not 'fpo->rxq' / 'fpo->txq'. This way driver can switch to between dummy and real burst function without worrying Rx/Tx queue validity. @Chengwen, @Ruifeng, can this solve the issue for relaxed memory ordering systems? >>> >>> BTW,  I don't see any implementation for RTE_ETH_EVENT_ERR_RECOVERING >>> within >>> either testpmd or any other example apps. >>> Am I missing something? >> >> Currently it just promote the event. > > > Ok, can I suggest then to add a proper usage for into in testpmd? > It looks really strange that we add new feature into ethdev (and 2 PMDs), > but didn't provide any way for users to test it. > >> >>> If not, then probably it could be a good starting point - let's >>> incorporate it inside testpmd >>> (new forwarding engine probably) so everyone can test/try it. >>> >>>           * It means that the application cannot send or receive any >>> packets >>>           * during this period. >>>           * @note Before the PMD reports the recovery result, >>>           * the PMD may report the RTE_ETH_EVENT_ERR_RECOVERING event >>> again, >>>           * because a larger error may occur during the recovery. >>>           */ >>>          RTE_ETH_EVENT_ERR_RECOVERING, >>> >>>>>> It probably needs to be either deprecated or reworked. >>>>> Looking at the commit, it does not say anything about the data >>>>> plane functions which probably means, the error recovery is >>>> happening within the data plane thread. What happens to other data >>>> plane threads that are polling the same port on which the error >>>> recovery is happening? >>>> >>>> The commit log says: "the PMD sets the data path pointers to dummy >>>> functions". >>>> >>>> So the data plane threads will receive non-packet and send zero with >>>> port which in error recovery. >>>> >>>>> >>>>> Also, the commit log says that while the error recovery is under >>>>> progress, the application should not call any control plane APIs. Does >>>> that mean, the application has to check for error condition every >>>> time it calls a control plane API? >>>> >>>> If application has not register event (RTE_ETH_EVENT_ERR_RECOVERING) >>>> callback, it could calls control plane API, but it will return >>>> failed. >>>> If application has register above callback, it can wait for recovery >>>> result, or direct call without wait but this will return failed. >>>> >>>>> >>>>> The commit message also says that "PMD makes sure the control path >>>>> operations failed with retcode -EBUSY". It does not say how it >>>> does this. But, any communication from the PMD thread to control >>>> plane thread may introduce race conditions if not done correctly. >>>> >>>> First there are no PMD thread, do you mean eal-intr-thread ? >>>> >>>> As for this question, you can see PMDs which already implement it, >>>> they both provides mutual exclusion protection. >>>> >>>>> >>>>>> >>>>>>> >>>>>>>> >>>>>>>> Would something like this work better? >>>>>>>> >>>>>>>> Note: there is another bug in current code. The check for link >>>>>>>> state >>>>>>>> interrupt and link_ops could return -ENOTSUP and leave device in >>>>>> indeterminate state. >>>>>>>> The check should be done before calling PMD. >>>>>>>> >>>>>>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >>>>>>>> index >>>>>>>> 0266cc82acb6..d6c163ed85e7 100644 >>>>>>>> --- a/lib/ethdev/rte_ethdev.c >>>>>>>> +++ b/lib/ethdev/rte_ethdev.c >>>>>>>> @@ -1582,6 +1582,14 @@ rte_eth_dev_start(uint16_t port_id) >>>>>>>>           return 0; >>>>>>>>       } >>>>>>>> >>>>>>>> +    if (dev->data->dev_conf.intr_conf.lsc == 0 && >>>>>>>> +        dev->dev_ops->link_update == NULL) { >>>>>>>> +        RTE_ETHDEV_LOG(INFO, >>>>>>>> +                   "Device with port_id=%"PRIu16" link update not >>>>>> supported\n", >>>>>>>> +                   port_id); >>>>>>>> +            return -ENOTSUP; >>>>>>>> +    } >>>>>>>> + >>>>>>>>       ret = rte_eth_dev_info_get(port_id, &dev_info); >>>>>>>>       if (ret != 0) >>>>>>>>           return ret; >>>>>>>> @@ -1591,9 +1599,7 @@ rte_eth_dev_start(uint16_t port_id) >>>>>>>>           eth_dev_mac_restore(dev, &dev_info); >>>>>>>> >>>>>>>>       diag = (*dev->dev_ops->dev_start)(dev); >>>>>>>> -    if (diag == 0) >>>>>>>> -        dev->data->dev_started = 1; >>>>>>>> -    else >>>>>>>> +    if (diag != 0) >>>>>>>>           return eth_err(port_id, diag); >>>>>>>> >>>>>>>>       ret = eth_dev_config_restore(dev, &dev_info, port_id); @@ >>>>>>>> -1611,16 >>>>>>>> +1617,18 @@ rte_eth_dev_start(uint16_t port_id) >>>>>>>>           return ret; >>>>>>>>       } >>>>>>>> >>>>>>>> -    if (dev->data->dev_conf.intr_conf.lsc == 0) { >>>>>>>> -        if (*dev->dev_ops->link_update == NULL) >>>>>>>> -            return -ENOTSUP; >>>>>>>> -        (*dev->dev_ops->link_update)(dev, 0); >>>>>>>> -    } >>>>>>>> - >>>>>>>>       /* expose selection of PMD fast-path functions */ >>>>>>>>       eth_dev_fp_ops_setup(rte_eth_fp_ops + port_id, dev); >>>>>>>> >>>>>>>> +    /* ensure state is set before marking device ready */ >>>>>>>> +    rte_smp_wmb(); >>>>>>>> + >>>>>>>>       rte_ethdev_trace_start(port_id); >>>>>>>> + >>>>>>>> +    /* Update current link state */ >>>>>>>> +    if (dev->data->dev_conf.intr_conf.lsc == 0) >>>>>>>> +        (*dev->dev_ops->link_update)(dev, 0); >>>>>>>> + >>>>>>>>       return 0; >>>>>>>>   } >>>>>>>> >>>>>>>> >>>>>>>> . >>>>>>>> >>>>> >