Re: [ovs-dev] [PATCH] dpif-netdev-perf: Fix linker unresolved symbols on Windows

[Ben Pfaff]

On Tue, May 15, 2018 at 03:48:59AM +0300, Alin Gabriel Serdean wrote:
> MSVC complains:
> "libopenvswitch.lib(dpif-netdev.obj) : error LNK2019: unresolved external
> symbol pmd_perf_start_iteration referenced in function pmd_thread_main
> libopenvswitch.lib(dpif-netdev.obj) : error LNK2019: unresolved external
> symbol pmd_perf_end_iteration referenced in function pmd_thread_main"
> 
> Remove inline keyword from the declaration of:
> `pmd_perf_start_iteration` and `pmd_perf_end_iteration`
> 
> More on the subject:
> https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/function-inlining-problems
> 
> Fixes: broken build on Windows
> 
> Signed-off-by: Alin Gabriel Serdean 

It's pretty risky to use "inline" other than as part of "static inline"
because of portability problems, even between different versions of the
same compiler (GCC changed its own default interpretation from one
version to another to comply with C99 rules).

The OVS coding style document discourages "inline" in .c files anyway.

Acked-by: Ben Pfaff 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 06/10] ovn-controller: Incremental logical flow processing

[Han Zhou]

Persistents flow-table and implements change handler of flow_output
for SB lflow changes.

Signed-off-by: Han Zhou 
---
 include/ovn/actions.h   |   3 +
 ovn/controller/lflow.c  | 178 +++--
 ovn/controller/lflow.h  |  20 +++-
 ovn/controller/ofctrl.c | 241 
 ovn/controller/ofctrl.h |  29 -
 ovn/controller/ovn-controller.c | 116 +--
 ovn/controller/physical.c   |  86 +++---
 ovn/controller/physical.h   |   5 +-
 ovn/lib/actions.c   |   6 +-
 ovn/lib/extend-table.c  |  60 +++---
 ovn/lib/extend-table.h  |  16 ++-
 11 files changed, 550 insertions(+), 210 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 6384651..5e4bd5b 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -520,6 +520,9 @@ struct ovnact_encode_params {
 /* A struct to figure out the meter_id for meter actions. */
 struct ovn_extend_table *meter_table;
 
+/* The logical flow uuid that drove this action. */
+struct uuid lflow_uuid;
+
 /* OVN maps each logical flow table (ltable), one-to-one, onto a physical
  * OpenFlow flow table (ptable).  A number of parameters describe this
  * mapping and data related to flow tables:
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index f022aa3..d75e03f 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -58,7 +58,8 @@ struct condition_aux {
 const struct chassis_index *chassis_index;
 };
 
-static void consider_logical_flow(struct controller_ctx *ctx,
+static bool consider_logical_flow(struct ovn_desired_flow_table *flow_table,
+  struct controller_ctx *ctx,
   const struct chassis_index *chassis_index,
   const struct sbrec_logical_flow *lflow,
   const struct hmap *local_datapaths,
@@ -71,7 +72,6 @@ static void consider_logical_flow(struct controller_ctx *ctx,
   uint32_t *conj_id_ofs,
   const struct shash *addr_sets,
   const struct shash *port_groups,
-  struct hmap *flow_table,
   struct sset *active_tunnels,
   struct sset *local_lport_ids);
 
@@ -134,7 +134,8 @@ is_switch(const struct sbrec_datapath_binding *ldp)
 
 /* Adds the logical flows from the Logical_Flow table to flow tables. */
 static void
-add_logical_flows(struct controller_ctx *ctx,
+add_logical_flows(struct ovn_desired_flow_table *flow_table,
+  struct controller_ctx *ctx,
   const struct chassis_index *chassis_index,
   const struct hmap *local_datapaths,
   struct ovn_extend_table *group_table,
@@ -142,11 +143,10 @@ add_logical_flows(struct controller_ctx *ctx,
   const struct sbrec_chassis *chassis,
   const struct shash *addr_sets,
   const struct shash *port_groups,
-  struct hmap *flow_table,
   struct sset *active_tunnels,
-  struct sset *local_lport_ids)
+  struct sset *local_lport_ids,
+  uint32_t *conj_id_ofs)
 {
-uint32_t conj_id_ofs = 1;
 const struct sbrec_logical_flow *lflow;
 
 struct hmap dhcp_opts = HMAP_INITIALIZER(_opts);
@@ -168,12 +168,16 @@ add_logical_flows(struct controller_ctx *ctx,
 nd_ra_opts_init(_ra_opts);
 
 SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
-consider_logical_flow(ctx, chassis_index,
-  lflow, local_datapaths,
-  group_table, meter_table, chassis,
-  _opts, _opts, _ra_opts,
-  _id_ofs, addr_sets, port_groups,
-  flow_table, active_tunnels, local_lport_ids);
+if (!consider_logical_flow(flow_table, ctx, chassis_index,
+   lflow, local_datapaths,
+   group_table, meter_table, chassis,
+   _opts, _opts, _ra_opts,
+   conj_id_ofs, addr_sets, port_groups,
+   active_tunnels, local_lport_ids)) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+VLOG_ERR_RL(, "Conjunction id overflow when processing lflow "
+UUID_FMT, UUID_ARGS(>header_.uuid));
+}
 }
 
 dhcp_opts_destroy(_opts);
@@ -181,8 +185,95 @@ add_logical_flows(struct controller_ctx *ctx,
 nd_ra_opts_destroy(_ra_opts);
 }
 
-static void
-consider_logical_flow(struct controller_ctx *ctx,
+bool
+lflow_handle_changed_flows(struct 

[ovs-dev] [PATCH 09/10] ovn-controller: Avoid forced recompute when not needed

[Han Zhou]

Signed-off-by: Han Zhou 
---
 ovn/controller/ovn-controller.c | 11 ---
 ovn/lib/inc-proc-eng.c  | 19 +++
 ovn/lib/inc-proc-eng.h  |  4 
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index a0f0315..19209eb 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -1150,9 +1150,14 @@ main(int argc, char *argv[])
 
 }
 if (old_engine_run_id == engine_run_id) {
-VLOG_DBG("engine did not run, force recompute next time: "
- "br_int %p, chassis %p", br_int, chassis);
-engine_set_force_recompute(true);
+if (engine_need_run(_flow_output)) {
+VLOG_DBG("engine did not run, force recompute next time: "
+ "br_int %p, chassis %p", br_int, chassis);
+engine_set_force_recompute(true);
+} else {
+VLOG_DBG("engine did not run, and it was not needed either: "
+ "br_int %p, chassis %p", br_int, chassis);
+}
 } else {
 engine_set_force_recompute(false);
 }
diff --git a/ovn/lib/inc-proc-eng.c b/ovn/lib/inc-proc-eng.c
index 54c7fd6..2e5e907 100644
--- a/ovn/lib/inc-proc-eng.c
+++ b/ovn/lib/inc-proc-eng.c
@@ -123,3 +123,22 @@ engine_run(struct engine_node *node, uint64_t run_id)
 
 }
 
+bool
+engine_need_run(struct engine_node *node)
+{
+size_t i;
+
+if (!node->n_inputs) {
+node->run(node);
+VLOG_DBG("input node: %s, changed: %d", node->name, node->changed);
+return node->changed;
+}
+
+for (i = 0; i < node->n_inputs; i++) {
+if (engine_need_run(node->inputs[i].node)) {
+return true;
+}
+}
+
+return false;
+}
diff --git a/ovn/lib/inc-proc-eng.h b/ovn/lib/inc-proc-eng.h
index 3e0ec16..c5deaa5 100644
--- a/ovn/lib/inc-proc-eng.h
+++ b/ovn/lib/inc-proc-eng.h
@@ -131,6 +131,10 @@ engine_run(struct engine_node *node, uint64_t run_id);
 void
 engine_cleanup(struct engine_node *node);
 
+/* Check if engine needs to run, i.e. any change to be processed */
+bool
+engine_need_run(struct engine_node *node);
+
 /* Get the input node with  for  */
 static inline struct engine_node *
 engine_get_input(const char *input_name, struct engine_node *node)
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 08/10] ovn-controller: port-binding incremental processing for physical flows

[Han Zhou]

This patch implements change handler for port-binding in flow_output
for physical flows computing, so that physical flow computing will
be incremental.

This patch together with previous incremental processing engine
related changes supports incremental processing for lflow changes
and port-binding changes of lports on other HVs, which are the most
common scenarios in a cloud where workloads come up and down.

In ovn-scale-test env [1], the total execution time of creating and
binding 10k ports on 1k HVs with 40 lswitches and 8 lrouters
(5 lswitches/lrouter), decreased from 3h40m to 1h50m because of the
less CPU on HVs. The CPU time of ovn-controller for additional 500
lports creating and binding (on top of already existed 10k lports)
decreased 90% comparing with master. Latency for end-to-end operations
of one extra port on top of the 10k lports, start from port-creation
until all flows installation on all related HVs is also improved
significantly from 20.6s to 7.3s.

[1] https://github.com/openvswitch/ovn-scale-test

Signed-off-by: Han Zhou 
---
 ovn/controller/ovn-controller.c |  76 ++-
 ovn/controller/physical.c   | 130 +---
 ovn/controller/physical.h   |   9 +++
 3 files changed, 180 insertions(+), 35 deletions(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 2d021c4..a0f0315 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -899,6 +899,80 @@ flow_output_sb_logical_flow_handler(struct engine_node 
*node)
 return handled;
 }
 
+static bool
+flow_output_sb_port_binding_handler(struct engine_node *node)
+{
+struct controller_ctx *ctx = (struct controller_ctx *)node->context;
+struct ed_type_runtime_data *data =
+(struct ed_type_runtime_data *)engine_get_input(
+"runtime_data", node)->data;
+struct hmap *local_datapaths = >local_datapaths;
+struct sset *active_tunnels = >active_tunnels;
+struct chassis_index *chassis_index = >chassis_index;
+struct simap *ct_zones = >ct_zones;
+const struct ovsrec_bridge *br_int = get_br_int(ctx);
+
+const char *chassis_id = get_chassis_id(ctx->ovs_idl);
+
+
+const struct sbrec_chassis *chassis = NULL;
+if (chassis_id) {
+chassis = get_chassis(ctx->ovnsb_idl, chassis_id);
+}
+
+ovs_assert(br_int && chassis);
+
+struct ed_type_flow_output *fod =
+(struct ed_type_flow_output *)node->data;
+struct ovn_desired_flow_table *flow_table = >flow_table;
+
+/* XXX: now we handles port-binding changes for physical flow processing
+ * only, but port-binding change can have impact to logical flow
+ * processing, too, in below circumstances:
+ *
+ *  - When a port-binding for a lport is inserted/deleted but the lflow
+ *using that lport doesn't change.
+ *
+ *This is likely to happen only when the lport name is used by ACL
+ *match condition, which is specified by user. Even in that case, when
+ *port is actually bound on the chassis it will trigger recompute on
+ *that chassis since ovs interface is updated. So the only situation
+ *this would have real impact is when user defines an ACL that includes
+ *lport that is not the ingress/egress lport, e.g.:
+ *
+ *to-lport 1000 'outport=="A" && inport=="B"' allow-related
+ *
+ *If "B" is created and bound after the ACL is created, the ACL may not
+ *take effect on the chassis where "A" is bound, until a recompute is
+ *triggered there later.
+ *
+ *  - When is_chassis_resident is used in lflow. In this case the port
+ *binding is patch type, since this condition is used only for lrouter
+ *ports. In current "runtime_data" handling, port-binding changes of
+ *patch ports always trigger recomputing. So it is fine even if we do
+ *not handle it here.
+ *
+ *  - When a mac-binding doesn't change but the port-binding related to
+ *that mac-binding is deleted. In this case the neighbor flow generated
+ *for the mac-binding should be deleted. This would not cause any real
+ *issue for now, since mac-binding change triggers recomputing.
+ *
+ * To address the above issues, we will need to maintain a mapping between
+ * lport names and the lflows that uses them, and reprocess the related
+ * lflows when a port-binding corresponding to a lport name changes.
+ */
+
+enum mf_field_id mff_ovn_geneve = ofctrl_get_mf_field_id();
+physical_handle_port_binding_changes(flow_table,
+ ctx, mff_ovn_geneve,
+ chassis, ct_zones,
+ local_datapaths,
+ chassis_index, active_tunnels);
+
+node->changed = true;
+return true;
+}
+
 

[ovs-dev] [PATCH 10/10] ovn-controller: incremental processing for multicast group changes

[Han Zhou]

Signed-off-by: Han Zhou 
---
 ovn/controller/ovn-controller.c | 37 -
 ovn/controller/physical.c   | 23 +++
 ovn/controller/physical.h   |  8 +++-
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 19209eb..d77b8f7 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -973,6 +973,41 @@ flow_output_sb_port_binding_handler(struct engine_node 
*node)
 return true;
 }
 
+static bool
+flow_output_sb_multicast_group_handler(struct engine_node *node)
+{
+struct controller_ctx *ctx = (struct controller_ctx *)node->context;
+struct ed_type_runtime_data *data =
+(struct ed_type_runtime_data *)engine_get_input(
+"runtime_data", node)->data;
+struct hmap *local_datapaths = >local_datapaths;
+struct simap *ct_zones = >ct_zones;
+const struct ovsrec_bridge *br_int = get_br_int(ctx);
+
+const char *chassis_id = get_chassis_id(ctx->ovs_idl);
+
+
+const struct sbrec_chassis *chassis = NULL;
+if (chassis_id) {
+chassis = get_chassis(ctx->ovnsb_idl, chassis_id);
+}
+
+ovs_assert(br_int && chassis);
+
+struct ed_type_flow_output *fod =
+(struct ed_type_flow_output *)node->data;
+struct ovn_desired_flow_table *flow_table = >flow_table;
+
+enum mf_field_id mff_ovn_geneve = ofctrl_get_mf_field_id();
+physical_handle_mc_group_changes(flow_table,
+ ctx, mff_ovn_geneve,
+ chassis, ct_zones,
+ local_datapaths);
+node->changed = true;
+return true;
+
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -1053,7 +1088,7 @@ main(int argc, char *argv[])
 
 engine_add_input(_flow_output, _sb_chassis, NULL);
 engine_add_input(_flow_output, _sb_encap, NULL);
-engine_add_input(_flow_output, _sb_multicast_group, NULL);
+engine_add_input(_flow_output, _sb_multicast_group, 
flow_output_sb_multicast_group_handler);
 engine_add_input(_flow_output, _sb_datapath_binding, NULL);
 engine_add_input(_flow_output, _sb_port_binding, 
flow_output_sb_port_binding_handler);
 engine_add_input(_flow_output, _sb_mac_binding, NULL);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 32b4f6e..8482e31 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -937,6 +937,29 @@ physical_handle_port_binding_changes(struct 
ovn_desired_flow_table *flow_table,
 }
 
 void
+physical_handle_mc_group_changes(struct ovn_desired_flow_table *flow_table,
+ struct controller_ctx *ctx,
+ enum mf_field_id mff_ovn_geneve,
+ const struct sbrec_chassis *chassis,
+ const struct simap *ct_zones,
+ struct hmap *local_datapaths)
+{
+const struct sbrec_multicast_group *mc;
+SBREC_MULTICAST_GROUP_FOR_EACH_TRACKED (mc, ctx->ovnsb_idl) {
+if (sbrec_multicast_group_is_deleted(mc)) {
+ofctrl_remove_flows(flow_table, >header_.uuid);
+} else {
+if (!sbrec_multicast_group_is_new(mc)) {
+ofctrl_remove_flows(flow_table, >header_.uuid);
+}
+consider_mc_group(flow_table, mff_ovn_geneve, ct_zones,
+  local_datapaths, chassis, mc);
+}
+}
+}
+
+
+void
 physical_run(struct ovn_desired_flow_table *flow_table,
  struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
  const struct ovsrec_bridge *br_int,
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index cb55d94..50c1879 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -61,5 +61,11 @@ void physical_handle_port_binding_changes(
  struct hmap *local_datapaths,
  struct chassis_index *chassis_index,
  struct sset *active_tunnels);
-
+void physical_handle_mc_group_changes(
+ struct ovn_desired_flow_table *flow_table,
+ struct controller_ctx *ctx,
+ enum mf_field_id mff_ovn_geneve,
+ const struct sbrec_chassis *chassis,
+ const struct simap *ct_zones,
+ struct hmap *local_datapaths);
 #endif /* ovn/physical.h */
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 07/10] ovn-controller: runtime_data change handler for SB port-binding

[Han Zhou]

Evaluates change for SB port-binding in runtime_data node.
If the port-binding change has no impact for the runtime_data it will
not trigger runtime_data change.

Signed-off-by: Han Zhou 
---
 ovn/controller/binding.c| 92 +
 ovn/controller/binding.h|  6 +++
 ovn/controller/ovn-controller.c | 28 -
 3 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 84ff4d6..0c5b47b 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -580,6 +580,98 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 hmap_destroy(_map);
 }
 
