When hv_sock was originally added, __vsock_stream_recvmsg() and
vsock_stream_has_data() actually only needed to know whether there
is any readable data or not, so hvs_stream_has_data() was written to
return 1 or 0 for simplicity.
However, now hvs_stream_has_data() should return the readable bytes
because vsock_data_ready() -> vsock_stream_has_data() needs to know the
actual bytes rather than a boolean value of 1 or 0.
The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
the readable bytes.
Let hvs_stream_has_data() return the readable bytes of the payload in
the next host-to-guest VMBus hv_sock packet.
Note: there may be multpile incoming hv_sock packets pending in the
VMBus channel's ringbuffer, but so far there is not a VMBus API that
allows us to know all the readable bytes in total without reading and
caching the payload of the multiple packets, so let's just return the
readable bytes of the next single packet. In the future, we'll either
add a VMBus API that allows us to know the total readable bytes without
touching the data in the ringbuffer, or the hv_sock driver needs to
understand the VMBus packet format and parse the packets directly.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
net/vmw_vsock/hyperv_transport.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 31342ab502b4fc35feb812d2c94e0e35ded73771..432fcbbd14d4f44bd2550be8376e42ce65122758 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -694,15 +694,26 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
static s64 hvs_stream_has_data(struct vsock_sock *vsk)
{
struct hvsock *hvs = vsk->trans;
+ bool need_refill;
s64 ret;
if (hvs->recv_data_len > 0)
- return 1;
+ return hvs->recv_data_len;
switch (hvs_channel_readable_payload(hvs->chan)) {
case 1:
- ret = 1;
- break;
+ need_refill = !hvs->recv_desc;
+ if (!need_refill)
+ return -EIO;
+
+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
+ if (!hvs->recv_desc)
+ return -ENOBUFS;
+
+ ret = hvs_update_recv_data(hvs);
+ if (ret)
+ return ret;
+ return hvs->recv_data_len;
case 0:
vsk->peer_shutdown |= SEND_SHUTDOWN;
ret = 0;
--
2.34.1On Sun, Jul 06, 2025 at 12:36:29PM +0800, Xuewei Niu wrote:
>When hv_sock was originally added, __vsock_stream_recvmsg() and
>vsock_stream_has_data() actually only needed to know whether there
>is any readable data or not, so hvs_stream_has_data() was written to
>return 1 or 0 for simplicity.
>
>However, now hvs_stream_has_data() should return the readable bytes
>because vsock_data_ready() -> vsock_stream_has_data() needs to know the
>actual bytes rather than a boolean value of 1 or 0.
>
>The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
>the readable bytes.
>
>Let hvs_stream_has_data() return the readable bytes of the payload in
>the next host-to-guest VMBus hv_sock packet.
>
>Note: there may be multpile incoming hv_sock packets pending in the
s/multpile/multiple
>VMBus channel's ringbuffer, but so far there is not a VMBus API that
>allows us to know all the readable bytes in total without reading and
>caching the payload of the multiple packets, so let's just return the
>readable bytes of the next single packet. In the future, we'll either
>add a VMBus API that allows us to know the total readable bytes without
>touching the data in the ringbuffer, or the hv_sock driver needs to
>understand the VMBus packet format and parse the packets directly.
>
>Signed-off-by: Dexuan Cui <decui@microsoft.com>
>---
> net/vmw_vsock/hyperv_transport.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 31342ab502b4fc35feb812d2c94e0e35ded73771..432fcbbd14d4f44bd2550be8376e42ce65122758 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -694,15 +694,26 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> {
> struct hvsock *hvs = vsk->trans;
>+ bool need_refill;
> s64 ret;
>
> if (hvs->recv_data_len > 0)
>- return 1;
>+ return hvs->recv_data_len;
>
> switch (hvs_channel_readable_payload(hvs->chan)) {
> case 1:
>- ret = 1;
>- break;
>+ need_refill = !hvs->recv_desc;
>+ if (!need_refill)
>+ return -EIO;
>+
>+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
>+ if (!hvs->recv_desc)
>+ return -ENOBUFS;
>+
>+ ret = hvs_update_recv_data(hvs);
>+ if (ret)
>+ return ret;
>+ return hvs->recv_data_len;
> case 0:
> vsk->peer_shutdown |= SEND_SHUTDOWN;
> ret = 0;
>
>--
>2.34.1
>
On 2025/7/7 21:42, Stefano Garzarella wrote:
> On Sun, Jul 06, 2025 at 12:36:29PM +0800, Xuewei Niu wrote:
>> When hv_sock was originally added, __vsock_stream_recvmsg() and
>> vsock_stream_has_data() actually only needed to know whether there
>> is any readable data or not, so hvs_stream_has_data() was written to
>> return 1 or 0 for simplicity.
>>
>> However, now hvs_stream_has_data() should return the readable bytes
>> because vsock_data_ready() -> vsock_stream_has_data() needs to know the
>> actual bytes rather than a boolean value of 1 or 0.
>>
>> The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
>> the readable bytes.
>>
>> Let hvs_stream_has_data() return the readable bytes of the payload in
>> the next host-to-guest VMBus hv_sock packet.
>>
>> Note: there may be multpile incoming hv_sock packets pending in the
>
> s/multpile/multiple
Will do.
Thanks,
Xuewei
>> VMBus channel's ringbuffer, but so far there is not a VMBus API that
>> allows us to know all the readable bytes in total without reading and
>> caching the payload of the multiple packets, so let's just return the
>> readable bytes of the next single packet. In the future, we'll either
>> add a VMBus API that allows us to know the total readable bytes without
>> touching the data in the ringbuffer, or the hv_sock driver needs to
>> understand the VMBus packet format and parse the packets directly.
>>
>> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>> ---
>> net/vmw_vsock/hyperv_transport.c | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index 31342ab502b4fc35feb812d2c94e0e35ded73771..432fcbbd14d4f44bd2550be8376e42ce65122758 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -694,15 +694,26 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
>> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>> {
>> struct hvsock *hvs = vsk->trans;
>> + bool need_refill;
>> s64 ret;
>>
>> if (hvs->recv_data_len > 0)
>> - return 1;
>> + return hvs->recv_data_len;
>>
>> switch (hvs_channel_readable_payload(hvs->chan)) {
>> case 1:
>> - ret = 1;
>> - break;
>> + need_refill = !hvs->recv_desc;
>> + if (!need_refill)
>> + return -EIO;
>> +
>> + hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
>> + if (!hvs->recv_desc)
>> + return -ENOBUFS;
>> +
>> + ret = hvs_update_recv_data(hvs);
>> + if (ret)
>> + return ret;
>> + return hvs->recv_data_len;
>> case 0:
>> vsk->peer_shutdown |= SEND_SHUTDOWN;
>> ret = 0;
>>
>> --
>> 2.34.1
>>
On Sun, Jul 06, 2025 at 12:36:29PM +0800, Xuewei Niu wrote:
Again, this patch should have `From: Dexuan Cui <decui@microsoft.com>`
on first line, and this is done automatically by `git format-patch` (or
others tools) if the author is the right one in your branch.
I'm not sure what is going on on your side, but you should avoid to
reset the original author.
Applying this patch we will have:
commit ed36075e04ecbb1dd02a3d8eba5bfac6469d73e4
Author: Xuewei Niu <niuxuewei97@gmail.com>
Date: Sun Jul 6 12:36:29 2025 +0800
hv_sock: Return the readable bytes in hvs_stream_has_data()
This is not what we want, right?
>When hv_sock was originally added, __vsock_stream_recvmsg() and
>vsock_stream_has_data() actually only needed to know whether there
>is any readable data or not, so hvs_stream_has_data() was written to
>return 1 or 0 for simplicity.
>
>However, now hvs_stream_has_data() should return the readable bytes
>because vsock_data_ready() -> vsock_stream_has_data() needs to know the
>actual bytes rather than a boolean value of 1 or 0.
>
>The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
>the readable bytes.
>
>Let hvs_stream_has_data() return the readable bytes of the payload in
>the next host-to-guest VMBus hv_sock packet.
>
>Note: there may be multpile incoming hv_sock packets pending in the
>VMBus channel's ringbuffer, but so far there is not a VMBus API that
>allows us to know all the readable bytes in total without reading and
>caching the payload of the multiple packets, so let's just return the
>readable bytes of the next single packet. In the future, we'll either
>add a VMBus API that allows us to know the total readable bytes without
>touching the data in the ringbuffer, or the hv_sock driver needs to
>understand the VMBus packet format and parse the packets directly.
>
>Signed-off-by: Dexuan Cui <decui@microsoft.com>
Your S-o-b was fine here, I was talking about the author.
Thanks,
Stefano
>---
> net/vmw_vsock/hyperv_transport.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 31342ab502b4fc35feb812d2c94e0e35ded73771..432fcbbd14d4f44bd2550be8376e42ce65122758 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -694,15 +694,26 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> {
> struct hvsock *hvs = vsk->trans;
>+ bool need_refill;
> s64 ret;
>
> if (hvs->recv_data_len > 0)
>- return 1;
>+ return hvs->recv_data_len;
>
> switch (hvs_channel_readable_payload(hvs->chan)) {
> case 1:
>- ret = 1;
>- break;
>+ need_refill = !hvs->recv_desc;
>+ if (!need_refill)
>+ return -EIO;
>+
>+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
>+ if (!hvs->recv_desc)
>+ return -ENOBUFS;
>+
>+ ret = hvs_update_recv_data(hvs);
>+ if (ret)
>+ return ret;
>+ return hvs->recv_data_len;
> case 0:
> vsk->peer_shutdown |= SEND_SHUTDOWN;
> ret = 0;
>
>--
>2.34.1
>
On 2025/7/7 21:40, Stefano Garzarella wrote:
> On Sun, Jul 06, 2025 at 12:36:29PM +0800, Xuewei Niu wrote:
>
> Again, this patch should have `From: Dexuan Cui <decui@microsoft.com>` on first line, and this is done automatically by `git format-patch` (or others tools) if the author is the right one in your branch.
> I'm not sure what is going on on your side, but you should avoid to reset the original author.
> Applying this patch we will have:
>
> commit ed36075e04ecbb1dd02a3d8eba5bfac6469d73e4
> Author: Xuewei Niu <niuxuewei97@gmail.com>
> Date: Sun Jul 6 12:36:29 2025 +0800
>
> hv_sock: Return the readable bytes in hvs_stream_has_data()
>
> This is not what we want, right?
>
>> When hv_sock was originally added, __vsock_stream_recvmsg() and
>> vsock_stream_has_data() actually only needed to know whether there
>> is any readable data or not, so hvs_stream_has_data() was written to
>> return 1 or 0 for simplicity.
>>
>> However, now hvs_stream_has_data() should return the readable bytes
>> because vsock_data_ready() -> vsock_stream_has_data() needs to know the
>> actual bytes rather than a boolean value of 1 or 0.
>>
>> The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
>> the readable bytes.
>>
>> Let hvs_stream_has_data() return the readable bytes of the payload in
>> the next host-to-guest VMBus hv_sock packet.
>>
>> Note: there may be multpile incoming hv_sock packets pending in the
>> VMBus channel's ringbuffer, but so far there is not a VMBus API that
>> allows us to know all the readable bytes in total without reading and
>> caching the payload of the multiple packets, so let's just return the
>> readable bytes of the next single packet. In the future, we'll either
>> add a VMBus API that allows us to know the total readable bytes without
>> touching the data in the ringbuffer, or the hv_sock driver needs to
>> understand the VMBus packet format and parse the packets directly.
>>
>> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>
> Your S-o-b was fine here, I was talking about the author.
I misunderstood the meaning of author. Sorry about that.
Will update this in v6.
Thanks,
Xuewei
> Thanks,
> Stefano
>
>> ---
>> net/vmw_vsock/hyperv_transport.c | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index 31342ab502b4fc35feb812d2c94e0e35ded73771..432fcbbd14d4f44bd2550be8376e42ce65122758 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -694,15 +694,26 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
>> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>> {
>> struct hvsock *hvs = vsk->trans;
>> + bool need_refill;
>> s64 ret;
>>
>> if (hvs->recv_data_len > 0)
>> - return 1;
>> + return hvs->recv_data_len;
>>
>> switch (hvs_channel_readable_payload(hvs->chan)) {
>> case 1:
>> - ret = 1;
>> - break;
>> + need_refill = !hvs->recv_desc;
>> + if (!need_refill)
>> + return -EIO;
>> +
>> + hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
>> + if (!hvs->recv_desc)
>> + return -ENOBUFS;
>> +
>> + ret = hvs_update_recv_data(hvs);
>> + if (ret)
>> + return ret;
>> + return hvs->recv_data_len;
>> case 0:
>> vsk->peer_shutdown |= SEND_SHUTDOWN;
>> ret = 0;
>>
>> --
>> 2.34.1
>>
© 2016 - 2025 Red Hat, Inc.