[ovs-dev,2/2] dpdk docs: Drop file share in libvirt config.

Message ID 1523442352-164297-2-git-send-email-tiago.lam@intel.com
State Accepted
Delegated to: Ian Stokes
Headers show
Series
  • [ovs-dev,1/2] dpdk docs: Drop qemu-kvm for qemu-system-x86_64.
Related show

Commit Message

Lam, Tiago April 11, 2018, 10:25 a.m.
When explaining on how to add vhost-user ports to a guest, using
libvirt, the following piece of configuration is used:
    <disk type='dir' device='disk'>
      <driver name='qemu' type='fat'/>
      <source dir='/usr/src/dpdk-stable-17.11.1'/>
      <target dev='vdb' bus='virtio'/>
      <readonly/>
    </disk>

This is used to facilitate sharing of a DPDK directory between the host
and the guest. However, for this to work selinux also needs to be
configured (or disabled).  Furthermore, if one is using Ubuntu, libvirtd
would need to be added to complain only in AppArmor. Instead, in [1] it
is advised to use wget to get the DPDK sources over the internet, which
avoids this differentiation. Thus, we drop this piece of configuration
here as well and keep the example configuration as simple as possible.

This has been verified on both a Fedora 27 image and a Ubuntu 16.04 LTS
image.

[1] http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest

Signed-off-by: Tiago Lam <tiago.lam@intel.com>
---

CC'ed Stephen,

I took the liberty of removing your TODO from here, as I read it to be related
to the (now removed) SELinux instruction below. If you think it should still be
there let me know and I'll gladly send a v2.

 Documentation/topics/dpdk/vhost-user.rst | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

Comments

Aaron Conole April 11, 2018, 1:54 p.m. | #1
Tiago Lam <tiago.lam@intel.com> writes:

> When explaining on how to add vhost-user ports to a guest, using
> libvirt, the following piece of configuration is used:
>     <disk type='dir' device='disk'>
>       <driver name='qemu' type='fat'/>
>       <source dir='/usr/src/dpdk-stable-17.11.1'/>
>       <target dev='vdb' bus='virtio'/>
>       <readonly/>
>     </disk>
>
> This is used to facilitate sharing of a DPDK directory between the host
> and the guest. However, for this to work selinux also needs to be
> configured (or disabled).  Furthermore, if one is using Ubuntu, libvirtd
> would need to be added to complain only in AppArmor. Instead, in [1] it
> is advised to use wget to get the DPDK sources over the internet, which
> avoids this differentiation. Thus, we drop this piece of configuration
> here as well and keep the example configuration as simple as possible.
>
> This has been verified on both a Fedora 27 image and a Ubuntu 16.04 LTS
> image.
>
> [1] http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest
>
> Signed-off-by: Tiago Lam <tiago.lam@intel.com>
> ---
>
> CC'ed Stephen,
>
> I took the liberty of removing your TODO from here, as I read it to be related
> to the (now removed) SELinux instruction below. If you think it should still be
> there let me know and I'll gladly send a v2.

I think it should remain until the selinux issues have been addressed.

Is there a list somewhere of the AVC denials?  Maybe it makes sense to
allow them.
Stephen Finucane April 11, 2018, 2:03 p.m. | #2
On Wed, 2018-04-11 at 09:54 -0400, Aaron Conole wrote:
> Tiago Lam <tiago.lam@intel.com> writes:
> 
> > When explaining on how to add vhost-user ports to a guest, using
> > libvirt, the following piece of configuration is used:
> >     <disk type='dir' device='disk'>
> >       <driver name='qemu' type='fat'/>
> >       <source dir='/usr/src/dpdk-stable-17.11.1'/>
> >       <target dev='vdb' bus='virtio'/>
> >       <readonly/>
> >     </disk>
> > 
> > This is used to facilitate sharing of a DPDK directory between the host
> > and the guest. However, for this to work selinux also needs to be
> > configured (or disabled).  Furthermore, if one is using Ubuntu, libvirtd
> > would need to be added to complain only in AppArmor. Instead, in [1] it
> > is advised to use wget to get the DPDK sources over the internet, which
> > avoids this differentiation. Thus, we drop this piece of configuration
> > here as well and keep the example configuration as simple as possible.
> > 
> > This has been verified on both a Fedora 27 image and a Ubuntu 16.04 LTS
> > image.
> > 
> > [1] http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest
> > 
> > Signed-off-by: Tiago Lam <tiago.lam@intel.com>
> > ---
> > 
> > CC'ed Stephen,
> > 
> > I took the liberty of removing your TODO from here, as I read it to be related
> > to the (now removed) SELinux instruction below. If you think it should still be
> > there let me know and I'll gladly send a v2.
> 
> I think it should remain until the selinux issues have been addressed.
> 
> Is there a list somewhere of the AVC denials?  Maybe it makes sense to
> allow them.

