Re: [PATCH net-next] vsock/test: Cover more CIDs in transport_uaf test

[Michal Luczaj <mhal@rbox.co>]

On 5/28/25 11:08, Stefano Garzarella wrote:
> On Wed, May 28, 2025 at 10:58:28AM +0200, Michal Luczaj wrote:
>> Administrative query: while net-next is closed, am I supposed to mark this
>> series as "RFC" and post v2 for a review as usual, or is it better to just
>> hold off until net-next opens?
> 
> Whichever you prefer, if you are uncertain about the next version and 
> want to speed things up with a review while waiting, then go with RFC, 
> but if you think all comments are resolved and the next version is ready 
> to be merged, wait for the reopening.
> Thanks for asking!

All right then, I gave RFC a try:
https://lore.kernel.org/netdev/20250528-vsock-test-inc-cov-v2-0-8f655b40d57c@rbox.co/

>>>>>>>> +static void test_stream_transport_uaf_client(const struct test_opts *opts)
>>>>>>>> +{
>>>>>>>> +	bool tested = false;
>>>>>>>> +	int cid;
>>>>>>>> +
>>>>>>>> +	for (cid = VMADDR_CID_HYPERVISOR; cid <= VMADDR_CID_HOST + 1; ++cid)
>>>>>>>
>>>>>>>> +		tested |= test_stream_transport_uaf(cid);
>>>>>>>> +
>>>>>>>> +	if (!tested)
>>>>>>>> +		fprintf(stderr, "No transport tested\n");
>>>>>>>> +
>>>>>>>> 	control_writeln("DONE");
>>>>>>>
>>>>>>> While we're at it, I think we can remove this message, looking at
>>>>>>> run_tests() in util.c, we already have a barrier.
>>>>>>
>>>>>> Ok, sure. Note that console output gets slightly de-synchronised: server
>>>>>> will immediately print next test's prompt and wait there.
>>>>>
>>>>> I see, however I don't have a strong opinion, you can leave it that way
>>>>> if you prefer.
>>>>
>>>> How about adding a sync point to run_tests()? E.g.
>>>
>>> Yep, why not, of course in another series :-)
>>>
>>> And if you like, you can remove that specific sync point in that series
>>> and check also other tests, but I think we have only that one.
>>
>> OK, I'll leave that for later.
> 
> Yep, feel free to discard my suggestion, we can fix it later.

I was thinking about doing a console-output-beautification series
with: 1) drop the redundant sync in test_stream_transport_uaf_*, 2) add a
sync in run_tests(). But I guess we can have the sync dropping part here.
Definitely less churn this way.

Thanks,
Michal