* [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock
@ 2015-12-24 14:14 Dexuan Cui
2016-01-02 4:29 ` David Miller
2016-01-05 12:31 ` Vitaly Kuznetsov
0 siblings, 2 replies; 5+ messages in thread
From: Dexuan Cui @ 2015-12-24 14:14 UTC (permalink / raw)
To: gregkh, davem, stephen, netdev, linux-kernel, driverdev-devel,
olaf, apw, jasowang, kys
Cc: pebolle, stefanha, vkuznets, dan.carpenter
To get the payload of hvsock, we need raw=0 to skip the level-1 header
(i.e., struct vmpacket_descriptor desc) and we also need to skip the
level-2 header (i.e., struct vmpipe_proto_header pipe_hdr).
NB: if the length of the hvsock payload is not aligned with the 8-byte
boundeary, at most 7 padding bytes are appended, so the real hvsock
payload's length must be retrieved by the pipe_hdr.data_size field.
I 'upgrade' the 'raw' parameter of hv_ringbuffer_read() to a
'read_flags', trying to share the logic of the function.
This patch is required by the next patch, which will introduce the hvsock
send/recv APIs.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/hv/channel.c | 10 +++++----
drivers/hv/hyperv_vmbus.h | 13 +++++++++++-
drivers/hv/ring_buffer.c | 54 ++++++++++++++++++++++++++++++++++++-----------
include/linux/hyperv.h | 12 +++++++++++
4 files changed, 72 insertions(+), 17 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index eaaa066..cc49966 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -940,13 +940,14 @@ EXPORT_SYMBOL_GPL(vmbus_sendpacket_multipagebuffer);
static inline int
__vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
u32 bufferlen, u32 *buffer_actual_len, u64 *requestid,
- bool raw)
+ u32 read_flags)
{
int ret;
bool signal = false;
ret = hv_ringbuffer_read(&channel->inbound, buffer, bufferlen,
- buffer_actual_len, requestid, &signal, raw);
+ buffer_actual_len, requestid, &signal,
+ read_flags);
if (signal)
vmbus_setevent(channel);
@@ -959,7 +960,7 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
u64 *requestid)
{
return __vmbus_recvpacket(channel, buffer, bufferlen,
- buffer_actual_len, requestid, false);
+ buffer_actual_len, requestid, 0);
}
EXPORT_SYMBOL(vmbus_recvpacket);
@@ -971,6 +972,7 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,
u64 *requestid)
{
return __vmbus_recvpacket(channel, buffer, bufferlen,
- buffer_actual_len, requestid, true);
+ buffer_actual_len, requestid,
+ HV_RINGBUFFER_READ_FLAG_RAW);
}
EXPORT_SYMBOL_GPL(vmbus_recvpacket_raw);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0411b7b..46206b6 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -619,9 +619,20 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info *ring_info,
struct kvec *kv_list,
u32 kv_count, bool *signal);
+/*
+ * By default, a read_flags of 0 means: the payload offset is
+ * sizeof(struct vmpacket_descriptor).
+ *
+ * If HV_RINGBUFFER_READ_FLAG_RAW is used, the payload offset is 0.
+ *
+ * If HV_RINGBUFFER_READ_FLAG_HVSOCK is used, the payload offset is
+ * sizeof(struct vmpacket_descriptor) + sizeof(struct vmpipe_proto_header).
+ */
+#define HV_RINGBUFFER_READ_FLAG_RAW (1 << 0)
+#define HV_RINGBUFFER_READ_FLAG_HVSOCK (1 << 1)
int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
void *buffer, u32 buflen, u32 *buffer_actual_len,
- u64 *requestid, bool *signal, bool raw);
+ u64 *requestid, bool *signal, u32 read_flags);
void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
struct hv_ring_buffer_debug_info *debug_info);
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index b53702c..03a509c 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -382,32 +382,43 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info *outring_info,
int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
void *buffer, u32 buflen, u32 *buffer_actual_len,
- u64 *requestid, bool *signal, bool raw)
+ u64 *requestid, bool *signal, u32 read_flags)
{
+ bool raw = !!(read_flags & HV_RINGBUFFER_READ_FLAG_RAW);
+ bool hvsock = !!(read_flags & HV_RINGBUFFER_READ_FLAG_HVSOCK);
+
u32 bytes_avail_towrite;
u32 bytes_avail_toread;
u32 next_read_location = 0;
u64 prev_indices = 0;
unsigned long flags;
- struct vmpacket_descriptor desc;
+ struct vmpipe_proto_header *pipe_hdr;
+ struct vmpacket_descriptor *desc;
u32 offset;
- u32 packetlen;
+ u32 packetlen, tot_hdrlen;
int ret = 0;
if (buflen <= 0)
return -EINVAL;
+ tot_hdrlen = sizeof(*desc);
+ if (hvsock)
+ tot_hdrlen += sizeof(*pipe_hdr);
+
spin_lock_irqsave(&inring_info->ring_lock, flags);
*buffer_actual_len = 0;
- *requestid = 0;
+
+ /* If some driver doesn't need the info, a NULL is passed in. */
+ if (requestid)
+ *requestid = 0;
hv_get_ringbuffer_availbytes(inring_info,
&bytes_avail_toread,
&bytes_avail_towrite);
/* Make sure there is something to read */
- if (bytes_avail_toread < sizeof(desc)) {
+ if (bytes_avail_toread < tot_hdrlen) {
/*
* No error is set when there is even no header, drivers are
* supposed to analyze buffer_actual_len.
@@ -415,17 +426,26 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
goto out_unlock;
}
+ if (tot_hdrlen > buflen) {
+ ret = -ENOBUFS;
+ goto out_unlock;
+ }
+
+ desc = (struct vmpacket_descriptor *)buffer;
+
next_read_location = hv_get_next_read_location(inring_info);
- next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc,
- sizeof(desc),
+ next_read_location = hv_copyfrom_ringbuffer(inring_info, desc,
+ tot_hdrlen,
next_read_location);
+ offset = 0;
+ if (!raw)
+ offset += (desc->offset8 << 3);
+ if (hvsock)
+ offset += sizeof(*pipe_hdr);
- offset = raw ? 0 : (desc.offset8 << 3);
- packetlen = (desc.len8 << 3) - offset;
- *buffer_actual_len = packetlen;
- *requestid = desc.trans_id;
+ packetlen = (desc->len8 << 3) - offset;
- if (bytes_avail_toread < packetlen + offset) {
+ if (bytes_avail_toread < (desc->len8 << 3)) {
ret = -EAGAIN;
goto out_unlock;
}
@@ -435,6 +455,16 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
goto out_unlock;
}
+ if (requestid)
+ *requestid = desc->trans_id;
+
+ if (!hvsock)
+ *buffer_actual_len = packetlen;
+ else {
+ pipe_hdr = (struct vmpipe_proto_header *)(desc + 1);
+ *buffer_actual_len = pipe_hdr->data_size;
+ }
+
next_read_location =
hv_get_next_readlocation_withoffset(inring_info, offset);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index b835d80..e005223 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1268,4 +1268,16 @@ extern __u32 vmbus_proto_version;
int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
const uuid_le *shv_host_servie_id);
+struct vmpipe_proto_header {
+ u32 pkt_type;
+
+ union {
+ u32 data_size;
+ struct {
+ u16 data_size;
+ u16 offset;
+ } partial;
+ };
+} __packed;
+
#endif /* _HYPERV_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock
2015-12-24 14:14 [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock Dexuan Cui
@ 2016-01-02 4:29 ` David Miller
2016-01-03 11:55 ` Dexuan Cui
2016-01-05 12:31 ` Vitaly Kuznetsov
1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2016-01-02 4:29 UTC (permalink / raw)
To: decui
Cc: gregkh, stephen, netdev, linux-kernel, driverdev-devel, olaf,
apw, jasowang, kys, pebolle, stefanha, vkuznets, dan.carpenter
From: Dexuan Cui <decui@microsoft.com>
Date: Thu, 24 Dec 2015 06:14:36 -0800
> +#define HV_RINGBUFFER_READ_FLAG_RAW (1 << 0)
> +#define HV_RINGBUFFER_READ_FLAG_HVSOCK (1 << 1)
Please use BIT().
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock
2016-01-02 4:29 ` David Miller
@ 2016-01-03 11:55 ` Dexuan Cui
0 siblings, 0 replies; 5+ messages in thread
From: Dexuan Cui @ 2016-01-03 11:55 UTC (permalink / raw)
To: David Miller
Cc: gregkh, stephen, netdev, linux-kernel, driverdev-devel, olaf,
apw, jasowang, KY Srinivasan, pebolle, stefanha, vkuznets,
dan.carpenter
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Saturday, January 2, 2016 12:30
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; stephen@networkplumber.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan <kys@microsoft.com>; pebolle@tiscali.nl;
> stefanha@redhat.com; vkuznets@redhat.com; dan.carpenter@oracle.com
> Subject: Re: [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance
> hv_ringbuffer_read() to support hvsock
>
> From: Dexuan Cui <decui@microsoft.com>
> Date: Thu, 24 Dec 2015 06:14:36 -0800
>
> > +#define HV_RINGBUFFER_READ_FLAG_RAW (1 << 0)
> > +#define HV_RINGBUFFER_READ_FLAG_HVSOCK (1 << 1)
>
> Please use BIT().
Hi David,
Thanks for the suggestion! I'll fix it.
-- Dexuan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock
2015-12-24 14:14 [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock Dexuan Cui
2016-01-02 4:29 ` David Miller
@ 2016-01-05 12:31 ` Vitaly Kuznetsov
2016-01-05 15:41 ` Dexuan Cui
1 sibling, 1 reply; 5+ messages in thread
From: Vitaly Kuznetsov @ 2016-01-05 12:31 UTC (permalink / raw)
To: Dexuan Cui
Cc: gregkh, davem, stephen, netdev, linux-kernel, driverdev-devel,
olaf, apw, jasowang, kys, pebolle, stefanha, dan.carpenter
Dexuan Cui <decui@microsoft.com> writes:
> To get the payload of hvsock, we need raw=0 to skip the level-1 header
> (i.e., struct vmpacket_descriptor desc) and we also need to skip the
> level-2 header (i.e., struct vmpipe_proto_header pipe_hdr).
>
> NB: if the length of the hvsock payload is not aligned with the 8-byte
> boundeary, at most 7 padding bytes are appended, so the real hvsock
> payload's length must be retrieved by the pipe_hdr.data_size field.
>
> I 'upgrade' the 'raw' parameter of hv_ringbuffer_read() to a
> 'read_flags', trying to share the logic of the function.
When I was touching this code last time I was actually thinking about
eliminating 'raw' flag by making all ring reads raw and moving this
header filtering job to the upper layer (as we already have
vmbus_recvpacket()/vmbus_recvpacket_raw()) but for some reason I didn't
do it. I believe you have more or less the same reasoing for introducing
new read type instead of parsing this at a higher level. Some comments
below ...
>
> This patch is required by the next patch, which will introduce the hvsock
> send/recv APIs.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> drivers/hv/channel.c | 10 +++++----
> drivers/hv/hyperv_vmbus.h | 13 +++++++++++-
> drivers/hv/ring_buffer.c | 54 ++++++++++++++++++++++++++++++++++++-----------
> include/linux/hyperv.h | 12 +++++++++++
> 4 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index eaaa066..cc49966 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -940,13 +940,14 @@ EXPORT_SYMBOL_GPL(vmbus_sendpacket_multipagebuffer);
> static inline int
> __vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
> u32 bufferlen, u32 *buffer_actual_len, u64 *requestid,
> - bool raw)
> + u32 read_flags)
> {
> int ret;
> bool signal = false;
>
> ret = hv_ringbuffer_read(&channel->inbound, buffer, bufferlen,
> - buffer_actual_len, requestid, &signal, raw);
> + buffer_actual_len, requestid, &signal,
> + read_flags);
>
> if (signal)
> vmbus_setevent(channel);
> @@ -959,7 +960,7 @@ int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
> u64 *requestid)
> {
> return __vmbus_recvpacket(channel, buffer, bufferlen,
> - buffer_actual_len, requestid, false);
> + buffer_actual_len, requestid, 0);
> }
> EXPORT_SYMBOL(vmbus_recvpacket);
>
> @@ -971,6 +972,7 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,
> u64 *requestid)
> {
> return __vmbus_recvpacket(channel, buffer, bufferlen,
> - buffer_actual_len, requestid, true);
> + buffer_actual_len, requestid,
> + HV_RINGBUFFER_READ_FLAG_RAW);
> }
> EXPORT_SYMBOL_GPL(vmbus_recvpacket_raw);
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 0411b7b..46206b6 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -619,9 +619,20 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info *ring_info,
> struct kvec *kv_list,
> u32 kv_count, bool *signal);
>
> +/*
> + * By default, a read_flags of 0 means: the payload offset is
> + * sizeof(struct vmpacket_descriptor).
> + *
> + * If HV_RINGBUFFER_READ_FLAG_RAW is used, the payload offset is 0.
> + *
> + * If HV_RINGBUFFER_READ_FLAG_HVSOCK is used, the payload offset is
> + * sizeof(struct vmpacket_descriptor) + sizeof(struct
> vmpipe_proto_header).
So these are mutually exclusive, right? Should we introduce 'int
payload_offset' parameter instead of flags?
> + */
> +#define HV_RINGBUFFER_READ_FLAG_RAW (1 << 0)
> +#define HV_RINGBUFFER_READ_FLAG_HVSOCK (1 << 1)
> int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
> void *buffer, u32 buflen, u32 *buffer_actual_len,
> - u64 *requestid, bool *signal, bool raw);
> + u64 *requestid, bool *signal, u32 read_flags);
>
> void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
> struct hv_ring_buffer_debug_info *debug_info);
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index b53702c..03a509c 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -382,32 +382,43 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info *outring_info,
>
> int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
> void *buffer, u32 buflen, u32 *buffer_actual_len,
> - u64 *requestid, bool *signal, bool raw)
> + u64 *requestid, bool *signal, u32 read_flags)
> {
> + bool raw = !!(read_flags & HV_RINGBUFFER_READ_FLAG_RAW);
> + bool hvsock = !!(read_flags & HV_RINGBUFFER_READ_FLAG_HVSOCK);
> +
> u32 bytes_avail_towrite;
> u32 bytes_avail_toread;
> u32 next_read_location = 0;
> u64 prev_indices = 0;
> unsigned long flags;
> - struct vmpacket_descriptor desc;
> + struct vmpipe_proto_header *pipe_hdr;
> + struct vmpacket_descriptor *desc;
> u32 offset;
> - u32 packetlen;
> + u32 packetlen, tot_hdrlen;
> int ret = 0;
>
> if (buflen <= 0)
> return -EINVAL;
>
> + tot_hdrlen = sizeof(*desc);
> + if (hvsock)
> + tot_hdrlen += sizeof(*pipe_hdr);
> +
> spin_lock_irqsave(&inring_info->ring_lock, flags);
>
> *buffer_actual_len = 0;
> - *requestid = 0;
> +
> + /* If some driver doesn't need the info, a NULL is passed in. */
> + if (requestid)
> + *requestid = 0;
>
> hv_get_ringbuffer_availbytes(inring_info,
> &bytes_avail_toread,
> &bytes_avail_towrite);
>
> /* Make sure there is something to read */
> - if (bytes_avail_toread < sizeof(desc)) {
> + if (bytes_avail_toread < tot_hdrlen) {
> /*
> * No error is set when there is even no header, drivers are
> * supposed to analyze buffer_actual_len.
> @@ -415,17 +426,26 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
> goto out_unlock;
> }
>
> + if (tot_hdrlen > buflen) {
> + ret = -ENOBUFS;
> + goto out_unlock;
> + }
> +
> + desc = (struct vmpacket_descriptor *)buffer;
> +
> next_read_location = hv_get_next_read_location(inring_info);
> - next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc,
> - sizeof(desc),
> + next_read_location = hv_copyfrom_ringbuffer(inring_info, desc,
> + tot_hdrlen,
> next_read_location);
> + offset = 0;
> + if (!raw)
> + offset += (desc->offset8 << 3);
> + if (hvsock)
> + offset += sizeof(*pipe_hdr);
So in case of !raw and hvsock we add both offsets?
>
> - offset = raw ? 0 : (desc.offset8 << 3);
> - packetlen = (desc.len8 << 3) - offset;
> - *buffer_actual_len = packetlen;
> - *requestid = desc.trans_id;
> + packetlen = (desc->len8 << 3) - offset;
>
> - if (bytes_avail_toread < packetlen + offset) {
> + if (bytes_avail_toread < (desc->len8 << 3)) {
> ret = -EAGAIN;
> goto out_unlock;
> }
> @@ -435,6 +455,16 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
> goto out_unlock;
> }
>
> + if (requestid)
> + *requestid = desc->trans_id;
> +
> + if (!hvsock)
> + *buffer_actual_len = packetlen;
> + else {
> + pipe_hdr = (struct vmpipe_proto_header *)(desc + 1);
> + *buffer_actual_len = pipe_hdr->data_size;
> + }
> +
> next_read_location =
> hv_get_next_readlocation_withoffset(inring_info, offset);
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index b835d80..e005223 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1268,4 +1268,16 @@ extern __u32 vmbus_proto_version;
>
> int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
> const uuid_le *shv_host_servie_id);
> +struct vmpipe_proto_header {
> + u32 pkt_type;
> +
> + union {
> + u32 data_size;
> + struct {
> + u16 data_size;
> + u16 offset;
> + } partial;
> + };
> +} __packed;
> +
> #endif /* _HYPERV_H */
--
Vitaly
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock
2016-01-05 12:31 ` Vitaly Kuznetsov
@ 2016-01-05 15:41 ` Dexuan Cui
0 siblings, 0 replies; 5+ messages in thread
From: Dexuan Cui @ 2016-01-05 15:41 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: gregkh, davem, stephen, netdev, linux-kernel, driverdev-devel,
olaf, apw, jasowang, KY Srinivasan, pebolle, stefanha,
dan.carpenter
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Tuesday, January 5, 2016 20:31
> ...
> > To get the payload of hvsock, we need raw=0 to skip the level-1 header
> > (i.e., struct vmpacket_descriptor desc) and we also need to skip the
> > level-2 header (i.e., struct vmpipe_proto_header pipe_hdr).
> >
> > NB: if the length of the hvsock payload is not aligned with the 8-byte
> > boundeary, at most 7 padding bytes are appended, so the real hvsock
> > payload's length must be retrieved by the pipe_hdr.data_size field.
> >
> > I 'upgrade' the 'raw' parameter of hv_ringbuffer_read() to a
> > 'read_flags', trying to share the logic of the function.
>
> When I was touching this code last time I was actually thinking about
> eliminating 'raw' flag by making all ring reads raw and moving this
> header filtering job to the upper layer (as we already have
> vmbus_recvpacket()/vmbus_recvpacket_raw()) but for some reason I didn't
> do it. I believe you have more or less the same reasoing for introducing
> new read type instead of parsing this at a higher level. Some comments
> below ...
I feel it's more convenient to do the parsing in the vmbus driver than in
all the driver users of vmbus driver.
However, yes, I admit hv_ringbuffer_read() becomes less readable with
my introduction of 'read_flags'.
It may be a better idea to do the parsing in higher level, i.e., the hvsock driver,
in my case.
It looks I can avoid introducing vmbus_recvpacket_hvsock() and use
vmbus_recvpacket() directly in my hvsock driver.
Let me try to make a new patch this way.
> > This patch is required by the next patch, which will introduce the hvsock
> > send/recv APIs.
> >
> > ...
> > @@ -619,9 +619,20 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info
> *ring_info,
> > struct kvec *kv_list,
> > u32 kv_count, bool *signal);
> >
> > +/*
> > + * By default, a read_flags of 0 means: the payload offset is
> > + * sizeof(struct vmpacket_descriptor).
> > + *
> > + * If HV_RINGBUFFER_READ_FLAG_RAW is used, the payload offset is 0.
> > + *
> > + * If HV_RINGBUFFER_READ_FLAG_HVSOCK is used, the payload offset is
> > + * sizeof(struct vmpacket_descriptor) + sizeof(struct
> > vmpipe_proto_header).
>
> So these are mutually exclusive, right? Should we introduce 'int
> payload_offset' parameter instead of flags?
Sorry for making the code less readable. :-)
As I mentioned above, let me try to do things in a better way.
> > @@ -415,17 +426,26 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info
> *inring_info,
> > goto out_unlock;
> > }
> >
> > + if (tot_hdrlen > buflen) {
> > + ret = -ENOBUFS;
> > + goto out_unlock;
> > + }
> > +
> > + desc = (struct vmpacket_descriptor *)buffer;
> > +
> > next_read_location = hv_get_next_read_location(inring_info);
> > - next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc,
> > - sizeof(desc),
> > + next_read_location = hv_copyfrom_ringbuffer(inring_info, desc,
> > + tot_hdrlen,
> > next_read_location);
> > + offset = 0;
> > + if (!raw)
> > + offset += (desc->offset8 << 3);
> > + if (hvsock)
> > + offset += sizeof(*pipe_hdr);
>
> So in case of !raw and hvsock we add both offsets?
Yes...
Thanks for you review, Vitaly.
Thanks,
-- Dexuan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-05 15:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-24 14:14 [PATCH V5 4/9] Drivers: hv: ring_buffer: enhance hv_ringbuffer_read() to support hvsock Dexuan Cui
2016-01-02 4:29 ` David Miller
2016-01-03 11:55 ` Dexuan Cui
2016-01-05 12:31 ` Vitaly Kuznetsov
2016-01-05 15:41 ` Dexuan Cui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).