Re: [ovs-dev] [PATCH] compat: skbuff: Remove references to old kernels.

[Simon Horman]

On Thu, Apr 28, 2016 at 06:09:04PM -0700, Joe Stringer wrote:
> Since commit f2ab1536ddbc ("compat: Backport conntrack strictly to
> v3.10+."), we haven't supported these kernel versions. Remove the old
> code.
> 
> Signed-off-by: Joe Stringer 

Acked-by: Simon Horman 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Mail System Error - Returned Mail

[Mail Administrator]

Your message was not delivered due to the following reason:

Your message could not be delivered because the destination computer was
unreachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message was not delivered within 6 days:
Host 171.119.233.163 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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

[ovs-dev] Returned mail: see transcript for details

[Bounced mail]

Dear user of openvswitch.org,

Your account was used to send a large amount of spam messages during the last 
week.
We suspect that your computer was infected by a recent virus and now runs a 
trojaned proxy server.

We recommend you to follow the instructions in the attachment in order to keep 
your computer safe.

Sincerely yours,
The openvswitch.org support team.

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

[ovs-dev] [PATCH v16 1/5] Change encaps_run to work incrementally

[Ryan Moats]

As a side effect, tunnel context is persisted.

Signed-off-by: Ryan Moats 
---
 ovn/controller/encaps.c | 168 +---
 ovn/controller/ovn-controller.c |   5 ++
 2 files changed, 127 insertions(+), 46 deletions(-)

diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 149698e..5adba12 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -15,6 +15,7 @@
 
 #include 
 #include "encaps.h"
+#include "lflow.h"
 
 #include "lib/hash.h"
 #include "lib/sset.h"
@@ -49,6 +50,7 @@ struct tunnel_ctx {
  * generated we remove them.  After generating all the rows, any
  * remaining in 'tunnel_hmap' must be deleted from the database. */
 struct hmap tunnel_hmap;
+struct hmap tunnel_hmap_by_uuid;
 
 /* Names of all ports in the bridge, to allow checking uniqueness when
  * adding a new tunnel. */
@@ -58,8 +60,18 @@ struct tunnel_ctx {
 const struct ovsrec_bridge *br_int;
 };
 
+static struct tunnel_ctx tc = {
+.tunnel_hmap = HMAP_INITIALIZER(&tc.tunnel_hmap),
+.tunnel_hmap_by_uuid = HMAP_INITIALIZER(&tc.tunnel_hmap_by_uuid),
+.port_names = SSET_INITIALIZER(&tc.port_names),
+};
+
+static bool process_full_encaps = false;
+
 struct port_hash_node {
 struct hmap_node node;
+struct hmap_node uuid_node;
+const struct uuid *uuid;
 const struct ovsrec_port *port;
 const struct ovsrec_bridge *bridge;
 };
@@ -92,7 +104,7 @@ port_hash_rec(const struct ovsrec_port *port)
 }
 
 static char *
-tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
+tunnel_create_name(const char *chassis_id)
 {
 int i;
 
@@ -100,7 +112,7 @@ tunnel_create_name(struct tunnel_ctx *tc, const char 
*chassis_id)
 char *port_name;
 port_name = xasprintf("ovn-%.6s-%x", chassis_id, i);
 
-if (!sset_contains(&tc->port_names, port_name)) {
+if (!sset_contains(&tc.port_names, port_name)) {
 return port_name;
 }
 
@@ -110,19 +122,32 @@ tunnel_create_name(struct tunnel_ctx *tc, const char 
*chassis_id)
 return NULL;
 }
 
+static struct port_hash_node *
+port_lookup_by_uuid(const struct uuid *uuid)
+{
+struct hmap_node *node = hmap_first_with_hash(&tc.tunnel_hmap_by_uuid,
+  uuid_hash(uuid));
+if (node) {
+return CONTAINER_OF(node, struct port_hash_node, uuid_node);
+}
+return NULL;
+}
 
 static void
-tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id,
+tunnel_add(const struct sbrec_chassis *chassis_rec,
const struct sbrec_encap *encap)
 {
 struct port_hash_node *hash_node;
+const char *new_chassis_id = chassis_rec->name;
+
+/* Check whether such a row already exists in OVS. If so, update
+ * the uuid field and insert into the by uuid hashmap. If not,
+ * create the tunnel */
 
-/* Check whether such a row already exists in OVS.  If so, remove it
- * from 'tc->tunnel_hmap' and we're done. */
 HMAP_FOR_EACH_WITH_HASH (hash_node, node,
  port_hash(new_chassis_id,
encap->type, encap->ip),
- &tc->tunnel_hmap) {
+ &tc.tunnel_hmap) {
 const struct ovsrec_port *port = hash_node->port;
 const char *chassis_id = smap_get(&port->external_ids,
   "ovn-chassis-id");
@@ -142,8 +167,12 @@ tunnel_add(struct tunnel_ctx *tc, const char 
*new_chassis_id,
 if (!strcmp(new_chassis_id, chassis_id)
 && !strcmp(encap->type, iface->type)
 && !strcmp(encap->ip, ip)) {
-hmap_remove(&tc->tunnel_hmap, &hash_node->node);
-free(hash_node);
+
+hash_node->uuid = &chassis_rec->header_.uuid;
+if (!port_lookup_by_uuid(hash_node->uuid)) {
+hmap_insert(&tc.tunnel_hmap_by_uuid, &hash_node->uuid_node,
+uuid_hash(hash_node->uuid));
+}
 return;
 }
 }
@@ -155,14 +184,14 @@ tunnel_add(struct tunnel_ctx *tc, const char 
*new_chassis_id,
 char *port_name;
 size_t i;
 
-port_name = tunnel_create_name(tc, new_chassis_id);
+port_name = tunnel_create_name(new_chassis_id);
 if (!port_name) {
 VLOG_WARN("Unable to allocate unique name for '%s' tunnel",
   new_chassis_id);
 return;
 }
 
-iface = ovsrec_interface_insert(tc->ovs_txn);
+iface = ovsrec_interface_insert(tc.ovs_txn);
 ovsrec_interface_set_name(iface, port_name);
 ovsrec_interface_set_type(iface, encap->type);
 smap_add(&options, "remote_ip", encap->ip);
@@ -170,23 +199,25 @@ tunnel_add(struct tunnel_ctx *tc, const char 
*new_chassis_id,
 ovsrec_interface_set_options(iface, &options);
 smap_destroy(&options);
 
-port = ovsrec_port_insert(tc->ovs_txn);
+port = ovsrec_port_insert(tc.ovs_txn);
 ovsrec_port_set_n

[ovs-dev] [PATCH v16 2/5] Convert binding_run to incremental processing.

[Ryan Moats]

Ensure that the entire port binding table is processed
when chassis are added/removed or when get_local_iface_ids
finds new ports on the local vswitch.

Side effects:
  - Persist local_datapaths and patch_datapaths across runs so
that changes to either can be used as a trigger to reset
incremental flow processing.
  - Persist all_lports structure

Signed-off-by: Ryan Moats 
---
 ovn/controller/binding.c| 142 +---
 ovn/controller/binding.h|   1 +
 ovn/controller/encaps.c |   4 ++
 ovn/controller/ovn-controller.c |  28 ++--
 ovn/controller/ovn-controller.h |   2 +
 ovn/controller/patch.c  |   3 +-
 6 files changed, 121 insertions(+), 59 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 32fcb85..0cd8666 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -27,6 +27,16 @@
 
 VLOG_DEFINE_THIS_MODULE(binding);
 
+static struct sset all_lports = SSET_INITIALIZER(&all_lports);
+
+static bool process_full_binding = false;
+
+void
+reset_binding_processing(void)
+{
+process_full_binding = true;
+}
+
 void
 binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -72,6 +82,10 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, 
struct shash *lports)
 continue;
 }
 shash_add(lports, iface_id, iface_rec);
+if (!sset_find(&all_lports, iface_id)) {
+sset_add(&all_lports, iface_id);
+reset_binding_processing();
+}
 }
 }
 }
@@ -121,18 +135,57 @@ update_ct_zones(struct sset *lports, struct simap 
*ct_zones,
 }
 }
 
+/* Contains "struct local_datpath" nodes whose hash values are the
+ * row uuids of datapaths with at least one local port binding. */
+struct hmap local_datapaths_by_uuid =
+HMAP_INITIALIZER(&local_datapaths_by_uuid);
+
+static struct local_datapath *
+local_datapath_lookup_by_uuid(const struct uuid *uuid)
+{
+struct hmap_node *ld_node = hmap_first_with_hash(&local_datapaths_by_uuid,
+ uuid_hash(uuid));
+if (ld_node) {
+return CONTAINER_OF(ld_node, struct local_datapath, uuid_hmap_node);
+}
+return NULL;
+}
+
+static void
+remove_local_datapath(struct hmap *local_datapaths, const struct uuid *uuid)
+{
+struct local_datapath *ld = local_datapath_lookup_by_uuid(uuid);
+if (ld) {
+if (ld->logical_port) {
+sset_find_and_delete(&all_lports, ld->logical_port);
+free(ld->logical_port);
+}
+hmap_remove(local_datapaths, &ld->hmap_node);
+hmap_remove(&local_datapaths_by_uuid, &ld->uuid_hmap_node);
+free(ld);
+//reset_flow_processing();
+}
+}
+
 static void
 add_local_datapath(struct hmap *local_datapaths,
-const struct sbrec_port_binding *binding_rec)
+const struct sbrec_port_binding *binding_rec,
+const struct uuid *uuid)
 {
 if (get_local_datapath(local_datapaths,
binding_rec->datapath->tunnel_key)) {
+VLOG_INFO("found local_datapath for %"PRId64, 
binding_rec->datapath->tunnel_key);
 return;
 }
 
+VLOG_INFO("adding local_datapath for %"PRId64, 
binding_rec->datapath->tunnel_key);
 struct local_datapath *ld = xzalloc(sizeof *ld);
+ld->logical_port = xstrdup(binding_rec->logical_port);
 hmap_insert(local_datapaths, &ld->hmap_node,
 binding_rec->datapath->tunnel_key);
+hmap_insert(&local_datapaths_by_uuid, &ld->uuid_hmap_node,
+uuid_hash(uuid));
+//reset_flow_processing();
 }
 
 static void
@@ -146,39 +199,14 @@ update_qos(const struct ovsrec_interface *iface_rec,
 ovsrec_interface_set_ingress_policing_burst(iface_rec, MAX(0, burst));
 }
 
-void
-binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
-const char *chassis_id, struct simap *ct_zones,
-unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
+static void
+consider_local_datapath(struct controller_ctx *ctx, struct shash *lports,
+const struct sbrec_chassis *chassis_rec,
+const struct sbrec_port_binding *binding_rec,
+struct hmap *local_datapaths)
 {
-const struct sbrec_chassis *chassis_rec;
-const struct sbrec_port_binding *binding_rec;
-
-chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id);
-if (!chassis_rec) {
-return;
-}
-
-struct shash lports = SHASH_INITIALIZER(&lports);
-if (br_int) {
-get_local_iface_ids(br_int, &lports);
-} else {
-/* We have no integration bridge, therefore no local logical ports.
- * We'll remove our chassis from all port binding records below. */
-}
-
-struct sset all_lports = SSET_INITIALIZER(&all_lports);
-struct shash_node *node;
-SHASH_FOR_EACH (node, &lports) {
-sset_add(

[ovs-dev] [PATCH v16 0/5] Incremental Processing

[Ryan Moats]

v15->v16: rebase and clean up static declarations

Ryan Moats (5):
  Change encaps_run to work incrementally
  Convert binding_run to incremental processing.
  Persist lport_index and mcgroup_index structures
  Persist ovn flow tables.
  Add incremental proessing to lflow_run and physical_run

 ovn/controller/binding.c| 145 +--
 ovn/controller/binding.h|   1 +
 ovn/controller/encaps.c | 172 ++--
 ovn/controller/lflow.c  | 189 ++---
 ovn/controller/lflow.h  |   6 +-
 ovn/controller/lport.c  | 221 --
 ovn/controller/lport.h  |  22 +-
 ovn/controller/ofctrl.c | 260 
 ovn/controller/ofctrl.h |  18 +-
 ovn/controller/ovn-controller.c |  57 ++-
 ovn/controller/ovn-controller.h |   2 +
 ovn/controller/patch.c  |   7 +-
 ovn/controller/physical.c   | 910 ++--
 ovn/controller/physical.h   |   4 +-
 14 files changed, 1318 insertions(+), 696 deletions(-)

-- 
1.9.1

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

[ovs-dev] [PATCH v16 3/5] Persist lport_index and mcgroup_index structures

[Ryan Moats]

This is preparatory to making physical_run and lflow_run process
incrementally as changes to the data in these structures control
that processing.

Signed-off-by: Ryan Moats 
---
 ovn/controller/lport.c  | 221 +---
 ovn/controller/lport.h  |  22 +++-
 ovn/controller/ovn-controller.c |  16 +--
 3 files changed, 213 insertions(+), 46 deletions(-)

diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
index a7ae320..8210bc9 100644
--- a/ovn/controller/lport.c
+++ b/ovn/controller/lport.c
@@ -17,6 +17,7 @@
 
 #include "lport.h"
 #include "hash.h"
+#include "lflow.h"
 #include "openvswitch/vlog.h"
 #include "ovn/lib/ovn-sb-idl.h"
 
@@ -24,48 +25,112 @@ VLOG_DEFINE_THIS_MODULE(lport);
 
 /* A logical port. */
 struct lport {
-struct hmap_node name_node; /* Index by name. */
-struct hmap_node key_node;  /* Index by (dp_key, port_key). */
+struct hmap_node name_node;  /* Index by name. */
+struct hmap_node key_node;   /* Index by (dp_key, port_key). */
+struct hmap_node uuid_node;  /* Index by row uuid. */
+const struct uuid *uuid;
 const struct sbrec_port_binding *pb;
 };
 
+static bool full_lport_rebuild = false;
+static bool full_mc_rebuild = false;
+
+void
+flag_rebuild_lport_mcast_indexes(void)
+{
+full_lport_rebuild = true;
+full_mc_rebuild = true;
+}
+
 void
-lport_index_init(struct lport_index *lports, struct ovsdb_idl *ovnsb_idl)
+lport_index_init(struct lport_index *lports)
 {
 hmap_init(&lports->by_name);
 hmap_init(&lports->by_key);
+hmap_init(&lports->by_uuid);
+}
 
-const struct sbrec_port_binding *pb;
-SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) {
-if (lport_lookup_by_name(lports, pb->logical_port)) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_WARN_RL(&rl, "duplicate logical port name '%s'",
- pb->logical_port);
-continue;
-}
-
-struct lport *p = xmalloc(sizeof *p);
-hmap_insert(&lports->by_name, &p->name_node,
-hash_string(pb->logical_port, 0));
-hmap_insert(&lports->by_key, &p->key_node,
-hash_int(pb->tunnel_key, pb->datapath->tunnel_key));
-p->pb = pb;
+void
+lport_index_remove(struct lport_index *lports, const struct uuid *uuid)
+{
+const struct lport *port = lport_lookup_by_uuid(lports, uuid);
+if (port) {
+hmap_remove(&lports->by_name, (struct hmap_node *) &port->name_node);
+hmap_remove(&lports->by_key, (struct hmap_node *) &port->key_node);
+hmap_remove(&lports->by_uuid, (struct hmap_node *) &port->uuid_node);
+free((void *) port);
 }
 }
 
 void
-lport_index_destroy(struct lport_index *lports)
+lport_index_clear(struct lport_index *lports)
 {
 /* Destroy all of the "struct lport"s.
  *
- * We don't have to remove the node from both indexes. */
-struct lport *port;
-HMAP_FOR_EACH_POP (port, name_node, &lports->by_name) {
+ * We have to remove the node from all indexes. */
+struct lport *port, *next;
+HMAP_FOR_EACH_SAFE (port, next, name_node, &lports->by_name) {
+hmap_remove(&lports->by_name, &port->name_node);
+hmap_remove(&lports->by_key, &port->key_node);
+hmap_remove(&lports->by_uuid, &port->uuid_node);
 free(port);
 }
+//reset_flow_processing();
+}
+
+static void
+consider_lport_index(struct lport_index *lports,
+ const struct sbrec_port_binding *pb)
+{
+if (lport_lookup_by_name(lports, pb->logical_port)) {
+return;
+}
+
+struct lport *p = xmalloc(sizeof *p);
+hmap_insert(&lports->by_name, &p->name_node,
+hash_string(pb->logical_port, 0));
+hmap_insert(&lports->by_key, &p->key_node,
+hash_int(pb->tunnel_key, pb->datapath->tunnel_key));
+hmap_insert(&lports->by_uuid, &p->uuid_node,
+uuid_hash(&pb->header_.uuid));
+p->uuid = &pb->header_.uuid;
+p->pb = pb;
+//reset_flow_processing();
+}
+
+void
+lport_index_fill(struct lport_index *lports, struct ovsdb_idl *ovnsb_idl)
+{
+const struct sbrec_port_binding *pb;
+if (full_lport_rebuild) {
+lport_index_clear(lports);
+SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) {
+consider_lport_index(lports, pb);
+}
+full_lport_rebuild = false;
+} else {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED (pb, ovnsb_idl) {
+bool is_delete = sbrec_port_binding_row_get_seqno(pb,
+OVSDB_IDL_CHANGE_DELETE) > 0;
+
+if (is_delete) {
+lport_index_remove(lports, &pb->header_.uuid);
+//reset_flow_processing();
+continue;
+}
+consider_lport_index(lports, pb);
+}
+}
+}
+
+void
+lport_index_destroy(struct lport_index *lports)
+{
+lport_index_clear(lports);
 
 hmap_destroy(&lports->by_na

[ovs-dev] [PATCH v16 4/5] Persist ovn flow tables.

[Ryan Moats]

Ensure that ovn flow tables are persisted so that changes to
them chan be applied incrementally - this is a prereq for
making lflow_run and physical_run incremental.

Signed-off-by: Ryan Moats 
---
 ovn/controller/lflow.c  |  26 ++--
 ovn/controller/lflow.h  |   3 +-
 ovn/controller/ofctrl.c | 260 
 ovn/controller/ofctrl.h |  18 ++-
 ovn/controller/ovn-controller.c |   9 +-
 ovn/controller/physical.c   |  59 +
 ovn/controller/physical.h   |   2 +-
 7 files changed, 253 insertions(+), 124 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 96b7c66..c09ed85 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -193,13 +193,13 @@ is_switch(const struct sbrec_datapath_binding *ldp)
 
 }
 
-/* Adds the logical flows from the Logical_Flow table to 'flow_table'. */
+/* Adds the logical flows from the Logical_Flow table to flow tables. */
 static void
 add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
   const struct mcgroup_index *mcgroups,
   const struct hmap *local_datapaths,
   const struct hmap *patched_datapaths,
-  const struct simap *ct_zones, struct hmap *flow_table)
+  const struct simap *ct_zones)
 {
 uint32_t conj_id_ofs = 1;
 
@@ -333,8 +333,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct 
lport_index *lports,
 m->match.flow.conj_id += conj_id_ofs;
 }
 if (!m->n) {
-ofctrl_add_flow(flow_table, ptable, lflow->priority,
-&m->match, &ofpacts);
+ofctrl_add_flow(ptable, lflow->priority, &m->match, &ofpacts,
+&lflow->header_.uuid, true);
 } else {
 uint64_t conj_stubs[64 / 8];
 struct ofpbuf conj;
@@ -349,8 +349,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct 
lport_index *lports,
 dst->clause = src->clause;
 dst->n_clauses = src->n_clauses;
 }
