diff mbox series

[net-next,v9,3/4] vsock: update SO_RCVLOWAT setting callback

Message ID 20231214091947.395892-4-avkrasnov@salutedevices.com (mailing list archive)
State New, archived
Headers show
Series send credit update during setting SO_RCVLOWAT | expand

Commit Message

Arseniy Krasnov Dec. 14, 2023, 9:19 a.m. UTC 
Do not return if transport callback for SO_RCVLOWAT is set (only in
error case). In this case we don't need to set 'sk_rcvlowat' field in
each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat'
is now set only in af_vsock.c, change callback name from 'set_rcvlowat'
to 'notify_set_rcvlowat'.

Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 Changelog:
 v3 -> v4:
  * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'.
  * Commit message updated.

 include/net/af_vsock.h           | 2 +-
 net/vmw_vsock/af_vsock.c         | 9 +++++++--
 net/vmw_vsock/hyperv_transport.c | 4 ++--
 3 files changed, 10 insertions(+), 5 deletions(-)

Comments

Michael S. Tsirkin Dec. 14, 2023, 10:29 a.m. UTC | #1 
On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote:
> Do not return if transport callback for SO_RCVLOWAT is set (only in
> error case). In this case we don't need to set 'sk_rcvlowat' field in
> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat'
> is now set only in af_vsock.c, change callback name from 'set_rcvlowat'
> to 'notify_set_rcvlowat'.
> 
> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Maybe squash this with patch 2/4?

> ---
>  Changelog:
>  v3 -> v4:
>   * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'.
>   * Commit message updated.
> 
>  include/net/af_vsock.h           | 2 +-
>  net/vmw_vsock/af_vsock.c         | 9 +++++++--
>  net/vmw_vsock/hyperv_transport.c | 4 ++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index e302c0e804d0..535701efc1e5 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -137,7 +137,6 @@ 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 *vsk, int val);
>  
>  	/* SEQ_PACKET. */
>  	ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
> @@ -168,6 +167,7 @@ struct vsock_transport {
>  		struct vsock_transport_send_notify_data *);
>  	/* sk_lock held by the caller */
>  	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
> +	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
>  
>  	/* Shutdown. */
>  	int (*shutdown)(struct vsock_sock *, int);
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 816725af281f..54ba7316f808 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val)
>  
>  	transport = vsk->transport;
>  
> -	if (transport && transport->set_rcvlowat)
> -		return transport->set_rcvlowat(vsk, val);
> +	if (transport && transport->notify_set_rcvlowat) {
> +		int err;
> +
> +		err = transport->notify_set_rcvlowat(vsk, val);
> +		if (err)
> +			return err;
> +	}
>  
>  	WRITE_ONCE(sk->sk_rcvlowat, val ? : 1);
>  	return 0;



I would s

> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 7cb1a9d2cdb4..e2157e387217 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written,
>  }
>  
>  static
> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val)
> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
>  {
>  	return -EOPNOTSUPP;
>  }
> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = {
>  	.notify_send_pre_enqueue  = hvs_notify_send_pre_enqueue,
>  	.notify_send_post_enqueue = hvs_notify_send_post_enqueue,
>  
> -	.set_rcvlowat             = hvs_set_rcvlowat
> +	.notify_set_rcvlowat      = hvs_notify_set_rcvlowat
>  };
>  
>  static bool hvs_check_transport(struct vsock_sock *vsk)
> -- 
> 2.25.1
Arseniy Krasnov Dec. 14, 2023, 10:52 a.m. UTC | #2 
On 14.12.2023 13:29, Michael S. Tsirkin wrote:
> On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote:
>> Do not return if transport callback for SO_RCVLOWAT is set (only in
>> error case). In this case we don't need to set 'sk_rcvlowat' field in
>> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat'
>> is now set only in af_vsock.c, change callback name from 'set_rcvlowat'
>> to 'notify_set_rcvlowat'.
>>
>> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Maybe squash this with patch 2/4?

You mean just do 'git squash' without updating commit message manually?

Thanks, Arseniy

