[U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
struct ip_udp_hdr is naturally packed. There is no point in adding a __packed attribute. With the attribute the network stack 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); | ^~~~~~~~~~~~~~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 75a16e4c8f..bd875b56f5 100644 --- a/include/net.h +++ b/include/net.h @@ -390,7 +390,7 @@ struct ip_udp_hdr { u16 udp_dst; /* UDP destination port */ u16 udp_len; /* Length of UDP packet */ u16 udp_xsum; /* Checksum */ -} __attribute__((packed)); +};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE) -- 2.23.0
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a __packed attribute. With the attribute the network stack 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); | ^~~~~~~~~~~~~~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 75a16e4c8f..bd875b56f5 100644 --- a/include/net.h +++ b/include/net.h @@ -390,7 +390,7 @@ struct ip_udp_hdr { u16 udp_dst; /* UDP destination port */ u16 udp_len; /* Length of UDP packet */ u16 udp_xsum; /* Checksum */ -} __attribute__((packed)); +};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from: commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a Author: Denis Pynkin denis.pynkin@collabora.com Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled by default for '-O2':
BOOTP broadcast 1 data abort pc : [<8ff8bb30>] lr : [<00004f1f>] reloc pc : [<17832b30>] lr : [<878abf1f>] sp : 8f558bc0 ip : 00000000 fp : 8ffef5a4 r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594 r7 : 0000000e r6 : 8ffed700 r5 : 00000000 r4 : 8ffed74e r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 00000ddd Flags: nzcv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
Core reason is usage of structures for network headers without packed attribute.
Reviewed-by: Yauheni Kaliuta yauheni.kaliuta@redhat.com Signed-off-by: Denis Pynkin denis.pynkin@collabora.com Acked-by: Joe Hershberger joe.hershberger@ni.com
... so adding a few folks to the list and lets see if patchwork picks up this tag: Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a __packed attribute. With the attribute the network stack 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); | ^~~~~~~~~~~~~~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 75a16e4c8f..bd875b56f5 100644 --- a/include/net.h +++ b/include/net.h @@ -390,7 +390,7 @@ struct ip_udp_hdr { u16 udp_dst; /* UDP destination port */ u16 udp_len; /* Length of UDP packet */ u16 udp_xsum; /* Checksum */ -} __attribute__((packed)); +};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from: commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a Author: Denis Pynkin denis.pynkin@collabora.com Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled by default for '-O2': BOOTP broadcast 1 data abort pc : [<8ff8bb30>] lr : [<00004f1f>] reloc pc : [<17832b30>] lr : [<878abf1f>] sp : 8f558bc0 ip : 00000000 fp : 8ffef5a4 r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594 r7 : 0000000e r6 : 8ffed700 r5 : 00000000 r4 : 8ffed74e r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 00000ddd Flags: nzcv IRQs off FIQs off Mode SVC_32 Resetting CPU ... Core reason is usage of structures for network headers without packed attribute. Reviewed-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com> Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com> Acked-by: Joe Hershberger <joe.hershberger@ni.com>... so adding a few folks to the list and lets see if patchwork picks up this tag: Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a __packed attribute. With the attribute the network stack 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); | ^~~~~~~~~~~~~~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 75a16e4c8f..bd875b56f5 100644 --- a/include/net.h +++ b/include/net.h @@ -390,7 +390,7 @@ struct ip_udp_hdr { u16 udp_dst; /* UDP destination port */ u16 udp_len; /* Length of UDP packet */ u16 udp_xsum; /* Checksum */ -} __attribute__((packed)); +};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from: commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a Author: Denis Pynkin denis.pynkin@collabora.com Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled by default for '-O2':
BOOTP broadcast 1 data abort pc : [<8ff8bb30>] lr : [<00004f1f>] reloc pc : [<17832b30>] lr : [<878abf1f>] sp : 8f558bc0 ip : 00000000 fp : 8ffef5a4 r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594 r7 : 0000000e r6 : 8ffed700 r5 : 00000000 r4 : 8ffed74e r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 00000ddd Flags: nzcv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
Core reason is usage of structures for network headers without packed attribute.
Reviewed-by: Yauheni Kaliuta yauheni.kaliuta@redhat.com Signed-off-by: Denis Pynkin denis.pynkin@collabora.com Acked-by: Joe Hershberger joe.hershberger@ni.com
... so adding a few folks to the list and lets see if patchwork picks up this tag: Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
Hello Joe,
how will we carry on, to get the issue resolved?
Best regards
Heinrich
On 10/12/19 3:29 PM, Heinrich Schuchardt wrote:
On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a __packed attribute. With the attribute the network stack 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); | ^~~~~~~~~~~~~~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 75a16e4c8f..bd875b56f5 100644 --- a/include/net.h +++ b/include/net.h @@ -390,7 +390,7 @@ struct ip_udp_hdr { u16 udp_dst; /* UDP destination port */ u16 udp_len; /* Length of UDP packet */ u16 udp_xsum; /* Checksum */ -} __attribute__((packed)); +};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from: commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a Author: Denis Pynkin denis.pynkin@collabora.com Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled by default for '-O2':
BOOTP broadcast 1 data abort pc : [<8ff8bb30>] lr : [<00004f1f>] reloc pc : [<17832b30>] lr : [<878abf1f>] sp : 8f558bc0 ip : 00000000 fp : 8ffef5a4 r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594 r7 : 0000000e r6 : 8ffed700 r5 : 00000000 r4 : 8ffed74e r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 00000ddd Flags: nzcv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
Core reason is usage of structures for network headers without packed attribute.
Reviewed-by: Yauheni Kaliuta yauheni.kaliuta@redhat.com Signed-off-by: Denis Pynkin denis.pynkin@collabora.com Acked-by: Joe Hershberger joe.hershberger@ni.com
... so adding a few folks to the list and lets see if patchwork picks up this tag: Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
Hello Joe,
how will we carry on, to get the issue resolved?
Best regards
Heinrich _______________________________________________
Hello Tom, hello Joe,
I still cannot build sandbox_defconfig. How will we resolve this issue?
Best regards
Heinrich
On Mon, Nov 04, 2019 at 07:07:46PM +0100, Heinrich Schuchardt wrote:
On 10/12/19 3:29 PM, Heinrich Schuchardt wrote:
On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a __packed attribute. With the attribute the network stack 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); | ^~~~~~~~~~~~~~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h index 75a16e4c8f..bd875b56f5 100644 --- a/include/net.h +++ b/include/net.h @@ -390,7 +390,7 @@ struct ip_udp_hdr { u16 udp_dst; /* UDP destination port */ u16 udp_len; /* Length of UDP packet */ u16 udp_xsum; /* Checksum */ -} __attribute__((packed)); +};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from: commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a Author: Denis Pynkin denis.pynkin@collabora.com Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled by default for '-O2':
BOOTP broadcast 1 data abort pc : [<8ff8bb30>] lr : [<00004f1f>] reloc pc : [<17832b30>] lr : [<878abf1f>] sp : 8f558bc0 ip : 00000000 fp : 8ffef5a4 r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594 r7 : 0000000e r6 : 8ffed700 r5 : 00000000 r4 : 8ffed74e r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 00000ddd Flags: nzcv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
Core reason is usage of structures for network headers without packed attribute.
Reviewed-by: Yauheni Kaliuta yauheni.kaliuta@redhat.com Signed-off-by: Denis Pynkin denis.pynkin@collabora.com Acked-by: Joe Hershberger joe.hershberger@ni.com
... so adding a few folks to the list and lets see if patchwork picks up this tag: Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
Hello Joe,
how will we carry on, to get the issue resolved?
Best regards
Heinrich _______________________________________________
Hello Tom, hello Joe,
I still cannot build sandbox_defconfig. How will we resolve this issue?
We can't drop gcc-7.1 support at this point. Perhaps use an accessor function of some sort?
participants (3)
-
Heinrich Schuchardt -
Heinrich Schuchardt -
Tom Rini