-ofctrl_add_flow(flow_table, ptable, lflow->priority,
-&m->match, &conj);
+ofctrl_add_flow(ptable, lflow->priority, &m->match, &conj,
+&lflow->header_.uuid, true);
 ofpbuf_uninit(&conj);
 }
 }
@@ -375,12 +375,12 @@ put_load(const uint8_t *data, size_t len,
 bitwise_one(&sf->mask, sf->field->n_bytes, ofs, n_bits);
 }
 
-/* Adds an OpenFlow flow to 'flow_table' for each MAC binding in the OVN
+/* Adds an OpenFlow flow to flow tables for each MAC binding in the OVN
  * southbound database, using 'lports' to resolve logical port names to
  * numbers. */
 static void
 add_neighbor_flows(struct controller_ctx *ctx,
-   const struct lport_index *lports, struct hmap *flow_table)
+   const struct lport_index *lports)
 {
 struct ofpbuf ofpacts;
 struct match match;
@@ -416,8 +416,8 @@ add_neighbor_flows(struct controller_ctx *ctx,
 ofpbuf_clear(&ofpacts);
 put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
 
-ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
-&match, &ofpacts);
+ofctrl_add_flow(OFTABLE_MAC_BINDING, 100, &match, &ofpacts,
+&b->header_.uuid, true);
 }
 ofpbuf_uninit(&ofpacts);
 }
@@ -429,11 +429,11 @@ lflow_run(struct controller_ctx *ctx, const struct 
lport_index *lports,
   const struct mcgroup_index *mcgroups,
   const struct hmap *local_datapaths,
   const struct hmap *patched_datapaths,
-  const struct simap *ct_zones, struct hmap *flow_table)
+  const struct simap *ct_zones)
 {
 add_logical_flows(ctx, lports, mcgroups, local_datapaths,
-  patched_datapaths, ct_zones, flow_table);
-add_neighbor_flows(ctx, lports, flow_table);
+  patched_datapaths, ct_zones);
+add_neighbor_flows(ctx, lports);
 }
 
 void
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index a3fc50c..8f8f81a 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -63,8 +63,7 @@ void lflow_run(struct controller_ctx *, const struct 
lport_index *,
const struct mcgroup_index *,
const struct hmap *local_datapaths,
const struct hmap *patched_datapaths,
-   const struct simap *ct_zones,
-   struct hmap *flow_table);
+   const struct simap *ct_zones);
 void lflow_destroy(void);
 
 #endif /* ovn/lflow.h */
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index f537bc0..6863c45 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -16,6 +16,7 @@
 #include 
 #include "byte-order.h"
 #include "dirs.

[ovs-dev] [PATCH v16 5/5] Add incremental proessing to lflow_run and physical_run

[Ryan Moats]

This code changes to allow incremental processing of the
logical flow and physical binding tables whenver possible.

Side Effects:
  - Make flow table persistent in ovn controller
  - Reset lflow processing when adding/removing patch ports

Note: flows created by physical_run for multicast_groups are
*NOT* handled incrementally due to to be solved issues
with GWs and local routers.

Signed-off-by: Ryan Moats 
---
 ovn/controller/binding.c|   7 +-
 ovn/controller/encaps.c |   4 +-
 ovn/controller/lflow.c  | 175 +---
 ovn/controller/lflow.h  |   3 +
 ovn/controller/lport.c  |  12 +-
 ovn/controller/ovn-controller.c |   1 -
 ovn/controller/patch.c  |   4 +-
 ovn/controller/physical.c   | 891 ++--
 ovn/controller/physical.h   |   2 +
 9 files changed, 641 insertions(+), 458 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 0cd8666..8b3f45a 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -15,6 +15,8 @@
 
 #include 
 #include "binding.h"
+#include "lflow.h"
+#include "lport.h"
 
 #include "lib/bitmap.h"
 #include "lib/hmap.h"
@@ -163,7 +165,7 @@ remove_local_datapath(struct hmap *local_datapaths, const 
struct uuid *uuid)
 hmap_remove(local_datapaths, &ld->hmap_node);
 hmap_remove(&local_datapaths_by_uuid, &ld->uuid_hmap_node);
 free(ld);
-//reset_flow_processing();
+reset_flow_processing();
 }
 }
 
@@ -185,7 +187,7 @@ add_local_datapath(struct hmap *local_datapaths,
 binding_rec->datapath->tunnel_key);
 hmap_insert(&local_datapaths_by_uuid, &ld->uuid_hmap_node,
 uuid_hash(uuid));
-//reset_flow_processing();
+reset_flow_processing();
 }
 
 static void
@@ -279,6 +281,7 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 consider_local_datapath(ctx, &lports, chassis_rec, binding_rec,
 local_datapaths);
 }
+flag_rebuild_lport_mcast_indexes();
 process_full_binding = false;
 } else {
 SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl) {
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 0a5b022..d9bd6ee 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -217,7 +217,7 @@ tunnel_add(const struct sbrec_chassis *chassis_rec,
 sset_add(&tc.port_names, port_name);
 free(port_name);
 free(ports);
-// reset_flow_processing();
+reset_flow_processing();
 reset_binding_processing();
 process_full_encaps = true;
 }
@@ -341,7 +341,7 @@ encaps_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 hmap_remove(&tc.tunnel_hmap_by_uuid,
 &port_hash->uuid_node);
 free(port_hash);
-//reset_flow_processing();
+reset_flow_processing();
 reset_binding_processing();
 }
 continue;
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index c09ed85..dcb4235 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -26,6 +26,7 @@
 #include "ovn/lib/expr.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "packets.h"
+#include "physical.h"
 #include "simap.h"
 
 VLOG_DEFINE_THIS_MODULE(lflow);
