diff mbox series

net/ixgbe: fix ixgbe firmware version get

Message ID 20230104095142.86796-1-shiyangx.he@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series net/ixgbe: fix ixgbe firmware version get |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Shiyang He Jan. 4, 2023, 9:51 a.m. UTC 
  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(-)

Comments

Morten Brørup Jan. 4, 2023, 10:18 a.m. UTC | #1 
> 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
>
Shiyang He Jan. 5, 2023, 2:52 a.m. UTC | #2 
>-----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.
Morten Brørup Jan. 5, 2023, 6:57 a.m. UTC | #3 
> 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
Shiyang He Jan. 5, 2023, 7:33 a.m. UTC | #4 
>-----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!
diff mbox series

Patch

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;