[dpdk-dev] Question about hardware error handling policy

[fengchengwen]

On 2021/7/22 23:46, Thomas Monjalon wrote:
> 22/07/2021 15:50, fengchengwen:
>> 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.

> 
>>     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.
> 
> 
> .
>