[PATCH] net/ixgbe: fix ixgbe firmware version get

[PATCH] net/ixgbe: fix ixgbe firmware version get

[Shiyang He]

The firmware version obtained by dpdk-ethtool is inconsistent with that
obtained by linux-ethtool.

This commit fixes the issue.

Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
Cc: stable@dpdk.org

Signed-off-by: Shiyang He <shiyangx.he@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ae9f65b334..012096021e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3859,7 +3859,7 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
 	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
 
-	etrack_id = (eeprom_verh << 16) | eeprom_verl;
+	etrack_id = (eeprom_verl << 16) | eeprom_verh;
 	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
 	if (ret < 0)
 		return -EINVAL;
-- 
2.34.1

RE: [PATCH] net/ixgbe: fix ixgbe firmware version get

[Morten Brørup]

> From: Shiyang He [mailto:shiyangx.he@intel.com]
> Sent: Wednesday, 4 January 2023 10.52
> 
> The firmware version obtained by dpdk-ethtool is inconsistent with that
> obtained by linux-ethtool.
> 
> This commit fixes the issue.
> 
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ae9f65b334..012096021e 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3859,7 +3859,7 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
>  	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
>  	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
> 
> -	etrack_id = (eeprom_verh << 16) | eeprom_verl;
> +	etrack_id = (eeprom_verl << 16) | eeprom_verh;
>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);

This looks wrong, assuming the high-word in 0x2e and the low-word in 0x2d.

Perhaps the linux ethtool needs to be fixed instead?

I don't think DPDK should aim for "bug compatibility" with Linux. :-(

>  	if (ret < 0)
>  		return -EINVAL;
> --
> 2.34.1
> 

RE: [PATCH] net/ixgbe: fix ixgbe firmware version get

[He, ShiyangX]

>-----Original Message-----
>From: Morten Brørup <mb@smartsharesystems.com>
>Sent: Wednesday, January 4, 2023 6:19 PM
>To: He, ShiyangX <shiyangx.he@intel.com>; dev@dpdk.org
>Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org; Yang, Qiming
><qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>; Remy
>Horton <remy.horton@intel.com>
>Subject: RE: [PATCH] net/ixgbe: fix ixgbe firmware version get
>
>> From: Shiyang He [mailto:shiyangx.he@intel.com]
>> Sent: Wednesday, 4 January 2023 10.52
>>
>> The firmware version obtained by dpdk-ethtool is inconsistent with
>> that obtained by linux-ethtool.
>>
>> This commit fixes the issue.
>>
>> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
>> ---
>>  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index ae9f65b334..012096021e 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -3859,7 +3859,7 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev,
>> char *fw_version, size_t fw_size)
>>  	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
>>  	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
>>
>> -	etrack_id = (eeprom_verh << 16) | eeprom_verl;
>> +	etrack_id = (eeprom_verl << 16) | eeprom_verh;
>>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
>
>This looks wrong, assuming the high-word in 0x2e and the low-word in 0x2d.
>
>Perhaps the linux ethtool needs to be fixed instead?
>
>I don't think DPDK should aim for "bug compatibility" with Linux. :-(

After following your suggestions, I read the driver source code again, 
which is implemented as the solution. I will send out V2 patch soon.

RE: [PATCH] net/ixgbe: fix ixgbe firmware version get

[Morten Brørup]

> From: He, ShiyangX [mailto:shiyangx.he@intel.com]
> Sent: Thursday, 5 January 2023 03.52
> 
> >From: Morten Brørup <mb@smartsharesystems.com>
> >Sent: Wednesday, January 4, 2023 6:19 PM
> >
> >> From: Shiyang He [mailto:shiyangx.he@intel.com]
> >> Sent: Wednesday, 4 January 2023 10.52
> >>
> >> The firmware version obtained by dpdk-ethtool is inconsistent with
> >> that obtained by linux-ethtool.
> >>
> >> This commit fixes the issue.
> >>
> >> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
> >> ---
> >>  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> index ae9f65b334..012096021e 100644
> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> @@ -3859,7 +3859,7 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev,
> >> char *fw_version, size_t fw_size)
> >>  	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
> >>  	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
> >>
> >> -	etrack_id = (eeprom_verh << 16) | eeprom_verl;
> >> +	etrack_id = (eeprom_verl << 16) | eeprom_verh;
> >>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> >
> >This looks wrong, assuming the high-word in 0x2e and the low-word in
> 0x2d.
> >
> >Perhaps the linux ethtool needs to be fixed instead?
> >
> >I don't think DPDK should aim for "bug compatibility" with Linux. :-(
> 
> After following your suggestions, I read the driver source code again,
> which is implemented as the solution. I will send out V2 patch soon.

I suppose what you are looking for is here:
https://elixir.bootlin.com/linux/v6.1.3/source/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c#L4106

RE: [PATCH] net/ixgbe: fix ixgbe firmware version get

[He, ShiyangX]

