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 DBD50A0542 for ; Fri, 18 Nov 2022 17:31:37 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CFFBA40E03; Fri, 18 Nov 2022 17:31:37 +0100 (CET) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id 18FCC4003F; Fri, 18 Nov 2022 17:31:35 +0100 (CET) Received: from frapeml100007.china.huawei.com (unknown [172.18.147.207]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4NDMg43mSsz6HJJn; Sat, 19 Nov 2022 00:29:04 +0800 (CST) Received: from frapeml500007.china.huawei.com (7.182.85.172) by frapeml100007.china.huawei.com (7.182.85.133) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Fri, 18 Nov 2022 17:31:34 +0100 Received: from frapeml500007.china.huawei.com ([7.182.85.172]) by frapeml500007.china.huawei.com ([7.182.85.172]) with mapi id 15.01.2375.031; Fri, 18 Nov 2022 17:31:34 +0100 From: Konstantin Ananyev To: Stephen Hemminger , Luc Pelletier CC: "grive@u256.net" , "dev@dpdk.org" , "stable@dpdk.org" , "hyonkim@cisco.com" , "johndale@cisco.com" Subject: RE: [PATCH] failsafe: fix segfault on hotplug event Thread-Topic: [PATCH] failsafe: fix segfault on hotplug event Thread-Index: AQHY9SprVcNNgKa3LUC7cpmW1Gqcra5B0XAQgAA+xYCAAAlzAIACbdFw Date: Fri, 18 Nov 2022 16:31:34 +0000 Message-ID: <774190f3094342c28de2e28f9d93d486@huawei.com> References: <20221110163410.12734-1-lucp.at.work@gmail.com> <20221116142548.554a7a9e@hermes.local> In-Reply-To: <20221116142548.554a7a9e@hermes.local> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.126.170.76] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 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 Hi Luc, =20 > > Hi Konstantin, > > > > > It is not recommended way to update rte_eth_fp_ops[] contents directl= y. > > > There are eth_dev_fp_ops_setup()/ eth_dev_fp_ops_reset() that suppose= d > > > to be used for that. > > > > Good to know. I see another fix that was made in a different PMD that > > does exactly the same thing: > > > > https://github.com/DPDK/dpdk/commit/bcd68b68415172815e55fc67cf3947c0433= baf74 > > > > CC'ing the authors for awareness. > > > > > About the fix itself - while it might help till some extent, > > > I think it will not remove the problem completely. > > > There still remain a race-condition between rte_eth_rx_burst() and fa= ilsafe_eth_rmv_event_callback(). > > > Right now DPDK doesn't support switching PMD fast-ops functions (or u= pdating rxq/txq data) > > > on the fly. > > > > Thanks for the information. This is very helpful. > > > > Are you saying that the previous code also had that same race > > condition? Yes, I believe so.=20 > It was only updating the rte_eth_dev structure, but I > > assume the problem would have been the same since rte_eth_rx_burst() > > in DPDK versions <=3D20 use the function pointers in rte_eth_dev, not > > rte_eth_fp_ops. > > > > Can you think of a possible solution to this problem? I'm happy to > > provide a patch to properly fix the problem. Having your guidance > > would be extremely helpful. > > > > Thanks! >=20 > Changing burst mode on a running device is not safe because > of lack of locking and/or memory barriers. >=20 > Would have been better to not to do this optimization. > Just have one rx_burst/tx_burst function and look at what > ever conditions are present there. I think Stephen is right - within DPDK it is just not possible to switch RX= /TX function on the fly (without some external synchronization). So the safe way is to always use safe version of RX/TX call. I personally don't think such few extra checks will affect performance that= much. As another nit: inside failsafe rx_burst functions it is probably better no= t to access dev->data->rx_queues directly, but call rte_eth_rx_burst(sdev->sdev_port_id, ...); instead. Same for TX. Thanks Konstantin =20