Bug 1544255 - [RFE] Add virtio packed ring support in libvirt
Summary: [RFE] Add virtio packed ring support in libvirt
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Red Hat Enterprise Linux Advanced Virtualization
Classification: Red Hat
Component: libvirt
Version: ---
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: rc
: ---
Assignee: Ján Tomko
QA Contact: Han Han
URL:
Whiteboard:
: 1601357 (view as bug list)
Depends On: 1544252 1544253 1544254 1544758 1601349 1601351 1601354 1601355
Blocks: 1563552 1564988
TreeView+ depends on / blocked
 
Reported: 2018-02-11 18:06 UTC by Amnon Ilan
Modified: 2019-12-12 16:38 UTC (History)
20 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of: 1544254
: 1563552 1601357 (view as bug list)
Environment:
Last Closed: 2019-12-12 16:38:07 UTC
Type: Feature Request
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Amnon Ilan 2018-02-11 18:06:30 UTC
Description of problem:

Virtio-1.1 defines a new ring layout for Virtio, should add support for it 
in Libvirt

Comment 3 jason wang 2018-03-26 02:09:50 UTC
For libvirt, we probably just need a new property for virtio-net like:

-device virtio-net-pci,ring_packed=true

Will update here, when the qemu patch was in.

Thanks

Comment 7 Ján Tomko 2019-03-28 09:03:00 UTC
*** Bug 1601357 has been marked as a duplicate of this bug. ***

Comment 10 Daniel Berrangé 2019-04-17 13:17:10 UTC
(In reply to jason wang from comment #3)
> For libvirt, we probably just need a new property for virtio-net like:
> 
> -device virtio-net-pci,ring_packed=true
> 
> Will update here, when the qemu patch was in.

Ideally libvirt would not need to expose this flag at all. IIUC virtio 1.1 should be fully back compatible with virtio 1.0, so QEMU should negotiate with guest whether to use this new ring layout.

Thus the main thing to take care with is the migration ABI compat, which can be solved if QEMU sets ring_packed=false for existing machine types, and sets ring_packed=true for new machine types.

This way all mgmt apps would automatically get the new improved ring layout without any changes being required up the stack.

Comment 11 Jaroslav Suchanek 2019-04-17 13:39:58 UTC
(In reply to Daniel Berrange from comment #10)
> (In reply to jason wang from comment #3)
> > For libvirt, we probably just need a new property for virtio-net like:
> > 
> > -device virtio-net-pci,ring_packed=true
> > 
> > Will update here, when the qemu patch was in.
> 
> Ideally libvirt would not need to expose this flag at all. IIUC virtio 1.1
> should be fully back compatible with virtio 1.0, so QEMU should negotiate
> with guest whether to use this new ring layout.
> 
> Thus the main thing to take care with is the migration ABI compat, which can
> be solved if QEMU sets ring_packed=false for existing machine types, and
> sets ring_packed=true for new machine types.
> 
> This way all mgmt apps would automatically get the new improved ring layout
> without any changes being required up the stack.

Jason, what do you think?

Comment 12 jason wang 2019-04-18 02:35:54 UTC
(In reply to Daniel Berrange from comment #10)
> (In reply to jason wang from comment #3)
> > For libvirt, we probably just need a new property for virtio-net like:
> > 
> > -device virtio-net-pci,ring_packed=true
> > 
> > Will update here, when the qemu patch was in.
> 
> Ideally libvirt would not need to expose this flag at all. IIUC virtio 1.1
> should be fully back compatible with virtio 1.0, so QEMU should negotiate
> with guest whether to use this new ring layout.
> 
> Thus the main thing to take care with is the migration ABI compat, which can
> be solved if QEMU sets ring_packed=false for existing machine types, and
> sets ring_packed=true for new machine types.
> 
> This way all mgmt apps would automatically get the new improved ring layout
> without any changes being required up the stack.

If you worry about the default value, we can change it to be enabled by default (upstream). But for RHEL, maybe we need to start 1.1 as technical preview so disable it by default. It's pretty new and it lacks sufficient testing and benchmarking.


Thanks

Comment 13 Ján Tomko 2019-05-29 13:29:24 UTC
So if there will be no need to turn this feature off and the only reason not to enable it by default is insufficient testing,
can the testing be done by libvirt's command passthrough hack?
https://libvirt.org/drvqemu.html#qemucommand

<domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
  ...
  <qemu:commandline>
    <qemu:arg value='-set'/>
    <qemu:arg value='device.net0.ring_packed=true'/>
  </qemu:commandline>
</domain>

I really don't like introducing an XML attribute that will be unused in a few months.

Comment 16 Jaroslav Suchanek 2019-12-12 16:38:07 UTC
Per discussion libvirt support is not needed thus closing it for now. Thanks.


Note You need to log in before you can comment on or make changes to this bug.