Linux Virtualization - Sep 2023 - [PATCH net-next v1 08/12] vsock: enable setting SO_ZEROCOPY

On Tue, Sep 26, 2023 at 10:38:06PM +0300, Arseniy Krasnov
wrote:
>
>
>On 26.09.2023 15:56, Stefano Garzarella wrote:
>> On Fri, Sep 22, 2023 at 08:24:24AM +0300, Arseniy Krasnov wrote:
>>> For AF_VSOCK, zerocopy tx mode depends on transport, so this option
must
>>> be set in AF_VSOCK implementation where transport is accessible (if
>>> transport is not set during setting SO_ZEROCOPY: for example socket
is
>>> not connected, then SO_ZEROCOPY will be enabled, but once transport
will
>>> be assigned, support of this type of transmission will be checked).
>>>
>>> To handle SO_ZEROCOPY, AF_VSOCK implementation uses
SOCK_CUSTOM_SOCKOPT
>>> bit, thus handling SOL_SOCKET option operations, but all of them
except
>>> SO_ZEROCOPY will be forwarded to the generic handler by calling
>>> 'sock_setsockopt()'.
>>>
>>> Signed-off-by: Arseniy Krasnov <avkrasnov at
salutedevices.com>
>>> ---
>>> Changelog:
>>> v5(big patchset) -> v1:
>>> ?* Compact 'if' conditions.
>>> ?* Rename 'zc_val' to 'zerocopy'.
>>> ?* Use 'zerocopy' value directly in
'sock_valbool_flag()', without
>>> ?? ?: operator.
>>> ?* Set 'SOCK_CUSTOM_SOCKOPT' bit for connectible sockets
only, as
>>> ?? suggested by Bobby Eshleman <bobbyeshleman at gmail.com>.
>>>
>>> net/vmw_vsock/af_vsock.c | 46
++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 44 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>>> index 482300eb88e0..c05a42e02a17 100644
>>> --- a/net/vmw_vsock/af_vsock.c
>>> +++ b/net/vmw_vsock/af_vsock.c
>>> @@ -1406,8 +1406,16 @@ static int vsock_connect(struct socket
*sock, struct sockaddr *addr,
>>> ??????????? goto out;
>>> ??????? }
>>>
>>> -??????? if (vsock_msgzerocopy_allow(transport))
>>> +??????? if (vsock_msgzerocopy_allow(transport)) {
>>> ??????????? set_bit(SOCK_SUPPORT_ZC,
&sk->sk_socket->flags);
>>> +??????? } else if (sock_flag(sk, SOCK_ZEROCOPY)) {
>>> +??????????? /* If this option was set before 'connect()',
>>> +???????????? * when transport was unknown, check that this
>>> +???????????? * feature is supported here.
>>> +???????????? */
>>> +??????????? err = -EOPNOTSUPP;
>>> +??????????? goto out;
>>> +??????? }
>>>
>>> ??????? err = vsock_auto_bind(vsk);
>>> ??????? if (err)
>>> @@ -1643,7 +1651,7 @@ static int
vsock_connectible_setsockopt(struct socket *sock,
>>> ????const struct vsock_transport *transport;
>>> ????u64 val;
>>>
>>> -??? if (level != AF_VSOCK)
>>> +??? if (level != AF_VSOCK && level != SOL_SOCKET)
>>> ??????? return -ENOPROTOOPT;
>>>
>>> #define COPY_IN(_v)?????????????????????????????????????? \
>>> @@ -1666,6 +1674,34 @@ static int
vsock_connectible_setsockopt(struct socket *sock,
>>>
>>> ????transport = vsk->transport;
>>>
>>> +??? if (level == SOL_SOCKET) {
>>> +??????? int zerocopy;
>>> +
>>> +??????? if (optname != SO_ZEROCOPY) {
>>> +??????????? release_sock(sk);
>>> +??????????? return sock_setsockopt(sock, level, optname, optval,
optlen);
>>> +??????? }
>>> +
>>> +??????? /* Use 'int' type here, because variable to
>>> +???????? * set this option usually has this type.
>>> +???????? */
>>> +??????? COPY_IN(zerocopy);
>>> +
>>> +??????? if (zerocopy < 0 || zerocopy > 1) {
>>> +??????????? err = -EINVAL;
>>> +??????????? goto exit;
>>> +??????? }
>>> +
>>> +??????? if (transport &&
!vsock_msgzerocopy_allow(transport)) {
>>> +??????????? err = -EOPNOTSUPP;
>>> +??????????? goto exit;
>>> +??????? }
>>> +
>>> +??????? sock_valbool_flag(sk, SOCK_ZEROCOPY,
>>> +????????????????? zerocopy);
>>
>> it's not necessary to wrap this call.
>
>Sorry, what do you mean ?
I mean that can be on the same line:

	sock_valbool_flag(sk, SOCK_ZEROCOPY, zerocopy);

Stefano