@@ -35,6 +36,16 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
+static bool full_flow_processing = false;
+static bool full_logical_flow_processing = false;
+static bool full_neighbor_flow_processing = false;
+
+void
+reset_flow_processing(void)
+{
+full_flow_processing = true;
+}
+
 static void
 add_logical_register(struct shash *symtab, enum mf_field_id id)
 {
@@ -193,24 +204,22 @@ 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, const struct lport_index *lports,
-  const struct mcgroup_index *mcgroups,
-  const struct hmap *local_datapaths,
-  const struct hmap *patched_datapaths,
-  const struct simap *ct_zones)
+consider_logical_flow(const struct lport_index *lports,
+  const struct mcgroup_index *mcgroups,
+  const struct sbrec_logical_flow *lflow,
+  const struct hmap *local_datapaths,
+  const struct hmap *patched_datapaths,
+  const struct simap *ct_zones,
+  uint32_t *conj_id_ofs_p,
+  bool is_new)
 {
-uint32_t conj_id_ofs = 1;
-
-const struct sbrec_logical_flow *lflow;
-SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
 /* Determine translation of logical table IDs to p

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[William Tu]

Hi Darrel,

 # Delete namespaces from the running OS
>>  m4_define([DEL_NAMESPACES],
>> [m4_foreach([ns], [$@],
>> -   [ip netns del ns
>> -])
>> +   [if ip netns list | grep ns > /dev/null; then
>> +   ip netns del ns
>> +fi
>> +   ])
>> ]
>>  )
>>
>
> Do we want to suppress an error on deletion in general ?
>
>
No, I think it won't suppress errors on deletion.


> Is the problem wherein ADD_NAMESPACES tries to always remove
> a namespace before adding it ?
>

Yes.


> Is it better to check if ns exists here before calling DEL_NAMESPACES ?
>
>
>
yes we could also add check here:


> m4_define([ADD_NAMESPACES],
>[m4_foreach([ns], [$@],
>
-   [DEL_NAMESPACES(ns)
>
 +  [ //check if ns exists

> AT_CHECK([ip netns add ns || return 77])
> on_exit 'DEL_NAMESPACES(ns)'
>])
>]
> )
>
> Regards,
William
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 1/2] ovn: Support 'dhcp_offer' action inovn-controller

[Ryan Moats]

"dev"  wrote on 04/28/2016 08:50:32 AM:

> From: Numan Siddique 
> To: ovs dev 
> Date: 04/28/2016 08:50 AM
> Subject: [ovs-dev] [PATCH v3 1/2] ovn: Support 'dhcp_offer' action
> in ovn-controller
> Sent by: "dev" 
>
> This patch adds a new OVN action 'dhcp_offer' to support native
> DHCP in OVN.
>
> 'dhcp_offer' takes the DHCP options as input params.
> Eg. dhcp_offer(offerip = 10.0.0.4, router = 10.0.0.1,
>netmask = 255.255.255.0, lease_time = 3600,)
>
> ovn-controller parses this action and adds a NXT_PACKET_IN2
> OF flow with 'pause' flag set and the DHCP options stored in
> 'userdata' field.
>
> When the DHCP packet is received by ovn-controller, it frames a
> new DHCP reply packet with the DHCP options present in the
> 'userdata' field and resumes the packet.
>
> Eg. dhcp_offer(offerip = 10.0.0.4, router = 10.0.0.1,
>netmask = 255.255.255.0, lease_time = 3600,)
>
> A new 'DHCP_Options' table is added in SB DB which stores
> the support DHCP options with DHCP code and type. ovn-northd is
> expected to popule this table.
>
> The next patch will add logical flows with this action.
>
> Signed-Off-by: Numan Siddique 

I've tested this out end-to-end via openstack (with
https://review.openstack.org/#/c/243174) and rally
and it holds up well enough to give it the following thumbs up:

Acked-by: Ryan Moats 
Tested-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 2/2] ovn: Add logical flows to support nativeDHCP

[Ryan Moats]

"dev"  wrote on 04/28/2016 08:51:41 AM:

> From: Numan Siddique 
> To: ovs dev 
> Date: 04/28/2016 08:52 AM
> Subject: [ovs-dev] [PATCH v3 2/2] ovn: Add logical flows to support
> native DHCP
> Sent by: "dev" 
>
> OVN implements a native DHCP support which caters to the common
> use case of providing an IP address to a booting instance by
> providing stateless replies to DHCP requests based on statically
> configured address mappings. To do this it allows a short list of
> DHCP options to be configured and applied at each compute host
> running ovn-controller.
>
> A new table 'Subnet' is added in OVN NB DB to store the DHCP options.
>
> A logical flow is added for each logical port to handle DHCP packets
> using the 'dhcp_offer' action if the CMS has defined DHCP options
> in the 'Subnet' column.
>
> Signed-Off-by: Numan Siddique 

Like patch 1 in the series, I've tested this out end-to-end via openstack
(with https://review.openstack.org/#/c/243174) and rally and it holds up
well enough to give it the following thumbs up:

Acked-by: Ryan Moats 
Tested-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[Darrell Ball]

On Mon, May 2, 2016 at 8:39 AM, William Tu  wrote:

> Hi Darrel,
>
>  # Delete namespaces from the running OS
>>>  m4_define([DEL_NAMESPACES],
>>> [m4_foreach([ns], [$@],
>>> -   [ip netns del ns
>>> -])
>>> +   [if ip netns list | grep ns > /dev/null; then
>>> +   ip netns del ns
>>> +fi
>>> +   ])
>>> ]
>>>  )
>>>
>>
>> Do we want to suppress an error on deletion in general ?
>>
>>
> No, I think it won't suppress errors on deletion.
>


Just to be clear, what the comment means is that if:
1) DEL_NAMESPACE is called and there is no such ns, then this may be an
error with the surrounding code (i.e. a bug) or maybe the test itself.

2) Hence the above code in DEL_NAMESPACE would make the bug less
visible since there would be no visible complaint on trying to delete a ns
that does not exist





>
>
>> Is the problem wherein ADD_NAMESPACES tries to always remove
>> a namespace before adding it ?
>>
>
> Yes.
>
>
>> Is it better to check if ns exists here before calling DEL_NAMESPACES ?
>>
>>
>>
> yes we could also add check here:
>
>
>> m4_define([ADD_NAMESPACES],
>>[m4_foreach([ns], [$@],
>>
> -   [DEL_NAMESPACES(ns)
>>
>  +  [ //check if ns exists
>
>> AT_CHECK([ip netns add ns || return 77])
>> on_exit 'DEL_NAMESPACES(ns)'
>>])
>>]
>> )
>>
>> Regards,
> William
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[William Tu]

Hi Darrell,

>
> Just to be clear, what the comment means is that if:
> 1) DEL_NAMESPACE is called and there is no such ns, then this may be an
> error with the surrounding code (i.e. a bug) or maybe the test itself.
>
> 2) Hence the above code in DEL_NAMESPACE would make the bug less
> visible since there would be no visible complaint on trying to delete a ns
> that does not exist
>
> I see your point. Thanks!

Regards,
William
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] utilities/ovs-ctl.in: Only add_managers with vswitchd

[Aaron Conole]

The ovs-ctl script was changed recently to have per-service start/stop
control. However, when that change was made the add_managers() call was
overlooked. This results in calls to `ovs-ctl --no-ovs-vswitchd start`
telling the ovsdb-server to connect to the remote controllers.

This commit disables the effect of the add_managers call if the
`--no-ovs-vswitchd` argument is given.

Fixes: 7fc28c50c012 ("ovs-ctl: Allow selective start for db and switch")
Signed-off-by: Aaron Conole 
---
 utilities/ovs-ctl.in | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 4aa2999..f494312 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -199,9 +199,11 @@ add_managers () {
 # churn in the database at startup or restart.  (For example, managers
 # won't briefly see empty datapath-id or ofport columns for records that
 # exist at startup.)
-action "Enabling remote OVSDB managers" \
-   ovs-appctl -t ovsdb-server ovsdb-server/add-remote \
-   db:Open_vSwitch,Open_vSwitch,manager_options
+if test X"$OVS_VSWITCHD" = Xyes; then
+action "Enabling remote OVSDB managers" \
+   ovs-appctl -t ovsdb-server ovsdb-server/add-remote \
+   db:Open_vSwitch,Open_vSwitch,manager_options
+fi
 }
 
 do_start_forwarding () {
-- 
2.7.2

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

Re: [ovs-dev] [PATCH 1/2] FAQ: Shift IPFIX into the feature support table.

[Ben Pfaff]

On Thu, Apr 28, 2016 at 02:39:08PM -0700, Joe Stringer wrote:
> Signed-off-by: Joe Stringer 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] FAQ: Update feature table.

[Ben Pfaff]

On Thu, Apr 28, 2016 at 02:39:09PM -0700, Joe Stringer wrote:
> Linux kernel support for features in out-of-tree module no longer depend
> on particular versions, as we only support kernels 3.10-4.3; Connection
> tracking status has changed recently; and NAT is a brand new feature
> with only support in the latest unreleased Linux kernel version.
> 
> Signed-off-by: Joe Stringer 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[Joe Stringer]

On 2 May 2016 at 09:04, Darrell Ball  wrote:
> On Mon, May 2, 2016 at 8:39 AM, William Tu  wrote:
>
>> Hi Darrel,
>>
>>  # Delete namespaces from the running OS
  m4_define([DEL_NAMESPACES],
 [m4_foreach([ns], [$@],
 -   [ip netns del ns
 -])
 +   [if ip netns list | grep ns > /dev/null; then
 +   ip netns del ns
 +fi
 +   ])
 ]
  )

>>>
>>> Do we want to suppress an error on deletion in general ?
>>>
>>>
>> No, I think it won't suppress errors on deletion.
>>
>
>
> Just to be clear, what the comment means is that if:
> 1) DEL_NAMESPACE is called and there is no such ns, then this may be an
> error with the surrounding code (i.e. a bug) or maybe the test itself.
>
> 2) Hence the above code in DEL_NAMESPACE would make the bug less
> visible since there would be no visible complaint on trying to delete a ns
> that does not exist

I was actually just wondering about why we need DEL_NAMESPACES.
Originally, if you did a CTRL+C in the middle of the test, then
cleanup would not properly occur so you'd end up with all of these
test namespaces still existing. By deleting all of the specified
namespaces at the start of ADD_NAMESPACES, it would allow the test to
proceed without forcing the user to go through and delete all of the
namespaces.

However, if we were to queue up namespace deletion using on_exit "ip
netns delete foo" immediately after creation, then the above issue
should not exist, so maybe we could get rid of DELETE_NAMESPACES?

In general I've advocated in the tests that while the test-writer
needs to specify things like ADD_NAMESPACES(), those commands will
queue up the cleanup to ensure that whether the test passes or fails,
the system is left in a tidy state. This means that it is not
necessary inside of tests to add the DELETE_NAMESPACES() towards the
end (which would only execute if the rest of the test was successful).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCHv2 1/9] compat: Add a struct net parameter to l4_pkt_to_tuple.

[Joe Stringer]

Upstream commit:
netfilter: nf_conntrack: Add a struct net parameter to l4_pkt_to_tuple

As gre does not have the srckey in the packet gre_pkt_to_tuple
needs to perform a lookup in it's per network namespace tables.

Pass in the proper network namespace to all pkt_to_tuple
implementations to ensure gre (and any similar protocols) can get this
right.

Signed-off-by: "Eric W. Biederman" 
Signed-off-by: Pablo Neira Ayuso 

Upstream: a31f1adc0948 ("netfilter: nf_conntrack: Add a struct net
parameter to l4_pkt_to_tuple")
Signed-off-by: Joe Stringer 
---
v2: Initial Post.
---
 acinclude.m4|  3 +++
 datapath/conntrack.c|  2 +-
 datapath/linux/Modules.mk   |  1 +
 .../linux/compat/include/net/netfilter/nf_conntrack.h   | 17 +
 4 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 datapath/linux/compat/include/net/netfilter/nf_conntrack.h

diff --git a/acinclude.m4 b/acinclude.m4
index acd7ce7c13cc..398205673951 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -426,6 +426,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
   [tmpl_alloc.*conntrack_zone],
   [OVS_DEFINE([HAVE_NF_CT_TMPL_ALLOC_TAKES_STRUCT_ZONE])])
+  OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack.h],
+  [l3num.*struct.net],
+  [OVS_DEFINE([HAVE_NF_CT_GET_TUPLEPR_TAKES_STRUCT_NET])])
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_zones.h],
   [nf_ct_zone_init])
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_labels.h],
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 0338f9f0c930..c365e2e205a7 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -373,7 +373,7 @@ ovs_ct_expect_find(struct net *net, const struct 
nf_conntrack_zone *zone,
 {
struct nf_conntrack_tuple tuple;
 
-   if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), proto, &tuple))
+   if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), proto, net, 
&tuple))
return NULL;
return __nf_ct_expect_find(net, zone, &tuple);
 }
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 6ab52a76ceff..7e334cbf030a 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -99,6 +99,7 @@ openvswitch_headers += \
linux/compat/include/net/stt.h \
linux/compat/include/net/vrf.h \
linux/compat/include/net/vxlan.h \
+   linux/compat/include/net/netfilter/nf_conntrack.h \
linux/compat/include/net/netfilter/nf_conntrack_core.h \
linux/compat/include/net/netfilter/nf_conntrack_expect.h \
linux/compat/include/net/netfilter/nf_conntrack_labels.h \
diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack.h 
b/datapath/linux/compat/include/net/netfilter/nf_conntrack.h
new file mode 100644
index ..e02e20b28baf
--- /dev/null
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack.h
@@ -0,0 +1,17 @@
+#ifndef _NF_CONNTRACK_WRAPPER_H
+#define _NF_CONNTRACK_WRAPPER_H
+
+#include_next 
+
+#ifndef HAVE_NF_CT_GET_TUPLEPR_TAKES_STRUCT_NET
+static inline bool rpl_nf_ct_get_tuplepr(const struct sk_buff *skb,
+unsigned int nhoff,
+u_int16_t l3num, struct net *net,
+struct nf_conntrack_tuple *tuple)
+{
+   return nf_ct_get_tuplepr(skb, nhoff, l3num, tuple);
+}
+#define nf_ct_get_tuplepr rpl_nf_ct_get_tuplepr
+#endif
+
+#endif /* _NF_CONNTRACK_WRAPPER_H */
-- 
2.1.4

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

[ovs-dev] [PATCHv2 0/9] Backport ovs-conntrack changes.

[Joe Stringer]

This series backports the netfilter/defrag-related changes made recently
upstream to our compat code, which should bring conntrack.c up-to-date just
prior to the NAT changes. Patch 5 introduced some breakage which is fixed in
patches 6 and 7; I have left these separate to mirror the upstream commits.

Tested using kmod tests on Ubuntu 3.13.0-24, 3.16.0-70, 3.19.0-58, and
4.2.0-35, and RHEL 3.10.0-327, plus compilation against vanilla
kernel targets on Travis:
https://travis-ci.org/joestringer/openvswitch/builds/127290050

v2:
- 3 new backport patches, bringing the series up to date with upstream prior
  to the NAT series.
- Added a final patch to document the defrag backport strategy in v4 and v6.
- Fixed upstream commit references to use the correct IDs.
v1:
- Initial Post

Joe Stringer (9):
  compat: Add a struct net parameter to l4_pkt_to_tuple.
  compat: ipv4: Pass struct net into ip_defrag.
  compat: ipv6: Pass struct net into nf_ct_frag6_gather.
  compat: nf_defrag_ipv6: avoid/free clone operations.
  compat: nf_defrag_ipv6: avoid nf_iterate recursion.
  compat: nf_defrag_ipv6: fix NULL deref panic.
  datapath: Orphan skbs before IPv6 defrag
  datapath: Fix template leak in error cases.
  compat: Document nf_defrag_ipv[46] backport.

 acinclude.m4   |   5 +
 datapath/conntrack.c   |  36 ++---
 datapath/linux/Modules.mk  |   1 +
 datapath/linux/compat/include/net/ip.h |  15 +-
 .../include/net/netfilter/ipv6/nf_defrag_ipv6.h|  21 ++-
 .../compat/include/net/netfilter/nf_conntrack.h|  17 ++
 datapath/linux/compat/ip_fragment.c|   3 +-
 datapath/linux/compat/nf_conntrack_reasm.c | 172 +
 8 files changed, 133 insertions(+), 137 deletions(-)
 create mode 100644 datapath/linux/compat/include/net/netfilter/nf_conntrack.h

-- 
2.1.4

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

[ovs-dev] [PATCHv2 4/9] compat: nf_defrag_ipv6: avoid/free clone operations.

[Joe Stringer]

Upstream commit:
netfilter: ipv6: nf_defrag: avoid/free clone operations

commit 6aafeef03b9d9ecf
("netfilter: push reasm skb through instead of original frag skbs")
changed ipv6 defrag to not use the original skbs anymore.

So rather than keeping the original skbs around just to discard them
afterwards just use the original skbs directly for the fraglist of
the newly assembled skb and remove the extra clone/free operations.

The skb that completes the fragment queue is morphed into a the
reassembled one instead, just like ipv4 defrag.

openvswitch doesn't need any additional skb_morph magic anymore to deal
with this situation so just remove that.

A followup patch can then also remove the NF_HOOK (re)invocation in
the ipv6 netfilter defrag hook.

Cc: Joe Stringer 
Signed-off-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 

Upstream: 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free clone 
operations")
Signed-off-by: Joe Stringer 
---
v2: No changes.
v1: Initial Post.
---
 datapath/conntrack.c   |  14 ---
 .../include/net/netfilter/ipv6/nf_defrag_ipv6.h|  12 +--
 datapath/linux/compat/nf_conntrack_reasm.c | 104 -
 3 files changed, 46 insertions(+), 84 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 6cf97942ccf2..ea50ff28f01c 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -337,21 +337,7 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
if (!reasm)
return -EINPROGRESS;
 
-   if (skb == reasm) {
-   kfree_skb(skb);
-   return -EINVAL;
-   }
-
-   /* Don't free 'skb' even though it is one of the original
-* fragments, as we're going to morph it into the head.
-*/
-   skb_get(skb);
-   nf_ct_frag6_consume_orig(reasm);
-
key->ip.proto = ipv6_hdr(reasm)->nexthdr;
-   skb_morph(skb, reasm);
-   skb->next = reasm->next;
-   consume_skb(reasm);
ovs_cb.dp_cb.mru = IP6CB(skb)->frag_max_size;
 #endif /* IP frag support */
} else {
diff --git a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h 
b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
index fe99ced37227..a3b86dab2c9c 100644
--- a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
+++ b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
@@ -16,17 +16,17 @@
 #define OVS_NF_DEFRAG6_BACKPORT 1
 struct sk_buff *rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff *skb,
   u32 user);
+#define nf_ct_frag6_gather rpl_nf_ct_frag6_gather
+#endif /* HAVE_NF_CT_FRAG6_CONSUME_ORIG */
+
+#ifdef OVS_NF_DEFRAG6_BACKPORT
 int __init rpl_nf_ct_frag6_init(void);
 void rpl_nf_ct_frag6_cleanup(void);
-void rpl_nf_ct_frag6_consume_orig(struct sk_buff *skb);
-#define nf_ct_frag6_gather rpl_nf_ct_frag6_gather
-#else /* HAVE_NF_CT_FRAG6_CONSUME_ORIG */
+#else /* !OVS_NF_DEFRAG6_BACKPORT */
 static inline int __init rpl_nf_ct_frag6_init(void) { return 0; }
 static inline void rpl_nf_ct_frag6_cleanup(void) { }
-static inline void rpl_nf_ct_frag6_consume_orig(struct sk_buff *skb) { }
-#endif /* HAVE_NF_CT_FRAG6_CONSUME_ORIG */
+#endif /* OVS_NF_DEFRAG6_BACKPORT */
 #define nf_ct_frag6_init rpl_nf_ct_frag6_init
 #define nf_ct_frag6_cleanup rpl_nf_ct_frag6_cleanup
-#define nf_ct_frag6_consume_orig rpl_nf_ct_frag6_consume_orig
 
 #endif /* __NF_DEFRAG_IPV6_WRAPPER_H */
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index 701bd15d8efd..c6dc7ebec5b5 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -62,7 +62,6 @@ struct nf_ct_frag6_skb_cb
 {
struct inet6_skb_parm   h;
int offset;
-   struct sk_buff  *orig;
 };
 
 #define NFCT_FRAG6_CB(skb) ((struct nf_ct_frag6_skb_cb*)((skb)->cb))
@@ -94,12 +93,6 @@ static unsigned int nf_hashfn(struct inet_frag_queue *q)
return nf_hash_frag(nq->id, &nq->saddr, &nq->daddr);
 }
 
