[dpdk-dev] [PATCH] net/i40e: firmware status check

[dpdk-dev] [PATCH] net/i40e: firmware status check

[Xiaoyun Li]

Check the firmware status at init time. If the firmware is in
recovery mode, alert the user to check it.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 85a6a86..30cec47 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1210,7 +1210,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_vsi *vsi;
 	int ret;
-	uint32_t len;
+	uint32_t len, val;
 	uint8_t aq_fail = 0;
 
 	PMD_INIT_FUNC_TRACE();
@@ -1261,6 +1261,15 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	 */
 	hw->switch_tag = 0xffff;
 
+	val = I40E_READ_REG(hw, I40E_GL_FWSTS);
+	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
+		PMD_INIT_LOG(ERR, "\nERROR: "
+			"Firmware recovery mode detected. Limiting functionality.\n"
+			"Refer to the Intel(R) Ethernet Adapters and Devices "
+			"User Guide for details on firmware recovery mode.");
+		return -EIO;
+	}
+
 	/* Check if need to support multi-driver */
 	i40e_support_multi_driver(dev);
 
-- 
2.7.4

[dpdk-dev] [PATCH v2] net/i40e: firmware status check

[Xiaoyun Li]

Check the firmware status at init time. If the firmware is in
recovery mode, alert the user to check it.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v2:
 * Rebase to the newest codes.
---
 drivers/net/i40e/i40e_ethdev.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index fea42b0..93cc17b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1273,7 +1273,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_vsi *vsi;
 	int ret;
-	uint32_t len;
+	uint32_t len, val;
 	uint8_t aq_fail = 0;
 
 	PMD_INIT_FUNC_TRACE();
@@ -1324,6 +1324,15 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	 */
 	hw->switch_tag = 0xffff;
 
+	val = I40E_READ_REG(hw, I40E_GL_FWSTS);
+	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
+		PMD_INIT_LOG(ERR, "\nERROR: "
+			"Firmware recovery mode detected. Limiting functionality.\n"
+			"Refer to the Intel(R) Ethernet Adapters and Devices "
+			"User Guide for details on firmware recovery mode.");
+		return -EIO;
+	}
+
 	/* Check if need to support multi-driver */
 	i40e_support_multi_driver(dev);
 	/* Check if users want the latest supported vec path */
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] net/i40e: firmware status check

[Zhang, Qi Z]

> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Monday, December 3, 2018 3:08 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH v2] net/i40e: firmware status check
> 
> Check the firmware status at init time. If the firmware is in recovery mode, alert
> the user to check it.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [dpdk-dev] [PATCH v2] net/i40e: firmware status check

[Kevin Traynor]

On 12/03/2018 08:07 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Li, Xiaoyun
>> Sent: Monday, December 3, 2018 3:08 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
>> Subject: [PATCH v2] net/i40e: firmware status check
>>
>> Check the firmware status at init time. If the firmware is in recovery mode, alert
>> the user to check it.
>>
>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.
> 

This was applied with a 'Cc:stable' tag, but no 'Fixes' tag.
What stable branches is it relevant for?

> Thanks
> Qi
> 
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/i40e: firmware status check

[Ferruh Yigit]

On 12/14/2018 4:59 PM, Kevin Traynor wrote:
> On 12/03/2018 08:07 AM, Zhang, Qi Z wrote:
>>
>>
>>> -----Original Message-----
>>> From: Li, Xiaoyun
>>> Sent: Monday, December 3, 2018 3:08 PM
>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
>>> Subject: [PATCH v2] net/i40e: firmware status check
>>>
>>> Check the firmware status at init time. If the firmware is in recovery mode, alert
>>> the user to check it.
>>>
>>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>
>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>
>> Applied to dpdk-next-net-intel.
>>
> 
> This was applied with a 'Cc:stable' tag, but no 'Fixes' tag.

This fixes a behavior in the driver, but not a specific code/commit, and author
request this behavior change to backport. This request makes sense to me but
what do you think from stable tree point of view? Are you OK with this kind of
request?

> What stable branches is it relevant for?

I agree it is hard to define the scope of the fix without having the code that
is fixed. Do you have any suggestion how to formalize the request for these kind
of issues?

> 
>> Thanks
>> Qi
>>
>>
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/i40e: firmware status check

