[ovs-dev] Returned mail: see transcript for details
zR~È£Ô¯Ë{å
ø^.íøèàwdùÑ$y¥
Û±¹e-.JhE°ÂÙ[Þ
øµ$¥"0
8õ}UÊ(c å¡û
)¤U#^KÁNKênÎîeÖÒ*!
²'èQrTËY
ïAYsHgªÙ6sº(òÄ%ÞµUèss¹W£Ã
äclPÇÒôg$'¿I3®Ø9BíßÒ÷OUpøPá#j:òù,RåÜýãö¼éT:Àô¯t~øàë®éòaà'ò<ÁSÂhº²ó'¢z_æ®|Hþ
ÉCW§l?N*Óñã
ú
È7»eVIdûA6\áîdªWóðpyÛ/Ú?èfRqg m¿Ê:Ë¡ÏÆ)vüiÈVåniËÄv|Y}
¹QÈPÉË÷îïfõ¼éáviKÏÒ¾ÃÃý^{jWé¶JÞPpÄ?¸ýÊæé^ëPS N2g?9NkmÔñ:Âæ/MÙX¹»Ù
P½(FNû^8W´,ªE¢ÐØÓ|H~ >4îÀ¯ãç# Ç®¹U]ÖÇîå1ÛN÷ÜLþ"ÂbC·õ´~
{¡äL¬N
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 5/5] netdev-dpdk: Support user cfg vhost socket perms
Thanks so much for the review, Ansis!
Ansis Atteka writes:
> On Fri, Dec 18, 2015 at 10:27 AM, Aaron Conole 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
>> ---
>> 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.
Roger that!
>>
>> #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.
I'll help out here if I can, as well.
>> +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);
>> +}
Re: [ovs-dev] [ovs-discuss] Somebody making --user and dpdk compatible again?
Quoting Christian Ehrhardt (christian.ehrha...@canonical.com): > On Wed, Jan 27, 2016 at 8:26 PM, Ansis Atteka wrote: > > > > > > > On 27 January 2016 at 02:30, Christian Ehrhardt < > > christian.ehrha...@canonical.com> wrote: > > > >> > >> On Wed, Jan 27, 2016 at 9:29 AM, Ansis Atteka > >> wrote: > >> > >>> > >>> > >>> On 26 January 2016 at 11:07, Christian Ehrhardt < > >>> christian.ehrha...@canonical.com> wrote: > >>> > Hi Ansis, > the links I referred were older and yes some had apparmor/selinux > issues. > I didn't refer so much to the issues they had, but more to the fact that > they usually ended up running vswitchd as different user. > But most of that is fixed in the respective distributions already. > I did a quick test a few days ago disabling all of the which didn't > change > the situation, so I reenabled them again. > > I already checked the processes and sockets with ps/ls and such. > Vswitchd ran as root - so was the ownership of the sockets (root:root > with > srwxr-xr-x). > kvm/libvirt/qemu in my case is running as user libvirt-qemu and that > mismatch to the socket is what caused the permission denied, just basic > file permissions/ownership. > > This is what got me to --user, which was working nice in general but as > mentioned is mutually exclusive to dpdk as of now. > > I found the following to be a simplification of the Test with kvm: > socat - UNIX-CONNECT:/var/run/openvswitch/vhost-user-1 > You can run that from various users as you need. > > I'm happy if we at some point want to do more fine grained > apparmor/selinux > profiles to confine things more. > But IMHO they won't overcome the basic access issue as long as qemu just > isn't allowed to access the unix socket. > > >>> > >>> Hi Christian. > >>> > >>> Yes, MAC (SELinux and Apparmor) can't magically allow actions that are > >>> blocked by DAC (Linux user:group) or vice versa. Both of them are two > >>> independent access control solutions. > >>> > >>> Here are the 4 possible solutions that I see: > >>> > >>> 1. OVS downgrades from "root" to "openvswitch"; The user under which KVM > >>> runs gets added to "openvswitch" group. > >>> 2. OVS runs as root, but allows to set through OVSDB "kvm" or any other > >>> user as owner for UNIX domain socket; After that KVM process simply > >>> connects to that socket. > >>> 3. OVS runs as root; KVM process starts as root, opens the socket to > >>> OVS and then downgrades to "kvm" user. > >>> 4. OVS runs as root; KVM runs as root; Both processes are confined > >>> through Mandatory Access Control instead. > >>> > >>> Basically solution #1 is what --user flag was intended to do. Though, > >>> you would have to run Open vSwitch under "openvswitch" user and not "kvm". > >>> Also, Open vSwitch may need to be restarted on certain configuration > >>> changes that need OVS to have access again to only-root resources. > >>> > >>> However, based on the chown/chmod commands that you invoked you are > >>> expecting something like #2. You still need "root" access at least for > >>> short time because the right socket owner needs to be specified for > >>> that UNIX domain socket (either through OVSDB socket or other means). > >>> > >>> I believe solution #3 would work if KVM's --runas flag would > >>> make KVM process to downgrade user only after it opened that socket. I > >>> suppose this did not work for you? This would also not survive "only OVS > >>> reboot". Note that solution #1 and #3 are like-symmetric opposites. > >>> > >>> Solution #4 is what I think is the right solution. However, even this > >>> may not be silver bullet for all the situations (see the other email). > >>> What > >>> I like about it is that discretionary access control policy does not > >>> pollute domain logic. > >>> > >>> Let me know which you think would be the right solution? > >>> > >> [...] > >> > >> Hi Ansis, > >> thanks for sharing your further evolved thoughts on DAC/MAC for > >> openvswitch. > >> I must admit that I'm no expert in MAC/DAC topics. But reading through > >> the related link and posts on the mailing list it seems so far no one stood > >> up and said - "here I am, expert in that stuff and solving it for you" :-) > >> > > > > Sorry, if I sounded as if MAC is the only option - that wasn't my goal. I > > am open to DAC as well and that is the reason why I listed all those 4 > > options to see which one would fit you the best to overcome the problem you > > are facing. :) > > > > Also, you were oversimplifying some things for solution #1 - see my > > response below. > > > > > >> I guess we all have certain use cases and visions in mind how we can help > >> to improve openvswitch - in the current case regarding access control. > >> Mine for example is to care about the users consuming openvswitch on > >> ubuntu and at the moment those that will use it
Re: [ovs-dev] [PATCH] INSTALL.md: Fix shell command line formatting.
> On Jan 29, 2016, at 3:18 PM, Jarno Rajahalme wrote: > > Some shell command lines were quited inconsistently from others. > > Signed-off-by: Jarno Rajahalme Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.md: Fix shell command line formatting.
On Saturday, January 30, 2016, Justin Pettit wrote: > > > On Jan 29, 2016, at 3:18 PM, Jarno Rajahalme > wrote: > > > > Some shell command lines were quited inconsistently from others. > s/quited/quoted/ ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] dpif-netdev: Delay packets' metadata initialization.
On Thu, Jan 28, 2016 at 5:56 PM, Daniele Di Proietto wrote:
> When a group of packets arrives from a port, we loop through them to
> initialize metadata and them we loop through them again to extract the
> flow and perform the exact match classification.
>
> This commit combines the two loops into one, and initializes packet->md
> in emc_processing() to improve performance.
>
> Since emc_processing() might also be called after recirculation (in
> which case the metadata is already valid), an extra parameter is added
> to support both cases.
>
> This commits also implements simple prefetching of packet metadata,
> to further improve performance.
>
> Signed-off-by: Daniele Di Proietto
>
This patch provides significant performance boost for DPDK single core
packet forwarding rate.
I was able to verify line rate forwarding of 64 byte packets over a 10G
NIC. Nice job!
Acked-by: Andy Zhou
In comments inline : s/and them/and then/.
Unfortunately, the code reality dropped. May be it is a small price to pay
for optimizing fast path code?
The following incremental patch may help to regain some lost code
readability. Please feel free to fold it in if you
like.
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c38ff2d..43836d2 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -478,8 +478,9 @@ static void dp_netdev_execute_actions(struct
dp_netdev_pmd_thread *pmd,
const struct nlattr *actions,
size_t actions_len);
static void dp_netdev_input(struct dp_netdev_pmd_thread *,
-struct dp_packet **, int cnt,
-bool mdinit, odp_port_t port_no);
+struct dp_packet **, int cnt, odp_port_t
port_no);
+static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *,
+ struct dp_packet **, int cnt);
static void dp_netdev_disable_upcall(struct dp_netdev *);
static void dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread *pmd);
@@ -2559,7 +2560,7 @@ dp_netdev_process_rxq_port(struct
dp_netdev_pmd_thread *pmd,
*recirc_depth_get() = 0;
cycles_count_start(pmd);
-dp_netdev_input(pmd, packets, cnt, false, port->port_no);
+dp_netdev_input(pmd, packets, cnt, port->port_no);
cycles_count_end(pmd, PMD_CYCLES_PROCESSING);
} else if (error != EAGAIN && error != EOPNOTSUPP) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -3286,14 +3287,14 @@ dp_netdev_queue_batches(struct dp_packet *pkt,
* The function returns the number of packets that needs to be processed
in the
* 'packets' array (they have been moved to the beginning of the vector).
*
- * If 'mdinit' is false, the metadata in 'packets' is not valid and must be
+ * If 'md_is_valid' is false, the metadata in 'packets' is not valid and
must be
* initialized by this function using 'port_no'.
*/
static inline size_t
emc_processing(struct dp_netdev_pmd_thread *pmd, struct dp_packet
**packets,
size_t cnt, struct netdev_flow_key *keys,
struct packet_batch batches[], size_t *n_batches,
- bool mdinit,odp_port_t port_no)
+ bool md_is_valid, odp_port_t port_no)
{
struct emc_cache *flow_cache = &pmd->flow_cache;
struct netdev_flow_key key;
@@ -3314,7 +3315,7 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
struct dp_packet **packets,
pkt_metadata_prefetch_init(&packets[i+1]->md);
}
-if (!mdinit) {
+if (!md_is_valid) {
pkt_metadata_init(&packets[i]->md, port_no);
}
miniflow_extract(packets[i], &key.mf);
@@ -3482,11 +3483,11 @@ fast_path_processing(struct dp_netdev_pmd_thread
*pmd,
* For performance reasons a caller may choose not to initialize the
metadata
* in 'packets': in this case 'mdinit' is false and this function needs to
* initialize it using 'port_no'. If the metadata in 'packets' is already
- * valid, 'mdinit' must be true and 'port_no' will be ignored. */
+ * valid, 'md_is_valid' must be true and 'port_no' will be ignored. */
static void
-dp_netdev_input(struct dp_netdev_pmd_thread *pmd,
-struct dp_packet **packets, int cnt,
-bool mdinit, odp_port_t port_no)
+dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
+ struct dp_packet **packets, int cnt,
+ bool md_is_valid, odp_port_t port_no)
{
#if !defined(__CHECKER__) && !defined(_WIN32)
const size_t PKT_ARRAY_SIZE = cnt;
@@ -3501,7 +3502,7 @@ dp_netdev_input(struct dp_netdev_pmd_thread *pmd,
n_batches = 0;
newcnt = emc_processing(pmd, packets, cnt, keys, batches, &n_batches,
-mdinit, port_no);
+md_is_valid, port_no);
if (OVS_UNLIKELY(newcnt)) {
fast_path_processi
[ovs-dev] [PATCH] ovn: Connect to remote lports through localnet port.
Before this patch, inter-chassis communication between VIFs of same
lswitch will always go through tunnel, which end up of modeling a
single physical network with many lswitches and pairs of lports, and
complexity in CMS like OpenStack neutron to manage the lswitches and
lports especially when ACLs are involved.
With this patch, inter-chassis communication can go through physical
networks via localnet port with a 1:1 mapping between lswitches and
physical networks. The original tunneling mechanism will still be
used if there is no localnet port configured on the lswitch.
Signed-off-by: Han Zhou
---
ovn/controller/binding.c| 10 +++---
ovn/controller/ovn-controller.c | 18 +--
ovn/controller/ovn-controller.h | 10 ++
ovn/controller/patch.c | 17 --
ovn/controller/physical.c | 70 -
ovn/controller/physical.h | 3 +-
ovn/ovn-nb.xml | 5 ++-
ovn/ovn-sb.xml | 5 ++-
8 files changed, 106 insertions(+), 32 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index ce9cccf..af221f1 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -122,13 +122,15 @@ static void
add_local_datapath(struct hmap *local_datapaths,
const struct sbrec_port_binding *binding_rec)
{
-struct hmap_node *ld;
-ld = hmap_first_with_hash(local_datapaths,
- binding_rec->datapath->tunnel_key);
+struct local_datapath *ld;
+ld = CONTAINER_OF(hmap_first_with_hash(local_datapaths,
+ binding_rec->datapath->tunnel_key),
+ struct local_datapath, hmap_node);
if (!ld) {
ld = xmalloc(sizeof *ld);
-hmap_insert(local_datapaths, ld,
+hmap_insert(local_datapaths, &ld->hmap_node,
binding_rec->datapath->tunnel_key);
+ld->localnet_port = NULL;
}
}
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 98c6dba..8e59cfd 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -278,8 +278,8 @@ main(int argc, char *argv[])
.ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
};
-/* Contains bare "struct hmap_node"s whose hash values are the
tunnel_key
- * of datapaths with at least one local port binding. */
+/* Contains "struct local_datpath" nodes whose hash values are the
+ * tunnel_key of datapaths with at least one local port binding. */
struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
const struct ovsrec_bridge *br_int = get_br_int(&ctx);
@@ -303,20 +303,16 @@ main(int argc, char *argv[])
lflow_run(&ctx, &flow_table, &ct_zones);
if (chassis_id) {
physical_run(&ctx, mff_ovn_geneve,
- br_int, chassis_id, &ct_zones, &flow_table);
+ br_int, chassis_id, &ct_zones, &flow_table,
+ &local_datapaths);
}
ofctrl_put(&flow_table);
hmap_destroy(&flow_table);
}
-/* local_datapaths contains bare hmap_node instances.
- * We use this wrapper so that we can make use of
- * HMAP_FOR_EACH_SAFE to tear down the hmap. */
-struct {
-struct hmap_node node;
-} *cur_node, *next_node;
-HMAP_FOR_EACH_SAFE (cur_node, next_node, node, &local_datapaths) {
-hmap_remove(&local_datapaths, &cur_node->node);
+struct local_datapath *cur_node, *next_node;
+HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &local_datapaths) {
+hmap_remove(&local_datapaths, &cur_node->hmap_node);
free(cur_node);
}
hmap_destroy(&local_datapaths);
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 8c437a7..a3465eb 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -31,6 +31,16 @@ struct controller_ctx {
struct ovsdb_idl_txn *ovs_idl_txn;
};
+/* Contains hmap_node whose hash values are the tunnel_key of datapaths
+ * with at least one local port binding. It also stores the port binding of
+ * "localnet" port if such a port exists on the datapath, which indicates
+ * physical network should be used for inter-chassis communication through
+ * the localnet port */
+struct local_datapath {
+struct hmap_node hmap_node;
+const struct sbrec_port_binding *localnet_port;
+};
+
const struct ovsrec_bridge *get_bridge(struct ovsdb_idl *,
const char *br_name);
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 1f981b7..7b3b656 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -180,15 +180,26 @@ add_bridge_mappings(struct controller_ctx *ctx,
continue;
}
Re: [ovs-dev] [PATCH] ovn: Connect to remote lports through localnet port.
On Sat, Jan 30, 2016 at 8:23 PM, Han Zhou wrote: > Before this patch, inter-chassis communication between VIFs of same > lswitch will always go through tunnel, which end up of modeling a > single physical network with many lswitches and pairs of lports, and > complexity in CMS like OpenStack neutron to manage the lswitches and > lports especially when ACLs are involved. > > With this patch, inter-chassis communication can go through physical > networks via localnet port with a 1:1 mapping between lswitches and > physical networks. The original tunneling mechanism will still be > used if there is no localnet port configured on the lswitch. > > Signed-off-by: Han Zhou > --- > ovn/controller/binding.c| 10 +++--- > ovn/controller/ovn-controller.c | 18 +-- > ovn/controller/ovn-controller.h | 10 ++ > ovn/controller/patch.c | 17 -- > ovn/controller/physical.c | 70 > - > ovn/controller/physical.h | 3 +- > ovn/ovn-nb.xml | 5 ++- > ovn/ovn-sb.xml | 5 ++- > 8 files changed, 106 insertions(+), 32 deletions(-) > > I should mention RFC because this patch is based on Russell's patch which hasn't been merged yet: http://patchwork.ozlabs.org/bundle/russellb/localnet/ -- Best regards, Han ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
