In D54873#1254599, @markj wrote:

If someone comes along wanting to add snapshot support for other arches, they'll have to revert this patch.

IMO it would be better to just have a check in the bhyve makefile:

.if ${MK_BHYVE_SNAPSHOT} != "no" && ${MACHINE_CPUARCH} != "amd64"
.error bhyve snapshot support is only implemented on amd64.
.endif

Couple of comments/questions:

In D54720#1251266, @siva wrote:

Thanks! Although I don't have a ports commit bit yet, so you'll have to do it :)

Thanks, feel free to merge!

Looks good, a couple of minor questions.

Fix ident.

Thanks, comments resolved.

get_config_value_node(nvl, "mtu"); -> get_config_value_node(config, "mtu");

s/SLIRP_MTU/DEFAULT_MTU/.

Drop unnecessary include.

Pass MTU to slirp-helper as an integer.

In D54133#1246073, @markj wrote:

In D54133#1245938, @novel wrote:

In D54133#1245871, @markj wrote:

We shouldn't modify the config tree in net_backend_slirp.c. Is there something else we can do?

Do you mean the nvlist_clone() part for setting vmname? If so, could that be passed as a part of argv to bhyve-slirp-helper?

Oh, I missed that we do that. In that case, can we please unconditionally pass along the MTU to the helper process so that it doesn't need to define a default?

Drop the DEFAULT_MTU constant from the SLIRP code:

In D54133#1245871, @markj wrote:

We shouldn't modify the config tree in net_backend_slirp.c. Is there something else we can do?

• SLIRP_MTU -> DEFAULT_MTU
• unsigned long -> size_t for mtu values
• move buf allocation before mevent_add_disabled(), update "err" cleanup routines to free() it

Relatively simple way to trigger this error is to run kldunload nmdm and then run bhyve -c 1 -o bootrom=/usr/local/share/u-boot/u-boot-bhyve-arm64/u-boot.bin -o console=/dev/nmdmFoobarA test.

Rebased on top of commit daef625cf884, works fine now.

In D54133#1237392, @novel wrote:

In D54133#1237195, @markj wrote:

In D54133#1237097, @novel wrote:

In D54133#1237026, @markj wrote:

In D54133#1237024, @novel wrote:

In D54133#1237001, @markj wrote:

Actually, the slirp backend has a fixed MTU. We should either update it to handle a configured size, or make it an error to specify the MTU.

Thanks, I'll try to see if I could add MTU configuration for SLIRP.

Cool! Search for uses of SLIRP_MTU, they all need to be updated.