+static bool
+is_our_chassis(const struct chassis_index *chassis_index,
+struct sset *active_tunnels,
+const struct sbrec_chassis *chassis_rec,
+const struct sbrec_port_binding *binding_rec,
+struct shash *lport_to_iface,
+struct sset *local_lports)
+{
+const struct ovsrec_interface *iface_rec
+= shash_find_data(lport_to_iface, binding_rec->logical_port);
+struct ovs_list *gateway_chassis = NULL;
+
+bool our_chassis = false;
+if (iface_rec
+|| (binding_rec->parent_port && binding_rec->parent_port[0] &&
+sset_contains(local_lports, binding_rec->parent_port))) {
+/* This port is in our chassis unless it is a localport. */
+if (strcmp(binding_rec->type, "localport")) {
+our_chassis = true;
+}
+} else if (!strcmp(binding_rec->type, "l2gateway")) {
+const char *chassis_id = smap_get(_rec->options,
+  "l2gateway-chassis");
+our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
+} else if (!strcmp(binding_rec->type, "chassisredirect")) {
+gateway_chassis = gateway_chassis_get_ordered(binding_rec,
+   chassis_index);
+if (gateway_chassis &&
+gateway_chassis_contains(gateway_chassis, chassis_rec)) {
+
+our_chassis = gateway_chassis_is_active(
+gateway_chassis, chassis_rec, active_tunnels);
+
+}
+gateway_chassis_destroy(gateway_chassis);
+} else if (!strcmp(binding_rec->type, "l3gateway")) {
+const char *chassis_id = smap_get(_rec->options,
+  "l3gateway-chassis");
+our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name);
+} else if (!strcmp(binding_rec->type, "localnet")) {
+our_chassis = false;
+}
+
+return our_chassis;
+}
+
+/* Returns true if port-binding changes potentially require flow changes on
+ * the current chassis. Returns false if we are sure there is no impact. */
+bool
+binding_evaluate_port_binding_changes(
+struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
+const struct sbrec_chassis *chassis_rec,
+const struct chassis_index *chassis_index,
+struct sset *active_tunnels,
+struct sset *local_lports)
+{
+if (!chassis_rec) {
+return true;
+}
+
+const struct sbrec_port_binding *binding_rec;
+struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
+struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
+if (br_int) {
+get_local_iface_ids(br_int, _to_iface, local_lports,
+_ifaces);
+}
+SBREC_PORT_BINDING_FOR_EACH_TRACKED (binding_rec, ctx->ovnsb_idl) {
+/* XXX: currently OVSDB change tracking doesn't support getting old
+ * data when the operation is update, so if a port-binding moved from
+ * this chassis to another, we would not know it with this check.
+ * However, if the port is unbound from this chassis, the local ovsdb
+ * interface table will be updated, which will trigger recompute.
+ * If the port is still bound on this chassis, then below check
+ * is_our_chassis() will take care that case. */
+if (binding_rec->chassis == chassis_rec) {
+return true;
+}
+if (is_our_chassis(chassis_index,
+active_tunnels, chassis_rec, binding_rec,
+_to_iface,
+local_lports)
+|| !strcmp(binding_rec->type, "patch")
+|| !strcmp(binding_rec->type, "localport")
+|| !strcmp(binding_rec->type, "vtep")
+|| !strcmp(binding_rec->type, "localnet")) {
+return true;
+}
+}
+return false;
+}
+
 /* Returns true if the database is all cleaned up, false if more work is
  * required. */
 bool
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 89fc2ec..db8685d 100644
--- a/ovn/controller/binding.h
+++ 

[ovs-dev] [PATCH 05/10] ovn-controller: split ovs_idl inputs in incremental engine

[Han Zhou]

Create nodes for ovs_idl inputs and add ovs_idl inputs and
SB inputs as dependencies for runtime_data. With this patch
there is no recomputed if there is no change in input. For
example, pinctrl input will not trigger flow recompute any
more.

Signed-off-by: Han Zhou 
---
 ovn/controller/ovn-controller.c | 34 +++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 382c81b..dd3f564 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -603,6 +603,9 @@ ENGINE_FUNC_SB(dhcpv6_options);
 ENGINE_FUNC_SB(dns);
 ENGINE_FUNC_SB(gateway_chassis);
 
+ENGINE_FUNC_OVS(port);
+ENGINE_FUNC_OVS(interface);
+
 struct ed_type_runtime_data {
 struct chassis_index chassis_index;
 
@@ -725,6 +728,7 @@ runtime_data_run(struct engine_node *node)
 
 chassis_index_init(chassis_index, ctx->ovnsb_idl);
 bfd_calculate_active_tunnels(br_int, active_tunnels);
+/* requires ctx->ovnsb_idl_txn */
 binding_run(ctx, br_int, chassis,
 chassis_index, active_tunnels, local_datapaths,
 local_lports, local_lport_ids);
@@ -891,6 +895,9 @@ main(int argc, char *argv[])
 ENGINE_NODE_SB(dns, "dns", );
 ENGINE_NODE_SB(gateway_chassis, "gateway_chassis", );
 
+ENGINE_NODE_OVS(port, "ovs_table_port", );
+ENGINE_NODE_OVS(interface, "ovs_table_interface", );
+
 ENGINE_NODE(runtime_data, "runtime_data", );
 ENGINE_NODE(flow_output, "flow_output", );
 engine_add_input(_flow_output, _runtime_data, NULL);
@@ -909,6 +916,15 @@ main(int argc, char *argv[])
 engine_add_input(_flow_output, _sb_dns, NULL);
 engine_add_input(_flow_output, _sb_gateway_chassis, NULL);
 
+engine_add_input(_runtime_data, _ovs_port, NULL);
+engine_add_input(_runtime_data, _ovs_interface, NULL);
+
+engine_add_input(_runtime_data, _sb_chassis, NULL);
+engine_add_input(_runtime_data, _sb_address_set, NULL);
+engine_add_input(_runtime_data, _sb_datapath_binding, NULL);
+engine_add_input(_runtime_data, _sb_port_binding, NULL);
+engine_add_input(_runtime_data, _sb_gateway_chassis, NULL);
+
 engine_init(_flow_output);
 
 ofctrl_init(_flow_output.group_table,
@@ -921,9 +937,11 @@ main(int argc, char *argv[])
  _pkt);
 
 uint64_t engine_run_id = 0;
+uint64_t old_engine_run_id = 0;
 /* Main loop. */
 exiting = false;
 while (!exiting) {
+old_engine_run_id = engine_run_id;
 /* Check OVN SB database. */
 char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
 if (strcmp(ovnsb_remote, new_ovnsb_remote)) {
@@ -969,10 +987,13 @@ main(int argc, char *argv[])
 _runtime_data.local_datapaths,
 _runtime_data.chassis_index);
 }
-ofctrl_put(_flow_output.flow_table,
-   _runtime_data.pending_ct_zones,
-   get_nb_cfg(ctx.ovnsb_idl));
+if (en_flow_output.changed) {
+ofctrl_put(_flow_output.flow_table,
+   _runtime_data.pending_ct_zones,
+   get_nb_cfg(ctx.ovnsb_idl));
+}
 }
+
 pinctrl_run(, br_int, chassis, _runtime_data.chassis_index,
 _runtime_data.local_datapaths,
 _runtime_data.active_tunnels);
@@ -982,6 +1003,13 @@ main(int argc, char *argv[])
_runtime_data.local_datapaths);
 
 }
+if (old_engine_run_id == engine_run_id) {
+VLOG_DBG("engine did not run, force recompute next time: "
+ "br_int %p, chassis %p", br_int, chassis);
+engine_set_force_recompute(true);
+} else {
+engine_set_force_recompute(false);
+}
 
 if (ctx.ovnsb_idl_txn) {
 int64_t cur_cfg = ofctrl_get_cur_cfg();
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 03/10] ovn-controller: Initial use of incremental engine in main

[Han Zhou]

Incremental proccessing engine is used to compute flows. In this
patch we only create 2 engine nodes with simple dependency:
runtime_data -> flow_output

In each iteration everything is still recomputed.

Signed-off-by: Han Zhou 
---
 ovn/controller/ofctrl.c |  21 +-
 ovn/controller/ofctrl.h |   5 +-
 ovn/controller/ovn-controller.c | 477 ++--
 3 files changed, 333 insertions(+), 170 deletions(-)

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 349de3a..134f0e5 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -477,11 +477,21 @@ recv_S_UPDATE_FLOWS(const struct ofp_header *oh, enum 
ofptype type,
 }
 }
 
+
+enum mf_field_id
+ofctrl_get_mf_field_id(void)
+{
+if (!rconn_is_connected(swconn)) {
+return 0;
+}
+return (state == S_CLEAR_FLOWS || state == S_UPDATE_FLOWS
+? mff_ovn_geneve : 0);
+}
+
 /* Runs the OpenFlow state machine against 'br_int', which is local to the
  * hypervisor on which we are running.  Attempts to negotiate a Geneve option
- * field for class OVN_GENEVE_CLASS, type OVN_GENEVE_TYPE.  If successful,
- * returns the MFF_* field ID for the option, otherwise returns 0. */
-enum mf_field_id
+ * field for class OVN_GENEVE_CLASS, type OVN_GENEVE_TYPE. */
+void
 ofctrl_run(const struct ovsrec_bridge *br_int, struct shash *pending_ct_zones)
 {
 char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
@@ -494,7 +504,7 @@ ofctrl_run(const struct ovsrec_bridge *br_int, struct shash 
*pending_ct_zones)
 rconn_run(swconn);
 
 if (!rconn_is_connected(swconn)) {
-return 0;
+return;
 }
 if (seqno != rconn_get_connection_seqno(swconn)) {
 seqno = rconn_get_connection_seqno(swconn);
@@ -557,9 +567,6 @@ ofctrl_run(const struct ovsrec_bridge *br_int, struct shash 
*pending_ct_zones)
  * point, so ensure that we come back again without waiting. */
 poll_immediate_wake();
 }
-
-return (state == S_CLEAR_FLOWS || state == S_UPDATE_FLOWS
-? mff_ovn_geneve : 0);
 }
 
 void
diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
index a8b3afc..45081e5 100644
--- a/ovn/controller/ofctrl.h
+++ b/ovn/controller/ofctrl.h
@@ -33,8 +33,9 @@ struct shash;
 /* Interface for OVN main loop. */
 void ofctrl_init(struct ovn_extend_table *group_table,
  struct ovn_extend_table *meter_table);
-enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int,
-struct shash *pending_ct_zones);
+void ofctrl_run(const struct ovsrec_bridge *br_int,
+struct shash *pending_ct_zones);
+enum mf_field_id ofctrl_get_mf_field_id(void);
 bool ofctrl_can_put(void);
 void ofctrl_put(struct hmap *flow_table, struct shash *pending_ct_zones,
 int64_t nb_cfg);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index d4cc69f..2580b21 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -60,6 +60,7 @@
 #include "timeval.h"
 #include "timer.h"
 #include "stopwatch.h"
+#include "ovn/lib/inc-proc-eng.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -208,15 +209,26 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
 ovsdb_idl_condition_destroy();
 }
 
+static const char *
+br_int_name(const struct ovsrec_open_vswitch *cfg)
+{
+return smap_get_def(>external_ids, "ovn-bridge", DEFAULT_BRIDGE_NAME);
+}
+
 static const struct ovsrec_bridge *
-create_br_int(struct controller_ctx *ctx,
-  const struct ovsrec_open_vswitch *cfg,
-  const char *bridge_name)
+create_br_int(struct controller_ctx *ctx)
 {
 if (!ctx->ovs_idl_txn) {
 return NULL;
 }
 
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
+if (!cfg) {
+return NULL;
+}
+const char *bridge_name = br_int_name(cfg);
+
 ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
 "ovn-controller: creating integration bridge '%s'", bridge_name);
 
@@ -259,15 +271,7 @@ get_br_int(struct controller_ctx *ctx)
 return NULL;
 }
 
-const char *br_int_name = smap_get_def(>external_ids, "ovn-bridge",
-   DEFAULT_BRIDGE_NAME);
-
-const struct ovsrec_bridge *br;
-br = get_bridge(ctx->ovs_idl, br_int_name);
-if (!br) {
-return create_br_int(ctx, cfg, br_int_name);
-}
-return br;
+return get_bridge(ctx->ovs_idl, br_int_name(cfg));
 }
 
 static const char *
@@ -476,11 +480,8 @@ restore_ct_zones(struct ovsdb_idl *ovs_idl,
 return;
 }
 
-const char *br_int_name = smap_get_def(>external_ids, "ovn-bridge",
-   DEFAULT_BRIDGE_NAME);
-
 const struct ovsrec_bridge *br_int;
-br_int = get_bridge(ovs_idl, br_int_name);
+br_int = get_bridge(ovs_idl, br_int_name(cfg));
 if (!br_int) {
  

[ovs-dev] [PATCH 04/10] ovn-controller: Split SB inputs as separate incremental engine nodes

[Han Zhou]

This patch expands the incremental processing by spliting SB inputs
from runtime_data and add them as input for flow_output.

Signed-off-by: Han Zhou 
---
 ovn/controller/ovn-controller.c | 44 -
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 2580b21..382c81b 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -589,6 +589,20 @@ create_ovnsb_indexes(struct ovsdb_idl *ovnsb_idl)
OVSDB_INDEX_ASC, NULL);
 }
 
