| Message ID | 20170804170057.23047-6-aconole@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Russell Bryant |
| Headers | show |
On Fri, Aug 4, 2017 at 1:00 PM, Aaron Conole <aconole@redhat.com> wrote: > After this commit, users may start a dpdk-enabled ovs setup as a > non-root user. This is accomplished by exporting the $HOME directory, > which dpdk uses to fill in it's semi-persistent RTE configuration. > > This change may be a bit controversial since it modifies /dev/hugepages > as part of starting the ovs-vswitchd to set a hugetlbfs group > ownership. This is used to enable writing to /dev/hugepages so that the > dpdk_init will successfully complete. There is an alternate way of > accomplishing this - namely to initialize DPDK before dropping > privileges. However, this would mean that if DPDK ever grows an uninit > / reinit function, non-root ovs likely could never use it. Indeed ... the modifications to /dev/hugepages don't look ideal ... If this was truly limited to when DPDK was in use, I'd feel better about it. We want to build a single package for OVS, right? The package will have DPDK enabled, even for normal uses that won't use DPDK. That means these modifications take place even for non-DPDK use. I'd feel more comfortable if it could be restricted to only when DPDK was actually in use. Maybe some of this logic could be moved into ovs-ctl so that the check could be at runtime? > > This does not change OvS+DPDK's SELinux requirements. It still must be > disabled. > > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > Documentation/intro/install/dpdk.rst | 7 +++++++ > NEWS | 1 + > rhel/README.RHEL.rst | 11 +++++++++++ > rhel/openvswitch-fedora.spec.in | 13 +++++++++++++ > rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 5 +++++ > 5 files changed, 37 insertions(+)
Russell Bryant <russell@ovn.org> writes: > On Fri, Aug 4, 2017 at 1:00 PM, Aaron Conole <aconole@redhat.com> wrote: >> After this commit, users may start a dpdk-enabled ovs setup as a >> non-root user. This is accomplished by exporting the $HOME directory, >> which dpdk uses to fill in it's semi-persistent RTE configuration. >> >> This change may be a bit controversial since it modifies /dev/hugepages >> as part of starting the ovs-vswitchd to set a hugetlbfs group >> ownership. This is used to enable writing to /dev/hugepages so that the >> dpdk_init will successfully complete. There is an alternate way of >> accomplishing this - namely to initialize DPDK before dropping >> privileges. However, this would mean that if DPDK ever grows an uninit >> / reinit function, non-root ovs likely could never use it. > > Indeed ... the modifications to /dev/hugepages don't look ideal ... > > If this was truly limited to when DPDK was in use, I'd feel better > about it. We want to build a single package for OVS, right? The > package will have DPDK enabled, even for normal uses that won't use > DPDK. That means these modifications take place even for non-DPDK > use. I'd feel more comfortable if it could be restricted to only when > DPDK was actually in use. Maybe some of this logic could be moved > into ovs-ctl so that the check could be at runtime? I couldn't find a way of doing that check. It is possible to dynamically enable dpdk (since commit ec2b070143c2 "dpdk: Late initialization"), which means we would need something constantly polling for the status change -OR- we would need to have a way of changing gid in response to the database change. The second might be possible but would require some changes in ovs-vswitchd. >> >> This does not change OvS+DPDK's SELinux requirements. It still must be >> disabled. >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> Documentation/intro/install/dpdk.rst | 7 +++++++ >> NEWS | 1 + >> rhel/README.RHEL.rst | 11 +++++++++++ >> rhel/openvswitch-fedora.spec.in | 13 +++++++++++++ >> rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 5 +++++ >> 5 files changed, 37 insertions(+)
On Sun, Aug 6, 2017 at 7:24 AM, Aaron Conole <aconole@redhat.com> wrote: > Russell Bryant <russell@ovn.org> writes: > >> On Fri, Aug 4, 2017 at 1:00 PM, Aaron Conole <aconole@redhat.com> wrote: >>> After this commit, users may start a dpdk-enabled ovs setup as a >>> non-root user. This is accomplished by exporting the $HOME directory, >>> which dpdk uses to fill in it's semi-persistent RTE configuration. >>> >>> This change may be a bit controversial since it modifies /dev/hugepages >>> as part of starting the ovs-vswitchd to set a hugetlbfs group >>> ownership. This is used to enable writing to /dev/hugepages so that the >>> dpdk_init will successfully complete. There is an alternate way of >>> accomplishing this - namely to initialize DPDK before dropping >>> privileges. However, this would mean that if DPDK ever grows an uninit >>> / reinit function, non-root ovs likely could never use it. >> >> Indeed ... the modifications to /dev/hugepages don't look ideal ... >> >> If this was truly limited to when DPDK was in use, I'd feel better >> about it. We want to build a single package for OVS, right? The >> package will have DPDK enabled, even for normal uses that won't use >> DPDK. That means these modifications take place even for non-DPDK >> use. I'd feel more comfortable if it could be restricted to only when >> DPDK was actually in use. Maybe some of this logic could be moved >> into ovs-ctl so that the check could be at runtime? > > I couldn't find a way of doing that check. It is possible to > dynamically enable dpdk (since commit ec2b070143c2 "dpdk: Late > initialization"), which means we would need something constantly polling > for the status change -OR- we would need to have a way of changing gid > in response to the database change. The second might be possible but > would require some changes in ovs-vswitchd. OK, then I don't have any alternatives to propose at this point. I've applied this series to master and branch-2.8.
Russell Bryant <russell@ovn.org> writes: > On Sun, Aug 6, 2017 at 7:24 AM, Aaron Conole <aconole@redhat.com> wrote: >> Russell Bryant <russell@ovn.org> writes: >> >>> On Fri, Aug 4, 2017 at 1:00 PM, Aaron Conole <aconole@redhat.com> wrote: >>>> After this commit, users may start a dpdk-enabled ovs setup as a >>>> non-root user. This is accomplished by exporting the $HOME directory, >>>> which dpdk uses to fill in it's semi-persistent RTE configuration. >>>> >>>> This change may be a bit controversial since it modifies /dev/hugepages >>>> as part of starting the ovs-vswitchd to set a hugetlbfs group >>>> ownership. This is used to enable writing to /dev/hugepages so that the >>>> dpdk_init will successfully complete. There is an alternate way of >>>> accomplishing this - namely to initialize DPDK before dropping >>>> privileges. However, this would mean that if DPDK ever grows an uninit >>>> / reinit function, non-root ovs likely could never use it. >>> >>> Indeed ... the modifications to /dev/hugepages don't look ideal ... >>> >>> If this was truly limited to when DPDK was in use, I'd feel better >>> about it. We want to build a single package for OVS, right? The >>> package will have DPDK enabled, even for normal uses that won't use >>> DPDK. That means these modifications take place even for non-DPDK >>> use. I'd feel more comfortable if it could be restricted to only when >>> DPDK was actually in use. Maybe some of this logic could be moved >>> into ovs-ctl so that the check could be at runtime? >> >> I couldn't find a way of doing that check. It is possible to >> dynamically enable dpdk (since commit ec2b070143c2 "dpdk: Late >> initialization"), which means we would need something constantly polling >> for the status change -OR- we would need to have a way of changing gid >> in response to the database change. The second might be possible but >> would require some changes in ovs-vswitchd. > > OK, then I don't have any alternatives to propose at this point. > > I've applied this series to master and branch-2.8. Thanks Russell!
diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index b2b91d4..4b0828c 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -134,6 +134,13 @@ has to be configured with DPDK support (``--with-dpdk``). Additional information can be found in :doc:`general`. +.. note:: + If you are running using the Fedora or Red Hat package, the Open vSwitch + daemon will run as a non-root user. This implies that you must have a + working IOMMU. Visit the `RHEL README`__ for additional information. + +__ https://github.com/openvswitch/ovs/blob/master/rhel/README.RHEL.rst + Setup ----- diff --git a/NEWS b/NEWS index a89e718..a7bc1c3 100644 --- a/NEWS +++ b/NEWS @@ -70,6 +70,7 @@ Post-v2.7.0 First supported use case is encap/decap for Ethernet. - Fedora Packaging: * OVN services are no longer restarted automatically after upgrade. + * ovs-vswitchd and ovsdb-server run as non-root users by default. - Add --cleanup option to command 'ovs-appctl exit' (see ovs-vswitchd(8)). - L3 tunneling: * Use new tunnel port option "packet_type" to configure L2 vs. L3. diff --git a/rhel/README.RHEL.rst b/rhel/README.RHEL.rst index 6affdba..f3d2942 100644 --- a/rhel/README.RHEL.rst +++ b/rhel/README.RHEL.rst @@ -337,6 +337,17 @@ running. All other commands where executed when Open vSwitch was successfully running. +Non-root User Support +----------------------- +Fedora and RHEL support running the Open vSwitch daemons as a non-root user. +By default, a fresh installation will create an *openvswitch* user, along +with any additional support groups needed (such as *hugetlbfs* for DPDK +support). + +This is controlled by modifying the ``OVS_USER_ID`` option. Setting this +to 'root:root', or commenting the variable out will revert this behavior. + + Reporting Bugs -------------- diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in index 1061824..2eccada 100644 --- a/rhel/openvswitch-fedora.spec.in +++ b/rhel/openvswitch-fedora.spec.in @@ -95,6 +95,10 @@ Requires: openssl hostname iproute module-init-tools Requires(post): /usr/bin/getent Requires(post): /usr/sbin/useradd Requires(post): /usr/bin/sed +%if %{with dpdk} +Requires(post): /usr/sbin/usermod +Requires(post): /usr/sbin/groupadd +%endif Requires(post): systemd-units Requires(preun): systemd-units Requires(postun): systemd-units @@ -379,6 +383,15 @@ if [ $1 -eq 1 ]; then sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch +%if %{with dpdk} + getent group hugetlbfs >/dev/null || \ + groupadd hugetlbfs + usermod -a -G hugetlbfs openvswitch + sed -i \ + 's@OVS_USER_ID="openvswitch:openvswitch"@OVS_USER_ID="openvswitch:hugetlbfs"@'\ + /etc/sysconfig/openvswitch +%endif + # In the case of upgrade, this is not needed. chown -R openvswitch:openvswitch /etc/openvswitch fi diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in index 9aff70b..bf0f058 100644 --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in @@ -10,8 +10,13 @@ PartOf=openvswitch.service [Service] Type=forking Restart=on-failure +Environment=HOME=/var/run/openvswitch EnvironmentFile=/etc/openvswitch/default.conf EnvironmentFile=-/etc/sysconfig/openvswitch +@begin_dpdk@ +ExecStartPre=/usr/bin/chown :hugetlbfs /dev/hugepages +ExecStartPre=/usr/bin/chmod 0775 /dev/hugepages +@end_dpdk@ ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \ --no-ovsdb-server --no-monitor --system-id=random \ --ovs-user=${OVS_USER_ID} \
After this commit, users may start a dpdk-enabled ovs setup as a non-root user. This is accomplished by exporting the $HOME directory, which dpdk uses to fill in it's semi-persistent RTE configuration. This change may be a bit controversial since it modifies /dev/hugepages as part of starting the ovs-vswitchd to set a hugetlbfs group ownership. This is used to enable writing to /dev/hugepages so that the dpdk_init will successfully complete. There is an alternate way of accomplishing this - namely to initialize DPDK before dropping privileges. However, this would mean that if DPDK ever grows an uninit / reinit function, non-root ovs likely could never use it. This does not change OvS+DPDK's SELinux requirements. It still must be disabled. Signed-off-by: Aaron Conole <aconole@redhat.com> --- Documentation/intro/install/dpdk.rst | 7 +++++++ NEWS | 1 + rhel/README.RHEL.rst | 11 +++++++++++ rhel/openvswitch-fedora.spec.in | 13 +++++++++++++ rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 5 +++++ 5 files changed, 37 insertions(+)