On 23/10/2020 12:58, Mark Michelson wrote:
On 10/23/20 3:38 AM, Anton Ivanov wrote:

On 22/10/2020 20:50, Mark Michelson wrote:
This appears to be exactly the same as patch 4 from the previous series. I pointed out some problems with this patch in that series. Please at least address the bugs I pointed out. Thanks.


No comments from you on patch 4 in series 4:

https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/

You had comments on 1 and 3. I addressed both.

A.

Since my reply seems to have been consumed by the mailing list, here's a pastebin of my original reply:


1. I had similar ideas - trying to deduce execution times and I decided not to. There is no way to guess the key deciding factor which is how much time it takes to sync-up and is it worth it to push things into helper threads.

2. The settings at present default to always in-parallel. It will not be a large penalty on small systems and it will be a significant advantage on large ones.

I am happy to drop this patch for now out of the sequence and leave it to "always in parallel" except on systems which have less than 4 cores per NUMA node (or no NUMA and less than 4 cores total).

Brgds,

A.


https://paste.centos.org/view/11e768f0




On 10/14/20 12:27 PM, [email protected] wrote:
From: Anton Ivanov <[email protected]>

Add commands to control and display single-threaded to multi-
threaded cutoff.

Signed-off-by: Anton Ivanov <[email protected]>
---
   northd/ovn-northd.c | 47 ++++++++++++++++++++++++++++++++++++++++++---
   1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index e8f167d8d..cb9e791f3 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -59,6 +59,8 @@ static unixctl_cb_func ovn_northd_resume;
   static unixctl_cb_func ovn_northd_is_paused;
   static unixctl_cb_func ovn_northd_status;
   static unixctl_cb_func cluster_state_reset_cmd;
+static unixctl_cb_func get_param_cutoff;
+static unixctl_cb_func set_param_cutoff;
     struct northd_context {
       struct ovsdb_idl *ovnnb_idl;
@@ -11492,8 +11494,8 @@ static void init_lflows_thread_pool(void)
    * Setting to 1 forces "all parallel" lflow build.
    */
   -#define OD_CUTOFF 1
-#define OP_CUTOFF 1
+static ssize_t lflow_od_cuttoff = 1;
+static ssize_t lflow_op_cutoff = 1;
     static void
   build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports, @@ -11504,7 +11506,8 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
   {
       char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);    -    if (hmap_count(datapaths) > OD_CUTOFF || hmap_count(ports) > OP_CUTOFF) {
+    if (hmap_count(datapaths) > lflow_od_cuttoff ||
+            hmap_count(ports) > lflow_op_cutoff) {
             struct hmap *lflow_segs;
           struct lswitch_flow_build_info *lsiv;
@@ -13184,6 +13187,14 @@ main(int argc, char *argv[])
       unixctl_command_register("is-paused", "", 0, 0, ovn_northd_is_paused,
                                &state);
       unixctl_command_register("status", "", 0, 0, ovn_northd_status, &state);
+    unixctl_command_register("set-datapath-cutoff", "", 0, 0,
+                             set_param_cutoff, &lflow_od_cuttoff);
+    unixctl_command_register("set-port-cutoff", "", 0, 0,
+                             set_param_cutoff, &lflow_op_cutoff);
+    unixctl_command_register("get-datapath-cutoff", "", 0, 0,
+                             get_param_cutoff, &lflow_od_cuttoff);
+    unixctl_command_register("get-port-cutoff", "", 0, 0,
+                             get_param_cutoff, &lflow_op_cutoff);
         bool reset_ovnsb_idl_min_index = false;
       unixctl_command_register("sb-cluster-state-reset", "", 0, 0,
@@ -13609,3 +13620,33 @@ cluster_state_reset_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED,
       poll_immediate_wake();
       unixctl_command_reply(conn, NULL);
   }
+
+static void set_param_cutoff
+(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                          const char *argv[],
+                          void *param_)
+{
+    long new_cutoff;
+    ssize_t *param = param_;
+
+    if (str_to_long(argv[1], 10, &new_cutoff)) {
+        if (new_cutoff > 0) {
+            *param = new_cutoff;
+            return;
+        }
+    }
+    unixctl_command_reply_error(conn, "unsigned integer required");
+}
+
+static void get_param_cutoff
+(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                          const char *argv[] OVS_UNUSED,
+                          void *param_)
+{
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    ssize_t *param = param_;
+
+    ds_put_format(&ds, "%ld\n", *param);
+    unixctl_command_reply(conn, ds_cstr(&ds));
+    ds_destroy(&ds);
+}






--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to