| Message ID | 1627281811-45185-1-git-send-email-fengchengwen@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Ferruh Yigit |
| Headers | show |
| Series | ethdev: modify comment of INTR RESET event | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-testing | success | Testing PASS |
| ci/iol-abi-testing | success | Testing PASS |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/github-robot | success | github build: passed |
| ci/checkpatch | success | coding style OK |
Hi Chengwen, On 7/26/2021 12:13 PM, 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. > > This patch modifies the comment of RTE_ETH_EVENT_INTR_RESET event, so > that it could use in all resets. > > Signed-off-by: Chengwen Feng<fengchengwen@huawei.com> > --- > 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 */ Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
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. [1] Commit ae19955e7c86 ("i40evf: support reporting PF reset") Commit 514302ff6e00 ("ethdev: add NIC reset operation") > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com> > --- > 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 */ >
Hi Ferruh, Sorry for 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 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 <fengchengwen@huawei.com> > > --- > > 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 */ > > > >
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 */
According to the definition of rte_eth_dev_reset(), the RTE_ETH_EVENT_INTR_RESET event could also use when PF resets. This patch modifies the comment of RTE_ETH_EVENT_INTR_RESET event, so that it could use in all resets. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com> --- lib/ethdev/rte_ethdev.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)