| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6b918a27-4c87-4c8c-bb31-7a5cca15844f@gmail.com>
Date: Sat, 26 Apr 2025 15:33:07 +0700
From: Bui Quang Minh <minhquangbui99@...il.com>
To: netdev@...r.kernel.org
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
<eperezma@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>, virtualization@...ts.linux.dev,
linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [RFC PATCH net-next] virtio-net: support zerocopy multi buffer
XDP in mergeable
On 4/26/25 15:27, Bui Quang Minh wrote:
> Currently, in zerocopy mode with mergeable receive buffer, virtio-net
> does not support multi buffer but a single buffer only. This commit adds
> support for multi mergeable receive buffer in the zerocopy XDP path by
> utilizing XDP buffer with frags.
>
> Signed-off-by: Bui Quang Minh <minhquangbui99@...il.com>
> ---
> drivers/net/virtio_net.c | 107 +++++++++++++++++++--------------------
> 1 file changed, 51 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 848fab51dfa1..8d21767dd607 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -45,6 +45,8 @@ module_param(napi_tx, bool, 0644);
> #define VIRTIO_XDP_TX BIT(0)
> #define VIRTIO_XDP_REDIR BIT(1)
>
> +#define VIRTNET_MAX_ZC_SEGS 8
> +
> /* RX packet size EWMA. The average packet size is used to determine the packet
> * buffer size when refilling RX rings. As the entire RX ring may be refilled
> * at once, the weight is chosen so that the EWMA will be insensitive to short-
> @@ -1232,65 +1234,53 @@ static void xsk_drop_follow_bufs(struct net_device *dev,
> }
> }
>
> -static int xsk_append_merge_buffer(struct virtnet_info *vi,
> - struct receive_queue *rq,
> - struct sk_buff *head_skb,
> - u32 num_buf,
> - struct virtio_net_hdr_mrg_rxbuf *hdr,
> - struct virtnet_rq_stats *stats)
> +static int virtnet_build_xsk_buff_mrg(struct virtnet_info *vi,
> + struct receive_queue *rq,
> + u32 num_buf,
> + struct xdp_buff *xdp,
> + struct virtnet_rq_stats *stats)
> {
> - struct sk_buff *curr_skb;
> - struct xdp_buff *xdp;
> - u32 len, truesize;
> - struct page *page;
> + unsigned int len;
> void *buf;
>
> - curr_skb = head_skb;
> + if (num_buf < 2)
> + return 0;
> +
> + while (num_buf > 1) {
> + struct xdp_buff *new_xdp;
>
> - while (--num_buf) {
> buf = virtqueue_get_buf(rq->vq, &len);
> - if (unlikely(!buf)) {
> - pr_debug("%s: rx error: %d buffers out of %d missing\n",
> - vi->dev->name, num_buf,
> - virtio16_to_cpu(vi->vdev,
> - hdr->num_buffers));
> + if (!unlikely(buf)) {
> + pr_debug("%s: rx error: %d buffers missing\n",
> + vi->dev->name, num_buf);
> DEV_STATS_INC(vi->dev, rx_length_errors);
> - return -EINVAL;
> - }
> -
> - u64_stats_add(&stats->bytes, len);
> -
> - xdp = buf_to_xdp(vi, rq, buf, len);
> - if (!xdp)
> - goto err;
> -
> - buf = napi_alloc_frag(len);
> - if (!buf) {
> - xsk_buff_free(xdp);
> - goto err;
> + return -1;
> }
>
> - memcpy(buf, xdp->data - vi->hdr_len, len);
> -
> - xsk_buff_free(xdp);
> + new_xdp = buf_to_xdp(vi, rq, buf, len);
> + if (!new_xdp)
> + goto drop_bufs;
>
> - page = virt_to_page(buf);
> + /* In virtnet_add_recvbuf_xsk(), we ask the host to fill from
> + * xdp->data - vi->hdr_len with both virtio_net_hdr and data.
> + * However, only the first packet has the virtio_net_hdr, the
> + * following ones do not. So we need to adjust the following
> + * packets' data pointer to the correct place.
> + */
> + new_xdp->data -= vi->hdr_len;
> + new_xdp->data_end = new_xdp->data + len;
>
> - truesize = len;
> + if (!xsk_buff_add_frag(xdp, new_xdp))
> + goto drop_bufs;
>
> - curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
> - buf, len, truesize);
> - if (!curr_skb) {
> - put_page(page);
> - goto err;
> - }
> + num_buf--;
> }
>
> return 0;
>
> -err:
> +drop_bufs:
> xsk_drop_follow_bufs(vi->dev, rq, num_buf, stats);
> - return -EINVAL;
> + return -1;
> }
>
> static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct virtnet_info *vi,
> @@ -1307,23 +1297,28 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct
> num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
>
> ret = XDP_PASS;
> + if (virtnet_build_xsk_buff_mrg(vi, rq, num_buf, xdp, stats))
> + goto drop;
> +
> rcu_read_lock();
> prog = rcu_dereference(rq->xdp_prog);
> - /* TODO: support multi buffer. */
> - if (prog && num_buf == 1)
> + if (prog)
> ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
> rcu_read_unlock();
>
> switch (ret) {
> case XDP_PASS:
> - skb = xsk_construct_skb(rq, xdp);
> + skb = xdp_build_skb_from_zc(xdp);
This function has a bug which needs this fix commit to be applied to
work correctly:
https://lore.kernel.org/netdev/20250426081220.40689-2-minhquangbui99@gmail.com/
> if (!skb)
> - goto drop_bufs;
> + break;
>
> - if (xsk_append_merge_buffer(vi, rq, skb, num_buf, hdr, stats)) {
> - dev_kfree_skb(skb);
> - goto drop;
> - }
> + /* Later, in virtnet_receive_done(), eth_type_trans()
> + * is called. However, in xdp_build_skb_from_zc(), it is called
> + * already. As a result, we need to reset the data to before
> + * the mac header so that the later call in
> + * virtnet_receive_done() works correctly.
> + */
> + skb_push(skb, ETH_HLEN);
>
> return skb;
>
> @@ -1332,14 +1327,11 @@ static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct
> return NULL;
>
> default:
> - /* drop packet */
> - xsk_buff_free(xdp);
> + break;
> }
>
> -drop_bufs:
> - xsk_drop_follow_bufs(dev, rq, num_buf, stats);
> -
> drop:
> + xsk_buff_free(xdp);
> u64_stats_inc(&stats->drops);
> return NULL;
> }
> @@ -1396,6 +1388,8 @@ static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue
> return -ENOMEM;
>
> len = xsk_pool_get_rx_frame_size(pool) + vi->hdr_len;
> + /* Reserve some space for skb_shared_info */
> + len -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>
> for (i = 0; i < num; ++i) {
> /* Use the part of XDP_PACKET_HEADROOM as the virtnet hdr space.
> @@ -6721,6 +6715,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->netdev_ops = &virtnet_netdev;
> dev->stat_ops = &virtnet_stat_ops;
> dev->features = NETIF_F_HIGHDMA;
> + dev->xdp_zc_max_segs = VIRTNET_MAX_ZC_SEGS;
>
> dev->ethtool_ops = &virtnet_ethtool_ops;
> SET_NETDEV_DEV(dev, &vdev->dev);
Powered by blists - more mailing lists