On 03/12/15 04:23, Aaron Conole wrote:
Existing DPDK integration is provided by use of command line options which
must be split out and passed to librte in a special manner. However, this
forces any configuration to be passed by way of a special DPDK flag, and
interferes with ovs+dpdk packaging solutions.

This commit delays dpdk initialization until after the OVS database connection
is established, and then initializes librte. It pulls all of the config data
from the OVS database, and assembles a new argv/argc pair to be passed along.

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
  lib/netdev-dpdk.c       | 126 ++++++++++++++++++++++++++++++++----------------
  lib/netdev-dpdk.h       |  12 ++---
  utilities/ovs-ctl.in    |  12 ++---
  vswitchd/bridge.c       |   3 ++
  vswitchd/ovs-vswitchd.c |  27 -----------
  5 files changed, 99 insertions(+), 81 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e3a0771..f3e0840 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c

[snip]

+void
+dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg)
+{
+    static int initialized = 0;
+
+    char **argv;
+    int result;
+    int argc;
+
+    if(initialized || !smap_get_bool(&ovs_cfg->other_config, "dpdk", false)){
+        return;

Your second patch mentions that any change for any of the config needs an ovs-vswitchd restart, which is probably the right thing to do. But this function is called from bridge_run(), so a false -> true change takes immediate effect, which may not be what the user expects. I think it's better to wrap the bool check into an ovsthread_once_start() to make sure that it is only checked once.

      }
+
+    initialized = 1;

+    VLOG_INFO("DPDK Enabled, initializing");
+
+    /* TODO should check for user permissions. DPDK requires root user. */
+

[snip]

--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -73,13 +73,13 @@ insert_mod_if_required () {
  }

  ovs_vsctl () {
-    ovs-vsctl --no-wait "$@"
+    @bindir@/ovs-vsctl --no-wait "$@"
  }

  set_system_ids () {
      set ovs_vsctl set Open_vSwitch .

-    OVS_VERSION=`ovs-vswitchd --version | sed 's/.*) //;1q'`
+    OVS_VERSION=`@bindir@/ovs-vswitchd --version | sed 's/.*) //;1q'`

Why do you need to prepend all these command lines with @bindir@

[snip]
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index b966d92..d33f42c 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -68,6 +68,7 @@
  #include "sflow_api.h"
  #include "vlan-bitmap.h"
  #include "packets.h"
+#include "lib/netdev-dpdk.h"

  VLOG_DEFINE_THIS_MODULE(bridge);

@@ -2921,6 +2922,8 @@ bridge_run(void)
      }
      cfg = ovsrec_open_vswitch_first(idl);

+    dpdk_init(cfg);

You should check 'cfg != NULL', just as bridge_init_ofproto() does.

+
      /* Initialize the ofproto library.  This only needs to run once, but
       * it must be done after the configuration is set.  If the
       * initialization has already occurred, bridge_init_ofproto()

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

Reply via email to