mbox series

[net-next,v3,0/4] virtio/vsock: some updates for MSG_PEEK flag

Message ID 20230725172912.1659970-1-AVKrasnov@sberdevices.ru (mailing list archive)
Headers show
Series virtio/vsock: some updates for MSG_PEEK flag | expand

Message

Arseniy Krasnov July 25, 2023, 5:29 p.m. UTC
Hello,

This patchset does several things around MSG_PEEK flag support. In
general words it reworks MSG_PEEK test and adds support for this flag
in SOCK_SEQPACKET logic. Here is per-patch description:

1) This is cosmetic change for SOCK_STREAM implementation of MSG_PEEK:
   1) I think there is no need of "safe" mode walk here as there is no
      "unlink" of skbs inside loop (it is MSG_PEEK mode - we don't change
      queue).
   2) Nested while loop is removed: in case of MSG_PEEK we just walk
      over skbs and copy data from each one. I guess this nested loop
      even didn't behave as loop - it always executed just for single
      iteration.

2) This adds MSG_PEEK support for SOCK_SEQPACKET. It could be implemented
   be reworking MSG_PEEK callback for SOCK_STREAM to support SOCK_SEQPACKET
   also, but I think it will be more simple and clear from potential
   bugs to implemented it as separate function thus not mixing logics
   for both types of socket. So I've added it as dedicated function.

3) This is reworked MSG_PEEK test for SOCK_STREAM. Previous version just
   sent single byte, then tried to read it with MSG_PEEK flag, then read
   it in normal way. New version is more complex: now sender uses buffer
   instead of single byte and this buffer is initialized with random
   values. Receiver tests several things:
   1) Read empty socket with MSG_PEEK flag.
   2) Read part of buffer with MSG_PEEK flag.
   3) Read whole buffer with MSG_PEEK flag, then checks that it is same
      as buffer from 2) (limited by size of buffer from 2) of course).
   4) Read whole buffer without any flags, then checks that it is same
      as buffer from 3).

4) This is MSG_PEEK test for SOCK_SEQPACKET. It works in the same way
   as for SOCK_STREAM, except it also checks combination of MSG_TRUNC
   and MSG_PEEK.

Head is:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a5a91f546444940f3d75e2edf3c53b4d235f0557

Link to v1:
https://lore.kernel.org/netdev/20230618062451.79980-1-AVKrasnov@sberdevices.ru/
Link to v2:
https://lore.kernel.org/netdev/20230719192708.1775162-1-AVKrasnov@sberdevices.ru/

Changelog:
 v1 -> v2:
 * Patchset is rebased on the new HEAD of net-next.
 * 0001: R-b tag added.
 * 0003: check return value of 'send()' call. 
 v2 -> v3:
 * Patchset is rebased (and tested) on the new HEAD of net-next.
 * 'RFC' tag is replaced with 'net-next'.
 * Small refactoring in 0004:
   '__test_msg_peek_client()' -> 'test_msg_peek_client()'.
   '__test_msg_peek_server()' -> 'test_msg_peek_server()'.

Arseniy Krasnov (4):
  virtio/vsock: rework MSG_PEEK for SOCK_STREAM
  virtio/vsock: support MSG_PEEK for SOCK_SEQPACKET
  vsock/test: rework MSG_PEEK test for SOCK_STREAM
  vsock/test: MSG_PEEK test for SOCK_SEQPACKET

 net/vmw_vsock/virtio_transport_common.c | 104 +++++++++++++-----
 tools/testing/vsock/vsock_test.c        | 136 ++++++++++++++++++++++--
 2 files changed, 208 insertions(+), 32 deletions(-)

Comments