[Kevin Traynor]

On 12/17/2018 10:57 AM, Ferruh Yigit wrote:
> On 12/14/2018 4:59 PM, Kevin Traynor wrote:
>> On 12/03/2018 08:07 AM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Li, Xiaoyun
>>>> Sent: Monday, December 3, 2018 3:08 PM
>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>>>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
>>>> Subject: [PATCH v2] net/i40e: firmware status check
>>>>
>>>> Check the firmware status at init time. If the firmware is in recovery mode, alert
>>>> the user to check it.
>>>>
>>>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>>
>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>
>>> Applied to dpdk-next-net-intel.
>>>
>>
>> This was applied with a 'Cc:stable' tag, but no 'Fixes' tag.
> 
> This fixes a behavior in the driver, but not a specific code/commit, and author
> request this behavior change to backport. This request makes sense to me but
> what do you think from stable tree point of view? Are you OK with this kind of
> request?
> 

It makes sense to me also to backport, I guess it can be seen as a fix
for the original pmd which was missing this code:

Fixes: 4861cde46116 ("i40e: new poll mode driver")

or if only relevant since some base driver/firmware change, then Fixes:
from that update.

>> What stable branches is it relevant for?
> 
> I agree it is hard to define the scope of the fix without having the code that
> is fixed. Do you have any suggestion how to formalize the request for these kind
> of issues?
> 

I tend to think if it's a *fix*, then some code was previously added
that was incorrect, or had a missing piece, or became incorrect at some
point due to another other change, so 'Fixes:' should almost always be
possible.

However, if for some reason it's not clear and there's not too many,
then a simple solution is to reply to thread (cc'ing stable) saying
which which stable branches it is relevant for. I will check the thread
for info when I see a patch like that. 'Fixes:' is much preferred
though, so not to have manual checking of email threads.

Kevin.

>>
>>> Thanks
>>> Qi
>>>
>>>
>>
> 

Re: [dpdk-dev] [PATCH] net/i40e: firmware status check

[Xing, Beilei]

> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Wednesday, June 6, 2018 2:17 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH] net/i40e: firmware status check
> 
> Check the firmware status at init time. If the status is abnormal, alert the
> user to exit DPDK.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 13c5d32..c1c19d3 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1192,7 +1192,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void
> *init_params __rte_unused)
>  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	struct i40e_vsi *vsi;
>  	int ret;
> -	uint32_t len;
> +	uint32_t len, val;
>  	uint8_t aq_fail = 0;
> 
>  	PMD_INIT_FUNC_TRACE();
> @@ -1236,6 +1236,14 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void
> *init_params __rte_unused)
>  	hw->bus.func = pci_dev->addr.function;
>  	hw->adapter_stopped = 0;
> 
> +	val = I40E_READ_REG(hw, I40E_GL_FWSTS);
> +	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
> +		PMD_INIT_LOG(ERR, "\nERROR:"
> +				  " Firmware status of this driver is
> abnormal."
> +				  " Please quit the app.");

"Please quit the app." can be replaced with some notice, for example "please check if firmware is updating" and ? 

> +		return -EIO;

Seems I/O error is not suitable for firmware status. How about EBUSY or something else?

> +	}
> +
>  	/* Check if need to support multi-driver */
>  	i40e_support_multi_driver(dev);
> 
> --
> 2.7.4

[dpdk-dev] [PATCH] net/i40e: firmware status check

[Xiaoyun Li]

Check the firmware status at init time. If the status is abnormal,
alert the user to exit DPDK.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 13c5d32..c1c19d3 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1192,7 +1192,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_vsi *vsi;
 	int ret;
-	uint32_t len;
+	uint32_t len, val;
 	uint8_t aq_fail = 0;
 
 	PMD_INIT_FUNC_TRACE();
@@ -1236,6 +1236,14 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	hw->bus.func = pci_dev->addr.function;
 	hw->adapter_stopped = 0;
 
+	val = I40E_READ_REG(hw, I40E_GL_FWSTS);
+	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
+		PMD_INIT_LOG(ERR, "\nERROR:"
+				  " Firmware status of this driver is abnormal."
+				  " Please quit the app.");
+		return -EIO;
+	}
+
 	/* Check if need to support multi-driver */
 	i40e_support_multi_driver(dev);
 
-- 
2.7.4