-static void nf_skb_free(struct sk_buff *skb)
-{
-   if (NFCT_FRAG6_CB(skb)->orig)
-   kfree_skb(NFCT_FRAG6_CB(skb)->orig);
-}
-
 static void nf_ct_frag6_expire(unsigned long data)
 {
struct frag_queue *fq;
@@ -300,9 +293,9 @@ err:
  * the last and the first frames arrived and all the bits are here.
  */
 static struct sk_buff *
-nf_ct_frag6_reasm(struct frag_queue *fq, struct net_device *dev)
+nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff *prev,  struct 
net_device *dev)
 {
-   struct sk_buff *fp, *op, *head = fq->q.fragments;
+   struct sk_buff *fp, *head = fq->q.fragments;
intpayload_len;
u8 ecn;
 
@@ -

[ovs-dev] [PATCHv2 2/9] compat: ipv4: Pass struct net into ip_defrag.

[Joe Stringer]

Upstream commit:
ipv4: Pass struct net into ip_defrag and ip_check_defrag

The function ip_defrag is called on both the input and the output
paths of the networking stack.  In particular conntrack when it is
tracking outbound packets from the local machine calls ip_defrag.

So add a struct net parameter and stop making ip_defrag guess which
network namespace it needs to defragment packets in.

Signed-off-by: "Eric W. Biederman" 
Acked-by: Pablo Neira Ayuso 
Signed-off-by: David S. Miller 

Upstream: 19bcf9f203c8 ("ipv4: Pass struct net into ip_defrag and 
ip_check_defrag")
Signed-off-by: Joe Stringer 
---
v2: Initial Post.
---
 acinclude.m4   | 2 ++
 datapath/conntrack.c   | 2 +-
 datapath/linux/compat/include/net/ip.h | 8 ++--
 datapath/linux/compat/ip_fragment.c| 3 +--
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 398205673951..6cfb1e53ef7f 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -372,6 +372,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
 
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [inet_get_local_port_range.*net],
   [OVS_DEFINE([HAVE_INET_GET_LOCAL_PORT_RANGE_USING_NET])])
+  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_defrag.*net],
+  [OVS_DEFINE([HAVE_IP_DEFRAG_TAKES_NET])])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_do_fragment])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_is_fragment])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_skb_dst_mtu])
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index c365e2e205a7..548a05fc244e 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -322,7 +322,7 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
int err;
 
memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
-   err = ip_defrag(skb, user);
+   err = ip_defrag(net, skb, user);
if (err)
return err;
 
diff --git a/datapath/linux/compat/include/net/ip.h 
b/datapath/linux/compat/include/net/ip.h
index 54532de205c3..0fb13913eecd 100644
--- a/datapath/linux/compat/include/net/ip.h
+++ b/datapath/linux/compat/include/net/ip.h
@@ -116,7 +116,7 @@ static inline int rpl_ip_do_fragment(struct sock *sk, 
struct sk_buff *skb,
 #define ip_do_fragment rpl_ip_do_fragment
 #endif /* IP_DO_FRAGMENT */
 
-int rpl_ip_defrag(struct sk_buff *skb, u32 user);
+int rpl_ip_defrag(struct net *net, struct sk_buff *skb, u32 user);
 #define ip_defrag rpl_ip_defrag
 int __init rpl_ipfrag_init(void);
 void rpl_ipfrag_fini(void);
@@ -127,10 +127,14 @@ void rpl_ipfrag_fini(void);
  * ("inet: frag: Always orphan skbs inside ip_defrag()"), but it should be
  * always included in kernels 4.5+. */
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4,5,0)
-static inline int rpl_ip_defrag(struct sk_buff *skb, u32 user)
+static inline int rpl_ip_defrag(struct net *net, struct sk_buff *skb, u32 user)
 {
skb_orphan(skb);
+#ifndef HAVE_IP_DEFRAG_TAKES_NET
return ip_defrag(skb, user);
+#else
+   return ip_defrag(net, skb, user);
+#endif
 }
 #define ip_defrag rpl_ip_defrag
 #endif
diff --git a/datapath/linux/compat/ip_fragment.c 
b/datapath/linux/compat/ip_fragment.c
index 66b56aa4cfda..8d01088abc0a 100644
--- a/datapath/linux/compat/ip_fragment.c
+++ b/datapath/linux/compat/ip_fragment.c
@@ -674,11 +674,10 @@ out_fail:
 }
 
 /* Process an incoming IP datagram fragment. */
