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 B8C6F41CF1; Mon, 20 Feb 2023 23:04:27 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 99CD743100; Mon, 20 Feb 2023 23:04:27 +0100 (CET) Received: from mail-lj1-f173.google.com (mail-lj1-f173.google.com [209.85.208.173]) by mails.dpdk.org (Postfix) with ESMTP id BDE5E40395 for ; Mon, 20 Feb 2023 23:04:26 +0100 (CET) Received: by mail-lj1-f173.google.com with SMTP id z5so2492406ljc.8 for ; Mon, 20 Feb 2023 14:04:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=atomicrules-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=8vMA4KgtUu9NmWqlG6BI8ZrzP+nfYzfA/r45fV0JmSs=; b=AXlVimMGdQ1w+VFfFvapQYj4q4HoUFXeSm4ckHK7HbeRID+Q+wZhJl09l8WdHAeJDD fngrKvqc0s83jdud2AvECqieMeYdsSWiFireH7xKIfxpBYsl34yOXG7fL2mEHgqjkWWD kmyKX+sovzrpyF2plz33Xb45XpTF9HI+qI6Hn+XF/azhMeOzDfRC0I3wLmvoInUoehK1 8cMGviGxSPOlHWDl08AWVNzOvlUpHRYi9MOf3d273yC7/RjHey9KynVZSfAiWnIi+Plh rwQnYQ+DQ4lfFBdlD+xALErCEazYU4nxMKbHJpfTRviyLEWKzeT/U85sqEV5VPU/N3KM YNdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8vMA4KgtUu9NmWqlG6BI8ZrzP+nfYzfA/r45fV0JmSs=; b=t74fvrif/bpfWFNsIHivhDIa13+1KSUh7tBjpafyga4KE7H2wCYxr/trVaAaKTmWBA A9A9mciOcKqM5FVjG7IRxCkfu9lvgc+2HY/UwpBOU4M1Ra4qUvlEnFalTIaFnoA4lkmw dxwJaQtw/re1NmN/WVro+Bqp5Bh2lwMgenCC7WXhHvtERID4Nk0gZ+3deuQ8LVblfLhc geQHE0gRa8n1LoRif7kxfeT8lPJBetaHY90QMrN7/8DZge4JDwShynnLbYy/hETwpaME V/PZncqEEfOarw/7Qlh7fLcF0In0gPG34M/Rqdpt/DTV1+wNXTh/CDCwbcni47dASF8i f4zg== X-Gm-Message-State: AO0yUKVCt/bSOOuCa3ULw7TUksrSP9kNR//me/1uaMPRZ7VNQnSWnQb8 HcOLGDlk+dpAUV3ErrkMoAqmEx6YuTXOIJ5Nqz76EQ== X-Google-Smtp-Source: AK7set9lsXDM6xSyS0ZGX3fyhaDZnIw+Yq/GqT95HfsTzLfOesDD57m03NVNvNFxhfE9N6cq4aPDdzyoZH1H0aigWyI= X-Received: by 2002:a05:651c:1987:b0:293:4ab4:3bb1 with SMTP id bx7-20020a05651c198700b002934ab43bb1mr1667417ljb.4.1676930666157; Mon, 20 Feb 2023 14:04:26 -0800 (PST) MIME-Version: 1.0 References: <20230217160039.2487085-1-ed.czeck@atomicrules.com> <20230217215923.2561685-1-ed.czeck@atomicrules.com> In-Reply-To: From: Ed Czeck Date: Mon, 20 Feb 2023 17:04:14 -0500 Message-ID: Subject: Re: [PATCH v2 1/3] net/ark: support secondary process To: Ferruh Yigit Cc: dev@dpdk.org, stephen@networkplumber.org, John Miller , Shepard Siegel , Anatoly Burakov Content-Type: multipart/alternative; boundary="000000000000ab742405f528d852" 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 --000000000000ab742405f528d852 Content-Type: text/plain; charset="UTF-8" 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. 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; > > --000000000000ab742405f528d852 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi ferruh,
We have limited support for= secondary processes.=C2=A0 This patch simply avoids corrupting the FPGA st= ate if a secondary process attaches.
Improved support for se= condary processes is on our list, but we need a strong customer driver for = this feature.
An update patch is following soon.
Thanks= for the review.
Ed.

On Mon, Feb 20, 2023 at 9:17 AM Fer= ruh Yigit <ferruh.yigit@amd.com<= /a>> wrote:
O= n 2/17/2023 9:59 PM, Ed Czeck wrote:
> From: John Miller <john.miller@atomicrules.com>
>
> disable device configuration for secondary processes
>
> Signed-off-by: John Miller <john.miller@atomicrules.com>
> ---
> v2:
> * Use standard logging
> ---
>=C2=A0 drivers/net/ark/ark_ethdev.c | 11 ++++++++---
>=C2=A0 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,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct rte_eth_dev *eth_dev;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int ret;
>=C2=A0
> +=C2=A0 =C2=A0 =C2=A0if (rte_eal_process_type() =3D=3D RTE_PROC_SECOND= ARY)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ARK_PMD_LOG(DEBUG, &q= uot;ARK probed by secondary process\n");
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0eth_dev =3D rte_eth_dev_pci_allocate(pci_dev= , sizeof(struct ark_adapter));
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (eth_dev =3D=3D NULL)
> @@ -385,9 +388,11 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x= cafef00d, ark->sysctrl.t32[4], __func__);
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/* We are a single function multi-port devic= e. */
> -=C2=A0 =C2=A0 =C2=A0ret =3D ark_config_device(dev);
> -=C2=A0 =C2=A0 =C2=A0if (ret)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -1;
> +=C2=A0 =C2=A0 =C2=A0if (rte_eal_process_type() =3D=3D RTE_PROC_PRIMAR= Y) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D ark_config_de= vice(dev);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return -1;
> +=C2=A0 =C2=A0 =C2=A0}


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 ea= ch
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 proces= ses
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' upda= ted, to make
sure secondaries don't update shared data.

>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0dev->dev_ops =3D &ark_eth_dev_ops; >=C2=A0 =C2=A0 =C2=A0 =C2=A0dev->rx_queue_count =3D eth_ark_dev_rx_qu= eue_count;

--000000000000ab742405f528d852--