| Message ID | 20230417072641.1656960-1-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 99676a5766412f3936c55b9d18565d248e5463ee |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: lan966x: Fix lan966x_ifh_get | expand |
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Mon, 17 Apr 2023 09:26:41 +0200 you wrote: > From time to time, it was observed that the nanosecond part of the > received timestamp, which is extracted from the IFH, it was actually > bigger than 1 second. So then when actually calculating the full > received timestamp, based on the nanosecond part from IFH and the second > part which is read from HW, it was actually wrong. > > The issue seems to be inside the function lan966x_ifh_get, which > extracts information from an IFH(which is an byte array) and returns the > value in a u64. When extracting the timestamp value from the IFH, which > starts at bit 192 and have the size of 32 bits, then if the most > significant bit was set in the timestamp, then this bit was extended > then the return value became 0xffffffff... . And the reason of this is > because constants without any postfix are treated as signed longs and > that is the reason why '1 << 31' becomes 0xffffffff80000000. > This is fixed by adding the postfix 'ULL' to 1. > > [...] Here is the summary with links: - [net-next,v2] net: lan966x: Fix lan966x_ifh_get https://git.kernel.org/netdev/net-next/c/99676a576641 You are awesome, thank you!
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c index 80e2ea7e6ce8a..5f01b21acdd1b 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c @@ -605,7 +605,7 @@ static u64 lan966x_ifh_get(u8 *ifh, size_t pos, size_t length) v = ifh[IFH_LEN_BYTES - (j / 8) - 1]; if (v & (1 << k)) - val |= (1 << i); + val |= (1ULL << i); } return val;
From time to time, it was observed that the nanosecond part of the received timestamp, which is extracted from the IFH, it was actually bigger than 1 second. So then when actually calculating the full received timestamp, based on the nanosecond part from IFH and the second part which is read from HW, it was actually wrong. The issue seems to be inside the function lan966x_ifh_get, which extracts information from an IFH(which is an byte array) and returns the value in a u64. When extracting the timestamp value from the IFH, which starts at bit 192 and have the size of 32 bits, then if the most significant bit was set in the timestamp, then this bit was extended then the return value became 0xffffffff... . And the reason of this is because constants without any postfix are treated as signed longs and that is the reason why '1 << 31' becomes 0xffffffff80000000. This is fixed by adding the postfix 'ULL' to 1. Fixes: fd7627833ddf ("net: lan966x: Stop using packing library") Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- v1->v2: - use postfix ULL when setting the bit in the val instead of masking in the end the val. --- drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)