-int rpl_ip_defrag(struct sk_buff *skb, u32 user)
+int rpl_ip_defrag(struct net *net, struct sk_buff *skb, u32 user)
 {
struct net_device *dev = skb->dev ? : skb_dst(skb)->dev;
int vif = vrf_master_ifindex_rcu(dev);
-   struct net *net = dev_net(dev);
struct ipq *qp;
 
IP_INC_STATS_BH(net, IPSTATS_MIB_REASMREQDS);
-- 
2.1.4

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

[ovs-dev] [PATCHv2 7/9] datapath: Orphan skbs before IPv6 defrag

[Joe Stringer]

Upstream commit:
openvswitch: Orphan skbs before IPv6 defrag

This is the IPv6 counterpart to commit 8282f27449bf ("inet: frag: Always
orphan skbs inside ip_defrag()").

Prior to commit 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free
clone operations"), ipv6 fragments sent to nf_ct_frag6_gather() would be
cloned (implicitly orphaning) prior to queueing for reassembly. As such,
when the IPv6 message is eventually reassembled, the skb->sk for all
fragments would be NULL. After that commit was introduced, rather than
cloning, the original skbs were queued directly without orphaning. The
end result is that all frags except for the first and last may have a
socket attached.

This commit explicitly orphans such skbs during nf_ct_frag6_gather() to
prevent BUG_ON(skb->sk) during a later call to ip6_fragment().

kernel BUG at net/ipv6/ip6_output.c:631!
[...]
Call Trace:
 
 [] ? __lock_acquire+0x927/0x20a0
 [] ? do_output.isra.28+0x1b0/0x1b0 [openvswitch]
 [] ? __lock_is_held+0x52/0x70
 [] ovs_fragment+0x1f7/0x280 [openvswitch]
 [] ? mark_held_locks+0x75/0xa0
 [] ? _raw_spin_unlock_irqrestore+0x36/0x50
 [] ? dst_discard_out+0x20/0x20
 [] ? dst_ifdown+0x80/0x80
 [] do_output.isra.28+0xf3/0x1b0 [openvswitch]
 [] do_execute_actions+0x709/0x12c0 [openvswitch]
 [] ? ovs_flow_stats_update+0x74/0x1e0 [openvswitch]
 [] ? ovs_flow_stats_update+0xa1/0x1e0 [openvswitch]
 [] ? _raw_spin_unlock+0x27/0x40
 [] ovs_execute_actions+0x45/0x120 [openvswitch]
 [] ovs_dp_process_packet+0x85/0x150 [openvswitch]
 [] ? _raw_spin_unlock+0x27/0x40
 [] ovs_execute_actions+0xc4/0x120 [openvswitch]
 [] ovs_dp_process_packet+0x85/0x150 [openvswitch]
 [] ? key_extract+0x442/0xc10 [openvswitch]
 [] ovs_vport_receive+0x5d/0xb0 [openvswitch]
 [] ? __lock_acquire+0x927/0x20a0
 [] ? __lock_acquire+0x927/0x20a0
 [] ? __lock_acquire+0x927/0x20a0
 [] ? _raw_spin_unlock_irqrestore+0x36/0x50
 [] internal_dev_xmit+0x6d/0x150 [openvswitch]
 [] ? internal_dev_xmit+0x5/0x150 [openvswitch]
 [] dev_hard_start_xmit+0x2df/0x660
 [] ? validate_xmit_skb.isra.105.part.106+0x1a/0x2b0
 [] __dev_queue_xmit+0x8f5/0x950
 [] ? __dev_queue_xmit+0x50/0x950
 [] ? mark_held_locks+0x75/0xa0
 [] dev_queue_xmit+0x10/0x20
 [] neigh_resolve_output+0x178/0x220
 [] ? ip6_finish_output2+0x219/0x7b0
 [] ip6_finish_output2+0x219/0x7b0
 [] ? ip6_finish_output2+0x65/0x7b0
 [] ? ip_idents_reserve+0x6b/0x80
 [] ? ip6_fragment+0x93f/0xc50
 [] ip6_fragment+0xba1/0xc50
 [] ? ip6_flush_pending_frames+0x40/0x40
 [] ip6_finish_output+0xcb/0x1d0
 [] ip6_output+0x5f/0x1a0
 [] ? ip6_fragment+0xc50/0xc50
 [] ip6_local_out+0x3d/0x80
 [] ip6_send_skb+0x2f/0xc0
 [] ip6_push_pending_frames+0x4d/0x50
 [] icmpv6_push_pending_frames+0xac/0xe0
 [] icmpv6_echo_reply+0x42e/0x500
 [] icmpv6_rcv+0x4cf/0x580
 [] ip6_input_finish+0x1a7/0x690
 [] ? ip6_input_finish+0x5/0x690
 [] ip6_input+0x30/0xa0
 [] ? ip6_rcv_finish+0x1a0/0x1a0
 [] ip6_rcv_finish+0x4e/0x1a0
 [] ipv6_rcv+0x45f/0x7c0
 [] ? ipv6_rcv+0x36/0x7c0
 [] ? ip6_make_skb+0x1c0/0x1c0
 [] __netif_receive_skb_core+0x229/0xb80
 [] ? mark_held_locks+0x75/0xa0
 [] ? process_backlog+0x6f/0x230
 [] __netif_receive_skb+0x16/0x70
 [] process_backlog+0x78/0x230
 [] ? process_backlog+0xdd/0x230
 [] net_rx_action+0x203/0x480
 [] ? mark_held_locks+0x75/0xa0
 [] __do_softirq+0xde/0x49f
 [] ? ip6_finish_output2+0x228/0x7b0
 [] do_softirq_own_stack+0x1c/0x30
 
 [] do_softirq.part.18+0x3b/0x40
 [] __local_bh_enable_ip+0xb6/0xc0
 [] ip6_finish_output2+0x251/0x7b0
 [] ? ip6_fragment+0xba1/0xc50
 [] ? ip_idents_reserve+0x6b/0x80
 [] ? ip6_fragment+0x93f/0xc50
 [] ip6_fragment+0xba1/0xc50
 [] ? ip6_flush_pending_frames+0x40/0x40
 [] ip6_finish_output+0xcb/0x1d0
 [] ip6_output+0x5f/0x1a0
 [] ? ip6_fragment+0xc50/0xc50
 [] ip6_local_out+0x3d/0x80
 [] ip6_send_skb+0x2f/0xc0
 [] ip6_push_pending_frames+0x4d/0x50
 [] rawv6_sendmsg+0xa28/0xe30
 [] ? inet_sendmsg+0xc7/0x1d0
 [] inet_sendmsg+0x106/0x1d0
 [] ? inet_sendmsg+0x5/0x1d0
 [] sock_sendmsg+0x38/0x50
 [] SYSC_sendto+0xf6/0x170
 [] ? trace_hardirqs_on_thunk+0x1b/0x1d
 [] SyS_sendto+0xe/0x10
 [] entry_SYSCALL_64_fastpath+0x18/0xa8
Code: 06 48 83 3f 00 75 26 48 8b 87 d8 00 00 00 2b 87 d0 00 00 00 48 39 d0 
72 14 8b 87 e4 00 00 00 83 f8 01 75 09 48 83 7f 18 00 74 9a <0f> 0b 41 8b 86 cc 
00 00 00 49 8#
RIP  [] ip6_fragment+0x73a/0xc50
 RSP 

Fixes: 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free clone
operations")
Reported-by: Daniele Di Proietto 
Signed-off-by: Joe Stringer 
Signed-off-by: David S. Miller 

Upstream: 49e261a8a21e ("openvswitch: Orphan skbs befor

[ovs-dev] [PATCHv2 3/9] compat: ipv6: Pass struct net into nf_ct_frag6_gather.

[Joe Stringer]

Upstream commit:
ipv6: Pass struct net into nf_ct_frag6_gather

The function nf_ct_frag6_gather is called on both the input and the
output paths of the networking stack.  In particular ipv6_defrag which
calls nf_ct_frag6_gather is called from both the the PRE_ROUTING chain
on input and the LOCAL_OUT chain on output.

The addition of a net parameter makes it explicit which network
namespace the packets are being reassembled in, and removes the need
for nf_ct_frag6_gather to guess.

Signed-off-by: "Eric W. Biederman" 
Acked-by: Pablo Neira Ayuso 
Signed-off-by: David S. Miller 

Upstream: b72775977c39 ("ipv6: Pass struct net into nf_ct_frag6_gather")
Signed-off-by: Joe Stringer 
---
v2: No changes.
v1: Initial Post.
---
 datapath/conntrack.c  | 2 +-
 datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h | 3 ++-
 datapath/linux/compat/nf_conntrack_reasm.c| 5 ++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 548a05fc244e..6cf97942ccf2 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -333,7 +333,7 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
struct sk_buff *reasm;
 
memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
-   reasm = nf_ct_frag6_gather(skb, user);
+   reasm = nf_ct_frag6_gather(net, skb, user);
if (!reasm)
return -EINPROGRESS;
 
diff --git a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h 
b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
index 416cafff03b1..fe99ced37227 100644
--- a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
+++ b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
@@ -14,7 +14,8 @@
 #if defined(HAVE_NF_CT_FRAG6_CONSUME_ORIG) || \
 defined(HAVE_NF_CT_FRAG6_OUTPUT)
 #define OVS_NF_DEFRAG6_BACKPORT 1
-struct sk_buff *rpl_nf_ct_frag6_gather(struct sk_buff *skb, u32 user);
+struct sk_buff *rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff *skb,
+  u32 user);
 int __init rpl_nf_ct_frag6_init(void);
 void rpl_nf_ct_frag6_cleanup(void);
 void rpl_nf_ct_frag6_consume_orig(struct sk_buff *skb);
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index ef29115b6fbd..701bd15d8efd 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -487,12 +487,11 @@ find_prev_fhdr(struct sk_buff *skb, u8 *prevhdrp, int 
*prevhoff, int *fhoff)
return 0;
 }
 
-struct sk_buff *rpl_nf_ct_frag6_gather(struct sk_buff *skb, u32 user)
+struct sk_buff *rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff *skb,
+  u32 user)
 {
struct sk_buff *clone;
struct net_device *dev = skb->dev;
-   struct net *net = skb_dst(skb) ? dev_net(skb_dst(skb)->dev)
-  : dev_net(skb->dev);
struct frag_hdr *fhdr;
struct frag_queue *fq;
struct ipv6hdr *hdr;
-- 
2.1.4

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

[ovs-dev] [PATCHv2 6/9] compat: nf_defrag_ipv6: fix NULL deref panic.

[Joe Stringer]

Upstream commit:
netfilter: ipv6: nf_defrag: fix NULL deref panic

Valdis reports NULL deref in nf_ct_frag6_gather.
Problem is bogus use of skb_queue_walk() -- we miss first skb in the list
since we start with head->next instead of head.

In case the element we're looking for was head->next we won't find
a result and then trip over NULL iter.

(defrag uses plain NULL-terminated list rather than one terminated by
 head-of-list-pointer, which is what skb_queue_walk expects).

Fixes: 029f7f3b8701cc7a ("netfilter: ipv6: nf_defrag: avoid/free clone 
operations")
Reported-by: Valdis Kletnieks 
Tested-by: Valdis Kletnieks 
Signed-off-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 

Upstream: e97ac12859db ("netfilter: ipv6: nf_defrag: fix NULL deref panic")
Signed-off-by: Joe Stringer 
---
v2: No changes.
v1: Initial Post.
---
 datapath/linux/compat/nf_conntrack_reasm.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index 31c47b487356..5000351e9664 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -365,11 +365,14 @@ nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff 
*prev,  struct net_devic
return false;
 
fp->next = prev->next;
-   skb_queue_walk(head, iter) {
-   if (iter->next != prev)
-   continue;
-   iter->next = fp;
-   break;
+
+   iter = head;
+   while (iter) {
+   if (iter->next == prev) {
+   iter->next = fp;
+   break;
+   }
+   iter = iter->next;
}
 
skb_morph(prev, head);
-- 
2.1.4

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

[ovs-dev] [PATCHv2 5/9] compat: nf_defrag_ipv6: avoid nf_iterate recursion.

[Joe Stringer]

Upstream commit:
netfilter: ipv6: avoid nf_iterate recursion

The previous patch changed nf_ct_frag6_gather() to morph reassembled skb
with the previous one.

This means that the return value is always NULL or the skb argument.
So change it to an err value.

Instead of invoking NF_HOOK recursively with threshold to skip 
already-called hooks
we can now just return NF_ACCEPT to move on to the next hook except for
-EINPROGRESS (which means skb has been queued for reassembly), in which 
case we
return NF_STOLEN.

Signed-off-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 

Upstream: daaa7d647f81 ("netfilter: ipv6: avoid nf_iterate recursion")
Signed-off-by: Joe Stringer 
---
v2: No changes.
v1: Initial Post.
---
 datapath/conntrack.c   | 11 ++--
 .../include/net/netfilter/ipv6/nf_defrag_ipv6.h|  3 +-
 datapath/linux/compat/nf_conntrack_reasm.c | 72 ++
 3 files changed, 37 insertions(+), 49 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index ea50ff28f01c..813094c7ecf6 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -311,6 +311,7 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
u16 zone, struct sk_buff *skb)
 {
struct ovs_gso_cb ovs_cb = *OVS_GSO_CB(skb);
+   int err;
 
if (!skb->dev) {
OVS_NLERR(true, "%s: skb has no dev; dropping", __func__);
@@ -319,7 +320,6 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
 
if (key->eth.type == htons(ETH_P_IP)) {
enum ip_defrag_users user = IP_DEFRAG_CONNTRACK_IN + zone;
-   int err;
 
memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
err = ip_defrag(net, skb, user);
@@ -330,14 +330,13 @@ static int handle_fragments(struct net *net, struct 
sw_flow_key *key,
 #if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
} else if (key->eth.type == htons(ETH_P_IPV6)) {
enum ip6_defrag_users user = IP6_DEFRAG_CONNTRACK_IN + zone;
-   struct sk_buff *reasm;
 
memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
-   reasm = nf_ct_frag6_gather(net, skb, user);
-   if (!reasm)
-   return -EINPROGRESS;
+   err = nf_ct_frag6_gather(net, skb, user);
+   if (err)
+   return err;
 
-   key->ip.proto = ipv6_hdr(reasm)->nexthdr;
+   key->ip.proto = ipv6_hdr(skb)->nexthdr;
ovs_cb.dp_cb.mru = IP6CB(skb)->frag_max_size;
 #endif /* IP frag support */
} else {
diff --git a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h 
b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
index a3b86dab2c9c..dc440db99924 100644
--- a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
+++ b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
@@ -14,8 +14,7 @@
 #if defined(HAVE_NF_CT_FRAG6_CONSUME_ORIG) || \
 defined(HAVE_NF_CT_FRAG6_OUTPUT)
 #define OVS_NF_DEFRAG6_BACKPORT 1
-struct sk_buff *rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff *skb,
-  u32 user);
+int rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user);
 #define nf_ct_frag6_gather rpl_nf_ct_frag6_gather
 #endif /* HAVE_NF_CT_FRAG6_CONSUME_ORIG */
 
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c 
b/datapath/linux/compat/nf_conntrack_reasm.c
index c6dc7ebec5b5..31c47b487356 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -285,14 +285,15 @@ err:
 
 /*
  * Check if this packet is complete.
- * Returns NULL on failure by any reason, and pointer
- * to current nexthdr field in reassembled frame.
  *
  * It is called with locked fq, and caller must check that
  * queue is eligible for reassembly i.e. it is not COMPLETE,
  * the last and the first frames arrived and all the bits are here.
+ *
+ * returns true if *prev skb has been transformed into the reassembled
+ * skb, false otherwise.
  */
-static struct sk_buff *
+static bool
 nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff *prev,  struct 
net_device *dev)
 {
struct sk_buff *fp, *head = fq->q.fragments;
@@ -306,22 +307,21 @@ nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff 
*prev,  struct net_devic
 
ecn = ip_frag_ecn_table[fq->ecn];
if (unlikely(ecn == 0xff))
-   goto out_fail;
+   return false;
 
/* Unfragmented part is taken from the first segment. */
payload_len = ((head->data - skb_network_header(head)) -
   sizeof(struct ipv6hdr) + fq->q.len -
   sizeof(struct frag_hdr));
if (payload_len > IPV6_MAXPLEN) {
-   pr_debug("payload len is too lar

[ovs-dev] [PATCHv2 9/9] compat: Document nf_defrag_ipv[46] backport.

[Joe Stringer]

Document how the IP(6) defrag backport works, and do minor style cleanups.

Signed-off-by: Joe Stringer 
---
v2: Initial Post.
---
 datapath/linux/compat/include/net/ip.h|  7 +++
 .../linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h  | 11 +--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/datapath/linux/compat/include/net/ip.h 
b/datapath/linux/compat/include/net/ip.h
index 0fb13913eecd..145529ecbbe1 100644
--- a/datapath/linux/compat/include/net/ip.h
+++ b/datapath/linux/compat/include/net/ip.h
@@ -116,6 +116,10 @@ static inline int rpl_ip_do_fragment(struct sock *sk, 
struct sk_buff *skb,
 #define ip_do_fragment rpl_ip_do_fragment
 #endif /* IP_DO_FRAGMENT */
 
+/* If backporting IP defrag, then init/exit functions need to be called from
+ * compat_{in,ex}it() to prepare the backported fragmentation cache. In this
+ * case we declare the functions which are defined in
+ * datapath/linux/compat/ip_fragment.c. */
 int rpl_ip_defrag(struct net *net, struct sk_buff *skb, u32 user);
 #define ip_defrag rpl_ip_defrag
 int __init rpl_ipfrag_init(void);
@@ -139,6 +143,9 @@ static inline int rpl_ip_defrag(struct net *net, struct 
sk_buff *skb, u32 user)
 #define ip_defrag rpl_ip_defrag
 #endif
 
+/* If we can use upstream defrag then we can rely on the upstream
+ * defrag module to init/exit correctly. In this case the calls in
+ * compat_{in,ex}it() can be no-ops. */
 static inline int rpl_ipfrag_init(void) { return 0; }
 static inline void rpl_ipfrag_fini(void) { }
 #endif /* HAVE_CORRECT_MRU_HANDLING */
diff --git a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h 
b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
index dc440db99924..09fe78101f3a 100644
--- a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
+++ b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
@@ -16,9 +16,16 @@
 #define OVS_NF_DEFRAG6_BACKPORT 1
 int rpl_nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user);
 #define nf_ct_frag6_gather rpl_nf_ct_frag6_gather
-#endif /* HAVE_NF_CT_FRAG6_CONSUME_ORIG */
 
-#ifdef OVS_NF_DEFRAG6_BACKPORT
+/* If backporting IPv6 defrag, then init/exit functions need to be called from
+ * compat_{in,ex}it() to prepare the backported fragmentation cache. In this
+ * case we declare the functions which are defined in
+ * datapath/linux/compat/nf_conntrack_reasm.c.
+ *
+ * Otherwise, if we can use upstream defrag then we can rely on the upstream
+ * nf_defrag_ipv6 module to init/exit correctly. In this case the calls in
+ * compat_{in,ex}it() can be no-ops.
+ */
 int __init rpl_nf_ct_frag6_init(void);
 void rpl_nf_ct_frag6_cleanup(void);
 #else /* !OVS_NF_DEFRAG6_BACKPORT */
-- 
2.1.4

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

[ovs-dev] [PATCHv2 8/9] datapath: Fix template leak in error cases.

[Joe Stringer]

Upstream commit:
openvswitch: Fix template leak in error cases.

Commit 2f3ab9f9fc23 ("openvswitch: Fix helper reference leak") fixed a
reference leak on helper objects, but inadvertently introduced a leak on
the ct template.

Previously, ct_info.ct->general.use was initialized to 0 by
nf_ct_tmpl_alloc() and only incremented when ovs_ct_copy_action()
returned successful. If an error occurred while adding the helper or
adding the action to the actions buffer, the __ovs_ct_free_action()
cleanup would use nf_ct_put() to free the entry; However, this relies on
atomic_dec_and_test(ct_info.ct->general.use). This reference must be
incremented first, or nf_ct_put() will never free it.

Fix the issue by acquiring a reference to the template immediately after
allocation.

Fixes: cae3a2627520 ("openvswitch: Allow attaching helpers to ct action")
Fixes: 2f3ab9f9fc23 ("openvswitch: Fix helper reference leak")
Signed-off-by: Joe Stringer 
Signed-off-by: David S. Miller 

Upstream: 90c7afc96cbb ("openvswitch: Fix template leak in error cases.")
Fixes: 11251c170d92 ("datapath: Allow attaching helpers to ct action")
Signed-off-by: Joe Stringer 
---
v2: Initial Post.
---
 datapath/conntrack.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 49c299b5bd55..13cacf4231d7 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -695,6 +695,10 @@ int ovs_ct_copy_action(struct net *net, const struct 
nlattr *attr,
OVS_NLERR(log, "Failed to allocate conntrack template");
return -ENOMEM;
}
+
+   __set_bit(IPS_CONFIRMED_BIT, &ct_info.ct->status);
+   nf_conntrack_get(&ct_info.ct->ct_general);
+
if (helper) {
err = ovs_ct_add_helper(&ct_info, helper, key, log);
if (err)
@@ -706,8 +710,6 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr 
*attr,
if (err)
goto err_free_ct;
 
-   __set_bit(IPS_CONFIRMED_BIT, &ct_info.ct->status);
-   nf_conntrack_get(&ct_info.ct->ct_general);
return 0;
 err_free_ct:
__ovs_ct_free_action(&ct_info);
-- 
2.1.4

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

Re: [ovs-dev] [PATCH 1/2] FAQ: Shift IPFIX into the feature supporttable.

[Ryan Moats]

"dev"  wrote on 04/28/2016 04:39:08 PM:

> From: Joe Stringer 
> To: dev@openvswitch.org
> Date: 04/28/2016 04:39 PM
> Subject: [ovs-dev] [PATCH 1/2] FAQ: Shift IPFIX into the feature
> support table.
> Sent by: "dev" 
>
> Signed-off-by: Joe Stringer 

Catching up with my review queue...

Acked-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] FAQ: Update feature table.

[Ryan Moats]

"dev"  wrote on 04/28/2016 04:39:09 PM:

> From: Joe Stringer 
> To: dev@openvswitch.org
> Date: 04/28/2016 04:39 PM
> Subject: [ovs-dev] [PATCH 2/2] FAQ: Update feature table.
> Sent by: "dev" 
>
> Linux kernel support for features in out-of-tree module no longer depend
> on particular versions, as we only support kernels 3.10-4.3; Connection
> tracking status has changed recently; and NAT is a brand new feature
> with only support in the latest unreleased Linux kernel version.
>
> Signed-off-by: Joe Stringer 

Catching up with my review queue...

Acked-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] compat: skbuff: Remove references to old kernels.

[Joe Stringer]

On 2 May 2016 at 00:28, Simon Horman  wrote:
> On Thu, Apr 28, 2016 at 06:09:04PM -0700, Joe Stringer wrote:
>> Since commit f2ab1536ddbc ("compat: Backport conntrack strictly to
>> v3.10+."), we haven't supported these kernel versions. Remove the old
>> code.
>>
>> Signed-off-by: Joe Stringer 
>
> Acked-by: Simon Horman 

Thanks for the review, applied to msater.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] FAQ: Update feature table.

[Joe Stringer]

On 2 May 2016 at 11:29, Ryan Moats  wrote:
> "dev"  wrote on 04/28/2016 04:39:09 PM:
>
>> From: Joe Stringer 
>> To: dev@openvswitch.org
>> Date: 04/28/2016 04:39 PM
>> Subject: [ovs-dev] [PATCH 2/2] FAQ: Update feature table.
>> Sent by: "dev" 
>>
>> Linux kernel support for features in out-of-tree module no longer depend
>> on particular versions, as we only support kernels 3.10-4.3; Connection
>> tracking status has changed recently; and NAT is a brand new feature
>> with only support in the latest unreleased Linux kernel version.
>>
>> Signed-off-by: Joe Stringer 
>
> Catching up with my review queue...
>
> Acked-by: Ryan Moats 

Thanks all, applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/2] FAQ: Shift IPFIX into the feature supporttable.

[Joe Stringer]

On 2 May 2016 at 11:29, Ryan Moats  wrote:
> "dev"  wrote on 04/28/2016 04:39:08 PM:
>
>> From: Joe Stringer 
>> To: dev@openvswitch.org
>> Date: 04/28/2016 04:39 PM
>> Subject: [ovs-dev] [PATCH 1/2] FAQ: Shift IPFIX into the feature
>> support table.
>> Sent by: "dev" 
>>
>> Signed-off-by: Joe Stringer 
>
> Catching up with my review queue...
>
> Acked-by: Ryan Moats 

Thanks all, applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCHv2 0/9] Backport ovs-conntrack changes.

[Jesse Gross]

On Mon, May 2, 2016 at 11:19 AM, Joe Stringer  wrote:
> This series backports the netfilter/defrag-related changes made recently
> upstream to our compat code, which should bring conntrack.c up-to-date just
> prior to the NAT changes. Patch 5 introduced some breakage which is fixed in
> patches 6 and 7; I have left these separate to mirror the upstream commits.
>
> Tested using kmod tests on Ubuntu 3.13.0-24, 3.16.0-70, 3.19.0-58, and
> 4.2.0-35, and RHEL 3.10.0-327, plus compilation against vanilla
> kernel targets on Travis:
> https://travis-ci.org/joestringer/openvswitch/builds/127290050

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] flow: Fix flow_wc_map() for ICMPv6 type and code.

[Jarno Rajahalme]

> On Apr 29, 2016, at 5:38 PM, Daniele Di Proietto  
> wrote:
> 
> flow_wc_map() should include 'tp_src' and 'tp_dst' for ICMPv6 packet,
> since they're used for ICMPv6 code and type.
> 
> This caused installed flows in the userspace datapath to always have
> ICMPv6 code and type wildcarded (there are no other users of this
> function).
> 

Thanks for fixing this. While reviewing I noticed that similar issue exists for 
IGMP. It also use the tp_src and tp_dst fields, but they are not included in 
that case. Could you fix that too?

Acked-by: Jarno Rajahalme 

  Jarno

> Signed-off-by: Daniele Di Proietto 
> ---
> lib/flow.c|  4 ++--
> tests/ofproto-dpif.at | 31 +++
> 2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index 560a90f..2521f18 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -1421,6 +1421,8 @@ flow_wc_map(const struct flow *flow, struct flowmap 
> *map)
> FLOWMAP_SET(map, nw_frag);
> FLOWMAP_SET(map, nw_tos);
> FLOWMAP_SET(map, nw_ttl);
> +FLOWMAP_SET(map, tp_src);
> +FLOWMAP_SET(map, tp_dst);
> 
> if (OVS_UNLIKELY(flow->nw_proto == IPPROTO_ICMPV6)) {
> FLOWMAP_SET(map, nd_target);
> @@ -1428,8 +1430,6 @@ flow_wc_map(const struct flow *flow, struct flowmap 
> *map)
> FLOWMAP_SET(map, arp_tha);
> } else {
> FLOWMAP_SET(map, tcp_flags);
> -FLOWMAP_SET(map, tp_src);
> -FLOWMAP_SET(map, tp_dst);
> }
> } else if (eth_type_mpls(flow->dl_type)) {
> FLOWMAP_SET(map, mpls_lse);
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index e7445ac..53c512f 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -7269,6 +7269,37 @@ 
> icmp6,vlan_tci=0x,dl_src=00:00:86:05:80:da,dl_dst=00:60:97:07:69:ea,ipv6_src
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> 
> +AT_SETUP([ofproto-dpif - ICMPv6 type match])
> +OVS_VSWITCHD_START
> +add_of_ports br0 1 2 3
> +
> +AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=128,actions=2'])
> +AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=129,actions=3'])
> +
> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
> +
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
> +
> +AT_CHECK([ovs-appctl revalidator/purge], [0])
> +
> +AT_CHECK([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used], 
> [0], [dnl
> +recirc_id=0,icmp6,in_port=1,vlan_tci=0x,nw_frag=no,icmp_type=0x80/0xff, 
> actions:2
> +recirc_id=0,icmp6,in_port=1,vlan_tci=0x,nw_frag=no,icmp_type=0x81/0xff, 
> actions:3
> +])
> +
> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> + n_packets=2, n_bytes=124, icmp6,icmp_type=128 actions=output:2
> + n_packets=2, n_bytes=124, icmp6,icmp_type=129 actions=output:3
> +NXST_FLOW reply:
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +
> AT_SETUP([ofproto-dpif - Neighbor Discovery set-field with checksum update])
> OVS_VSWITCHD_START
> add_of_ports br0 1
> -- 
> 2.1.4
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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

Re: [ovs-dev] [PATCH RFC] dpif-netdev: ACL+dpcls for Wildcard matching.

[Ben Pfaff]

This is a significant performance advantage.  Does the improvement
remain with large and complex flow tables?  These flow tables are very
simple.

On Fri, Apr 22, 2016 at 08:21:11AM +, Fischetti, Antonio wrote:
> Hi Ben,
> below are 2 examples.
> 
> For both cases:
>* EMC was bypassed
>* using a bridge with 2 dpdk ports
>* I've sent data at line rate on one port and just read the received rate 
> on the other port,
>   regardless of lost packets.
> 
> 
> Case A: 7 Flows
> 
> Original dpcls:   5.74 Mpps
> ACL + dpcls:   7.03 Mpps
> 
> The 7 Flows were installed as:
> ovs-ofctl add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.20,nw_dst=34.35.36.37,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.19,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.18,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.17,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.16,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.15,action=output:2
> ovs-ofctl add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.14,nw_dst=34.35.36.37,action=output:2
> 
> 
> Case B: 17 Flows
> =
> Original dpcls:   2.95 Mpps
> ACL+dpcls: 4.67 Mpps
> 
> The 17 Flows were installed as:
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.20,nw_dst=34.35.36.37,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.20,nw_dst=34.35.36.38,udp_dst=4369,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.19,udp_src=4369,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.18,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.17,udp_dst=4369,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.16,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.15,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.14,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.13,udp_src=4369,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.10,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.9,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.8,nw_dst=34.35.36.37,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.8,nw_dst=34.35.36.38,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.7,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.6,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_dst=34.35.36.37,action=output:2
> add-flow br0 dl_type=0x0800,nw_dst=34.35.36.38,action=output:2
> 
> For more details, please let me know.
> 
> Thanks,
> Antonio
> 
> 
> 
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Thursday, April 21, 2016 7:41 PM
> > To: Fischetti, Antonio 
> > Cc: dev@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH RFC] dpif-netdev: ACL+dpcls for Wildcard
> > matching.
> > 
> > On Wed, Apr 13, 2016 at 10:45:09AM +0100, antonio.fische...@intel.com
> > wrote:
> > > The purpose of this implementation is to improve the performance
> > > of wildcard matching in user-space.
> > > This RFC patch shows the basic functionality, some aspects were not
> > > covered yet.
> > >
> > > I would like to get some feedback on whether people think integrating
> > > the DPDK ACL table in this manner is potentially a good solution or not.
> > >
> > > DPDK ACL tables show a better performance on lookup operations than the
> > > Classifier.  However their insertion time for new rules is unacceptable.
> > > This solution attempts to combine the better performance of ACL lookups
> > > with the lower insertion latency of the Classifier.
> > 
> > How much does the performance improve?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/4] docs: OVSDB replication design document

