Re: [PATCH net-next v5 5/5] vsock/test: Add test for an unexpectedly lingering close()

[Michal Luczaj <mhal@rbox.co>]

On 5/21/25 16:56, Stefano Garzarella wrote:
> On Wed, May 21, 2025 at 12:55:23AM +0200, Michal Luczaj wrote:
>> There was an issue with SO_LINGER: instead of blocking until all queued
>> messages for the socket have been successfully sent (or the linger timeout
>> has been reached), close() would block until packets were handled by the
>> peer.
>>
>> Add a test to alert on close() lingering when it should not.
>>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>> tools/testing/vsock/vsock_test.c | 49 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index f401c6a79495bc7fda97012e5bfeabec7dbfb60a..1040503333cf315e52592c876f2c1809b36fdfdb 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -1839,6 +1839,50 @@ static void test_stream_linger_server(const struct test_opts *opts)
>> 	close(fd);
>> }
>>
>> +static void test_stream_nolinger_client(const struct test_opts *opts)
>> +{
>> +	bool nowait;
>> +	time_t ns;
>> +	int fd;
>> +
>> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>> +	if (fd < 0) {
>> +		perror("connect");
>> +		exit(EXIT_FAILURE);
>> +	}
>> +
>> +	enable_so_linger(fd);
> 
> If we use a parameter for the linger timeout, IMO will be easy to 
> understand this test, defining the timeout in this test, set it and 
> check the value, without defining LINGER_TIMEOUT in util.h.

Yes, you're right. I'll fix that.

>> +	send_byte(fd, 1, 0); /* Left unread to expose incorrect behaviour. */
>> +	nowait = vsock_wait_sent(fd);
>> +
>> +	ns = current_nsec();
>> +	close(fd);
>> +	ns = current_nsec() - ns;
>> +
>> +	if (nowait) {
>> +		fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
>> +	} else if ((ns + NSEC_PER_SEC - 1) / NSEC_PER_SEC >= LINGER_TIMEOUT) {
> 
> Should we define a macro for this conversion?
> 
> Or just use DIV_ROUND_UP:

Arrgh, I was looking for that. If you don't care much for a new macro, I'll
explicitly use DIV_ROUND_UP for now.

Thanks!
Michal