If I'm reading this correctly, Tiago is saying these exceptions only
happen because we're sharing an arbitrary directory with the guest to
avoid downloading the DPDK sources twice. Given that there's a valid
workaround (just fetching sources twice), simply removing that section
of the XML removes the need to disable SELinux. If so, dropping the
warning does make sense in my mind.

Stephen
Aaron Conole April 11, 2018, 2:55 p.m. | #3
Stephen Finucane <stephen@that.guru> writes:

> On Wed, 2018-04-11 at 09:54 -0400, Aaron Conole wrote:
>> Tiago Lam <tiago.lam@intel.com> writes:
>> 
>> > When explaining on how to add vhost-user ports to a guest, using
>> > libvirt, the following piece of configuration is used:
>> >     <disk type='dir' device='disk'>
>> >       <driver name='qemu' type='fat'/>
>> >       <source dir='/usr/src/dpdk-stable-17.11.1'/>
>> >       <target dev='vdb' bus='virtio'/>
>> >       <readonly/>
>> >     </disk>
>> > 
>> > This is used to facilitate sharing of a DPDK directory between the host
>> > and the guest. However, for this to work selinux also needs to be
>> > configured (or disabled).  Furthermore, if one is using Ubuntu, libvirtd
>> > would need to be added to complain only in AppArmor. Instead, in [1] it
>> > is advised to use wget to get the DPDK sources over the internet, which
>> > avoids this differentiation. Thus, we drop this piece of configuration
>> > here as well and keep the example configuration as simple as possible.
>> > 
>> > This has been verified on both a Fedora 27 image and a Ubuntu 16.04 LTS
>> > image.
>> > 
>> > [1] http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest
>> > 
>> > Signed-off-by: Tiago Lam <tiago.lam@intel.com>
>> > ---
>> > 
>> > CC'ed Stephen,
>> > 
>> > I took the liberty of removing your TODO from here, as I read it to be related
>> > to the (now removed) SELinux instruction below. If you think it should still be
>> > there let me know and I'll gladly send a v2.
>> 
>> I think it should remain until the selinux issues have been addressed.
>> 
>> Is there a list somewhere of the AVC denials?  Maybe it makes sense to
>> allow them.
>
> If I'm reading this correctly, Tiago is saying these exceptions only
> happen because we're sharing an arbitrary directory with the guest to
> avoid downloading the DPDK sources twice.

Okay, I guess I read the change in the section a bit differently.  If
you think it's okay, then I'm fine (I'm always happy to see a
'setenforce 0' disappear).

> Given that there's a valid
> workaround (just fetching sources twice), simply removing that section
> of the XML removes the need to disable SELinux. If so, dropping the
> warning does make sense in my mind.
>
> Stephen
Lam, Tiago April 12, 2018, 7:24 a.m. | #4
On 11/04/2018 15:03, Stephen Finucane wrote:
> On Wed, 2018-04-11 at 09:54 -0400, Aaron Conole wrote:
>> Tiago Lam <tiago.lam@intel.com> writes:
>>
>>> When explaining on how to add vhost-user ports to a guest, using
>>> libvirt, the following piece of configuration is used:
>>>      <disk type='dir' device='disk'>
>>>        <driver name='qemu' type='fat'/>
>>>        <source dir='/usr/src/dpdk-stable-17.11.1'/>
>>>        <target dev='vdb' bus='virtio'/>
>>>        <readonly/>
>>>      </disk>
>>>
>>> This is used to facilitate sharing of a DPDK directory between the host
>>> and the guest. However, for this to work selinux also needs to be
>>> configured (or disabled).  Furthermore, if one is using Ubuntu, libvirtd
>>> would need to be added to complain only in AppArmor. Instead, in [1] it
>>> is advised to use wget to get the DPDK sources over the internet, which
>>> avoids this differentiation. Thus, we drop this piece of configuration
>>> here as well and keep the example configuration as simple as possible.
>>>
>>> This has been verified on both a Fedora 27 image and a Ubuntu 16.04 LTS
>>> image.
>>>
>>> [1] http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest
>>>
>>> Signed-off-by: Tiago Lam <tiago.lam@intel.com>
>>> ---
>>>
>>> CC'ed Stephen,
>>>
>>> I took the liberty of removing your TODO from here, as I read it to be related
>>> to the (now removed) SELinux instruction below. If you think it should still be
>>> there let me know and I'll gladly send a v2.
>>
>> I think it should remain until the selinux issues have been addressed.
>>
>> Is there a list somewhere of the AVC denials?  Maybe it makes sense to
>> allow them.
> 
> If I'm reading this correctly, Tiago is saying these exceptions only
> happen because we're sharing an arbitrary directory with the guest to
> avoid downloading the DPDK sources twice. Given that there's a valid
> workaround (just fetching sources twice), simply removing that section
> of the XML removes the need to disable SELinux. If so, dropping the
> warning does make sense in my mind.
> 
> Stephen
> 

