-----Original Message-----
From: Richardson, Bruce <redacted>
Sent: Tuesday, September 26, 2023 10:06 PM
To: Zhang, Qi Z <redacted>
Cc: He, ShiyangX <redacted>; dev@dpdk.org; Zhou, YidingX
[off-list ref]; Wang, Liang-min [off-list ref];
Su, Simei [off-list ref]; Wu, Wenjun1 [off-list ref];
Zhang, Yuying [off-list ref]; Xing, Beilei
[off-list ref]; Yang, Qiming [off-list ref]; Wu,
Jingjing [off-list ref]
Subject: Re: [PATCH v3] net/iavf: add devargs to enable vf auto-reset

On Tue, Sep 26, 2023 at 01:15:28PM +0100, Zhang, Qi Z wrote:

quoted

quoted

-----Original Message-----
From: Bruce Richardson <redacted>
Sent: Tuesday, September 26, 2023 3:49 PM
To: He, ShiyangX <redacted>
Cc: dev@dpdk.org; Zhou, YidingX <redacted>; Wang,
Liang- min [off-list ref]; Su, Simei
[off-list ref]; Wu,
Wenjun1 [off-list ref]; Zhang, Yuying
[off-list ref]; Xing, Beilei [off-list ref];
Yang, Qiming [off-list ref]; Wu, Jingjing
[off-list ref]
Subject: Re: [PATCH v3] net/iavf: add devargs to enable vf
auto-reset

On Fri, Sep 15, 2023 at 01:02:49PM +0000, Shiyang He wrote:

quoted

Originally, the iavf PMD does not perform special actions when it
receives a PF-to-VF reset event, resulting in vf being offline and
unavailable.

This patch enables vf auto-reset by setting 'watchdog_period'
devargs to true. The iavf PMD will perform an automatic reset to
bring the vf back online when it receives a PF-to-VF event.

v2: handling reset by event handler
v3: change reset process

Signed-off-by: Shiyang He <redacted>
Signed-off-by: Liang-Min Larry Wang <redacted>
---
 doc/guides/nics/intel_vf.rst           |  3 +
 doc/guides/rel_notes/release_23_11.rst |  3 +
 drivers/net/iavf/iavf.h                |  7 +++
 drivers/net/iavf/iavf_ethdev.c         | 86 +++++++++++++++++++++++---
 drivers/net/iavf/iavf_rxtx.c           | 52 ++++++++++------
 drivers/net/iavf/iavf_vchnl.c          | 11 +++-
 6 files changed, 135 insertions(+), 27 deletions(-)

diff --git a/doc/guides/nics/intel_vf.rst

b/doc/guides/nics/intel_vf.rst index d365dbc185..c0acd2a7f5 100644

--- a/doc/guides/nics/intel_vf.rst
+++ b/doc/guides/nics/intel_vf.rst

@@ -101,6 +101,9 @@ For more detail on SR-IOV, please refer to the

following documents:

quoted

     Set ``devargs`` parameter ``watchdog_period`` to adjust the
watchdog

period in microseconds, or set it to 0 to disable the watchdog,

quoted

     for example, ``-a 18:01.0,watchdog_period=5000`` or ``-a

18:01.0,watchdog_period=0``.

quoted

+    Enable vf auto-reset by setting the ``devargs`` parameter
+ like ``-a

18:01.0,enable_auto_reset=1`` when IAVF is backed

quoted

+    by an Intel(r) E810 device or an Intel(r) 700 Series Ethernet device.
+

Why do we need a devargs for this? If the VF is unavailable - as you
mention in the commit log above - should this behaviour not always
be the case without the user having to ask?

Ideally it does not need, but with below considerations:

1. Not break existing scenario, which still assume some application will call

dev_reset /dev_configure/ queue_setup / ...  after receive
RTE_ETH_EVENT_INTR_RESET event to recover the VF manually, the devargs
make sure application be aware of this new feature and will not call
rte_eth_dev_reset which will fail now.

quoted

2. intent to ensure a smoother transition, in case some corner case issues

evaded our validation, keeping this devargs provides us with the flexibility to
remove it once we determine that the implementation is stable enough.

quoted

Thanks for the clear explanation.

One small suggestion: in the commit log, at the end of the first paragraph
change "resulting in the VF being offline and unavailable" to "... offline and
unavailable until the application resets the device on receipt of the
RTE_ETH_EVENT_INTR_RESET event". Similarly at the end of the second
paragraph you could add "This change removes the need for the application
to handle the reset event, as it is transparently handled inside the driver".

Thank you for the suggestion. The commit log has been refined in the dpdk-next-net-intel repo.

Qi

Regards,
/Bruce