search for: virtio_transport_inc_tx_pkt

.../vmw_vsock/virtio_transport_common.c > index a85559d4d974..34a2b42313b7 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -210,11 +210,11 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) > { > - spin_lock_bh(&vvs->tx_lock); > + spin_lock_bh(&vvs->rx_lock); > vvs->last_fwd_cnt = vvs->fwd_cnt; > pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); > pkt->hdr.buf_alloc = cpu_to_...

...irtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index a85559d4d974..34a2b42313b7 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -210,11 +210,11 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) { - spin_lock_bh(&vvs->tx_lock); + spin_lock_bh(&vvs->rx_lock); vvs->last_fwd_cnt = vvs->fwd_cnt; pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc); - spi...

...irtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 4fd4987511a9..694d9805f989 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -211,10 +211,10 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) { - spin_lock_bh(&vvs->tx_lock); + spin_lock_bh(&vvs->rx_lock); pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc); - spin_unlock_bh(&vvs->tx_lock); + spin_...

...ree_space = vvs->buf_alloc - (vvs->fwd_cnt - vvs->last_fwd_cnt); > > Locking? These fields should be accessed under tx_lock. > Yes, we need a lock, but looking in the code, vvs->fwd_cnd is written taking rx_lock (virtio_transport_dec_rx_pkt) and it is read with the tx_lock (virtio_transport_inc_tx_pkt). Maybe we should use another spin_lock shared between RX and TX for those fields or use atomic variables. What do you suggest? Thanks, Stefano

...ree_space = vvs->buf_alloc - (vvs->fwd_cnt - vvs->last_fwd_cnt); > > Locking? These fields should be accessed under tx_lock. > Yes, we need a lock, but looking in the code, vvs->fwd_cnd is written taking rx_lock (virtio_transport_dec_rx_pkt) and it is read with the tx_lock (virtio_transport_inc_tx_pkt). Maybe we should use another spin_lock shared between RX and TX for those fields or use atomic variables. What do you suggest? Thanks, Stefano

...ck/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 602715fc9a75..f32301d823f5 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -206,6 +206,7 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) { spin_lock_bh(&vvs->tx_lock); + vvs->last_fwd_cnt = vvs->fwd_cnt; pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc); spin_unlock_bh(&vvs->tx_lock); @@...

...n.c b/net/vmw_vsock/virtio_transport_common.c > index 095221f94786..a85559d4d974 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -211,6 +211,7 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) > { > spin_lock_bh(&vvs->tx_lock); > + vvs->last_fwd_cnt = vvs->fwd_cnt; > pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); > pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc); > spin_unlock...

...n.c b/net/vmw_vsock/virtio_transport_common.c > index 095221f94786..a85559d4d974 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -211,6 +211,7 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) > { > spin_lock_bh(&vvs->tx_lock); > + vvs->last_fwd_cnt = vvs->fwd_cnt; > pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); > pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc); > spin_unlock...

