Hi Suzuki,
On Tue, Aug 09, 2022 at 01:48:16PM +0100, Suzuki K Poulose wrote:
> When a script is specified for a guest nic setup, we fork() and execl()s
> the script when it is time to execute the script. However this is not
> optimal, given we are running a VM. The fork() will trigger marking the
> entire page-table of the current process as CoW, which will trigger
> unmapping the entire stage2 page tables from the guest. Anyway, the
This looks correct to me, virtio_notify_status() is called when the guest
writes to the status register, which in turn can end up calling
virtio_net_exec_script(). This happens when the guest is up and running,
when stage 2 has been created and populated.
> child process will exec the script as soon as we fork(), making all
> these mm operations moot. Also, this operation could be problematic
> for confidential compute VMs, where it may be expensive (and sometimes
> destructive) to make changes to the stage2 page tables.
>
> So, instead we could use vfork() and avoid the CoW and unmap of the stage2.
>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> ---
> virtio/net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/virtio/net.c b/virtio/net.c
> index c4e302bd..a5e0cea5 100644
> --- a/virtio/net.c
> +++ b/virtio/net.c
> @@ -295,7 +295,7 @@ static int virtio_net_exec_script(const char* script, const char *tap_name)
> pid_t pid;
> int status;
>
> - pid = fork();
> + pid = vfork();
> if (pid == 0) {
> execl(script, script, tap_name, NULL);
This matches the man 2 page for vfork, which basically says that vfork can
be used only if the child immediately issues one of the exec family of
functions. Which is what is happening here.
The man page for vfork also says this:
"vfork() differs from fork(2) in that the calling thread is suspended until
the child terminates (either normally, by calling _exit(2), or abnormally,
after delivery of a fatal signal), or it makes a call to execve(2)".
waitpid (which is invoked in the parent in the else branch) waits until the
child has changed state, which is defined in man 2 waitpid as:
"A state change is considered to be: the child terminated; the child was
stopped by a signal; or the child was resumed by a signal."
It doesn't mention anything about waitpid returning after the child make a
call to execve, so I guess it's correct to keep the call to waitpid in the
parent:
Reviewed-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Thanks,
Alex
> _exit(1);
> --
> 2.37.1
>
