vnet: Ensure space allocated by vnet_data_alloc() is properly aligned

[zlei]

Summary

Some 32 bit architectures, e.g. armv7, want 8-byte alignment while
atomically accessing a 64bit int typed memory. Make the start of vnet
modspace and the space allocated by vnet_data_alloc() properly aligned
to avoid misaligned access.

PR: 265639
Diagnosed by: markj
Co-authored-by: mjg
Co-authored-by: jhb
MFC after: 1 week

Test Plan

Running the test on QEMU with arm7 image.

Build a testing module align.ko,

% cat Makefile
PACKAGE=examples
SRCS	= align.c
KMOD	= align

.include <bsd.kmod.mk>

% cat align.c

#include <sys/types.h>
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/module.h>
#include <sys/sysctl.h>
#include <sys/kernel.h>
#include <net/vnet.h>

static uint64_t counter;
VNET_DEFINE_STATIC(uint64_t, vnetcounter);
VNET_DEFINE_STATIC(uint64_t, atomicvnetcounter);

SYSCTL_U64(_sysctl, OID_AUTO, counter, CTLFLAG_RW, &counter, 0,
    "Test read / write 64bit counter on arm32");

SYSCTL_U64(_sysctl, OID_AUTO, vnetcounter, CTLFLAG_VNET | CTLFLAG_RW,
    &VNET_NAME(vnetcounter), 0,
    "Test read / write 64bit vnet counter on arm32");

static int
sysctl_handle_64_atomic(SYSCTL_HANDLER_ARGS)
{
	int error = 0;
	uint64_t tmp;

	/*
	 * Attempt to get a coherent snapshot by making a copy of the data.
	 */
	tmp = atomic_load_64((uint64_t *)arg1);
	error = SYSCTL_OUT(req, &tmp, sizeof(uint64_t));

	if (error || !req->newptr)
		return (error);

	error = SYSCTL_IN(req, &tmp, sizeof(uint64_t));
	if (error == 0)
		atomic_store_64((uint64_t *)arg1, tmp);

	return (error);
}

SYSCTL_PROC(_sysctl, OID_AUTO, atomicvnetcounter,
    CTLTYPE_U64 | CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(atomicvnetcounter), 0,
    sysctl_handle_64_atomic, "QU",
    "Test read / write 64bit vnet counter on arm32");

static void
vnet_print_address(void *unused __unused)
{
	printf("address of vnetcounter is:\t%p\n", VNET_PTR(vnetcounter));
	printf("address of atomicvnetcounter is:\t%p\n", VNET_PTR(atomicvnetcounter));
}
VNET_SYSINIT(vnet_print_address, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY,
    vnet_print_address, NULL);


static int
module_load(module_t mod, int cmd, void *arg)
{
	int error;

	error = 0;
	switch (cmd) {
	case MOD_LOAD:
		printf("address of counter is:\t%p\n", &counter);
	case MOD_UNLOAD:
		break;
	default:
		error = EOPNOTSUPP;
		break;
	}
	return (error);
}

static moduledata_t mod_data = {
	"align",
	module_load,
	0
};

DECLARE_MODULE(align, mod_data, SI_SUB_EXEC, SI_ORDER_ANY);

Load the if_epair.ko module, to occupy 4 bytes of modspace and make the later allocation misaligned, and then load the testing module align.ko,

% elfdump -c /boot/kernel/if_epair.ko | grep -A5 set_vnet | grep sh_size
	sh_size: 4
# kldload if_epair
# kldload ./align.ko

Verify the address of vnet variables are properly aligned, and test with atomic access via sysctl.

address of counter is:	0xe1f8bb58
address of vnetcounter is:	0xc6ee7198
address of atomicvnetcounter is:	0xc6ee71a0

# sysctl sysctl.atomicvnetcounter=1
sysctl.atomicvnetcounter: 0 -> 1

[jhb]

Nice testing harness!

sys/net/vnet.c

184

Centralizing this is a nice cleanup.

[markj]

sys/net/vnet.c

414

Right before the return, I would KASSERT((uintptr_t)s & (VNET_DATAALIGN - 1) == 0, ("unaligned vnet alloc %p", s)); or similar.

[mjg]

I did not co-autor the change, so please drop that tag.

Perhaps it would make sense to dedup roundup2(size, VNET_DATAALIGN) itself instead of the specific alignment (maybe something like vnet_data_aligned_size(size)?).

[zlei]

In D55560#1271351, @mjg wrote:

I did not co-autor the change, so please drop that tag.

OK.

Perhaps it would make sense to dedup (size, VNET_DATAALIGN) itself instead of the specific alignment (maybe something like vnet_data_aligned_size(size)?).

I would like to do that if there're at least three usages. Currently vnet_data_alloc() and vnet_data_free() are the only consumers, and probably there will be no new consumers.
So I think the two roundup2(size, VNET_DATAALIGN) should make no burdens ( right now, and in near future ) ?