* [PATCH net] vhost: fix OOB in get_rx_bufs()
From: Jason Wang @ 2019-01-28 7:05 UTC (permalink / raw)
To: mst, jasowang, stefanha; +Cc: netdev, linux-kernel, kvm, virtualization
After batched used ring updating was introduced in commit e2b3b35eb989
("vhost_net: batch used ring update in rx"). We tend to batch heads in
vq->heads for more than one packet. But the quota passed to
get_rx_bufs() was not correctly limited, which can result a OOB write
in vq->heads.
headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
vhost_len, &in, vq_log, &log,
likely(mergeable) ? UIO_MAXIOV : 1);
UIO_MAXIOV was still used which is wrong since we could have batched
used in vq->heads, this will cause OOB if the next buffer needs more
than 960 (1024 (UIO_MAXIOV) - 64 (VHOST_NET_BATCH)) heads after we've
batched 64 (VHOST_NET_BATCH) heads:
=============================================================================
BUG kmalloc-8k (Tainted: G B ): Redzone overwritten
-----------------------------------------------------------------------------
INFO: 0x00000000fd93b7a2-0x00000000f0713384. First byte 0xa9 instead of 0xcc
INFO: Allocated in alloc_pd+0x22/0x60 age=3933677 cpu=2 pid=2674
kmem_cache_alloc_trace+0xbb/0x140
alloc_pd+0x22/0x60
gen8_ppgtt_create+0x11d/0x5f0
i915_ppgtt_create+0x16/0x80
i915_gem_create_context+0x248/0x390
i915_gem_context_create_ioctl+0x4b/0xe0
drm_ioctl_kernel+0xa5/0xf0
drm_ioctl+0x2ed/0x3a0
do_vfs_ioctl+0x9f/0x620
ksys_ioctl+0x6b/0x80
__x64_sys_ioctl+0x11/0x20
do_syscall_64+0x43/0xf0
entry_SYSCALL_64_after_hwframe+0x44/0xa9
INFO: Slab 0x00000000d13e87af objects=3 used=3 fp=0x (null) flags=0x200000000010201
INFO: Object 0x0000000003278802 @offset=17064 fp=0x00000000e2e6652b
Fixing this by allocating UIO_MAXIOV + VHOST_NET_BATCH iovs for
vhost-net. This is done through set the limitation through
vhost_dev_init(), then set_owner can allocate the number of iov in a
per device manner.
This fixes CVE-2018-16880.
Fixes: e2b3b35eb989 ("vhost_net: batch used ring update in rx")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 3 ++-
drivers/vhost/scsi.c | 2 +-
drivers/vhost/vhost.c | 7 ++++---
drivers/vhost/vhost.h | 4 +++-
drivers/vhost/vsock.c | 2 +-
5 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index bca86bf7189f..df51a35cf537 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1337,7 +1337,8 @@ static int vhost_net_open(struct inode *inode, struct file *f)
n->vqs[i].rx_ring = NULL;
vhost_net_buf_init(&n->vqs[i].rxq);
}
- vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
+ vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX,
+ UIO_MAXIOV + VHOST_NET_BATCH);
vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 344684f3e2e4..23593cb23dd0 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1627,7 +1627,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
vqs[i] = &vs->vqs[i].vq;
vs->vqs[i].vq.handle_kick = vhost_scsi_handle_kick;
}
- vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ);
+ vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ, UIO_MAXIOV);
vhost_scsi_init_inflight(vs, NULL);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 15a216cdd507..24a129fcdd61 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -390,9 +390,9 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
vq->indirect = kmalloc_array(UIO_MAXIOV,
sizeof(*vq->indirect),
GFP_KERNEL);
- vq->log = kmalloc_array(UIO_MAXIOV, sizeof(*vq->log),
+ vq->log = kmalloc_array(dev->iov_limit, sizeof(*vq->log),
GFP_KERNEL);
- vq->heads = kmalloc_array(UIO_MAXIOV, sizeof(*vq->heads),
+ vq->heads = kmalloc_array(dev->iov_limit, sizeof(*vq->heads),
GFP_KERNEL);
if (!vq->indirect || !vq->log || !vq->heads)
goto err_nomem;
@@ -414,7 +414,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev *dev)
}
void vhost_dev_init(struct vhost_dev *dev,
- struct vhost_virtqueue **vqs, int nvqs)
+ struct vhost_virtqueue **vqs, int nvqs, int iov_limit)
{
struct vhost_virtqueue *vq;
int i;
@@ -427,6 +427,7 @@ void vhost_dev_init(struct vhost_dev *dev,
dev->iotlb = NULL;
dev->mm = NULL;
dev->worker = NULL;
+ dev->iov_limit = iov_limit;
init_llist_head(&dev->work_list);
init_waitqueue_head(&dev->wait);
INIT_LIST_HEAD(&dev->read_list);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 1b675dad5e05..9490e7ddb340 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -170,9 +170,11 @@ struct vhost_dev {
struct list_head read_list;
struct list_head pending_list;
wait_queue_head_t wait;
+ int iov_limit;
};
-void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs);
+void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
+ int nvqs, int iov_limit);
long vhost_dev_set_owner(struct vhost_dev *dev);
bool vhost_dev_has_owner(struct vhost_dev *dev);
long vhost_dev_check_owner(struct vhost_dev *);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 3fbc068eaa9b..bb5fc0e9fbc2 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -531,7 +531,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick;
vsock->vqs[VSOCK_VQ_RX].handle_kick = vhost_vsock_handle_rx_kick;
- vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs));
+ vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs), UIO_MAXIOV);
file->private_data = vsock;
spin_lock_init(&vsock->send_pkt_list_lock);
--
2.17.1
^ permalink raw reply related
* Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain
From: hch @ 2019-01-28 8:00 UTC (permalink / raw)
To: Peng Fan
Cc: jgross@suse.com, Stefano Stabellini, Andy Duan, mst@redhat.com,
linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, hch@infradead.org,
luto@kernel.org, xen-devel@lists.xenproject.org,
boris.ostrovsky@oracle.com
In-Reply-To: <AM0PR04MB4481F3986CFB6D1EF26FA135889B0@AM0PR04MB4481.eurprd04.prod.outlook.com>
On Fri, Jan 25, 2019 at 09:45:26AM +0000, Peng Fan wrote:
> Just have a question,
>
> Since vmalloc_to_page is ok for cma area, no need to take cma and per device
> cma into consideration right?
The CMA area itself it a physical memory region. If it is a non-highmem
region you can call virt_to_page on the virtual addresses for it. If
it is in highmem it doesn't even have a kernel virtual address by
default.
> we only need to implement a piece code to handle per device specific region
> using RESERVEDMEM_OF_DECLARE, just like:
> RESERVEDMEM_OF_DECLARE(rpmsg-dma, "rpmsg-dma-pool",
> rmem_rpmsg_dma_setup);
> And implement the device_init call back and build a map between page and phys.
> Then in rpmsg driver, scatter list could use page structure, no need vmalloc_to_page
> for per device dma.
>
> Is this the right way?
I think this should work fine. If you have the cycles for it I'd
actually love to be able to have generic CMA DT glue for non DMA API
driver allocations, as there obviously is a need for it. So basically
the same as above, just added to kernel/cma.c as a generic API.
^ permalink raw reply
* Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
From: Christoph Hellwig @ 2019-01-28 8:05 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel, Christoph Hellwig
In-Reply-To: <20190124095150.GL32526@8bytes.org>
On Thu, Jan 24, 2019 at 10:51:51AM +0100, Joerg Roedel wrote:
> On Thu, Jan 24, 2019 at 09:42:21AM +0100, Christoph Hellwig wrote:
> > Yes. But more importantly it would fix the limit for all other block
> > drivers that set large segment sizes when running over swiotlb.
>
> True, so it would be something like the diff below? I havn't worked on
> the block layer, so I don't know if that needs additional checks for
> ->dev or anything.
Looks sensible. Maybe for now we'll just do the virtio-blk case
that triggered it, and we'll do something like this patch for the
next merge window. We'll also need to apply the same magic to the
DMA boundary.
^ permalink raw reply
* Re: [PATCH v3 10/23] drm/qxl: move qxl_primary_apply_cursor to correct place
From: Gerd Hoffmann @ 2019-01-28 8:10 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <b3c756b3-c7b6-f4d3-5208-c8de0168d655@tronnes.org>
> > The cursor must be set again after creating the primary surface.
> > Also drop the error message.
> > if (!bo->is_primary) {
> > - if (!same_shadow)
> > + if (!same_shadow) {
> > qxl_io_create_primary(qdev, 0, bo);
> > + qxl_primary_apply_cursor(plane);
> > + }
> > bo->is_primary = true;
> > }
> >
> >
>
> I don't see how the commit message matches what you're doing. It gives
> the impression that it must be applied under yet another condition, but
> the condition for applying the cursor is changed from bo_old->is_primary
> to !bo->is_primary.
The qxl device ties the cursor to the primary surface. Therefore
calling qxl_io_destroy_primary() and qxl_io_create_primary() to switch
the framebuffer causes the cursor information being lost and the driver
must re-apply it.
The correct call order to do that is qxl_io_destroy_primary() +
qxl_io_create_primary() + qxl_primary_apply_cursor().
The old code did qxl_io_destroy_primary() + qxl_primary_apply_cursor() +
qxl_io_create_primary(). Due to qxl_primary_apply_cursor request being
queued in a ringbuffer and qxl_io_create_primary() trapping to the
hypervisor instantly there is a high chance that qxl_io_create_primary()
is processed first even with the wrong call order. But it's racy and
thus not reliable.
> It probably makes sense to someone that knows the driver.
If the above explains things better to you I should probably replace the
commit message with that.
> Acked-by: Noralf Trønnes <noralf@tronnes.org>
thanks,
Gerd
^ permalink raw reply
* Re: [PATCH v3 17/23] drm/qxl: use generic fbdev emulation
From: Gerd Hoffmann @ 2019-01-28 8:59 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <e31c0fdd-315a-59cf-e7fe-dfd6a064f6ee@tronnes.org>
On Fri, Jan 25, 2019 at 06:25:27PM +0100, Noralf Trønnes wrote:
>
>
> Den 18.01.2019 13.20, skrev Gerd Hoffmann:
> > Switch qxl over to the new generic fbdev emulation.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > drivers/gpu/drm/qxl/qxl_display.c | 7 -------
> > drivers/gpu/drm/qxl/qxl_drv.c | 2 ++
> > 2 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> > index ef832f98ab..9c751f01e3 100644
> > --- a/drivers/gpu/drm/qxl/qxl_display.c
> > +++ b/drivers/gpu/drm/qxl/qxl_display.c
> > @@ -1221,18 +1221,11 @@ int qxl_modeset_init(struct qxl_device *qdev)
> > qxl_display_read_client_monitors_config(qdev);
> >
> > drm_mode_config_reset(&qdev->ddev);
> > -
> > - /* primary surface must be created by this point, to allow
> > - * issuing command queue commands and having them read by
> > - * spice server. */
> > - qxl_fbdev_init(qdev);
> > return 0;
> > }
> >
> > void qxl_modeset_fini(struct qxl_device *qdev)
> > {
> > - qxl_fbdev_fini(qdev);
> > -
> > qxl_destroy_monitors_object(qdev);
> > drm_mode_config_cleanup(&qdev->ddev);
> > }
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 13c8a662f9..3fce7d16df 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -93,6 +93,8 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > if (ret)
> > goto modeset_cleanup;
> >
> > + drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
>
> I couldn't find that this was part of old fbdev code, so it would be
> nice to mention it in the commit message.
It actually is, but a bit hidden because it doesn't use a helper you can
easily grep for. Instead sets fb_info->apertures->ranges[0] in
qxlfb_create(), which has the same effect.
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH v3 10/23] drm/qxl: move qxl_primary_apply_cursor to correct place
From: Noralf Trønnes @ 2019-01-28 10:38 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <20190128081005.j6z2ecnytuocfidh@sirius.home.kraxel.org>
Den 28.01.2019 09.10, skrev Gerd Hoffmann:
>>> The cursor must be set again after creating the primary surface.
>>> Also drop the error message.
>
>>> if (!bo->is_primary) {
>>> - if (!same_shadow)
>>> + if (!same_shadow) {
>>> qxl_io_create_primary(qdev, 0, bo);
>>> + qxl_primary_apply_cursor(plane);
>>> + }
>>> bo->is_primary = true;
>>> }
>>>
>>>
>>
>> I don't see how the commit message matches what you're doing. It gives
>> the impression that it must be applied under yet another condition, but
>> the condition for applying the cursor is changed from bo_old->is_primary
>> to !bo->is_primary.
>
> The qxl device ties the cursor to the primary surface. Therefore
> calling qxl_io_destroy_primary() and qxl_io_create_primary() to switch
> the framebuffer causes the cursor information being lost and the driver
> must re-apply it.
>
> The correct call order to do that is qxl_io_destroy_primary() +
> qxl_io_create_primary() + qxl_primary_apply_cursor().
>
> The old code did qxl_io_destroy_primary() + qxl_primary_apply_cursor() +
> qxl_io_create_primary(). Due to qxl_primary_apply_cursor request being
> queued in a ringbuffer and qxl_io_create_primary() trapping to the
> hypervisor instantly there is a high chance that qxl_io_create_primary()
> is processed first even with the wrong call order. But it's racy and
> thus not reliable.
>
>> It probably makes sense to someone that knows the driver.
>
> If the above explains things better to you I should probably replace the
> commit message with that.
>
This is actually my first review of a driver that I'm not familiar with.
I'm not quite sure how much in depth understanding that is required to
put my ack on it. Going further into the patchset I realised that
there's no way that I can verify the logic without being intimate with
the driver. So I have tried to verify things from a kms point of view.
I liked your expanded explanation better.
Noralf.
>> Acked-by: Noralf Trønnes <noralf@tronnes.org>
>
> thanks,
> Gerd
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 17/23] drm/qxl: use generic fbdev emulation
From: Noralf Trønnes @ 2019-01-28 10:39 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <20190128085951.gkap3by26xgkbwaa@sirius.home.kraxel.org>
Den 28.01.2019 09.59, skrev Gerd Hoffmann:
> On Fri, Jan 25, 2019 at 06:25:27PM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 18.01.2019 13.20, skrev Gerd Hoffmann:
>>> Switch qxl over to the new generic fbdev emulation.
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>> drivers/gpu/drm/qxl/qxl_display.c | 7 -------
>>> drivers/gpu/drm/qxl/qxl_drv.c | 2 ++
>>> 2 files changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
>>> index ef832f98ab..9c751f01e3 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_display.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_display.c
>>> @@ -1221,18 +1221,11 @@ int qxl_modeset_init(struct qxl_device *qdev)
>>> qxl_display_read_client_monitors_config(qdev);
>>>
>>> drm_mode_config_reset(&qdev->ddev);
>>> -
>>> - /* primary surface must be created by this point, to allow
>>> - * issuing command queue commands and having them read by
>>> - * spice server. */
>>> - qxl_fbdev_init(qdev);
>>> return 0;
>>> }
>>>
>>> void qxl_modeset_fini(struct qxl_device *qdev)
>>> {
>>> - qxl_fbdev_fini(qdev);
>>> -
>>> qxl_destroy_monitors_object(qdev);
>>> drm_mode_config_cleanup(&qdev->ddev);
>>> }
>>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>>> index 13c8a662f9..3fce7d16df 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>>> @@ -93,6 +93,8 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>> if (ret)
>>> goto modeset_cleanup;
>>>
>>> + drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
>>
>> I couldn't find that this was part of old fbdev code, so it would be
>> nice to mention it in the commit message.
>
> It actually is, but a bit hidden because it doesn't use a helper you can
> easily grep for. Instead sets fb_info->apertures->ranges[0] in
> qxlfb_create(), which has the same effect.
>
Indeed,
Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 10/23] drm/qxl: move qxl_primary_apply_cursor to correct place
From: Gerd Hoffmann @ 2019-01-28 11:40 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <44e41026-e091-659d-ac9c-fefa47e2335b@tronnes.org>
Hi,
> > If the above explains things better to you I should probably replace the
> > commit message with that.
>
> This is actually my first review of a driver that I'm not familiar with.
> I'm not quite sure how much in depth understanding that is required to
> put my ack on it.
Usually I try to show that by picking either Reviewed-by (I'm confident
the changes are fine) or Acked-by (Changes look sane, but I don't know
the code base and/or hardware good enough to be sure).
> Going further into the patchset I realised that
> there's no way that I can verify the logic without being intimate with
> the driver.
Yep. Same for me when looking at patches for other drivers. I think
this is one reason why it isn't that easy to get patches for drivers
reviewed where effectively only a single maintainer/developer is working
on.
> I liked your expanded explanation better.
Updated the commit message.
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH 0/5 v3] Fix virtio-blk issue with SWIOTLB
From: Michael S. Tsirkin @ 2019-01-28 15:20 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Jens Axboe, jroedel, brijesh.singh, mst, Joerg Roedel, jon.grimm,
jfehlig, linux-kernel, virtualization, linux-block, iommu,
Christoph Hellwig
In-Reply-To: <20190123211453.GA2474@char.us.oracle.com>
On Wed, Jan 23, 2019 at 04:14:53PM -0500, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 23, 2019 at 01:51:29PM -0500, Michael S. Tsirkin wrote:
> > On Wed, Jan 23, 2019 at 05:30:44PM +0100, Joerg Roedel wrote:
> > > Hi,
> > >
> > > here is the third version of this patch-set. Previous
> > > versions can be found here:
> > >
> > > V1: https://lore.kernel.org/lkml/20190110134433.15672-1-joro@8bytes.org/
> > >
> > > V2: https://lore.kernel.org/lkml/20190115132257.6426-1-joro@8bytes.org/
> > >
> > > The problem solved here is a limitation of the SWIOTLB implementation,
> > > which does not support allocations larger than 256kb. When the
> > > virtio-blk driver tries to read/write a block larger than that, the
> > > allocation of the dma-handle fails and an IO error is reported.
> >
> >
> > OK looks good to me.
> > I will park this in my tree for now this way it will get
> > testing in linux-next.
> > Can I get an ack from DMA maintainers on the DMA bits for
> > merging this in 5.0?
>
> You got mine (SWIOTBL is my area).
OK so
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >
> > > Changes to v2 are:
> > >
> > > * Check if SWIOTLB is active before returning its limit in
> > > dma_direct_max_mapping_size()
> > >
> > > * Only apply the maximum segment limit in virtio-blk when
> > > DMA-API is used for the vring
> > >
> > > Please review.
> > >
> > > Thanks,
> > >
> > > Joerg
> > >
> > > Joerg Roedel (5):
> > > swiotlb: Introduce swiotlb_max_mapping_size()
> > > swiotlb: Add is_swiotlb_active() function
> > > dma: Introduce dma_max_mapping_size()
> > > virtio: Introduce virtio_max_dma_size()
> > > virtio-blk: Consider virtio_max_dma_size() for maximum segment size
> > >
> > > drivers/block/virtio_blk.c | 10 ++++++----
> > > drivers/virtio/virtio_ring.c | 10 ++++++++++
> > > include/linux/dma-mapping.h | 16 ++++++++++++++++
> > > include/linux/swiotlb.h | 11 +++++++++++
> > > include/linux/virtio.h | 2 ++
> > > kernel/dma/direct.c | 11 +++++++++++
> > > kernel/dma/swiotlb.c | 10 ++++++++++
> > > 7 files changed, 66 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.17.1
^ permalink raw reply
* Re: [PATCH 0/5 v3] Fix virtio-blk issue with SWIOTLB
From: Konrad Rzeszutek Wilk @ 2019-01-28 15:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, jroedel, brijesh.singh, Joerg Roedel, jon.grimm,
jfehlig, linux-kernel, virtualization, linux-block, iommu,
Christoph Hellwig
In-Reply-To: <20190128101852-mutt-send-email-mst@kernel.org>
On Mon, Jan 28, 2019 at 10:20:05AM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 04:14:53PM -0500, Konrad Rzeszutek Wilk wrote:
> > On Wed, Jan 23, 2019 at 01:51:29PM -0500, Michael S. Tsirkin wrote:
> > > On Wed, Jan 23, 2019 at 05:30:44PM +0100, Joerg Roedel wrote:
> > > > Hi,
> > > >
> > > > here is the third version of this patch-set. Previous
> > > > versions can be found here:
> > > >
> > > > V1: https://lore.kernel.org/lkml/20190110134433.15672-1-joro@8bytes.org/
> > > >
> > > > V2: https://lore.kernel.org/lkml/20190115132257.6426-1-joro@8bytes.org/
> > > >
> > > > The problem solved here is a limitation of the SWIOTLB implementation,
> > > > which does not support allocations larger than 256kb. When the
> > > > virtio-blk driver tries to read/write a block larger than that, the
> > > > allocation of the dma-handle fails and an IO error is reported.
> > >
> > >
> > > OK looks good to me.
> > > I will park this in my tree for now this way it will get
> > > testing in linux-next.
> > > Can I get an ack from DMA maintainers on the DMA bits for
> > > merging this in 5.0?
> >
> > You got mine (SWIOTBL is my area).
>
> OK so
>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Yes :-)
>
>
>
> > >
> > > > Changes to v2 are:
> > > >
> > > > * Check if SWIOTLB is active before returning its limit in
> > > > dma_direct_max_mapping_size()
> > > >
> > > > * Only apply the maximum segment limit in virtio-blk when
> > > > DMA-API is used for the vring
> > > >
> > > > Please review.
> > > >
> > > > Thanks,
> > > >
> > > > Joerg
> > > >
> > > > Joerg Roedel (5):
> > > > swiotlb: Introduce swiotlb_max_mapping_size()
> > > > swiotlb: Add is_swiotlb_active() function
> > > > dma: Introduce dma_max_mapping_size()
> > > > virtio: Introduce virtio_max_dma_size()
> > > > virtio-blk: Consider virtio_max_dma_size() for maximum segment size
> > > >
> > > > drivers/block/virtio_blk.c | 10 ++++++----
> > > > drivers/virtio/virtio_ring.c | 10 ++++++++++
> > > > include/linux/dma-mapping.h | 16 ++++++++++++++++
> > > > include/linux/swiotlb.h | 11 +++++++++++
> > > > include/linux/virtio.h | 2 ++
> > > > kernel/dma/direct.c | 11 +++++++++++
> > > > kernel/dma/swiotlb.c | 10 ++++++++++
> > > > 7 files changed, 66 insertions(+), 4 deletions(-)
> > > >
> > > > --
> > > > 2.17.1
^ permalink raw reply
* Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
From: Michael S. Tsirkin @ 2019-01-28 17:14 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, jroedel, brijesh.singh, Konrad Rzeszutek Wilk,
Joerg Roedel, jon.grimm, jfehlig, linux-kernel, virtualization,
linux-block, iommu
In-Reply-To: <20190128080526.GA3874@lst.de>
On Mon, Jan 28, 2019 at 09:05:26AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 24, 2019 at 10:51:51AM +0100, Joerg Roedel wrote:
> > On Thu, Jan 24, 2019 at 09:42:21AM +0100, Christoph Hellwig wrote:
> > > Yes. But more importantly it would fix the limit for all other block
> > > drivers that set large segment sizes when running over swiotlb.
> >
> > True, so it would be something like the diff below? I havn't worked on
> > the block layer, so I don't know if that needs additional checks for
> > ->dev or anything.
>
> Looks sensible. Maybe for now we'll just do the virtio-blk case
> that triggered it, and we'll do something like this patch for the
> next merge window. We'll also need to apply the same magic to the
> DMA boundary.
So do I get an ack for this patch then?
--
MST
^ permalink raw reply
* Re: [PATCH 2/5] swiotlb: Add is_swiotlb_active() function
From: Michael S. Tsirkin @ 2019-01-28 17:19 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jens Axboe, jroedel, brijesh.singh, Konrad Rzeszutek Wilk,
jon.grimm, jfehlig, linux-kernel, virtualization, linux-block,
iommu, Christoph Hellwig
In-Reply-To: <20190124150000.GN32526@8bytes.org>
On Thu, Jan 24, 2019 at 04:00:00PM +0100, Joerg Roedel wrote:
> On Thu, Jan 24, 2019 at 09:41:07AM +0100, Christoph Hellwig wrote:
> > On Thu, Jan 24, 2019 at 09:29:23AM +0100, Joerg Roedel wrote:
> > > > As I've just introduced and fixed a bug in this area in the current
> > > > cycle - I don't think no_iotlb_memory is what your want (and maybe
> > > > not useful at all): if the arch valls swiotlb_exit after previously
> > > > initializing a buffer it won't be set. You probably want to check
> > > > for non-zero io_tlb_start and/or io_tlb_end.
> > >
> > > Okay, but that requires that I also set io_tlb_start and friends back to
> > > zero in the failure path of swiotlb_init(). Otherwise it could be left
> > > non-zero in case swiotlb_init_with_tbl() returns an error.
> >
> > Indeed, and we'll need to do that anyway as otherwise the dma mapping
> > path might cause problems similar to the one when swiotlb_exit is
> > called that I fixed.
>
> Turns out the the error path in swiotlb_init() is redundant because it
> will never be executed. If the function returns it will always return 0
> because in case of failure it will just panic (through memblock_alloc).
>
> I'll clean that up in a separate patch-set. There are more users of that
> function and all of them panic when the function fails.
>
>
> Joerg
OK so are you going to post a new version then? Time's running out for 5.0.
This isn't a regression so maybe we should just defer it all to 5.1.
--
MST
^ permalink raw reply
* [PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, Willem de Bruijn, virtualization, Jesper Dangaard Brouer
While I'm looking into how to account standard tx counters on XDP tx
processing, I found several bugs around XDP tx and napi_tx.
Patch1: Fix oops on error path. Patch2 depends on this.
Patch2: Fix memory corruption on freeing xdp_frames with napi_tx enabled.
Patch3: Minor fix patch5 depends on.
Patch4: Fix memory corruption on processing xdp_frames when XDP is disabled.
Also patch5 depends on this.
Patch5: Fix memory corruption on processing xdp_frames while XDP is being
disabled.
Patch6: Minor fix patch7 depends on.
Patch7: Fix memory corruption on freeing sk_buff or xdp_frames when a normal
queue is reused for XDP and vise versa.
v2:
- patch5: Make rcu_assign_pointer/synchronize_net conditional instead of
_virtnet_set_queues.
- patch7: Use napi_consume_skb() instead of dev_consume_skb_any()
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Toshiaki Makita (7):
virtio_net: Don't enable NAPI when interface is down
virtio_net: Don't call free_old_xmit_skbs for xdp_frames
virtio_net: Fix not restoring real_num_rx_queues
virtio_net: Fix out of bounds access of sq
virtio_net: Don't process redirected XDP frames when XDP is disabled
virtio_net: Use xdp_return_frame to free xdp_frames on destroying vqs
virtio_net: Differentiate sk_buff and xdp_frame on freeing
drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++--------------
1 file changed, 112 insertions(+), 47 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH v2 net 1/7] virtio_net: Don't enable NAPI when interface is down
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang; +Cc: netdev, virtualization
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
Commit 4e09ff536284 ("virtio-net: disable NAPI only when enabled during
XDP set") tried to fix inappropriate NAPI enabling/disabling when
!netif_running(), but was not complete.
On error path virtio_net could enable NAPI even when !netif_running().
This can cause enabling NAPI twice on virtnet_open(), which would
trigger BUG_ON() in napi_enable().
Fixes: 4941d472bf95b ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8fadd8e..8e4c5d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2430,8 +2430,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
return 0;
err:
- for (i = 0; i < vi->max_queue_pairs; i++)
- virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+ if (netif_running(dev)) {
+ for (i = 0; i < vi->max_queue_pairs; i++)
+ virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+ }
if (prog)
bpf_prog_sub(prog, vi->max_queue_pairs - 1);
return err;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net 2/7] virtio_net: Don't call free_old_xmit_skbs for xdp_frames
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, Willem de Bruijn, virtualization
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
When napi_tx is enabled, virtnet_poll_cleantx() called
free_old_xmit_skbs() even for xdp send queue.
This is bogus since the queue has xdp_frames, not sk_buffs, thus mangled
device tx bytes counters because skb->len is meaningless value, and even
triggered oops due to general protection fault on freeing them.
Since xdp send queues do not aquire locks, old xdp_frames should be
freed only in virtnet_xdp_xmit(), so just skip free_old_xmit_skbs() for
xdp send queues.
Similarly virtnet_poll_tx() called free_old_xmit_skbs(). This NAPI
handler is called even without calling start_xmit() because cb for tx is
by default enabled. Once the handler is called, it enabled the cb again,
and then the handler would be called again. We don't need this handler
for XDP, so don't enable cb as well as not calling free_old_xmit_skbs().
Also, we need to disable tx NAPI when disabling XDP, so
virtnet_poll_tx() can safely access curr_queue_pairs and
xdp_queue_pairs, which are not atomically updated while disabling XDP.
Fixes: b92f1e6751a6 ("virtio-net: transmit napi")
Fixes: 7b0411ef4aa6 ("virtio-net: clean tx descriptors from rx napi")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8e4c5d4..046f955 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1358,6 +1358,16 @@ static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
u64_stats_update_end(&sq->stats.syncp);
}
+static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
+{
+ if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
+ return false;
+ else if (q < vi->curr_queue_pairs)
+ return true;
+ else
+ return false;
+}
+
static void virtnet_poll_cleantx(struct receive_queue *rq)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
@@ -1365,7 +1375,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq)
struct send_queue *sq = &vi->sq[index];
struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, index);
- if (!sq->napi.weight)
+ if (!sq->napi.weight || is_xdp_raw_buffer_queue(vi, index))
return;
if (__netif_tx_trylock(txq)) {
@@ -1442,8 +1452,16 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
{
struct send_queue *sq = container_of(napi, struct send_queue, napi);
struct virtnet_info *vi = sq->vq->vdev->priv;
- struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq));
+ unsigned int index = vq2txq(sq->vq);
+ struct netdev_queue *txq;
+ if (unlikely(is_xdp_raw_buffer_queue(vi, index))) {
+ /* We don't need to enable cb for XDP */
+ napi_complete_done(napi, 0);
+ return 0;
+ }
+
+ txq = netdev_get_tx_queue(vi->dev, index);
__netif_tx_lock(txq, raw_smp_processor_id());
free_old_xmit_skbs(sq, true);
__netif_tx_unlock(txq);
@@ -2402,9 +2420,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
}
/* Make sure NAPI is not using any XDP TX queues for RX. */
- if (netif_running(dev))
- for (i = 0; i < vi->max_queue_pairs; i++)
+ if (netif_running(dev)) {
+ for (i = 0; i < vi->max_queue_pairs; i++) {
napi_disable(&vi->rq[i].napi);
+ virtnet_napi_tx_disable(&vi->sq[i].napi);
+ }
+ }
netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
@@ -2423,16 +2444,22 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
}
if (old_prog)
bpf_prog_put(old_prog);
- if (netif_running(dev))
+ if (netif_running(dev)) {
virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+ virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+ &vi->sq[i].napi);
+ }
}
return 0;
err:
if (netif_running(dev)) {
- for (i = 0; i < vi->max_queue_pairs; i++)
+ for (i = 0; i < vi->max_queue_pairs; i++) {
virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+ virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+ &vi->sq[i].napi);
+ }
}
if (prog)
bpf_prog_sub(prog, vi->max_queue_pairs - 1);
@@ -2615,16 +2642,6 @@ static void free_receive_page_frags(struct virtnet_info *vi)
put_page(vi->rq[i].alloc_frag.page);
}
-static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
-{
- if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
- return false;
- else if (q < vi->curr_queue_pairs)
- return true;
- else
- return false;
-}
-
static void free_unused_bufs(struct virtnet_info *vi)
{
void *buf;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net 3/7] virtio_net: Fix not restoring real_num_rx_queues
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang; +Cc: netdev, virtualization
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
When _virtnet_set_queues() failed we did not restore real_num_rx_queues.
Fix this by placing the change of real_num_rx_queues after
_virtnet_set_queues().
This order is also in line with virtnet_set_channels().
Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 046f955..0e1a369 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2427,10 +2427,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
}
}
- netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
if (err)
goto err;
+ netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
vi->xdp_queue_pairs = xdp_qp;
for (i = 0; i < vi->max_queue_pairs; i++) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net 4/7] virtio_net: Fix out of bounds access of sq
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang; +Cc: netdev, virtualization
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
When XDP is disabled, curr_queue_pairs + smp_processor_id() can be
larger than max_queue_pairs.
There is no guarantee that we have enough XDP send queues dedicated for
each cpu when XDP is disabled, so do not count drops on sq in that case.
Fixes: 5b8f3c8d30a6 ("virtio_net: Add XDP related stats")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0e1a369..669b65c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -491,20 +491,17 @@ static int virtnet_xdp_xmit(struct net_device *dev,
int ret, err;
int i;
- sq = virtnet_xdp_sq(vi);
-
- if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
- ret = -EINVAL;
- drops = n;
- goto out;
- }
-
/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
* indicate XDP resources have been successfully allocated.
*/
xdp_prog = rcu_dereference(rq->xdp_prog);
- if (!xdp_prog) {
- ret = -ENXIO;
+ if (!xdp_prog)
+ return -ENXIO;
+
+ sq = virtnet_xdp_sq(vi);
+
+ if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
+ ret = -EINVAL;
drops = n;
goto out;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, virtualization, Jesper Dangaard Brouer
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
ready for XDP") tried to avoid access to unexpected sq while XDP is
disabled, but was not complete.
There was a small window which causes out of bounds sq access in
virtnet_xdp_xmit() while disabling XDP.
An example case of
- curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
- online_cpu_num = xdp_queue_paris = 4
when XDP is enabled:
CPU 0 CPU 1
(Disabling XDP) (Processing redirected XDP frames)
virtnet_xdp_xmit()
virtnet_xdp_set()
_virtnet_set_queues()
set curr_queue_pairs (2)
check if rq->xdp_prog is not NULL
virtnet_xdp_sq(vi)
qp = curr_queue_pairs -
xdp_queue_pairs +
smp_processor_id()
= 2 - 4 + 1 = -1
sq = &vi->sq[qp] // out of bounds access
set xdp_queue_pairs (0)
rq->xdp_prog = NULL
Basically we should not change curr_queue_pairs and xdp_queue_pairs
while someone can read the values. Thus, when disabling XDP, assign NULL
to rq->xdp_prog first, and wait for RCU grace period, then change
xxx_queue_pairs.
Note that we need to keep the current order when enabling XDP though.
- v2: Make rcu_assign_pointer/synchronize_net conditional instead of
_virtnet_set_queues.
Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 669b65c..cea52e4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2410,6 +2410,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
return -ENOMEM;
}
+ old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
+ if (!prog && !old_prog)
+ return 0;
+
if (prog) {
prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
if (IS_ERR(prog))
@@ -2424,21 +2428,30 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
}
}
+ if (!prog) {
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
+ if (i == 0)
+ virtnet_restore_guest_offloads(vi);
+ }
+ synchronize_net();
+ }
+
err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
if (err)
goto err;
netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
vi->xdp_queue_pairs = xdp_qp;
- for (i = 0; i < vi->max_queue_pairs; i++) {
- old_prog = rtnl_dereference(vi->rq[i].xdp_prog);
- rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
- if (i == 0) {
- if (!old_prog)
+ if (prog) {
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
+ if (i == 0 && !old_prog)
virtnet_clear_guest_offloads(vi);
- if (!prog)
- virtnet_restore_guest_offloads(vi);
}
+ }
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
if (old_prog)
bpf_prog_put(old_prog);
if (netif_running(dev)) {
@@ -2451,6 +2464,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
return 0;
err:
+ if (!prog) {
+ virtnet_clear_guest_offloads(vi);
+ for (i = 0; i < vi->max_queue_pairs; i++)
+ rcu_assign_pointer(vi->rq[i].xdp_prog, old_prog);
+ }
+
if (netif_running(dev)) {
for (i = 0; i < vi->max_queue_pairs; i++) {
virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net 6/7] virtio_net: Use xdp_return_frame to free xdp_frames on destroying vqs
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, virtualization, Jesper Dangaard Brouer
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
put_page() can work as a fallback for freeing xdp_frames, but the
appropriate way is to use xdp_return_frame().
Fixes: cac320c850ef ("virtio_net: convert to use generic xdp_frame and xdp_return_frame API")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cea52e4..1d454ce 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2669,7 +2669,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
if (!is_xdp_raw_buffer_queue(vi, i))
dev_kfree_skb(buf);
else
- put_page(virt_to_head_page(buf));
+ xdp_return_frame(buf);
}
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing
From: Toshiaki Makita @ 2019-01-29 0:45 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang; +Cc: netdev, virtualization
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
We do not reset or free up unused buffers when enabling/disabling XDP,
so it can happen that xdp_frames are freed after disabling XDP or
sk_buffs are freed after enabling XDP on xdp tx queues.
Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
of XDP setting.
One way to trigger this problem is to disable XDP when napi_tx is
enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
which invokes free_old_xmit_skbs() for queues which have been used by
XDP.
Note that even with this change we need to keep skipping
free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
tx queues do not aquire queue locks.
- v2: Use napi_consume_skb() instead of dev_consume_skb_any()
Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
NOTE: Dropped Acked-by because of the v2 change.
drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 12 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1d454ce..2594481 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -57,6 +57,8 @@
#define VIRTIO_XDP_TX BIT(0)
#define VIRTIO_XDP_REDIR BIT(1)
+#define VIRTIO_XDP_FLAG BIT(0)
+
/* 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-
@@ -252,6 +254,21 @@ struct padded_vnet_hdr {
char padding[4];
};
+static bool is_xdp_frame(void *ptr)
+{
+ return (unsigned long)ptr & VIRTIO_XDP_FLAG;
+}
+
+static void *xdp_to_ptr(struct xdp_frame *ptr)
+{
+ return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
+}
+
+static struct xdp_frame *ptr_to_xdp(void *ptr)
+{
+ return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
+}
+
/* Converting between virtqueue no. and kernel tx/rx queue no.
* 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
*/
@@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
sg_init_one(sq->sg, xdpf->data, xdpf->len);
- err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
+ err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
+ GFP_ATOMIC);
if (unlikely(err))
return -ENOSPC; /* Caller handle free/refcnt */
@@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
{
struct virtnet_info *vi = netdev_priv(dev);
struct receive_queue *rq = vi->rq;
- struct xdp_frame *xdpf_sent;
struct bpf_prog *xdp_prog;
struct send_queue *sq;
unsigned int len;
int drops = 0;
int kicks = 0;
int ret, err;
+ void *ptr;
int i;
/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
@@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
}
/* Free up any pending old buffers before queueing new ones. */
- while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
- xdp_return_frame(xdpf_sent);
+ while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+ if (likely(is_xdp_frame(ptr)))
+ xdp_return_frame(ptr_to_xdp(ptr));
+ else
+ napi_consume_skb(ptr, false);
+ }
for (i = 0; i < n; i++) {
struct xdp_frame *xdpf = frames[i];
@@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
{
- struct sk_buff *skb;
unsigned int len;
unsigned int packets = 0;
unsigned int bytes = 0;
+ void *ptr;
- while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
- pr_debug("Sent skb %p\n", skb);
+ while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+ if (likely(!is_xdp_frame(ptr))) {
+ struct sk_buff *skb = ptr;
- bytes += skb->len;
- packets++;
+ pr_debug("Sent skb %p\n", skb);
- napi_consume_skb(skb, in_napi);
+ bytes += skb->len;
+ napi_consume_skb(skb, in_napi);
+ } else {
+ struct xdp_frame *frame = ptr_to_xdp(ptr);
+
+ bytes += frame->len;
+ xdp_return_frame(frame);
+ }
+ packets++;
}
/* Avoid overhead when no packets have been processed
@@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
for (i = 0; i < vi->max_queue_pairs; i++) {
struct virtqueue *vq = vi->sq[i].vq;
while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
- if (!is_xdp_raw_buffer_queue(vi, i))
+ if (!is_xdp_frame(buf))
dev_kfree_skb(buf);
else
- xdp_return_frame(buf);
+ xdp_return_frame(ptr_to_xdp(buf));
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled
From: Jason Wang @ 2019-01-29 2:20 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller, Michael S. Tsirkin
Cc: netdev, Jesper Dangaard Brouer, virtualization
In-Reply-To: <1548722759-2470-6-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
> ready for XDP") tried to avoid access to unexpected sq while XDP is
> disabled, but was not complete.
>
> There was a small window which causes out of bounds sq access in
> virtnet_xdp_xmit() while disabling XDP.
>
> An example case of
> - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
> - online_cpu_num = xdp_queue_paris = 4
> when XDP is enabled:
>
> CPU 0 CPU 1
> (Disabling XDP) (Processing redirected XDP frames)
>
> virtnet_xdp_xmit()
> virtnet_xdp_set()
> _virtnet_set_queues()
> set curr_queue_pairs (2)
> check if rq->xdp_prog is not NULL
> virtnet_xdp_sq(vi)
> qp = curr_queue_pairs -
> xdp_queue_pairs +
> smp_processor_id()
> = 2 - 4 + 1 = -1
> sq = &vi->sq[qp] // out of bounds access
> set xdp_queue_pairs (0)
> rq->xdp_prog = NULL
>
> Basically we should not change curr_queue_pairs and xdp_queue_pairs
> while someone can read the values. Thus, when disabling XDP, assign NULL
> to rq->xdp_prog first, and wait for RCU grace period, then change
> xxx_queue_pairs.
> Note that we need to keep the current order when enabling XDP though.
>
> - v2: Make rcu_assign_pointer/synchronize_net conditional instead of
> _virtnet_set_queues.
>
> Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing
From: Jason Wang @ 2019-01-29 2:23 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller, Michael S. Tsirkin
Cc: netdev, virtualization
In-Reply-To: <1548722759-2470-8-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> We do not reset or free up unused buffers when enabling/disabling XDP,
> so it can happen that xdp_frames are freed after disabling XDP or
> sk_buffs are freed after enabling XDP on xdp tx queues.
> Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
> of XDP setting.
> One way to trigger this problem is to disable XDP when napi_tx is
> enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
> which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
> which invokes free_old_xmit_skbs() for queues which have been used by
> XDP.
>
> Note that even with this change we need to keep skipping
> free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
> tx queues do not aquire queue locks.
>
> - v2: Use napi_consume_skb() instead of dev_consume_skb_any()
>
> Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> NOTE: Dropped Acked-by because of the v2 change.
>
> drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1d454ce..2594481 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -57,6 +57,8 @@
> #define VIRTIO_XDP_TX BIT(0)
> #define VIRTIO_XDP_REDIR BIT(1)
>
> +#define VIRTIO_XDP_FLAG BIT(0)
> +
> /* 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-
> @@ -252,6 +254,21 @@ struct padded_vnet_hdr {
> char padding[4];
> };
>
> +static bool is_xdp_frame(void *ptr)
> +{
> + return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> +}
> +
> +static void *xdp_to_ptr(struct xdp_frame *ptr)
> +{
> + return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
> +}
> +
> +static struct xdp_frame *ptr_to_xdp(void *ptr)
> +{
> + return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
> +}
> +
> /* Converting between virtqueue no. and kernel tx/rx queue no.
> * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
> */
> @@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
>
> sg_init_one(sq->sg, xdpf->data, xdpf->len);
>
> - err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
> + err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
> + GFP_ATOMIC);
> if (unlikely(err))
> return -ENOSPC; /* Caller handle free/refcnt */
>
> @@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> {
> struct virtnet_info *vi = netdev_priv(dev);
> struct receive_queue *rq = vi->rq;
> - struct xdp_frame *xdpf_sent;
> struct bpf_prog *xdp_prog;
> struct send_queue *sq;
> unsigned int len;
> int drops = 0;
> int kicks = 0;
> int ret, err;
> + void *ptr;
> int i;
>
> /* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
> @@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> }
>
> /* Free up any pending old buffers before queueing new ones. */
> - while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> - xdp_return_frame(xdpf_sent);
> + while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> + if (likely(is_xdp_frame(ptr)))
> + xdp_return_frame(ptr_to_xdp(ptr));
> + else
> + napi_consume_skb(ptr, false);
> + }
>
> for (i = 0; i < n; i++) {
> struct xdp_frame *xdpf = frames[i];
> @@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>
> static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
> {
> - struct sk_buff *skb;
> unsigned int len;
> unsigned int packets = 0;
> unsigned int bytes = 0;
> + void *ptr;
>
> - while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> - pr_debug("Sent skb %p\n", skb);
> + while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> + if (likely(!is_xdp_frame(ptr))) {
> + struct sk_buff *skb = ptr;
>
> - bytes += skb->len;
> - packets++;
> + pr_debug("Sent skb %p\n", skb);
>
> - napi_consume_skb(skb, in_napi);
> + bytes += skb->len;
> + napi_consume_skb(skb, in_napi);
> + } else {
> + struct xdp_frame *frame = ptr_to_xdp(ptr);
> +
> + bytes += frame->len;
> + xdp_return_frame(frame);
> + }
> + packets++;
> }
>
> /* Avoid overhead when no packets have been processed
> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
> for (i = 0; i < vi->max_queue_pairs; i++) {
> struct virtqueue *vq = vi->sq[i].vq;
> while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> - if (!is_xdp_raw_buffer_queue(vi, i))
> + if (!is_xdp_frame(buf))
I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
can sent a patch on top to remove it.
> dev_kfree_skb(buf);
> else
> - xdp_return_frame(buf);
> + xdp_return_frame(ptr_to_xdp(buf));
> }
> }
>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next V4 0/5] vhost: accelerate metadata access through vmap()
From: Jason Wang @ 2019-01-29 2:34 UTC (permalink / raw)
To: Michael S. Tsirkin, David Miller
Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190126193126-mutt-send-email-mst@kernel.org>
On 2019/1/27 上午8:31, Michael S. Tsirkin wrote:
> On Sat, Jan 26, 2019 at 02:37:08PM -0800, David Miller wrote:
>> From: Jason Wang <jasowang@redhat.com>
>> Date: Wed, 23 Jan 2019 17:55:52 +0800
>>
>>> This series tries to access virtqueue metadata through kernel virtual
>>> address instead of copy_user() friends since they had too much
>>> overheads like checks, spec barriers or even hardware feature
>>> toggling.
>>>
>>> Test shows about 24% improvement on TX PPS. It should benefit other
>>> cases as well.
>> I've read over the discussion of patch #5 a few times.
>>
>> And it seems to me that, at a minimum, a few things still need to
>> be resolved:
>>
>> 1) More perf data added to commit message.
Ok.
>>
>> 2) Whether invalidate_range_start() and invalidate_range_end() must
>> be paired.
The reason that vhost doesn't need an invalidate_range_end() is because
we have a fallback to copy_to_user() friends. So there's no requirement
to setup the mapping in range_end() or lock the vq between range_start()
and range_end(). We try to delay the setup of vmap until it will be
really used in vhost_meta_prefetch() and we hold mmap_sem when trying to
setup vmap, this will guarantee there's no intermediate state at this time.
>
> Add dirty tracking.
I think this could be solved by introducing e.g
vhost_meta_prefetch_done() at the end of handle_tx()/handle_rx() and
call set_page_dirty() for used pages instead of the tricks of
classifying VMA. (As I saw hugetlbfs has its own set dirty method).
Thanks
>
>> Etc. So I am marking this series "Changes Requested".
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing
From: Toshiaki Makita @ 2019-01-29 2:35 UTC (permalink / raw)
To: Jason Wang, David S. Miller, Michael S. Tsirkin; +Cc: netdev, virtualization
In-Reply-To: <6267c42f-ce1e-1b63-2af9-84a76090f686@redhat.com>
On 2019/01/29 11:23, Jason Wang wrote:
> On 2019/1/29 上午8:45, Toshiaki Makita wrote:
...
>> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct
>> virtnet_info *vi)
>> for (i = 0; i < vi->max_queue_pairs; i++) {
>> struct virtqueue *vq = vi->sq[i].vq;
>> while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>> - if (!is_xdp_raw_buffer_queue(vi, i))
>> + if (!is_xdp_frame(buf))
>
>
> I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
> can sent a patch on top to remove it.
Actually patch2 added new users of it ;)
>
>
>> dev_kfree_skb(buf);
>> else
>> - xdp_return_frame(buf);
>> + xdp_return_frame(ptr_to_xdp(buf));
>> }
>> }
>>
>
>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
Thanks!
--
Toshiaki Makita
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing
From: Jason Wang @ 2019-01-29 2:49 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller, Michael S. Tsirkin
Cc: netdev, virtualization
In-Reply-To: <5f114c25-1f42-996b-134c-962b64597e74@lab.ntt.co.jp>
On 2019/1/29 上午10:35, Toshiaki Makita wrote:
> On 2019/01/29 11:23, Jason Wang wrote:
>> On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> ...
>>> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct
>>> virtnet_info *vi)
>>> for (i = 0; i < vi->max_queue_pairs; i++) {
>>> struct virtqueue *vq = vi->sq[i].vq;
>>> while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>>> - if (!is_xdp_raw_buffer_queue(vi, i))
>>> + if (!is_xdp_frame(buf))
>>
>> I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
>> can sent a patch on top to remove it.
> Actually patch2 added new users of it ;)
Right, I missed that.
Thanks
>
>>
>>> dev_kfree_skb(buf);
>>> else
>>> - xdp_return_frame(buf);
>>> + xdp_return_frame(ptr_to_xdp(buf));
>>> }
>>> }
>>>
>>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>>
> Thanks!
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox