| Message ID | 20230618062451.79980-3-AVKrasnov@sberdevices.ru (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio/vsock: some updates for MSG_PEEK flag | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 8 this patch: 8 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 75 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Sun, Jun 18, 2023 at 09:24:49AM +0300, Arseniy Krasnov wrote: >This adds support of MSG_PEEK flag for SOCK_SEQPACKET type of socket. >Difference with SOCK_STREAM is that this callback returns either length >of the message or error. > >Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >--- > net/vmw_vsock/virtio_transport_common.c | 63 +++++++++++++++++++++++-- > 1 file changed, 60 insertions(+), 3 deletions(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index 2ee40574c339..352d042b130b 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -460,6 +460,63 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, > return err; > } > >+static ssize_t >+virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk, >+ struct msghdr *msg) >+{ >+ struct virtio_vsock_sock *vvs = vsk->trans; >+ struct sk_buff *skb; >+ size_t total, len; >+ >+ spin_lock_bh(&vvs->rx_lock); >+ >+ if (!vvs->msg_count) { >+ spin_unlock_bh(&vvs->rx_lock); >+ return 0; >+ } >+ >+ total = 0; >+ len = msg_data_left(msg); >+ >+ skb_queue_walk(&vvs->rx_queue, skb) { >+ struct virtio_vsock_hdr *hdr; >+ >+ if (total < len) { >+ size_t bytes; >+ int err; >+ >+ bytes = len - total; >+ if (bytes > skb->len) >+ bytes = skb->len; >+ >+ spin_unlock_bh(&vvs->rx_lock); >+ >+ /* sk_lock is held by caller so no one else can dequeue. >+ * Unlock rx_lock since memcpy_to_msg() may sleep. >+ */ >+ err = memcpy_to_msg(msg, skb->data, bytes); >+ if (err) >+ return err; >+ >+ spin_lock_bh(&vvs->rx_lock); >+ } >+ >+ total += skb->len; >+ hdr = virtio_vsock_hdr(skb); >+ >+ if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) { >+ if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR) >+ msg->msg_flags |= MSG_EOR; >+ >+ break; >+ } >+ } >+ >+ spin_unlock_bh(&vvs->rx_lock); >+ >+ return total; Should we return the minimum between total and len? Thanks, Stefano >+} >+ > static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, > struct msghdr *msg, > int flags) >@@ -554,9 +611,9 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk, > int flags) > { > if (flags & MSG_PEEK) >- return -EOPNOTSUPP; >- >- return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); >+ return virtio_transport_seqpacket_do_peek(vsk, msg); >+ else >+ return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); > } > EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_dequeue); > >-- >2.25.1 >
On 26.06.2023 19:28, Stefano Garzarella wrote: > On Sun, Jun 18, 2023 at 09:24:49AM +0300, Arseniy Krasnov wrote: >> This adds support of MSG_PEEK flag for SOCK_SEQPACKET type of socket. >> Difference with SOCK_STREAM is that this callback returns either length >> of the message or error. >> >> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >> --- >> net/vmw_vsock/virtio_transport_common.c | 63 +++++++++++++++++++++++-- >> 1 file changed, 60 insertions(+), 3 deletions(-) >> >> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >> index 2ee40574c339..352d042b130b 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -460,6 +460,63 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, >> return err; >> } >> >> +static ssize_t >> +virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk, >> + struct msghdr *msg) >> +{ >> + struct virtio_vsock_sock *vvs = vsk->trans; >> + struct sk_buff *skb; >> + size_t total, len; >> + >> + spin_lock_bh(&vvs->rx_lock); >> + >> + if (!vvs->msg_count) { >> + spin_unlock_bh(&vvs->rx_lock); >> + return 0; >> + } >> + >> + total = 0; >> + len = msg_data_left(msg); >> + >> + skb_queue_walk(&vvs->rx_queue, skb) { >> + struct virtio_vsock_hdr *hdr; >> + >> + if (total < len) { >> + size_t bytes; >> + int err; >> + >> + bytes = len - total; >> + if (bytes > skb->len) >> + bytes = skb->len; >> + >> + spin_unlock_bh(&vvs->rx_lock); >> + >> + /* sk_lock is held by caller so no one else can dequeue. >> + * Unlock rx_lock since memcpy_to_msg() may sleep. >> + */ >> + err = memcpy_to_msg(msg, skb->data, bytes); >> + if (err) >> + return err; >> + >> + spin_lock_bh(&vvs->rx_lock); >> + } >> + >> + total += skb->len; >> + hdr = virtio_vsock_hdr(skb); >> + >> + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) { >> + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR) >> + msg->msg_flags |= MSG_EOR; >> + >> + break; >> + } >> + } >> + >> + spin_unlock_bh(&vvs->rx_lock); >> + >> + return total; > > Should we return the minimum between total and len? I guess no, because seqpacket dequeue callback always returns length of message, then, in af_vsock.c we return either number of bytes read or length of message depending on MSG_TRUNC flags. Thanks, Arseniy > > Thanks, > Stefano > >> +} >> + >> static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, >> struct msghdr *msg, >> int flags) >> @@ -554,9 +611,9 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk, >> int flags) >> { >> if (flags & MSG_PEEK) >> - return -EOPNOTSUPP; >> - >> - return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); >> + return virtio_transport_seqpacket_do_peek(vsk, msg); >> + else >> + return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); >> } >> EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_dequeue); >> >> -- >> 2.25.1 >> >
On Tue, Jun 27, 2023 at 07:34:29AM +0300, Arseniy Krasnov wrote: > > >On 26.06.2023 19:28, Stefano Garzarella wrote: >> On Sun, Jun 18, 2023 at 09:24:49AM +0300, Arseniy Krasnov wrote: >>> This adds support of MSG_PEEK flag for SOCK_SEQPACKET type of socket. >>> Difference with SOCK_STREAM is that this callback returns either length >>> of the message or error. >>> >>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >>> --- >>> net/vmw_vsock/virtio_transport_common.c | 63 +++++++++++++++++++++++-- >>> 1 file changed, 60 insertions(+), 3 deletions(-) >>> >>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >>> index 2ee40574c339..352d042b130b 100644 >>> --- a/net/vmw_vsock/virtio_transport_common.c >>> +++ b/net/vmw_vsock/virtio_transport_common.c >>> @@ -460,6 +460,63 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, >>> return err; >>> } >>> >>> +static ssize_t >>> +virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk, >>> + struct msghdr *msg) >>> +{ >>> + struct virtio_vsock_sock *vvs = vsk->trans; >>> + struct sk_buff *skb; >>> + size_t total, len; >>> + >>> + spin_lock_bh(&vvs->rx_lock); >>> + >>> + if (!vvs->msg_count) { >>> + spin_unlock_bh(&vvs->rx_lock); >>> + return 0; >>> + } >>> + >>> + total = 0; >>> + len = msg_data_left(msg); >>> + >>> + skb_queue_walk(&vvs->rx_queue, skb) { >>> + struct virtio_vsock_hdr *hdr; >>> + >>> + if (total < len) { >>> + size_t bytes; >>> + int err; >>> + >>> + bytes = len - total; >>> + if (bytes > skb->len) >>> + bytes = skb->len; >>> + >>> + spin_unlock_bh(&vvs->rx_lock); >>> + >>> + /* sk_lock is held by caller so no one else can dequeue. >>> + * Unlock rx_lock since memcpy_to_msg() may sleep. >>> + */ >>> + err = memcpy_to_msg(msg, skb->data, bytes); >>> + if (err) >>> + return err; >>> + >>> + spin_lock_bh(&vvs->rx_lock); >>> + } >>> + >>> + total += skb->len; >>> + hdr = virtio_vsock_hdr(skb); >>> + >>> + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) { >>> + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR) >>> + msg->msg_flags |= MSG_EOR; >>> + >>> + break; >>> + } >>> + } >>> + >>> + spin_unlock_bh(&vvs->rx_lock); >>> + >>> + return total; >> >> Should we return the minimum between total and len? > >I guess no, because seqpacket dequeue callback always returns length of message, >then, in af_vsock.c we return either number of bytes read or length of message >depending on MSG_TRUNC flags. Right! We should always return the total lenght of the packet. Thanks, Stefano > >Thanks, Arseniy > >> >> Thanks, >> Stefano >> >>> +} >>> + >>> static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, >>> struct msghdr *msg, >>> int flags) >>> @@ -554,9 +611,9 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk, >>> int flags) >>> { >>> if (flags & MSG_PEEK) >>> - return -EOPNOTSUPP; >>> - >>> - return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); >>> + return virtio_transport_seqpacket_do_peek(vsk, msg); >>> + else >>> + return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); >>> } >>> EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_dequeue); >>> >>> -- >>> 2.25.1 >>> >> >
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 2ee40574c339..352d042b130b 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -460,6 +460,63 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, return err; } +static ssize_t +virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk, + struct msghdr *msg) +{ + struct virtio_vsock_sock *vvs = vsk->trans; + struct sk_buff *skb; + size_t total, len; + + spin_lock_bh(&vvs->rx_lock); + + if (!vvs->msg_count) { + spin_unlock_bh(&vvs->rx_lock); + return 0; + } + + total = 0; + len = msg_data_left(msg); + + skb_queue_walk(&vvs->rx_queue, skb) { + struct virtio_vsock_hdr *hdr; + + if (total < len) { + size_t bytes; + int err; + + bytes = len - total; + if (bytes > skb->len) + bytes = skb->len; + + spin_unlock_bh(&vvs->rx_lock); + + /* sk_lock is held by caller so no one else can dequeue. + * Unlock rx_lock since memcpy_to_msg() may sleep. + */ + err = memcpy_to_msg(msg, skb->data, bytes); + if (err) + return err; + + spin_lock_bh(&vvs->rx_lock); + } + + total += skb->len; + hdr = virtio_vsock_hdr(skb); + + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) { + if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR) + msg->msg_flags |= MSG_EOR; + + break; + } + } + + spin_unlock_bh(&vvs->rx_lock); + + return total; +} + static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, struct msghdr *msg, int flags) @@ -554,9 +611,9 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk, int flags) { if (flags & MSG_PEEK) - return -EOPNOTSUPP; - - return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); + return virtio_transport_seqpacket_do_peek(vsk, msg); + else + return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags); } EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_dequeue);
This adds support of MSG_PEEK flag for SOCK_SEQPACKET type of socket. Difference with SOCK_STREAM is that this callback returns either length of the message or error. Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> --- net/vmw_vsock/virtio_transport_common.c | 63 +++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-)