I have updated this revision with these changes. It works fine for me with MTU values from 1476 to 4096 (didn't test larger), but for lower values it fails with:

Assertion failed: ((size_t)n <= priv->mtu), function slirp_recv, file /usr/home/novel/code/freebsd-src/usr.sbin/bhyve/net_backend_slirp.c, line 238.

I'll debug it further, but now I'm not sure: currently I do not validate the MTU value range in the SLIRP backend assuming it's already validated on the device side (e.g. in pci_virtio_net.c). Now I wonder if the backend might have stricter requirements for MTU than the device, should the backend implement its own validation on top of the device side validation?

Do you have commit 69f61cee2efb1eec0640ca7de9b2d51599569a5d commit applied to the host kernel?

Yes, I have commit 69f61cee2efb1eec0640ca7de9b2d51599569a5d (and also commit 82d8a5029a80a77166dca098b8fedb10d84e4e38 which appears to be a follow up of the former) included in the host kernel.

In the meantime, I was giving it some more testing and noticed another issues:

While running wget https://download.freebsd.org/releases/ISO-IMAGES/15.0/FreeBSD-15.0-RELEASE-arm64-aarch64-dvd1.iso -O /dev/null, somewhere half way it fails with:

`#1 0x000000080114c934 in raise (s=s@entry=6) at /usr/home/novel/code/freebsd-src/lib/libc/gen/raise.c:48
#2 0x00000008011fe7a9 in abort () at /usr/home/novel/code/freebsd-src/lib/libc/stdlib/abort.c:61
#3 0x000000080112f521 in assert (func=<optimized out>, file=<optimized out>, line=line@entry=115, failedexpr=<optimized out>) at /usr/home/novel/code/freebsd-src/lib/libc/gen/assert.c:47
#4 0x000000000102500b in slirp_cb_send_packet (buf=<optimized out>, len=1514, param=<optimized out>) at /usr/home/novel/code/freebsd-src/usr.sbin/bhyve/slirp/slirp-helper.c:115
#5 0x000000080141ad02 in ?? () from /usr/local/lib/libslirp.so.0
#6 0x0000000801412f7d in ?? () from /usr/local/lib/libslirp.so.0
#7 0x0000000801416366 in ?? () from /usr/local/lib/libslirp.so.0
#8 0x0000000801421197 in ?? () from /usr/local/lib/libslirp.so.0
#9 0x000000080141ef1a in ?? () from /usr/local/lib/libslirp.so.0
#10 0x0000000001024edd in slirp_pollfd_loop (priv=0x7fffffffe7e0) at /usr/home/novel/code/freebsd-src/usr.sbin/bhyve/slirp/slirp-helper.c:303
#11 main (argc=<optimized out>, argv=<optimized out>) at /usr/home/novel/code/freebsd-src/usr.sbin/bhyve/slirp/slirp-helper.c:580
(gdb) fr 4
#4 0x000000000102500b in slirp_cb_send_packet (buf=<optimized out>, len=1514, param=<optimized out>) at /usr/home/novel/code/freebsd-src/usr.sbin/bhyve/slirp/slirp-helper.c:115
115 assert((size_t)n == len);
(gdb) p n
$1 = 0
(gdb) `

That's with a fairly large mtu=4092. So it's probably a bug in my code, not a kernel issue.

In D54133#1237195, @markj wrote:

In D54133#1237097, @novel wrote:

In D54133#1237026, @markj wrote:

In D54133#1237024, @novel wrote:

In D54133#1237001, @markj wrote:

Actually, the slirp backend has a fixed MTU. We should either update it to handle a configured size, or make it an error to specify the MTU.

Thanks, I'll try to see if I could add MTU configuration for SLIRP.

Cool! Search for uses of SLIRP_MTU, they all need to be updated.

I have updated this revision with these changes. It works fine for me with MTU values from 1476 to 4096 (didn't test larger), but for lower values it fails with:

Assertion failed: ((size_t)n <= priv->mtu), function slirp_recv, file /usr/home/novel/code/freebsd-src/usr.sbin/bhyve/net_backend_slirp.c, line 238.

I'll debug it further, but now I'm not sure: currently I do not validate the MTU value range in the SLIRP backend assuming it's already validated on the device side (e.g. in pci_virtio_net.c). Now I wonder if the backend might have stricter requirements for MTU than the device, should the backend implement its own validation on top of the device side validation?

Do you have commit 69f61cee2efb1eec0640ca7de9b2d51599569a5d commit applied to the host kernel?

In D54133#1237246, @quentin.thebault_defenso.fr wrote:

An ng_device(4) is not an interface, so it does not have a MAC address. The way I see it, it is just a wire. If you think about a tap(4), it's like an ng_eiface(4) linked to a ng_device. But the MAC address belongs to the ng_eiface end, not the ng_device one.

For the MTU it might be a bit different because the ng_device could maybe impose a limit on to the size it could transport, but in any case there is currently no code allowing the user to configure it. The ioctls are not implemented.

• bhyve.8: Drop ngdN related changes
• net_backend_slirp, slirp-helper: move buf to struct slirp_priv
• net_backend_slirp: remove assert from slirp_peek_recvlen()
• slirp-helper: update assert in slirp_cb_send_packet()

In D54133#1237026, @markj wrote:

In D54133#1237024, @novel wrote:

In D54133#1237001, @markj wrote:

Actually, the slirp backend has a fixed MTU. We should either update it to handle a configured size, or make it an error to specify the MTU.

Thanks, I'll try to see if I could add MTU configuration for SLIRP.

Cool! Search for uses of SLIRP_MTU, they all need to be updated.

Update the SLIRP backend to respect MTU configuration.

In D54133#1237001, @markj wrote:

Actually, the slirp backend has a fixed MTU. We should either update it to handle a configured size, or make it an error to specify the MTU.

I was looking into getting virtio-input working for Linux guests which apparently do not support legacy virtio input devices.