On Wed, Aug 03, 2022 at 01:51:05PM +0000, Arseniy Krasnov wrote:
This adds transport specific callback for SO_RCVLOWAT, because in some transports it may be difficult to know current available number of bytes ready to read. Thus, when SO_RCVLOWAT is set, transport may reject it. Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx> --- include/net/af_vsock.h | 1 + net/vmw_vsock/af_vsock.c | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index f742e50207fb..eae5874bae35 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -134,6 +134,7 @@ struct vsock_transport { u64 (*stream_rcvhiwat)(struct vsock_sock *); bool (*stream_is_active)(struct vsock_sock *); bool (*stream_allow)(u32 cid, u32 port); + int (*set_rcvlowat)(struct vsock_sock *, int);
checkpatch suggests to add identifier names. For some we put them in, for others we didn't, but I suggest putting them in for the new ones because I think it's clearer too.
WARNING: function definition argument 'struct vsock_sock *' should also have an identifier name
#25: FILE: include/net/af_vsock.h:137: + int (*set_rcvlowat)(struct vsock_sock *, int); WARNING: function definition argument 'int' should also have an identifier name #25: FILE: include/net/af_vsock.h:137: + int (*set_rcvlowat)(struct vsock_sock *, int); total: 0 errors, 2 warnings, 0 checks, 44 lines checked
/* SEQ_PACKET. */ ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index f04abf662ec6..016ad5ff78b7 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -2129,6 +2129,30 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, return err; } +static int vsock_set_rcvlowat(struct sock *sk, int val) +{ + const struct vsock_transport *transport; + struct vsock_sock *vsk; + int err = 0; + + vsk = vsock_sk(sk); + + if (val > vsk->buffer_size) + return -EINVAL; + + transport = vsk->transport; + + if (!transport) + return -EOPNOTSUPP;
I don't know whether it is better in this case to write it in sk->sk_rcvlowat, maybe we can return EOPNOTSUPP only when the trasport is assigned and set_rcvlowat is not defined. This is because usually the options are set just after creation, when the transport is practically unassigned.
I mean something like this: if (transport) { if (transport->set_rcvlowat) return transport->set_rcvlowat(vsk, val); else return -EOPNOTSUPP; } WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); return 0;
+ + if (transport->set_rcvlowat) + err = transport->set_rcvlowat(vsk, val); + else + WRITE_ONCE(sk->sk_rcvlowat, val ? : 1); + + return err; +} + static const struct proto_ops vsock_stream_ops = { .family = PF_VSOCK, .owner = THIS_MODULE, @@ -2148,6 +2172,7 @@ static const struct proto_ops vsock_stream_ops = { .recvmsg = vsock_connectible_recvmsg, .mmap = sock_no_mmap, .sendpage = sock_no_sendpage, + .set_rcvlowat = vsock_set_rcvlowat, }; static const struct proto_ops vsock_seqpacket_ops = { -- 2.25.1