...t;+ size_t skb_len; >+ >+ skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE, rest_len); >+ >+ skb = virtio_transport_alloc_skb(info, skb_len, >+ src_cid, src_port, >+ dst_cid, dst_port); >+ if (!skb) { >+ ret = -ENOMEM; >+ break; >+ } >+ >+ virtio_transport_inc_tx_pkt(vvs, skb); > >- virtio_transport_inc_tx_pkt(vvs, skb); >+ ret = t_ops->send_pkt(skb); > >- return t_ops->send_pkt(skb); >+ if (ret < 0) >+ break; >+ >+ /* Both virtio and vhost 'send_pkt()' returns 'skb_len', >+ * but for reliability use...

...>+ size_t skb_len; >+ >+ skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE, rest_len); >+ >+ skb = virtio_transport_alloc_skb(info, skb_len, >+ src_cid, src_port, >+ dst_cid, dst_port); >+ if (!skb) { >+ ret = -ENOMEM; >+ break; >+ } > >- virtio_transport_inc_tx_pkt(vvs, skb); >+ virtio_transport_inc_tx_pkt(vvs, skb); > >- return t_ops->send_pkt(skb); >+ ret = t_ops->send_pkt(skb); ... here. >+ nit: we can remove this extra line >+ if (ret < 0) >+ break; >+ >+ /* Both virtio and vhost 'send_pkt()' returns &...

...nfo, skb_len, >>> +???????????????????????? src_cid, src_port, >>> +???????????????????????? dst_cid, dst_port); >>> +??????? if (!skb) { >>> +??????????? ret = -ENOMEM; >>> +??????????? break; >>> +??????? } >>> + >>> +??????? virtio_transport_inc_tx_pkt(vvs, skb); >>> + >>> +??????? ret = t_ops->send_pkt(skb); >>> + >>> +??????? if (ret < 0) >>> +??????????? break; >>> >>> -??? virtio_transport_inc_tx_pkt(vvs, skb); >>> +??????? rest_len -= skb_len; >> >> t_o...

...9 489.34 256K 156.35 149.81 152.21 536.37 512K 151.65 150.74 151.52 519.93 [1] https://github.com/stefano-garzarella/iperf/ Stefano Garzarella (5): vsock/virtio: limit the memory used per-socket vsock/virtio: reduce credit update messages vsock/virtio: fix locking in virtio_transport_inc_tx_pkt() vhost/vsock: split packets to send using multiple buffers vsock/virtio: change the maximum packet size allowed drivers/vhost/vsock.c | 68 ++++++++++++----- include/linux/virtio_vsock.h | 4 +- net/vmw_vsock/virtio_transport.c | 1 + net/vmw_vsock/virti...

...9 489.34 256K 156.35 149.81 152.21 536.37 512K 151.65 150.74 151.52 519.93 [1] https://github.com/stefano-garzarella/iperf/ Stefano Garzarella (5): vsock/virtio: limit the memory used per-socket vsock/virtio: reduce credit update messages vsock/virtio: fix locking in virtio_transport_inc_tx_pkt() vhost/vsock: split packets to send using multiple buffers vsock/virtio: change the maximum packet size allowed drivers/vhost/vsock.c | 68 ++++++++++++----- include/linux/virtio_vsock.h | 4 +- net/vmw_vsock/virtio_transport.c | 1 + net/vmw_vsock/virti...

...c | 7 ------- > 3 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h > index b92e88d..ff6850a 100644 > --- a/include/linux/virtio_vsock.h > +++ b/include/linux/virtio_vsock.h > @@ -156,4 +156,11 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vs > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted); > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit); > > +static inline const struct virtio_transport *virtio_transport_get_ops(voi...

...c | 7 ------- > 3 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h > index b92e88d..ff6850a 100644 > --- a/include/linux/virtio_vsock.h > +++ b/include/linux/virtio_vsock.h > @@ -156,4 +156,11 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vs > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted); > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit); > > +static inline const struct virtio_transport *virtio_transport_get_ops(voi...

...150.74 151.52 519.93 > > > > [1] https://github.com/stefano-garzarella/iperf/ > > > > Stefano Garzarella (5): > > vsock/virtio: limit the memory used per-socket > > vsock/virtio: reduce credit update messages > > vsock/virtio: fix locking in virtio_transport_inc_tx_pkt() > > vhost/vsock: split packets to send using multiple buffers > > vsock/virtio: change the maximum packet size allowed > > > > drivers/vhost/vsock.c | 68 ++++++++++++----- > > include/linux/virtio_vsock.h | 4 +- > > net/vmw...

...9 489.34 256K 156.35 149.81 152.21 536.37 512K 151.65 150.74 151.52 519.93 [1] https://github.com/stefano-garzarella/iperf/ Stefano Garzarella (5): vsock/virtio: limit the memory used per-socket vsock/virtio: reduce credit update messages vsock/virtio: fix locking in virtio_transport_inc_tx_pkt() vhost/vsock: split packets to send using multiple buffers vsock/virtio: change the maximum packet size allowed drivers/vhost/vsock.c | 68 ++++++++++++----- include/linux/virtio_vsock.h | 4 +- net/vmw_vsock/virtio_transport.c | 1 + net/vmw_vsock/virti...

...9 489.34 256K 156.35 149.81 152.21 536.37 512K 151.65 150.74 151.52 519.93 [1] https://github.com/stefano-garzarella/iperf/ Stefano Garzarella (5): vsock/virtio: limit the memory used per-socket vsock/virtio: reduce credit update messages vsock/virtio: fix locking in virtio_transport_inc_tx_pkt() vhost/vsock: split packets to send using multiple buffers vsock/virtio: change the maximum packet size allowed drivers/vhost/vsock.c | 68 ++++++++++++----- include/linux/virtio_vsock.h | 4 +- net/vmw_vsock/virtio_transport.c | 1 + net/vmw_vsock/virti...

This series tries to increase the throughput of virtio-vsock with slight changes. While I was testing the v2 of this series I discovered an huge use of memory, so I added patch 1 to mitigate this issue. I put it in this series in order to better track the performance trends. v3: - Patch 1: added a threshold to copy only small packets [Jason] - Patch 1: replaced the allocation of a new buffer

...s->fwd_cnt - vvs->last_fwd_cnt); > > > > Locking? These fields should be accessed under tx_lock. > > > > Yes, we need a lock, but looking in the code, vvs->fwd_cnd is written > taking rx_lock (virtio_transport_dec_rx_pkt) and it is read with the > tx_lock (virtio_transport_inc_tx_pkt). > > Maybe we should use another spin_lock shared between RX and TX for those > fields or use atomic variables. > > What do you suggest? Or make vvs->fwd_cnt atomic if it's the only field that needs to be accessed in this manner. Stefan -------------- next part ----------...