All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot]  [PATCH] Use packed structures for networking
@ 2017-07-21 16:28 Denis Pynkin
  2017-07-21 18:05 ` Fabio Estevam
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Denis Pynkin @ 2017-07-21 16:28 UTC (permalink / raw)
  To: u-boot

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>
---
 include/net.h | 14 +++++++-------
 net/bootp.h   |  2 +-
 net/dns.h     |  2 +-
 net/nfs.h     |  2 +-
 net/sntp.h    |  2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/net.h b/include/net.h
index 2eaa882..e126948 100644
--- a/include/net.h
+++ b/include/net.h
@@ -308,7 +308,7 @@ struct ethernet_hdr {
 	u8		et_dest[ARP_HLEN];	/* Destination node	*/
 	u8		et_src[ARP_HLEN];	/* Source node		*/
 	u16		et_protlen;		/* Protocol or length	*/
-};
+} __attribute__((packed));
 
 /* Ethernet header size */
 #define ETHER_HDR_SIZE	(sizeof(struct ethernet_hdr))
@@ -326,7 +326,7 @@ struct e802_hdr {
 	u8		et_snap2;
 	u8		et_snap3;
 	u16		et_prot;		/* 802 protocol		*/
-};
+} __attribute__((packed));
 
 /* 802 + SNAP + ethernet header size */
 #define E802_HDR_SIZE	(sizeof(struct e802_hdr))
@@ -340,7 +340,7 @@ struct vlan_ethernet_hdr {
 	u16		vet_vlan_type;		/* PROT_VLAN		*/
 	u16		vet_tag;		/* TAG of VLAN		*/
 	u16		vet_type;		/* protocol type	*/
-};
+} __attribute__((packed));
 
 /* VLAN Ethernet header size */
 #define VLAN_ETHER_HDR_SIZE	(sizeof(struct vlan_ethernet_hdr))
@@ -369,7 +369,7 @@ struct ip_hdr {
 	u16		ip_sum;		/* checksum			*/
 	struct in_addr	ip_src;		/* Source IP address		*/
 	struct in_addr	ip_dst;		/* Destination IP address	*/
-};
+} __attribute__((packed));
 
 #define IP_OFFS		0x1fff /* ip offset *= 8 */
 #define IP_FLAGS	0xe000 /* first 3 bits */
@@ -397,7 +397,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)
@@ -435,7 +435,7 @@ struct arp_hdr {
 	u8		ar_tha[];	/* Target hardware address	*/
 	u8		ar_tpa[];	/* Target protocol address	*/
 #endif /* 0 */
-};
+} __attribute__((packed));
 
 #define ARP_HDR_SIZE	(8+20)		/* Size assuming ethernet	*/
 
@@ -470,7 +470,7 @@ struct icmp_hdr {
 		} frag;
 		u8 data[0];
 	} un;
-};
+} __attribute__((packed));
 
 #define ICMP_HDR_SIZE		(sizeof(struct icmp_hdr))
 #define IP_ICMP_HDR_SIZE	(IP_HDR_SIZE + ICMP_HDR_SIZE)
diff --git a/net/bootp.h b/net/bootp.h
index fcb0a64..a1291be 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -49,7 +49,7 @@ struct bootp_hdr {
 	char		bp_sname[64];	/* Server host name		*/
 	char		bp_file[128];	/* Boot file name		*/
 	char		bp_vend[OPT_FIELD_SIZE]; /* Vendor information	*/
-};
+}__attribute__((packed));
 
 #define BOOTP_HDR_SIZE	sizeof(struct bootp_hdr)
 
diff --git a/net/dns.h b/net/dns.h
index c4e96af..c55a5c1 100644
--- a/net/dns.h
+++ b/net/dns.h
@@ -29,7 +29,7 @@ struct header {
 	uint16_t	nauth;		/* Authority PRs */
 	uint16_t	nother;		/* Other PRs */
 	unsigned char	data[1];	/* Data, variable length */
-};
+} __attribute__((packed));
 
 void dns_start(void);		/* Begin DNS */
 
diff --git a/net/nfs.h b/net/nfs.h
index 45da246..70a1a6d 100644
--- a/net/nfs.h
+++ b/net/nfs.h
@@ -79,7 +79,7 @@ struct rpc_t {
 			uint32_t data[NFS_READ_SIZE];
 		} reply;
 	} u;
-};
+} __attribute__((packed));
 void nfs_start(void);	/* Begin NFS */
 
 
diff --git a/net/sntp.h b/net/sntp.h
index 6a9c6bb..c38bcee 100644
--- a/net/sntp.h
+++ b/net/sntp.h
@@ -51,7 +51,7 @@ struct sntp_pkt_t {
 	unsigned long long originate_timestamp;
 	unsigned long long receive_timestamp;
 	unsigned long long transmit_timestamp;
-};
+} __attribute__((packed));
 
 void sntp_start(void);	/* Begin SNTP */
 
-- 
2.10.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-21 16:28 [U-Boot] [PATCH] Use packed structures for networking Denis Pynkin
@ 2017-07-21 18:05 ` Fabio Estevam
  2017-07-21 19:00 ` Tom Rini
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2017-07-21 18:05 UTC (permalink / raw)
  To: u-boot

Adding Tom and Joe.

On Fri, Jul 21, 2017 at 1:28 PM, Denis Pynkin
<denis.pynkin@collabora.com> wrote:
> 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>
> ---
>  include/net.h | 14 +++++++-------
>  net/bootp.h   |  2 +-
>  net/dns.h     |  2 +-
>  net/nfs.h     |  2 +-
>  net/sntp.h    |  2 +-
>  5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/include/net.h b/include/net.h
> index 2eaa882..e126948 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -308,7 +308,7 @@ struct ethernet_hdr {
>         u8              et_dest[ARP_HLEN];      /* Destination node     */
>         u8              et_src[ARP_HLEN];       /* Source node          */
>         u16             et_protlen;             /* Protocol or length   */
> -};
> +} __attribute__((packed));
>
>  /* Ethernet header size */
>  #define ETHER_HDR_SIZE (sizeof(struct ethernet_hdr))
> @@ -326,7 +326,7 @@ struct e802_hdr {
>         u8              et_snap2;
>         u8              et_snap3;
>         u16             et_prot;                /* 802 protocol         */
> -};
> +} __attribute__((packed));
>
>  /* 802 + SNAP + ethernet header size */
>  #define E802_HDR_SIZE  (sizeof(struct e802_hdr))
> @@ -340,7 +340,7 @@ struct vlan_ethernet_hdr {
>         u16             vet_vlan_type;          /* PROT_VLAN            */
>         u16             vet_tag;                /* TAG of VLAN          */
>         u16             vet_type;               /* protocol type        */
> -};
> +} __attribute__((packed));
>
>  /* VLAN Ethernet header size */
>  #define VLAN_ETHER_HDR_SIZE    (sizeof(struct vlan_ethernet_hdr))
> @@ -369,7 +369,7 @@ struct ip_hdr {
>         u16             ip_sum;         /* checksum                     */
>         struct in_addr  ip_src;         /* Source IP address            */
>         struct in_addr  ip_dst;         /* Destination IP address       */
> -};
> +} __attribute__((packed));
>
>  #define IP_OFFS                0x1fff /* ip offset *= 8 */
>  #define IP_FLAGS       0xe000 /* first 3 bits */
> @@ -397,7 +397,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)
> @@ -435,7 +435,7 @@ struct arp_hdr {
>         u8              ar_tha[];       /* Target hardware address      */
>         u8              ar_tpa[];       /* Target protocol address      */
>  #endif /* 0 */
> -};
> +} __attribute__((packed));
>
>  #define ARP_HDR_SIZE   (8+20)          /* Size assuming ethernet       */
>
> @@ -470,7 +470,7 @@ struct icmp_hdr {
>                 } frag;
>                 u8 data[0];
>         } un;
> -};
> +} __attribute__((packed));
>
>  #define ICMP_HDR_SIZE          (sizeof(struct icmp_hdr))
>  #define IP_ICMP_HDR_SIZE       (IP_HDR_SIZE + ICMP_HDR_SIZE)
> diff --git a/net/bootp.h b/net/bootp.h
> index fcb0a64..a1291be 100644
> --- a/net/bootp.h
> +++ b/net/bootp.h
> @@ -49,7 +49,7 @@ struct bootp_hdr {
>         char            bp_sname[64];   /* Server host name             */
>         char            bp_file[128];   /* Boot file name               */
>         char            bp_vend[OPT_FIELD_SIZE]; /* Vendor information  */
> -};
> +}__attribute__((packed));
>
>  #define BOOTP_HDR_SIZE sizeof(struct bootp_hdr)
>
> diff --git a/net/dns.h b/net/dns.h
> index c4e96af..c55a5c1 100644
> --- a/net/dns.h
> +++ b/net/dns.h
> @@ -29,7 +29,7 @@ struct header {
>         uint16_t        nauth;          /* Authority PRs */
>         uint16_t        nother;         /* Other PRs */
>         unsigned char   data[1];        /* Data, variable length */
> -};
> +} __attribute__((packed));
>
>  void dns_start(void);          /* Begin DNS */
>
> diff --git a/net/nfs.h b/net/nfs.h
> index 45da246..70a1a6d 100644
> --- a/net/nfs.h
> +++ b/net/nfs.h
> @@ -79,7 +79,7 @@ struct rpc_t {
>                         uint32_t data[NFS_READ_SIZE];
>                 } reply;
>         } u;
> -};
> +} __attribute__((packed));
>  void nfs_start(void);  /* Begin NFS */
>
>
> diff --git a/net/sntp.h b/net/sntp.h
> index 6a9c6bb..c38bcee 100644
> --- a/net/sntp.h
> +++ b/net/sntp.h
> @@ -51,7 +51,7 @@ struct sntp_pkt_t {
>         unsigned long long originate_timestamp;
>         unsigned long long receive_timestamp;
>         unsigned long long transmit_timestamp;
> -};
> +} __attribute__((packed));
>
>  void sntp_start(void); /* Begin SNTP */
>
> --
> 2.10.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-21 16:28 [U-Boot] [PATCH] Use packed structures for networking Denis Pynkin
  2017-07-21 18:05 ` Fabio Estevam
