On 5/5/15 11:34 AM, Loftus, Ciara wrote:
On 04/24/2015 04:01 PM, Flavio Leitner wrote:
On Fri, 24 Apr 2015 14:17:17 +0300
Panu Matilainen <pmati...@redhat.com> wrote:

Hi,

A few comments inline...

On 04/21/2015 01:10 PM, Ciara Loftus wrote:
This patch adds support for a new port type to the userspace
datapath called dpdkvhostuser. It adds to the existing
infrastructure of vhost-cuse, however disables vhost-cuse ports
as the default port type, in favour of vhost-user ports. Refer
to the documentation for enabling vhost-cuse ports if desired.

A new dpdkvhostuser port will create a unix domain socket which
when provided to QEMU is used to facilitate communication between
the virtio-net device on the VM and the OVS port on the host.

Signed-off-by: Ciara Loftus <ciara.lof...@intel.com>
[...]

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index f69154b..deb8b83 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -101,8 +101,13 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))

    #define MAX_PKT_BURST 32           /* Max burst size for RX/TX */

+#ifdef VHOST_CUSE
    /* Character device cuse_dev_name. */
    char *cuse_dev_name = NULL;
+#else
+#define VHOST_USER_PORT_SOCK_PATH "/tmp/%s" /* Socket
Location
Template */
Using /tmp for these seems like asking for trouble to me, how about
somewhere /var/run? At least it should be configurable via cli,
similar to the cuse device name.
Why using /tmp would be problematic?
Because it opens up a whole can of worms that is best avoided since
there's no good reason to open it in this case, AFAICS. Predictable
names in a world-writable directory and all, and these are not really
temporary files in the usual sense anyway.

Current rte_vhost_driver_register() will unconditionally nuke any file
in the path passed to it. I'm sure you can imagine some unwanted
scenarios if 'ovs-vsctl add-port' goes around deleting files in a
world-shared directory :)

Anyway, systemd has the RunTimeDirectory= option to make sure the
private directory is created (under /run) when the service is
started and that's the default RPM %{_rundir} where the sock.db, .pid
files and other OVS files are created. It seems you could get the right
path using ovs_rundir().
Right.

        - Panu -

fbl

Thanks for the feedback, I appreciate the input. The latest revision of the 
patch sets the default socket location to the return value of ovs_rundir(). The 
user can also specify an alternate socket location using the --vhost-sock-dir 
flag on the ovs-vswitchd command line.
Great! This is absolutely the right thing to do.

Thanks,
Ciara
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


--
Thomas F. Herbert

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to