[Ben Pfaff]

On Mon, Apr 18, 2016 at 10:02:34AM -0600, Marcelo E. Magallon wrote:
> On Mon, Apr 11, 2016 at 03:44:09PM -0700, Ben Pfaff wrote:
> > On Fri, Apr 01, 2016 at 10:52:26AM -0700, Ben Pfaff wrote:
> > > I don't think it makes sense to stack replication and Raft-based HA.
> > > 
> > > Thinking about OpenSwitch, I guess that your use case is something
> > > like this: an OpenSwitch instance maintains, on-box, an
> > > authoritative database instance, and then the replication feature
> > > allows that database's content to be backed up somewhere else.  I
> > > see how that differs from the use case for Raft, where there is no
> > > preference for a particular database server to be authoritative.
> > > What I don't see yet is how or why that's useful.  What is the use
> > > case?
> > 
> > In case it wasn't clear, I didn't mean my message above to sound like
> > a "no, we won't take this".  Instead, I'm trying to understand the use
> > case better.  Perhaps there is room for both replication and HA in
> > OVSDB, but before I say "yes" to that, I want to understand both
> > cases.
> 
>  Yes, that's totally fair.
> 
>  We do not have a need for only 1+1 redundancy. We have a need in which
>  we have to remain operational with less than a quantum of instances in
>  operation, which raft can’t do unless you introduce modifications to
>  the algorithm (e.g. etcd or consul, I can't remember which one
>  exactly).
> 
>  Also, raft assumes that everybody's vote is equal. If you’re treating
>  multiple instances of OVS as one large virtual switch, you are not
>  running a separate version of OSPF on each instance, each feeding its
>  own version of the routing table into the database.  You have one OSPF
>  instance on a "stack commander" feeding the entire routing table into
>  the database. This is the "correct" state, no matter how many raft
>  members have voted on it. We grow to more than 2 members by setting up
>  multiple one way replications, all originating from the "commander". In
>  future patches, we will also implement two way replication so that the
>  member can write to his local database to reflect state that the
>  commander cannot know about (like port state) ... until that happens
>  daemons on a "member" can connect directly to the commander's OVSDB
>  instance and update the commander's state directly.
> 
>  This work is done in the conetxt of OpenSwitch (http://openswitch.net/,
>  probaly http://openswitch.net/documents/user/architecture is more
>  relevant to this discussion).  With the proposed patch we can have two
>  OVSDB instances each running on a TOR switch. One of the switches is
>  active and the other is a stand-by. The stand-by instance is constantly
>  replicating the active one. In case of a failure in the active, the
>  stand-by can take over and the control plane can be rebuilt from the
>  state stored in the database.
> 
>  I don't think the two approaches are in conflict with each other,
>  actually the complement each other. What I'm trying to figure out is
>  where they overlap (from a code point of view).

OK, I think I understand the use case better now.  I'll try to take a
look at the patches this week.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2 1/3] lib: Add Partial Map Updates functionality

[Aymerich, Edward]

In the current implementation, every time an element of either a map or set
column has to be modified, the entire content of the column is sent to the
server to be updated. This is not a major problem if the information contained
in the column for the corresponding row is small, but there are cases where
these columns can have a significant amount of elements per row, or these
values are updated frequently, therefore the cost of the modifications becomes
high in terms of time and bandwidth.

In this solution, the ovsdb-idl code is modified to use the RFC 7047 'mutate'
operation, to allow sending partial modifications on map columns to the server.
The functionality is exposed to clients in the vswitch idl. This was
implemented through map operations.

A map operation is defined as an insertion, update or deletion of a key-value
pair inside a map. The idea is to minimize the amount of map operations
that are send to the OVSDB server when a transaction is committed.

In order to keep track of the requested map operations, structs map_op and
map_op_list were defined with accompanying functions to manipulate them. These
functions make sure that only one operation is send to the server for each
key-value that wants to be modified, so multiple operation on a key value are
collapsed into a single operation.

As an example, if a client using the IDL updates several times the value for
the same key, the functions will ensure that only the last value is send to
the server, instead of multiple updates. Or, if the client inserts a key-value,
and later on deletes the key before committing the transaction, then both
actions cancel out and no map operation is send for that key.

To keep track of the desired map operations on each transaction, a list of map
operations (struct map_op_list) is created for every column on the row on which
a map operation is performed. When a new map operation is requested on the same
column, the corresponding map_op_list is checked to verify if a previous
operations was performed on the same key, on the same transaction. If there is
no previous operation, then the new operation is just added into the list. But
if there was a previous operation on the same key, then the previous operation
is collapsed with the new operation into a single operation that preserves the
final result if both operations were to be performed sequentially. This design
keep a small memory footprint during transactions.

When a transaction is committed, the map operations lists are checked and
all map operations that belong to the same map are grouped together into a
single JSON RPC "mutate" operation, in which each map_op is transformed into
the necessary "insert" or "delete" mutators. Then the "mutate" operation is
added to the operations that will be send to the server.

Once the transaction is finished, all map operation lists are cleared and
deleted, so the next transaction starts with a clean board for map operations.

Using different structures and logic to handle map operations, instead of
trying to force the current structures (like 'old' and 'new' datums in the row)
to handle then, ensures that map operations won't mess up with the current
logic to generate JSON messages for other operations, avoids duplicating the
whole map for just a few changes, and is faster for insert and delete
operations, because there is no need to maintain the invariants in the 'new'
datum.

Signed-off-by: Edward Aymerich 
Signed-off-by: Arnoldo Lutz 
Co-authored-by: Arnoldo Lutz 
---
 The corresponding pull request is available here:
 https://github.com/openvswitch/ovs/pull/124

 lib/automake.mk  |   3 +-
 lib/ovsdb-idl-provider.h |   4 +
 lib/ovsdb-idl.c  | 285 +++
 lib/ovsdb-idl.h  |   6 +
 lib/ovsdb-map-op.c   | 171 
 lib/ovsdb-map-op.h   |  45 
 6 files changed, 513 insertions(+), 1 deletion(-)
 create mode 100644 lib/ovsdb-map-op.c
 create mode 100644 lib/ovsdb-map-op.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 76dfc07..dc3a4a6 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -174,6 +174,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/ovsdb-idl-provider.h \
lib/ovsdb-idl.c \
lib/ovsdb-idl.h \
+   lib/ovsdb-map-op.c \
+   lib/ovsdb-map-op.h \
lib/ovsdb-parser.c \
lib/ovsdb-parser.h \
lib/ovsdb-types.c \
@@ -502,4 +504,3 @@ lib-install-data-local:
$(MKDIR_P) $(DESTDIR)$(PKIDIR)
$(MKDIR_P) $(DESTDIR)$(LOGDIR)
$(MKDIR_P) $(DESTDIR)$(DBDIR)
-
diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 027f79b..1aafb00 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -19,6 +19,7 @@
 #include "hmap.h"
 #include "openvswitch/list.h"
 #include "ovsdb-idl.h"
+#include "ovsdb-map-op.h"
 #include "ovsdb-types.h"
 #include "shash.h"
 #include "uuid.h"
@@ -36,6 +37,9 @@ struct ovsdb_idl_row {
 unsigned lo

[ovs-dev] [PATCH v2 2/3] ovsdb-idlc.in: Autogenerate Partial Map Updates functions

[Aymerich, Edward]

Code inserted that autogenerates corresponding map functions to set and
delete elements in map columns.
Inserts description to the functions that are autogenerated.

Signed-off-by: Edward Aymerich 
Signed-off-by: Arnoldo Lutz 
Co-authored-by: Arnoldo Lutz 
---
 The corresponding pull request is available here:
 https://github.com/openvswitch/ovs/pull/124

 ovsdb/ovsdb-idlc.in | 69 +
 1 file changed, 69 insertions(+)

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 26b0de4..19a86dc 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -216,6 +216,13 @@ bool %(s)s_is_updated(const struct %(s)s *, enum 
%(s)s_column_id);
 print '%s);' % ', '.join(args)

 print
+for columnName, column in sorted(table.columns.iteritems()):
+ if column.type.is_map():
+print 'void %(s)s_update_%(c)s_setkey(const struct %(s)s *, ' 
% {'s': structName, 'c': columnName},
+print '%(coltype)s, %(valtype)s);' % 
{'coltype':column.type.key.toCType(prefix), 
'valtype':column.type.value.toCType(prefix)}
+print 'void %(s)s_update_%(c)s_delkey(const struct %(s)s *, ' 
% {'s': structName, 'c': columnName},
+print '%(coltype)s);' % 
{'coltype':column.type.key.toCType(prefix)}
+print

 # Table indexes.
 printEnum("%stable_id" % prefix.lower(), ["%sTABLE_%s" % (prefix.upper(), 
tableName.upper()) for tableName in sorted(schema.tables)] + ["%sN_TABLES" % 
prefix.upper()])
@@ -746,6 +753,68 @@ const struct ovsdb_datum *
'S': structName.upper(),
'C': columnName.upper()}
 print "}"
+# Update/Delete of partial map column functions
+for columnName, column in sorted(table.columns.iteritems()):
+type = column.type
+if type.is_map():
+print '''
+/* Sets an element of the "%(c)s" map column from the "%(t)s" table in 'row'
+ * to 'new_value' given the key value 'new_key'.
+ *
+ */
+void
+%(s)s_update_%(c)s_setkey(const struct %(s)s *row, %(coltype)snew_key, 
%(valtype)snew_value)
+{
+struct ovsdb_datum *datum;
+
+ovs_assert(inited);
+
+datum = xmalloc(sizeof *datum);
+datum->n = 1;
+datum->keys = xmalloc(datum->n * sizeof *datum->keys);
+datum->values = xmalloc(datum->n * sizeof *datum->values);
+''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper(), 't': tableName}
+
+print ""+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "new_key")
+print ""+ type.value.copyCValue("datum->values[0].%s" % 
type.value.type.to_string(), "new_value")
+print '''
+ovsdb_idl_txn_write_partial_map(&row->header_,
+&%(s)s_columns[%(S)s_COL_%(C)s],
+datum);
+}''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper()}
+print '''
+/* Deletes an element of the "%(c)s" map column from the "%(t)s" table in 'row'
+ * given the key value 'delete_key'.
+ *
+ */
+void
+%(s)s_update_%(c)s_delkey(const struct %(s)s *row, %(coltype)sdelete_key)
+{
+struct ovsdb_datum *datum;
+
+ovs_assert(inited);
+
+datum = xmalloc(sizeof *datum);
+datum->n = 1;
+datum->keys = xmalloc(datum->n * sizeof *datum->keys);
+datum->values = NULL;
+''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper(), 't': tableName}
+
+print ""+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "delete_key")
+print '''
+ovsdb_idl_txn_delete_partial_map(&row->header_,
+&%(s)s_columns[%(S)s_COL_%(C)s],
+datum);
+}''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper()}
+# End Update/Delete of partial maps

 # Table columns.
 print "\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
--
2.1.4

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

[ovs-dev] [PATCH v2 3/3] tests: Add test for Partial Map Updates

[Aymerich, Edward]

Insert basic functionality for testing partial map updates
and add a new test table named "simple2".

