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 D2CC3A00C5; Sat, 15 Jan 2022 07:44:26 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AE9040E0F; Sat, 15 Jan 2022 07:44:26 +0100 (CET) Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by mails.dpdk.org (Postfix) with ESMTP id 75B44406A2 for ; Sat, 15 Jan 2022 07:44:24 +0100 (CET) Received: by mail-ed1-f50.google.com with SMTP id u21so42128353edd.5 for ; Fri, 14 Jan 2022 22:44:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=MQ6WUvoVrAQhnJXfimEGdZJ7e3Zk7MXsfYDz1oJGTXs=; b=Kg/te9gaY3NHJdBIUtdZUSmU0CPI+UkGY3qno6AsVpUCxFSYLfHk6L4B8aYwatPT6N sKe+sOLbho0GapE7mKO02KMTOmbposI5nepBRSLwWCLqjBjYlGXRoeW+XsQSNua93yh9 oiFOvpC8yXiu9xe4YC+8OwroRsuuqA+/YDuwQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MQ6WUvoVrAQhnJXfimEGdZJ7e3Zk7MXsfYDz1oJGTXs=; b=ciKuH/4EjQC3ak8Pj0Zru+s8rgIyjS3EZpd+YJw06KH32yjX6Vf0A+PF/oyGkXiCK9 1Feqkzta1mtyAqU6iMaTmvoQdmSK/vPNzTkyL6EotZJDKsdsPjPnmxE9IVeFEyxQ6j3z +LK8rsfEjlnXhCokWmyPHLh77u6aCI+TjkgOHhacdMU6rDKofbGyFo1lxz//96TbI+H5 nlUxFIKiLAr2bEbfoj8jvpiLjxmIIUUmkwTQh+nOCjR4GMANoWX7UVOb6VrOdQ1pV6rL cJ8b2NeOdgB6VJhzAKNvMN1neF900xPOsTIuRzEQ77rQEhwKyAHs8AQbxM4x90FkGnkU bigw== X-Gm-Message-State: AOAM532sm2xbH886fzx1934gSDpblGQkd1MVHBOywOrsnn5W6+07/eUM REdyoV4x33o2+KBME7fQLlgugBNR12KgtPr1EN41hw== X-Google-Smtp-Source: ABdhPJzK8tq1tqpDrkbsIg/7eJhgFbgwYDX2wV09+GQX/jFdvEKSwD2xSNfL7eiTLWVPJZAtx+cvO3KCPBJ6hX2T7YY= X-Received: by 2002:a17:906:19d7:: with SMTP id h23mr8514395ejd.605.1642229063770; Fri, 14 Jan 2022 22:44:23 -0800 (PST) MIME-Version: 1.0 References: <1627281811-45185-1-git-send-email-fengchengwen@huawei.com> <5f36c0d0-3bce-ff70-d355-269686cad41a@intel.com> In-Reply-To: <5f36c0d0-3bce-ff70-d355-269686cad41a@intel.com> From: Kalesh Anakkur Purayil Date: Sat, 15 Jan 2022 12:14:12 +0530 Message-ID: Subject: Re: [dpdk-dev] [PATCH] ethdev: modify comment of INTR RESET event To: Ferruh Yigit Cc: Chengwen Feng , Thomas Monjalon , Andrew Rybchenko , dpdk-dev , Jingjing Wu , Ajit Khaparde Content-Type: multipart/alternative; boundary="000000000000fd14da05d5993f51" 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 --000000000000fd14da05d5993f51 Content-Type: text/plain; charset="UTF-8" Hi Ferruh, Sorry for the late reply. Please see inline. On Mon, Sep 27, 2021 at 9:49 PM Ferruh Yigit wrote: > On 7/26/2021 7:43 AM, Chengwen Feng wrote: > > According to the definition of rte_eth_dev_reset(), the > > RTE_ETH_EVENT_INTR_RESET event could also use when PF resets. > > > > Can you please highlight the part in the 'rte_eth_dev_reset()' definition > related to the RESET event usage for PF? > > > This patch modifies the comment of RTE_ETH_EVENT_INTR_RESET event, so > > that it could use in all resets. > > > > The original intention seems as the comment mentions, please check related > commits [1]. > > As far as I can get from below comments, usecase is, > - PF sends reset command to VFs (driver internal command) > - VF sends RESET event to application, to request reset to be performed by > application. > > So event is more like a reset request from driver to application. > > Overall it is OK to extend the usage of the RESET event to PF, if there is > a > usecase for it. What is your usecase? > And should we extend comment (API documentation) a little more to clarify > when > this even should be sent and what application should do when event > received, > what do you think? > > btw, cc'ed Ajit & Kales, as far as I remember in the past they suggest a > recover > event, maybe relevant with this discussion. > [Kalesh]: The recovery event we proposed was for a different purpose and looks different from this. In that case, PMD itself recovers from the FW error/reset conditions using a handshaking protocol between PMD and the device FW without needing application intervention. I will send a new version of the patch set incorporating the comments I received on the last version I had sent. Regards, Kalesh > > > > [1] > Commit ae19955e7c86 ("i40evf: support reporting PF reset") > Commit 514302ff6e00 ("ethdev: add NIC reset operation") > > > Signed-off-by: Chengwen Feng > > --- > > lib/ethdev/rte_ethdev.h | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > > index d2b27c3..e6646a6 100644 > > --- a/lib/ethdev/rte_ethdev.h > > +++ b/lib/ethdev/rte_ethdev.h > > @@ -3499,8 +3499,7 @@ enum rte_eth_event_type { > > RTE_ETH_EVENT_INTR_LSC, /**< lsc interrupt event */ > > RTE_ETH_EVENT_QUEUE_STATE, > > /**< queue state event (enabled/disabled) > */ > > - RTE_ETH_EVENT_INTR_RESET, > > - /**< reset interrupt event, sent to VF on PF reset > */ > > + RTE_ETH_EVENT_INTR_RESET, /**< reset interrupt event */ > > RTE_ETH_EVENT_VF_MBOX, /**< message from the VF received by PF */ > > RTE_ETH_EVENT_MACSEC, /**< MACsec offload related event */ > > RTE_ETH_EVENT_INTR_RMV, /**< device removal event */ > > > > -- Regards, Kalesh A P --000000000000fd14da05d5993f51 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Ferruh,
Sorry=C2=A0for the late reply. Please see inline.

