Re: [PATCH v5 3/3] vsock/test: verify socket options after setting them

[Konstantin Shkolnyy <kshk@linux.ibm.com>]

On 11/12/2024 02:58, Stefano Garzarella wrote:
> On Thu, Nov 07, 2024 at 07:17:26PM -0600, Konstantin Shkolnyy wrote:
>> Replace setsockopt() calls with calls to functions that follow
>> setsockopt() with getsockopt() and check that the returned value and its
>> size are the same as have been set.
>>
>> Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
>> ---
>> tools/testing/vsock/Makefile              |   8 +-
>> tools/testing/vsock/control.c             |   8 +-
>> tools/testing/vsock/msg_zerocopy_common.c |   8 +-
>> tools/testing/vsock/util_socket.c         | 149 ++++++++++++++++++++++
>> tools/testing/vsock/util_socket.h         |  19 +++
>> tools/testing/vsock/vsock_perf.c          |  24 ++--
>> tools/testing/vsock/vsock_test.c          |  40 +++---
>> 7 files changed, 208 insertions(+), 48 deletions(-)
>> create mode 100644 tools/testing/vsock/util_socket.c
>> create mode 100644 tools/testing/vsock/util_socket.h
>>
>> diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
>> index 6e0b4e95e230..1ec0b3a67aa4 100644
>> --- a/tools/testing/vsock/Makefile
>> +++ b/tools/testing/vsock/Makefile
>> @@ -1,12 +1,12 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> all: test vsock_perf
>> test: vsock_test vsock_diag_test vsock_uring_test
>> -vsock_test: vsock_test.o vsock_test_zerocopy.o timeout.o control.o 
>> util.o msg_zerocopy_common.o
>> -vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o
>> -vsock_perf: vsock_perf.o msg_zerocopy_common.o
>> +vsock_test: vsock_test.o vsock_test_zerocopy.o timeout.o control.o 
>> util.o msg_zerocopy_common.o util_socket.o
>> +vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o 
>> util_socket.o
>> +vsock_perf: vsock_perf.o msg_zerocopy_common.o util_socket.o
> 
> I would add the new functions to check setsockopt in util.c
> 
> vsock_perf is more of a tool to measure performance than a test, so
> we can avoid calling these checks there, tests should cover all
> cases regardless of vsock_perf.

The problem is that vsock_perf calls enable_so_zerocopy() which has to
call the new setsockopt_int_check() because it's also called by 
vsock_test. Do you prefer to give vsock_perf its own version of
enable_so_zerocopy() which doesn't call setsockopt_int_check()?