@ 2017-07-21 19:00 ` Tom Rini
  2017-07-27 21:26   ` Joe Hershberger
  2017-08-01 16:37 ` Joe Hershberger
  2017-08-07 20:33 ` [U-Boot] net: " Joe Hershberger
  3 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2017-07-21 19:00 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 21, 2017 at 07:28:42PM +0300, Denis Pynkin wrote:

> 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>
> ---
>  include/net.h | 14 +++++++-------
>  net/bootp.h   |  2 +-
>  net/dns.h     |  2 +-
>  net/nfs.h     |  2 +-
>  net/sntp.h    |  2 +-
>  5 files changed, 11 insertions(+), 11 deletions(-)

So, what I've been wondering, and others have poked me about (who can
chime in if they like), how is the kernel not also tripping over this?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170721/4da72d93/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-21 19:00 ` Tom Rini
@ 2017-07-27 21:26   ` Joe Hershberger
  2017-07-29  9:53     ` Denis Pynkin
  2017-07-29  9:56     ` Denis Pynkin
  0 siblings, 2 replies; 9+ messages in thread
From: Joe Hershberger @ 2017-07-27 21:26 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 21, 2017 at 2:00 PM, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Jul 21, 2017 at 07:28:42PM +0300, Denis Pynkin wrote:
>
>> 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>
>> ---
>>  include/net.h | 14 +++++++-------
>>  net/bootp.h   |  2 +-
>>  net/dns.h     |  2 +-
>>  net/nfs.h     |  2 +-
>>  net/sntp.h    |  2 +-
>>  5 files changed, 11 insertions(+), 11 deletions(-)
>
> So, what I've been wondering, and others have poked me about (who can
> chime in if they like), how is the kernel not also tripping over this?

