diff mbox series

[RFC,v1,05/16] af_vsock: use SOCK_STREAM function to check data

Message ID 20210628100250.570726-1-arseny.krasnov@kaspersky.com
State New
Headers show
Series Improve SOCK_SEQPACKET receive logic | expand

Commit Message

Arseny Krasnov June 28, 2021, 10:02 a.m. UTC
Also remove 'seqpacket_has_data' callback from transport.

Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
---
 include/net/af_vsock.h   |  1 -
 net/vmw_vsock/af_vsock.c | 12 +-----------
 2 files changed, 1 insertion(+), 12 deletions(-)

Comments

Stefano Garzarella June 30, 2021, 12:10 p.m. UTC | #1
On Mon, Jun 28, 2021 at 01:02:47PM +0300, Arseny Krasnov wrote:
>Also remove 'seqpacket_has_data' callback from transport.

>

>Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>

>---

> include/net/af_vsock.h   |  1 -

> net/vmw_vsock/af_vsock.c | 12 +-----------

> 2 files changed, 1 insertion(+), 12 deletions(-)


In order to avoid issues while bisecting the kernel, we should have
commit that doesn't break the build or the runtime, so please take this
in mind also for other commits.

For example here we removed the seqpacket_has_data callbacks assignment
before to remove where we use the callback, with a potential fault at
runtime.

I think you can simply put patches from 1 to 5 together in a single
patch.

In addition, we should move these changes after we don't need
vsock_connectible_has_data() anymore, for example, where we replace the
receive loop logic.

Thanks,
Stefano

>

>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h

>index ab207677e0a8..bf5ea1873e6f 100644

>--- a/include/net/af_vsock.h

>+++ b/include/net/af_vsock.h

>@@ -141,7 +141,6 @@ struct vsock_transport {

>       int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg,

>                                size_t len);

>       bool (*seqpacket_allow)(u32 remote_cid);

>-      u32 (*seqpacket_has_data)(struct vsock_sock *vsk);

>

>       /* Notification. */

>       int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);

>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c

>index 21ccf450e249..59ce35da2e5b 100644

>--- a/net/vmw_vsock/af_vsock.c

>+++ b/net/vmw_vsock/af_vsock.c

>@@ -860,16 +860,6 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk)

> }

> EXPORT_SYMBOL_GPL(vsock_stream_has_data);

>

>-static s64 vsock_connectible_has_data(struct vsock_sock *vsk)

>-{

>-      struct sock *sk = sk_vsock(vsk);

>-

>-      if (sk->sk_type == SOCK_SEQPACKET)

>-              return vsk->transport->seqpacket_has_data(vsk);

>-      else

>-              return vsock_stream_has_data(vsk);

>-}

>-

> s64 vsock_stream_has_space(struct vsock_sock *vsk)

> {

>       return vsk->transport->stream_has_space(vsk);

>@@ -1881,7 +1871,7 @@ static int vsock_connectible_wait_data(struct

>sock *sk,

>       err = 0;

>       transport = vsk->transport;

>

>-      while ((data = vsock_connectible_has_data(vsk)) == 0) {

>+      while ((data = vsock_stream_has_data(vsk)) == 0) {

>               prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE);

>

>               if (sk->sk_err != 0 ||

>-- 2.25.1

>
Arseny Krasnov June 30, 2021, 5:47 p.m. UTC | #2
On 30.06.2021 15:10, Stefano Garzarella wrote:
> On Mon, Jun 28, 2021 at 01:02:47PM +0300, Arseny Krasnov wrote:

>> Also remove 'seqpacket_has_data' callback from transport.

>>

>> Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>

>> ---

>> include/net/af_vsock.h   |  1 -

>> net/vmw_vsock/af_vsock.c | 12 +-----------

>> 2 files changed, 1 insertion(+), 12 deletions(-)

> In order to avoid issues while bisecting the kernel, we should have

> commit that doesn't break the build or the runtime, so please take this

> in mind also for other commits.

>

> For example here we removed the seqpacket_has_data callbacks assignment

> before to remove where we use the callback, with a potential fault at

> runtime.

>

> I think you can simply put patches from 1 to 5 together in a single

> patch.

>

> In addition, we should move these changes after we don't need

> vsock_connectible_has_data() anymore, for example, where we replace the

> receive loop logic.

>

> Thanks,

> Stefano

Ack
>

>> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h

>> index ab207677e0a8..bf5ea1873e6f 100644

>> --- a/include/net/af_vsock.h

>> +++ b/include/net/af_vsock.h

>> @@ -141,7 +141,6 @@ struct vsock_transport {

>>       int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg,

>>                                size_t len);

>>       bool (*seqpacket_allow)(u32 remote_cid);

>> -      u32 (*seqpacket_has_data)(struct vsock_sock *vsk);

>>

>>       /* Notification. */

>>       int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);

>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c

>> index 21ccf450e249..59ce35da2e5b 100644

>> --- a/net/vmw_vsock/af_vsock.c

>> +++ b/net/vmw_vsock/af_vsock.c

>> @@ -860,16 +860,6 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk)

>> }

>> EXPORT_SYMBOL_GPL(vsock_stream_has_data);

>>

>> -static s64 vsock_connectible_has_data(struct vsock_sock *vsk)

>> -{

>> -      struct sock *sk = sk_vsock(vsk);

>> -

>> -      if (sk->sk_type == SOCK_SEQPACKET)

>> -              return vsk->transport->seqpacket_has_data(vsk);

>> -      else

>> -              return vsock_stream_has_data(vsk);

>> -}

>> -

>> s64 vsock_stream_has_space(struct vsock_sock *vsk)

>> {

>>       return vsk->transport->stream_has_space(vsk);

>> @@ -1881,7 +1871,7 @@ static int vsock_connectible_wait_data(struct

>> sock *sk,

>>       err = 0;

>>       transport = vsk->transport;

>>

>> -      while ((data = vsock_connectible_has_data(vsk)) == 0) {

>> +      while ((data = vsock_stream_has_data(vsk)) == 0) {

>>               prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE);

>>

>>               if (sk->sk_err != 0 ||

>> -- 2.25.1

>>

>
diff mbox series

Patch

diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index ab207677e0a8..bf5ea1873e6f 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -141,7 +141,6 @@  struct vsock_transport {
 	int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg,
 				 size_t len);
 	bool (*seqpacket_allow)(u32 remote_cid);
-	u32 (*seqpacket_has_data)(struct vsock_sock *vsk);
 
 	/* Notification. */
 	int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 21ccf450e249..59ce35da2e5b 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -860,16 +860,6 @@  s64 vsock_stream_has_data(struct vsock_sock *vsk)
 }
 EXPORT_SYMBOL_GPL(vsock_stream_has_data);
 
-static s64 vsock_connectible_has_data(struct vsock_sock *vsk)
-{
-	struct sock *sk = sk_vsock(vsk);
-
-	if (sk->sk_type == SOCK_SEQPACKET)
-		return vsk->transport->seqpacket_has_data(vsk);
-	else
-		return vsock_stream_has_data(vsk);
-}
-
 s64 vsock_stream_has_space(struct vsock_sock *vsk)
 {
 	return vsk->transport->stream_has_space(vsk);
@@ -1881,7 +1871,7 @@  static int vsock_connectible_wait_data(struct sock *sk,
 	err = 0;
 	transport = vsk->transport;
 
-	while ((data = vsock_connectible_has_data(vsk)) == 0) {
+	while ((data = vsock_stream_has_data(vsk)) == 0) {
 		prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE);
 
 		if (sk->sk_err != 0 ||