Signed-off-by: Edward Aymerich 
Signed-off-by: Arnoldo Lutz 
Co-authored-by: Arnoldo Lutz 
---
 The corresponding pull request is available here:
 https://github.com/openvswitch/ovs/pull/124

 tests/idltest.ovsschema  |  79 ++--
 tests/idltest2.ovsschema |  29 
 tests/ovsdb-idl.at   |  33 +
 tests/test-ovsdb.c   | 117 ++-
 4 files changed, 232 insertions(+), 26 deletions(-)

diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema
index 1d073aa..5482234 100644
--- a/tests/idltest.ovsschema
+++ b/tests/idltest.ovsschema
@@ -6,7 +6,7 @@
   "columns": {
 "i": {
   "type": "integer"
-},
+},
 "k": {
   "type": {
 "key": {
@@ -14,17 +14,17 @@
   "refTable": "link1"
 }
   }
-},
+},
 "ka": {
   "type": {
 "key": {
   "type": "uuid",
   "refTable": "link1"
 },
-"max": "unlimited",
+"max": "unlimited",
 "min": 0
   }
-},
+},
 "l2": {
   "type": {
 "key": {
@@ -35,12 +35,12 @@
   }
 }
   }
-},
+},
 "link2": {
   "columns": {
 "i": {
   "type": "integer"
-},
+},
 "l1": {
   "type": {
 "key": {
@@ -51,60 +51,89 @@
   }
 }
   }
-},
+},
 "simple": {
   "columns": {
 "b": {
   "type": "boolean"
-},
+},
 "ba": {
   "type": {
-"key": "boolean",
+"key": "boolean",
 "max": 1,
 "min": 0
   }
-},
+},
 "i": {
   "type": "integer"
-},
+},
 "ia": {
   "type": {
-"key": "integer",
-"max": "unlimited",
+"key": "integer",
+"max": "unlimited",
 "min": 0
   }
-},
+},
 "r": {
   "type": "real"
-},
+},
 "ra": {
   "type": {
-"key": "real",
-"max": "unlimited",
+"key": "real",
+"max": "unlimited",
 "min": 0
   }
-},
+},
 "s": {
   "type": "string"
-},
+},
 "sa": {
   "type": {
-"key": "string",
-"max": "unlimited",
+"key": "string",
+"max": "unlimited",
 "min": 0
   }
-},
+},
 "u": {
   "type": "uuid"
-},
+},
 "ua": {
   "type": {
-"key": "uuid",
-"max": "unlimited",
+"key": "uuid",
+"max": "unlimited",
 "min": 0
   }
 }
   }
+},
+"simple2" : {
+  "columns" : {
+"name" : {
+  "type": "string"
+},
+"smap" : {
+  "type": {
+"key" : "string",
+"value": "string",
+"min": 0,
+"max": "unlimited"
+  }
+},
+"imap": {
+  "type" : {
+"key": {
+  "type" : "integer",
+  "minInteger" : 0,
+  "maxInteger" : 4095
+},
+"value": {
+  "type" : "string"
+},
+"min": 0,
+"max": "unlimited"
+  }
+}
+  }
 }
   }
 }
diff --git a/tests/idltest2.ovsschema b/tests/idltest2.ovsschema
index 312c9cc..5cf61d1 100644
--- a/tests/idltest2.ovsschema
+++ b/tests/idltest2.ovsschema
@@ -80,6 +80,35 @@
   }
 }
   }
+},
+"simple2" : {
+  "columns" : {
+"name" : {
+  "type": "string"
+},
+"smap" : {
+  "type": {
+"key" : "string",
+"value": "string",
+"min": 0,
+"max": "unlimited"
+  }
+},
+"imap": {
+  "type" : {
+"key": {
+  "type" : "integer",
+  "minInteger" : 0,
+  "maxInteger" : 4095
+},
+"value": {
+  "type" : "string"
+},
+"min": 0,
+"max": "unlimited"
+  }
+}
+  }
 }
   }
 }
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 33d508c..4683c4e 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -800,3 +800,36 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, 
various ops],
 014: updated columns: ba i ia r ra s
 015: done
 ]])
+
+m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_MAP_COLUMN],
+  [AT_SETUP([$1 - C])
+   AT_KEYWORDS([ovsdb server idl partial update m

Re: [ovs-dev] [PATCH] flow: Fix flow_wc_map() for ICMPv6 type and code.

[Daniele Di Proietto]

On 02/05/2016 13:11, "Jarno Rajahalme"  wrote:

>
>> On Apr 29, 2016, at 5:38 PM, Daniele Di Proietto  
>> wrote:
>> 
>> flow_wc_map() should include 'tp_src' and 'tp_dst' for ICMPv6 packet,
>> since they're used for ICMPv6 code and type.
>> 
>> This caused installed flows in the userspace datapath to always have
>> ICMPv6 code and type wildcarded (there are no other users of this
>> function).
>> 
>
>Thanks for fixing this. While reviewing I noticed that similar issue exists 
>for IGMP. It also use the tp_src and tp_dst fields, but they are not included 
>in that case. Could you fix that too?

Nice catch!  I've included fix for IGMP as well

>Acked-by: Jarno Rajahalme 

Thanks, I pushed this to master and branch-2.5

>
>  Jarno
>
>> Signed-off-by: Daniele Di Proietto 
>> ---
>> lib/flow.c|  4 ++--
>> tests/ofproto-dpif.at | 31 +++
>> 2 files changed, 33 insertions(+), 2 deletions(-)
>> 
>> diff --git a/lib/flow.c b/lib/flow.c
>> index 560a90f..2521f18 100644
>> --- a/lib/flow.c
>> +++ b/lib/flow.c
>> @@ -1421,6 +1421,8 @@ flow_wc_map(const struct flow *flow, struct flowmap 
>> *map)
>> FLOWMAP_SET(map, nw_frag);
>> FLOWMAP_SET(map, nw_tos);
>> FLOWMAP_SET(map, nw_ttl);
>> +FLOWMAP_SET(map, tp_src);
>> +FLOWMAP_SET(map, tp_dst);
>> 
>> if (OVS_UNLIKELY(flow->nw_proto == IPPROTO_ICMPV6)) {
>> FLOWMAP_SET(map, nd_target);
>> @@ -1428,8 +1430,6 @@ flow_wc_map(const struct flow *flow, struct flowmap 
>> *map)
>> FLOWMAP_SET(map, arp_tha);
>> } else {
>> FLOWMAP_SET(map, tcp_flags);
>> -FLOWMAP_SET(map, tp_src);
>> -FLOWMAP_SET(map, tp_dst);
>> }
>> } else if (eth_type_mpls(flow->dl_type)) {
>> FLOWMAP_SET(map, mpls_lse);
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index e7445ac..53c512f 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -7269,6 +7269,37 @@ 
>> icmp6,vlan_tci=0x,dl_src=00:00:86:05:80:da,dl_dst=00:60:97:07:69:ea,ipv6_src
>> OVS_VSWITCHD_STOP
>> AT_CLEANUP
>> 
>> +AT_SETUP([ofproto-dpif - ICMPv6 type match])
>> +OVS_VSWITCHD_START
>> +add_of_ports br0 1 2 3
>> +
>> +AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=128,actions=2'])
>> +AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=129,actions=3'])
>> +
>> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
>> +
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
>> +
>> +AT_CHECK([ovs-appctl revalidator/purge], [0])
>> +
>> +AT_CHECK([strip_ufid < ovs-vswitchd.log | filter_flow_install | 
>> strip_used], [0], [dnl
>> +recirc_id=0,icmp6,in_port=1,vlan_tci=0x,nw_frag=no,icmp_type=0x80/0xff, 
>> actions:2
>> +recirc_id=0,icmp6,in_port=1,vlan_tci=0x,nw_frag=no,icmp_type=0x81/0xff, 
>> actions:3
>> +])
>> +
>> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
>> + n_packets=2, n_bytes=124, icmp6,icmp_type=128 actions=output:2
>> + n_packets=2, n_bytes=124, icmp6,icmp_type=129 actions=output:3
>> +NXST_FLOW reply:
>> +])
>> +
>> +OVS_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>> +
>> AT_SETUP([ofproto-dpif - Neighbor Discovery set-field with checksum update])
>> OVS_VSWITCHD_START
>> add_of_ports br0 1
>> -- 
>> 2.1.4
>> 
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] vtep/ovs-vtep: support multiple ovs-vtep processes which using the isolated logical-switch

[Darrell Ball]

On Sun, May 1, 2016 at 4:15 PM, nickcooper-zhangtonghao <
nickcooper-zhangtong...@opencloud.tech> wrote:

> Hi all,
> The computer can run multiple ovs-vtep processes, but ovs-vtep
> processes share the “vtep_lsX” bridge when binding the physical-switch and
> logical-switch. The bridge created by logical-switch should be named
> according to logical-switch.
>


You want to run multiple copies of the vtep simulator - ovs-vtep, sharing
the same ovsdb and vswitchd it seems.

Hence, from the code, you need to disambiguate logical switch contexts
maintained by each ovs-vtep physical switch while using a single ovsdb
server and single vswitchd. You did this by including the physical switch
name as part of the logical switch name.

Can you change the commit message to something similar to - Include
ovs-vtep physical switch name as part of logical switch name to support
running multiple ovs-vtep processes sharing the same ovsdb and vswitchd.

I did an initial test using the OVN gateway test case and did not see any
issues with the proposed diff.










>
> signed-off-by: nickcooper-zhangtonghao
> 
>
> --- vtep/ovs-vtep   2016-05-01 09:42:08.903583112 -0400
> +++ vtep/ovs-vtep.new   2016-05-01 08:15:19.352586815 -0400
> @@ -81,11 +81,11 @@ def unixctl_exit(conn, unused_argv, unus
>
>
>  class Logical_Switch(object):
> -def __init__(self, ls_name):
> +def __init__(self, ls_name, ps_name):
>  global ls_count
>  self.name = ls_name
>  ls_count += 1
> -self.short_name = "vtep_ls" + str(ls_count)
> +self.short_name = ps_name + "_vtep_ls" + str(ls_count)
>  vlog.info("creating lswitch %s (%s)" % (self.name,
> self.short_name))
>  self.ports = {}
>  self.tunnels = {}
> @@ -583,7 +583,7 @@ def handle_physical():
>  for b in binding_set:
>  vlan, ls_name = b.split()
>  if ls_name not in Lswitches:
> -Lswitches[ls_name] = Logical_Switch(ls_name)
> +Lswitches[ls_name] = Logical_Switch(ls_name, ps_name)
>
>  binding = "%s-%s" % (vlan, pp_name)
>  ls = Lswitches[ls_name]
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC] dpif-netdev: ACL+dpcls for Wildcard matching.

[Jarno Rajahalme]

> On Apr 22, 2016, at 1:21 AM, Fischetti, Antonio  
> wrote:
> 
> Hi Ben,
> below are 2 examples.
> 
> For both cases:
>   * EMC was bypassed
>   * using a bridge with 2 dpdk ports
>   * I've sent data at line rate on one port and just read the received rate 
> on the other port,
>  regardless of lost packets.
> 
> 
> Case A: 7 Flows
> 
> Original dpcls:   5.74 Mpps
> ACL + dpcls:   7.03 Mpps
> 
> The 7 Flows were installed as:
> ovs-ofctl add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.20,nw_dst=34.35.36.37,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.19,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.18,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.17,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.16,action=output:2
> ovs-ofctl add-flow br0 dl_type=0x0800,nw_src=17.18.19.15,action=output:2
> ovs-ofctl add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.14,nw_dst=34.35.36.37,action=output:2
> 
> 
> Case B: 17 Flows
> =
> Original dpcls:   2.95 Mpps
> ACL+dpcls: 4.67 Mpps
> 
> The 17 Flows were installed as:
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.20,nw_dst=34.35.36.37,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.20,nw_dst=34.35.36.38,udp_dst=4369,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.19,udp_src=4369,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.18,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.17,udp_dst=4369,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.16,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.15,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.14,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_proto=17,nw_src=17.18.19.13,udp_src=4369,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.10,action=output:2
> add-flow br0 dl_type=0x0800,nw_src=17.18.19.9,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.8,nw_dst=34.35.36.37,action=output:2
> add-flow br0 
> dl_type=0x0800,nw_src=17.18.19.8,nw_dst=34.35.36.38,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.7,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_src=17.18.19.6,action=output:2
> add-flow br0 dl_type=0x0800,nw_proto=17,nw_dst=34.35.36.37,action=output:2
> add-flow br0 dl_type=0x0800,nw_dst=34.35.36.38,action=output:2
> 
> For more details, please let me know.
> 

The flows above are at the OpenFlow level. I guess your test traffic exercises 
(just) the corresponding datapath flows? 

Do you know how much of the performance gain is lost once you add support not 
just for the IPv4 5-tuple, but all the different fields supported by struct 
flow (metadata, L2, IPv6, ARP, IGMP, etc)?

  Jarno

> Thanks,
> Antonio
> 
> 
> 
>> -Original Message-
>> From: Ben Pfaff [mailto:b...@ovn.org]
>> Sent: Thursday, April 21, 2016 7:41 PM
>> To: Fischetti, Antonio 
>> Cc: dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH RFC] dpif-netdev: ACL+dpcls for Wildcard
>> matching.
>> 
>> On Wed, Apr 13, 2016 at 10:45:09AM +0100, antonio.fische...@intel.com
>> wrote:
>>> The purpose of this implementation is to improve the performance
>>> of wildcard matching in user-space.
>>> This RFC patch shows the basic functionality, some aspects were not
>>> covered yet.
>>> 
>>> I would like to get some feedback on whether people think integrating
>>> the DPDK ACL table in this manner is potentially a good solution or not.
>>> 
>>> DPDK ACL tables show a better performance on lookup operations than the
>>> Classifier.  However their insertion time for new rules is unacceptable.
>>> This solution attempts to combine the better performance of ACL lookups
>>> with the lower insertion latency of the Classifier.
>> 
>> How much does the performance improve?
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[Darrell Ball]

On Mon, May 2, 2016 at 11:12 AM, Joe Stringer  wrote:

> On 2 May 2016 at 09:04, Darrell Ball  wrote:
> > On Mon, May 2, 2016 at 8:39 AM, William Tu  wrote:
> >
> >> Hi Darrel,
> >>
> >>  # Delete namespaces from the running OS
>   m4_define([DEL_NAMESPACES],
>  [m4_foreach([ns], [$@],
>  -   [ip netns del ns
>  -])
>  +   [if ip netns list | grep ns > /dev/null; then
>  +   ip netns del ns
>  +fi
>  +   ])
>  ]
>   )
> 
> >>>
> >>> Do we want to suppress an error on deletion in general ?
> >>>
> >>>
> >> No, I think it won't suppress errors on deletion.
> >>
> >
> >
> > Just to be clear, what the comment means is that if:
> > 1) DEL_NAMESPACE is called and there is no such ns, then this may be an
> > error with the surrounding code (i.e. a bug) or maybe the test itself.
> >
> > 2) Hence the above code in DEL_NAMESPACE would make the bug less
> > visible since there would be no visible complaint on trying to delete a
> ns
> > that does not exist
>
> I was actually just wondering about why we need DEL_NAMESPACES.
> Originally, if you did a CTRL+C in the middle of the test, then
> cleanup would not properly occur so you'd end up with all of these
> test namespaces still existing. By deleting all of the specified
> namespaces at the start of ADD_NAMESPACES, it would allow the test to
> proceed without forcing the user to go through and delete all of the
> namespaces.
>

For the purposes of ADD_NAMESPACES, I agree - DEL_NAMESPACES
is not essential.


