| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 17 Aug 2022 05:28:43 +0000
From: Arseniy Krasnov <AVKrasnov@...rdevices.ru>
To: Bobby Eshleman <bobbyeshleman@...il.com>,
Bobby Eshleman <bobby.eshleman@...il.com>
CC: "virtio-dev@...ts.oasis-open.org" <virtio-dev@...ts.oasis-open.org>,
"Bobby Eshleman" <bobby.eshleman@...edance.com>,
Cong Wang <cong.wang@...edance.com>,
Jiang Wang <jiang.wang@...edance.com>,
Stefan Hajnoczi <stefanha@...hat.com>,
Stefano Garzarella <sgarzare@...hat.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
"Stephen Hemminger" <sthemmin@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: Re: [virtio-dev] Re: [PATCH 2/6] vsock: return errors other than
-ENOMEM to socket
On 16.08.2022 05:30, Bobby Eshleman wrote:
> CC'ing virtio-dev@...ts.oasis-open.org
>
> On Mon, Aug 15, 2022 at 10:56:05AM -0700, Bobby Eshleman wrote:
>> This commit allows vsock implementations to return errors
>> to the socket layer other than -ENOMEM. One immediate effect
>> of this is that upon the sk_sndbuf threshold being reached -EAGAIN
>> will be returned and userspace may throttle appropriately.
>>
>> Resultingly, a known issue with uperf is resolved[1].
>>
>> Additionally, to preserve legacy behavior for non-virtio
>> implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
>> is unchanged.
>>
>> [1]: https://gitlab.com/vsock/vsock/-/issues/1
>>
>> Signed-off-by: Bobby Eshleman <bobby.eshleman@...edance.com>
>> ---
>> include/linux/virtio_vsock.h | 3 +++
>> net/vmw_vsock/af_vsock.c | 3 ++-
>> net/vmw_vsock/hyperv_transport.c | 2 +-
>> net/vmw_vsock/virtio_transport_common.c | 3 ---
>> net/vmw_vsock/vmci_transport.c | 9 ++++++++-
>> 5 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> index 17ed01466875..9a37eddbb87a 100644
>> --- a/include/linux/virtio_vsock.h
>> +++ b/include/linux/virtio_vsock.h
>> @@ -8,6 +8,9 @@
>> #include <net/sock.h>
>> #include <net/af_vsock.h>
>>
>> +/* Threshold for detecting small packets to copy */
>> +#define GOOD_COPY_LEN 128
>> +
>> enum virtio_vsock_metadata_flags {
>> VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
>> VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index e348b2d09eac..1893f8aafa48 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
>> written = transport->stream_enqueue(vsk,
>> msg, len - total_written);
>> }
>> +
>> if (written < 0) {
>> - err = -ENOMEM;
>> + err = written;
>> goto out_err;
>> }
IIUC, for stream, this thing will have effect, only one first transport access fails. In this
case 'total_written' will be 0, so 'err' == 'written' will be returned. But when 'total_written > 0',
'err' will be overwritten by 'total_written' below, preserving current behaviour. Is it what You
supposed?
Thanks
>>
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index fd98229e3db3..e99aea571f6f 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
>> if (bytes_written)
>> ret = bytes_written;
>> kfree(send_buf);
>> - return ret;
>> + return ret < 0 ? -ENOMEM : ret;
>> }
>>
>> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> index 920578597bb9..d5780599fe93 100644
>> --- a/net/vmw_vsock/virtio_transport_common.c
>> +++ b/net/vmw_vsock/virtio_transport_common.c
>> @@ -23,9 +23,6 @@
>> /* How long to wait for graceful shutdown of a connection */
>> #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>>
>> -/* Threshold for detecting small packets to copy */
>> -#define GOOD_COPY_LEN 128
>> -
>> static const struct virtio_transport *
>> virtio_transport_get_ops(struct vsock_sock *vsk)
>> {
>> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> index b14f0ed7427b..c927a90dc859 100644
>> --- a/net/vmw_vsock/vmci_transport.c
>> +++ b/net/vmw_vsock/vmci_transport.c
>> @@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
>> struct msghdr *msg,
>> size_t len)
>> {
>> - return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> + int err;
>> +
>> + err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +
>> + if (err < 0)
>> + err = -ENOMEM;
>> +
>> + return err;
>> }
>>
>> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
>> --
>> 2.35.1
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@...ts.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@...ts.oasis-open.org
>
Powered by blists - more mailing lists