Add Bhyve TCP raw socket on console

[aokblast]

Summary

This patch add raw tcp connection ability on bhyve and has been tested on my own machine.
I don't handle disconnection currently because there is no good way to detect this without largely changing the code right now.

An example about this is as following:

bhyve -c 2 -m 4G -w -H \
                                       -s 0,hostbridge \
                                       -s 1,virtio-blk,./FreeBSD-14.0-STABLE-amd64-20240118-cef433d3fb38-266364.raw \
                                       -s 30,xhci,tablet \
                                       -s 31,lpc -l com1,tcp=127.0.0.1:8085 \
                                       -l bootrom,/usr/local/share/uefi-firmware/BHYVE_UEFI.fd \
                                        uefivm

Then we can use nc by:

netcat 127.0.0.1 8085

[corvink]

You should add a manpage entry.

usr.sbin/bhyve/uart_emul.c

30–31

Afaik, sys/types.h should always be the first header to include.

653

stray newline change

737

So, you can only connect once to the vm? If the connection gets lost, you won't be able to reconnect?

746

Shouldn't we lock sc->mtx before writing those values?

778

Maybe be more verbose and log some kind of error message. This makes it easier to solve issue like when reusing an address.

793

795

[aokblast]

Add lock to sc and imporve the error logging message

[corvink]

usr.sbin/bhyve/uart_emul.c

31–33

The sort order looks still wrong according to style (https://man.freebsd.org/cgi/man.cgi?style(9)). sys includes should be first and the rest should be ordered alphabetical.

Note that FreeBSD ships a .clang-format (https://github.com/freebsd/freebsd-src/blob/120ceebab5d4947dfc80d7492effa034a925d43e/.clang-format. So, you can sort this correctly by using the clang-format tool (https://clang.llvm.org/docs/ClangFormat.html)

clang-format -i --lines=30:40 --style=file uart_emul.c

[aokblast]

usr.sbin/bhyve/uart_emul.c

31–33

I try to use clang-format but I think neither the original including order nor the patch I have submitted follows the style(9). Should I overwrite the original including order in this circumstance?

737

Yes. In my knowledge base, the only way to detect the disconnection in TCP is by the return size for the data or just setting the timer.
I don't have any good idea to achieve it without largely change the code in codebase because the mevent don't support timeout.

[aokblast]

Add reconnection mechanism

[aokblast]

I figure out the disconnection part. Hope everyone can give it a review. Thanks!

[aokblast]

Fix indentation

[corvink]

usr.sbin/bhyve/uart_emul.c

31–33

You can change the original order but that should be posted as additional commit. Try to follow style(9) as much as possible without changing the original order.

176

I'm not sure but I don't think that we should set errno in user space.

744

You're lucky that bhyve has a bad habit and keeps asserts in release code. Just use an if statement to avoid that this call is skipped accidentally when bhyve stops keeping asserts in release code.

763–767

[aokblast]

usr.sbin/bhyve/uart_emul.c

176

Sorry but do you have any good idea on this? Because as I know the only way to detect disconnection is rely on read size in nonblocking mode. Can I just pass the returned value as pointer in parameter and just return the return value of read?

[corvink]

usr.sbin/bhyve/uart_emul.c

176

Why can't you just check if the function returned -1?

[aokblast]

Use pointer instead of setting errno in userspace

[aokblast]

usr.sbin/bhyve/uart_emul.c

176

As my experiment, the way to detect disconnection is when return size is zero. You can check the code in the link. This code segment is aim at handling the disconnection of TCP like what we do. So we have to let our ttyread to be possible to distinguish between 0 and -1 return case.

[aokblast]

usr.sbin/bhyve/uart_emul.c

176

BTW, the function "pci_vtcon_port_add" in the file of the link also change the errno. If this is allowed, I think the original code is more elegant. I don't know if it is another bad habit. This is the first time I contribute on bhyve.

[aokblast]

This comment was removed by aokblast.

[aokblast]

Roll back to correct version

[corvink]

usr.sbin/bhyve/uart_emul.c

169

You're reading a char, so you should return a char.

763–767

You can ignore this comment as this function is always called with sc->mtx locked.

[corvink]

You have to add an entry to the manpage before we can commit it.

[aokblast]

I understand, but I'm awaiting testing from the OpenStack team to ensure everything is functioning properly. Additionally, I intend to incorporate IPv6 support, so I've chosen to clean up the commit after receiving feedback from the OpenStack team.

[aokblast]

• Add ipv6 support

[aokblast]

Add ipv6 support

[aokblast]

• Add manual page and use uniform interface for tcp

[aokblast]

Tidy up commit

[aokblast]

Abandon because I submit the new one.

[lwhsu]

In D43514#1029129, @aokblast wrote:

Abandon because I submit the new one.

https://reviews.freebsd.org/D45120