Skip to content

fix[vAccel]: optimized hash function in idToGuestCID to reduce collisions - #895

Open
Cicada1107 wants to merge 2 commits into
urunc-dev:mainfrom
Cicada1107:idToGuestCID_fix
Open

fix[vAccel]: optimized hash function in idToGuestCID to reduce collisions#895
Cicada1107 wants to merge 2 commits into
urunc-dev:mainfrom
Cicada1107:idToGuestCID_fix

Conversation

@Cicada1107

Copy link
Copy Markdown

Description

Currently the vsock CID for the guest unikontainers spawned with vAccel are restricted to the range 3 to 99, leaving the actually permitted ceiling space of 2^32 - 1 unused, thus introducing redundancy. This is a very small range and thus the probability of a collision is very high. Moreover, the hash function used (ASCII Summation + Modulo Arithmetic) does not account for anagrams.

This PR optimizes this redundancy by replacing with the standard hash/fnc function for hashing which increases the range to the full capacity of 3 to 2^32 - 1, thus reducing the probability of a collision near zero (not zero however). Moreover, due to the hash/fnv function, the anagram problem is solved, as, unlike ASCII addition, the hash/fnv function depends on the direction of traversal as well; thus, Anagramic addresses would result in completely different hashes and thus collision would be avoided.

Also, this PR modifies the vAcccel_test.go file to account for the new 32 bit addresses.

Related issues

How was this tested?

  • Automated Unit Tests: Updated TestIdToGuestCID in pkg/unikontainers/vaccel_test.go to assert against the new 32-bit hashes. Also added a specific test using the exact anagram IDs ("abcdef0123456789" * 4 vs "9876543210fedcba" * 4). All tests passed.
  • Manual E2E Testing:
  1. Executed a vaccel-enabled container image, explicitly passing the required annotations to trigger vsock initialization:
sudo docker run --runtime io.containerd.urunc.v2 --rm -it \
  --annotation com.urunc.unikernel.vAccel="vsock" \
  --annotation com.urunc.unikernel.RPCAddress="vsock://2:2049" \
  harbor.nbfc.io/nubificus/ubuntu-vaccel-urunc-qemu:x86_64

(Debug level set to 4 in env variables)

  1. Verified that the QEMU CLI execution accurately reflected the high-entropy CID (32 bit uint): -device vhost-vsock-pci, id=vhost-vsock-pci0,guest-cid=688968749.

LLM usage

Gemini 3.1 Pro

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint).
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
  • If LLMs were used: I have read the llm policy.

Signed-off-by: Arijit Dubey <arijitdubey2018@gmail.com>
Signed-off-by: Arijit Dubey <arijitdubey2018@gmail.com>
@netlify

netlify Bot commented Aug 3, 2026

Copy link
Copy Markdown

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit 2d81418
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a70e1c0a249db000874b60e

@Nachiket-Roy

Nachiket-Roy commented Aug 3, 2026

Copy link
Copy Markdown

This still doesn't eliminate the possibility of collisions. I think we'd need some sort of map/table. I initially thought of uuid but that's 128bits

@Cicada1107

Cicada1107 commented Aug 4, 2026

Copy link
Copy Markdown
Author

Hello @Nachiket-Roy , yes, this doesn't eliminate the possibility of collisions completely. However, as was discussed in the community call, the current approach is sub optimal for no apparent reason and leaves a lot of unused space on the table. As discussed, this PR's role is to utilize that space and decrease the collision rate significantly, for the time being, for as long as we don't have a Deterministic non-collision solution, and to replace the currently redundant algorithm for now.

As for non-collision solutions, any approach that I can think of that would completely eliminate collisions, would have to rely on Kernel Level probing, which is probably not a good choice for urunc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

idToGuestCID generates frequent vsock CID collisions due to limited output space

2 participants