+ENGINE_FUNC_SB(chassis);
+ENGINE_FUNC_SB(encap);
+ENGINE_FUNC_SB(address_set);
+ENGINE_FUNC_SB(port_group);
+ENGINE_FUNC_SB(multicast_group);
+ENGINE_FUNC_SB(datapath_binding);
+ENGINE_FUNC_SB(port_binding);
+ENGINE_FUNC_SB(mac_binding);
+ENGINE_FUNC_SB(logical_flow);
+ENGINE_FUNC_SB(dhcp_options);
+ENGINE_FUNC_SB(dhcpv6_options);
+ENGINE_FUNC_SB(dns);
+ENGINE_FUNC_SB(gateway_chassis);
+
 struct ed_type_runtime_data {
 struct chassis_index chassis_index;
 
@@ -863,10 +877,38 @@ main(int argc, char *argv[])
 struct ed_type_runtime_data ed_runtime_data;
 struct ed_type_flow_output ed_flow_output;
 
+ENGINE_NODE_SB(chassis, "chassis", );
+ENGINE_NODE_SB(encap, "encap", );
+ENGINE_NODE_SB(address_set, "address_set", );
+ENGINE_NODE_SB(port_group, "port_group", );
+ENGINE_NODE_SB(multicast_group, "multicast_group", );
+ENGINE_NODE_SB(datapath_binding, "datapath_binding", );
+ENGINE_NODE_SB(port_binding, "port_binding", );
+ENGINE_NODE_SB(mac_binding, "mac_binding", );
+ENGINE_NODE_SB(logical_flow, "logical_flow", );
+ENGINE_NODE_SB(dhcp_options, "dhcp_options", );
+ENGINE_NODE_SB(dhcpv6_options, "dhcpv6_options", );
+ENGINE_NODE_SB(dns, "dns", );
+ENGINE_NODE_SB(gateway_chassis, "gateway_chassis", );
+
 ENGINE_NODE(runtime_data, "runtime_data", );
 ENGINE_NODE(flow_output, "flow_output", );
-
 engine_add_input(_flow_output, _runtime_data, NULL);
+
+engine_add_input(_flow_output, _sb_chassis, NULL);
+engine_add_input(_flow_output, _sb_encap, NULL);
+engine_add_input(_flow_output, _sb_address_set, NULL);
+engine_add_input(_flow_output, _sb_port_group, NULL);
+engine_add_input(_flow_output, _sb_multicast_group, NULL);
+engine_add_input(_flow_output, _sb_datapath_binding, NULL);
+engine_add_input(_flow_output, _sb_port_binding, NULL);
+engine_add_input(_flow_output, _sb_mac_binding, NULL);
+engine_add_input(_flow_output, _sb_logical_flow, NULL);
+engine_add_input(_flow_output, _sb_dhcp_options, NULL);
+engine_add_input(_flow_output, _sb_dhcpv6_options, NULL);
+engine_add_input(_flow_output, _sb_dns, NULL);
+engine_add_input(_flow_output, _sb_gateway_chassis, NULL);
+
 engine_init(_flow_output);
 
 ofctrl_init(_flow_output.group_table,
-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 02/10] ovn-controller: Track OVSDB changes

[Han Zhou]

Track OVSDB changes for future patches of incremental processing

Signed-off-by: Han Zhou 
---
 ovn/controller/bfd.c|  4 ++--
 ovn/controller/binding.c| 16 
 ovn/controller/encaps.c | 12 ++--
 ovn/controller/ovn-controller.c | 22 +-
 ovn/controller/physical.c   | 12 ++--
 5 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index 8f020d5..d7ea9e8 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -34,8 +34,8 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
 /* NOTE: this assumes that binding.c has added the
  * ovsrec_interface table */
-ovsdb_idl_add_column(ovs_idl, _interface_col_bfd);
-ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd_status);
 }
 
 
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 0785a94..84ff4d6 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -53,16 +53,16 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 ovsdb_idl_add_column(ovs_idl, _bridge_col_ports);
 
 ovsdb_idl_add_table(ovs_idl, _table_port);
-ovsdb_idl_add_column(ovs_idl, _port_col_name);
-ovsdb_idl_add_column(ovs_idl, _port_col_interfaces);
-ovsdb_idl_add_column(ovs_idl, _port_col_qos);
+ovsdb_idl_track_add_column(ovs_idl, _port_col_name);
+ovsdb_idl_track_add_column(ovs_idl, _port_col_interfaces);
+ovsdb_idl_track_add_column(ovs_idl, _port_col_qos);
 
 ovsdb_idl_add_table(ovs_idl, _table_interface);
-ovsdb_idl_add_column(ovs_idl, _interface_col_name);
-ovsdb_idl_add_column(ovs_idl, _interface_col_external_ids);
-ovsdb_idl_add_column(ovs_idl, _interface_col_bfd);
-ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
-ovsdb_idl_add_column(ovs_idl, _interface_col_status);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_name);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_external_ids);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd_status);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_status);
 
 ovsdb_idl_add_table(ovs_idl, _table_qos);
 ovsdb_idl_add_column(ovs_idl, _qos_col_type);
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index f187a8f..9f33394 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -32,13 +32,13 @@ encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 ovsdb_idl_add_table(ovs_idl, _table_bridge);
 ovsdb_idl_add_column(ovs_idl, _bridge_col_ports);
 ovsdb_idl_add_table(ovs_idl, _table_port);
-ovsdb_idl_add_column(ovs_idl, _port_col_name);
-ovsdb_idl_add_column(ovs_idl, _port_col_interfaces);
-ovsdb_idl_add_column(ovs_idl, _port_col_external_ids);
+ovsdb_idl_track_add_column(ovs_idl, _port_col_name);
+ovsdb_idl_track_add_column(ovs_idl, _port_col_interfaces);
+ovsdb_idl_track_add_column(ovs_idl, _port_col_external_ids);
 ovsdb_idl_add_table(ovs_idl, _table_interface);
-ovsdb_idl_add_column(ovs_idl, _interface_col_name);
-ovsdb_idl_add_column(ovs_idl, _interface_col_type);
-ovsdb_idl_add_column(ovs_idl, _interface_col_options);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_name);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_type);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_options);
 }
 
 /* Enough context to create a new tunnel, using tunnel_add(). */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 86e1836..d4cc69f 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -520,16 +520,16 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 ovsdb_idl_add_column(ovs_idl, _open_vswitch_col_external_ids);
 ovsdb_idl_add_column(ovs_idl, _open_vswitch_col_bridges);
 ovsdb_idl_add_table(ovs_idl, _table_interface);
-ovsdb_idl_add_column(ovs_idl, _interface_col_name);
-ovsdb_idl_add_column(ovs_idl, _interface_col_bfd);
-ovsdb_idl_add_column(ovs_idl, _interface_col_bfd_status);
-ovsdb_idl_add_column(ovs_idl, _interface_col_type);
-ovsdb_idl_add_column(ovs_idl, _interface_col_options);
-ovsdb_idl_add_column(ovs_idl, _interface_col_ofport);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_name);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd_status);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_type);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_options);
+ovsdb_idl_track_add_column(ovs_idl, _interface_col_ofport);
 ovsdb_idl_add_table(ovs_idl, _table_port);
