On Tue, Aug 09, 2022 at 09:45:47AM +0000, Arseniy Krasnov wrote:
On 09.08.2022 12:37, Arseniy Krasnov wrote:
On 08.08.2022 13:30, Stefano Garzarella wrote:
On Mon, Aug 8, 2022 at 12:23 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
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;
Since hv_sock implements `set_rcvlowat` to return EOPNOTSUPP. maybe we
can just do the following:
if (transport && transport->set_rcvlowat)
return transport->set_rcvlowat(vsk, val);
WRITE_ONCE(sk->sk_rcvlowat, val ? : 1);
return 0;
That is, the default behavior is to set sk->sk_rcvlowat, but for
transports that want a different behavior, they need to define
set_rcvlowat() (like hv_sock).
Hm ok, i see. I've implemented logic when non-empty transport is required, because hyperv transport
forbids to set SO_RCVLOWAT, so user needs to call this setsockopt AFTER transport is assigned(to check
that transport allows it. Not after socket creation as You mentioned above). Otherwise there is no sense
in such callback - it will be never used. Also in code above - for hyperv we will have different behavior
depends on when set_rcvlowat is called: before or after transport assignment. Is it ok?
sorry, i mean: for hyperv, if user sets sk_rcvlowat before transport is assigned, it sees 0 - success, but in fact
hyperv transport forbids this option.
I see, but I think it's better to set it and not respect in hyperv (as
we've practically done until now with all transports) than to prevent
the setting until we assign a transport.
At most when we use hyperv anyway we get notified per byte, so we should
just get more notifications than we expect.
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
