Message ID | 20190801152541.245833-11-sgarzare@redhat.com |
---|---|
State | Changes Requested |
Delegated to: | David Ahern |
Headers | show |
Series | VSOCK: add vsock_test test suite | expand |
Hello! On 08/01/2019 06:25 PM, Stefano Garzarella wrote: > When VMCI transport is used, if the guest closes a connection, > all data is gone and EOF is returned, so we should skip the read > of data written by the peer before closing the connection. > > Reported-by: Jorgen Hansen <jhansen@vmware.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c > index cb606091489f..64adf45501ca 100644 > --- a/tools/testing/vsock/vsock_test.c > +++ b/tools/testing/vsock/vsock_test.c [...] > @@ -79,16 +80,27 @@ static void test_stream_client_close_server(const struct test_opts *opts) > exit(EXIT_FAILURE); > } > > + local_cid = vsock_get_local_cid(fd); > + > control_expectln("CLOSED"); > > send_byte(fd, -EPIPE); > - recv_byte(fd, 1); > + > + /* Skip the read of data wrote by the peer if we are on VMCI and s/wrote/written/? > + * we are on the host side, because when the guest closes a > + * connection, all data is gone and EOF is returned. > + */ > + if (!(opts->transport == TEST_TRANSPORT_VMCI && > + local_cid == VMADDR_CID_HOST)) > + recv_byte(fd, 1); > + > recv_byte(fd, 0); > close(fd); > } [...] MBR, Sergei
On Thu, Aug 01, 2019 at 06:53:32PM +0300, Sergei Shtylyov wrote: > Hello! > Hi :) > On 08/01/2019 06:25 PM, Stefano Garzarella wrote: > > > When VMCI transport is used, if the guest closes a connection, > > all data is gone and EOF is returned, so we should skip the read > > of data written by the peer before closing the connection. > > > > Reported-by: Jorgen Hansen <jhansen@vmware.com> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > --- > > tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c > > index cb606091489f..64adf45501ca 100644 > > --- a/tools/testing/vsock/vsock_test.c > > +++ b/tools/testing/vsock/vsock_test.c > [...] > > @@ -79,16 +80,27 @@ static void test_stream_client_close_server(const struct test_opts *opts) > > exit(EXIT_FAILURE); > > } > > > > + local_cid = vsock_get_local_cid(fd); > > + > > control_expectln("CLOSED"); > > > > send_byte(fd, -EPIPE); > > - recv_byte(fd, 1); > > + > > + /* Skip the read of data wrote by the peer if we are on VMCI and > > s/wrote/written/? > Thanks, I'll fix it! Stefano > > + * we are on the host side, because when the guest closes a > > + * connection, all data is gone and EOF is returned. > > + */ > > + if (!(opts->transport == TEST_TRANSPORT_VMCI && > > + local_cid == VMADDR_CID_HOST)) > > + recv_byte(fd, 1); > > + > > recv_byte(fd, 0); > > close(fd); > > } > [...] > > MBR, Sergei --
On Thu, Aug 01, 2019 at 05:25:40PM +0200, Stefano Garzarella wrote: > When VMCI transport is used, if the guest closes a connection, > all data is gone and EOF is returned, so we should skip the read > of data written by the peer before closing the connection. All transports should aim for identical semantics. I think virtio-vsock should behave the same as VMCI since userspace applications should be transport-independent. Let's view this as a vsock bug. Is it feasible to change the VMCI behavior so it's more like TCP sockets? If not, let's change the virtio-vsock behavior to be compatible with VMCI.
On Tue, Aug 20, 2019 at 09:32:03AM +0100, Stefan Hajnoczi wrote: > On Thu, Aug 01, 2019 at 05:25:40PM +0200, Stefano Garzarella wrote: > > When VMCI transport is used, if the guest closes a connection, > > all data is gone and EOF is returned, so we should skip the read > > of data written by the peer before closing the connection. > > All transports should aim for identical semantics. I think virtio-vsock > should behave the same as VMCI since userspace applications should be > transport-independent. Yes, it is a good point! > > Let's view this as a vsock bug. Is it feasible to change the VMCI > behavior so it's more like TCP sockets? If not, let's change the > virtio-vsock behavior to be compatible with VMCI. I'm not sure it is feasible to change the VMCI behavior. IIUC reading the Jorgen's answer [1], this was a decision made during the implementation. @Jorgen: please, can you confirm? or not :-) If it is the case, I'll change virtio-vsock to the same behavior. Thanks, Stefano [1] https://patchwork.ozlabs.org/cover/847998/#1831400
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index cb606091489f..64adf45501ca 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -71,6 +71,7 @@ static void test_stream_client_close_client(const struct test_opts *opts) static void test_stream_client_close_server(const struct test_opts *opts) { + unsigned int local_cid; int fd; fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL); @@ -79,16 +80,27 @@ static void test_stream_client_close_server(const struct test_opts *opts) exit(EXIT_FAILURE); } + local_cid = vsock_get_local_cid(fd); + control_expectln("CLOSED"); send_byte(fd, -EPIPE); - recv_byte(fd, 1); + + /* Skip the read of data wrote by the peer if we are on VMCI and + * we are on the host side, because when the guest closes a + * connection, all data is gone and EOF is returned. + */ + if (!(opts->transport == TEST_TRANSPORT_VMCI && + local_cid == VMADDR_CID_HOST)) + recv_byte(fd, 1); + recv_byte(fd, 0); close(fd); } static void test_stream_server_close_client(const struct test_opts *opts) { + unsigned int local_cid; int fd; fd = vsock_stream_connect(opts->peer_cid, 1234); @@ -97,10 +109,20 @@ static void test_stream_server_close_client(const struct test_opts *opts) exit(EXIT_FAILURE); } + local_cid = vsock_get_local_cid(fd); + control_expectln("CLOSED"); send_byte(fd, -EPIPE); - recv_byte(fd, 1); + + /* Skip the read of data wrote by the peer if we are on VMCI and + * we are on the host side, because when the guest closes a + * connection, all data is gone and EOF is returned. + */ + if (!(opts->transport == TEST_TRANSPORT_VMCI && + local_cid == VMADDR_CID_HOST)) + recv_byte(fd, 1); + recv_byte(fd, 0); close(fd); }
When VMCI transport is used, if the guest closes a connection, all data is gone and EOF is returned, so we should skip the read of data written by the peer before closing the connection. Reported-by: Jorgen Hansen <jhansen@vmware.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)