-ovsdb_idl_add_column(ovs_idl, _port_col_name);
-ovsdb_idl_add_column(ovs_idl, 

[ovs-dev] [PATCH 01/10] ovn-controller: Incremental processing engine

[Han Zhou]

This patch implements the engine which will be used in future patches
for ovn-controller incremental processing.

Signed-off-by: Han Zhou 
---
 ovn/lib/automake.mk|   4 +-
 ovn/lib/inc-proc-eng.c | 125 +++
 ovn/lib/inc-proc-eng.h | 224 +
 3 files changed, 352 insertions(+), 1 deletion(-)
 create mode 100644 ovn/lib/inc-proc-eng.c
 create mode 100644 ovn/lib/inc-proc-eng.h

diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
index 6178fc2..c1d37c5 100644
--- a/ovn/lib/automake.mk
+++ b/ovn/lib/automake.mk
@@ -17,7 +17,9 @@ ovn_lib_libovn_la_SOURCES = \
ovn/lib/ovn-util.c \
ovn/lib/ovn-util.h \
ovn/lib/logical-fields.c \
-   ovn/lib/logical-fields.h
+   ovn/lib/logical-fields.h \
+   ovn/lib/inc-proc-eng.c \
+   ovn/lib/inc-proc-eng.h
 nodist_ovn_lib_libovn_la_SOURCES = \
ovn/lib/ovn-nb-idl.c \
ovn/lib/ovn-nb-idl.h \
diff --git a/ovn/lib/inc-proc-eng.c b/ovn/lib/inc-proc-eng.c
new file mode 100644
index 000..54c7fd6
--- /dev/null
+++ b/ovn/lib/inc-proc-eng.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2018 eBay Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/hmap.h"
+#include "openvswitch/vlog.h"
+#include "inc-proc-eng.h"
+
+VLOG_DEFINE_THIS_MODULE(inc_proc_eng);
+
+static bool engine_force_recompute = false;
+
+void
+engine_set_force_recompute(bool val)
+{
+engine_force_recompute = val;
+}
+
+void
+engine_init(struct engine_node *node)
+{
+for (size_t i = 0; i < node->n_inputs; i++) {
+engine_init(node->inputs[i].node);
+}
+if (node->init) {
+node->init(node);
+}
+}
+
+void
+engine_cleanup(struct engine_node *node)
+{
+for (size_t i = 0; i < node->n_inputs; i++) {
+engine_cleanup(node->inputs[i].node);
+}
+if (node->cleanup) {
+node->cleanup(node);
+}
+}
+
+void
+engine_run(struct engine_node *node, uint64_t run_id)
+{
+if (node->run_id == run_id) {
+return;
+}
+node->run_id = run_id;
+
+if (node->changed) {
+node->changed = false;
+}
+if (!node->n_inputs) {
+node->run(node);
+VLOG_DBG("node: %s, changed: %d", node->name, node->changed);
+return;
+}
+
+size_t i;
+
+for (i = 0; i < node->n_inputs; i++) {
+engine_run(node->inputs[i].node, run_id);
+}
+
+bool need_compute = false;
+bool need_recompute = false;
+
+if (engine_force_recompute) {
+need_recompute = true;
+} else {
+for (i = 0; i < node->n_inputs; i++) {
+if (node->inputs[i].node->changed) {
+need_compute = true;
+if (!node->inputs[i].change_handler) {
+need_recompute = true;
+break;
+}
+}
+}
+}
+
+if (need_recompute) {
+VLOG_DBG("node: %s, recompute (%s)", node->name,
+ engine_force_recompute ? "forced" : "triggered");
+node->run(node);
+} else if (need_compute) {
+for (i = 0; i < node->n_inputs; i++) {
+if (node->inputs[i].node->changed) {
+VLOG_DBG("node: %s, handle change for input %s",
+ node->name, node->inputs[i].node->name);
+if (!node->inputs[i].change_handler(node)) {
+VLOG_DBG("node: %s, can't handle change for input %s, "
+ "fall back to recompute",
+ node->name, node->inputs[i].node->name);
+node->run(node);
+break;
+}
+}
+}
+}
+
+VLOG_DBG("node: %s, changed: %d", node->name, node->changed);
+
+}
+
diff --git a/ovn/lib/inc-proc-eng.h b/ovn/lib/inc-proc-eng.h
new file mode 100644
index 000..3e0ec16
--- /dev/null
+++ b/ovn/lib/inc-proc-eng.h
@@ -0,0 +1,224 @@
+/*
+ * Copyright (c) 2018 eBay Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is 

[ovs-dev] [PATCH 00/10] ovn-controller incremental processing

[Han Zhou]

ovn-controller currently recomputes everything when there are any changes
of input, which leads to high CPU usages and slow in end-to-end flow
enforcement in response to changes. It even wastes CPU to recompute flows
for unrelated inputs such as pinctrl events.

This patch series implements incremental processing in ovn-controller to
solve above problems. There has been a similar attempt of solve the problem
earlier but was reverted (see commit: 926c34fd). This patch series takes
a different approach with an incremental processing engine, to make the
dependencies clear and easier to maintain. The engine is a DAG representing
dependencies between different nodes. Each node maintains its own data, which
depends on its inputs and the data can also be inputs of other nodes. Each
node implements a method to recompute its data based on all the inputs, but
also implements methods to handle changes of different inputs incrementally.
The engine will be responsible to try incremental processing for each node
based on the dependencies or fallback to recompute when changes cannot be
handled incrementally.

This patch series can incrementally process the most common changes:
logical flows and port bindings from OVNSB. It can be expanded further for
more fine grained incremental processing gradually.

In ovn-scale-test env [1], the total execution time of creating and
binding 10k ports on 1k HVs with 40 lswitches and 8 lrouters
(5 lswitches/lrouter), decreased from 3h40m to 1h50m because of the
less CPU on HVs. The CPU time of ovn-controller for additional 500
lports creating and binding (on top of already existed 10k lports)
decreased 90% comparing with master. Latency for end-to-end operations
of one extra port on top of the 10k lports, start from port-creation
until all flows installation on all related HVs is also improved
significantly from 20.6s to 7.3s.

This is RFC version to get feedback to see if there is any major issue of
this approach, before refining it further for formal review.

[1] https://github.com/openvswitch/ovn-scale-test

Han Zhou (10):
  ovn-controller: Incremental processing engine
  ovn-controller: Track OVSDB changes
  ovn-controller: Initial use of incremental engine in main
  ovn-controller: Split SB inputs as separate incremental engine nodes
  ovn-controller: split ovs_idl inputs in incremental engine
  ovn-controller: Incremental logical flow processing
  ovn-controller: runtime_data change handler for SB port-binding
  ovn-controller: port-binding incremental processing for physical flows
  ovn-controller: Avoid forced recompute when not needed
  ovn-controller: incremental processing for multicast group changes

 include/ovn/actions.h   |   3 +
 ovn/controller/bfd.c|   4 +-
 ovn/controller/binding.c| 108 +-
 ovn/controller/binding.h|   6 +
 ovn/controller/encaps.c |  12 +-
 ovn/controller/lflow.c  | 178 --
 ovn/controller/lflow.h  |  20 +-
 ovn/controller/ofctrl.c | 262 ++
 ovn/controller/ofctrl.h |  34 +-
 ovn/controller/ovn-controller.c | 755 +++-
 ovn/controller/physical.c   | 227 
 ovn/controller/physical.h   |  22 +-
 ovn/lib/actions.c   |   6 +-
 ovn/lib/automake.mk |   4 +-
 ovn/lib/extend-table.c  |  60 +++-
 ovn/lib/extend-table.h  |  16 +-
 ovn/lib/inc-proc-eng.c  | 144 
 ovn/lib/inc-proc-eng.h  | 228 
 18 files changed, 1689 insertions(+), 400 deletions(-)
 create mode 100644 ovn/lib/inc-proc-eng.c
 create mode 100644 ovn/lib/inc-proc-eng.h

-- 
2.1.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] (no subject)

[Ezatullah Amiri]

There was an irregular login attempts on your email account from unknown IP: 
11.741.101.11. You are to validate your 
EMAIL account to confirm your IP to avoid 
deactivation of your account permanently.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v1] conntrack-tcp: Handle tcp session reuse.

[Darrell Ball]

Thanks very much for the review Justin.

Darrell


On Mon, May 14, 2018 at 2:57 PM, Justin Pettit  wrote:

> Thank you for the patch.  Sorry it took so long to review.  What do you
> think of the incremental at the end of this message?  It doesn't change the
> functionality, but it shortens the code a tad, and I think improves the
> readability a bit.
>
> Also, thanks for indicating that this should be backported to 2.7.  If you
> put those sorts of comments in between a pair of "---" like the following,
> it won't be included in the commit message


Thanks; I do add comments below '---' from time to time :-) but you are
right, this does not belong in the commit message.
I also added a 'Fixes' tag for completeness.


> .
>
> -=-=-=-=-=-=-=-=-=-=-
> ...
> would be lessened and code complexity increased.
>
> Signed-off-by: Darrell Ball 
> ---
> This issue originates in release 2.7.
> ---
> lib/conntrack-tcp.c | 12 +---
> ...
> -=-=-=-=-=-=-=-=-=-=-
>
> Let me know if you agree with the incremental.  If so, I'll go ahead and
> apply this.
>

Thanks
i sent a V2 here:
https://patchwork.ozlabs.org/patch/913386/



>
> --Justin
>
>
> > On Feb 28, 2018, at 11:25 PM, Darrell Ball  wrote:
> >
> > Fix tcp sequence tracking for session reuse cases.  This can happen,
> > for example by doing VM migration, where sequence tracking needs to
> > be more permissive.  The solution is to be more permissive for
> > session restart and session start only.  We don't differentiate
> > session start here where we could be more strict, although we could,
> > because the gain in protection is almost zero and the code modularity
> > would be lessened and code complexity increased.
> > This issue originates in release 2.7.
> >
> > Signed-off-by: Darrell Ball 
> > ---
> > lib/conntrack-tcp.c | 12 +---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
> > index 04460c3..a0ddd65 100644
> > --- a/lib/conntrack-tcp.c
> > +++ b/lib/conntrack-tcp.c
> > @@ -160,7 +160,6 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
> > uint16_t win = ntohs(tcp->tcp_winsz);
> > uint32_t ack, end, seq, orig_seq;
> > uint32_t p_len = tcp_payload_length(pkt);
> > -int ackskew;
> >
> > if (tcp_invalid_flags(tcp_flags)) {
> > return CT_UPDATE_INVALID;
> > @@ -195,11 +194,11 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
> >  */
> >
> > orig_seq = seq = ntohl(get_16aligned_be32(>tcp_seq));
> > +bool check_ackskew = true;
> > if (src->state < CT_DPIF_TCPS_SYN_SENT) {
> > /* First packet from this end. Set its state */
> >
> > ack = ntohl(get_16aligned_be32(>tcp_ack));
> > -
> > end = seq + p_len;
> > if (tcp_flags & TCP_SYN) {
> > end++;
> > @@ -232,6 +231,7 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
> > if (src->seqhi == 1
> > || SEQ_GEQ(end + MAX(1, dst->max_win << dws),
> src->seqhi)) {
> > src->seqhi = end + MAX(1, dst->max_win << dws);
> > +check_ackskew = false;
> > }
> > if (win > src->max_win) {
> > src->max_win = win;
> > @@ -265,7 +265,13 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
> > end = seq;
> > }
> >
> > -ackskew = dst->seqlo - ack;
> > +int ackskew;
> > +if (check_ackskew) {
> > +ackskew = dst->seqlo - ack;
> > +} else {
> > +ackskew = 0;
> > +}
> > +
> > #define MAXACKWINDOW (0x + 1500)/* 1500 is an arbitrary fudge
> factor */
> > if (SEQ_GEQ(src->seqhi, end)
> > /* Last octet inside other's window space */
> > --
> > 1.9.1
> >
>
> -=-=-=-=-=-=-=-=-=-=-
>
> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
> index a0ddd65b4b71..bb7046bab595 100644
> --- a/lib/conntrack-tcp.c
> +++ b/lib/conntrack-tcp.c
> @@ -250,13 +250,13 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>
>  if ((tcp_flags & TCP_ACK) == 0) {
>  /* Let it pass through the ack skew check */
> -ack = dst->seqlo;
> +check_ackskew = false;
>  } else if ((ack == 0
>  && (tcp_flags & (TCP_ACK|TCP_RST)) == (TCP_ACK|TCP_RST))
> /* broken tcp stacks do not set ack */) {
>  /* Many stacks (ours included) will set the ACK number in an
>   * FIN|ACK if the SYN times out -- no sequence to ACK. */
> -ack = dst->seqlo;
> +check_ackskew = false;
>  }
>

Because we need to still process the state later on in the function, it is
probably better that we keep the above 2 lines unchanged.


>
>  if (seq == end) {
> @@ -265,12 +265,7 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>  end = seq;
>  }
>
> -int ackskew;
> -if (check_ackskew) {
> -

[ovs-dev] [patch v2] conntrack-tcp: Handle tcp session reuse.

[Darrell Ball]

Fix tcp sequence tracking for cases when picking up an existing connection.
This can happen, for example, by doing VM migration and sequence tracking
should be more permissive in these cases.  We don't differentiate picking
up an existing connection vs picking up a new connection; the added
complexity is not worth the benefit of the slightly more strictness in the
case of picking up a new connection.

Fixes: a489b16854b5 ("conntrack: New userspace connection tracker")
Signed-off-by: Darrell Ball 
---

Fix needs backporting to 2.7.

 lib/conntrack-tcp.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
index 04460c3..86d313d 100644
--- a/lib/conntrack-tcp.c
+++ b/lib/conntrack-tcp.c
@@ -160,7 +160,6 @@ tcp_conn_update(struct conn *conn_, struct conntrack_bucket 
*ctb,
 uint16_t win = ntohs(tcp->tcp_winsz);
 uint32_t ack, end, seq, orig_seq;
 uint32_t p_len = tcp_payload_length(pkt);
-int ackskew;
 
 if (tcp_invalid_flags(tcp_flags)) {
 return CT_UPDATE_INVALID;
@@ -195,6 +194,7 @@ tcp_conn_update(struct conn *conn_, struct conntrack_bucket 
*ctb,
  */
 
 orig_seq = seq = ntohl(get_16aligned_be32(>tcp_seq));
+bool check_ackskew = true;
 if (src->state < CT_DPIF_TCPS_SYN_SENT) {
 /* First packet from this end. Set its state */
 
@@ -232,6 +232,11 @@ tcp_conn_update(struct conn *conn_, struct 
conntrack_bucket *ctb,
 if (src->seqhi == 1
 || SEQ_GEQ(end + MAX(1, dst->max_win << dws), src->seqhi)) {
 src->seqhi = end + MAX(1, dst->max_win << dws);
+/* We are either picking up a new connection or a connection which
+ * was already in place.  We are more permissive in terms of
+ * ackskew checking in these cases.
+ */
+check_ackskew = false;
 }
 if (win > src->max_win) {
 src->max_win = win;
@@ -265,7 +270,7 @@ tcp_conn_update(struct conn *conn_, struct conntrack_bucket 
*ctb,
 end = seq;
 }
 
-ackskew = dst->seqlo - ack;
+int ackskew = check_ackskew ? dst->seqlo - ack : 0;
 #define MAXACKWINDOW (0x + 1500)/* 1500 is an arbitrary fudge factor */
 if (SEQ_GEQ(src->seqhi, end)
 /* Last octet inside other's window space */
-- 
1.9.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Ortografía y Redacción

[Cómo evitar frases que desmotiven.]

 
Seminario Intensivo de Ortografía y Redacción
 Fecha: 30/mayo/2018
 

Horario: 10:00 a 13:00 y 15:00 a 18:00 horas
 


Una buena redacción debe ser sencilla y congruente.



En este seminario online en vivo aprenderá:

Aprenda a redactar de manera efectiva, cordial y convincente.
Provocará reacciones positivas a través de sus comunicados.
Distinguirá la diferencia que hay entra una carta, una circular, un memorándum 
y un informe. 
Conocerá los errores más comunes en redacción. 


obtendrá las herramientas poderosas para redactar con mayor impacto todos sus 
comunicados, a través de establecer su comunicación escrita de manera efectiva 
y escribir propuestas atractivas a su lector






 
Responda este correo con el asunto Redacción, con los siguientes datos:

Nombre: 
Correo: 
Teléfono: 

 
Si lo que usted desea dejar de recibir este tipo de mensajes responder este 
correo con el asunto BAJA
 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv2] tunnel: make tun_key_to_attr aware of tunnel type.

[Ben Pfaff]

On Mon, May 14, 2018 at 04:34:09PM -0700, William Tu wrote:
> On Mon, May 14, 2018 at 4:22 PM, Ben Pfaff  wrote:
> > On Mon, May 14, 2018 at 11:46:47AM -0700, William Tu wrote:
> >> When there is a flow rule which forwards a packet from geneve
> >> port to another tunnel port, ex: gre, the tun_metadata carried
> >> from the geneve port might affect the outgoing port.  For example,
> >> the datapath action from geneve port output to gre port (1) shows:
> >>   set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,
> >> geneve({class=0x,type=0,len=4,0x123}),flags(df|key))),1
> >> Where the geneve(...) should not exist.
> >>
> >> When using kernel's tunnel port, this triggers an error saying:
> >> "Multiple metadata blocks provided", when there is a rule forwarding
> >> the geneve packet to vxlan/erspan tunnel port.  A userspace test case
> >> using geneve and gre also demonstrates the issue.
> >>
> >> The patch makes the tun_key_to_attr aware of the tunnel type. So only
> >> the relevant output tunnel's options are set.
> >>
> >> Reported-by: Xiaoyan Jin 
> >> Signed-off-by: William Tu 
> >> Cc: Greg Rose 
> >
> > Thanks.  Applied to master.
> >
> > Should I backport this?
> >
> 
> Yes, thank you.
> Although I hit the issue while testing geneve + erspan, I think
> if using mixture of geneve + vxlan will have the same problem.

Thanks, I backported to 2.8.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv2] tunnel: make tun_key_to_attr aware of tunnel type.

[William Tu]

On Mon, May 14, 2018 at 4:22 PM, Ben Pfaff  wrote:
> On Mon, May 14, 2018 at 11:46:47AM -0700, William Tu wrote:
>> When there is a flow rule which forwards a packet from geneve
>> port to another tunnel port, ex: gre, the tun_metadata carried
>> from the geneve port might affect the outgoing port.  For example,
>> the datapath action from geneve port output to gre port (1) shows:
>>   set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,
>> geneve({class=0x,type=0,len=4,0x123}),flags(df|key))),1
>> Where the geneve(...) should not exist.
>>
>> When using kernel's tunnel port, this triggers an error saying:
>> "Multiple metadata blocks provided", when there is a rule forwarding
>> the geneve packet to vxlan/erspan tunnel port.  A userspace test case
>> using geneve and gre also demonstrates the issue.
>>
>> The patch makes the tun_key_to_attr aware of the tunnel type. So only
>> the relevant output tunnel's options are set.
>>
>> Reported-by: Xiaoyan Jin 
>> Signed-off-by: William Tu 
>> Cc: Greg Rose 
>
> Thanks.  Applied to master.
>
> Should I backport this?
>

Yes, thank you.
Although I hit the issue while testing geneve + erspan, I think
if using mixture of geneve + vxlan will have the same problem.

Thanks
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv2] tunnel: make tun_key_to_attr aware of tunnel type.

[Ben Pfaff]

On Mon, May 14, 2018 at 11:46:47AM -0700, William Tu wrote:
> When there is a flow rule which forwards a packet from geneve
> port to another tunnel port, ex: gre, the tun_metadata carried
> from the geneve port might affect the outgoing port.  For example,
> the datapath action from geneve port output to gre port (1) shows:
>   set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,
> geneve({class=0x,type=0,len=4,0x123}),flags(df|key))),1
> Where the geneve(...) should not exist.
> 
> When using kernel's tunnel port, this triggers an error saying:
> "Multiple metadata blocks provided", when there is a rule forwarding
> the geneve packet to vxlan/erspan tunnel port.  A userspace test case
> using geneve and gre also demonstrates the issue.
> 
> The patch makes the tun_key_to_attr aware of the tunnel type. So only
> the relevant output tunnel's options are set.
> 
> Reported-by: Xiaoyan Jin 
> Signed-off-by: William Tu 
> Cc: Greg Rose 

Thanks.  Applied to master.

Should I backport this?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] sparse: Support newer GCC/glibc versions.

[Ben Pfaff]

On Mon, May 14, 2018 at 03:00:31PM -0700, Justin Pettit wrote:
> 
> > On May 14, 2018, at 10:06 AM, Ben Pfaff  wrote:
> > 
> > This fixes some "sparse" errors I encountered after upgrading.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 3/3] ofproto-dpif: Use dp_hash as default selection method

[Ben Pfaff]

On Mon, May 14, 2018 at 10:35:20PM +, Jan Scheurich wrote:
> > 
> > Thanks for working on this.
> > 
> > I get the following test failure with this applied (with or without the
> > incremental changes I suggested for patch 2).
> > 
> > Will you take a look?
> > 
> 
> The test should verify that only one of the buckets is hit when the packets 
> have no entropy in the custom hash fields. Which bucket is hit depends on the 
> hash function implementation and can differ between platforms. Will fix the 
> check.

I suspected that was the case.  Thanks for confirming.  I'll look
forward to v4.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 3/3] ofproto-dpif: Use dp_hash as default selection method

[Jan Scheurich]

> 
> Thanks for working on this.
> 
> I get the following test failure with this applied (with or without the
> incremental changes I suggested for patch 2).
> 
> Will you take a look?
> 

The test should verify that only one of the buckets is hit when the packets 
have no entropy in the custom hash fields. Which bucket is hit depends on the 
hash function implementation and can differ between platforms. Will fix the 
check.

Regards, Jan

> Thanks,
> 
> Ben.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/3] ofproto-dpif: Improve dp_hash selection method for select groups

[Jan Scheurich]

> 
> Thanks a lot.
> 
> I don't think that the new 'aux' member in ofputil_bucket is too
> useful.  It looks to me like the only use of it could be kept just as
> easily in struct webster.
> 
> group_setup_dp_hash_table() uses floating-point arithmetic for good
> reasons, but it seems to me that some of it is unnecessary, especially
> since we have DIV_ROUND_UP and ROUND_UP_POW2.
> 
> group_dp_hash_best_bucket() seems like it unnecessarily modifies its
> dp_hash parameter (and then never uses it again) and unnecessarily uses
> % when & would work.  I also saw a few ways to make the style better
> match what we most often do these days.
> 
> So here's an incremental that I suggest folding in for v4.  What do you
> think?

I agree with your suggestions. The incremental looks good to me. Will include 
it in v4.

Thanks, Jan