Great question.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-27 21:26   ` Joe Hershberger
@ 2017-07-29  9:53     ` Denis Pynkin
  2017-07-29 12:04       ` Tom Rini
  2017-07-29  9:56     ` Denis Pynkin
  1 sibling, 1 reply; 9+ messages in thread
From: Denis Pynkin @ 2017-07-29  9:53 UTC (permalink / raw)
  To: u-boot

28.07.2017 00:26, Joe Hershberger wrote:

>>> PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
>>> by default for '-O2':

>> So, what I've been wondering, and others have poked me about (who can
>> chime in if they like), how is the kernel not also tripping over this?
> 
> Great question.

Believe kernel do not work in single static buffer for the whole packet 
as u-boot do.

Problem is in ethernet header structure with size ETHER_HDR_SIZE=14, so 
IP and BOOTP headers are not aligned to 4 bytes.

Let's take bootp.c code as example:

755     bp = (struct bootp_hdr *)pkt;
756     bp->bp_op = OP_BOOTREQUEST;
757     bp->bp_htype = HWT_ETHER;
758     bp->bp_hlen = HWL_ETHER;
759     bp->bp_hops = 0;

Without '-fstore-merging' or with enabled 'packed' structures assembler 
code looks like:
   6a:   77a3          strb    r3, [r4, #30]
   6c:   f884 b01c     strb.w  fp, [r4, #28]
   70:   f884 b01d     strb.w  fp, [r4, #29]
   74:   77e5          strb    r5, [r4, #31]

but with option '-fstore-merging' all these instructions merged into 
single instruction:
   62:   61e3          str     r3, [r4, #28]

and store address is not aligned to 32b boundary here, so it results to 
'data abort'.

I see some possible solutions here:
- add option '-fno-store-merging' -- works, I test it already
- use packed structures -- since there are some unsafe code in sources
- rewrite networking part to work with any protocol separately and merge 
headers and data only on send.

Even simple reordering of header flags initialization helps here, but it 
looks like a bit of black magic in code.

PS forgot to mention in patch description -- '-Os' includes 
'-fstore-merging' option as well for gcc 7.1.


-- 
wbr, Denis

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-27 21:26   ` Joe Hershberger
  2017-07-29  9:53     ` Denis Pynkin
@ 2017-07-29  9:56     ` Denis Pynkin
  1 sibling, 0 replies; 9+ messages in thread
From: Denis Pynkin @ 2017-07-29  9:56 UTC (permalink / raw)
  To: u-boot



28.07.2017 00:26, Joe Hershberger wrote:

>>> PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
>>> by default for '-O2':

>> So, what I've been wondering, and others have poked me about (who can
>> chime in if they like), how is the kernel not also tripping over this?
> 
> Great question.
> 

Believe kernel do not work in single static buffer for the whole packet 
as u-boot do.

Problem is in ethernet header structure with size ETHER_HDR_SIZE=14, so 
IP and BOOTP headers are not aligned to 4 bytes.

Let's take bootp.c code as example:

755     bp = (struct bootp_hdr *)pkt;
756     bp->bp_op = OP_BOOTREQUEST;
757     bp->bp_htype = HWT_ETHER;
758     bp->bp_hlen = HWL_ETHER;
759     bp->bp_hops = 0;

Without '-fstore-merging' or with enabled 'packed' structures assembler 
code looks like:
   6a:   77a3          strb    r3, [r4, #30]
   6c:   f884 b01c     strb.w  fp, [r4, #28]
   70:   f884 b01d     strb.w  fp, [r4, #29]
   74:   77e5          strb    r5, [r4, #31]

but with option '-fstore-merging' all these instructions merged into 
single instruction:
   62:   61e3          str     r3, [r4, #28]

and store address is not aligned to 32b boundary here, so it results to 
'data abort'.

I see some possible solutions here:
- add option '-fno-store-merging' -- works, I test it already
- use packed structures -- since there are some unsafe code in sources
- rewrite networking part to work with any protocol separately and merge 
headers and data only on send.

Even simple reordering of header flags initialization helps here, but it 
looks like a bit of black magic in code.

PS forgot to mention in patch description -- '-Os' includes 
'-fstore-merging' option as well for gcc 7.1.

-- 
wbr, Denis

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-29  9:53     ` Denis Pynkin
@ 2017-07-29 12:04       ` Tom Rini
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2017-07-29 12:04 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 29, 2017 at 12:53:36PM +0300, Denis Pynkin wrote:
> 28.07.2017 00:26, Joe Hershberger wrote:
> 
> >>>PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
> >>>by default for '-O2':
> 
> >>So, what I've been wondering, and others have poked me about (who can
> >>chime in if they like), how is the kernel not also tripping over this?
> >
> >Great question.
> 
> Believe kernel do not work in single static buffer for the whole
> packet as u-boot do.
> 
> Problem is in ethernet header structure with size ETHER_HDR_SIZE=14,
> so IP and BOOTP headers are not aligned to 4 bytes.
> 
> Let's take bootp.c code as example:
> 
> 755     bp = (struct bootp_hdr *)pkt;
> 756     bp->bp_op = OP_BOOTREQUEST;
> 757     bp->bp_htype = HWT_ETHER;
> 758     bp->bp_hlen = HWL_ETHER;
> 759     bp->bp_hops = 0;
> 
> Without '-fstore-merging' or with enabled 'packed' structures
> assembler code looks like:
>   6a:   77a3          strb    r3, [r4, #30]
>   6c:   f884 b01c     strb.w  fp, [r4, #28]
>   70:   f884 b01d     strb.w  fp, [r4, #29]
>   74:   77e5          strb    r5, [r4, #31]
> 
> but with option '-fstore-merging' all these instructions merged into
> single instruction:
>   62:   61e3          str     r3, [r4, #28]
> 
> and store address is not aligned to 32b boundary here, so it results
> to 'data abort'.
> 
> I see some possible solutions here:
> - add option '-fno-store-merging' -- works, I test it already
> - use packed structures -- since there are some unsafe code in sources
> - rewrite networking part to work with any protocol separately and
> merge headers and data only on send.
> 
> Even simple reordering of header flags initialization helps here,
> but it looks like a bit of black magic in code.
> 
> PS forgot to mention in patch description -- '-Os' includes
> '-fstore-merging' option as well for gcc 7.1.

Thanks for explaining.  My inclination is that we should have a packed
change in for -rc1.  Philipp, have you had a chance to edit
doc/README.unaligned-memory-access.txt to your liking?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170729/8b39f85b/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH] Use packed structures for networking
  2017-07-21 16:28 [U-Boot] [PATCH] Use packed structures for networking Denis Pynkin
  2017-07-21 18:05 ` Fabio Estevam
  2017-07-21 19:00 ` Tom Rini
@ 2017-08-01 16:37 ` Joe Hershberger
  2017-08-07 20:33 ` [U-Boot] net: " Joe Hershberger
  3 siblings, 0 replies; 9+ messages in thread
From: Joe Hershberger @ 2017-08-01 16:37 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 21, 2017 at 11:28 AM, Denis Pynkin
<denis.pynkin@collabora.com> wrote:
> 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>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] net: Use packed structures for networking
  2017-07-21 16:28 [U-Boot] [PATCH] Use packed structures for networking Denis Pynkin
                   ` (2 preceding siblings ...)
  2017-08-01 16:37 ` Joe Hershberger
@ 2017-08-07 20:33 ` Joe Hershberger
  3 siblings, 0 replies; 9+ messages in thread
From: Joe Hershberger @ 2017-08-07 20:33 UTC (permalink / raw)
  To: u-boot

Hi Denis,

https://patchwork.ozlabs.org/patch/792238/ was applied to u-boot-net.git.

Thanks!
-Joe

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-08-07 20:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-21 16:28 [U-Boot] [PATCH] Use packed structures for networking Denis Pynkin
2017-07-21 18:05 ` Fabio Estevam
2017-07-21 19:00 ` Tom Rini
2017-07-27 21:26   ` Joe Hershberger
2017-07-29  9:53     ` Denis Pynkin
2017-07-29 12:04       ` Tom Rini
2017-07-29  9:56     ` Denis Pynkin
2017-08-01 16:37 ` Joe Hershberger
2017-08-07 20:33 ` [U-Boot] net: " Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.