On Thu, 19 Jan 2023 at 11:18, Vyacheslav V. Mitrofanov v.v.mitrofanov@yadro.com wrote:

On Wed, 2023-01-18 at 20:52 +0300, Sergei Antonov wrote:

...

Commands "ping6" and "tftpboot ... -ipv6" did not work on ARM because machine code expects 4-byte alignment and some structures from net6.h are not aligned in memory.

Fix by adding __packed, since it is already used in this file.

Signed-off-by: Sergei Antonov saproj@gmail.com

include/net6.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net6.h b/include/net6.h index 9b3de028e6dc..2d7c5a096046 100644 --- a/include/net6.h +++ b/include/net6.h @@ -24,7 +24,7 @@ struct in6_addr { #define s6_addr in6_u.u6_addr8 #define s6_addr16 in6_u.u6_addr16 #define s6_addr32 in6_u.u6_addr32 -}; +} __packed;

#define IN6ADDRSZ sizeof(struct in6_addr) #define INETHADDRSZ sizeof(net_ethaddr) @@ -62,7 +62,7 @@ struct ip6_hdr { u8 hop_limit; struct in6_addr saddr; struct in6_addr daddr; -}; +} __packed; #define IP6_HDR_SIZE (sizeof(struct ip6_hdr))

/* struct udp_hdr - User Datagram Protocol header */ @@ -164,7 +164,7 @@ struct icmp6hdr { #define icmp6_addrconf_managed icmp6_dataun.u_nd_ra.managed #define icmp6_addrconf_other icmp6_dataun.u_nd_ra.other #define icmp6_rt_lifetime icmp6_dataun.u_nd_ra.rt_lifetime -}; +} __packed;

extern struct in6_addr const net_null_addr_ip6; /* NULL IPv6 address */ extern struct in6_addr net_gateway6; /* Our gateways IPv6 address

*/

2.34.1

Hello, Sergei!

I didn't get you a little bit. You mean holes between fields of structures or alignment of the beginning?

No, it is not about holes between fields. It is about the address at which a structure is placed in memory. In combination with store merging it leads to unaligned memory access. At least for struct ip6_hdr, which I studied using assembly listing files. The struct ip6_hdr is not placed at 4-byte aligned address because it is 14 bytes off from the beginning of the Ethernet packet.

Also see how the corresponding IPv4 struct ip_hdr from net.h was fixed by commit 704f3acfcf553. I should have referred to it in my commit description.

Frankly speaking I always thought that in that kind of structure like below it is not necessary. To be honest I disassembled the same code on arm and there were no holes.

The following code from net6.c

int ip6_add_hdr(uchar *xip, struct in6_addr *src, struct in6_addr *dest, int nextheader, int hoplimit, int payload_len) { struct ip6_hdr *ip6 = (struct ip6_hdr *)xip;

ip6->version = 6; ip6->priority = 0; ip6->flow_lbl[0] = 0; ip6->flow_lbl[1] = 0; ip6->flow_lbl[2] = 0;

is optimized into: 801ab138: e3a02060 mov r2, #96 @ 0x60 801ab13c: e1a04000 mov r4, r0 801ab140: e5802000 str r2, [r0]

The last instruction does all 5 different assignments from C code at once.