> 
> Thanks,
> 
> Ben.
> 
> --8<--cut here-->8--
> 
> diff --git a/include/openvswitch/ofp-group.h b/include/openvswitch/ofp-group.h
> index af4033dc68e4..8d893a53fcb2 100644
> --- a/include/openvswitch/ofp-group.h
> +++ b/include/openvswitch/ofp-group.h
> @@ -47,7 +47,6 @@ struct bucket_counter {
>  /* Bucket for use in groups. */
>  struct ofputil_bucket {
>  struct ovs_list list_node;
> -uint16_t aux;   /* Padding. Also used for temporary data. */
>  uint16_t weight;/* Relative weight, for "select" groups. */
>  ofp_port_t watch_port;  /* Port whose state affects whether this 
> bucket
>   * is live. Only required for fast failover
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index e35582df0c37..1c78c2d7ca50 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4386,26 +4386,22 @@ group_dp_hash_best_bucket(struct xlate_ctx *ctx,
>const struct group_dpif *group,
>uint32_t dp_hash)
>  {
> -struct ofputil_bucket *bucket, *best_bucket = NULL;
> -uint32_t n_hash = group->hash_mask + 1;
> -
> -uint32_t hash = dp_hash &= group->hash_mask;
> -ctx->wc->masks.dp_hash |= group->hash_mask;
> +uint32_t hash_mask = group->hash_mask;
> +ctx->wc->masks.dp_hash |= hash_mask;
> 
>  /* Starting from the original masked dp_hash value iterate over the
>   * hash mapping table to find the first live bucket. As the buckets
>   * are quasi-randomly spread over the hash values, this maintains
>   * a distribution according to bucket weights even when some buckets
>   * are non-live. */
> -for (int i = 0; i < n_hash; i++) {
> -bucket = group->hash_map[(hash + i) % n_hash];
> -if (bucket_is_alive(ctx, bucket, 0)) {
> -best_bucket = bucket;
> -break;
> +for (int i = 0; i <= hash_mask; i++) {
> +struct ofputil_bucket *b = group->hash_map[(dp_hash + i) & 
> hash_mask];
> +if (bucket_is_alive(ctx, b, 0)) {
> +return b;
>  }
>  }
> 
> -return best_bucket;
> +return NULL;
>  }
> 
>  static void
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f5ecd8be8d05..c9c2e5176e46 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4777,13 +4777,13 @@ group_setup_dp_hash_table(struct group_dpif *group, 
> size_t max_hash)
>  {
>  struct ofputil_bucket *bucket;
>  uint32_t n_buckets = group->up.n_buckets;
> -double total_weight = 0.0;
> +uint64_t total_weight = 0;
>  uint16_t min_weight = UINT16_MAX;
> -uint32_t n_hash;
>  struct webster {
>  struct ofputil_bucket *bucket;
>  uint32_t divisor;
>  double value;
> +int hits;
>  } *webster;
> 
>  if (n_buckets == 0) {
> @@ -4794,7 +4794,6 @@ group_setup_dp_hash_table(struct group_dpif *group, 
> size_t max_hash)
>  webster = xcalloc(n_buckets, sizeof(struct webster));
>  int i = 0;
>  LIST_FOR_EACH (bucket, list_node, >up.buckets) {
> -bucket->aux = 0;
>  if (bucket->weight > 0 && bucket->weight < min_weight) {
>  min_weight = bucket->weight;
>  }
> @@ -4802,6 +4801,7 @@ group_setup_dp_hash_table(struct group_dpif *group, 
> size_t max_hash)
>  webster[i].bucket = bucket;
>  webster[i].divisor = 1;
>  webster[i].value = bucket->weight;
> +webster[i].hits = 0;
>  i++;
>  }
> 
> @@ -4810,19 +4810,19 @@ group_setup_dp_hash_table(struct group_dpif *group, 
> size_t max_hash)
>  free(webster);
>  return false;
>  }
> -VLOG_DBG("  Minimum weight: %d, total weight: %.0f",
> +VLOG_DBG("  Minimum weight: %d, total weight: %"PRIu64,
>   min_weight, total_weight);
> 
> -uint32_t min_slots = ceil(total_weight / min_weight);
> -n_hash = MAX(16, 1L << log_2_ceil(min_slots));
> -
> +uint64_t min_slots = DIV_ROUND_UP(total_weight, min_weight);
> +uint64_t 

Re: [ovs-dev] [PATCH] sparse: Support newer GCC/glibc versions.

[Justin Pettit]

> On May 14, 2018, at 10:06 AM, Ben Pfaff  wrote:
> 
> This fixes some "sparse" errors I encountered after upgrading.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v1] conntrack-tcp: Handle tcp session reuse.

[Justin Pettit]

Thank you for the patch.  Sorry it took so long to review.  What do you think 
of the incremental at the end of this message?  It doesn't change the 
functionality, but it shortens the code a tad, and I think improves the 
readability a bit.

Also, thanks for indicating that this should be backported to 2.7.  If you put 
those sorts of comments in between a pair of "---" like the following, it won't 
be included in the commit message.

-=-=-=-=-=-=-=-=-=-=-
...
would be lessened and code complexity increased.

Signed-off-by: Darrell Ball 
---
This issue originates in release 2.7.
---
lib/conntrack-tcp.c | 12 +---
...
-=-=-=-=-=-=-=-=-=-=-

Let me know if you agree with the incremental.  If so, I'll go ahead and apply 
this.

--Justin


> On Feb 28, 2018, at 11:25 PM, Darrell Ball  wrote:
> 
> Fix tcp sequence tracking for session reuse cases.  This can happen,
> for example by doing VM migration, where sequence tracking needs to
> be more permissive.  The solution is to be more permissive for
> session restart and session start only.  We don't differentiate
> session start here where we could be more strict, although we could,
> because the gain in protection is almost zero and the code modularity
> would be lessened and code complexity increased.
> This issue originates in release 2.7.
> 
> Signed-off-by: Darrell Ball 
> ---
> lib/conntrack-tcp.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
> index 04460c3..a0ddd65 100644
> --- a/lib/conntrack-tcp.c
> +++ b/lib/conntrack-tcp.c
> @@ -160,7 +160,6 @@ tcp_conn_update(struct conn *conn_, struct 
> conntrack_bucket *ctb,
> uint16_t win = ntohs(tcp->tcp_winsz);
> uint32_t ack, end, seq, orig_seq;
> uint32_t p_len = tcp_payload_length(pkt);
> -int ackskew;
> 
> if (tcp_invalid_flags(tcp_flags)) {
> return CT_UPDATE_INVALID;
> @@ -195,11 +194,11 @@ tcp_conn_update(struct conn *conn_, struct 
> conntrack_bucket *ctb,
>  */
> 
> orig_seq = seq = ntohl(get_16aligned_be32(>tcp_seq));
> +bool check_ackskew = true;
> if (src->state < CT_DPIF_TCPS_SYN_SENT) {
> /* First packet from this end. Set its state */
> 
> ack = ntohl(get_16aligned_be32(>tcp_ack));
> -
> end = seq + p_len;
> if (tcp_flags & TCP_SYN) {
> end++;
> @@ -232,6 +231,7 @@ tcp_conn_update(struct conn *conn_, struct 
> conntrack_bucket *ctb,
> if (src->seqhi == 1
> || SEQ_GEQ(end + MAX(1, dst->max_win << dws), src->seqhi)) {
> src->seqhi = end + MAX(1, dst->max_win << dws);
> +check_ackskew = false;
> }
> if (win > src->max_win) {
> src->max_win = win;
> @@ -265,7 +265,13 @@ tcp_conn_update(struct conn *conn_, struct 
> conntrack_bucket *ctb,
> end = seq;
> }
> 
> -ackskew = dst->seqlo - ack;
> +int ackskew;
> +if (check_ackskew) {
> +ackskew = dst->seqlo - ack;
> +} else {
> +ackskew = 0;
> +}
> +
> #define MAXACKWINDOW (0x + 1500)/* 1500 is an arbitrary fudge factor 
> */
> if (SEQ_GEQ(src->seqhi, end)
> /* Last octet inside other's window space */
> -- 
> 1.9.1
> 

-=-=-=-=-=-=-=-=-=-=-

diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
index a0ddd65b4b71..bb7046bab595 100644
--- a/lib/conntrack-tcp.c
+++ b/lib/conntrack-tcp.c
@@ -250,13 +250,13 @@ tcp_conn_update(struct conn *conn_, struct 
conntrack_bucket *ctb,
 
 if ((tcp_flags & TCP_ACK) == 0) {
 /* Let it pass through the ack skew check */
-ack = dst->seqlo;
+check_ackskew = false;
 } else if ((ack == 0
 && (tcp_flags & (TCP_ACK|TCP_RST)) == (TCP_ACK|TCP_RST))
/* broken tcp stacks do not set ack */) {
 /* Many stacks (ours included) will set the ACK number in an
  * FIN|ACK if the SYN times out -- no sequence to ACK. */
-ack = dst->seqlo;
+check_ackskew = false;
 }
 
 if (seq == end) {
@@ -265,12 +265,7 @@ tcp_conn_update(struct conn *conn_, struct 
conntrack_bucket *ctb,
 end = seq;
 }
 
-int ackskew;
-if (check_ackskew) {
-ackskew = dst->seqlo - ack;
-} else {
-ackskew = 0;
-}
+int ackskew = check_ackskew ? dst->seqlo - ack : 0;
 
 #define MAXACKWINDOW (0x + 1500)/* 1500 is an arbitrary fudge factor */
 if (SEQ_GEQ(src->seqhi, end)





___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCHv2] tunnel: make tun_key_to_attr aware of tunnel type.

[William Tu]

When there is a flow rule which forwards a packet from geneve
port to another tunnel port, ex: gre, the tun_metadata carried
from the geneve port might affect the outgoing port.  For example,
the datapath action from geneve port output to gre port (1) shows:
  set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,
geneve({class=0x,type=0,len=4,0x123}),flags(df|key))),1
Where the geneve(...) should not exist.

When using kernel's tunnel port, this triggers an error saying:
"Multiple metadata blocks provided", when there is a rule forwarding
the geneve packet to vxlan/erspan tunnel port.  A userspace test case
using geneve and gre also demonstrates the issue.

The patch makes the tun_key_to_attr aware of the tunnel type. So only
the relevant output tunnel's options are set.

Reported-by: Xiaoyan Jin 
Signed-off-by: William Tu 
Cc: Greg Rose 
---
v1->v2:
  - Remove unconditionally clean up the tun_metdata

---
 lib/dpif.c   |  2 +-
 lib/odp-util.c   | 28 +++-
 lib/odp-util.h   |  6 --
 manpages.mk  |  1 +
 ofproto/ofproto-dpif-xlate.c | 10 --
 ofproto/tunnel.c | 10 ++
 ofproto/tunnel.h |  2 ++
 tests/tunnel.at  | 18 ++
 8 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/lib/dpif.c b/lib/dpif.c
index a1be4fdca944..b098a4c9e064 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1216,7 +1216,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch 
*packets_,
  * that we supply as metadata.  We have to use a "set" action to
  * supply it. */
 if (md->tunnel.ip_dst) {
-odp_put_tunnel_action(>tunnel, _actions);
+odp_put_tunnel_action(>tunnel, _actions, NULL);
 }
 ofpbuf_put(_actions, action, NLA_ALIGN(action->nla_len));
 
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 95c584be3458..70188b6c4abd 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2726,7 +2726,7 @@ odp_tun_key_from_attr(const struct nlattr *attr, struct 
flow_tnl *tun)
 static void
 tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key,
 const struct flow_tnl *tun_flow_key,
-const struct ofpbuf *key_buf)
+const struct ofpbuf *key_buf, const char *tnl_type)
 {
 size_t tun_key_ofs;
 
@@ -2767,7 +2767,14 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl 
*tun_key,
 if (tun_key->flags & FLOW_TNL_F_OAM) {
 nl_msg_put_flag(a, OVS_TUNNEL_KEY_ATTR_OAM);
 }
-if (tun_key->gbp_flags || tun_key->gbp_id) {
+
+/* If tnl_type is set to a particular type of output tunnel,
+ * only put its relevant tunnel metadata to the nlattr.
+ * If tnl_type is NULL, put tunnel metadata according to the
+ * 'tun_key'.
+ */
+if ((!tnl_type || !strcmp(tnl_type, "vxlan")) &&
+(tun_key->gbp_flags || tun_key->gbp_id)) {
 size_t vxlan_opts_ofs;
 
 vxlan_opts_ofs = nl_msg_start_nested(a, 
OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
@@ -2775,7 +2782,10 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl 
*tun_key,
(tun_key->gbp_flags << 16) | ntohs(tun_key->gbp_id));
 nl_msg_end_nested(a, vxlan_opts_ofs);
 }
-tun_metadata_to_geneve_nlattr(tun_key, tun_flow_key, key_buf, a);
+
+if (!tnl_type || !strcmp(tnl_type, "geneve")) {
+tun_metadata_to_geneve_nlattr(tun_key, tun_flow_key, key_buf, a);
+}
 
 nl_msg_end_nested(a, tun_key_ofs);
 }
@@ -5409,7 +5419,7 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 
 if (flow_tnl_dst_is_set(>tunnel) || export_mask) {
 tun_key_to_attr(buf, >tunnel, >flow->tunnel,
-parms->key_buf);
+parms->key_buf, NULL);
 }
 
 nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark);
@@ -5661,7 +5671,7 @@ odp_key_from_dp_packet(struct ofpbuf *buf, const struct 
dp_packet *packet)
 nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, md->skb_priority);
 
 if (flow_tnl_dst_is_set(>tunnel)) {
-tun_key_to_attr(buf, >tunnel, >tunnel, NULL);
+tun_key_to_attr(buf, >tunnel, >tunnel, NULL, NULL);
 }
 
 nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, md->pkt_mark);
@@ -6697,10 +6707,10 @@ odp_put_push_eth_action(struct ofpbuf *odp_actions,
 
 void
 odp_put_tunnel_action(const struct flow_tnl *tunnel,
-  struct ofpbuf *odp_actions)
+  struct ofpbuf *odp_actions, const char *tnl_type)
 {
 size_t offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
-tun_key_to_attr(odp_actions, tunnel, tunnel, NULL);
+tun_key_to_attr(odp_actions, tunnel, tunnel, NULL, tnl_type);
 nl_msg_end_nested(odp_actions, offset);
 }
 
@@ -6755,7 +6765,7 @@ commit_masked_set_action(struct ofpbuf *odp_actions,
  * only on tunneling 

[ovs-dev] Cómo evitar frases que desmotiven.

[Ortografía y Redacción]

 
Seminario Intensivo de Ortografía y Redacción
 Fecha: 30/mayo/2018
 

Horario: 10:00 a 13:00 y 15:00 a 18:00 horas
 


Una buena redacción debe ser sencilla y congruente.



En este seminario online en vivo aprenderá:

Aprenda a redactar de manera efectiva, cordial y convincente.
Provocará reacciones positivas a través de sus comunicados.
Distinguirá la diferencia que hay entra una carta, una circular, un memorándum 
y un informe. 
Conocerá los errores más comunes en redacción. 


obtendrá las herramientas poderosas para redactar con mayor impacto todos sus 
comunicados, a través de establecer su comunicación escrita de manera efectiva 
y escribir propuestas atractivas a su lector






 
Responda este correo con el asunto Redacción, con los siguientes datos:

Nombre: 
Correo: 
Teléfono: 

 
Si lo que usted desea dejar de recibir este tipo de mensajes responder este 
correo con el asunto BAJA
 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

[Ben Pfaff]

On Mon, May 14, 2018 at 11:23:11PM +0530, Numan Siddique wrote:
> On Mon, May 14, 2018, 10:44 PM Ben Pfaff  wrote:
> 
> > On Fri, May 11, 2018 at 04:08:00PM +0530, nusid...@redhat.com wrote:
> > > From: Numan Siddique 
> > >
> > > Presently when a VM's IPv6 stack sends a Neighbor Solicitation request
> > for its
> > > router IP, (mostly when the ND cache entry for the router is in STALE
> > state)
> > > ovn-controller responds with a Neighbor Adv packet (using the action
> > nd_na).
> > > But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861
> > page 23).
> > > Because of which, the VM deletes the default route. The default route
> > gets added
> > > again when the next RA is received (but would again gets deleted if its
> > sends
> > > NS request). And this results in disruption of IPv6 traffic.
> > >
> > > This patch addresses this issue by adding a new action 'nd_na_router'
> > which is
> > > same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags.
> > ovn-northd
> > > uses this action. A new action is added instead of modifying the
> > existing 'nd_na'
> > > action. This is because
> > >   - We cannot set the RSO flags in the "nd_na { ..actions .. }"
> > >   - It would be ugly to have something like nd_na { router_flags,
> > ...actions .. }
> > >
> > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> > > Signed-off-by: Numan Siddique 
> > > Acked-by: Mark Michelson 
> >
> > Thanks, applied to master.
> >
> 
> Thank you Ben for the review and applying the patch.
> 
> Can this be applied to branch 2.9 as well ? I resolved conflicts and
> submitted the patch for
> branch 2.9 - https://patchwork.ozlabs.org/patch/913165/

Usually, I think of upgrades within a given series like 2.9.x as being
simple and lightweight.  If we apply this to branch-2.9, though, an
upgrade from 2.9.0 to 2.9. will take as much care as an upgrade from
2.8.x to 2.9.x, because the upgrades to each node have to be carefully
ordered.  Is that desirable?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Cómo crear sus propias Macros en Excel

[Excel: Macros]

Junio 06 - webinar Interactivo
Excel: Macros

Introducción:

Una de las funciones más destacadas de Excel es la creación de Macros que 
permite automatizar los procesos, optimizando el tiempo de trabajo. Este curso 
online está diseñado para ser una experiencia online totalmente personalizada. 
Los asistentes podrán interactuar en todo momento con el instructor que 
presentará los temas y ejercicios prácticos para dominar y crear sus propias 
Macros en Excel. 

Temas a tratar:

Grabar Macros Concepto, métodos y requisitos para macros. 
Introducción a Visual Basic 
Macros de Asignación, ubicación y funciones. 
Variables y cálculos 
Macros especiales 
 
 
Temario e Inscripciones:

Respondiendo por este medio "MExcel"+Teléfono + Empresa+ Nombre o marcando al:

045 + 5515546630 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/3] ofproto-dpif: Improve dp_hash selection method for select groups

[Ben Pfaff]

On Fri, Apr 27, 2018 at 02:20:33PM +0200, Jan Scheurich wrote:
> The current implementation of the "dp_hash" selection method suffers
> from two deficiences: 1. The hash mask and hence the number of dp_hash
> values is just large enough to cover the number of group buckets, but
> does not consider the case that buckets have different weights. 2. The
> xlate-time selection of best bucket from the masked dp_hash value often
> results in bucket load distributions that are quite different from the
> bucket weights because the number of available masked dp_hash values
> is too small (2-6 bits compared to 32 bits of a full hash in the default
> hash selection method).
> 
> This commit provides a more accurate implementation of the dp_hash
> select group by applying the well known Webster method for distributing
> a small number of "seats" fairly over the weighted "parties"
> (see https://en.wikipedia.org/wiki/Webster/Sainte-Lagu%C3%AB_method).
> The dp_hash mask is autmatically chosen large enough to provide good
> enough accuracy even with widely differing weights.
> 
> This distribution happens at group modification time and the resulting
> table is stored with the group-dpif struct. At xlation time, we use the
> masked dp_hash values as index to look up the assigned bucket.
> 
> If the bucket should not be live, we do a circular search over the
> mapping table until we find the first live bucket. As the buckets in
> the table are by construction in pseudo-random order with a frequency
> according to their weight, this method maintains correct distribution
> even if one or more buckets are non-live.
> 
> Xlation is further simplified by storing some derived select group state
> at group construction in struct group-dpif in a form better suited for
> xlation purposes.
> 
> Adapted the unit test case for dp_hash select group accordingly.
> 
> Signed-off-by: Jan Scheurich 
> Signed-off-by: Nitin Katiyar 
> Co-authored-by: Nitin Katiyar 

Thanks a lot.

I don't think that the new 'aux' member in ofputil_bucket is too
useful.  It looks to me like the only use of it could be kept just as
easily in struct webster.

group_setup_dp_hash_table() uses floating-point arithmetic for good
reasons, but it seems to me that some of it is unnecessary, especially
since we have DIV_ROUND_UP and ROUND_UP_POW2.

group_dp_hash_best_bucket() seems like it unnecessarily modifies its
dp_hash parameter (and then never uses it again) and unnecessarily uses
% when & would work.  I also saw a few ways to make the style better
match what we most often do these days.

So here's an incremental that I suggest folding in for v4.  What do you
think?

Thanks,

Ben.

--8<--cut here-->8--

diff --git a/include/openvswitch/ofp-group.h b/include/openvswitch/ofp-group.h
index af4033dc68e4..8d893a53fcb2 100644
--- a/include/openvswitch/ofp-group.h
+++ b/include/openvswitch/ofp-group.h
@@ -47,7 +47,6 @@ struct bucket_counter {
 /* Bucket for use in groups. */
 struct ofputil_bucket {
 struct ovs_list list_node;
-uint16_t aux;   /* Padding. Also used for temporary data. */
 uint16_t weight;/* Relative weight, for "select" groups. */
 ofp_port_t watch_port;  /* Port whose state affects whether this bucket
  * is live. Only required for fast failover
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e35582df0c37..1c78c2d7ca50 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4386,26 +4386,22 @@ group_dp_hash_best_bucket(struct xlate_ctx *ctx,
   const struct group_dpif *group,
   uint32_t dp_hash)
 {
-struct ofputil_bucket *bucket, *best_bucket = NULL;
-uint32_t n_hash = group->hash_mask + 1;
-
-uint32_t hash = dp_hash &= group->hash_mask;
-ctx->wc->masks.dp_hash |= group->hash_mask;
+uint32_t hash_mask = group->hash_mask;
+ctx->wc->masks.dp_hash |= hash_mask;
 
 /* Starting from the original masked dp_hash value iterate over the
  * hash mapping table to find the first live bucket. As the buckets
  * are quasi-randomly spread over the hash values, this maintains
  * a distribution according to bucket weights even when some buckets
  * are non-live. */
-for (int i = 0; i < n_hash; i++) {
-bucket = group->hash_map[(hash + i) % n_hash];
-if (bucket_is_alive(ctx, bucket, 0)) {
-best_bucket = bucket;
-break;
+for (int i = 0; i <= hash_mask; i++) {
+struct ofputil_bucket *b = group->hash_map[(dp_hash + i) & hash_mask];
+if (bucket_is_alive(ctx, b, 0)) {
+return b;
 }
 }
 
-return best_bucket;
+return NULL;
 }
 
 static void
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 

Re: [ovs-dev] [PATCH v3 1/3] userspace datapath: Add OVS_HASH_L4_SYMMETRIC dp_hash algorithm

[Ben Pfaff]

On Fri, Apr 27, 2018 at 02:20:32PM +0200, Jan Scheurich wrote:
> This commit implements a new dp_hash algorithm OVS_HASH_L4_SYMMETRIC in
> the netdev datapath. It will be used as default hash algorithm for the
> dp_hash-based select groups in a subsequent commit to maintain
> compatibility with the symmetry property of the current default hash
> selection method.
> 
> A new dpif_backer_support field 'max_hash_alg' is introduced to reflect
> the highest hash algorithm a datapath supports in the dp_hash action.
> 
> Signed-off-by: Jan Scheurich 
> Signed-off-by: Nitin Katiyar 
> Co-authored-by: Nitin Katiyar 

Acked-by: Ben Pfaff 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

[Numan Siddique]

On Mon, May 14, 2018, 10:44 PM Ben Pfaff  wrote:

> On Fri, May 11, 2018 at 04:08:00PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > Presently when a VM's IPv6 stack sends a Neighbor Solicitation request
> for its
> > router IP, (mostly when the ND cache entry for the router is in STALE
> state)
> > ovn-controller responds with a Neighbor Adv packet (using the action
> nd_na).
> > But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861
> page 23).
> > Because of which, the VM deletes the default route. The default route
> gets added
> > again when the next RA is received (but would again gets deleted if its
> sends
> > NS request). And this results in disruption of IPv6 traffic.
> >
> > This patch addresses this issue by adding a new action 'nd_na_router'
> which is
> > same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags.
> ovn-northd
> > uses this action. A new action is added instead of modifying the
> existing 'nd_na'
> > action. This is because
> >   - We cannot set the RSO flags in the "nd_na { ..actions .. }"
> >   - It would be ugly to have something like nd_na { router_flags,
> ...actions .. }
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> > Signed-off-by: Numan Siddique 
> > Acked-by: Mark Michelson 
>
> Thanks, applied to master.
>

Thank you Ben for the review and applying the patch.

Can this be applied to branch 2.9 as well ? I resolved conflicts and
submitted the patch for
branch 2.9 - https://patchwork.ozlabs.org/patch/913165/


Thanks
Numan

>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [branch-2.9 PATCHv1] ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

[nusiddiq]

From: Numan Siddique 

Presently when a VM's IPv6 stack sends a Neighbor Solicitation request for its
router IP, (mostly when the ND cache entry for the router is in STALE state)
ovn-controller responds with a Neighbor Adv packet (using the action nd_na).
But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861 page 
23).
Because of which, the VM deletes the default route. The default route gets added
again when the next RA is received (but would again gets deleted if its sends
NS request). And this results in disruption of IPv6 traffic.

This patch addresses this issue by adding a new action 'nd_na_router' which is
same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags. ovn-northd
uses this action. A new action is added instead of modifying the existing 
'nd_na'
action. This is because
  - We cannot set the RSO flags in the "nd_na { ..actions .. }"
  - It would be ugly to have something like nd_na { router_flags, ...actions .. 
}

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
Signed-off-by: Numan Siddique 
Acked-by: Mark Michelson 

(cherry-picked from c9756229ed818e4c354d8bfd7c65c656f92fc98b and resolved 
conflicts)
---
 include/ovn/actions.h   |  7 +++
 ovn/controller/pinctrl.c| 23 +
 ovn/lib/actions.c   | 23 +
 ovn/northd/ovn-northd.8.xml | 34 ++
 ovn/northd/ovn-northd.c |  2 +-
 ovn/ovn-sb.xml  | 41 +
 ovn/utilities/ovn-trace.c   |  1 +
 tests/ovn.at|  5 +
 8 files changed, 123 insertions(+), 13 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index d2a79383f..c3251275b 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -65,6 +65,7 @@ struct ovn_extend_table;
 OVNACT(CLONE, ovnact_nest)\
 OVNACT(ARP,   ovnact_nest)\
 OVNACT(ND_NA, ovnact_nest)\
+OVNACT(ND_NA_ROUTER,  ovnact_nest)\
 OVNACT(GET_ARP,   ovnact_get_mac_bind)\
 OVNACT(PUT_ARP,   ovnact_put_mac_bind)\
 OVNACT(GET_ND,ovnact_get_mac_bind)\
@@ -429,6 +430,12 @@ enum action_opcode {
  * The actions, in OpenFlow 1.3 format, follow the action_header.
  */
 ACTION_OPCODE_ND_NS,
+
+/* "nd_na_router { ...actions... }" with rso flag 'ND_RSO_ROUTER' set.
+*
+* The actions, in OpenFlow 1.3 format, follow the action_header.
+*/
+ACTION_OPCODE_ND_NA_ROUTER,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index daf485173..f83b26601 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -79,7 +79,8 @@ static void send_garp_run(struct controller_ctx *ctx,
   struct sset *active_tunnels);
 static void pinctrl_handle_nd_na(const struct flow *ip_flow,
  const struct match *md,
- struct ofpbuf *userdata);
+ struct ofpbuf *userdata,
+ bool is_router);
 static void reload_metadata(struct ofpbuf *ofpacts,
 const struct match *md);
 static void pinctrl_handle_put_nd_ra_opts(
@@ -1001,7 +1002,11 @@ process_packet_in(const struct ofp_header *msg, struct 
controller_ctx *ctx)
 break;
 
 case ACTION_OPCODE_ND_NA:
-pinctrl_handle_nd_na(, _metadata, );
+pinctrl_handle_nd_na(, _metadata, , false);
+break;
+
+case ACTION_OPCODE_ND_NA_ROUTER:
+pinctrl_handle_nd_na(, _metadata, , true);
 break;
 
 case ACTION_OPCODE_PUT_ND:
@@ -2145,7 +2150,7 @@ reload_metadata(struct ofpbuf *ofpacts, const struct 
match *md)
 
 static void
 pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
- struct ofpbuf *userdata)
+ struct ofpbuf *userdata, bool is_router)
 {
 /* This action only works for IPv6 ND packets, and the switch should only
  * send us ND packets this way, but check here just to be sure. */
@@ -2159,13 +2164,15 @@ pinctrl_handle_nd_na(const struct flow *ip_flow, const 
struct match *md,
 struct dp_packet packet;
 dp_packet_use_stub(, packet_stub, sizeof packet_stub);
 
-/* xxx These flags are not exactly correct.  Look at section 7.2.4
- * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
- * xxx router's interfaces and ND_RSO_SOLICITED only if it was
- * xxx requested. */
+/* These flags are not exactly correct.  Look at section 7.2.4
+ * of RFC 4861. */
+uint32_t rso_flags = ND_RSO_SOLICITED | ND_RSO_OVERRIDE;
+if (is_router) {
+rso_flags |= ND_RSO_ROUTER;
+}
 compose_nd_na(, ip_flow->dl_dst, ip_flow->dl_src,
   _flow->nd_target, 

Re: [ovs-dev] [PATCH v4] ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

[Ben Pfaff]

On Fri, May 11, 2018 at 04:08:00PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> Presently when a VM's IPv6 stack sends a Neighbor Solicitation request for its
> router IP, (mostly when the ND cache entry for the router is in STALE state)
> ovn-controller responds with a Neighbor Adv packet (using the action nd_na).
> But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861 page 
> 23).
> Because of which, the VM deletes the default route. The default route gets 
> added
> again when the next RA is received (but would again gets deleted if its sends
> NS request). And this results in disruption of IPv6 traffic.
> 
> This patch addresses this issue by adding a new action 'nd_na_router' which is
> same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags. ovn-northd
> uses this action. A new action is added instead of modifying the existing 
> 'nd_na'
> action. This is because
>   - We cannot set the RSO flags in the "nd_na { ..actions .. }"
>   - It would be ugly to have something like nd_na { router_flags, ...actions 
> .. }
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
> Signed-off-by: Numan Siddique 
> Acked-by: Mark Michelson 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Wait for NDPs to be sent in tunnel-push-pop-ipv6.

[Ben Pfaff]

On Mon, May 14, 2018 at 04:35:43PM +0300, Ilya Maximets wrote:
> Otherwise the tests can fail under heavy load (or with valgrind).
> 
> Signed-off-by: Ilya Maximets 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] sparse: Support newer GCC/glibc versions.

[Ben Pfaff]

This fixes some "sparse" errors I encountered after upgrading.

Signed-off-by: Ben Pfaff 
---
 include/sparse/automake.mk |  2 ++
 include/sparse/bits/floatn.h   | 32 
 include/sparse/sys/sysmacros.h | 32 
 3 files changed, 66 insertions(+)
 create mode 100644 include/sparse/bits/floatn.h
 create mode 100644 include/sparse/sys/sysmacros.h

diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk
index e9b42f52ccb7..ce445fab1e5c 100644
--- a/include/sparse/automake.mk
+++ b/include/sparse/automake.mk
@@ -1,5 +1,6 @@
 noinst_HEADERS += \
 include/sparse/arpa/inet.h \
+include/sparse/bits/floatn.h \
 include/sparse/assert.h \
 include/sparse/bmi2intrin.h \
 include/sparse/emmintrin.h \
@@ -13,5 +14,6 @@ noinst_HEADERS += \
 include/sparse/rte_memcpy.h \
 include/sparse/rte_vect.h \
 include/sparse/sys/socket.h \
+include/sparse/sys/sysmacros.h \
 include/sparse/sys/types.h \
 include/sparse/sys/wait.h
diff --git a/include/sparse/bits/floatn.h b/include/sparse/bits/floatn.h
new file mode 100644
index ..a247331eba6b
--- /dev/null
+++ b/include/sparse/bits/floatn.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2018 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef __CHECKER__
+#error "Use this header only with sparse.  It is not a correct implementation."
+#endif
+
+#ifndef __BITS_FLOATN_SPARSE
+#define __BITS_FLOATN_SPARSE 1
+
+/* "sparse" claims to be a recent version of GCC but doesn't support IEEE 754
+ * binary128, so we define macros to make that clear. */
+
+#define __HAVE_FLOAT128 0
+#define __HAVE_FLOAT64X 0
+
+#include 
+
+#endif /*  for sparse */
diff --git a/include/sparse/sys/sysmacros.h b/include/sparse/sys/sysmacros.h
new file mode 100644
index ..0c962a0af43a
--- /dev/null
+++ b/include/sparse/sys/sysmacros.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2018 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef __CHECKER__
+#error "Use this header only with sparse.  It is not a correct implementation."
+#endif
+
+#ifndef __SYS_SYSMACROS_SPARSE
+#define __SYS_SYSMACROS_SPARSE 1
+
+/* "sparse" doesn't like the large constants in , complaining
+ * that they are so large that they have type "unsigned long long".  This
+ * header avoids the problem. */
+
+unsigned int major(dev_t);
+unsigned int minor(dev_t);
+dev_t makedev(unsigned int, unsigned int);
+
+#endif /*  for sparse */
-- 
2.16.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 4/5] selinux: introduce domain transitioned kmod helper

[Ansis Atteka]

On Fri, 4 May 2018 at 11:28, Aaron Conole  wrote:

> This commit uses the previously defined selinux label to transition
> from the openvswitch_t to openvswitch_load_module_t domain by
> executing ovs-kmod-ctl that is labelled with
> openvswitch_load_module_exec_t type.

> Note that unless the selinux relabel operation is invoked, the script
> will not be labelled.  This merely instructs the selinux tools that
> ovs-kmod-ctl should have a label applied.

> Acked-By: Timothy Redaelli 
> Signed-off-by: Aaron Conole 
Acked-by: Ansis Atteka 
> ---
>   selinux/.gitignore   | 4 
>   selinux/automake.mk  | 3 ++-
>   selinux/openvswitch-custom.fc.in | 1 +
>   3 files changed, 7 insertions(+), 1 deletion(-)
>   create mode 100644 selinux/openvswitch-custom.fc.in

> diff --git a/selinux/.gitignore b/selinux/.gitignore
> index 83a0afb51..64e834cd1 100644
> --- a/selinux/.gitignore
> +++ b/selinux/.gitignore
> @@ -1 +1,5 @@
>   openvswitch-custom.te
> +openvswitch-custom.fc
> +openvswitch-custom.pp
> +openvswitch-custom.if
> +tmp/
> diff --git a/selinux/automake.mk b/selinux/automake.mk
> index b37e8f337..c7dfe6ed5 100644
> --- a/selinux/automake.mk
> +++ b/selinux/automake.mk
> @@ -6,11 +6,12 @@
>   # without warranty of any kind.

>   EXTRA_DIST += \
> +selinux/openvswitch-custom.fc.in \
>   selinux/openvswitch-custom.te.in

>   PHONY: selinux-policy

> -selinux-policy: selinux/openvswitch-custom.te
> +selinux-policy: selinux/openvswitch-custom.te
selinux/openvswitch-custom.fc
>  $(MAKE) -C selinux/ -f /usr/share/selinux/devel/Makefile

>   CLEANFILES += \
> diff --git a/selinux/openvswitch-custom.fc.in b/selinux/
openvswitch-custom.fc.in
> new file mode 100644
> index 0..c2756d04b
> --- /dev/null
> +++ b/selinux/openvswitch-custom.fc.in
> @@ -0,0 +1 @@
> +@pkgdatadir@/scripts/ovs-kmod-ctl --
gen_context(system_u:object_r:openvswitch_load_module_exec_t,s0)
> --
> 2.14.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests: Wait for NDPs to be sent in tunnel-push-pop-ipv6.

[Ilya Maximets]

Otherwise the tests can fail under heavy load (or with valgrind).

Signed-off-by: Ilya Maximets 
---
 tests/tunnel-push-pop-ipv6.at | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
index 78fbf41..772f62c 100644
--- a/tests/tunnel-push-pop-ipv6.at
+++ b/tests/tunnel-push-pop-ipv6.at
@@ -45,6 +45,11 @@ dnl Check Neighbour discovery.
 AT_CHECK([ovs-vsctl -- set Interface p0 options:pcap=p0.pcap])
 
 AT_CHECK([ovs-appctl netdev-dummy/receive int-br 
'in_port(2),eth(src=aa:55:aa:55:00:00,dst=f8:bc:12:ff:ff:ff),eth_type(0x0800),ipv4(src=1.1.3.92,dst=1.1.3.88,proto=1,tos=0,ttl=64,frag=no),icmp(type=0,code=0)'])
+
+dnl Wait for the two Neighbor Solicitation packets to be sent.
+dnl Sometimes the system can be slow (e.g. under valgrind)
+OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | sort | uniq | wc -l` -ge 2])
+
 AT_CHECK([ovs-pcap p0.pcap > p0.pcap.txt 2>&1])
 
 AT_CHECK([cat p0.pcap.txt | grep 92aa55aa5586dd60203aff2001cafe | 
uniq], [0], [dnl
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

[Ilya Maximets]

Hello Anju, Ben,

Looks like I fixed a leak reported here in my recent patch:
  https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346892.html

Could you, please, take a look at it?

I actually managed to reproduce the packet leak on tunnel-push-pop
unit tests with valgrind:

==6445== 2,912 (2,208 direct, 704 indirect) bytes in 4 blocks are definitely 
lost in loss record 400 of 410
==6445==at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6445==by 0x516314: xmalloc (util.c:120)
==6445==by 0x466154: dp_packet_new (dp-packet.c:138)
==6445==by 0x466154: dp_packet_new_with_headroom (dp-packet.c:148)
==6445==by 0x4F6CFE: eth_from_hex (packets.c:498)
==6445==by 0x48EB43: eth_from_packet (netdev-dummy.c:1450)
==6445==by 0x48EB43: netdev_dummy_receive (netdev-dummy.c:1562)
==6445==by 0x515980: process_command (unixctl.c:313)
==6445==by 0x515980: run_connection (unixctl.c:347)
==6445==by 0x515980: unixctl_server_run (unixctl.c:398)
==6445==by 0x406F1E: main (ovs-vswitchd.c:120)

Above patch fixes it.

Best regards, Ilya Maximets.

> Hi Ben,
> 
> I will work on these changes as well.
> 
> Regards
> Anju
> 
> -Original Message-
> From: Ben Pfaff [mailto:blp at ovn.org] 
> Sent: Friday, May 11, 2018 2:00 AM
> To: Anju Thomas 
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
> 
> On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote:
>> It looks like your commit message describes at least two other bugs in 
>> OVS, though.  First, if OVS crashes when it pushes tunnel headers, 
>> even if there's not enough headroom, that's really bad.  At worst, it 
>> should drop the packet.  Do you know where the crash occurs?  We 
>> should fix the problem, since it might recur in some other context.
>> 
>> [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual 
>> assert in our case.
>> The rootcause is that dp receives the actions after the upcall (say with >=3 
>> tunnel pushes ) . Now as part of action processing , since it is a tunnel 
>> push action , we try to find space in the dpdk mbuf packet headroom (which 
>> Is 128 bytes). By the time we try to process the third tunnel push , there 
>> is no headroom left since each tunnel header is of 50 bytes (50 *3 > 128 
>> bytes headroom). Hence it manually asserts .  This assert is to catch any 
>> unexpected code flow . Do you think that in this case we should still go 
>> ahead and  prevent the crash ? 
> 
> I don't understand why it's OK to crash in this case.  Why do you think so?
> 
>> Second, it's a little shocking to hear that an encapsulation action without 
>> a following output action causes a memory leak.  We also need to fix that.  
>> Do you have any more details?
>> [Anju] Now as explained above, the crash happens because we run out of 
>> headroom. But in case we have say 2 or less than 2 tunnel pushes we will 
>> have a mem leak as packet is never freed because the tnl push is the dp last 
>> action and there is no other output action or any other action like recirc 
>> that may translate to output action in the end leading  to packet buffer not 
>> being freed.
>> Are you proposing that we have some sort of preventive fix in the dp to 
>> handle an incorrect action list from the upcall handling? 
> 
> Yes.  It's unacceptable to leak memory because there's a packet modification 
> without an output action.  The kernel datapath never does this, for example.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] remittance

[Nilanthi Kumari]

Dear, 

Have a nice day ! Kindly check out T T Copy. We have send you payment in your 
account. 


When you receive our payment so confirm us. 


Thanks & Regards, 
Shoaib Miyanji 
Abdul Shakoor Miyanji (Director) 
Miyanji Trading Co., 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

[Lam, Tiago]

Hi Anju,

On 10/05/2018 09:51, Anju Thomas wrote:

[snip]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org] 
> Sent: Thursday, May 10, 2018 1:59 AM
> To: Anju Thomas 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
> 
> Thanks for the patch.  It applies OK and all the tests pass.
> 
> It looks like your commit message describes at least two other bugs in OVS, 
> though.  First, if OVS crashes when it pushes tunnel headers, even if there's 
> not enough headroom, that's really bad.  At worst, it should drop the packet. 
>  Do you know where the crash occurs?  We should fix the problem, since it 
> might recur in some other context.
> 
> [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual 
> assert in our case.
> The rootcause is that dp receives the actions after the upcall (say with >=3 
> tunnel pushes ) . Now as part of action processing , since it is a tunnel 
> push action , we try to find space in the dpdk mbuf packet headroom (which Is 
> 128 bytes). By the time we try to process the third tunnel push , there is no 
> headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes 
> headroom). Hence it manually asserts .  This assert is to catch any 
> unexpected code flow . Do you think that in this case we should still go 
> ahead and  prevent the crash ? 
> 
> The back trace was as below:-
> (gdb) bt
> #0  0x7ffa9a856c37 in __GI_raise (sig=sig@entry=6) at 
> ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> #1  0x7ffa9a85a028 in __GI_abort () at abort.c:89
> #2  0x005fc725 in dp_packet_resize__ (b=b@entry=0x7ffa87744680, 
> new_headroom=new_headroom@entry=64, new_tailroom=)
> at lib/dp-packet.c:258
> #3  0x005fcb1f in dp_packet_prealloc_headroom 
> (b=b@entry=0x7ffa87744680, size=size@entry=50) at lib/dp-packet.c:288
> #4  0x005fcf91 in dp_packet_push_uninit (b=b@entry=0x7ffa87744680, 
> size=size@entry=50) at lib/dp-packet.c:400
> #5  0x0069466c in netdev_tnl_push_ip_header 
> (packet=packet@entry=0x7ffa87744680, header=0x7ff85401bab0, size=50,
> ip_tot_size=ip_tot_size@entry=0x7ff8117f89fc) at 
> lib/netdev-native-tnl.c:152
> #6  0x0069472a in netdev_tnl_push_udp_header (packet=0x7ffa87744680, 
> data=) at lib/netdev-native-tnl.c:215
> #7  0x00625627 in netdev_push_header (netdev=0x3ca3990, 
> batch=batch@entry=0x7ff8117f9498, data=data@entry=0x7ff85401baa0)
> at lib/netdev.c:874
> #8  0x006069f2 in push_tnl_action (batch=0x7ff8117f9498, 
> attr=0x7ff85401ba9c, pmd=0x33efb30) at lib/dpif-netdev.c:4629
> #9  dp_execute_cb (aux_=aux_@entry=0x7ff8117f9840, 
> packets_=packets_@entry=0x7ff8117f9498, a=a@entry=0x7ff85401ba9c, 
> may_steal=false)
> 
> 
> static void
> dp_packet_resize__(struct dp_packet *b, size_t new_headroom, size_t 
> new_tailroom)
> {
> void *new_base, *new_data;
> size_t new_allocated;
> 
> new_allocated = new_headroom + dp_packet_size(b) + new_tailroom;
> 
> switch (b->source) {
> case DPBUF_DPDK:
> 
> OVS_NOT_REACHED();-->crashes
>  here
>
Just to point out that we are working on adding support for dynamically
allocating more mbufs, as part of our multi-segment mbufs series.

In this case however, still using a single mbuf, I think there's two
points worth noting:
1. Mbufs are allocated with the maximum possible packet size based on
the MTU. And given that a single mbuf is used per packet, one can't
(dynamically) allocate more memory to increase that limit. The solution
for this is to append more mbufs, the multi-segment mbufs approach;
2. As you mention, typically the size set for the headroom in a mbuf is
128B.

Point 2. doesn't seem to be enough for your use-case and, nonetheless, I
agree that crashing OvS because there's not enough space, either
headroom or tailroom, to accommodate for new data is not the best approach.

I was planning to work on a proposal to deal with this for single mbufs
as well, but I see you have already committed to that so I'll withdraw
myself. I'll keep and eye out, though, and will gladly review / test
once you submit.

Thanks,

Tiago.

Note: One can still increase the headroom by building DPDK using the
config option `CONFIG_RTE_PKTMBUF_HEADROOM`, which is set to 128B by
default. However, this doesn't seem to be done often and I can not vouch
for testing coverage here.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] About missing news entry for LSC backports

[Stokes, Ian]

> Hey there,
> 
> As per subject, you may want to write something down in the NEWS as part
> of the next dpdk_marge for the recent LSC backport in the branches 2.7,
> 2.8, and 2.9 as well.
> 
> As you know the LSC provides a quite important workaround for Fortville
> link check an

Good catch Federico, I had added it to NEWS as an incremental on master, forgot 
to add it on the back ports as you spotted. I'll take care of this.

Thanks
Ian

> Cheers,
> Federico
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Avoid crash in OvS while transmitting fragmented packets over tunnel.

[Rohith Basavaraja]

Hi Darell,

In my simple setup I have interface in namespace ns1 with IP 192.168.10.10 and 
in connected to bridge br-in1 (Just mentioning relevant config)
And br-in1 I have following flows.

lab@ubuntu-vm:/var/log/openvswitch$ sudo ovs-ofctl -OOpenFlow13 dump-flows 
br-in1
 cookie=0x0, duration=83.662s, table=0, n_packets=75, n_bytes=7294, 
in_port="br-in1-ns1" 
actions=push_mpls:0x8847,set_field:666->mpls_label,output:gre1
 cookie=0x0, duration=83.644s, table=0, n_packets=75, n_bytes=7594, 
mpls,in_port=gre1,mpls_label=999 
actions=pop_mpls:0x0800,set_field:26:ed:b2:5d:50:d7->eth_dst,output:"br-in1-ns1"

When I ping the remote interface things works fine

lab@ubuntu-vm:~$ sudo ip netns exec ns2 ping -W 1 192.168.10.10
PING 192.168.10.10 (192.168.10.10) 56(84) bytes of data.
64 bytes from 192.168.10.10: icmp_seq=1 ttl=64 time=1.62 ms
64 bytes from 192.168.10.10: icmp_seq=2 ttl=64 time=0.723 ms

and the corresponding dpctl entries are are as follows

lab@ubuntu-vm:~/testsuites$ sudo ovs-appctl dpif/dump-flows br-in1

tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-csum)),recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(dst=26:ed:b2:5d:50:d7),eth_type(0x8847),mpls(label=999/0xf,tc=0/0,ttl=64/0x0,bos=1/1),
 packets:33, bytes:3366, used:0.978s, actions:pop_mpls(eth_type=0x800),5

recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(tos=0/0xfc,ttl=64,frag=no),
 packets:33, bytes:3234, used:0.978s, 
actions:push_mpls(label=666,tc=0,ttl=64,bos=1,eth_type=0x8847),clone(tnl_push(tnl_port(7),header(size=38,type=3,eth(dst=86:67:03:ad:e7:e9,src=7a:9f:1f:ce:fc:b6,dl_type=0x0800),ipv4(src=10.0.0.1,dst=10.0.0.2,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x6558))),out_port(2)),8)


Then Moments I start transmitting the fragmented packet (by increasing the 
packet size) I hit the assert mentioned in the problem.

sudo ip netns exec ns2 ping -s 5000 -W 1 192.168.10.10 <=== Note packet size is 
increased to force fragmentation at the sender.

2018-05-14T18:35:44.571Z|00114|util|EMER|lib/odp-util.c:7008: assertion 
flow->nw_proto == base_flow->nw_proto && flow->nw_frag == base_flow->nw_frag 
failed in commit_set_ipv4_action()
2018-05-14T18:35:44.835Z|2|daemon_unix(monitor)|ERR|1 crashes: pid 19200 
died, killed (Aborted), core dumped, restarting

Thanks
Rohith

From: Darrell Ball 
Date: Friday, 11 May 2018 at 10:33 PM
To: Rohith Basavaraja 
Cc: Darrell Ball , "d...@openvswitch.org" 

Subject: Re: [ovs-dev] [PATCH] Avoid crash in OvS while transmitting fragmented 
packets over tunnel.



On Thu, May 10, 2018 at 11:35 PM, Rohith Basavaraja 
> wrote:
Hi Darell,

I reproduce the issue by making VM to transmit fragmented packets and in OvS if 
we have corresponding rule
to transmit the received fragmented packet (from VM) over tunnel then I always 
hit the crash.

Thanks Rohith

Unfortunately, I don't hit the assert.
I even tried again with the latest master with this change removed.
I used Geneve for the last test, but I am pretty sure I used Vxlan as well for 
this kind of test b4, as I use it often as well.

Would you mind describing your test in more detail (packets and flows)?

Thanks Darrell




Thanks
Rohith

On 11/05/18, 2:16 AM, "Darrell Ball" 
> wrote:

Thanks Rohith
I see this patch was applied, but I have one question inline.


On 4/20/18, 1:48 AM, 
"ovs-dev-boun...@openvswitch.org on 
behalf of Rohith Basavaraja" 
 on 
behalf of 
rohith.basavar...@ericsson.com> wrote:

Currently when fragmented packets are to be transmitted in to tunnel,
base_flow->nw_frag which was initially non-zero at reception is not
reset to zero when the base_flow and flow are rewritten
as part of the emulated tnl_push action in the ofproto-dpif-xlate
module.

Because of this when fragmented packets are transmitted out of tunnel,
we hit crash caused by the following assert.

lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()

Can you describe how you hit this assertion?
I have some testing in and around this code, but have not hit this yet, so 
I was curious?

With the following change propagate_tunnel_data_to_flow__
is modified to reset *nw_frag* to zero.


Also, that currently we don't
fragment tunnelled packets, we should reset *nw_frag* to zero in
propagate_tunnel_data_to_flow__.

Signed-off-by: Jan Scheurich 
>
From: Rohith Basavaraja 

Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow

[Stokes, Ian]

> Hi Ian,
> 
> It is taking almost a month to integrate this code.
> Anything should be done to expedite it?

Hi Olga,

I realize it has been a month since the last version was submitted, 
unfortunately in terms of resources for reviews and testing the community is 
limited.

I flagged at the community call two weeks ago that there were a number of 
"large" feature patchsets currently on the backlog that could overlap. It was 
my intention to merge the detailed pmd metrics patchset first and then 
prioritize the HWOL series. There was no arguments raised against this approach.

The detailed PMD metrics was merged last week and HWOL is the feature I will 
look to upstream next.

In terms of expediting it, if the HWOL series could be rebased to head of 
master this week that would help, I can do final validation and have it as part 
of the next pull request (this is aimed for the end of next week).

Thanks
Ian

> 
> Best regards,
> Olga
> 
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Stokes, Ian
> Sent: Thursday, May 10, 2018 10:19 PM
> To: Shahaf Shuler ; f...@napatech.com
> Cc: ovs-dev@openvswitch.org; Flavio Leitner ;
> simon.hor...@netronome.com
> Subject: Re: [ovs-dev] [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> 
> 
> 
> > -Original Message-
> > From: Shahaf Shuler [mailto:shah...@mellanox.com]
> > Sent: Thursday, May 10, 2018 1:41 PM
> > To: Stokes, Ian ; f...@napatech.com
> > Cc: ovs-dev@openvswitch.org; simon.hor...@netronome.com; Flavio
> > Leitner 
> > Subject: RE: [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> >
> > Hi,
> >
> > Friday, April 20, 2018 11:07 AM, Stokes, Ian:
> > > Subject: RE: [PATCH v9 0/7] OVS-DPDK flow offload with rte_flow
> > >
> > > > > Hi,
> > > > >
> > > > > Here is a joint work from Mellanox and Napatech, to enable the
> > > > > flow hw offload with the DPDK generic flow interface (rte_flow).
> > > >
> > > > Hi folks, I feel Mellanox/Netronome have reached the point where
> > > > HWOL can be introduced to OVS DPDK pending performance review.
> > >
> > > Apologies , Mellanox an Napatech.
> >
> > Any progress with this series?
> >
> 
> Hi Shahaf,
> 
> I'm finishing the pull request for this week now. This series is my
> priority in the next pull request.
> 
> The only outstanding item I need to do is a performance check before and
> after to ensure there is no negative impact on the non HWOL usecase. Other
> than that it should be ok.
> 
> Thanks
> Ian
> >
> > >
> > > Ian
> > > >
> > > > This has not been an easy path, but I will put one final request
> > > > out there for comments.
> > > >
> > > > If there are none then pending on Travis compilation and final
> > > > performance checks I'm thinking of introducing this to the code
> base.
> > > >
> > > > Keep in mind this feature is experimental, not only can it be
> > > > changed but it can also be removed pending future review if not
> > maintained.
> > > >
> > > > I invite all to give opinions here as I think this is an important
> > > > feature.
> > > >
> > > > Sincere thanks to all involved.
> > > >
> > > > Regards
> > > > Ian
> > > >
> > > >
> > > > >
> > > > > The basic idea is to associate the flow with a mark id (a
> > > > > unit32_t number).
> > > > > Later, we then get the flow directly from the mark id, which
> > > > > could bypass some heavy CPU operations, including but not
> > > > > limiting to mini flow extract, emc lookup, dpcls lookup, etc.
> > > > >
> > > > > The association is done with CMAP in patch 1. The CPU workload
> > > > > bypassing is done in patch 2. The flow offload is done in patch
> > > > > 3, which mainly does two things:
> > > > >
> > > > > - translate the ovs match to DPDK rte flow patterns
> > > > > - bind those patterns with a RSS + MARK action.
> > > > >
> > > > > Patch 5 makes the offload work happen in another thread, for
> > > > > leaving the datapath as light as possible.
> > > > >
> > > > > A PHY-PHY forwarding with 1000 mega flows (udp,tp_src=1000-1999)
> > > > > and
> > > > > 1 million streams (tp_src=1000-1999, tp_dst=2000-2999) show more
> > > > > than 260% performance boost.
> > > > >
> > > > > Note that it's disabled by default, which can be enabled by:
> > > > >
> > > > > $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
> > > > >
> > > > > v9: - introduced IP packet sanity checks in a seperate commit
> > > > > - moved structs and enums definition to the begining of the
> file
> > > > > - fixed sparse compilation error (error is assumed to be
> > > > > fixed,
> > > > issues
> > > > >   on Travis CI preventing from fully testing it.
> > > > >
> > > > > v8: - enhanced documentation with more details on supported
> > protocols
> > > > > - fixed VLOG to start with capital letter
> > > > > - fixed compilation issues
> > > > > - fixed coding style
> > > > > - 

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master

[Stokes, Ian]

> On Fri, May 11, 2018 at 11:57:30AM +, Stokes, Ian wrote:
> > Hi Ben,
> >
> > The following changes since commit
> d87e08978fc3b522acb4ecb21cffb5c2108b3b33:
> >
> >   ovn-nbctl: Support ACL commands on port groups. (2018-05-10 16:53:22 -
> 0700)
> >
> > are available in the git repository at:
> >
> >   https://github.com/istokes/ovs dpdk_merge
> >
> > for you to fetch changes up to eaa4358119e627171f1b16ce932a1a34f6ab1eb9:
> >
> >   netdev-dpdk: Fixed netdev_dpdk structure alignment (2018-05-11
> 08:08:24 +0100)
> 
> Thanks, merged to master.

Thanks Ben.

Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4] Upcall/Slowpath rate-limiter for OVS

[Manohar Krishnappa Chidambaraswamy]

Jan,

Have addressed your comments/suggestions. Please take a look and let me
know if you have any more comments.

Signed-off-by: Manohar K C

CC: Jan Scheurich 
---
v3 : v2 rebased to master and adpated to dpif-netdev-perf counters.
 https://patchwork.ozlabs.org/patch/909676/

v2 : v1 rebased to master.
 https://patchwork.ozlabs.org/patch/860687/

v1 : Initial patch for upcall rate-limiter based on token-bucket.
 https://patchwork.ozlabs.org/patch/836737/

 Documentation/howto/dpdk.rst |  53 ++
 lib/dpif-netdev-perf.h   |   1 +
 lib/dpif-netdev.c| 130 ---
 lib/dpif.h   |   1 +
 vswitchd/vswitch.xml |  42 ++
 5 files changed, 220 insertions(+), 7 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 380181d..b717804 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -358,6 +358,59 @@ devices to bridge ``br0``. Once complete, follow the below 
steps:
 
$ cat /proc/interrupts | grep virtio
 
+Upcall rate limiting
+
+In OVS, when an incoming packet does not match any flow in the flow
+table maintained in fast-path (either in userspace or kernel), the
+packet is processed in the slow-path via a mechanism known as upcall.
+In OVS-DPDK, both fast-path and slow-path execute in the context of a
+common poll-mode-driver (PMD) thread, without any partitioning of CPU
+cycles between the two.
+
+When there is a burst of new flows coming into the data-path, packets
+are punted to slow-path in the order they are received and the PMD is
+busy for the duration of the upcall. Slow-path processing of a packet
+consumes 100-200 times the cycles of fast-path handling. As a result,
+the forwarding performance of a PMD degrades significantly during an
+upcall burst. If the PMD was highly loaded already, it becomes temporarily
+overloaded and its rx queues start filling up. If the upcall burst is long
+enough, packets will be dropped when rx queues are full. This happens even
+if the new flows are unexpected and the slow-path decides to drop the
+packets.
+
+It is likely that most of the packets dropped due to rx queue overflow
+belong to established flows that should have been processed by the
+fast-path. Hence, the current OVS-DPDK architecture favors the handling
+of new flows over the forwarding of established flows. This is generally
+a sub-optimal approach.
+
+Without a limit to the rate of upcalls, OVS-DPDK is vulnerable for DoS
+attacks. But even sporadic bursts of e.g. unexpected multicast packets
+have shown to cause such packet drops.
+
+ovs-vsctl can be used to enable and configure upcall rate limit parameters.
+There are 2 configurable parameters ``upcall-rate`` and ``upcall-burst`` which
+take effect when the configuration parameter ``upcall-rl`` is set to true.
+
+Upcall rate-limiting is done at independent PMD level. Configured values for
+``upcall-rate`` and ``upcall-burst`` are used indepdently in each PMD
+(and non-PMD) threads which execute upcalls.
+
+Upcall rate should be set using ``upcall-rate`` in upcalls-per-sec. If not
+configured, default value of 500 upcalls-per-sec will be set. For example::
+
+$ ovs-vsctl set Open_vSwitch . other_config:upcall-rate=2000
+
+Upcall burst size should be set using ``upcall-burst`` in upcalls. If not
+configured, default value of 500 upcalls will be set. For example::
+
+$ ovs-vsctl set Open_vSwitch . other_config:upcall-burst=2000
+
+Upcall ratelimit feature should be globally enabled using ``upcall-rl``. For
+example::
+
+$ ovs-vsctl set Open_vSwitch . other_config:upcall-rl=true
+
 Further Reading
 ---
 
diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h
index 5993c25..e4b945a 100644
--- a/lib/dpif-netdev-perf.h
+++ b/lib/dpif-netdev-perf.h
@@ -55,6 +55,7 @@ enum pmd_stat_type {
  * number of packet passes through the datapath
  * pipeline and should not be overlapping with each
  * other. */
+PMD_STAT_RL_DROP,   /* Packets dropped due to upcall rate-limit. */
 PMD_STAT_MASKED_LOOKUP, /* Number of subtable lookups for flow table
hits. Each MASKED_HIT hit will have >= 1
MASKED_LOOKUP(s). */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index be31fd0..dba8629 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,11 @@ static struct shash dp_netdevs 
OVS_GUARDED_BY(dp_netdev_mutex)
 
 static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 
+/* These default values may be tuned based on upcall performance */
+#define UPCALL_RATELIMIT_DEFAULT false /* Disabled by default */
+#define UPCALL_RATE_DEFAULT  500  /* upcalls-per-sec */
+#define 

Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

[Anju Thomas]

Hi Ben,

I will work on these changes as well.

Regards
Anju

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Friday, May 11, 2018 2:00 AM
To: Anju Thomas 
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action

On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote:
> It looks like your commit message describes at least two other bugs in 
> OVS, though.  First, if OVS crashes when it pushes tunnel headers, 
> even if there's not enough headroom, that's really bad.  At worst, it 
> should drop the packet.  Do you know where the crash occurs?  We 
> should fix the problem, since it might recur in some other context.
> 
> [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual 
> assert in our case.
> The rootcause is that dp receives the actions after the upcall (say with >=3 
> tunnel pushes ) . Now as part of action processing , since it is a tunnel 
> push action , we try to find space in the dpdk mbuf packet headroom (which Is 
> 128 bytes). By the time we try to process the third tunnel push , there is no 
> headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes 
> headroom). Hence it manually asserts .  This assert is to catch any 
> unexpected code flow . Do you think that in this case we should still go 
> ahead and  prevent the crash ? 

I don't understand why it's OK to crash in this case.  Why do you think so?

> Second, it's a little shocking to hear that an encapsulation action without a 
> following output action causes a memory leak.  We also need to fix that.  Do 
> you have any more details?
> [Anju] Now as explained above, the crash happens because we run out of 
> headroom. But in case we have say 2 or less than 2 tunnel pushes we will have 
> a mem leak as packet is never freed because the tnl push is the dp last 
> action and there is no other output action or any other action like recirc 
> that may translate to output action in the end leading  to packet buffer not 
> being freed.
> Are you proposing that we have some sort of preventive fix in the dp to 
> handle an incorrect action list from the upcall handling? 

Yes.  It's unacceptable to leak memory because there's a packet modification 
without an output action.  The kernel datapath never does this, for example.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev