Re: CVS commit: src/sys

[Kamil Rytarowski <kamil%netbsd.org@localhost>]

On 03.02.2021 06:51, Roy Marples wrote:
> Module Name:	src
> Committed By:	roy
> Date:		Wed Feb  3 05:51:40 UTC 2021
> 
> Modified Files:
> 	src/sys/net: if_arp.h if_ether.h if_gre.h
> 	src/sys/netinet: if_ether.h igmp.h in.h ip.h ip6.h ip_carp.h ip_icmp.h
> 	    ip_mroute.h ip_var.h tcp.h tcp_debug.h tcp_var.h udp.h udp_var.h
> 
> Log Message:
> Remove __packed from various network structures
> 
> They are already network aligned and adding the __packed attribute
> just causes needless compiler warnings about accssing members of packed
> objects.
> 

This changes the ABI for userland programs. With __packed, these
structures, whenever contain at least 2-byte data, can be placed in a
different location inside another structure now.

#include <sys/cdefs.h>
#include <stdio.h>
#include <stdint.h>
#include <stdalign.h>

struct aaa {
        uint16_t a;
        uint8_t b;
        uint8_t c;
} __packed;

struct aaa2 {
        uint8_t x;
        struct aaa y;
};

struct bbb {
        uint16_t a;
        uint8_t b;
        uint8_t c;
};

struct bbb2 {
        uint8_t x;
        struct bbb y;
};

int
main()
{
        printf("sizeof(aaa2) = %zu, alignof(aaa2) = %zu\n",
sizeof(struct aaa2), alignof(struct aaa2));
        printf("sizeof(bbb2) = %zu, alignof(bbb2) = %zu\n",
sizeof(struct bbb2), alignof(struct bbb2));
}


$ ./a.out
sizeof(aaa) = 4, alignof(aaa) = 1
sizeof(bbb) = 4, alignof(bbb) = 2
sizeof(aaa2) = 5, alignof(aaa2) = 1
sizeof(bbb2) = 6, alignof(bbb2) = 2
Please try t access these members of the structs with memcpy(3),
memcmp(3), memset(3) etc rather than directly with * or [].

Before I saw your commit, I wanted to ask to revert the following changes:

icmp6: Remove __packed attribute from icmp6 structures

https://github.com/NetBSD/src/commit/427831ba4bdf388aecf3a378de8faf3a4d44a462

ip6: Remove __packed attribute from ip6 structures

https://github.com/NetBSD/src/commit/e82879afd70b0e801e6ee53bd14c27be3dd1738f

The fallout can be subtle and hard to debug. Once, I removed __packed
from one innocent networking structure myself, qemu networking stopped
working at all.