* [RFC PATCH v2 1/7] Revert "virtio_net: xsk: rx: support recv merge mode"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 2/7] Revert "virtio_net: xsk: rx: support recv small mode" Michael S. Tsirkin
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, bpf
This reverts commit 99c861b44eb1fb9dfe8776854116a6a9064c19bb.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 144 ---------------------------------------
1 file changed, 144 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c6af18948092..15e202dd6964 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -504,10 +504,6 @@ static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
struct virtnet_rq_stats *stats);
static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq,
struct sk_buff *skb, u8 flags);
-static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
- struct sk_buff *curr_skb,
- struct page *page, void *buf,
- int len, int truesize);
static bool is_xdp_frame(void *ptr)
{
@@ -988,11 +984,6 @@ static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
rq = &vi->rq[i];
- if (rq->xsk_pool) {
- xsk_buff_free((struct xdp_buff *)buf);
- return;
- }
-
if (!vi->big_packets || vi->mergeable_rx_bufs)
virtnet_rq_unmap(rq, buf, 0);
@@ -1161,139 +1152,6 @@ static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct
}
}
-static void xsk_drop_follow_bufs(struct net_device *dev,
- struct receive_queue *rq,
- u32 num_buf,
- struct virtnet_rq_stats *stats)
-{
- struct xdp_buff *xdp;
- u32 len;
-
- while (num_buf-- > 1) {
- xdp = virtqueue_get_buf(rq->vq, &len);
- if (unlikely(!xdp)) {
- pr_debug("%s: rx error: %d buffers missing\n",
- dev->name, num_buf);
- DEV_STATS_INC(dev, rx_length_errors);
- break;
- }
- u64_stats_add(&stats->bytes, len);
- xsk_buff_free(xdp);
- }
-}
-
-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)
-{
- struct sk_buff *curr_skb;
- struct xdp_buff *xdp;
- u32 len, truesize;
- struct page *page;
- void *buf;
-
- curr_skb = head_skb;
-
- 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));
- 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;
- }
-
- memcpy(buf, xdp->data - vi->hdr_len, len);
-
- xsk_buff_free(xdp);
-
- page = virt_to_page(buf);
-
- truesize = len;
-
- curr_skb = virtnet_skb_append_frag(head_skb, curr_skb, page,
- buf, len, truesize);
- if (!curr_skb) {
- put_page(page);
- goto err;
- }
- }
-
- return 0;
-
-err:
- xsk_drop_follow_bufs(vi->dev, rq, num_buf, stats);
- return -EINVAL;
-}
-
-static struct sk_buff *virtnet_receive_xsk_merge(struct net_device *dev, struct virtnet_info *vi,
- struct receive_queue *rq, struct xdp_buff *xdp,
- unsigned int *xdp_xmit,
- struct virtnet_rq_stats *stats)
-{
- struct virtio_net_hdr_mrg_rxbuf *hdr;
- struct bpf_prog *prog;
- struct sk_buff *skb;
- u32 ret, num_buf;
-
- hdr = xdp->data - vi->hdr_len;
- num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
-
- ret = XDP_PASS;
- rcu_read_lock();
- prog = rcu_dereference(rq->xdp_prog);
- /* TODO: support multi buffer. */
- if (prog && num_buf == 1)
- ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
- rcu_read_unlock();
-
- switch (ret) {
- case XDP_PASS:
- skb = xsk_construct_skb(rq, xdp);
- if (!skb)
- goto drop_bufs;
-
- if (xsk_append_merge_buffer(vi, rq, skb, num_buf, hdr, stats)) {
- dev_kfree_skb(skb);
- goto drop;
- }
-
- return skb;
-
- case XDP_TX:
- case XDP_REDIRECT:
- return NULL;
-
- default:
- /* drop packet */
- xsk_buff_free(xdp);
- }
-
-drop_bufs:
- xsk_drop_follow_bufs(dev, rq, num_buf, stats);
-
-drop:
- u64_stats_inc(&stats->drops);
- return NULL;
-}
-
static void virtnet_receive_xsk_buf(struct virtnet_info *vi, struct receive_queue *rq,
void *buf, u32 len,
unsigned int *xdp_xmit,
@@ -1323,8 +1181,6 @@ static void virtnet_receive_xsk_buf(struct virtnet_info *vi, struct receive_queu
if (!vi->mergeable_rx_bufs)
skb = virtnet_receive_xsk_small(dev, vi, rq, xdp, xdp_xmit, stats);
- else
- skb = virtnet_receive_xsk_merge(dev, vi, rq, xdp, xdp_xmit, stats);
if (skb)
virtnet_receive_done(vi, rq, skb, flags);
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 2/7] Revert "virtio_net: xsk: rx: support recv small mode"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 1/7] Revert "virtio_net: xsk: rx: support recv merge mode" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 3/7] Revert "virtio_net: xsk: rx: support fill with xsk buffer" Michael S. Tsirkin
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, bpf
This reverts commit a4e7ba7027012f009f22a68bcfde670f9298d3a4.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 198 ++++-----------------------------------
1 file changed, 19 insertions(+), 179 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 15e202dd6964..041c483a06c5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -498,12 +498,6 @@ struct virtio_net_common_hdr {
};
static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
-static int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
- struct net_device *dev,
- unsigned int *xdp_xmit,
- struct virtnet_rq_stats *stats);
-static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq,
- struct sk_buff *skb, u8 flags);
static bool is_xdp_frame(void *ptr)
{
@@ -1068,124 +1062,6 @@ static void sg_fill_dma(struct scatterlist *sg, dma_addr_t addr, u32 len)
sg->length = len;
}
-static struct xdp_buff *buf_to_xdp(struct virtnet_info *vi,
- struct receive_queue *rq, void *buf, u32 len)
-{
- struct xdp_buff *xdp;
- u32 bufsize;
-
- xdp = (struct xdp_buff *)buf;
-
- bufsize = xsk_pool_get_rx_frame_size(rq->xsk_pool) + vi->hdr_len;
-
- if (unlikely(len > bufsize)) {
- pr_debug("%s: rx error: len %u exceeds truesize %u\n",
- vi->dev->name, len, bufsize);
- DEV_STATS_INC(vi->dev, rx_length_errors);
- xsk_buff_free(xdp);
- return NULL;
- }
-
- xsk_buff_set_size(xdp, len);
- xsk_buff_dma_sync_for_cpu(xdp);
-
- return xdp;
-}
-
-static struct sk_buff *xsk_construct_skb(struct receive_queue *rq,
- struct xdp_buff *xdp)
-{
- unsigned int metasize = xdp->data - xdp->data_meta;
- struct sk_buff *skb;
- unsigned int size;
-
- size = xdp->data_end - xdp->data_hard_start;
- skb = napi_alloc_skb(&rq->napi, size);
- if (unlikely(!skb)) {
- xsk_buff_free(xdp);
- return NULL;
- }
-
- skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
-
- size = xdp->data_end - xdp->data_meta;
- memcpy(__skb_put(skb, size), xdp->data_meta, size);
-
- if (metasize) {
- __skb_pull(skb, metasize);
- skb_metadata_set(skb, metasize);
- }
-
- xsk_buff_free(xdp);
-
- return skb;
-}
-
-static struct sk_buff *virtnet_receive_xsk_small(struct net_device *dev, struct virtnet_info *vi,
- struct receive_queue *rq, struct xdp_buff *xdp,
- unsigned int *xdp_xmit,
- struct virtnet_rq_stats *stats)
-{
- struct bpf_prog *prog;
- u32 ret;
-
- ret = XDP_PASS;
- rcu_read_lock();
- prog = rcu_dereference(rq->xdp_prog);
- if (prog)
- ret = virtnet_xdp_handler(prog, xdp, dev, xdp_xmit, stats);
- rcu_read_unlock();
-
- switch (ret) {
- case XDP_PASS:
- return xsk_construct_skb(rq, xdp);
-
- case XDP_TX:
- case XDP_REDIRECT:
- return NULL;
-
- default:
- /* drop packet */
- xsk_buff_free(xdp);
- u64_stats_inc(&stats->drops);
- return NULL;
- }
-}
-
-static void virtnet_receive_xsk_buf(struct virtnet_info *vi, struct receive_queue *rq,
- void *buf, u32 len,
- unsigned int *xdp_xmit,
- struct virtnet_rq_stats *stats)
-{
- struct net_device *dev = vi->dev;
- struct sk_buff *skb = NULL;
- struct xdp_buff *xdp;
- u8 flags;
-
- len -= vi->hdr_len;
-
- u64_stats_add(&stats->bytes, len);
-
- xdp = buf_to_xdp(vi, rq, buf, len);
- if (!xdp)
- return;
-
- if (unlikely(len < ETH_HLEN)) {
- pr_debug("%s: short packet %i\n", dev->name, len);
- DEV_STATS_INC(dev, rx_length_errors);
- xsk_buff_free(xdp);
- return;
- }
-
- flags = ((struct virtio_net_common_hdr *)(xdp->data - vi->hdr_len))->hdr.flags;
-
- if (!vi->mergeable_rx_bufs)
- skb = virtnet_receive_xsk_small(dev, vi, rq, xdp, xdp_xmit, stats);
-
- if (skb)
- virtnet_receive_done(vi, rq, skb, flags);
-}
-
static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue *rq,
struct xsk_buff_pool *pool, gfp_t gfp)
{
@@ -2516,67 +2392,31 @@ static void refill_work(struct work_struct *work)
}
}
-static int virtnet_receive_xsk_bufs(struct virtnet_info *vi,
- struct receive_queue *rq,
- int budget,
- unsigned int *xdp_xmit,
- struct virtnet_rq_stats *stats)
-{
- unsigned int len;
- int packets = 0;
- void *buf;
-
- while (packets < budget) {
- buf = virtqueue_get_buf(rq->vq, &len);
- if (!buf)
- break;
-
- virtnet_receive_xsk_buf(vi, rq, buf, len, xdp_xmit, stats);
- packets++;
- }
-
- return packets;
-}
-
-static int virtnet_receive_packets(struct virtnet_info *vi,
- struct receive_queue *rq,
- int budget,
- unsigned int *xdp_xmit,
- struct virtnet_rq_stats *stats)
-{
- unsigned int len;
- int packets = 0;
- void *buf;
-
- if (!vi->big_packets || vi->mergeable_rx_bufs) {
- void *ctx;
- while (packets < budget &&
- (buf = virtnet_rq_get_buf(rq, &len, &ctx))) {
- receive_buf(vi, rq, buf, len, ctx, xdp_xmit, stats);
- packets++;
- }
- } else {
- while (packets < budget &&
- (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
- receive_buf(vi, rq, buf, len, NULL, xdp_xmit, stats);
- packets++;
- }
- }
-
- return packets;
-}
-
static int virtnet_receive(struct receive_queue *rq, int budget,
unsigned int *xdp_xmit)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
struct virtnet_rq_stats stats = {};
- int i, packets;
+ unsigned int len;
+ int packets = 0;
+ void *buf;
+ int i;
- if (rq->xsk_pool)
- packets = virtnet_receive_xsk_bufs(vi, rq, budget, xdp_xmit, &stats);
- else
- packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
+ if (!vi->big_packets || vi->mergeable_rx_bufs) {
+ void *ctx;
+
+ while (packets < budget &&
+ (buf = virtnet_rq_get_buf(rq, &len, &ctx))) {
+ receive_buf(vi, rq, buf, len, ctx, xdp_xmit, &stats);
+ packets++;
+ }
+ } else {
+ while (packets < budget &&
+ (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
+ receive_buf(vi, rq, buf, len, NULL, xdp_xmit, &stats);
+ packets++;
+ }
+ }
if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 3/7] Revert "virtio_net: xsk: rx: support fill with xsk buffer"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 1/7] Revert "virtio_net: xsk: rx: support recv merge mode" Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 2/7] Revert "virtio_net: xsk: rx: support recv small mode" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 4/7] Revert "virtio_net: xsk: bind/unbind xsk for rx" Michael S. Tsirkin
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, bpf
This reverts commit e9f3962441c0a4d6f16c656e6c8aa02a3ccdd568.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 70 +++-------------------------------------
1 file changed, 4 insertions(+), 66 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 041c483a06c5..3cb0f8adf2e6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -354,8 +354,6 @@ struct receive_queue {
/* xdp rxq used by xsk */
struct xdp_rxq_info xsk_rxq_info;
-
- struct xdp_buff **xsk_buffs;
};
/* This structure can contain rss message with maximum settings for indirection table and keysize
@@ -1056,53 +1054,6 @@ static void check_sq_full_and_disable(struct virtnet_info *vi,
}
}
-static void sg_fill_dma(struct scatterlist *sg, dma_addr_t addr, u32 len)
-{
- sg->dma_address = addr;
- sg->length = len;
-}
-
-static int virtnet_add_recvbuf_xsk(struct virtnet_info *vi, struct receive_queue *rq,
- struct xsk_buff_pool *pool, gfp_t gfp)
-{
- struct xdp_buff **xsk_buffs;
- dma_addr_t addr;
- int err = 0;
- u32 len, i;
- int num;
-
- xsk_buffs = rq->xsk_buffs;
-
- num = xsk_buff_alloc_batch(pool, xsk_buffs, rq->vq->num_free);
- if (!num)
- return -ENOMEM;
-
- len = xsk_pool_get_rx_frame_size(pool) + vi->hdr_len;
-
- for (i = 0; i < num; ++i) {
- /* Use the part of XDP_PACKET_HEADROOM as the virtnet hdr space.
- * We assume XDP_PACKET_HEADROOM is larger than hdr->len.
- * (see function virtnet_xsk_pool_enable)
- */
- addr = xsk_buff_xdp_get_dma(xsk_buffs[i]) - vi->hdr_len;
-
- sg_init_table(rq->sg, 1);
- sg_fill_dma(rq->sg, addr, len);
-
- err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, xsk_buffs[i], gfp);
- if (err)
- goto err;
- }
-
- return num;
-
-err:
- for (; i < num; ++i)
- xsk_buff_free(xsk_buffs[i]);
-
- return err;
-}
-
static int virtnet_xsk_wakeup(struct net_device *dev, u32 qid, u32 flag)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -2294,11 +2245,7 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
gfp_t gfp)
{
int err;
-
- if (rq->xsk_pool) {
- err = virtnet_add_recvbuf_xsk(vi, rq, rq->xsk_pool, gfp);
- goto kick;
- }
+ bool oom;
do {
if (vi->mergeable_rx_bufs)
@@ -2308,11 +2255,10 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
else
err = add_recvbuf_small(vi, rq, gfp);
+ oom = err == -ENOMEM;
if (err)
break;
} while (rq->vq->num_free);
-
-kick:
if (virtqueue_kick_prepare(rq->vq) && virtqueue_notify(rq->vq)) {
unsigned long flags;
@@ -2321,7 +2267,7 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
u64_stats_update_end_irqrestore(&rq->stats.syncp, flags);
}
- return err != -ENOMEM;
+ return !oom;
}
static void skb_recv_done(struct virtqueue *rvq)
@@ -5168,7 +5114,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
struct receive_queue *rq;
struct device *dma_dev;
struct send_queue *sq;
- int err, size;
+ int err;
if (vi->hdr_len > xsk_pool_get_headroom(pool))
return -EINVAL;
@@ -5199,12 +5145,6 @@ static int virtnet_xsk_pool_enable(struct net_device *dev,
if (!dma_dev)
return -EINVAL;
- size = virtqueue_get_vring_size(rq->vq);
-
- rq->xsk_buffs = kvcalloc(size, sizeof(*rq->xsk_buffs), GFP_KERNEL);
- if (!rq->xsk_buffs)
- return -ENOMEM;
-
err = xsk_pool_dma_map(pool, dma_dev, 0);
if (err)
goto err_xsk_map;
@@ -5239,8 +5179,6 @@ static int virtnet_xsk_pool_disable(struct net_device *dev, u16 qid)
xsk_pool_dma_unmap(pool, 0);
- kvfree(rq->xsk_buffs);
-
return err;
}
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 4/7] Revert "virtio_net: xsk: bind/unbind xsk for rx"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
` (2 preceding siblings ...)
2024-09-06 9:52 ` [RFC PATCH v2 3/7] Revert "virtio_net: xsk: rx: support fill with xsk buffer" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 5/7] Revert "virtio_net: rx remove premapped failover code" Michael S. Tsirkin
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, bpf
This reverts commit 09d2b3182c8e3a215a9b2a1834f81dd07305989f.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 134 ---------------------------------------
1 file changed, 134 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 3cb0f8adf2e6..0944430dfb1f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -25,7 +25,6 @@
#include <net/net_failover.h>
#include <net/netdev_rx_queue.h>
#include <net/netdev_queues.h>
-#include <net/xdp_sock_drv.h>
static int napi_weight = NAPI_POLL_WEIGHT;
module_param(napi_weight, int, 0444);
@@ -349,11 +348,6 @@ struct receive_queue {
/* Record the last dma info to free after new pages is allocated. */
struct virtnet_rq_dma *last_dma;
-
- struct xsk_buff_pool *xsk_pool;
-
- /* xdp rxq used by xsk */
- struct xdp_rxq_info xsk_rxq_info;
};
/* This structure can contain rss message with maximum settings for indirection table and keysize
@@ -5065,132 +5059,6 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
return virtnet_set_guest_offloads(vi, offloads);
}
-static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queue *rq,
- struct xsk_buff_pool *pool)
-{
- int err, qindex;
-
- qindex = rq - vi->rq;
-
- if (pool) {
- err = xdp_rxq_info_reg(&rq->xsk_rxq_info, vi->dev, qindex, rq->napi.napi_id);
- if (err < 0)
- return err;
-
- err = xdp_rxq_info_reg_mem_model(&rq->xsk_rxq_info,
- MEM_TYPE_XSK_BUFF_POOL, NULL);
- if (err < 0)
- goto unreg;
-
- xsk_pool_set_rxq_info(pool, &rq->xsk_rxq_info);
- }
-
- virtnet_rx_pause(vi, rq);
-
- err = virtqueue_reset(rq->vq, virtnet_rq_unmap_free_buf);
- if (err) {
- netdev_err(vi->dev, "reset rx fail: rx queue index: %d err: %d\n", qindex, err);
-
- pool = NULL;
- }
-
- rq->xsk_pool = pool;
-
- virtnet_rx_resume(vi, rq);
-
- if (pool)
- return 0;
-
-unreg:
- xdp_rxq_info_unreg(&rq->xsk_rxq_info);
- return err;
-}
-
-static int virtnet_xsk_pool_enable(struct net_device *dev,
- struct xsk_buff_pool *pool,
- u16 qid)
-{
- struct virtnet_info *vi = netdev_priv(dev);
- struct receive_queue *rq;
- struct device *dma_dev;
- struct send_queue *sq;
- int err;
-
- if (vi->hdr_len > xsk_pool_get_headroom(pool))
- return -EINVAL;
-
- /* In big_packets mode, xdp cannot work, so there is no need to
- * initialize xsk of rq.
- */
- if (vi->big_packets && !vi->mergeable_rx_bufs)
- return -ENOENT;
-
- if (qid >= vi->curr_queue_pairs)
- return -EINVAL;
-
- sq = &vi->sq[qid];
- rq = &vi->rq[qid];
-
- /* xsk assumes that tx and rx must have the same dma device. The af-xdp
- * may use one buffer to receive from the rx and reuse this buffer to
- * send by the tx. So the dma dev of sq and rq must be the same one.
- *
- * But vq->dma_dev allows every vq has the respective dma dev. So I
- * check the dma dev of vq and sq is the same dev.
- */
- if (virtqueue_dma_dev(rq->vq) != virtqueue_dma_dev(sq->vq))
- return -EINVAL;
-
- dma_dev = virtqueue_dma_dev(rq->vq);
- if (!dma_dev)
- return -EINVAL;
-
- err = xsk_pool_dma_map(pool, dma_dev, 0);
- if (err)
- goto err_xsk_map;
-
- err = virtnet_rq_bind_xsk_pool(vi, rq, pool);
- if (err)
- goto err_rq;
-
- return 0;
-
-err_rq:
- xsk_pool_dma_unmap(pool, 0);
-err_xsk_map:
- return err;
-}
-
-static int virtnet_xsk_pool_disable(struct net_device *dev, u16 qid)
-{
- struct virtnet_info *vi = netdev_priv(dev);
- struct xsk_buff_pool *pool;
- struct receive_queue *rq;
- int err;
-
- if (qid >= vi->curr_queue_pairs)
- return -EINVAL;
-
- rq = &vi->rq[qid];
-
- pool = rq->xsk_pool;
-
- err = virtnet_rq_bind_xsk_pool(vi, rq, NULL);
-
- xsk_pool_dma_unmap(pool, 0);
-
- return err;
-}
-
-static int virtnet_xsk_pool_setup(struct net_device *dev, struct netdev_bpf *xdp)
-{
- if (xdp->xsk.pool)
- return virtnet_xsk_pool_enable(dev, xdp->xsk.pool,
- xdp->xsk.queue_id);
- else
- return virtnet_xsk_pool_disable(dev, xdp->xsk.queue_id);
-}
-
static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
struct netlink_ext_ack *extack)
{
@@ -5316,8 +5184,6 @@ static int virtnet_xdp(struct net_device *dev, struct netdev_bpf *xdp)
switch (xdp->command) {
case XDP_SETUP_PROG:
return virtnet_xdp_set(dev, xdp->prog, xdp->extack);
- case XDP_SETUP_XSK_POOL:
- return virtnet_xsk_pool_setup(dev, xdp);
default:
return -EINVAL;
}
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 5/7] Revert "virtio_net: rx remove premapped failover code"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
` (3 preceding siblings ...)
2024-09-06 9:52 ` [RFC PATCH v2 4/7] Revert "virtio_net: xsk: bind/unbind xsk for rx" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
2024-09-06 10:02 ` Xuan Zhuo
2024-09-06 9:52 ` [RFC PATCH v2 6/7] Revert "virtio_net: big mode skip the unmap check" Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 7/7] Revert "virtio_ring: enable premapped mode whatever use_dma_api" Michael S. Tsirkin
6 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez
This reverts commit defd28aa5acb0fd7c15adc6bc40a8ac277d04dea.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 89 +++++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 37 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0944430dfb1f..0a2ec9570521 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -348,6 +348,9 @@ struct receive_queue {
/* Record the last dma info to free after new pages is allocated. */
struct virtnet_rq_dma *last_dma;
+
+ /* Do dma by self */
+ bool do_dma;
};
/* This structure can contain rss message with maximum settings for indirection table and keysize
@@ -867,7 +870,7 @@ static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
void *buf;
buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
- if (buf)
+ if (buf && rq->do_dma)
virtnet_rq_unmap(rq, buf, *len);
return buf;
@@ -880,6 +883,11 @@ static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
u32 offset;
void *head;
+ if (!rq->do_dma) {
+ sg_init_one(rq->sg, buf, len);
+ return;
+ }
+
head = page_address(rq->alloc_frag.page);
offset = buf - head;
@@ -905,42 +913,44 @@ static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
head = page_address(alloc_frag->page);
- dma = head;
+ if (rq->do_dma) {
+ dma = head;
- /* new pages */
- if (!alloc_frag->offset) {
- if (rq->last_dma) {
- /* Now, the new page is allocated, the last dma
- * will not be used. So the dma can be unmapped
- * if the ref is 0.
+ /* new pages */
+ if (!alloc_frag->offset) {
+ if (rq->last_dma) {
+ /* Now, the new page is allocated, the last dma
+ * will not be used. So the dma can be unmapped
+ * if the ref is 0.
+ */
+ virtnet_rq_unmap(rq, rq->last_dma, 0);
+ rq->last_dma = NULL;
+ }
+
+ dma->len = alloc_frag->size - sizeof(*dma);
+
+ addr = virtqueue_dma_map_single_attrs(rq->vq, dma + 1,
+ dma->len, DMA_FROM_DEVICE, 0);
+ if (virtqueue_dma_mapping_error(rq->vq, addr))
+ return NULL;
+
+ dma->addr = addr;
+ dma->need_sync = virtqueue_dma_need_sync(rq->vq, addr);
+
+ /* Add a reference to dma to prevent the entire dma from
+ * being released during error handling. This reference
+ * will be freed after the pages are no longer used.
*/
- virtnet_rq_unmap(rq, rq->last_dma, 0);
- rq->last_dma = NULL;
+ get_page(alloc_frag->page);
+ dma->ref = 1;
+ alloc_frag->offset = sizeof(*dma);
+
+ rq->last_dma = dma;
}
- dma->len = alloc_frag->size - sizeof(*dma);
-
- addr = virtqueue_dma_map_single_attrs(rq->vq, dma + 1,
- dma->len, DMA_FROM_DEVICE, 0);
- if (virtqueue_dma_mapping_error(rq->vq, addr))
- return NULL;
-
- dma->addr = addr;
- dma->need_sync = virtqueue_dma_need_sync(rq->vq, addr);
-
- /* Add a reference to dma to prevent the entire dma from
- * being released during error handling. This reference
- * will be freed after the pages are no longer used.
- */
- get_page(alloc_frag->page);
- dma->ref = 1;
- alloc_frag->offset = sizeof(*dma);
-
- rq->last_dma = dma;
+ ++dma->ref;
}
- ++dma->ref;
-
buf = head + alloc_frag->offset;
get_page(alloc_frag->page);
@@ -957,9 +967,12 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
if (!vi->mergeable_rx_bufs && vi->big_packets)
return;
- for (i = 0; i < vi->max_queue_pairs; i++)
- /* error should never happen */
- BUG_ON(virtqueue_set_dma_premapped(vi->rq[i].vq));
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ if (virtqueue_set_dma_premapped(vi->rq[i].vq))
+ continue;
+
+ vi->rq[i].do_dma = true;
+ }
}
static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
@@ -2107,7 +2120,8 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
if (err < 0) {
- virtnet_rq_unmap(rq, buf, 0);
+ if (rq->do_dma)
+ virtnet_rq_unmap(rq, buf, 0);
put_page(virt_to_head_page(buf));
}
@@ -2221,7 +2235,8 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
ctx = mergeable_len_to_ctx(len + room, headroom);
err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
if (err < 0) {
- virtnet_rq_unmap(rq, buf, 0);
+ if (rq->do_dma)
+ virtnet_rq_unmap(rq, buf, 0);
put_page(virt_to_head_page(buf));
}
@@ -5392,7 +5407,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
int i;
for (i = 0; i < vi->max_queue_pairs; i++)
if (vi->rq[i].alloc_frag.page) {
- if (vi->rq[i].last_dma)
+ if (vi->rq[i].do_dma && vi->rq[i].last_dma)
virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
put_page(vi->rq[i].alloc_frag.page);
}
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH v2 5/7] Revert "virtio_net: rx remove premapped failover code"
2024-09-06 9:52 ` [RFC PATCH v2 5/7] Revert "virtio_net: rx remove premapped failover code" Michael S. Tsirkin
@ 2024-09-06 10:02 ` Xuan Zhuo
2024-09-06 10:12 ` Michael S. Tsirkin
0 siblings, 1 reply; 10+ messages in thread
From: Xuan Zhuo @ 2024-09-06 10:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, virtualization, Si-Wei Liu, Darren Kenny,
Boris Ostrovsky, Eugenio Pérez, linux-kernel, netdev
On Fri, 6 Sep 2024 05:52:36 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> This reverts commit defd28aa5acb0fd7c15adc6bc40a8ac277d04dea.
>
> leads to crashes with no ACCESS_PLATFORM when
> sysctl net.core.high_order_alloc_disable=1
>
> Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 89 +++++++++++++++++++++++-----------------
> 1 file changed, 52 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0944430dfb1f..0a2ec9570521 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -348,6 +348,9 @@ struct receive_queue {
>
> /* Record the last dma info to free after new pages is allocated. */
> struct virtnet_rq_dma *last_dma;
> +
> + /* Do dma by self */
> + bool do_dma;
> };
>
> /* This structure can contain rss message with maximum settings for indirection table and keysize
> @@ -867,7 +870,7 @@ static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> void *buf;
>
> buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> - if (buf)
> + if (buf && rq->do_dma)
> virtnet_rq_unmap(rq, buf, *len);
>
> return buf;
> @@ -880,6 +883,11 @@ static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
> u32 offset;
> void *head;
>
> + if (!rq->do_dma) {
> + sg_init_one(rq->sg, buf, len);
> + return;
> + }
> +
> head = page_address(rq->alloc_frag.page);
>
> offset = buf - head;
> @@ -905,42 +913,44 @@ static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
>
> head = page_address(alloc_frag->page);
>
> - dma = head;
> + if (rq->do_dma) {
> + dma = head;
>
> - /* new pages */
> - if (!alloc_frag->offset) {
> - if (rq->last_dma) {
> - /* Now, the new page is allocated, the last dma
> - * will not be used. So the dma can be unmapped
> - * if the ref is 0.
> + /* new pages */
> + if (!alloc_frag->offset) {
> + if (rq->last_dma) {
> + /* Now, the new page is allocated, the last dma
> + * will not be used. So the dma can be unmapped
> + * if the ref is 0.
> + */
> + virtnet_rq_unmap(rq, rq->last_dma, 0);
> + rq->last_dma = NULL;
> + }
> +
> + dma->len = alloc_frag->size - sizeof(*dma);
> +
> + addr = virtqueue_dma_map_single_attrs(rq->vq, dma + 1,
> + dma->len, DMA_FROM_DEVICE, 0);
> + if (virtqueue_dma_mapping_error(rq->vq, addr))
> + return NULL;
> +
> + dma->addr = addr;
> + dma->need_sync = virtqueue_dma_need_sync(rq->vq, addr);
> +
> + /* Add a reference to dma to prevent the entire dma from
> + * being released during error handling. This reference
> + * will be freed after the pages are no longer used.
> */
> - virtnet_rq_unmap(rq, rq->last_dma, 0);
> - rq->last_dma = NULL;
> + get_page(alloc_frag->page);
> + dma->ref = 1;
> + alloc_frag->offset = sizeof(*dma);
> +
> + rq->last_dma = dma;
> }
>
> - dma->len = alloc_frag->size - sizeof(*dma);
> -
> - addr = virtqueue_dma_map_single_attrs(rq->vq, dma + 1,
> - dma->len, DMA_FROM_DEVICE, 0);
> - if (virtqueue_dma_mapping_error(rq->vq, addr))
> - return NULL;
> -
> - dma->addr = addr;
> - dma->need_sync = virtqueue_dma_need_sync(rq->vq, addr);
> -
> - /* Add a reference to dma to prevent the entire dma from
> - * being released during error handling. This reference
> - * will be freed after the pages are no longer used.
> - */
> - get_page(alloc_frag->page);
> - dma->ref = 1;
> - alloc_frag->offset = sizeof(*dma);
> -
> - rq->last_dma = dma;
> + ++dma->ref;
> }
>
> - ++dma->ref;
> -
> buf = head + alloc_frag->offset;
>
> get_page(alloc_frag->page);
> @@ -957,9 +967,12 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> if (!vi->mergeable_rx_bufs && vi->big_packets)
> return;
>
> - for (i = 0; i < vi->max_queue_pairs; i++)
> - /* error should never happen */
> - BUG_ON(virtqueue_set_dma_premapped(vi->rq[i].vq));
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> + continue;
> +
> + vi->rq[i].do_dma = true;
> + }
This is too much code to revert. We can just revert this and next one.
And add a patch to turn off the default premapped setting (return from this
function directly). Otherwise, we will have to do all the work again in the
future.
There is no need to revert xsk related code, xsk function cannot be enabled, in
the case that premapped mode is not turned on. There is no direct impact itself.
Thanks.
> }
>
> static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> @@ -2107,7 +2120,8 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
>
> err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> if (err < 0) {
> - virtnet_rq_unmap(rq, buf, 0);
> + if (rq->do_dma)
> + virtnet_rq_unmap(rq, buf, 0);
> put_page(virt_to_head_page(buf));
> }
>
> @@ -2221,7 +2235,8 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> ctx = mergeable_len_to_ctx(len + room, headroom);
> err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> if (err < 0) {
> - virtnet_rq_unmap(rq, buf, 0);
> + if (rq->do_dma)
> + virtnet_rq_unmap(rq, buf, 0);
> put_page(virt_to_head_page(buf));
> }
>
> @@ -5392,7 +5407,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> int i;
> for (i = 0; i < vi->max_queue_pairs; i++)
> if (vi->rq[i].alloc_frag.page) {
> - if (vi->rq[i].last_dma)
> + if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> put_page(vi->rq[i].alloc_frag.page);
> }
> --
> MST
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH v2 5/7] Revert "virtio_net: rx remove premapped failover code"
2024-09-06 10:02 ` Xuan Zhuo
@ 2024-09-06 10:12 ` Michael S. Tsirkin
0 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 10:12 UTC (permalink / raw)
To: Xuan Zhuo
Cc: Jason Wang, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, virtualization, Si-Wei Liu, Darren Kenny,
Boris Ostrovsky, Eugenio Pérez, linux-kernel, netdev
On Fri, Sep 06, 2024 at 06:02:50PM +0800, Xuan Zhuo wrote:
> On Fri, 6 Sep 2024 05:52:36 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > This reverts commit defd28aa5acb0fd7c15adc6bc40a8ac277d04dea.
> >
> > leads to crashes with no ACCESS_PLATFORM when
> > sysctl net.core.high_order_alloc_disable=1
> >
> > Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/net/virtio_net.c | 89 +++++++++++++++++++++++-----------------
> > 1 file changed, 52 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0944430dfb1f..0a2ec9570521 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -348,6 +348,9 @@ struct receive_queue {
> >
> > /* Record the last dma info to free after new pages is allocated. */
> > struct virtnet_rq_dma *last_dma;
> > +
> > + /* Do dma by self */
> > + bool do_dma;
> > };
> >
> > /* This structure can contain rss message with maximum settings for indirection table and keysize
> > @@ -867,7 +870,7 @@ static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
> > void *buf;
> >
> > buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> > - if (buf)
> > + if (buf && rq->do_dma)
> > virtnet_rq_unmap(rq, buf, *len);
> >
> > return buf;
> > @@ -880,6 +883,11 @@ static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
> > u32 offset;
> > void *head;
> >
> > + if (!rq->do_dma) {
> > + sg_init_one(rq->sg, buf, len);
> > + return;
> > + }
> > +
> > head = page_address(rq->alloc_frag.page);
> >
> > offset = buf - head;
> > @@ -905,42 +913,44 @@ static void *virtnet_rq_alloc(struct receive_queue *rq, u32 size, gfp_t gfp)
> >
> > head = page_address(alloc_frag->page);
> >
> > - dma = head;
> > + if (rq->do_dma) {
> > + dma = head;
> >
> > - /* new pages */
> > - if (!alloc_frag->offset) {
> > - if (rq->last_dma) {
> > - /* Now, the new page is allocated, the last dma
> > - * will not be used. So the dma can be unmapped
> > - * if the ref is 0.
> > + /* new pages */
> > + if (!alloc_frag->offset) {
> > + if (rq->last_dma) {
> > + /* Now, the new page is allocated, the last dma
> > + * will not be used. So the dma can be unmapped
> > + * if the ref is 0.
> > + */
> > + virtnet_rq_unmap(rq, rq->last_dma, 0);
> > + rq->last_dma = NULL;
> > + }
> > +
> > + dma->len = alloc_frag->size - sizeof(*dma);
> > +
> > + addr = virtqueue_dma_map_single_attrs(rq->vq, dma + 1,
> > + dma->len, DMA_FROM_DEVICE, 0);
> > + if (virtqueue_dma_mapping_error(rq->vq, addr))
> > + return NULL;
> > +
> > + dma->addr = addr;
> > + dma->need_sync = virtqueue_dma_need_sync(rq->vq, addr);
> > +
> > + /* Add a reference to dma to prevent the entire dma from
> > + * being released during error handling. This reference
> > + * will be freed after the pages are no longer used.
> > */
> > - virtnet_rq_unmap(rq, rq->last_dma, 0);
> > - rq->last_dma = NULL;
> > + get_page(alloc_frag->page);
> > + dma->ref = 1;
> > + alloc_frag->offset = sizeof(*dma);
> > +
> > + rq->last_dma = dma;
> > }
> >
> > - dma->len = alloc_frag->size - sizeof(*dma);
> > -
> > - addr = virtqueue_dma_map_single_attrs(rq->vq, dma + 1,
> > - dma->len, DMA_FROM_DEVICE, 0);
> > - if (virtqueue_dma_mapping_error(rq->vq, addr))
> > - return NULL;
> > -
> > - dma->addr = addr;
> > - dma->need_sync = virtqueue_dma_need_sync(rq->vq, addr);
> > -
> > - /* Add a reference to dma to prevent the entire dma from
> > - * being released during error handling. This reference
> > - * will be freed after the pages are no longer used.
> > - */
> > - get_page(alloc_frag->page);
> > - dma->ref = 1;
> > - alloc_frag->offset = sizeof(*dma);
> > -
> > - rq->last_dma = dma;
> > + ++dma->ref;
> > }
> >
> > - ++dma->ref;
> > -
> > buf = head + alloc_frag->offset;
> >
> > get_page(alloc_frag->page);
> > @@ -957,9 +967,12 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
> > if (!vi->mergeable_rx_bufs && vi->big_packets)
> > return;
> >
> > - for (i = 0; i < vi->max_queue_pairs; i++)
> > - /* error should never happen */
> > - BUG_ON(virtqueue_set_dma_premapped(vi->rq[i].vq));
> > + for (i = 0; i < vi->max_queue_pairs; i++) {
> > + if (virtqueue_set_dma_premapped(vi->rq[i].vq))
> > + continue;
> > +
> > + vi->rq[i].do_dma = true;
> > + }
>
> This is too much code to revert. We can just revert this and next one.
> And add a patch to turn off the default premapped setting (return from this
> function directly). Otherwise, we will have to do all the work again in the
> future.
>
> There is no need to revert xsk related code, xsk function cannot be enabled, in
> the case that premapped mode is not turned on. There is no direct impact itself.
>
> Thanks.
I tried but quickly got lost as the automatic
revert did not work, and it's very close to release, so
I wanted to be sure it's right.
Post your own version of a revert for testing then please.
>
> > }
> >
> > static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
> > @@ -2107,7 +2120,8 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
> >
> > err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > if (err < 0) {
> > - virtnet_rq_unmap(rq, buf, 0);
> > + if (rq->do_dma)
> > + virtnet_rq_unmap(rq, buf, 0);
> > put_page(virt_to_head_page(buf));
> > }
> >
> > @@ -2221,7 +2235,8 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
> > ctx = mergeable_len_to_ctx(len + room, headroom);
> > err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
> > if (err < 0) {
> > - virtnet_rq_unmap(rq, buf, 0);
> > + if (rq->do_dma)
> > + virtnet_rq_unmap(rq, buf, 0);
> > put_page(virt_to_head_page(buf));
> > }
> >
> > @@ -5392,7 +5407,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
> > int i;
> > for (i = 0; i < vi->max_queue_pairs; i++)
> > if (vi->rq[i].alloc_frag.page) {
> > - if (vi->rq[i].last_dma)
> > + if (vi->rq[i].do_dma && vi->rq[i].last_dma)
> > virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
> > put_page(vi->rq[i].alloc_frag.page);
> > }
> > --
> > MST
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v2 6/7] Revert "virtio_net: big mode skip the unmap check"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
` (4 preceding siblings ...)
2024-09-06 9:52 ` [RFC PATCH v2 5/7] Revert "virtio_net: rx remove premapped failover code" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
2024-09-06 9:52 ` [RFC PATCH v2 7/7] Revert "virtio_ring: enable premapped mode whatever use_dma_api" Michael S. Tsirkin
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez
This reverts commit a377ae542d8d0a20a3173da3bbba72e045bea7a9.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0a2ec9570521..6f3c39dc6f76 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -983,7 +983,7 @@ static void virtnet_rq_unmap_free_buf(struct virtqueue *vq, void *buf)
rq = &vi->rq[i];
- if (!vi->big_packets || vi->mergeable_rx_bufs)
+ if (rq->do_dma)
virtnet_rq_unmap(rq, buf, 0);
virtnet_rq_free_buf(vi, rq, buf);
@@ -2367,7 +2367,7 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
}
} else {
while (packets < budget &&
- (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
+ (buf = virtnet_rq_get_buf(rq, &len, NULL)) != NULL) {
receive_buf(vi, rq, buf, len, NULL, xdp_xmit, &stats);
packets++;
}
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH v2 7/7] Revert "virtio_ring: enable premapped mode whatever use_dma_api"
2024-09-06 9:52 [RFC PATCH v2 0/7] Revert "virtio_net: rx enable premapped mode by default" Michael S. Tsirkin
` (5 preceding siblings ...)
2024-09-06 9:52 ` [RFC PATCH v2 6/7] Revert "virtio_net: big mode skip the unmap check" Michael S. Tsirkin
@ 2024-09-06 9:52 ` Michael S. Tsirkin
6 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2024-09-06 9:52 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Xuan Zhuo, Jason Wang, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, virtualization, Si-Wei Liu,
Darren Kenny, Boris Ostrovsky, Eugenio Pérez
This reverts commit f9dac92ba9081062a6477ee015bd3b8c5914efc4.
leads to crashes with no ACCESS_PLATFORM when
sysctl net.core.high_order_alloc_disable=1
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Reported-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index be7309b1e860..06b5bdf0920e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2782,7 +2782,7 @@ EXPORT_SYMBOL_GPL(virtqueue_resize);
*
* Returns zero or a negative error.
* 0: success.
- * -EINVAL: too late to enable premapped mode, the vq already contains buffers.
+ * -EINVAL: vring does not use the dma api, so we can not enable premapped mode.
*/
int virtqueue_set_dma_premapped(struct virtqueue *_vq)
{
@@ -2798,6 +2798,11 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
return -EINVAL;
}
+ if (!vq->use_dma_api) {
+ END_USE(vq);
+ return -EINVAL;
+ }
+
vq->premapped = true;
vq->do_unmap = false;
--
MST
^ permalink raw reply related [flat|nested] 10+ messages in thread