| Message ID | 20210207151906.806343-1-arseny.krasnov@kaspersky.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio/vsock: introduce SOCK_SEQPACKET support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 2 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 59 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 2 this patch: 2 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Sun, Feb 07, 2021 at 06:19:03PM +0300, Arseny Krasnov wrote: >'virtio_transport_send_credit_update()' has some extra args: >1) 'type' may be set in 'virtio_transport_send_pkt_info()' using type > of socket. >2) This function is static and 'hdr' arg was always NULL. > Okay, I saw this patch after my previous comment. I think this looks good, but please move this before your changes (e.g. before patch 'virtio/vsock: dequeue callback for SOCK_SEQPACKET'). In this way you don't need to modify virtio_transport_notify_buffer_size(), calling virtio_transport_get_type() and then remove these changes. It's generally not a good idea to make changes in a patch and then remove them a few patches later in the same series. This should ring a bell about moving these changes before others. Thanks, Stefano >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >--- > net/vmw_vsock/virtio_transport_common.c | 20 +++++--------------- > 1 file changed, 5 insertions(+), 15 deletions(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c >b/net/vmw_vsock/virtio_transport_common.c >index 0aa0fd33e9d6..46308679c8a4 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -286,13 +286,10 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit) > } > EXPORT_SYMBOL_GPL(virtio_transport_put_credit); > >-static int virtio_transport_send_credit_update(struct vsock_sock *vsk, >- int type, >- struct virtio_vsock_hdr *hdr) >+static int virtio_transport_send_credit_update(struct vsock_sock *vsk) > { > struct virtio_vsock_pkt_info info = { > .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE, >- .type = type, > .vsk = vsk, > }; > >@@ -401,9 +398,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, > * with different values. > */ > if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) { >- virtio_transport_send_credit_update(vsk, >- VIRTIO_VSOCK_TYPE_STREAM, >- NULL); >+ virtio_transport_send_credit_update(vsk); > } > > return total; >@@ -525,9 +520,7 @@ size_t virtio_transport_seqpacket_seq_get_len(struct vsock_sock *vsk) > spin_unlock_bh(&vvs->rx_lock); > > if (bytes_dropped) >- virtio_transport_send_credit_update(vsk, >- VIRTIO_VSOCK_TYPE_SEQPACKET, >- NULL); >+ virtio_transport_send_credit_update(vsk); > > return vvs->user_read_seq_len; > } >@@ -624,8 +617,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, > > spin_unlock_bh(&vvs->rx_lock); > >- virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_SEQPACKET, >- NULL); >+ virtio_transport_send_credit_update(vsk); > > return err; > } >@@ -735,15 +727,13 @@ EXPORT_SYMBOL_GPL(virtio_transport_do_socket_init); > void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val) > { > struct virtio_vsock_sock *vvs = vsk->trans; >- int type; > > if (*val > VIRTIO_VSOCK_MAX_BUF_SIZE) > *val = VIRTIO_VSOCK_MAX_BUF_SIZE; > > vvs->buf_alloc = *val; > >- type = virtio_transport_get_type(sk_vsock(vsk)); >- virtio_transport_send_credit_update(vsk, type, NULL); >+ virtio_transport_send_credit_update(vsk); > } > EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size); > >-- >2.25.1 >
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 0aa0fd33e9d6..46308679c8a4 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -286,13 +286,10 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit) } EXPORT_SYMBOL_GPL(virtio_transport_put_credit); -static int virtio_transport_send_credit_update(struct vsock_sock *vsk, - int type, - struct virtio_vsock_hdr *hdr) +static int virtio_transport_send_credit_update(struct vsock_sock *vsk) { struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE, - .type = type, .vsk = vsk, }; @@ -401,9 +398,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, * with different values. */ if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) { - virtio_transport_send_credit_update(vsk, - VIRTIO_VSOCK_TYPE_STREAM, - NULL); + virtio_transport_send_credit_update(vsk); } return total; @@ -525,9 +520,7 @@ size_t virtio_transport_seqpacket_seq_get_len(struct vsock_sock *vsk) spin_unlock_bh(&vvs->rx_lock); if (bytes_dropped) - virtio_transport_send_credit_update(vsk, - VIRTIO_VSOCK_TYPE_SEQPACKET, - NULL); + virtio_transport_send_credit_update(vsk); return vvs->user_read_seq_len; } @@ -624,8 +617,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, spin_unlock_bh(&vvs->rx_lock); - virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_SEQPACKET, - NULL); + virtio_transport_send_credit_update(vsk); return err; } @@ -735,15 +727,13 @@ EXPORT_SYMBOL_GPL(virtio_transport_do_socket_init); void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val) { struct virtio_vsock_sock *vvs = vsk->trans; - int type; if (*val > VIRTIO_VSOCK_MAX_BUF_SIZE) *val = VIRTIO_VSOCK_MAX_BUF_SIZE; vvs->buf_alloc = *val; - type = virtio_transport_get_type(sk_vsock(vsk)); - virtio_transport_send_credit_update(vsk, type, NULL); + virtio_transport_send_credit_update(vsk); } EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size);
'virtio_transport_send_credit_update()' has some extra args: 1) 'type' may be set in 'virtio_transport_send_pkt_info()' using type of socket. 2) This function is static and 'hdr' arg was always NULL. Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> --- net/vmw_vsock/virtio_transport_common.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)