Thanks, Stephen. Yeah, that's what I was aiming at. In order to get the 
file sharing working properly, one must fiddle around with either 
SELinux or AppArmor, and that seems to be the sole reason why 
`setenforce 0` is there. Losing the dependency on the file sharing means 
we can lose any instructions that tell the user how to fiddle with 
either of those systems.

Just a note though, in that the user won't have to download the DPDK 
sources twice, only once. Following the guide, the user first sets up 
the vhost-user ports using libvirt, and once inside the VM he should 
follow up on running `testpmd` inside the guest [1], where he will be 
instructed to download the DPDK sources. This makes this piece of the 
docs a bit more consistent, I think.

[1] 
http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest
Stephen Finucane April 13, 2018, 8:28 a.m. | #5
On Thu, 2018-04-12 at 08:24 +0100, Lam, Tiago wrote:
> On 11/04/2018 15:03, Stephen Finucane wrote:
> > On Wed, 2018-04-11 at 09:54 -0400, Aaron Conole wrote:
> > > Tiago Lam <tiago.lam@intel.com> writes:
> > > 
> > > > When explaining on how to add vhost-user ports to a guest, using
> > > > libvirt, the following piece of configuration is used:
> > > >      <disk type='dir' device='disk'>
> > > >        <driver name='qemu' type='fat'/>
> > > >        <source dir='/usr/src/dpdk-stable-17.11.1'/>
> > > >        <target dev='vdb' bus='virtio'/>
> > > >        <readonly/>
> > > >      </disk>
> > > > 
> > > > This is used to facilitate sharing of a DPDK directory between the host
> > > > and the guest. However, for this to work selinux also needs to be
> > > > configured (or disabled).  Furthermore, if one is using Ubuntu, libvirtd
> > > > would need to be added to complain only in AppArmor. Instead, in [1] it
> > > > is advised to use wget to get the DPDK sources over the internet, which
> > > > avoids this differentiation. Thus, we drop this piece of configuration
> > > > here as well and keep the example configuration as simple as possible.
> > > > 
> > > > This has been verified on both a Fedora 27 image and a Ubuntu 16.04 LTS
> > > > image.
> > > > 
> > > > [1] http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest
> > > > 
> > > > Signed-off-by: Tiago Lam <tiago.lam@intel.com>
> > > > ---
> > > > 
> > > > CC'ed Stephen,
> > > > 
> > > > I took the liberty of removing your TODO from here, as I read it to be related
> > > > to the (now removed) SELinux instruction below. If you think it should still be
> > > > there let me know and I'll gladly send a v2.
> > > 
> > > I think it should remain until the selinux issues have been addressed.
> > > 
> > > Is there a list somewhere of the AVC denials?  Maybe it makes sense to
> > > allow them.
> > 
> > If I'm reading this correctly, Tiago is saying these exceptions only
> > happen because we're sharing an arbitrary directory with the guest to
> > avoid downloading the DPDK sources twice. Given that there's a valid
> > workaround (just fetching sources twice), simply removing that section
> > of the XML removes the need to disable SELinux. If so, dropping the
> > warning does make sense in my mind.
> > 
> > Stephen
> > 
> 
> Thanks, Stephen. Yeah, that's what I was aiming at. In order to get the 
> file sharing working properly, one must fiddle around with either 
> SELinux or AppArmor, and that seems to be the sole reason why 
> `setenforce 0` is there. Losing the dependency on the file sharing means 
> we can lose any instructions that tell the user how to fiddle with 
> either of those systems.
> 
> Just a note though, in that the user won't have to download the DPDK 
> sources twice, only once. Following the guide, the user first sets up 
> the vhost-user ports using libvirt, and once inside the VM he should 
> follow up on running `testpmd` inside the guest [1], where he will be 
> instructed to download the DPDK sources. This makes this piece of the 
> docs a bit more consistent, I think.
> 
> [1] 
> http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-the-guest

That all sounds fair to me.

