"Timothy M. Redaelli" <[email protected]> writes: > Hi Aaron,
Hi Timothy, Thanks for the review! > On 07/05/2017 07:56 PM, Aaron Conole 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. >> >> This does not change OvS+DPDK's SELinux requirements. It still must be >> disabled. >> >> Signed-off-by: Aaron Conole <[email protected]> >> --- >> rhel/.gitignore | 1 + >> rhel/automake.mk | 3 ++- >> rhel/openvswitch-fedora.spec.in | 13 >> +++++++++++++ >> ...rvice => usr_lib_systemd_system_ovs-vswitchd.service.in} | 4 ++++ >> 4 files changed, 20 insertions(+), 1 deletion(-) >> rename rhel/{usr_lib_systemd_system_ovs-vswitchd.service => >> usr_lib_systemd_system_ovs-vswitchd.service.in} (87%) >> >> diff --git a/rhel/.gitignore b/rhel/.gitignore >> index 164bb66..e584a1e 100644 >> --- a/rhel/.gitignore >> +++ b/rhel/.gitignore >> @@ -4,3 +4,4 @@ openvswitch-kmod-rhel6.spec >> openvswitch-kmod-fedora.spec >> openvswitch.spec >> openvswitch-fedora.spec >> +usr_lib_systemd_system_ovs-vswitchd.service >> diff --git a/rhel/automake.mk b/rhel/automake.mk >> index 2d9443f..439af26 100644 >> --- a/rhel/automake.mk >> +++ b/rhel/automake.mk >> @@ -29,6 +29,7 @@ EXTRA_DIST += \ >> rhel/usr_lib_systemd_system_openvswitch.service \ >> rhel/usr_lib_systemd_system_ovsdb-server.service \ >> rhel/usr_lib_systemd_system_ovs-vswitchd.service \ >> + rhel/usr_lib_systemd_system_ovs-vswitchd.service.in \ >> rhel/usr_lib_systemd_system_ovn-controller.service \ >> rhel/usr_lib_systemd_system_ovn-controller-vtep.service \ >> rhel/usr_lib_systemd_system_ovn-northd.service \ >> @@ -59,7 +60,7 @@ RPMBUILD_TOP := $(abs_top_builddir)/rpm/rpmbuild >> RPMBUILD_OPT ?= --without check >> >> # Build user-space RPMs >> -rpm-fedora: dist $(srcdir)/rhel/openvswitch-fedora.spec >> +rpm-fedora: dist $(srcdir)/rhel/openvswitch-fedora.spec >> $(srcdir)/rhel/usr_lib_systemd_system_ovs-vswitchd.service >> ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES >> cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES >> rpmbuild ${RPMBUILD_OPT} \ >> diff --git a/rhel/openvswitch-fedora.spec.in >> b/rhel/openvswitch-fedora.spec.in >> index 7c805b2..83cfb18 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 >> @@ -366,6 +370,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":'\ > > There is a typo on this sed, you need to terminate it by "@" and not ":" > or you will have: > "sed: -e expression #1, char 76: unterminated `s' command" error D'oh! Missed it during my cleanup. Thanks for this, I'll respin. >> + /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 >> b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in >> similarity index 87% >> rename from rhel/usr_lib_systemd_system_ovs-vswitchd.service >> rename to rhel/usr_lib_systemd_system_ovs-vswitchd.service.in >> index 48231b3..2d2e9e6 100644 >> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service >> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in >> @@ -10,8 +10,12 @@ PartOf=openvswitch.service >> [Service] >> Type=forking >> Restart=on-failure >> +Environment="HOME=/var/run/openvswitch" > > Usually in systemd service file you don't need to put quotes, unless you > need to assign a value containing spaces to a variable. Okay. >> EnvironmentFile=/etc/openvswitch/default.conf >> EnvironmentFile=-/etc/sysconfig/openvswitch >> +@begin_dpdk@ >> +ExecStartPre="/usr/sbin/chown :hugetlbfs /dev/hugepages" > > quotes are not needed in ExecStartPre too and I suggest to remove them > for coherency with the other service files. I added the quotes after looking at example documentation; I'll remove it. > chown is in "/usr/bin" both on Fedora and RHEL7. > Moreover, by default, on Fedora and RHEL have "/dev/hugepages" not group > writable and so rtemap files cannot be created so I suggest to add: D'oh! This was a fat-finger; I didn't retest my setup with dpdk, so of course I missed it. > ExecStartPre=/usr/bin/chmod 0775 /dev/hugepages Seems I had done that chmod manually without realizing (more likely one of my setup scripts altered the environment). Will incorporate it into v3. > Unlucky hugetlbfs doesn't support POSIX ACL or this will be more easy > using "setfacl -m "g:openvswitch:rwx" /dev/hugepages" without creating > an hugetlbfs group and without chmoding to 0775. > >> +@end_dpdk@ >> ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \ >> --no-ovsdb-server --no-monitor --system-id=random \ >> --ovs-user=${OVS_USER_ID} \ >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
