Re: [RFC PATCH v1 1/2] vsock: return errors other than -ENOMEM to socket
> On Mar 28, 2023, at 4:20 AM, Arseniy Krasnov wrote:
>
> !! External Email
>
> On 28.03.2023 14:19, Stefano Garzarella wrote:
>> On Tue, Mar 28, 2023 at 01:42:19PM +0300, Arseniy Krasnov wrote:
>>>
>>>
>>> On 28.03.2023 12:42, Stefano Garzarella wrote:
I pressed send too early...
CCing Bryan, Vishnu, and pv-driv...@vmware.com
On Tue, Mar 28, 2023 at 11:39 AM Stefano Garzarella
wrote:
>
> On Sun, Mar 26, 2023 at 01:13:11AM +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
>> ---
>> 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 19aea7cba26e..9262e0b77d47 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2007,7 +2007,7 @@ static int __vsock_stream_recvmsg(struct sock *sk,
>> struct msghdr *msg,
>>
>> read = transport->stream_dequeue(vsk, msg, len - copied,
>> flags);
>
> In vmci_transport_stream_dequeue() vmci_qpair_peekv() and
> vmci_qpair_dequev() return VMCI_ERROR_* in case of errors.
>
> Maybe we should return -ENOMEM in vmci_transport_stream_dequeue() if
> those functions fail to keep the same behavior.
>>>
>>> Yes, seems i missed it, because several months ago we had similar question
>>> for send
>>> logic:
>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Fkernel%2Fmsg4611091.html=05%7C01%7Cvdasa%40vmware.com%7C3b17793425384debe75708db2f7eec8c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C638155994413494900%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=MMfFcKuFFvMcJrbToKvWvIB%2FZmzp%2BdGGVWFVWztuSzg%3D=0
>>> And it was ok to not handle VMCI send path in this way. So i think current
>>> implementation
>>> for tx is a little bit buggy, because VMCI specific error from
>>> 'vmci_qpair_enquev()' is
>>> returned to af_vsock.c. I think error conversion must be added to VMCI
>>> transport for tx
>>> also.
>>
>> Good point!
>>
>> These are negative values, so there are no big problems, but I don't
>> know what the user expects in this case.
>>
>> @Vishnu Do we want to return an errno to the user or a VMCI_ERROR_*?
>
> Small remark, as i can see, VMCI_ERROR_ is not exported to user in
> include/uapi,
> so IIUC user won't be able to interpret such values correctly.
>
> Thanks, Arseniy
Let's just return -ENOMEM from vmci transport in case of error in
vmci_transport_stream_enqueue and vmci_transport_stream_dequeue.
@Arseniy,
Could you please add a separate patch in this set to handle the above?
Thanks,
Vishnu
>
>>
>> In both cases I think we should do the same for both enqueue and
>> dequeue.
>>
>>>
>>> Good thing is that Hyper-V uses general error codes.
>>
>> Yeah!
>>
>> Thanks,
>> Stefano
>>
>>>
>>> Thanks, Arseniy
>
> CCing Bryan, Vishnu, and pv-driv...@vmware.com
>
> The other transports seem okay to me.
>
> Thanks,
> Stefano
>
>> if (read < 0) {
>> - err = -ENOMEM;
>> + err = read;
>> break;
>> }
>>
>> @@ -2058,7 +2058,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
>>
>>>
>>
>
> !! External Email: This email originated from outside of the organization. Do
> not click links or open attachments unless you recognize the sender.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v1 1/2] vsock: return errors other than -ENOMEM to socket
On Tue, Mar 28, 2023 at 01:42:19PM +0300, Arseniy Krasnov wrote:
On 28.03.2023 12:42, Stefano Garzarella wrote:
I pressed send too early...
CCing Bryan, Vishnu, and pv-driv...@vmware.com
On Tue, Mar 28, 2023 at 11:39 AM Stefano Garzarella wrote:
On Sun, Mar 26, 2023 at 01:13:11AM +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
---
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 19aea7cba26e..9262e0b77d47 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2007,7 +2007,7 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct
msghdr *msg,
read = transport->stream_dequeue(vsk, msg, len - copied, flags);
In vmci_transport_stream_dequeue() vmci_qpair_peekv() and
vmci_qpair_dequev() return VMCI_ERROR_* in case of errors.
Maybe we should return -ENOMEM in vmci_transport_stream_dequeue() if
those functions fail to keep the same behavior.
Yes, seems i missed it, because several months ago we had similar question for
send
logic:
https://www.spinics.net/lists/kernel/msg4611091.html
And it was ok to not handle VMCI send path in this way. So i think current
implementation
for tx is a little bit buggy, because VMCI specific error from
'vmci_qpair_enquev()' is
returned to af_vsock.c. I think error conversion must be added to VMCI
transport for tx
also.
Good point!
These are negative values, so there are no big problems, but I don't
know what the user expects in this case.
@Vishnu Do we want to return an errno to the user or a VMCI_ERROR_*?
In both cases I think we should do the same for both enqueue and
dequeue.
Good thing is that Hyper-V uses general error codes.
Yeah!
Thanks,
Stefano
Thanks, Arseniy
CCing Bryan, Vishnu, and pv-driv...@vmware.com
The other transports seem okay to me.
Thanks,
Stefano
if (read < 0) {
- err = -ENOMEM;
+ err = read;
break;
}
@@ -2058,7 +2058,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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v1 1/2] vsock: return errors other than -ENOMEM to socket
I pressed send too early...
CCing Bryan, Vishnu, and pv-driv...@vmware.com
On Tue, Mar 28, 2023 at 11:39 AM Stefano Garzarella wrote:
>
> On Sun, Mar 26, 2023 at 01:13:11AM +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
> >---
> > 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 19aea7cba26e..9262e0b77d47 100644
> >--- a/net/vmw_vsock/af_vsock.c
> >+++ b/net/vmw_vsock/af_vsock.c
> >@@ -2007,7 +2007,7 @@ static int __vsock_stream_recvmsg(struct sock *sk,
> >struct msghdr *msg,
> >
> > read = transport->stream_dequeue(vsk, msg, len - copied,
> > flags);
>
> In vmci_transport_stream_dequeue() vmci_qpair_peekv() and
> vmci_qpair_dequev() return VMCI_ERROR_* in case of errors.
>
> Maybe we should return -ENOMEM in vmci_transport_stream_dequeue() if
> those functions fail to keep the same behavior.
>
> CCing Bryan, Vishnu, and pv-driv...@vmware.com
>
> The other transports seem okay to me.
>
> Thanks,
> Stefano
>
> > if (read < 0) {
> >- err = -ENOMEM;
> >+ err = read;
> > break;
> > }
> >
> >@@ -2058,7 +2058,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
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v1 1/2] vsock: return errors other than -ENOMEM to socket
On Sun, Mar 26, 2023 at 01:13:11AM +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
---
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 19aea7cba26e..9262e0b77d47 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2007,7 +2007,7 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct
msghdr *msg,
read = transport->stream_dequeue(vsk, msg, len - copied, flags);
In vmci_transport_stream_dequeue() vmci_qpair_peekv() and
vmci_qpair_dequev() return VMCI_ERROR_* in case of errors.
Maybe we should return -ENOMEM in vmci_transport_stream_dequeue() if
those functions fail to keep the same behavior.
CCing Bryan, Vishnu, and pv-driv...@vmware.com
The other transports seem okay to me.
Thanks,
Stefano
if (read < 0) {
- err = -ENOMEM;
+ err = read;
break;
}
@@ -2058,7 +2058,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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
