Re: [dpdk-dev] Question about hardware error handling policy

On 2021/7/22 23:46, Thomas Monjalon wrote:

22/07/2021 15:50, fengchengwen:

quoted

Hi, all

    I notice ethdev support dev_reset ops, which could be used to recover from
errors, and only 13+ drivers support this function.
    And also there is event for reset: RTE_ETH_EVENT_INTR_RESET, and only 6
drivers support it (most of them are VF).

    This provides users with two ways to handle hardware errors:
    a. driver report RTE_ETH_EVENT_INTR_RESET, and application do reset ops.
    b. application detect errors (the detection method is unclear), and call
    reset ops to recover.

    According to the design of this API, error handling is assigned to the
application, and the driver is only responsible for reporting events. This
simplifies the driver design (for example, the driver does not need to maintain
mutex locks).

    As we know, many modern NICs come with firmware, have PCIE interfaces,
support SR-IOV, the hardware errors can have: firmware reboot/PF reset/
VF reset/FLR, but these errors(particularly firmware/PF) are not addressed in
most drivers.

    Question 1: what do we think of these errors(particularly firmware/PF)? Do
we think that the probability is very low and that there is no need to deal with
them?

Even rare errors must be managed.

Because intel and mlx NIC are widely used, I look at i40e/mlx5 driver code, and found:
1) i40e PF driver, it only show logs when detect global reset and other error:
	if (icr0 & I40E_PFINT_ICR0_GRST_MASK)
		PMD_DRV_LOG(INFO, "ICR0: global reset requested");
	if (icr0 & I40E_PFINT_ICR0_PCI_EXCEPTION_MASK)
		PMD_DRV_LOG(INFO, "ICR0: PCI exception activated");
	if (icr0 & I40E_PFINT_ICR0_STORM_DETECT_MASK)
		PMD_DRV_LOG(INFO, "ICR0: a change in the storm control state");
   @Beilei Why not report RESET_EVENT in these cases ? or these error are very rarely
   so just report it. And also, the application may still send or recv packet, These
   resets, if not handled correctly, do not cause the hardware/driver to hang ?

2) mlx5 PF driver, I notice there is a mlx5_dev_interrupt_device_fatal which will
remove the device.
   @Matan Why not report RESET_EVENT in these cases ? so the application can be
recovered quickly.

quoted

    Question 2: I prefer to put error handling in the application layer, because
doing it in the driver can make the driver complex, but there is no app to
register the INTR_RESET event handler. I think we can build a standard handler
in testpmd, What do you think?

Absolutely. As any ethdev API, it must be tested with testpmd.


.