> 
>> ---
>>  Changelog:
>>  v3 -> v4:
>>   * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'.
>>   * Commit message updated.
>>
>>  include/net/af_vsock.h           | 2 +-
>>  net/vmw_vsock/af_vsock.c         | 9 +++++++--
>>  net/vmw_vsock/hyperv_transport.c | 4 ++--
>>  3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>> index e302c0e804d0..535701efc1e5 100644
>> --- a/include/net/af_vsock.h
>> +++ b/include/net/af_vsock.h
>> @@ -137,7 +137,6 @@ 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 *vsk, int val);
>>  
>>  	/* SEQ_PACKET. */
>>  	ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
>> @@ -168,6 +167,7 @@ struct vsock_transport {
>>  		struct vsock_transport_send_notify_data *);
>>  	/* sk_lock held by the caller */
>>  	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
>> +	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
>>  
>>  	/* Shutdown. */
>>  	int (*shutdown)(struct vsock_sock *, int);
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 816725af281f..54ba7316f808 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val)
>>  
>>  	transport = vsk->transport;
>>  
>> -	if (transport && transport->set_rcvlowat)
>> -		return transport->set_rcvlowat(vsk, val);
>> +	if (transport && transport->notify_set_rcvlowat) {
>> +		int err;
>> +
>> +		err = transport->notify_set_rcvlowat(vsk, val);
>> +		if (err)
>> +			return err;
>> +	}
>>  
>>  	WRITE_ONCE(sk->sk_rcvlowat, val ? : 1);
>>  	return 0;
> 
> 
> 
> I would s
> 
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index 7cb1a9d2cdb4..e2157e387217 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written,
>>  }
>>  
>>  static
>> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val)
>> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
>>  {
>>  	return -EOPNOTSUPP;
>>  }
>> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = {
>>  	.notify_send_pre_enqueue  = hvs_notify_send_pre_enqueue,
>>  	.notify_send_post_enqueue = hvs_notify_send_post_enqueue,
>>  
>> -	.set_rcvlowat             = hvs_set_rcvlowat
>> +	.notify_set_rcvlowat      = hvs_notify_set_rcvlowat
>>  };
>>  
>>  static bool hvs_check_transport(struct vsock_sock *vsk)
>> -- 
>> 2.25.1
>
Michael S. Tsirkin Dec. 14, 2023, 11:17 a.m. UTC | #3 
On Thu, Dec 14, 2023 at 01:52:50PM +0300, Arseniy Krasnov wrote:
> 
> 
> On 14.12.2023 13:29, Michael S. Tsirkin wrote:
> > On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote:
> >> Do not return if transport callback for SO_RCVLOWAT is set (only in
> >> error case). In this case we don't need to set 'sk_rcvlowat' field in
> >> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat'
> >> is now set only in af_vsock.c, change callback name from 'set_rcvlowat'
> >> to 'notify_set_rcvlowat'.
> >>
> >> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> >> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Maybe squash this with patch 2/4?
> 
> You mean just do 'git squash' without updating commit message manually?
> 
> Thanks, Arseniy

commit message should reflect all that patch does.

> > 
> >> ---
> >>  Changelog:
> >>  v3 -> v4:
> >>   * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'.
> >>   * Commit message updated.
> >>
> >>  include/net/af_vsock.h           | 2 +-
> >>  net/vmw_vsock/af_vsock.c         | 9 +++++++--
> >>  net/vmw_vsock/hyperv_transport.c | 4 ++--
> >>  3 files changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >> index e302c0e804d0..535701efc1e5 100644
> >> --- a/include/net/af_vsock.h
> >> +++ b/include/net/af_vsock.h
> >> @@ -137,7 +137,6 @@ 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 *vsk, int val);
> >>  
> >>  	/* SEQ_PACKET. */
> >>  	ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
> >> @@ -168,6 +167,7 @@ struct vsock_transport {
> >>  		struct vsock_transport_send_notify_data *);
> >>  	/* sk_lock held by the caller */
> >>  	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
> >> +	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
> >>  
> >>  	/* Shutdown. */
> >>  	int (*shutdown)(struct vsock_sock *, int);
> >> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >> index 816725af281f..54ba7316f808 100644
> >> --- a/net/vmw_vsock/af_vsock.c
> >> +++ b/net/vmw_vsock/af_vsock.c
> >> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val)
> >>  
> >>  	transport = vsk->transport;
> >>  
> >> -	if (transport && transport->set_rcvlowat)
> >> -		return transport->set_rcvlowat(vsk, val);
> >> +	if (transport && transport->notify_set_rcvlowat) {
> >> +		int err;
> >> +
> >> +		err = transport->notify_set_rcvlowat(vsk, val);
> >> +		if (err)
> >> +			return err;
> >> +	}
> >>  
> >>  	WRITE_ONCE(sk->sk_rcvlowat, val ? : 1);
> >>  	return 0;
> > 
> > 
> > 
> > I would s
> > 
> >> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> >> index 7cb1a9d2cdb4..e2157e387217 100644
> >> --- a/net/vmw_vsock/hyperv_transport.c
> >> +++ b/net/vmw_vsock/hyperv_transport.c
> >> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written,
> >>  }
> >>  
> >>  static
> >> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val)
> >> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> >>  {
> >>  	return -EOPNOTSUPP;
> >>  }
> >> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = {
> >>  	.notify_send_pre_enqueue  = hvs_notify_send_pre_enqueue,
> >>  	.notify_send_post_enqueue = hvs_notify_send_post_enqueue,
> >>  
> >> -	.set_rcvlowat             = hvs_set_rcvlowat
> >> +	.notify_set_rcvlowat      = hvs_notify_set_rcvlowat
> >>  };
> >>  
> >>  static bool hvs_check_transport(struct vsock_sock *vsk)
> >> -- 
> >> 2.25.1
> >
Arseniy Krasnov Dec. 14, 2023, 12:52 p.m. UTC | #4 
On 14.12.2023 13:29, Michael S. Tsirkin wrote:
> On Thu, Dec 14, 2023 at 12:19:46PM +0300, Arseniy Krasnov wrote:
>> Do not return if transport callback for SO_RCVLOWAT is set (only in
>> error case). In this case we don't need to set 'sk_rcvlowat' field in
>> each transport - only in 'vsock_set_rcvlowat()'. Also, if 'sk_rcvlowat'
>> is now set only in af_vsock.c, change callback name from 'set_rcvlowat'
>> to 'notify_set_rcvlowat'.
>>
>> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Maybe squash this with patch 2/4?

