diff mbox

[dpdk-dev] examples/ethtool: include case for 64-bit registers

Message ID 1462963714-21022-1-git-send-email-zr@semihalf.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Zyta Szpak May 11, 2016, 10:48 a.m. UTC 
  From: Zyta Szpak <zyta.szpak@semihalf.com>

rte_eth_dev_get_reg_length and rte_eth_dev_get_reg callbacks
do not provide register size to the app in any way. Example assuming
they are 32-bit wide always allocates not enough memory if the
registers are 64-bit wide. It results in memory corruption.
This commit is a quick fix to make enough room for 64-bit
register values when this returned value is given to malloc.

Signed-off-by: Zyta Szpak <zr@semihalf.com>
---
 examples/ethtool/lib/rte_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Remy Horton May 20, 2016, 8:25 a.m. UTC | #1 
Morning,

On 11/05/2016 11:48, zr@semihalf.com wrote:
> From: Zyta Szpak <zyta.szpak@semihalf.com>
>
> rte_eth_dev_get_reg_length and rte_eth_dev_get_reg callbacks
> do not provide register size to the app in any way. Example assuming
> they are 32-bit wide always allocates not enough memory if the
> registers are 64-bit wide. It results in memory corruption.
> This commit is a quick fix to make enough room for 64-bit
> register values when this returned value is given to malloc.
[..]

This is a loose end that needs to be fixed but my feeling is that it 
ought to be done via querying the driver rather than overstating 
register bank size. My suggestion would be to add something like 
get_reg_wordsize to struct eth_dev_ops and then to use sizeof(uint32) as 
fallback for drivers that don't implement the callback.

Regards,

..Rémy
Zyta Szpak May 23, 2016, 5:11 a.m. UTC | #2 
Hi,
sorry on my late reply I was on sick leave. Sure I can do that. This fix 
was the fastest possible without interfering with DPDK API. I will add 
the callback then.

Regards,
Zyta Szpak

On 20.05.2016 10:25, Remy Horton wrote:
> Morning,
>
> On 11/05/2016 11:48, zr@semihalf.com wrote:
>> From: Zyta Szpak <zyta.szpak@semihalf.com>
>>
>> rte_eth_dev_get_reg_length and rte_eth_dev_get_reg callbacks
>> do not provide register size to the app in any way. Example assuming
>> they are 32-bit wide always allocates not enough memory if the
>> registers are 64-bit wide. It results in memory corruption.
>> This commit is a quick fix to make enough room for 64-bit
>> register values when this returned value is given to malloc.
> [..]
>
> This is a loose end that needs to be fixed but my feeling is that it 
> ought to be done via querying the driver rather than overstating 
> register bank size. My suggestion would be to add something like 
> get_reg_wordsize to struct eth_dev_ops and then to use sizeof(uint32) 
> as fallback for drivers that don't implement the callback.
>
> Regards,
>
> ..Rémy
diff mbox

Patch

diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index 42e05f1..bf0a6ac 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -91,7 +91,7 @@  rte_ethtool_get_regs_len(uint8_t port_id)
 
 	count_regs = rte_eth_dev_get_reg_length(port_id);
 	if (count_regs > 0)
-		return count_regs * sizeof(uint32_t);
+		return count_regs * sizeof(uint64_t);
 	return count_regs;
 }