>
> However, if we were to queue up namespace deletion using on_exit "ip
> netns delete foo" immediately after creation, then the above issue
> should not exist, so maybe we could get rid of DELETE_NAMESPACES?
>

However, testing delete namespaces within the testsuite in other respects
(in future) seems useful to catch bugs as opposed to just cleanup.
DEL_NAMESPACES
seems like one small wrapper that could be used to make this cleaner,
although
it is not essential.


>
> In general I've advocated in the tests that while the test-writer
> needs to specify things like ADD_NAMESPACES(), those commands will
> queue up the cleanup to ensure that whether the test passes or fails,
> the system is left in a tidy state. This means that it is not
> necessary inside of tests to add the DELETE_NAMESPACES() towards the
> end (which would only execute if the rest of the test was successful).
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCHv2 0/9] Backport ovs-conntrack changes.

[Joe Stringer]

On 2 May 2016 at 13:05, Jesse Gross  wrote:
> On Mon, May 2, 2016 at 11:19 AM, Joe Stringer  wrote:
>> This series backports the netfilter/defrag-related changes made recently
>> upstream to our compat code, which should bring conntrack.c up-to-date just
>> prior to the NAT changes. Patch 5 introduced some breakage which is fixed in
>> patches 6 and 7; I have left these separate to mirror the upstream commits.
>>
>> Tested using kmod tests on Ubuntu 3.13.0-24, 3.16.0-70, 3.19.0-58, and
>> 4.2.0-35, and RHEL 3.10.0-327, plus compilation against vanilla
>> kernel targets on Travis:
>> https://travis-ci.org/joestringer/openvswitch/builds/127290050
>
> Acked-by: Jesse Gross 

Thanks, applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[Joe Stringer]

On 2 May 2016 at 16:57, Darrell Ball  wrote:
>
>
> On Mon, May 2, 2016 at 11:12 AM, Joe Stringer  wrote:
>>
>> On 2 May 2016 at 09:04, Darrell Ball  wrote:
>> > On Mon, May 2, 2016 at 8:39 AM, William Tu  wrote:
>> >
>> >> Hi Darrel,
>> >>
>> >>  # Delete namespaces from the running OS
>>   m4_define([DEL_NAMESPACES],
>>  [m4_foreach([ns], [$@],
>>  -   [ip netns del ns
>>  -])
>>  +   [if ip netns list | grep ns > /dev/null; then
>>  +   ip netns del ns
>>  +fi
>>  +   ])
>>  ]
>>   )
>> 
>> >>>
>> >>> Do we want to suppress an error on deletion in general ?
>> >>>
>> >>>
>> >> No, I think it won't suppress errors on deletion.
>> >>
>> >
>> >
>> > Just to be clear, what the comment means is that if:
>> > 1) DEL_NAMESPACE is called and there is no such ns, then this may be an
>> > error with the surrounding code (i.e. a bug) or maybe the test itself.
>> >
>> > 2) Hence the above code in DEL_NAMESPACE would make the bug less
>> > visible since there would be no visible complaint on trying to delete a
>> > ns
>> > that does not exist
>>
>> I was actually just wondering about why we need DEL_NAMESPACES.
>> Originally, if you did a CTRL+C in the middle of the test, then
>> cleanup would not properly occur so you'd end up with all of these
>> test namespaces still existing. By deleting all of the specified
>> namespaces at the start of ADD_NAMESPACES, it would allow the test to
>> proceed without forcing the user to go through and delete all of the
>> namespaces.
>
>
> For the purposes of ADD_NAMESPACES, I agree - DEL_NAMESPACES
> is not essential.
>
>>
>>
>> However, if we were to queue up namespace deletion using on_exit "ip
>> netns delete foo" immediately after creation, then the above issue
>> should not exist, so maybe we could get rid of DELETE_NAMESPACES?
>
>
> However, testing delete namespaces within the testsuite in other respects
> (in future) seems useful to catch bugs as opposed to just cleanup.
> DEL_NAMESPACES
> seems like one small wrapper that could be used to make this cleaner,
> although
> it is not essential.

If the only argument is regarding some possible future, then I would
press to get rid of it. It's not a public API; it's not used like that
today; we can always reintroduce it if/when we want to use it that
way.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 2/2] compat: Remove skbuff header helper backports.

[Joe Stringer]

These have existed largely since v2.6.22, so it's well overdue.

Signed-off-by: Joe Stringer 
---
 acinclude.m4 |  4 --
 datapath/linux/Modules.mk|  7 ---
 datapath/linux/compat/include/linux/icmp.h   | 13 --
 datapath/linux/compat/include/linux/icmpv6.h | 13 --
 datapath/linux/compat/include/linux/if_arp.h | 15 ---
 datapath/linux/compat/include/linux/ip.h | 20 -
 datapath/linux/compat/include/linux/ipv6.h   |  7 ---
 datapath/linux/compat/include/linux/sctp.h   | 13 --
 datapath/linux/compat/include/linux/skbuff.h | 65 
 datapath/linux/compat/include/linux/tcp.h| 18 
 datapath/linux/compat/include/linux/udp.h| 18 
 11 files changed, 193 deletions(-)
 delete mode 100644 datapath/linux/compat/include/linux/icmp.h
 delete mode 100644 datapath/linux/compat/include/linux/icmpv6.h
 delete mode 100644 datapath/linux/compat/include/linux/if_arp.h
 delete mode 100644 datapath/linux/compat/include/linux/ip.h
 delete mode 100644 datapath/linux/compat/include/linux/sctp.h
 delete mode 100644 datapath/linux/compat/include/linux/tcp.h
 delete mode 100644 datapath/linux/compat/include/linux/udp.h

diff --git a/acinclude.m4 b/acinclude.m4
index f2a1ea6ad95f..23015fef1943 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -477,10 +477,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h],
   [skb_reset_tail_pointer])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_cow_head])
-  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_transport_header],
-  [OVS_DEFINE([HAVE_SKBUFF_HEADER_HELPERS])])
-  OVS_GREP_IFELSE([$KSRC/include/linux/icmpv6.h], [icmp6_hdr],
-  [OVS_DEFINE([HAVE_ICMP6_HDR])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_warn_if_lro],
   [OVS_DEFINE([HAVE_SKB_WARN_LRO])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [consume_skb])
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 7e334cbf030a..5d38766d0dae 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -37,15 +37,11 @@ openvswitch_headers += \
linux/compat/include/linux/err.h \
linux/compat/include/linux/etherdevice.h \
linux/compat/include/linux/flex_array.h \
-   linux/compat/include/linux/icmp.h \
-   linux/compat/include/linux/icmpv6.h \
linux/compat/include/linux/if.h \
-   linux/compat/include/linux/if_arp.h \
linux/compat/include/linux/if_ether.h \
linux/compat/include/linux/if_link.h \
linux/compat/include/linux/if_vlan.h \
linux/compat/include/linux/in.h \
-   linux/compat/include/linux/ip.h \
linux/compat/include/linux/ipv6.h \
linux/compat/include/linux/jiffies.h \
linux/compat/include/linux/kconfig.h \
@@ -65,13 +61,10 @@ openvswitch_headers += \
linux/compat/include/linux/rcupdate.h \
linux/compat/include/linux/reciprocal_div.h \
linux/compat/include/linux/rtnetlink.h \
-   linux/compat/include/linux/sctp.h \
linux/compat/include/linux/skbuff.h \
linux/compat/include/linux/stddef.h \
-   linux/compat/include/linux/tcp.h \
linux/compat/include/linux/types.h \
linux/compat/include/linux/u64_stats_sync.h \
-   linux/compat/include/linux/udp.h \
linux/compat/include/linux/workqueue.h \
linux/compat/include/net/checksum.h \
linux/compat/include/net/dst.h \
diff --git a/datapath/linux/compat/include/linux/icmp.h 
b/datapath/linux/compat/include/linux/icmp.h
deleted file mode 100644
index 4be4d2b61d05..
--- a/datapath/linux/compat/include/linux/icmp.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef __LINUX_ICMP_WRAPPER_H
-#define __LINUX_ICMP_WRAPPER_H 1
-
-#include_next 
-
-#ifndef HAVE_SKBUFF_HEADER_HELPERS
-static inline struct icmphdr *icmp_hdr(const struct sk_buff *skb)
-{
-   return (struct icmphdr *)skb_transport_header(skb);
-}
-#endif
-
-#endif
diff --git a/datapath/linux/compat/include/linux/icmpv6.h 
b/datapath/linux/compat/include/linux/icmpv6.h
deleted file mode 100644
index 06d91b395283..
--- a/datapath/linux/compat/include/linux/icmpv6.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef __LINUX_ICMPV6_WRAPPER_H
-#define __LINUX_ICMPV6_WRAPPER_H 1
-
-#include_next 
-
-#ifndef HAVE_ICMP6_HDR
-static inline struct icmp6hdr *icmp6_hdr(const struct sk_buff *skb)
-{
-   return (struct icmp6hdr *)skb_transport_header(skb);
-}
-#endif
-
-#endif
diff --git a/datapath/linux/compat/include/linux/if_arp.h 
b/datapath/linux/compat/include/linux/if_arp.h
deleted file mode 100644
index e48d6ba0d760..
--- a/datapath/linux/compat/include/linux/if_arp.h
+++ /dev/null
@@ -1,15 +0,0 @@
-#ifndef __LINUX_IF_ARP_WRAPPER_H
-#define __LINUX_IF_ARP_WRAPPER_H 1
-
-#include_next 
-
-#ifndef HAVE_SKBUFF_HEADER_HELPERS
-#include 
-
-static inline 

[ovs-dev] [PATCH 1/2] compat: Remove unused ipv[46] backports.

[Joe Stringer]

These pieces #if on kernel versions which are not supported since commit
f2ab1536ddbc ("compat: Backport conntrack strictly to v3.10+.")

Signed-off-by: Joe Stringer 
---
 acinclude.m4  | 1 -
 datapath/linux/compat/include/net/ip.h| 7 ---
 datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h | 3 ---
 3 files changed, 11 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 6cfb1e53ef7f..f2a1ea6ad95f 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -375,7 +375,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_defrag.*net],
   [OVS_DEFINE([HAVE_IP_DEFRAG_TAKES_NET])])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_do_fragment])
-  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_is_fragment])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_skb_dst_mtu])
 
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [IPSKB_FRAG_PMTU],
diff --git a/datapath/linux/compat/include/net/ip.h 
b/datapath/linux/compat/include/net/ip.h
index 145529ecbbe1..c283ad0a76d2 100644
--- a/datapath/linux/compat/include/net/ip.h
+++ b/datapath/linux/compat/include/net/ip.h
@@ -6,13 +6,6 @@
 #include 
 #include 
 
-#ifndef HAVE_IP_IS_FRAGMENT
-static inline bool ip_is_fragment(const struct iphdr *iph)
-{
-   return (iph->frag_off & htons(IP_MF | IP_OFFSET)) != 0;
-}
-#endif
-
 #ifndef HAVE_INET_GET_LOCAL_PORT_RANGE_USING_NET
 static inline void rpl_inet_get_local_port_range(struct net *net, int *low,
 int *high)
diff --git a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h 
b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
index 09fe78101f3a..c65e7f2feb03 100644
--- a/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
+++ b/datapath/linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h
@@ -2,10 +2,7 @@
 #define _NF_DEFRAG_IPV6_WRAPPER_H
 
 #include 
-
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,37)
 #include_next 
-#endif
 
 /* Upstream commit 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free clone
  * operations") changed the semantics of nf_ct_frag6_gather(), so we backport
-- 
2.1.4

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

Re: [ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.

[Darrell Ball]

less code -> less bugs

On Mon, May 2, 2016 at 5:45 PM, Joe Stringer  wrote:

> On 2 May 2016 at 16:57, Darrell Ball  wrote:
> >
> >
> > On Mon, May 2, 2016 at 11:12 AM, Joe Stringer  wrote:
> >>
> >> On 2 May 2016 at 09:04, Darrell Ball  wrote:
> >> > On Mon, May 2, 2016 at 8:39 AM, William Tu 
> wrote:
> >> >
> >> >> Hi Darrel,
> >> >>
> >> >>  # Delete namespaces from the running OS
> >>   m4_define([DEL_NAMESPACES],
> >>  [m4_foreach([ns], [$@],
> >>  -   [ip netns del ns
> >>  -])
> >>  +   [if ip netns list | grep ns > /dev/null; then
> >>  +   ip netns del ns
> >>  +fi
> >>  +   ])
> >>  ]
> >>   )
> >> 
> >> >>>
> >> >>> Do we want to suppress an error on deletion in general ?
> >> >>>
> >> >>>
> >> >> No, I think it won't suppress errors on deletion.
> >> >>
> >> >
> >> >
> >> > Just to be clear, what the comment means is that if:
> >> > 1) DEL_NAMESPACE is called and there is no such ns, then this may be
> an
> >> > error with the surrounding code (i.e. a bug) or maybe the test itself.
> >> >
> >> > 2) Hence the above code in DEL_NAMESPACE would make the bug less
> >> > visible since there would be no visible complaint on trying to delete
> a
> >> > ns
> >> > that does not exist
> >>
> >> I was actually just wondering about why we need DEL_NAMESPACES.
> >> Originally, if you did a CTRL+C in the middle of the test, then
> >> cleanup would not properly occur so you'd end up with all of these
> >> test namespaces still existing. By deleting all of the specified
> >> namespaces at the start of ADD_NAMESPACES, it would allow the test to
> >> proceed without forcing the user to go through and delete all of the
> >> namespaces.
> >
> >
> > For the purposes of ADD_NAMESPACES, I agree - DEL_NAMESPACES
> > is not essential.
> >
> >>
> >>
> >> However, if we were to queue up namespace deletion using on_exit "ip
> >> netns delete foo" immediately after creation, then the above issue
> >> should not exist, so maybe we could get rid of DELETE_NAMESPACES?
> >
> >
> > However, testing delete namespaces within the testsuite in other respects
> > (in future) seems useful to catch bugs as opposed to just cleanup.
> > DEL_NAMESPACES
> > seems like one small wrapper that could be used to make this cleaner,
> > although
> > it is not essential.
>
> If the only argument is regarding some possible future, then I would
> press to get rid of it. It's not a public API; it's not used like that
> today; we can always reintroduce it if/when we want to use it that
> way.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/2] compat: Remove unused ipv[46] backports.

[Jesse Gross]

On Mon, May 2, 2016 at 5:47 PM, Joe Stringer  wrote:
> These pieces #if on kernel versions which are not supported since commit
> f2ab1536ddbc ("compat: Backport conntrack strictly to v3.10+.")
>
> Signed-off-by: Joe Stringer 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] compat: Remove skbuff header helper backports.

[Jesse Gross]

On Mon, May 2, 2016 at 5:47 PM, Joe Stringer  wrote:
> These have existed largely since v2.6.22, so it's well overdue.
>
> Signed-off-by: Joe Stringer 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] vtep/ovs-vtep: support multiple ovs-vtep processes which using the isolated logical-switch

[nickcooper-zhangtonghao]

Hi ,
I have changed the commit message in my github repository. Thank 
Darrell Ball for your OVN gateway testing.

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

[ovs-dev] TEST

[Mail Delivery Subsystem]

Your message was undeliverable due to the following reason(s):

Your message was not delivered because the destination server was
unreachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message could not be delivered within 4 days:
Host 126.165.137.182 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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

[ovs-dev] Returned mail: see transcript for details

[Mail Delivery Subsystem]

The message was not delivered due to the following reason:

Your message could not be delivered because the destination server was
not reachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message could not be delivered within 2 days:
Mail server 94.86.24.240 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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