Messages in this thread | | | Date | Wed, 27 Sep 2023 09:34:33 +0200 | From | Stefano Garzarella <> | Subject | Re: [PATCH net-next v1 02/12] vsock: read from socket's error queue |
| |
On Tue, Sep 26, 2023 at 10:36:58PM +0300, Arseniy Krasnov wrote: > > >On 26.09.2023 15:55, Stefano Garzarella wrote: >> On Fri, Sep 22, 2023 at 08:24:18AM +0300, Arseniy Krasnov wrote: >>> This adds handling of MSG_ERRQUEUE input flag in receive call. This flag >>> is used to read socket's error queue instead of data queue. Possible >>> scenario of error queue usage is receiving completions for transmission >>> with MSG_ZEROCOPY flag. This patch also adds new defines: 'SOL_VSOCK' >>> and 'VSOCK_RECVERR'. >>> >>> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com> >>> --- >>> Changelog: >>> v5(big patchset) -> v1: >>> * R-b tag removed, due to added defines to 'include/uapi/linux/vsock.h'. >>> Both 'SOL_VSOCK' and 'VSOCK_RECVERR' are needed by userspace, so >>> they were placed to 'include/uapi/linux/vsock.h'. At the same time, >>> the same define for 'SOL_VSOCK' was placed to 'include/linux/socket.h'. >>> This is needed because this file contains SOL_XXX defines for different >>> types of socket, so it prevents situation when another new SOL_XXX >>> will use constant 287. >>> >>> include/linux/socket.h | 1 + >>> include/uapi/linux/vsock.h | 9 +++++++++ >>> net/vmw_vsock/af_vsock.c | 6 ++++++ >>> 3 files changed, 16 insertions(+) >>> create mode 100644 include/uapi/linux/vsock.h >>> >>> diff --git a/include/linux/socket.h b/include/linux/socket.h >>> index 39b74d83c7c4..cfcb7e2c3813 100644 >>> --- a/include/linux/socket.h >>> +++ b/include/linux/socket.h >>> @@ -383,6 +383,7 @@ struct ucred { >>> #define SOL_MPTCP 284 >>> #define SOL_MCTP 285 >>> #define SOL_SMC 286 >>> +#define SOL_VSOCK 287 >>> >>> /* IPX options */ >>> #define IPX_TYPE 1 >>> diff --git a/include/uapi/linux/vsock.h b/include/uapi/linux/vsock.h >>> new file mode 100644 >>> index 000000000000..b25c1347a3b8 >>> --- /dev/null >>> +++ b/include/uapi/linux/vsock.h >> >> We already have include/uapi/linux/vm_sockets.h >> >> Should we include these changes there instead of creating a new header? >> >>> @@ -0,0 +1,9 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >>> +#ifndef _UAPI_LINUX_VSOCK_H >>> +#define _UAPI_LINUX_VSOCK_H >>> + >>> +#define SOL_VSOCK 287 >> >> Why we need to re-define this also here? > >Reason of this re-define is that SOL_VSOCK must be exported to userspace, so >i place it to include/uapi/XXX. At the same time include/linux/socket.h contains >constants for SOL_XXX and they goes sequentially in this file (e.g. one by one, >each new value is +1 to the previous). So if I add SOL_VSOCK to include/uapi/XXX >only, it is possible that someone will add new SOL_VERY_NEW_SOCKET == 287 to >include/linux/socket.h in future. I think it is not good that two SOL_XXX will >have same value. > >For example SOL_RDS and SOL_TIPS uses the same approach - there are two same defines: >one in include/uapi/ and another is in include/linux/socket.h
Okay, I was confused, I though socket.h was the uapi one. If others do the same, it's fine.
But why adding a new vsock.h instead of reusing vm_sockets.h?
> >> >> In that case, should we protect with some guards to avoid double >> defines? > >May be: > >in include/linux/socket.h > >#ifndef SOL_VSOCK >#define SOL_VSOCK 287 >#endif > >But not sure...
Nope, let's follow others definition.
Sorry for the confusion ;-)
> >> >>> + >>> +#define VSOCK_RECVERR 1 >>> + >>> +#endif /* _UAPI_LINUX_VSOCK_H */ >>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >>> index d841f4de33b0..4fd11bf34bc7 100644 >>> --- a/net/vmw_vsock/af_vsock.c >>> +++ b/net/vmw_vsock/af_vsock.c >>> @@ -110,6 +110,8 @@ >>> #include <linux/workqueue.h> >>> #include <net/sock.h> >>> #include <net/af_vsock.h> >>> +#include <linux/errqueue.h> >>> +#include <uapi/linux/vsock.h> >>> >>> static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr); >>> static void vsock_sk_destruct(struct sock *sk); >>> @@ -2137,6 +2139,10 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, >>> int err; >>> >>> sk = sock->sk; >>> + >>> + if (unlikely(flags & MSG_ERRQUEUE)) >>> + return sock_recv_errqueue(sk, msg, len, SOL_VSOCK, >>> VSOCK_RECVERR); >>> + >>> vsk = vsock_sk(sk); >>> err = 0; >>> >>> -- >>> 2.25.1 >>> >> >
| |