Hello,
this patchset removes behaviour, where error code returned from any
transport was always switched to ENOMEM. This works in the same way as
patch from Bobby Eshleman:
commit c43170b7e157 ("vsock: return errors other than -ENOMEM to socket"),
but for receive calls. VMCI transport is also updated (both tx and rx
SOCK_STREAM callbacks), because it returns VMCI specific error code to
af_vsock.c (like VMCI_ERROR_*). At the same time, virtio and Hyper-V
transports are using general error codes, so there is no need to update
them.
vsock_test suite is also updated.
Link to v1:
https://lore.kernel.org/netdev/[email protected]/
Changelog:
v1 -> v2:
- Add patch for VMCI as Vishnu Dasa suggested.
Arseniy Krasnov (3):
vsock: return errors other than -ENOMEM to socket
vsock/vmci: convert VMCI error code to -ENOMEM
vsock/test: update expected return values
net/vmw_vsock/af_vsock.c | 4 ++--
net/vmw_vsock/vmci_transport.c | 19 ++++++++++++++++---
tools/testing/vsock/vsock_test.c | 4 ++--
3 files changed, 20 insertions(+), 7 deletions(-)
--
2.25.1
This removes behaviour, where error code returned from any transport
was always switched to ENOMEM. This works in the same way as:
commit
c43170b7e157 ("vsock: return errors other than -ENOMEM to socket"),
but for receive calls.
Signed-off-by: Arseniy Krasnov <[email protected]>
---
net/vmw_vsock/af_vsock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 5f2dda35c980..413407bb646c 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2043,7 +2043,7 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg,
read = transport->stream_dequeue(vsk, msg, len - copied, flags);
if (read < 0) {
- err = -ENOMEM;
+ err = read;
break;
}
@@ -2094,7 +2094,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
msg_len = transport->seqpacket_dequeue(vsk, msg, flags);
if (msg_len < 0) {
- err = -ENOMEM;
+ err = msg_len;
goto out;
}
--
2.25.1
This adds conversion of VMCI specific error code to general -ENOMEM. It
is needed, because af_vsock.c passes error value returned from transport
to the user.
Signed-off-by: Arseniy Krasnov <[email protected]>
---
net/vmw_vsock/vmci_transport.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 36eb16a40745..45de3e75597f 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -1831,10 +1831,17 @@ static ssize_t vmci_transport_stream_dequeue(
size_t len,
int flags)
{
+ int err;
+
if (flags & MSG_PEEK)
- return vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
+ err = vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
else
- return vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
+ err = vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
+
+ if (err < 0)
+ err = -ENOMEM;
+
+ return err;
}
static ssize_t vmci_transport_stream_enqueue(
@@ -1842,7 +1849,13 @@ 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.25.1
This updates expected return values for invalid buffer test. Now such
values are returned from transport, not from af_vsock.c.
Signed-off-by: Arseniy Krasnov <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/vsock_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 3de10dbb50f5..a91d0ef963be 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -723,7 +723,7 @@ static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opt
exit(EXIT_FAILURE);
}
- if (errno != ENOMEM) {
+ if (errno != EFAULT) {
perror("unexpected errno of 'broken_buf'");
exit(EXIT_FAILURE);
}
@@ -887,7 +887,7 @@ static void test_inv_buf_client(const struct test_opts *opts, bool stream)
exit(EXIT_FAILURE);
}
- if (errno != ENOMEM) {
+ if (errno != EFAULT) {
fprintf(stderr, "unexpected recv(2) errno %d\n", errno);
exit(EXIT_FAILURE);
}
--
2.25.1
On Thu, Mar 30, 2023 at 10:05:45AM +0300, Arseniy Krasnov wrote:
>This removes behaviour, where error code returned from any transport
>was always switched to ENOMEM. This works in the same way as:
>commit
>c43170b7e157 ("vsock: return errors other than -ENOMEM to socket"),
>but for receive calls.
>
>Signed-off-by: Arseniy Krasnov <[email protected]>
>---
> net/vmw_vsock/af_vsock.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
We should first make sure that all transports return the right value,
and then expose it to the user, so I would move this patch, after
patch 2.
Thanks,
Stefano
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 5f2dda35c980..413407bb646c 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -2043,7 +2043,7 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg,
>
> read = transport->stream_dequeue(vsk, msg, len - copied, flags);
> if (read < 0) {
>- err = -ENOMEM;
>+ err = read;
> break;
> }
>
>@@ -2094,7 +2094,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
> msg_len = transport->seqpacket_dequeue(vsk, msg, flags);
>
> if (msg_len < 0) {
>- err = -ENOMEM;
>+ err = msg_len;
> goto out;
> }
>
>--
>2.25.1
>
On Thu, Mar 30, 2023 at 10:07:36AM +0300, Arseniy Krasnov wrote:
>This adds conversion of VMCI specific error code to general -ENOMEM. It
>is needed, because af_vsock.c passes error value returned from transport
>to the user.
>
>Signed-off-by: Arseniy Krasnov <[email protected]>
>---
> net/vmw_vsock/vmci_transport.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 36eb16a40745..45de3e75597f 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -1831,10 +1831,17 @@ static ssize_t vmci_transport_stream_dequeue(
> size_t len,
> int flags)
> {
>+ int err;
Please, use the same type returned by the function.
>+
> if (flags & MSG_PEEK)
>- return vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>+ err = vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
> else
>- return vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>+ err = vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>+
>+ if (err < 0)
>+ err = -ENOMEM;
>+
>+ return err;
> }
>
> static ssize_t vmci_transport_stream_enqueue(
>@@ -1842,7 +1849,13 @@ 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;
Ditto.
>+
>+ err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>+ if (err < 0)
>+ err = -ENOMEM;
>+
>+ return err;
> }
@Vishnu: should we backport the change for
vmci_transport_stream_enqueue() to stable branches?
In this case I would split this patch and I would send the
vmci_transport_stream_enqueue() change to the net branch including:
Fixes: c43170b7e157 ("vsock: return errors other than -ENOMEM to socket")
Thanks,
Stefano
On 30.03.2023 11:02, Stefano Garzarella wrote:
> On Thu, Mar 30, 2023 at 10:05:45AM +0300, Arseniy Krasnov wrote:
>> This removes behaviour, where error code returned from any transport
>> was always switched to ENOMEM. This works in the same way as:
>> commit
>> c43170b7e157 ("vsock: return errors other than -ENOMEM to socket"),
>> but for receive calls.
>>
>> Signed-off-by: Arseniy Krasnov <[email protected]>
>> ---
>> net/vmw_vsock/af_vsock.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> We should first make sure that all transports return the right value,
> and then expose it to the user, so I would move this patch, after
> patch 2.
Yes, right! I'll reorder patches and fix VMCI patch after reply from @Vishnu
Thanks, Arseniy
>
> Thanks,
> Stefano
>
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 5f2dda35c980..413407bb646c 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2043,7 +2043,7 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg,
>>
>> read = transport->stream_dequeue(vsk, msg, len - copied, flags);
>> if (read < 0) {
>> - err = -ENOMEM;
>> + err = read;
>> break;
>> }
>>
>> @@ -2094,7 +2094,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
>> msg_len = transport->seqpacket_dequeue(vsk, msg, flags);
>>
>> if (msg_len < 0) {
>> - err = -ENOMEM;
>> + err = msg_len;
>> goto out;
>> }
>>
>> --
>> 2.25.1
>>
>
> On Mar 30, 2023, at 1:19 AM, Stefano Garzarella <[email protected]> wrote:
>
> !! External Email
>
> On Thu, Mar 30, 2023 at 10:07:36AM +0300, Arseniy Krasnov wrote:
>> This adds conversion of VMCI specific error code to general -ENOMEM. It
>> is needed, because af_vsock.c passes error value returned from transport
>> to the user.
>>
>> Signed-off-by: Arseniy Krasnov <[email protected]>
>> ---
>> net/vmw_vsock/vmci_transport.c | 19 ++++++++++++++++---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> index 36eb16a40745..45de3e75597f 100644
>> --- a/net/vmw_vsock/vmci_transport.c
>> +++ b/net/vmw_vsock/vmci_transport.c
>> @@ -1831,10 +1831,17 @@ static ssize_t vmci_transport_stream_dequeue(
>> size_t len,
>> int flags)
>> {
>> + int err;
>
> Please, use the same type returned by the function.
>
>> +
>> if (flags & MSG_PEEK)
>> - return vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>> + err = vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>> else
>> - return vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>> + err = vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +
>> + if (err < 0)
>> + err = -ENOMEM;
>> +
>> + return err;
>> }
>>
>> static ssize_t vmci_transport_stream_enqueue(
>> @@ -1842,7 +1849,13 @@ 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;
>
> Ditto.
>
>> +
>> + err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> + if (err < 0)
>> + err = -ENOMEM;
>> +
>> + return err;
>> }
>
> @Vishnu: should we backport the change for
> vmci_transport_stream_enqueue() to stable branches?
>
> In this case I would split this patch and I would send the
> vmci_transport_stream_enqueue() change to the net branch including:
>
> Fixes: c43170b7e157 ("vsock: return errors other than -ENOMEM to socket")
Yes, good point. It would be better to do it this way for correctness.
Thanks,
Vishnu
>
> Thanks,
> Stefano
>
>
> !! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.