Hi Ben, I agree that the purpose of the canges was not cleared in the patch. The full discussion is at https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/046061.html. But in a nutshell, this change is a fancy way to avoid the actual problem, that's why it's not intuitive. The actual problem is the current method to change the ovs user at (daemon_become_new_user_linux), drops the CAP_SYS_ADMIN capability, even when you are "changing" from root to itself, which occurs in rhel based distros when you use OVS_USER="root:toot". My PoC solution [2] was to force the CAP_SYS_ADMIN during the user change, but this would give more permission to openvswitch process than necessary on most of the situations, for instance when DPDK and UIO are not required. The fix in the current patch was proposed by Aaron [3], which involves to avoid changing the user when it is already the desired user (and group), this fixes the problem without the downsides of the initial solution and avoids unnecessary calls.
[2] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045959.html [3] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045973.html Regards, Marcos Schwarz On Thu, Feb 1, 2018 at 7:27 PM, Ben Pfaff <[email protected]> wrote: > On Thu, Feb 01, 2018 at 03:42:03PM -0200, Marcos Felipe Schwarz wrote: > > Since 2.8.0 OVS runs as non-root user on rhel distros, but the current > > implementation breaks the ability to run as root with DPDK and as a > > consequence there is no way possible to use UIO drivers on kernel 4.0 and > > newer [1, 2]. > > [1] http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766 > > bd95a658b5e0686a62ec04f5b0 > > [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt > > > > Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user") > > Signed-off-by: Marcos Schwarz <[email protected]> > > Acked-by: Aaron Conole <[email protected]> > > Thanks for the patch. > > I understand from the commit message the ultimate goal of this patch. I > don't yet understand what change it is actually making. In particular, > what is the purpose of the following change; what does it do? > > > --- a/lib/daemon-unix.c > > +++ b/lib/daemon-unix.c > > @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec) > > } > > } > > > > - switch_user = true; > > + if (!uid_verify(uid) || !gid_verify(gid)) > > + switch_user = true; > > } > > Thanks, > > Ben. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