On Mon, Sep 27= , 2021 at 9:49 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
On 7/26/2021 7:43 AM, Chengwen Fe= ng wrote:
> According to the definition of rte_eth_dev_reset(), the
> RTE_ETH_EVENT_INTR_RESET event could also use when PF resets.
>

Can you please highlight the part in the 'rte_eth_dev_reset()' defi= nition
related to the RESET event usage for PF?

> This patch modifies the comment of RTE_ETH_EVENT_INTR_RESET event, so<= br> > that it could use in all resets.
>

The original intention seems as the comment mentions, please check related<= br> commits [1].

As far as I can get from below comments, usecase is,
- PF sends reset command to VFs (driver internal command)
- VF sends RESET event to application, to request reset to be performed by<= br> application.

So event is more like a reset request from driver to application.

Overall it is OK to extend the usage of the RESET event to PF, if there is = a
usecase for it. What is your usecase?
And should we extend comment (API documentation) a little more to clarify w= hen
this even should be sent and what application should do when event received= ,
what do you think?

btw, cc'ed Ajit & Kales, as far as I remember in the past they sugg= est a recover
event, maybe relevant with this discussion.
[Kalesh]: = The recovery event we proposed was for a different purpose and looks differ= ent from this.

In that case, PMD itself recovers f= rom the FW error/reset conditions using a handshaking protocol between PMD = and the device FW without needing application intervention.

<= /div>
I will send a new version of the patch set incorporating the comm= ents I received on the last version I had sent.

Re= gards,
Kalesh



[1]
Commit ae19955e7c86 ("i40evf: support reporting PF reset")
Commit 514302ff6e00 ("ethdev: add NIC reset operation")

> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>=C2=A0 lib/ethdev/rte_ethdev.h | 3 +--
>=C2=A0 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index d2b27c3..e6646a6 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3499,8 +3499,7 @@ enum rte_eth_event_type {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_INTR_LSC, /**< lsc interrup= t event */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_QUEUE_STATE,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/**< queue state event (enabled= /disabled) */
> -=C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_INTR_RESET,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0/**< reset interrupt event, sent to VF on PF reset */
> +=C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_INTR_RESET, /**< reset interrupt= event */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_VF_MBOX,=C2=A0 /**< message= from the VF received by PF */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_MACSEC,=C2=A0 =C2=A0/**< MA= Csec offload related event */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_ETH_EVENT_INTR_RMV, /**< device remov= al event */
>



--
Regards,
Kalesh A P
--000000000000fd14da05d5993f51--