Network cleanup fails when VMM process is already dead

[goyalpalak18]

Description

There is a resource leak where network interfaces (TAP devices) and Traffic Control (TC) rules are left behind if a VMM crashes or is killed externally.

Issue

The Kill() function currently returns an error immediately if vmm.Stop() fails (e.g. if the process is already dead/ESRCH). This skips the critical network.Cleanup() step.

Impact

This causes tap devices (e.g., tap0_urunc) to pile up in the namespace during crash loops, potentially exhausting resources.

[cmainas]

Hello @goyalpalak18 ,

thank you for opening this issue. Have you encountered such case? The kernel will clean up the network namespace (and all interfaces) as soon as all process in the namespace exit.

[goyalpalak18]

Hey @cmainas, valid point regarding the kernel cleanup. I actually ran into this in a real setup, which is why I dug into it.

The catch here is the CNI flow. Since the container runtime creates and manages the network namespace, the namespace itself stays alive (held open by the sandbox) even if the VMM process dies. Because the namespace persists, the kernel doesn't automatically cleanup the tap0_urunc device inside it.

Here is exactly how I reproduced it:

1. Setup: Standard CNI (TAP device inside the pod's netns).
2. Action: Manually killed the VMM with kill -9 <pid>.
3. Check: Ran urunc kill <container_id>.

Before the fix:
The command failed immediately because vmm.Stop() returned ESRCH (process already dead), causing the function to bail out before reaching the cleanup logic.
Result: ip link show confirmed tap0_urunc was still sitting there.

After the fix:
urunc kill handles the ESRCH gracefully (treats it as "already stopped") and proceeds to run network.Cleanup().
Result: The TAP device is correctly removed.

I also tested a crash-loop scenario. Without this fix, I saw tc filters piling up after a few restarts. With the fix, it stayed clean.

[cmainas]

Hello @goyalpalak18 ,

thank you for the instructions. Unfortunately, I was not able to replicate it.
After running a urunc container, I checked the namespaces and found the following ones:

4026532593 mnt         1 27818 root             /usr/bin/qemu-system-x86_64 -m 536M -L /usr/sha
4026532594 uts         1 27818 root             /usr/bin/qemu-system-x86_64 -m 536M -L /usr/sha
4026532595 ipc         1 27818 root             /usr/bin/qemu-system-x86_64 -m 536M -L /usr/sha
4026532596 pid         1 27818 root             /usr/bin/qemu-system-x86_64 -m 536M -L /usr/sha
4026532597 cgroup      1 27818 root             /usr/bin/qemu-system-x86_64 -m 536M -L /usr/sha
4026532598 net         1 27818 root             /usr/bin/qemu-system-x86_64

Then I killed the qemu process and all namespaces were gone, so the tap device can not be present anymore.

[goyalpalak18]

Hey @cmainas thanks for testing it out! I think the difference comes down to the sandbox setup.

In a CNI/K8s environment, the sandbox process holds the network namespace open. So even if QEMU dies, the namespace persists and the TAP device is left behind.

In a standalone setup where QEMU is the only process in the namespace, you're right—the kernel cleans everything up on exit. This fix is mainly targeting the orchestrated case where the namespace outlives the VMM.

[cmainas]

Hey @cmainas thanks for testing it out! I think the difference comes down to the sandbox setup.

In a CNI/K8s environment, the sandbox process holds the network namespace open. So even if QEMU dies, the namespace persists and the TAP device is left behind.

In a standalone setup where QEMU is the only process in the namespace, you're right—the kernel cleans everything up on exit. This fix is mainly targeting the orchestrated case where the namespace outlives the VMM.

Hello @goyalpalak18 I am not sure of what you mean sandbox. In a k8s environment the network namespace belongs to the pod (pause container) and not in the container. Hence, if the application in the container gets killed the network namespace remains since the pod is still there.

In that scenario, we thought it would be better idea to reuse the tap device later on (instead of recreating it), but this is unfortunately not working properly (See #406). So one approach would be to merge the work here #407. but in the deletion path as you do with your PR. However, when we receive a kill command we need to ensure that the monitor process dies, because otherwise the network cleanup will fail; we can not remove a tap device which is sued form a process.

[goyalpalak18]

@cmainas got it, makes sense. so inside Kill() after vmm.Stop(), i’ll swap the hardcoded network cleanup for the cleanupOrphanTaps() logic from #407, utilizing that NO-CARRIER check as a safety net to prevent yanking live taps. just a couple checks before i start though—should cleanupOrphanTaps fully replace the existing cleanup call or just sit alongside it? also, regarding #407, do you want that to handle just the setup path (and i take care of deletion here) or does it make more sense to keep the setup/teardown logic together in one place to avoid sync issues?

[cmainas]

Hello @goyalpalak18 ,

in https://github.com/urunc-dev/urunc/blob/main/pkg/unikontainers/hypervisors/utils.go#L73 we should keep the changes that you suggested in your PR but in https://github.com/urunc-dev/urunc/blob/main/pkg/unikontainers/unikontainers.go#L583 we should not change the return err if Stop fails.

Regarding the network cleanup, we should integrate the changes from #407 in the Cleanup function, so identify all tap devices that need to be cleaned and clean them.

Make sure to add @sidneychang in the sign off, since we will use the code from #407.

[goyalpalak18]

@cmainas Opened PR #476 for this.

I integrated the scanning logic from @sidneychang and added the check for ESRCH so it doesn't fail if the process is already dead. Ready for review!