Done in v10

Thanks, Arseniy

> 
>> ---
>>  Changelog:
>>  v3 -> v4:
>>   * Rename 'set_rcvlowat' to 'notify_set_rcvlowat'.
>>   * Commit message updated.
>>
>>  include/net/af_vsock.h           | 2 +-
>>  net/vmw_vsock/af_vsock.c         | 9 +++++++--
>>  net/vmw_vsock/hyperv_transport.c | 4 ++--
>>  3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>> index e302c0e804d0..535701efc1e5 100644
>> --- a/include/net/af_vsock.h
>> +++ b/include/net/af_vsock.h
>> @@ -137,7 +137,6 @@ 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 *vsk, int val);
>>  
>>  	/* SEQ_PACKET. */
>>  	ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
>> @@ -168,6 +167,7 @@ struct vsock_transport {
>>  		struct vsock_transport_send_notify_data *);
>>  	/* sk_lock held by the caller */
>>  	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
>> +	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
>>  
>>  	/* Shutdown. */
>>  	int (*shutdown)(struct vsock_sock *, int);
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 816725af281f..54ba7316f808 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2264,8 +2264,13 @@ static int vsock_set_rcvlowat(struct sock *sk, int val)
>>  
>>  	transport = vsk->transport;
>>  
>> -	if (transport && transport->set_rcvlowat)
>> -		return transport->set_rcvlowat(vsk, val);
>> +	if (transport && transport->notify_set_rcvlowat) {
>> +		int err;
>> +
>> +		err = transport->notify_set_rcvlowat(vsk, val);
>> +		if (err)
>> +			return err;
>> +	}
>>  
>>  	WRITE_ONCE(sk->sk_rcvlowat, val ? : 1);
>>  	return 0;
> 
> 
> 
> I would s
> 
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index 7cb1a9d2cdb4..e2157e387217 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -816,7 +816,7 @@ int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written,
>>  }
>>  
>>  static
>> -int hvs_set_rcvlowat(struct vsock_sock *vsk, int val)
>> +int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
>>  {
>>  	return -EOPNOTSUPP;
>>  }
>> @@ -856,7 +856,7 @@ static struct vsock_transport hvs_transport = {
>>  	.notify_send_pre_enqueue  = hvs_notify_send_pre_enqueue,
>>  	.notify_send_post_enqueue = hvs_notify_send_post_enqueue,
>>  
>> -	.set_rcvlowat             = hvs_set_rcvlowat
>> +	.notify_set_rcvlowat      = hvs_notify_set_rcvlowat
>>  };
>>  
>>  static bool hvs_check_transport(struct vsock_sock *vsk)
>> -- 
>> 2.25.1
>
diff mbox series

Patch

diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index e302c0e804d0..535701efc1e5 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -137,7 +137,6 @@  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 *vsk, int val);
 
 	/* SEQ_PACKET. */
 	ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
@@ -168,6 +167,7 @@  struct vsock_transport {
 		struct vsock_transport_send_notify_data *);
 	/* sk_lock held by the caller */
 	void (*notify_buffer_size)(struct vsock_sock *, u64 *);
+	int (*notify_set_rcvlowat)(struct vsock_sock *vsk, int val);
 
 	/* Shutdown. */
 	int (*shutdown)(struct vsock_sock *, int);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 816725af281f..54ba7316f808 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2264,8 +2264,13 @@  static int vsock_set_rcvlowat(struct sock *sk, int val)
 
 	transport = vsk->transport;
 
-	if (transport && transport->set_rcvlowat)
-		return transport->set_rcvlowat(vsk, val);
+	if (transport && transport->notify_set_rcvlowat) {
+		int err;
+
+		err = transport->notify_set_rcvlowat(vsk, val);
+		if (err)
+			return err;
+	}
 
 	WRITE_ONCE(sk->sk_rcvlowat, val ? : 1);
 	return 0;
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 7cb1a9d2cdb4..e2157e387217 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -816,7 +816,7 @@  int hvs_notify_send_post_enqueue(struct vsock_sock *vsk, ssize_t written,
 }
 
 static
-int hvs_set_rcvlowat(struct vsock_sock *vsk, int val)
+int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
 {
 	return -EOPNOTSUPP;
 }
@@ -856,7 +856,7 @@  static struct vsock_transport hvs_transport = {
 	.notify_send_pre_enqueue  = hvs_notify_send_pre_enqueue,
 	.notify_send_post_enqueue = hvs_notify_send_post_enqueue,
 
-	.set_rcvlowat             = hvs_set_rcvlowat
+	.notify_set_rcvlowat      = hvs_notify_set_rcvlowat
 };
 
 static bool hvs_check_transport(struct vsock_sock *vsk)