On Fri, Dec 18, 2015 at 10:27 AM, Aaron Conole <acon...@redhat.com> wrote:
> The current DPDK vhost socket user and group permissions are derived
> during creation from the DPDK library. This patch adds an action, post
> socket creation, to change the socket permissions and ownership to
> support multi-user systems.
>
> This is especially useful when using vhost-user sockets from qemu, which
> currently requires manual intervention to change the vhost-user sockets
> allowing the qemu user access.
>
> Signed-off-by: Aaron Conole <acon...@redhat.com>
> ---
> INSTALL.DPDK.md | 17 +++++++++++++++
> lib/netdev-dpdk.c | 63
> +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> index b9d92d0..86f934c 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -554,6 +554,23 @@ have arbitrary names.
>
> `./vswitchd/ovs-vsctl set Open_vSwitch .
> other_config:vhost_sock_dir=path`
>
> + - You may specify optional user:group and permissions for the vhost-user
> + sockets. If these are not present, the default ownership is the executing
> + user and group for ovs-vswitchd, and the default permissions are `0600`.
> +
> + To change the vhost-user ownership, you may modify the
> `vhost_sock_owners`
> + entry in the ovsdb like:
> +
> + `./vswitchd/ovs-vsctl set Open_vSwitch . \
> + other_config:vhost_sock_owners=[user][:group]`
> +
> + To change the vhost-user permissions, you may modify the octal value
> + `vhost_sock_permissions` entry in the ovsdb like:
> +
> + `./vswitchd/ovs-vsctl set Open_vSwitch . \
> + other_config:vhost_sock_permissions=permissions`
> +
> +
> DPDK vhost-user VM configuration:
> ---------------------------------
> Follow the steps below to attach vhost-user port(s) to a VM.
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 696430f..ee59250 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -52,6 +52,7 @@
> #include "openvswitch/vlog.h"
> #include "smap.h"
> #include "vswitch-idl.h"
> +#include "daemon.h"
I would try to keep #includes in alphabetical order. The reason for
this is that it reduces possibility of GIT conflicts if someone else
pushed patch after you sent yours for review, but before revewer
applied your patch in his repository.
>
> #include "rte_config.h"
> #include "rte_mbuf.h"
> @@ -106,6 +107,9 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>
> static char *cuse_dev_name = NULL; /* Character device cuse_dev_name. */
> static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */
> +static char *vhost_sock_owner = NULL; /* user:group notation for ownership
> + of vhost-user sockets */
> +static char *vhost_sock_perms = NULL; /* OCTAL string of permission bits */
>
> /*
> * Maximum amount of time in micro seconds to try and enqueue to vhost.
> @@ -682,6 +686,43 @@ netdev_dpdk_vhost_cuse_construct(struct netdev *netdev_)
> return err;
> }
>
> +static void
> +vhost_set_permissions(const char *vhost_sock_location)
> +{
> + unsigned long int mode = strtoul(vhost_sock_perms, NULL, 0);
> + int err = chmod(vhost_sock_location, (mode_t)mode);
> + if (err) {
> + VLOG_ERR("vhost-user socket cannot set permissions to %s (%s).\n",
> + vhost_sock_perms, ovs_strerror(err));
> + return;
> + }
> + VLOG_INFO("Socket %s changed permissions to %s\n", vhost_sock_location,
> + vhost_sock_perms);
> +}
> +
> +static void
> +vhost_set_ownership(const char *vhost_sock_location)
> +{
> + uid_t vhuid;
> + gid_t vhgid;
> +
> + if (get_owners_from_str(vhost_sock_owner, &vhuid, NULL, &vhgid, false)) {
> + VLOG_ERR("vhost-user socket unable to set ownership: %s\n",
> + vhost_sock_owner);
> + return;
> + }
> +
> + int err = chown(vhost_sock_location, vhuid, vhgid);
FYI:
chown and chmod need to be white listed in openvswitch SELinux policy
- https://github.com/TresysTechnology/refpolicy-contrib.git.
Otherwise, SELinux for OVS must be disabled on default Fedora, CentOS
and RHEL distributions for this to work in the first place.
However, ball for this is in my hands here, because I am working on a
patch where tailored SELinux policy will be distributed from our own
packages.
> + if (err) {
> + VLOG_ERR("vhost-user socket unable to set ownership to %s (%s).\n",
> + vhost_sock_owner, ovs_strerror(err));
> + return;
> + }
> +
> + VLOG_INFO("Socket %s changed ownership to %s.\n", vhost_sock_location,
> + vhost_sock_owner);
> +}
> +
> static int
> netdev_dpdk_vhost_user_construct(struct netdev *netdev_)
> {
> @@ -700,6 +741,15 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev_)
> netdev->vhost_id);
> }
> VLOG_INFO("Socket %s created for vhost-user port %s\n",
> netdev->vhost_id, netdev_->name);
> +
Try:
ovs-vsctl set Open_vSwitch . other_config:vhost_sock_owners=:nobody
<--- The first bug is that if group did not exist, then your code
picks up the last group (ssh in my case)
ovs-vsctl set Open_vSwitch . other_config:vhost_sock_permissions=0777
root@prmh-nsx-perf-server118:~/ovs# echo "I want this file to stay
under root:root" > /r
root@prmh-nsx-perf-server118:~/ovs# ls -la /r
-rw-r--r-- 1 root root 10 Jan 29 13:29 /r
root@prmh-nsx-perf-server118:~/ovs# ovs-vsctl add-port br0
"../../../../r" -- set interface "../../../../r" type=dpdkvhostuser
root@prmh-nsx-perf-server118:~/ovs# ls -la /r
-rwxrwxrwx 1 root ssh 10 Jan 29 13:29 /r <---- The second bug is
that you just gave away this file to ssh group.
Note, that running Open vSwitch under confined user does not fully
solve this issue, because you could still change user:group of
/etc/openvswitch/conf.db.
Here is log snippet:
2016-01-29T21:30:36.837Z|00078|dpdk|ERR|vhost-user socket device setup
failure for socket //../../../../r
2016-01-29T21:30:36.837Z|00079|dpdk|INFO|Socket //../../../../r
created for vhost-user port ../../../../r
2016-01-29T21:30:36.837Z|00080|dpdk|INFO|Socket //../../../../r
changed permissions to 0777
2016-01-29T21:30:36.837Z|00081|dpdk|INFO|Socket //../../../../r
changed ownership to :nobody.
In your patch I would change owner:group and permissions of a file
only and only if you are 100% sure that you were the one who just
created it.
> + if (vhost_sock_perms) {
> + vhost_set_permissions(netdev->vhost_id);
> + }
> +
> + if (vhost_sock_owner) {
> + vhost_set_ownership(netdev->vhost_id);
> + }
> +
> err = vhost_construct_helper(netdev_);
> ovs_mutex_unlock(&dpdk_mutex);
> return err;
> @@ -2217,12 +2267,16 @@ __dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg)
>
> VLOG_INFO("DPDK Enabled, initializing");
>
> -#ifdef VHOST_CUSE
> if (process_vhost_flags("cuse_dev_name", strdup("vhost-net"),
> PATH_MAX, ovs_cfg, &cuse_dev_name)) {
> -#else
> + /**
> + * This is ignored when vhost_user anyway...
> + */
> + }
> +
> if (process_vhost_flags("vhost_sock_dir", strdup(ovs_rundir()),
> NAME_MAX, ovs_cfg, &vhost_sock_dir)) {
> +#ifndef VHOST_CUSE
> struct stat s;
>
> err = stat(vhost_sock_dir, &s);
> @@ -2233,6 +2287,11 @@ __dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg)
> #endif
> }
>
> + (void)process_vhost_flags("vhost_sock_owners", NULL, NAME_MAX, ovs_cfg,
> + &vhost_sock_owner);
> + (void)process_vhost_flags("vhost_sock_permissions", "0600", NAME_MAX,
> + ovs_cfg, &vhost_sock_perms);
> +
> /* Get the main thread affinity */
> CPU_ZERO(&cpuset);
> err = pthread_getaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
> --
> 2.6.1.133.gf5b6079
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