Acked-by: Stephen Finucane <stephen@that.guru>
Stokes, Ian April 19, 2018, 8:40 a.m. | #6
> On Thu, 2018-04-12 at 08:24 +0100, Lam, Tiago wrote:
> > On 11/04/2018 15:03, Stephen Finucane wrote:
> > > On Wed, 2018-04-11 at 09:54 -0400, Aaron Conole wrote:
> > > > Tiago Lam <tiago.lam@intel.com> writes:
> > > >
> > > > > When explaining on how to add vhost-user ports to a guest, using
> > > > > libvirt, the following piece of configuration is used:
> > > > >      <disk type='dir' device='disk'>
> > > > >        <driver name='qemu' type='fat'/>
> > > > >        <source dir='/usr/src/dpdk-stable-17.11.1'/>
> > > > >        <target dev='vdb' bus='virtio'/>
> > > > >        <readonly/>
> > > > >      </disk>
> > > > >
> > > > > This is used to facilitate sharing of a DPDK directory between
> > > > > the host and the guest. However, for this to work selinux also
> > > > > needs to be configured (or disabled).  Furthermore, if one is
> > > > > using Ubuntu, libvirtd would need to be added to complain only
> > > > > in AppArmor. Instead, in [1] it is advised to use wget to get
> > > > > the DPDK sources over the internet, which avoids this
> > > > > differentiation. Thus, we drop this piece of configuration here as
> well and keep the example configuration as simple as possible.
> > > > >
> > > > > This has been verified on both a Fedora 27 image and a Ubuntu
> > > > > 16.04 LTS image.
> > > > >
> > > > > [1]
> > > > > http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dp
> > > > > dk-in-the-guest
> > > > >
> > > > > Signed-off-by: Tiago Lam <tiago.lam@intel.com>
> > > > > ---
> > > > >
> > > > > CC'ed Stephen,
> > > > >
> > > > > I took the liberty of removing your TODO from here, as I read it
> > > > > to be related to the (now removed) SELinux instruction below. If
> > > > > you think it should still be there let me know and I'll gladly
> send a v2.
> > > >
> > > > I think it should remain until the selinux issues have been
> addressed.
> > > >
> > > > Is there a list somewhere of the AVC denials?  Maybe it makes
> > > > sense to allow them.
> > >
> > > If I'm reading this correctly, Tiago is saying these exceptions only
> > > happen because we're sharing an arbitrary directory with the guest
> > > to avoid downloading the DPDK sources twice. Given that there's a
> > > valid workaround (just fetching sources twice), simply removing that
> > > section of the XML removes the need to disable SELinux. If so,
> > > dropping the warning does make sense in my mind.
> > >
> > > Stephen
> > >
> >
> > Thanks, Stephen. Yeah, that's what I was aiming at. In order to get
> > the file sharing working properly, one must fiddle around with either
> > SELinux or AppArmor, and that seems to be the sole reason why
> > `setenforce 0` is there. Losing the dependency on the file sharing
> > means we can lose any instructions that tell the user how to fiddle
> > with either of those systems.
> >
> > Just a note though, in that the user won't have to download the DPDK
> > sources twice, only once. Following the guide, the user first sets up
> > the vhost-user ports using libvirt, and once inside the VM he should
> > follow up on running `testpmd` inside the guest [1], where he will be
> > instructed to download the DPDK sources. This makes this piece of the
> > docs a bit more consistent, I think.
> >
> > [1]
> > http://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/#dpdk-in-
> > the-guest
> 
> That all sounds fair to me.
> 
> Acked-by: Stephen Finucane <stephen@that.guru>

Thanks all, pushed to DPDK_MERGE, I'll back port this to previous releases also.

Ian

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch

diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
index 74bab78..6bf16f7 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -187,21 +187,14 @@  where:
 Adding vhost-user ports to the guest (libvirt)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. TODO(stephenfin): This seems like something that wouldn't be acceptable in
-   production. Is this really required?
-
-To begin, you must change the user and group that libvirt runs under, configure
-access control policy and restart libvirtd.
+To begin, you must change the user and group that qemu runs under, and restart
+libvirtd.
 
 - In ``/etc/libvirt/qemu.conf`` add/edit the following lines::
 
       user = "root"
       group = "root"
 
-- Disable SELinux or set to permissive mode::
-
-      $ setenforce 0
-
 - Finally, restart the libvirtd process, For example, on Fedora::
 
       $ systemctl restart libvirtd.service
@@ -407,12 +400,6 @@  Sample XML
           <source file='/root/CentOS7_x86_64.qcow2'/>
           <target dev='vda' bus='virtio'/>
         </disk>
-        <disk type='dir' device='disk'>
-          <driver name='qemu' type='fat'/>
-          <source dir='/usr/src/dpdk-stable-17.11.1'/>
-          <target dev='vdb' bus='virtio'/>
-          <readonly/>
-        </disk>
         <interface type='vhostuser'>
           <mac address='00:00:00:00:00:01'/>
           <source type='unix' path='/usr/local/var/run/openvswitch/dpdkvhostuser0' mode='client'/>