> On Tue, May 5, 2015 at 8:58 AM, Ciara Loftus <[email protected]> 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. > > vhost-cuse 'dpdkvhost' ports are still available and can be > > enabled using a configure flag, steps for which are available > > in INSTALL.DPDK.md. > > > > 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 <[email protected]> > > Thanks for the patch I have couple of comments.
Thanks for the feedback. Replies inline. > > .... > > > diff --git a/configure.ac b/configure.ac > > index 068674e..3f635b4 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -165,6 +165,7 @@ AC_ARG_VAR(KARCH, [Kernel Architecture String]) > > AC_SUBST(KARCH) > > OVS_CHECK_LINUX > > OVS_CHECK_DPDK > > +OVS_CHECK_VHOST_CUSE > > OVS_CHECK_PRAGMA_MESSAGE > > AC_SUBST([OVS_CFLAGS]) > > AC_SUBST([OVS_LDFLAGS]) > > I think you can parse build/.config file in configure script and check > what type of vhost is configured for the DPDK build. We already do > this for kernel configuration. You can have look at macro > OVS_CHECK_LINUX_COMPAT in acinclude.m4. This way we can avoid new > OVS > configure option. I have included something similar to this in the new version. The configure flag is gone. > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index 5af15d4..54ead15 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -28,6 +28,7 @@ > > #include <unistd.h> > > #include <stdio.h> > > > > +#include "dirs.h" > > #include "dp-packet.h" > > #include "dpif-netdev.h" > > #include "list.h" > > @@ -101,8 +102,18 @@ 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 */ > > > > -/* Character device cuse_dev_name. */ > > -char *cuse_dev_name = NULL; > > +/* For vhost-user, the path where sockets will be created. > > + * For vhost-cuse, the name of the character device. */ > > +char *vhost_dev_or_sock = NULL; > > + > > +#ifdef VHOST_CUSE > > +char vhost_flag[] = "--cuse_dev_name"; > > +char vhost_flag_default_val[] = "vhost-net"; > > +#else > > +#define VHOST_USER > > +char vhost_flag[] = "--vhost_sock_dir"; > > +char vhost_flag_default_val[PATH_MAX]; /* Initialized at runtime via > ovs_rundir */ > > +#endif > > You can get rid of all the #ifdef by refactoring existing vhost code > and defining separate netdev_class for vost_user and vhost_cuse. I've created separate netdev classes for each in the new version. One #ifdef remains, for netdev_register_provider, as only one of either vhost-cuse or vhost-user ports can be used for an instance of the switch. The rest of the #ifdefs are gone, however. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
