| Message ID | 20231016120033.26933-14-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio-net: support AF_XDP zero copy | expand |
On Mon, 16 Oct 2023 20:00:27 +0800 Xuan Zhuo wrote: > @@ -305,9 +311,15 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, > > stats->bytes += xdp_get_frame_len(frame); > xdp_return_frame(frame); > + } else { > + stats->bytes += virtnet_ptr_to_xsk(ptr); > + ++xsknum; > } > stats->packets++; > } > + > + if (xsknum) > + xsk_tx_completed(sq->xsk.pool, xsknum); > } sparse complains: drivers/net/virtio/virtio_net.h:322:41: warning: incorrect type in argument 1 (different address spaces) drivers/net/virtio/virtio_net.h:322:41: expected struct xsk_buff_pool *pool drivers/net/virtio/virtio_net.h:322:41: got struct xsk_buff_pool [noderef] __rcu *pool please build test with W=1 C=1
On Mon, 16 Oct 2023 16:44:34 -0700, Jakub Kicinski <kuba@kernel.org> wrote: > On Mon, 16 Oct 2023 20:00:27 +0800 Xuan Zhuo wrote: > > @@ -305,9 +311,15 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, > > > > stats->bytes += xdp_get_frame_len(frame); > > xdp_return_frame(frame); > > + } else { > > + stats->bytes += virtnet_ptr_to_xsk(ptr); > > + ++xsknum; > > } > > stats->packets++; > > } > > + > > + if (xsknum) > > + xsk_tx_completed(sq->xsk.pool, xsknum); > > } > > sparse complains: > > drivers/net/virtio/virtio_net.h:322:41: warning: incorrect type in argument 1 (different address spaces) > drivers/net/virtio/virtio_net.h:322:41: expected struct xsk_buff_pool *pool > drivers/net/virtio/virtio_net.h:322:41: got struct xsk_buff_pool > [noderef] __rcu *pool > > please build test with W=1 C=1 OK. I will add C=1 to may script. Thanks. > -- > pw-bot: cr
On Tue, Oct 17, 2023 at 10:02:05AM +0800, Xuan Zhuo wrote: > On Mon, 16 Oct 2023 16:44:34 -0700, Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 16 Oct 2023 20:00:27 +0800 Xuan Zhuo wrote: > > > @@ -305,9 +311,15 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, > > > > > > stats->bytes += xdp_get_frame_len(frame); > > > xdp_return_frame(frame); > > > + } else { > > > + stats->bytes += virtnet_ptr_to_xsk(ptr); > > > + ++xsknum; > > > } > > > stats->packets++; > > > } > > > + > > > + if (xsknum) > > > + xsk_tx_completed(sq->xsk.pool, xsknum); > > > } > > > > sparse complains: > > > > drivers/net/virtio/virtio_net.h:322:41: warning: incorrect type in argument 1 (different address spaces) > > drivers/net/virtio/virtio_net.h:322:41: expected struct xsk_buff_pool *pool > > drivers/net/virtio/virtio_net.h:322:41: got struct xsk_buff_pool > > [noderef] __rcu *pool > > > > please build test with W=1 C=1 > > OK. I will add C=1 to may script. > > Thanks. And I hope we all understand, rcu has to be used properly it's not just about casting the warning away.
On Thu, 19 Oct 2023 02:38:16 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Oct 17, 2023 at 10:02:05AM +0800, Xuan Zhuo wrote: > > On Mon, 16 Oct 2023 16:44:34 -0700, Jakub Kicinski <kuba@kernel.org> wrote: > > > On Mon, 16 Oct 2023 20:00:27 +0800 Xuan Zhuo wrote: > > > > @@ -305,9 +311,15 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, > > > > > > > > stats->bytes += xdp_get_frame_len(frame); > > > > xdp_return_frame(frame); > > > > + } else { > > > > + stats->bytes += virtnet_ptr_to_xsk(ptr); > > > > + ++xsknum; > > > > } > > > > stats->packets++; > > > > } > > > > + > > > > + if (xsknum) > > > > + xsk_tx_completed(sq->xsk.pool, xsknum); > > > > } > > > > > > sparse complains: > > > > > > drivers/net/virtio/virtio_net.h:322:41: warning: incorrect type in argument 1 (different address spaces) > > > drivers/net/virtio/virtio_net.h:322:41: expected struct xsk_buff_pool *pool > > > drivers/net/virtio/virtio_net.h:322:41: got struct xsk_buff_pool > > > [noderef] __rcu *pool > > > > > > please build test with W=1 C=1 > > > > OK. I will add C=1 to may script. > > > > Thanks. > > And I hope we all understand, rcu has to be used properly it's not just > about casting the warning away. Yes. I see. I will use rcu_dereference() and rcu_read_xxx(). Thanks. > > -- > MST >
On Thu, Oct 19, 2023 at 03:13:48PM +0800, Xuan Zhuo wrote: > On Thu, 19 Oct 2023 02:38:16 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Tue, Oct 17, 2023 at 10:02:05AM +0800, Xuan Zhuo wrote: > > > On Mon, 16 Oct 2023 16:44:34 -0700, Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Mon, 16 Oct 2023 20:00:27 +0800 Xuan Zhuo wrote: > > > > > @@ -305,9 +311,15 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, > > > > > > > > > > stats->bytes += xdp_get_frame_len(frame); > > > > > xdp_return_frame(frame); > > > > > + } else { > > > > > + stats->bytes += virtnet_ptr_to_xsk(ptr); > > > > > + ++xsknum; > > > > > } > > > > > stats->packets++; > > > > > } > > > > > + > > > > > + if (xsknum) > > > > > + xsk_tx_completed(sq->xsk.pool, xsknum); > > > > > } > > > > > > > > sparse complains: > > > > > > > > drivers/net/virtio/virtio_net.h:322:41: warning: incorrect type in argument 1 (different address spaces) > > > > drivers/net/virtio/virtio_net.h:322:41: expected struct xsk_buff_pool *pool > > > > drivers/net/virtio/virtio_net.h:322:41: got struct xsk_buff_pool > > > > [noderef] __rcu *pool > > > > > > > > please build test with W=1 C=1 > > > > > > OK. I will add C=1 to may script. > > > > > > Thanks. > > > > And I hope we all understand, rcu has to be used properly it's not just > > about casting the warning away. > > > Yes. I see. I will use rcu_dereference() and rcu_read_xxx(). > > Thanks. When you do, pls don't forget to add comments documenting what does rcu_read_lock and synchronize_rcu.
diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h index 7c72a8bb1813..d4e620a084f4 100644 --- a/drivers/net/virtio/virtio_net.h +++ b/drivers/net/virtio/virtio_net.h @@ -243,6 +243,11 @@ struct virtnet_info { #include "xsk.h" +static inline bool virtnet_is_skb_ptr(void *ptr) +{ + return !((unsigned long)ptr & VIRTIO_XMIT_DATA_MASK); +} + static inline bool virtnet_is_xdp_frame(void *ptr) { return (unsigned long)ptr & VIRTIO_XDP_FLAG; @@ -279,11 +284,12 @@ static inline void *virtnet_sq_unmap(struct virtnet_sq *sq, void *data) static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, struct virtnet_sq_stats *stats) { + unsigned int xsknum = 0; unsigned int len; void *ptr; while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) { - if (!virtnet_is_xdp_frame(ptr)) { + if (virtnet_is_skb_ptr(ptr)) { struct sk_buff *skb; if (sq->do_dma) @@ -295,7 +301,7 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, stats->bytes += skb->len; napi_consume_skb(skb, in_napi); - } else { + } else if (virtnet_is_xdp_frame(ptr)) { struct xdp_frame *frame; if (sq->do_dma) @@ -305,9 +311,15 @@ static inline void virtnet_free_old_xmit(struct virtnet_sq *sq, bool in_napi, stats->bytes += xdp_get_frame_len(frame); xdp_return_frame(frame); + } else { + stats->bytes += virtnet_ptr_to_xsk(ptr); + ++xsknum; } stats->packets++; } + + if (xsknum) + xsk_tx_completed(sq->xsk.pool, xsknum); } static inline void virtnet_vq_napi_schedule(struct napi_struct *napi, diff --git a/drivers/net/virtio/xsk.h b/drivers/net/virtio/xsk.h index 1bd19dcda649..7ebc9bda7aee 100644 --- a/drivers/net/virtio/xsk.h +++ b/drivers/net/virtio/xsk.h @@ -14,6 +14,11 @@ static inline void *virtnet_xsk_to_ptr(u32 len) return (void *)(p | VIRTIO_XSK_FLAG); } +static inline u32 virtnet_ptr_to_xsk(void *ptr) +{ + return ((unsigned long)ptr) >> VIRTIO_XSK_FLAG_OFFSET; +} + int virtnet_xsk_pool_setup(struct net_device *dev, struct netdev_bpf *xdp); bool virtnet_xsk_xmit(struct virtnet_sq *sq, struct xsk_buff_pool *pool, int budget);
virtnet_free_old_xmit distinguishes three type ptr(skb, xdp frame, xsk buffer) by the last two types bits. Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio/virtio_net.h | 16 ++++++++++++++-- drivers/net/virtio/xsk.h | 5 +++++ 2 files changed, 19 insertions(+), 2 deletions(-)