fix(vsock): restore listener after accept

[archevel]

After accepting a connection, move the accepted socket to an ephemeral port and reset the listener entry to Listen state. Track the listen port separately in Socket so subsequent accept calls always find the listener entry regardless of how many connections have been accepted.

Fixes #2433

[archevel]

As a reference I made this pull request as well with a regression test:
hermit-os/hermit-rs#996

[github-actions]

Benchmark Results

Details

Benchmark	Current: 41c96c2	Previous: f83a235	Performance Ratio
startup_benchmark Build Time	79.62 s	77.08 s	1.03 ❗
startup_benchmark File Size	0.73 MB	0.73 MB	1.00 ❗
Startup Time - 1 core	0.74 s (±0.02 s)	0.73 s (±0.02 s)	1.01
Startup Time - 2 cores	0.74 s (±0.02 s)	0.75 s (±0.02 s)	0.98
Startup Time - 4 cores	0.75 s (±0.02 s)	0.76 s (±0.02 s)	0.99
multithreaded_benchmark Build Time	79.91 s	80.66 s	0.99 ❗
multithreaded_benchmark File Size	0.86 MB	0.86 MB	1.00 ❗
Multithreaded Pi Efficiency - 2 Threads	88.35 % (±7.90 %)	89.44 % (±5.91 %)	0.99
Multithreaded Pi Efficiency - 4 Threads	43.45 % (±2.46 %)	44.06 % (±2.65 %)	0.99
Multithreaded Pi Efficiency - 8 Threads	25.26 % (±1.78 %)	25.56 % (±1.89 %)	0.99
micro_benchmarks Build Time	78.55 s	87.20 s	0.90 ❗
micro_benchmarks File Size	0.87 MB	0.87 MB	1.00 ❗
Scheduling time - 1 thread	66.63 ticks (±3.03 ticks)	67.04 ticks (±1.91 ticks)	0.99
Scheduling time - 2 threads	37.61 ticks (±6.00 ticks)	39.33 ticks (±3.42 ticks)	0.96
Micro - Time for syscall (getpid)	4.29 ticks (±0.58 ticks)	3.43 ticks (±0.24 ticks)	1.25 ❗
Memcpy speed - (built_in) block size 4096	81635.89 MByte/s (±56414.08 MByte/s)	84527.91 MByte/s (±58322.62 MByte/s)	0.97
Memcpy speed - (built_in) block size 1048576	30365.44 MByte/s (±24468.19 MByte/s)	30845.87 MByte/s (±25037.16 MByte/s)	0.98
Memcpy speed - (built_in) block size 16777216	27687.52 MByte/s (±22934.08 MByte/s)	27492.52 MByte/s (±22708.20 MByte/s)	1.01
Memset speed - (built_in) block size 4096	81328.35 MByte/s (±56215.85 MByte/s)	84666.80 MByte/s (±58421.97 MByte/s)	0.96
Memset speed - (built_in) block size 1048576	31113.74 MByte/s (±24916.97 MByte/s)	31593.14 MByte/s (±25470.12 MByte/s)	0.98
Memset speed - (built_in) block size 16777216	28474.41 MByte/s (±23425.54 MByte/s)	28269.66 MByte/s (±23193.64 MByte/s)	1.01
Memcpy speed - (rust) block size 4096	75049.34 MByte/s (±52487.52 MByte/s)	74722.10 MByte/s (±52209.99 MByte/s)	1.00
Memcpy speed - (rust) block size 1048576	30245.07 MByte/s (±24516.30 MByte/s)	30824.11 MByte/s (±25054.28 MByte/s)	0.98
Memcpy speed - (rust) block size 16777216	27137.33 MByte/s (±22399.30 MByte/s)	27597.67 MByte/s (±22766.03 MByte/s)	0.98
Memset speed - (rust) block size 4096	75508.32 MByte/s (±52826.33 MByte/s)	74865.22 MByte/s (±52302.23 MByte/s)	1.01
Memset speed - (rust) block size 1048576	31005.85 MByte/s (±24967.81 MByte/s)	31565.59 MByte/s (±25484.76 MByte/s)	0.98
Memset speed - (rust) block size 16777216	27917.80 MByte/s (±22897.18 MByte/s)	28374.92 MByte/s (±23250.83 MByte/s)	0.98
alloc_benchmarks Build Time	72.98 s	81.01 s	0.90 ❗
alloc_benchmarks File Size	0.81 MB	0.81 MB	1.00 ❗
Allocations - Allocation success	91.32 %	91.32 %	1
Allocations - Deallocation success	100.00 %	100.00 %	1
Allocations - Pre-fail Allocations	61.46 %	61.46 %	1
Allocations - Average Allocation time	3295.34 Ticks (±989.58 Ticks)	6133.98 Ticks (±129.62 Ticks)	0.54 ❗
Allocations - Average Allocation time (no fail)	4267.24 Ticks (±886.44 Ticks)	6784.80 Ticks (±184.79 Ticks)	0.63 ❗
Allocations - Average Deallocation time	1150.03 Ticks (±363.47 Ticks)	1292.85 Ticks (±344.36 Ticks)	0.89
mutex_benchmark Build Time	80.76 s	81.90 s	0.99 ❗
mutex_benchmark File Size	0.87 MB	0.87 MB	1.00 ❗
Mutex Stress Test Average Time per Iteration - 1 Threads	11.92 ns (±0.27 ns)	12.08 ns (±0.27 ns)	0.99
Mutex Stress Test Average Time per Iteration - 2 Threads	37.06 ns (±3.31 ns)	17.94 ns (±3.62 ns)	2.07 ❗

This comment was automatically generated by workflow using github-action-benchmark.

[mkroening]

Thanks for finding and fixing this bug! :)

Sorry for the late reply. I can reproduce the fix.

Regarding the implementation:
If I understand correctly, your changes now track two ports for listening sockets, one for the listener and one for the last connection, right? This does not support multiple open connections at the same time, right?

Perhaps doing something similar to TCP would be more useful: return a proper connection FD from accept instead of the current dummy NullSocket.

Regarding the test:
Could you adapt the existing vsock tests instead?

[archevel]

No worries! I appreciate you taking the time to have look. I've kept working on this locally and had to do some additional changes. Those are out of scope for this I think, as they involved needing to patch libc and mio to handle epoll for hermit. I'm working on that in order to be able to use firecracker to run a hermit based vm and communicate over vsock. Anyway, that is a larger piece of work, but I think I actually ended up aligning this more with how the TCP is handled. I'll take a peek and adjust this PR.

[archevel]

I've rebased the changes on main. I then changed to align it more with how the tcp socket works (handling multiple concurrent connections). Let me know if there are any other issues I should address/fix! The CI failed, so I'll take a peek and see if I understand why.

[archevel]

I think the CI fails because it isn't aligned with the changes in hermit-os/hermit-rs#996

[archevel]

@mkroening I realized that the code doesn't really handle multiple cids currently. E.g. If the host assigns to vsock cids say 3 and 4. Currently listening on either cid on port 1234 will get connections for both I think. Might be a separate issue to solve.