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 7AD7B41D31; Tue, 21 Feb 2023 18:00:57 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0138943213; Tue, 21 Feb 2023 18:00:57 +0100 (CET) Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by mails.dpdk.org (Postfix) with ESMTP id 7C2CD4320F for ; Tue, 21 Feb 2023 18:00:55 +0100 (CET) Received: by mail-pj1-f43.google.com with SMTP id h17-20020a17090aea9100b0023739b10792so218983pjz.1 for ; Tue, 21 Feb 2023 09:00:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=qWfbjaTXGu076lCXz/tTWu+mo8o3cW5uCztLk/PB7pM=; b=yZzxdD1Xv7UBiU82R/AhhGYQl1tmz4Cu8kTlIWPqLcgkR5S13kKr92x5OmJTM56mVd veUjXsRptXZRtLyZTYkmDXPRnGj35DF1NEYQ06XJGJgiFWRQeE9PMZ5noDLzbZVeNVsZ yN29ogQsskgHMBDDD1fU4HEV7DPKo4zGA2pOBm4JdvJEpW1TDbgJQ84xJA8T9feue68H 5IKXG0eatoRnYXSi5o4uob7ZxCGlzJMW3hmJ66WIQrCSS0/dpSwyFCFFT9/gmfryQLN9 KuoS9GOII6Q2w4NZM4W0pbQ8UXC3JdFY4Kuf+nz5JX54dGtirsyMDLSvtc2wbSuyiZVd mRyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=qWfbjaTXGu076lCXz/tTWu+mo8o3cW5uCztLk/PB7pM=; b=3u/VWyH4zNA3DAIx0hOtRdBjPd5gTpYntUEKpyi+keyamh0bw6nyWX/kKIT3owh8VX qQvBB/kin7Csj3CQT+QJIzDaU1welR76WmONWPw04qLyeSlbr5zrIMZVaOgQPh6ypGjG 3feFIUw4CscpavbR3CKP4bmKtesLYcohanfX4DkkxB0bLrfTIooNHDaJqrGackl3KER6 JN1zf0UmsqelVwoYnY56MmuZlyG9gUHpEnm1IpsVbSfOIxaJkKyVmw/u/BbQP3zr7jnn b/auMU3giibOs3qA8hOYA7PYI28NrCDRWgrnnCyIXKrYNr6IuWHJvRTRdA6cxwRu3Jqo GhSw== X-Gm-Message-State: AO0yUKVFZLpFNW6QxJAKBQ2IicZmDPuOg2WRTewRZp2yMaK29YHpIiAv fbryDzgMF04w0uGLLi9Ti8E3bQ== X-Google-Smtp-Source: AK7set9N0IJL8Du4l5r8pCXQADnf+2ZHqYVebd9lPVNMYo75z2Ia4l/B4s4Z6+ToK75pM6yee52PeA== X-Received: by 2002:a17:902:ce83:b0:19a:a3f4:77a8 with SMTP id f3-20020a170902ce8300b0019aa3f477a8mr5158144plg.16.1676998854436; Tue, 21 Feb 2023 09:00:54 -0800 (PST) Received: from hermes.local (204-195-120-218.wavecable.com. [204.195.120.218]) by smtp.gmail.com with ESMTPSA id jc11-20020a17090325cb00b00198ef93d556sm10169646plb.147.2023.02.21.09.00.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Feb 2023 09:00:54 -0800 (PST) Date: Tue, 21 Feb 2023 09:00:53 -0800 From: Stephen Hemminger To: Ruifeng Wang Cc: fengchengwen , Ashok Kaladi , "jerinj@marvell.com" , "thomas@monjalon.net" , Honnappa Nagarahalli , "dev@dpdk.org" , "s.v.naga.harish.k@intel.com" , "erik.g.carrillo@intel.com" , "abhinandan.gujjar@intel.com" , "stable@dpdk.org" , nd Subject: Re: [PATCH 2/2] ethdev: fix race condition in fast-path ops setup Message-ID: <20230221090053.14d653bf@hermes.local> In-Reply-To: References: <20230220060839.1267349-1-ashok.k.kaladi@intel.com> <20230220060839.1267349-2-ashok.k.kaladi@intel.com> <4786db4b-63dc-5329-522d-77eb58d4cff4@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Tue, 21 Feb 2023 07:24:19 +0000 Ruifeng Wang wrote: > > -----Original Message----- > > From: fengchengwen > > Sent: Monday, February 20, 2023 2:58 PM > > To: Ashok Kaladi ; jerinj@marvell.com; thomas@monjalon.net > > Cc: dev@dpdk.org; s.v.naga.harish.k@intel.com; erik.g.carrillo@intel.com; > > abhinandan.gujjar@intel.com; stable@dpdk.org; Ruifeng Wang > > Subject: Re: [PATCH 2/2] ethdev: fix race condition in fast-path ops setup > > > > On 2023/2/20 14:08, Ashok Kaladi 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. 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; }