| Message ID | 20191011130758.22134-1-sgarzare@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | vsock: don't allow half-closed socket in the host transports | expand |
On Fri, Oct 11, 2019 at 03:07:56PM +0200, Stefano Garzarella wrote: > We are implementing a test suite for the VSOCK sockets and we discovered > that vmci_transport never allowed half-closed socket on the host side. > > As Jorgen explained [1] this is due to the implementation of VMCI. > > Since we want to have the same behaviour across all transports, this > series adds a section in the "Implementation notes" to exaplain this > behaviour, and changes the vhost_transport to behave the same way. > > [1] https://patchwork.ozlabs.org/cover/847998/#1831400 Half closed sockets are very useful, and lots of applications use tricks to swap a vsock for a tcp socket, which might as a result break. If VMCI really cares it can implement an ioctl to allow applications to detect that half closed sockets aren't supported. It does not look like VMCI wants to bother (users do not read kernel implementation notes) so it does not really care. So why do we want to cripple other transports intentionally? > Stefano Garzarella (2): > vsock: add half-closed socket details in the implementation notes > vhost/vsock: don't allow half-closed socket in the host > > drivers/vhost/vsock.c | 17 ++++++++++++++++- > net/vmw_vsock/af_vsock.c | 4 ++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > > -- > 2.21.0
On Fri, Oct 11, 2019 at 10:19:13AM -0400, Michael S. Tsirkin wrote: > On Fri, Oct 11, 2019 at 03:07:56PM +0200, Stefano Garzarella wrote: > > We are implementing a test suite for the VSOCK sockets and we discovered > > that vmci_transport never allowed half-closed socket on the host side. > > > > As Jorgen explained [1] this is due to the implementation of VMCI. > > > > Since we want to have the same behaviour across all transports, this > > series adds a section in the "Implementation notes" to exaplain this > > behaviour, and changes the vhost_transport to behave the same way. > > > > [1] https://patchwork.ozlabs.org/cover/847998/#1831400 > > Half closed sockets are very useful, and lots of > applications use tricks to swap a vsock for a tcp socket, > which might as a result break. Got it! > > If VMCI really cares it can implement an ioctl to > allow applications to detect that half closed sockets aren't supported. > > It does not look like VMCI wants to bother (users do not read > kernel implementation notes) so it does not really care. > So why do we want to cripple other transports intentionally? The main reason is that we are developing the test suite and we noticed the miss match. Since we want to make sure that applications behave in the same way on different transports, we thought we would solve it that way. But what you are saying (also in the reply of the patches) is actually quite right. Not being publicized, applications do not expect this behavior, so please discard this series. My problem during the tests, was trying to figure out if half-closed sockets were supported or not, so as you say adding an IOCTL or maybe better a getsockopt() could solve the problem. What do you think? Thanks, Stefano
On Fri, Oct 11, 2019 at 04:34:57PM +0200, Stefano Garzarella wrote: > On Fri, Oct 11, 2019 at 10:19:13AM -0400, Michael S. Tsirkin wrote: > > On Fri, Oct 11, 2019 at 03:07:56PM +0200, Stefano Garzarella wrote: > > > We are implementing a test suite for the VSOCK sockets and we discovered > > > that vmci_transport never allowed half-closed socket on the host side. > > > > > > As Jorgen explained [1] this is due to the implementation of VMCI. > > > > > > Since we want to have the same behaviour across all transports, this > > > series adds a section in the "Implementation notes" to exaplain this > > > behaviour, and changes the vhost_transport to behave the same way. > > > > > > [1] https://patchwork.ozlabs.org/cover/847998/#1831400 > > > > Half closed sockets are very useful, and lots of > > applications use tricks to swap a vsock for a tcp socket, > > which might as a result break. > > Got it! > > > > > If VMCI really cares it can implement an ioctl to > > allow applications to detect that half closed sockets aren't supported. > > > > It does not look like VMCI wants to bother (users do not read > > kernel implementation notes) so it does not really care. > > So why do we want to cripple other transports intentionally? > > The main reason is that we are developing the test suite and we noticed > the miss match. Since we want to make sure that applications behave in > the same way on different transports, we thought we would solve it that > way. > > But what you are saying (also in the reply of the patches) is actually > quite right. Not being publicized, applications do not expect this behavior, > so please discard this series. > > My problem during the tests, was trying to figure out if half-closed > sockets were supported or not, so as you say adding an IOCTL or maybe > better a getsockopt() could solve the problem. > > What do you think? > > Thanks, > Stefano Sure, why not.
On Sat, Oct 12, 2019 at 06:38:46PM -0400, Michael S. Tsirkin wrote: > On Fri, Oct 11, 2019 at 04:34:57PM +0200, Stefano Garzarella wrote: > > On Fri, Oct 11, 2019 at 10:19:13AM -0400, Michael S. Tsirkin wrote: > > > On Fri, Oct 11, 2019 at 03:07:56PM +0200, Stefano Garzarella wrote: > > > > We are implementing a test suite for the VSOCK sockets and we discovered > > > > that vmci_transport never allowed half-closed socket on the host side. > > > > > > > > As Jorgen explained [1] this is due to the implementation of VMCI. > > > > > > > > Since we want to have the same behaviour across all transports, this > > > > series adds a section in the "Implementation notes" to exaplain this > > > > behaviour, and changes the vhost_transport to behave the same way. > > > > > > > > [1] https://patchwork.ozlabs.org/cover/847998/#1831400 > > > > > > Half closed sockets are very useful, and lots of > > > applications use tricks to swap a vsock for a tcp socket, > > > which might as a result break. > > > > Got it! > > > > > > > > If VMCI really cares it can implement an ioctl to > > > allow applications to detect that half closed sockets aren't supported. > > > > > > It does not look like VMCI wants to bother (users do not read > > > kernel implementation notes) so it does not really care. > > > So why do we want to cripple other transports intentionally? > > > > The main reason is that we are developing the test suite and we noticed > > the miss match. Since we want to make sure that applications behave in > > the same way on different transports, we thought we would solve it that > > way. > > > > But what you are saying (also in the reply of the patches) is actually > > quite right. Not being publicized, applications do not expect this behavior, > > so please discard this series. > > > > My problem during the tests, was trying to figure out if half-closed > > sockets were supported or not, so as you say adding an IOCTL or maybe > > better a getsockopt() could solve the problem. > > > > What do you think? > > > > Thanks, > > Stefano > > Sure, why not. The aim is for applications using AF_VSOCK sockets to run on any transport. When the semantics differ between transports it creates a compatibility problem. That said, I do think keeping the standard sockets behavior is reasonable. If applications have problems on VMCI a sockopt may be necessary :(. Stefan