On Fri, Nov 17, 2023 at 10:12:38AM +0300, Arseniy Krasnov wrote:
On 15.11.2023 14:11, Stefano Garzarella wrote:
On Wed, Nov 08, 2023 at 10:20:04AM +0300, Arseniy Krasnov wrote:
This adds test which checks, that updating SO_RCVLOWAT value also sends
You can avoid "This adds", and write just "Add test ...".
See https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour.
Also in the other patch.
credit update message. Otherwise mutual hungup may happen when receiver
didn't send credit update and then calls 'poll()' with non default
SO_RCVLOWAT value (e.g. waiting enough bytes to read), while sender
waits for free space at receiver's side.
Signed-off-by: Arseniy Krasnov <avkrasnov@xxxxxxxxxxxxxxxxx>
---
tools/testing/vsock/vsock_test.c | 131 +++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index c1f7bc9abd22..c71b3875fd16 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -1180,6 +1180,132 @@ static void test_stream_shutrd_server(const struct test_opts *opts)
close(fd);
}
+#define RCVLOWAT_CREDIT_UPD_BUF_SIZE (1024 * 128)
+#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
What about adding a comment like the one in the cover letter about
dependency with kernel values?
Please add it also in the commit description.
I'm thinking if we should move all the defines that depends on the
kernel in some special header.
IIUC it will be new header file in tools/testing/vsock, which includes such defines. At
this moment in will contain only VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. Idea is that such defines
So this only works on the virtio transport though, not the other
transports, right? (but maybe the others don't have this problem, so
it's fine).
are not supposed to use by user (so do not move it to uapi headers), but needed by tests
to check kernel behaviour. Please correct me if i'm wrong.
Right!
Maybe if it's just one, we can leave it there for now, but with a
comment on top explaining where it comes.
Thanks,
Stefano