>-----Original Message-----
>From: Morten Brørup <mb@smartsharesystems.com>
>Sent: Thursday, January 5, 2023 2:57 PM
>To: He, ShiyangX <shiyangx.he@intel.com>; dev@dpdk.org
>Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org; Yang, Qiming
><qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>; Remy
>Horton <remy.horton@intel.com>
>Subject: RE: [PATCH] net/ixgbe: fix ixgbe firmware version get
>
>> From: He, ShiyangX [mailto:shiyangx.he@intel.com]
>> Sent: Thursday, 5 January 2023 03.52
>>
>> >From: Morten Brørup <mb@smartsharesystems.com>
>> >Sent: Wednesday, January 4, 2023 6:19 PM
>> >
>> >> From: Shiyang He [mailto:shiyangx.he@intel.com]
>> >> Sent: Wednesday, 4 January 2023 10.52
>> >>
>> >> The firmware version obtained by dpdk-ethtool is inconsistent with
>> >> that obtained by linux-ethtool.
>> >>
>> >> This commit fixes the issue.
>> >>
>> >> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
>> >> Cc: stable@dpdk.org
>> >>
>> >> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
>> >> ---
>> >>  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> index ae9f65b334..012096021e 100644
>> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> @@ -3859,7 +3859,7 @@ ixgbe_fw_version_get(struct rte_eth_dev
>*dev,
>> >> char *fw_version, size_t fw_size)
>> >>  	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
>> >>  	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
>> >>
>> >> -	etrack_id = (eeprom_verh << 16) | eeprom_verl;
>> >> +	etrack_id = (eeprom_verl << 16) | eeprom_verh;
>> >>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
>> >
>> >This looks wrong, assuming the high-word in 0x2e and the low-word in
>> 0x2d.
>> >
>> >Perhaps the linux ethtool needs to be fixed instead?
>> >
>> >I don't think DPDK should aim for "bug compatibility" with Linux. :-(
>>
>> After following your suggestions, I read the driver source code again,
>> which is implemented as the solution. I will send out V2 patch soon.
>
>I suppose what you are looking for is here:
>https://elixir.bootlin.com/linux/v6.1.3/source/drivers/net/ethernet/intel/ixg
>be/ixgbe_common.c#L4106

Thanks so much! The next patch will refer to this method!

[PATCH v2] net/ixgbe: fix ixgbe firmware version inconsistency

[Shiyang He]

This patch follows the code of ixgbe kernel driver so that it keeps the
firmware version obtained by dpdk-ethtool consistent with that obtained
by linux-ethtool.

Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
Cc: stable@dpdk.org

Signed-off-by: Shiyang He <shiyangx.he@intel.com>

v2: follow the code of ixgbe kernel driver
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ae9f65b334..65655b9212 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3852,23 +3852,32 @@ static int
 ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	u16 eeprom_verh, eeprom_verl;
-	u32 etrack_id;
+	struct ixgbe_nvm_version nvm_ver;
 	int ret;
 
-	ixgbe_read_eeprom(hw, 0x2e, &eeprom_verh);
-	ixgbe_read_eeprom(hw, 0x2d, &eeprom_verl);
+	ixgbe_get_oem_prod_version(hw, &nvm_ver);
+	if (nvm_ver.oem_valid) {
+		snprintf(fw_version, fw_size, "%x.%x.%x",
+			 nvm_ver.oem_major, nvm_ver.oem_minor,
+			 nvm_ver.oem_release);
+		return 0;
+	}
+
+	ixgbe_get_etk_id(hw, &nvm_ver);
+	ixgbe_get_orom_version(hw, &nvm_ver);
 
-	etrack_id = (eeprom_verh << 16) | eeprom_verl;
-	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
+	if (nvm_ver.or_valid) {
+		snprintf(fw_version, fw_size, "0x%08x, %d.%d.%d",
+			 nvm_ver.etk_id, nvm_ver.or_major,
+			 nvm_ver.or_build, nvm_ver.or_patch);
+		return 0;
+	}
+
+	ret = snprintf(fw_version, fw_size, "0x%08x", nvm_ver.etk_id);
 	if (ret < 0)
 		return -EINVAL;
 
-	ret += 1; /* add the size of '\0' */
-	if (fw_size < (size_t)ret)
-		return ret;
-	else
-		return 0;
+	return (fw_size < (size_t)ret++) ? ret : 0;
 }
 
 static int
-- 
2.34.1

RE: [PATCH v2] net/ixgbe: fix ixgbe firmware version inconsistency

[Morten Brørup]

> From: Shiyang He [mailto:shiyangx.he@intel.com]
> Sent: Monday, 9 January 2023 07.53
> 
> This patch follows the code of ixgbe kernel driver so that it keeps the
> firmware version obtained by dpdk-ethtool consistent with that obtained
> by linux-ethtool.
> 
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shiyang He <shiyangx.he@intel.com>
> 
> v2: follow the code of ixgbe kernel driver
> ---

Thank you for the updated patch. LGTM.

Acked-by: Morten Brørup <mb@smartsharesystems.com>

RE: [PATCH v2] net/ixgbe: fix ixgbe firmware version inconsistency

[Zhang, Qi Z]

> -----Original Message-----
> From: Morten Brørup <mb@smartsharesystems.com>
> Sent: Monday, January 9, 2023 3:48 PM
> To: He, ShiyangX <shiyangx.he@intel.com>; dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; stable@dpdk.org; Yang, Qiming
> <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>; Remy
> Horton <remy.horton@intel.com>
> Subject: RE: [PATCH v2] net/ixgbe: fix ixgbe firmware version inconsistency
> 
> > From: Shiyang He [mailto:shiyangx.he@intel.com]
> > Sent: Monday, 9 January 2023 07.53
> >
> > This patch follows the code of ixgbe kernel driver so that it keeps
> > the firmware version obtained by dpdk-ethtool consistent with that
> > obtained by linux-ethtool.
> >
> > Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Shiyang He <shiyangx.he@intel.com>
> >
> > v2: follow the code of ixgbe kernel driver
> > ---
> 
> Thank you for the updated patch. LGTM.
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>

Applied to dpdk-next-net-intel.

Thanks
Qi