| Message ID | 152234289201.17048.17760447330628182817.stgit@firesoul |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | XDP redirect memory return API | expand |
Hi Jesper,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jesper-Dangaard-Brouer/XDP-redirect-memory-return-API/20180330-203122
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/net/virtio_net.c:451:26: sparse: incorrect type in assignment (different base types) @@ expected restricted __virtio16 [usertype] hdr_len @@ got unsirestricted __virtio16 [usertype] hdr_len @@
drivers/net/virtio_net.c:451:26: expected restricted __virtio16 [usertype] hdr_len
drivers/net/virtio_net.c:451:26: got unsigned short [unsigned] [usertype] len
drivers/net/virtio_net.c:2140:27: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [unsigned] [usertype] ctrl_offloads @@ got unsigned] [usertype] ctrl_offloads @@
drivers/net/virtio_net.c:2140:27: expected unsigned long long [unsigned] [usertype] ctrl_offloads
drivers/net/virtio_net.c:2140:27: got restricted __virtio64
vim +451 drivers/net/virtio_net.c
417
418 static bool __virtnet_xdp_xmit(struct virtnet_info *vi,
419 struct xdp_buff *xdp)
420 {
421 struct virtio_net_hdr_mrg_rxbuf *hdr;
422 struct xdp_frame *xdpf, *xdpf_sent;
423 struct send_queue *sq;
424 unsigned int len;
425 unsigned int qp;
426 int err;
427
428 qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
429 sq = &vi->sq[qp];
430
431 /* Free up any pending old buffers before queueing new ones. */
432 while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
433 xdp_return_frame(xdpf_sent->data, &xdpf_sent->mem);
434
435 xdpf = convert_to_xdp_frame(xdp);
436 if (unlikely(!xdpf))
437 return -EOVERFLOW;
438
439 /* virtqueue want to use data area in-front of packet */
440 if (unlikely(xdpf->metasize > 0))
441 return -EOPNOTSUPP;
442
443 if (unlikely(xdpf->headroom < vi->hdr_len))
444 return -EOVERFLOW;
445
446 /* Make room for virtqueue hdr (also change xdpf->headroom?) */
447 xdpf->data -= vi->hdr_len;
448 /* Zero header and leave csum up to XDP layers */
449 hdr = xdpf->data;
450 memset(hdr, 0, vi->hdr_len);
> 451 hdr->hdr.hdr_len = xdpf->len; /* Q: is this needed? */
452 xdpf->len += vi->hdr_len;
453
454 sg_init_one(sq->sg, xdpf->data, xdpf->len);
455
456 err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
457 if (unlikely(err))
458 return false; /* Caller handle free/refcnt */
459
460 return true;
461 }
462
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, 31 Mar 2018 02:22:35 +0800 kbuild test robot <lkp@intel.com> wrote: > reproduce: > # apt-get install sparse > make ARCH=x86_64 allmodconfig > make C=1 CF=-D__CHECK_ENDIAN__ [...] > 417 > 418 static bool __virtnet_xdp_xmit(struct virtnet_info *vi, > 419 struct xdp_buff *xdp) > 420 { [...] > 445 > 446 /* Make room for virtqueue hdr (also change xdpf->headroom?) */ > 447 xdpf->data -= vi->hdr_len; > 448 /* Zero header and leave csum up to XDP layers */ > 449 hdr = xdpf->data; > 450 memset(hdr, 0, vi->hdr_len); > > 451 hdr->hdr.hdr_len = xdpf->len; /* Q: is this needed? */ I'll just remove this line, as Jason previously told me that it is not strictly necessary (so lets avoid playing with endianness here). > 452 xdpf->len += vi->hdr_len; > 453
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 7b187ec7411e..53acfac8390c 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -419,30 +419,41 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi, struct xdp_buff *xdp) { struct virtio_net_hdr_mrg_rxbuf *hdr; - unsigned int len; + struct xdp_frame *xdpf, *xdpf_sent; struct send_queue *sq; + unsigned int len; unsigned int qp; - void *xdp_sent; int err; qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id(); sq = &vi->sq[qp]; /* Free up any pending old buffers before queueing new ones. */ - while ((xdp_sent = virtqueue_get_buf(sq->vq, &len)) != NULL) { - struct page *sent_page = virt_to_head_page(xdp_sent); + while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL) + xdp_return_frame(xdpf_sent->data, &xdpf_sent->mem); - put_page(sent_page); - } + xdpf = convert_to_xdp_frame(xdp); + if (unlikely(!xdpf)) + return -EOVERFLOW; + + /* virtqueue want to use data area in-front of packet */ + if (unlikely(xdpf->metasize > 0)) + return -EOPNOTSUPP; + + if (unlikely(xdpf->headroom < vi->hdr_len)) + return -EOVERFLOW; - xdp->data -= vi->hdr_len; + /* Make room for virtqueue hdr (also change xdpf->headroom?) */ + xdpf->data -= vi->hdr_len; /* Zero header and leave csum up to XDP layers */ - hdr = xdp->data; + hdr = xdpf->data; memset(hdr, 0, vi->hdr_len); + hdr->hdr.hdr_len = xdpf->len; /* Q: is this needed? */ + xdpf->len += vi->hdr_len; - sg_init_one(sq->sg, xdp->data, xdp->data_end - xdp->data); + sg_init_one(sq->sg, xdpf->data, xdpf->len); - err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp->data, GFP_ATOMIC); + err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC); if (unlikely(err)) return false; /* Caller handle free/refcnt */
The virtio_net driver assumes XDP frames are always released based on page refcnt (via put_page). Thus, is only queues the XDP data pointer address and uses virt_to_head_page() to retrieve struct page. Use the XDP return API to get away from such assumptions. Instead queue an xdp_frame, which allow us to use the xdp_return_frame API, when releasing the frame. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- drivers/net/virtio_net.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)