Michael S. Tsirkin July 26, 2023, 10:02 a.m. UTC | #1
On Tue, Jul 25, 2023 at 08:29:08PM +0300, Arseniy Krasnov wrote:
> Hello,
> 
> This patchset does several things around MSG_PEEK flag support. In
> general words it reworks MSG_PEEK test and adds support for this flag
> in SOCK_SEQPACKET logic. Here is per-patch description:
> 
> 1) This is cosmetic change for SOCK_STREAM implementation of MSG_PEEK:
>    1) I think there is no need of "safe" mode walk here as there is no
>       "unlink" of skbs inside loop (it is MSG_PEEK mode - we don't change
>       queue).
>    2) Nested while loop is removed: in case of MSG_PEEK we just walk
>       over skbs and copy data from each one. I guess this nested loop
>       even didn't behave as loop - it always executed just for single
>       iteration.
> 
> 2) This adds MSG_PEEK support for SOCK_SEQPACKET. It could be implemented
>    be reworking MSG_PEEK callback for SOCK_STREAM to support SOCK_SEQPACKET
>    also, but I think it will be more simple and clear from potential
>    bugs to implemented it as separate function thus not mixing logics
>    for both types of socket. So I've added it as dedicated function.
> 
> 3) This is reworked MSG_PEEK test for SOCK_STREAM. Previous version just
>    sent single byte, then tried to read it with MSG_PEEK flag, then read
>    it in normal way. New version is more complex: now sender uses buffer
>    instead of single byte and this buffer is initialized with random
>    values. Receiver tests several things:
>    1) Read empty socket with MSG_PEEK flag.
>    2) Read part of buffer with MSG_PEEK flag.
>    3) Read whole buffer with MSG_PEEK flag, then checks that it is same
>       as buffer from 2) (limited by size of buffer from 2) of course).
>    4) Read whole buffer without any flags, then checks that it is same
>       as buffer from 3).
> 
> 4) This is MSG_PEEK test for SOCK_SEQPACKET. It works in the same way
>    as for SOCK_STREAM, except it also checks combination of MSG_TRUNC
>    and MSG_PEEK.

Acked-by: Michael S. Tsirkin <mst@redhat.com>



> Head is:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a5a91f546444940f3d75e2edf3c53b4d235f0557
> 
> Link to v1:
> https://lore.kernel.org/netdev/20230618062451.79980-1-AVKrasnov@sberdevices.ru/
> Link to v2:
> https://lore.kernel.org/netdev/20230719192708.1775162-1-AVKrasnov@sberdevices.ru/
> 
> Changelog:
>  v1 -> v2:
>  * Patchset is rebased on the new HEAD of net-next.
>  * 0001: R-b tag added.
>  * 0003: check return value of 'send()' call. 
>  v2 -> v3:
>  * Patchset is rebased (and tested) on the new HEAD of net-next.
>  * 'RFC' tag is replaced with 'net-next'.
>  * Small refactoring in 0004:
>    '__test_msg_peek_client()' -> 'test_msg_peek_client()'.
>    '__test_msg_peek_server()' -> 'test_msg_peek_server()'.
> 
> Arseniy Krasnov (4):
>   virtio/vsock: rework MSG_PEEK for SOCK_STREAM
>   virtio/vsock: support MSG_PEEK for SOCK_SEQPACKET
>   vsock/test: rework MSG_PEEK test for SOCK_STREAM
>   vsock/test: MSG_PEEK test for SOCK_SEQPACKET
> 
>  net/vmw_vsock/virtio_transport_common.c | 104 +++++++++++++-----
>  tools/testing/vsock/vsock_test.c        | 136 ++++++++++++++++++++++--
>  2 files changed, 208 insertions(+), 32 deletions(-)
> 
> -- 
> 2.25.1
patchwork-bot+netdevbpf@kernel.org July 27, 2023, 2 p.m. UTC | #2
Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 25 Jul 2023 20:29:08 +0300 you wrote:
> Hello,
> 
> This patchset does several things around MSG_PEEK flag support. In
> general words it reworks MSG_PEEK test and adds support for this flag
> in SOCK_SEQPACKET logic. Here is per-patch description:
> 
> 1) This is cosmetic change for SOCK_STREAM implementation of MSG_PEEK:
>    1) I think there is no need of "safe" mode walk here as there is no
>       "unlink" of skbs inside loop (it is MSG_PEEK mode - we don't change
>       queue).
>    2) Nested while loop is removed: in case of MSG_PEEK we just walk
>       over skbs and copy data from each one. I guess this nested loop
>       even didn't behave as loop - it always executed just for single
>       iteration.
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/4] virtio/vsock: rework MSG_PEEK for SOCK_STREAM
    https://git.kernel.org/netdev/net-next/c/051e77e33946
  - [net-next,v3,2/4] virtio/vsock: support MSG_PEEK for SOCK_SEQPACKET
    https://git.kernel.org/netdev/net-next/c/a75f501de88e
  - [net-next,v3,3/4] vsock/test: rework MSG_PEEK test for SOCK_STREAM
    https://git.kernel.org/netdev/net-next/c/587ed79f62a7
  - [net-next,v3,4/4] vsock/test: MSG_PEEK test for SOCK_SEQPACKET
    https://git.kernel.org/netdev/net-next/c/8a0697f23e5a

You are awesome, thank you!