> 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.
Thanks,
Ciara
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
