[U-Boot] [PATCH v2 1/1] net: avoid address-of-packed-member error
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: reword commit message add a comment simplify checksumming of last byte --- net/net.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/net/net.c b/net/net.c index ded86e7456..bcb9d5f07e 100644 --- a/net/net.c +++ b/net/net.c @@ -1274,7 +1274,7 @@ void net_process_received_packet(uchar *in_packet, int len) #ifdef CONFIG_UDP_CHECKSUM if (ip->udp_xsum != 0) { ulong xsum; - ushort *sumptr; + u8 *sumptr; ushort sumlen;
xsum = ip->ip_p; @@ -1285,22 +1285,16 @@ void net_process_received_packet(uchar *in_packet, int len) xsum += (ntohl(ip->ip_dst.s_addr) >> 0) & 0x0000ffff;
sumlen = ntohs(ip->udp_len); - sumptr = (ushort *)&(ip->udp_src); + sumptr = (u8 *)&ip->udp_src;
while (sumlen > 1) { - ushort sumdata; - - sumdata = *sumptr++; - xsum += ntohs(sumdata); + /* inlined ntohs() to avoid alignment errors */ + xsum += (sumptr[0] << 8) + sumptr[1]; + sumptr += 2; sumlen -= 2; } - if (sumlen > 0) { - ushort sumdata; - - sumdata = *(unsigned char *)sumptr; - sumdata = (sumdata << 8) & 0xff00; - xsum += sumdata; - } + if (sumlen > 0) + xsum += (sumptr[0] << 8) + sumptr[0]; while ((xsum >> 16) != 0) { xsum = (xsum & 0x0000ffff) + ((xsum >> 16) & 0x0000ffff); -- 2.24.0.rc1
On Tue, Nov 05, 2019 at 12:48:19PM +0100, Heinrich Schuchardt wrote:
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: reword commit message add a comment simplify checksumming of last byte
If I follow what Simon was saying yesterday, the whole message framing is wrong. The problem isn't that gcc 9.2 is showing a warning, the problem is that gcc 9.2 is showing that we have a problem (in terms of what can/can't happen in real life networking terms), which you're correcting. Simon, can you please suggest a commit message here if you're still not quite happy, as you understand the underlying problem well it seems. Thanks!
On Tue, Nov 5, 2019 at 2:52 PM Tom Rini trini@konsulko.com wrote:
On Tue, Nov 05, 2019 at 12:48:19PM +0100, Heinrich Schuchardt wrote:
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2: reword commit message add a comment simplify checksumming of last byte
If I follow what Simon was saying yesterday, the whole message framing is wrong. The problem isn't that gcc 9.2 is showing a warning, the problem is that gcc 9.2 is showing that we have a problem (in terms of what can/can't happen in real life networking terms), which you're correcting. Simon, can you please suggest a commit message here if you're still not quite happy, as you understand the underlying problem well it seems. Thanks!
Well, we do have an error and GCC 9.2 shows this. I don't know why other versions don't issue this warning.
This new commit message might still concentrate too much on the GCC version, but I think it's ok. I just wanted to prevent someone reading this in the future and taking it as a hint that the attribute 'packed' can be removed (which in turn might procude bugs on some platforms).
So: Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
On Tue, Nov 5, 2019 at 5:49 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Seems reasonable.
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
On 11/6/19 12:07 AM, Joe Hershberger wrote:
On Tue, Nov 5, 2019 at 5:49 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Seems reasonable.
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
Hello Joe,
this patch did not yet make it into https://gitlab.denx.de/u-boot/custodians/u-boot-net/commits/master
Is there something that needs to be changed?
Best regards
Heinrich
On Thu, Dec 05, 2019 at 08:16:15AM +0100, Heinrich Schuchardt wrote:
On 11/6/19 12:07 AM, Joe Hershberger wrote:
On Tue, Nov 5, 2019 at 5:49 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Seems reasonable.
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
Hello Joe,
this patch did not yet make it into https://gitlab.denx.de/u-boot/custodians/u-boot-net/commits/master
Is there something that needs to be changed?
And perhaps we need to split the net PR in to bug fixes to get now and stuff to put in to -next, for the next window? Or is this one of the changes that causes size overflow?
On Thu, Dec 5, 2019 at 1:19 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/6/19 12:07 AM, Joe Hershberger wrote:
On Tue, Nov 5, 2019 at 5:49 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
sandbox_defconfig does not compile using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’: net/net.c:1288:23: error: taking address of packed member of ‘struct ip_udp_hdr’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1288 | sumptr = (ushort *)&(ip->udp_src); | ^~~~~~~~~~~~~~
Avoid the error by using a u8 pointer instead of an u16 pointer and in-lining ntohs().
Seems reasonable.
Simplify the checksumming of the last message byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Acked-by: Joe Hershberger joe.hershberger@ni.com
Hello Joe,
this patch did not yet make it into https://gitlab.denx.de/u-boot/custodians/u-boot-net/commits/master
Is there something that needs to be changed?
No, it is among the patches I'm currently testing [1].
[1] - https://github.com/jhershbe/u-boot/commits/travis_ci/test_1184510
Cheers, -Joe
participants (4)
-
Heinrich Schuchardt -
Joe Hershberger -
Simon Goldschmidt -
Tom Rini