Add explicit vsock and TSI configuration API #483
Conversation
|
@slp After containers/libkrunfw#106 we don't enable UNIX hijacking in libkrun implicitly by default. Should I restore that behavior and enable it? |
mtjhrc
force-pushed
the
explicit-vsock-api
branch
from
1ca7f4c to
46bd3e2
Compare
|
@mtjhrc this is looking good, what's missing to be ready for review? |
mtjhrc
force-pushed
the
explicit-vsock-api
branch
2 times, most recently
from
a9a286d to
36827ac
Compare
mtjhrc
requested review from
MatiasVara,
jakecorrenti,
slp and
tylerfanelli
as code owners
Refactor the vsock device to explicitly configure usage of INET and UNIX proxies. This also removes the heuristic that enabled unix socket hijacking if "/" is the rootfs. This got broken anyways with the new libkrunfw, which now also requires a kernel argument for hijacking unix sockets. Add new public API functions for explicit vsock control: - krun_disable_implicit_vsock(): Disable the implicit vsock device - krun_add_vsock(): Add vsock with explicit TSI feature flags krun_add_vsock() requires krun_disable_implicit_vsock() to be called first, otherwise an error is returned - we only support 1 vsock device. Add a check in krun_set_port_map() and krun_add_vsock_port() to ensure vsock is enabled. Signed-off-by: Matej Hrica <[email protected]>
mtjhrc
force-pushed
the
explicit-vsock-api
branch
from
36827ac to
6a4dbc6
Compare
|
Note, this PR also removes the heuristic that enabled unix socket hijacking if "/" is the rootfs. This got broken anyways with the new libkrunfw, which now requires a kernel argument for hijacking unix sockets. |
| * | ||
| * By default, libkrun creates a vsock device implicitly with TSI hijacking | ||
| * enabled based on heuristics. Calling this function overrides the implicit | ||
| * behavior and explicitly configures the vsock device. |
There was a problem hiding this comment.
The comment above seems to point that calling this function will automatically disable the implicit vsock, but that's not what happens. Calling this one without calling krun_disable_implicit_vsock() first will lead to [libcrun:krun]: could not add vsock configuration: File exists.
I think we need to either have this function also disable the implicit vsock, or clarify that krun_disable_implicit_vsock() needs to be called first.
|
Tested with crun and found no regressions. Also, enabling TSI_UNIX from crun does make unix sockets to work, as expected. I've only have a minor complain explained in a comment, otherwise LGTM. |
slp
left a comment
slp
left a comment
There was a problem hiding this comment.
Please take a look at the comment about disabling the implicit vsock.
2 participants
Not sure if this is the API we want to go with , we should have some way to control TSI and vsock.