Page MenuHomeFreeBSD

Add Bhyve TCP raw socket on console
AbandonedPublic

Authored by aokblast on Jan 19 2024, 6:19 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 9:00 AM
Unknown Object (File)
Thu, Dec 12, 11:24 AM
Unknown Object (File)
Sun, Dec 1, 11:31 AM
Unknown Object (File)
Nov 24 2024, 7:37 PM
Unknown Object (File)
Nov 24 2024, 2:39 PM
Unknown Object (File)
Nov 3 2024, 5:56 AM
Unknown Object (File)
Nov 1 2024, 12:43 AM
Unknown Object (File)
Oct 18 2024, 4:42 PM

Details

Reviewers
corvink
manu
Group Reviewers
bhyve
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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 55819
Build 52708: arc lint + arc unit

Event Timeline

aokblast retitled this revision from Add Bhyve TCP raw socket to WIP: Add Bhyve TCP raw socket.Jan 19 2024, 6:23 PM
aokblast edited the summary of this revision. (Show Details)
aokblast retitled this revision from WIP: Add Bhyve TCP raw socket to WIP: Add Bhyve TCP raw socket on console.
aokblast added a subscriber: lwhsu.

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

735

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

744

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

776

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

791
793
aokblast marked 5 inline comments as done.
aokblast edited the summary of this revision. (Show Details)

Add lock to sc and imporve the error logging message

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
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?

735

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 marked an inline comment as not done.Feb 1 2024, 6:23 PM

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

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.

175

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

742

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.

761–765
usr.sbin/bhyve/uart_emul.c
175

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?

usr.sbin/bhyve/uart_emul.c
175

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

Use pointer instead of setting errno in userspace

aokblast retitled this revision from WIP: Add Bhyve TCP raw socket on console to Add Bhyve TCP raw socket on console.Feb 2 2024, 10:27 AM
aokblast edited the summary of this revision. (Show Details)
usr.sbin/bhyve/uart_emul.c
175

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.

usr.sbin/bhyve/uart_emul.c
175

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.

This comment was removed by aokblast.

Roll back to correct version

usr.sbin/bhyve/uart_emul.c
168

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

761–765

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

This revision is now accepted and ready to land.Feb 6 2024, 7:52 AM

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

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.

This revision now requires review to proceed.Mar 2 2024, 5:12 PM
  • Add manual page and use uniform interface for tcp

Abandon because I submit the new one.