[ovs-dev] [RFCv2 00/12] Revalidate flows with unique identifiers.

[Joe Stringer]

RFCv2:
- Revised early patches from v1 feedback
- Rebased

RFCv1:
http://openvswitch.org/pipermail/dev/2014-August/044383.html

Joe Stringer (12):
  revalidator: Use 'cmap' for storing ukeys.
  revalidator: Protect ukeys with a mutex.
  udpif: Separate udpif_key maps from revalidators.
  upcall: Rename dump_op -> ukey_op.
  upcall: Create ukeys in handler threads.
  upcall: Revalidate using cache of mask, actions.
  hash: Add 128-bit murmurhash.
  dpif: Add Unique flow identifiers.
  upcall: Generate unique flow identifiers.
  dpif-netdev: Support unique flow identifiers.
  dpif-linux: Support unique flow identifiers.
  revalidator: Reduce ukey contention.

 datapath/linux/compat/include/linux/openvswitch.h |   26 +
 include/openvswitch/types.h   |5 +
 lib/dpctl.c   |6 +-
 lib/dpif-linux.c  |   88 ++-
 lib/dpif-netdev.c |  204 +++--
 lib/dpif-provider.h   |8 +-
 lib/dpif.c|   37 +-
 lib/dpif.h|   34 +-
 lib/flow.h|6 +
 lib/hash.c|  194 -
 lib/hash.h|4 +-
 lib/odp-util.c|   51 ++
 lib/odp-util.h|   25 +
 ofproto/ofproto-dpif-upcall.c |  828 ++---
 ofproto/ofproto-dpif-upcall.h |3 +
 ofproto/ofproto-dpif.c|  108 ++-
 ofproto/ofproto-dpif.h|2 +
 tests/dpif-netdev.at  |3 +
 tests/ofproto-dpif.at |   20 +-
 tests/ofproto-macros.at   |1 +
 20 files changed, 1248 insertions(+), 405 deletions(-)

-- 
1.7.10.4

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

[ovs-dev] [RFCv2 01/12] revalidator: Use 'cmap' for storing ukeys.

[Joe Stringer]

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
v2: Call ovsrcu_quiesce() unconditionally.
Added ack.
RFC: Initial Post.
---
 ofproto/ofproto-dpif-upcall.c |   61 ++---
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 4e8c277..428520a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -21,6 +21,7 @@
 
 #include "connmgr.h"
 #include "coverage.h"
+#include "cmap.h"
 #include "dpif.h"
 #include "dynamic-string.h"
 #include "fail-open.h"
@@ -62,7 +63,7 @@ struct revalidator {
 struct udpif *udpif;   /* Parent udpif. */
 pthread_t thread;  /* Thread ID. */
 unsigned int id;   /* ovsthread_id_self(). */
-struct hmap *ukeys;/* Points into udpif->ukeys for this
+struct cmap *ukeys;/* Points into udpif->ukeys for this
   revalidator. Used for GC phase. */
 };
 
@@ -107,15 +108,15 @@ struct udpif {
 long long int dump_duration;   /* Duration of the last flow dump. */
 struct seq *dump_seq;  /* Increments each dump iteration. */
 
-/* There are 'n_revalidators' ukey hmaps. Each revalidator retains a
+/* There are 'n_revalidators' ukey cmaps. Each revalidator retains a
  * reference to one of these for garbage collection.
  *
  * During the flow dump phase, revalidators insert into these with a random
  * distribution. During the garbage collection phase, each revalidator
  * takes care of garbage collecting one of these hmaps. */
 struct {
-struct ovs_mutex mutex;/* Guards the following. */
-struct hmap hmap OVS_GUARDED;  /* Datapath flow keys. */
+struct ovs_mutex mutex;/* Take for writing to the following. */
+struct cmap cmap;  /* Datapath flow keys. */
 } *ukeys;
 
 /* Datapath flow statistics. */
@@ -184,12 +185,13 @@ struct upcall {
  * is not.  Therefore it is not safe to destroy a udpif_key except when all
  * revalidators are in garbage collection phase, or they aren't running. */
 struct udpif_key {
-struct hmap_node hmap_node; /* In parent revalidator 'ukeys' map. */
+struct cmap_node cmap_node; /* In parent revalidator 'ukeys' map. */
 
 /* These elements are read only once created, and therefore aren't
  * protected by a mutex. */
 const struct nlattr *key;  /* Datapath flow key. */
 size_t key_len;/* Length of 'key'. */
+uint32_t hash; /* Pre-computed hash for 'key'. */
 
 struct ovs_mutex mutex;   /* Guards the following. */
 struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
@@ -235,13 +237,14 @@ static void upcall_unixctl_dump_wait(struct unixctl_conn 
*conn, int argc,
  const char *argv[], void *aux);
 
 static struct udpif_key *ukey_create(const struct nlattr *key, size_t key_len,
- long long int used);
+ long long int used, uint32_t hash);
 static struct udpif_key *ukey_lookup(struct udpif *udpif,
  const struct nlattr *key, size_t key_len,
  uint32_t hash);
 static bool ukey_acquire(struct udpif *udpif, const struct nlattr *key,
  size_t key_len, long long int used,
  struct udpif_key **result);
+static void ukey_delete__(struct udpif_key *);
 static void ukey_delete(struct revalidator *, struct udpif_key *);
 static enum upcall_type classify_upcall(enum dpif_upcall_type type,
 const struct nlattr *userdata);
@@ -349,7 +352,7 @@ udpif_stop_threads(struct udpif *udpif)
  * double-counting stats. */
 revalidator_purge(revalidator);
 
-hmap_destroy(&udpif->ukeys[i].hmap);
+cmap_destroy(&udpif->ukeys[i].cmap);
 ovs_mutex_destroy(&udpif->ukeys[i].mutex);
 }
 
@@ -403,9 +406,9 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
 struct revalidator *revalidator = &udpif->revalidators[i];
 
 revalidator->udpif = udpif;
-hmap_init(&udpif->ukeys[i].hmap);
+cmap_init(&udpif->ukeys[i].cmap);
 ovs_mutex_init(&udpif->ukeys[i].mutex);
-revalidator->ukeys = &udpif->ukeys[i].hmap;
+revalidator->ukeys = &udpif->ukeys[i].cmap;
 revalidator->thread = ovs_thread_create(
 "revalidator", udpif_revalidator, revalidator);
 }
@@ -489,9 +492,7 @@ udpif_get_memory_usage(struct udpif *udpif, struct simap 
*usage)
 
 simap_increase(usage, "revalidators", udpif->n_revalidators);
 for (i = 0; i < udpif->n_revalidators; i++) {
-ovs_m

[ovs-dev] [RFCv2 03/12] udpif: Separate udpif_key maps from revalidators.

[Joe Stringer]

An upcoming patch will change the access patterns for ukey maps to
increase the number of writers, and shift write-access from revalidator
threads to upcall handler threads. As such, it no longer makes sense to
tie these maps to revalidators in a 1:1 relationship.

This patch separates the ukey maps from the revalidators, and increases
the number of maps used to store ukeys, to reduce contention.

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
v2: #define N_UMAPS.
Simplify upcall_unixctl_show() element counting.
Mention that 'ukeys' contains elements of type 'struct udpif_key'.
Added ack.
RFC: First post.
---
 ofproto/ofproto-dpif-upcall.c |  152 ++---
 1 file changed, 83 insertions(+), 69 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 04709d3..b3298e1 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -57,17 +57,21 @@ struct handler {
 uint32_t handler_id;   /* Handler id. */
 };
 
+/* In the absence of a multiple-writer multiple-reader datastructure for
+ * storing ukeys, we use a large number of cmaps, each with its own lock for
+ * writing. */
+#define N_UMAPS 128 /* per udpif. */
+struct umap {
+struct ovs_mutex mutex;/* Take for writing to the following. */
+struct cmap cmap;  /* Datapath flow keys. */
+};
+
 /* A thread that processes datapath flows, updates OpenFlow statistics, and
  * updates or removes them if necessary. */
 struct revalidator {
 struct udpif *udpif;   /* Parent udpif. */
 pthread_t thread;  /* Thread ID. */
 unsigned int id;   /* ovsthread_id_self(). */
-struct ovs_mutex *mutex;   /* Points into udpif->ukeys for this
-  revalidator. Required for writing
-  to 'ukeys'. */
-struct cmap *ukeys;/* Points into udpif->ukeys for this
-  revalidator. Used for GC phase. */
 };
 
 /* An upcall handler for ofproto_dpif.
@@ -111,16 +115,12 @@ struct udpif {
 long long int dump_duration;   /* Duration of the last flow dump. */
 struct seq *dump_seq;  /* Increments each dump iteration. */
 
-/* There are 'n_revalidators' ukey cmaps. Each revalidator retains a
- * reference to one of these for garbage collection.
+/* There are 'N_UMAPS' maps containing 'struct udpif_key' elements.
  *
  * During the flow dump phase, revalidators insert into these with a random
  * distribution. During the garbage collection phase, each revalidator
- * takes care of garbage collecting one of these hmaps. */
-struct {
-struct ovs_mutex mutex;/* Take for writing to the following. */
-struct cmap cmap;  /* Datapath flow keys. */
-} *ukeys;
+ * takes care of garbage collecting a slice of these maps. */
+struct umap *ukeys;
 
 /* Datapath flow statistics. */
 unsigned int max_n_flows;
@@ -184,9 +184,10 @@ struct upcall {
  * the dump phase, but are owned by a single revalidator, and are destroyed
  * by that revalidator during the garbage-collection phase.
  *
- * While some elements of a udpif_key are protected by a mutex, the ukey itself
- * is not.  Therefore it is not safe to destroy a udpif_key except when all
- * revalidators are in garbage collection phase, or they aren't running. */
+ * The mutex within the ukey protects some members of the ukey. The ukey
+ * itself is protected by RCU and is held within a umap in the parent udpif.
+ * Adding or removing a ukey from a umap is only safe when holding the
+ * corresponding umap lock. */
 struct udpif_key {
 struct cmap_node cmap_node; /* In parent revalidator 'ukeys' map. */
 
@@ -248,7 +249,7 @@ static bool ukey_acquire(struct udpif *udpif, const struct 
nlattr *key,
  size_t key_len, long long int used,
  struct udpif_key **result);
 static void ukey_delete__(struct udpif_key *);
-static void ukey_delete(struct revalidator *, struct udpif_key *);
+static void ukey_delete(struct umap *, struct udpif_key *);
 static enum upcall_type classify_upcall(enum dpif_upcall_type type,
 const struct nlattr *userdata);
 
@@ -292,6 +293,11 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
 atomic_init(&udpif->n_flows, 0);
 atomic_init(&udpif->n_flows_timestamp, LLONG_MIN);
 ovs_mutex_init(&udpif->n_flows_mutex);
+udpif->ukeys = xmalloc(N_UMAPS * sizeof *udpif->ukeys);
+for (int i = 0; i < N_UMAPS; i++) {
+cmap_init(&udpif->ukeys[i].cmap);
+ovs_mutex_init(&udpif->ukeys[i].mutex);
+}
 
 dpif_register_upcall_cb(dpif, upcall_cb, udpif);
 
@@ -318,6 +324,13 @@ udpif_destroy(struct udpif *udpif)
 {
 udpif_stop_threads(udpif);
 

[ovs-dev] [RFCv2 05/12] upcall: Create ukeys in handler threads.

[Joe Stringer]

Currently, when a revalidator thread first dumps a flow, it creates a
'udpif_key' object and caches a copy of a kernel flow key. This allows
us to perform lookups in the classifier to attribute stats and validate
the correctness of the datapath flow.

This patch sets up this cache from the handler threads, during flow
setup. This allows future patches to reduce the cost of flow dumping.

Signed-off-by: Joe Stringer 
---
v2: Rebase.
RFC: First post.
---
 ofproto/ofproto-dpif-upcall.c |  334 -
 1 file changed, 227 insertions(+), 107 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index cfd2af0..20d5d86 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -46,7 +46,9 @@
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
 
-COVERAGE_DEFINE(upcall_duplicate_flow);
+COVERAGE_DEFINE(handler_install_conflict);
+COVERAGE_DEFINE(upcall_ukey_contention);
+COVERAGE_DEFINE(revalidate_duplicate_flow);
 COVERAGE_DEFINE(revalidate_missed_dp_flow);
 
 /* A thread that reads upcalls from dpif, forwards each upcall's packet,
@@ -108,7 +110,6 @@ struct udpif {
 
 /* Revalidation. */
 struct seq *reval_seq; /* Incremented to force revalidation. */
-bool need_revalidate;  /* As indicated by 'reval_seq'. */
 bool reval_exit;   /* Set by leader on 'exit_latch. */
 struct ovs_barrier reval_barrier;  /* Barrier used by revalidators. */
 struct dpif_flow_dump *dump;   /* DPIF flow dump state. */
@@ -172,6 +173,10 @@ struct upcall {
 bool vsp_adjusted; /* 'packet' and 'flow' were adjusted for
   VLAN splinters if true. */
 
+struct udpif_key *ukey;/* Revalidator flow cache. */
+uint64_t dump_seq; /* udpif->dump_seq at translation time. */
+uint64_t reval_seq;/* udpif->reval_seq at translation time. */
+
 /* Not used by the upcall callback interface. */
 const struct nlattr *key;  /* Datapath flow key. */
 size_t key_len;/* Datapath flow key length. */
@@ -180,9 +185,10 @@ struct upcall {
 
 /* 'udpif_key's are responsible for tracking the little bit of state udpif
  * needs to do flow expiration which can't be pulled directly from the
- * datapath.  They may be created or maintained by any revalidator during
- * the dump phase, but are owned by a single revalidator, and are destroyed
- * by that revalidator during the garbage-collection phase.
+ * datapath.  They may be created by any handler thread at any time, and read
+ * by any revalidator during the dump phase. They are however each owned by a
+ * single revalidator which takes care of destroying them during the
+ * garbage-collection phase.
  *
  * The mutex within the ukey protects some members of the ukey. The ukey
  * itself is protected by RCU and is held within a umap in the parent udpif.
@@ -201,12 +207,14 @@ struct udpif_key {
 struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
 long long int created OVS_GUARDED;/* Estimate of creation time. */
 uint64_t dump_seq OVS_GUARDED;/* Tracks udpif->dump_seq. */
+uint64_t reval_seq OVS_GUARDED;   /* Tracks udpif->reval_seq. */
 bool flow_exists OVS_GUARDED; /* Ensures flows are only deleted
  once. */
 
 struct xlate_cache *xcache OVS_GUARDED;   /* Cache for xlate entries that
* are affected by this ukey.
* Used for stats and learning.*/
+
 union {
 struct odputil_keybuf key_buf;/* Memory for 'key'. */
 struct nlattr key_buf_nla;
@@ -247,14 +255,14 @@ static void upcall_unixctl_set_flow_limit(struct 
unixctl_conn *conn, int argc,
 static void upcall_unixctl_dump_wait(struct unixctl_conn *conn, int argc,
  const char *argv[], void *aux);
 
-static struct udpif_key *ukey_create(const struct nlattr *key, size_t key_len,
- long long int used, uint32_t hash);
-static struct udpif_key *ukey_lookup(struct udpif *udpif,
- const struct nlattr *key, size_t key_len,
- uint32_t hash);
-static bool ukey_acquire(struct udpif *udpif, const struct nlattr *key,
- size_t key_len, long long int used,
- struct udpif_key **result);
+static struct udpif_key *ukey_new(const struct udpif *, struct upcall *);
+static bool ukey_install_start(struct udpif *, struct udpif_key *ukey);
+static void ukey_install_finish(struct udpif_key *ukey, int error);
+static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
+static struct udpif_key *ukey_lookup(struct udpif *udpif, uint32_t hash,
+ 

[ovs-dev] [RFCv2 08/12] dpif: Add Unique flow identifiers.

[Joe Stringer]

One of the limiting factors on the number of flows that can be supported
in the datapath is the overhead of assembling flow dump messages in the
datapath. This patch adds a new alternative to dumping the key, mask and
actions from the datapath, which is a 128-bit unique identifier for the
flow. For each flow dump, the dpif user specifies which fields may be
omitted, allowing the common case to only dump a pair of 128-bit ID and
flow stats.

Signed-off-by: Joe Stringer 
---
 datapath/linux/compat/include/linux/openvswitch.h |   26 +++
 lib/dpctl.c   |6 ++-
 lib/dpif-linux.c  |5 +-
 lib/dpif-netdev.c |   12 -
 lib/dpif-provider.h   |8 +++-
 lib/dpif.c|   37 ---
 lib/dpif.h|   34 --
 lib/odp-util.c|   51 +
 lib/odp-util.h|   25 ++
 ofproto/ofproto-dpif-upcall.c |   11 +++--
 ofproto/ofproto-dpif.c|   13 --
 11 files changed, 202 insertions(+), 26 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 9c18b3b..33397ee 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -470,6 +470,10 @@ struct ovs_key_nd {
  * a wildcarded match. Omitting attribute is treated as wildcarding all
  * corresponding fields. Optional for all requests. If not present,
  * all flow key bits are exact match bits.
+ * @OVS_FLOW_ATTR_UID: Nested %OVS_UID_ATTR_* attributes specifying unique
+ * identifiers for flows and providing alternative semantics for flow
+ * installation and retrieval. Required for all requests if the datapath is
+ * created with %OVS_DP_F_INDEX_BY_UID.
  *
  * These attributes follow the &struct ovs_header within the Generic Netlink
  * payload for %OVS_FLOW_* commands.
@@ -483,12 +487,34 @@ enum ovs_flow_attr {
OVS_FLOW_ATTR_USED,  /* u64 msecs last used in monotonic time. */
OVS_FLOW_ATTR_CLEAR, /* Flag to clear stats, tcp_flags, used. */
OVS_FLOW_ATTR_MASK,  /* Sequence of OVS_KEY_ATTR_* attributes. */
+   OVS_FLOW_ATTR_UID,   /* 64-bit Unique flow identifier. */
__OVS_FLOW_ATTR_MAX
 };
 
 #define OVS_FLOW_ATTR_MAX (__OVS_FLOW_ATTR_MAX - 1)
 
 /**
+ * enum ovs_uid_attr - Unique identifier types.
+ *
+ * @OVS_UID_ATTR_FLAGS: A 32-bit value specifying changes to the behaviour of
+ * the current %OVS_FLOW_CMD_* request. Optional for all requests.
+ * @OVS_UID_ATTR_ID: A 64-bit value uniquely identifying the flow.
+ */
+enum ovs_uid_attr {
+   OVS_UID_ATTR_UNSPEC,
+   OVS_UID_ATTR_FLAGS, /* u32 of OVS_UID_F_* */
+   OVS_UID_ATTR_ID,/* u64 unique flow identifier. */
+   __OVS_UID_ATTR_MAX
+};
+
+#define OVS_UID_ATTR_MAX (__OVS_UID_ATTR_MAX - 1)
+
+/* Skip attributes for notifications. */
+#define OVS_UID_F_SKIP_KEY (1 << 0)
+#define OVS_UID_F_SKIP_MASK(1 << 1)
+#define OVS_UID_F_SKIP_ACTIONS (1 << 2)
+
+/**
  * enum ovs_sample_attr - Attributes for %OVS_ACTION_ATTR_SAMPLE action.
  * @OVS_SAMPLE_ATTR_PROBABILITY: 32-bit fraction of packets to sample with
  * @OVS_ACTION_ATTR_SAMPLE.  A value of 0 samples no packets, a value of
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 623f5b1..db4c8f5 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -736,7 +736,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 }
 
 ds_init(&ds);
-flow_dump = dpif_flow_dump_create(dpif);
+flow_dump = dpif_flow_dump_create(dpif, 0);
 flow_dump_thread = dpif_flow_dump_thread_create(flow_dump);
 while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) {
 if (filter) {
@@ -836,12 +836,13 @@ dpctl_put_flow(int argc, const char *argv[], enum 
dpif_flow_put_flags flags,
 dpctl_error(dpctl_p, error, "parsing actions");
 goto out_freeactions;
 }
+/* XXX: UID parsing */
 error = dpif_flow_put(dpif, flags,
   ofpbuf_data(&key), ofpbuf_size(&key),
   ofpbuf_size(&mask) == 0 ? NULL : ofpbuf_data(&mask),
   ofpbuf_size(&mask),
   ofpbuf_data(&actions), ofpbuf_size(&actions),
-  dpctl_p->print_statistics ? &stats : NULL);
+  NULL, 0, dpctl_p->print_statistics ? &stats : NULL);
 if (error) {
 dpctl_error(dpctl_p, error, "updating flow table");
 goto out_freeactions;
@@ -928,6 +929,7 @@ dpctl_del_flow(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 
 error = dpif_flow_del(dpif,
   ofpbuf_data(&key), ofpbuf_size(&key),
+   

[ovs-dev] [RFCv2 06/12] upcall: Revalidate using cache of mask, actions.

[Joe Stringer]

This allows us to ignore most fields of a flow_dump, requiring only the
flow key for looking up the ukey. Fetching flows can also be avoided in
the corner case where a flow is missed from a dump but revalidation is
required.

A future patch will modify the datapath interface to make these cached
fields optional.

Signed-off-by: Joe Stringer 
---
v2: Rebase.
RFC: First post.
---
 ofproto/ofproto-dpif-upcall.c |  119 ++---
 1 file changed, 51 insertions(+), 68 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 20d5d86..0a74412 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -201,6 +201,9 @@ struct udpif_key {
  * protected by a mutex. */
 const struct nlattr *key;  /* Datapath flow key. */
 size_t key_len;/* Length of 'key'. */
+const struct nlattr *mask; /* Datapath flow mask. */
+size_t mask_len;   /* Length of 'mask'. */
+struct ofpbuf *actions;/* Datapath flow actions as nlattrs. */
 uint32_t hash; /* Pre-computed hash for 'key'. */
 
 struct ovs_mutex mutex;   /* Guards the following. */
@@ -216,9 +219,9 @@ struct udpif_key {
* Used for stats and learning.*/
 
 union {
-struct odputil_keybuf key_buf;/* Memory for 'key'. */
-struct nlattr key_buf_nla;
-};
+struct odputil_keybuf buf;
+struct nlattr nla;
+} keybuf, maskbuf;
 };
 
 /* Datapath operation with optional ukey attached. */
@@ -1076,7 +1079,6 @@ static void
 handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
size_t n_upcalls)
 {
-struct odputil_keybuf mask_bufs[UPCALL_MAX_BATCH];
 struct dpif_op *opsp[UPCALL_MAX_BATCH * 2];
 struct ukey_op ops[UPCALL_MAX_BATCH * 2];
 unsigned int flow_limit;
@@ -1124,34 +1126,20 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
  *- We received this packet via some flow installed in the kernel
  *  already. */
 if (may_put && upcall->type == DPIF_UC_MISS) {
-struct ofpbuf mask;
-bool megaflow;
-
-atomic_read(&enable_megaflows, &megaflow);
-ofpbuf_use_stack(&mask, &mask_bufs[i], sizeof mask_bufs[i]);
-if (megaflow) {
-size_t max_mpls;
-bool recirc;
-
-recirc = ofproto_dpif_get_enable_recirc(upcall->ofproto);
-max_mpls = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);
-odp_flow_key_from_mask(&mask, &upcall->xout.wc.masks,
-   upcall->flow, UINT32_MAX, max_mpls,
-   recirc);
-}
+struct udpif_key *ukey = upcall->ukey;
 
-op = &ops[n_ops++];
-op->ukey = upcall->ukey;
 upcall->ukey = NULL;
+op = &ops[n_ops++];
+op->ukey = ukey;
 op->dop.type = DPIF_OP_FLOW_PUT;
 op->dop.u.flow_put.flags = DPIF_FP_CREATE;
-op->dop.u.flow_put.key = upcall->key;
-op->dop.u.flow_put.key_len = upcall->key_len;
-op->dop.u.flow_put.mask = ofpbuf_data(&mask);
-op->dop.u.flow_put.mask_len = ofpbuf_size(&mask);
+op->dop.u.flow_put.key = ukey->key;
+op->dop.u.flow_put.key_len = ukey->key_len;
+op->dop.u.flow_put.mask = ukey->mask;
+op->dop.u.flow_put.mask_len = ukey->mask_len;
 op->dop.u.flow_put.stats = NULL;
-op->dop.u.flow_put.actions = ofpbuf_data(&upcall->put_actions);
-op->dop.u.flow_put.actions_len = ofpbuf_size(&upcall->put_actions);
+op->dop.u.flow_put.actions = ofpbuf_data(ukey->actions);
+op->dop.u.flow_put.actions_len = ofpbuf_size(ukey->actions);
 }
 
 if (ofpbuf_size(upcall->xout.odp_actions)) {
@@ -1215,11 +1203,11 @@ ukey_new(const struct udpif *udpif, struct upcall 
*upcall)
 OVS_NO_THREAD_SAFETY_ANALYSIS
 {
 struct udpif_key *ukey = xzalloc(sizeof *ukey);
-struct ofpbuf key;
-bool recirc;
+struct ofpbuf key, mask;
+bool recirc, megaflow;
 
 recirc = ofproto_dpif_get_enable_recirc(upcall->ofproto);
-ofpbuf_use_stack(&key, &ukey->key_buf, sizeof ukey->key_buf);
+ofpbuf_use_stack(&key, &ukey->keybuf, sizeof ukey->keybuf);
 if (upcall->key_len) {
 ofpbuf_put(&key, upcall->key, upcall->key_len);
 } else {
@@ -1227,9 +1215,23 @@ ukey_new(const struct udpif *udpif, struct upcall 
*upcall)
upcall->flow->in_port.odp_port, recirc);
 }
 
+atomic_read(&enable_megaflows, &megaflow);
+ofpbuf_use_stack(&mask, &ukey->maskbuf, sizeof ukey->maskbuf);
+if (megaflow) {
+size_t max_mpls;
+
+max_mpls = ofproto_dpif_get_max_mpls_depth(upcall->ofproto);

[ovs-dev] [RFCv2 02/12] revalidator: Protect ukeys with a mutex.

[Joe Stringer]

Currently, udpif_keys are protected during revalidator_sweep__() as only
one thread accesses the ukey at a time. This is ensured using barriers:
all revalidators will be in the GC phase, so they will only access their
own ukey collection.

A future patch will change the access patterns to allow these ukey
collections to be read or modified while a revalidator is garbage
collecting it. To protect the ukeys, this patch adds locking on the ukey
collection.

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
v2: Added ack.
RFC: First post.
---
 ofproto/ofproto-dpif-upcall.c |   29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 428520a..04709d3 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -63,6 +63,9 @@ struct revalidator {
 struct udpif *udpif;   /* Parent udpif. */
 pthread_t thread;  /* Thread ID. */
 unsigned int id;   /* ovsthread_id_self(). */
+struct ovs_mutex *mutex;   /* Points into udpif->ukeys for this
+  revalidator. Required for writing
+  to 'ukeys'. */
 struct cmap *ukeys;/* Points into udpif->ukeys for this
   revalidator. Used for GC phase. */
 };
@@ -408,6 +411,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
 revalidator->udpif = udpif;
 cmap_init(&udpif->ukeys[i].cmap);
 ovs_mutex_init(&udpif->ukeys[i].mutex);
+revalidator->mutex = &udpif->ukeys[i].mutex;
 revalidator->ukeys = &udpif->ukeys[i].cmap;
 revalidator->thread = ovs_thread_create(
 "revalidator", udpif_revalidator, revalidator);
@@ -1487,9 +1491,11 @@ push_dump_ops(struct revalidator *revalidator,
 int i;
 
 push_dump_ops__(revalidator->udpif, ops, n_ops);
+ovs_mutex_lock(revalidator->mutex);
 for (i = 0; i < n_ops; i++) {
 ukey_delete(revalidator, ops[i].ukey);
 }
+ovs_mutex_unlock(revalidator->mutex);
 }
 
 static void
@@ -1590,7 +1596,6 @@ revalidate(struct revalidator *revalidator)
 static bool
 handle_missed_revalidation(struct revalidator *revalidator,
struct udpif_key *ukey)
-OVS_NO_THREAD_SAFETY_ANALYSIS
 {
 struct udpif *udpif = revalidator->udpif;
 struct dpif_flow flow;
@@ -1602,7 +1607,9 @@ handle_missed_revalidation(struct revalidator 
*revalidator,
 
 ofpbuf_use_stub(&buf, &stub, sizeof stub);
 if (!dpif_flow_get(udpif->dpif, ukey->key, ukey->key_len, &buf, &flow)) {
+ovs_mutex_lock(&ukey->mutex);
 keep = revalidate_ukey(udpif, ukey, &flow);
+ovs_mutex_unlock(&ukey->mutex);
 }
 ofpbuf_uninit(&buf);
 
@@ -1611,7 +1618,6 @@ handle_missed_revalidation(struct revalidator 
*revalidator,
 
 static void
 revalidator_sweep__(struct revalidator *revalidator, bool purge)
-OVS_NO_THREAD_SAFETY_ANALYSIS
 {
 struct dump_op ops[REVALIDATE_MAX_BATCH];
 struct udpif_key *ukey;
@@ -1621,13 +1627,18 @@ revalidator_sweep__(struct revalidator *revalidator, 
bool purge)
 n_ops = 0;
 dump_seq = seq_read(revalidator->udpif->dump_seq);
 
-/* During garbage collection, this revalidator completely owns its ukeys
- * map, and therefore doesn't need to do any locking. */
 CMAP_FOR_EACH(ukey, cmap_node, revalidator->ukeys) {
-if (ukey->flow_exists
+bool flow_exists, seq_mismatch;
+
+ovs_mutex_lock(&ukey->mutex);
+flow_exists = ukey->flow_exists;
+seq_mismatch = (ukey->dump_seq != dump_seq
+&& revalidator->udpif->need_revalidate);
+ovs_mutex_unlock(&ukey->mutex);
+
+if (flow_exists
 && (purge
-|| (ukey->dump_seq != dump_seq
-&& revalidator->udpif->need_revalidate
+|| (seq_mismatch
 && !handle_missed_revalidation(revalidator, ukey {
 struct dump_op *op = &ops[n_ops++];
 
@@ -1636,8 +1647,10 @@ revalidator_sweep__(struct revalidator *revalidator, 
bool purge)
 push_dump_ops(revalidator, ops, n_ops);
 n_ops = 0;
 }
-} else if (!ukey->flow_exists) {
+} else if (!flow_exists) {
+ovs_mutex_lock(revalidator->mutex);
 ukey_delete(revalidator, ukey);
+ovs_mutex_unlock(revalidator->mutex);
 }
 }
 
-- 
1.7.10.4

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

[ovs-dev] [RFCv2 04/12] upcall: Rename dump_op -> ukey_op.

[Joe Stringer]

Future patches will make use of the 'struct dump_op' in a broader sense,
so this patch renames it to make things a bit clearer.

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
v2: Added ack.
RFC: First post.
---
 ofproto/ofproto-dpif-upcall.c |   95 +
 1 file changed, 48 insertions(+), 47 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index b3298e1..cfd2af0 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -213,6 +213,13 @@ struct udpif_key {
 };
 };
 
+/* Datapath operation with optional ukey attached. */
+struct ukey_op {
+struct udpif_key *ukey;
+struct dpif_flow_stats stats; /* Stats for 'op'. */
+struct dpif_op dop;   /* Flow operation. */
+};
+
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 static struct list all_udpifs = LIST_INITIALIZER(&all_udpifs);
 
@@ -1056,7 +1063,7 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 {
 struct odputil_keybuf mask_bufs[UPCALL_MAX_BATCH];
 struct dpif_op *opsp[UPCALL_MAX_BATCH * 2];
-struct dpif_op ops[UPCALL_MAX_BATCH * 2];
+struct ukey_op ops[UPCALL_MAX_BATCH * 2];
 unsigned int flow_limit;
 size_t n_ops, i;
 bool may_put;
@@ -1078,7 +1085,7 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 for (i = 0; i < n_upcalls; i++) {
 struct upcall *upcall = &upcalls[i];
 const struct ofpbuf *packet = upcall->packet;
-struct dpif_op *op;
+struct ukey_op *op;
 
 if (upcall->vsp_adjusted) {
 /* This packet was received on a VLAN splinter port.  We added a
@@ -1119,32 +1126,32 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 }
 
 op = &ops[n_ops++];
-op->type = DPIF_OP_FLOW_PUT;
-op->u.flow_put.flags = DPIF_FP_CREATE;
-op->u.flow_put.key = upcall->key;
-op->u.flow_put.key_len = upcall->key_len;
-op->u.flow_put.mask = ofpbuf_data(&mask);
-op->u.flow_put.mask_len = ofpbuf_size(&mask);
-op->u.flow_put.stats = NULL;
-op->u.flow_put.actions = ofpbuf_data(&upcall->put_actions);
-op->u.flow_put.actions_len = ofpbuf_size(&upcall->put_actions);
+op->dop.type = DPIF_OP_FLOW_PUT;
+op->dop.u.flow_put.flags = DPIF_FP_CREATE;
+op->dop.u.flow_put.key = upcall->key;
+op->dop.u.flow_put.key_len = upcall->key_len;
+op->dop.u.flow_put.mask = ofpbuf_data(&mask);
+op->dop.u.flow_put.mask_len = ofpbuf_size(&mask);
+op->dop.u.flow_put.stats = NULL;
+op->dop.u.flow_put.actions = ofpbuf_data(&upcall->put_actions);
+op->dop.u.flow_put.actions_len = ofpbuf_size(&upcall->put_actions);
 }
 
 if (ofpbuf_size(upcall->xout.odp_actions)) {
 op = &ops[n_ops++];
-op->type = DPIF_OP_EXECUTE;
-op->u.execute.packet = CONST_CAST(struct ofpbuf *, packet);
+op->dop.type = DPIF_OP_EXECUTE;
+op->dop.u.execute.packet = CONST_CAST(struct ofpbuf *, packet);
 odp_key_to_pkt_metadata(upcall->key, upcall->key_len,
-&op->u.execute.md);
-op->u.execute.actions = ofpbuf_data(upcall->xout.odp_actions);
-op->u.execute.actions_len = ofpbuf_size(upcall->xout.odp_actions);
-op->u.execute.needs_help = (upcall->xout.slow & SLOW_ACTION) != 0;
+&op->dop.u.execute.md);
+op->dop.u.execute.actions = ofpbuf_data(upcall->xout.odp_actions);
+op->dop.u.execute.actions_len = 
ofpbuf_size(upcall->xout.odp_actions);
+op->dop.u.execute.needs_help = (upcall->xout.slow & SLOW_ACTION) 
!= 0;
 }
 }
 
 /* Execute batch. */
 for (i = 0; i < n_ops; i++) {
-opsp[i] = &ops[i];
+opsp[i] = &ops[i].dop;
 }
 dpif_operate(udpif->dpif, opsp, n_ops);
 }
@@ -1403,40 +1410,34 @@ exit:
 return ok;
 }
 
-struct dump_op {
-struct udpif_key *ukey;
-struct dpif_flow_stats stats; /* Stats for 'op'. */
-struct dpif_op op;/* Flow del operation. */
-};
-
 static void
-dump_op_init(struct dump_op *op, const struct nlattr *key, size_t key_len,
- struct udpif_key *ukey)
+delete_op_init(struct ukey_op *op, const struct nlattr *key, size_t key_len,
+   struct udpif_key *ukey)
 {
 op->ukey = ukey;
-op->op.type = DPIF_OP_FLOW_DEL;
-op->op.u.flow_del.key = key;
-op->op.u.flow_del.key_len = key_len;
-op->op.u.flow_del.stats = &op->stats;
+op->dop.type = DPIF_OP_FLOW_DEL;
+op->dop.u.flow_del.key = key;
+op->dop.u.flow_del.key_len = key_len;
+op->dop.u.flow_del.stats = &op->stats;
 }
 
 static void
-push_dump_ops__(struct udpif *udpif, struct dump_op *ops, size_t n_ops)
+push_ukey_ops__(struct udpif *udp

[ovs-dev] [RFCv2 07/12] hash: Add 128-bit murmurhash.

[Joe Stringer]

Add the 128-bit murmurhash by Austin Appleby, for 32-bit systems from:
http://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp

Signed-off-by: Joe Stringer 
---
There is also a version for 64-bit systems which I haven't tried yet,
mostly because this version provides the same code for x32/x64. I can
look at adding the alternative if we think it's worth doing.
---
 include/openvswitch/types.h |5 ++
 lib/hash.c  |  194 ++-
 lib/hash.h  |4 +-
 3 files changed, 201 insertions(+), 2 deletions(-)

diff --git a/include/openvswitch/types.h b/include/openvswitch/types.h
index 54541a4..3fb1243 100644
--- a/include/openvswitch/types.h
+++ b/include/openvswitch/types.h
@@ -81,6 +81,11 @@ typedef struct {
 #endif
 } ovs_32aligned_u64;
 
+/* XXX: endianness? */
+typedef struct {
+uint32_t h[4];
+} ovs_u128;
+
 /* A 64-bit value, in network byte order, that is only aligned on a 32-bit
  * boundary. */
 typedef struct {
diff --git a/lib/hash.c b/lib/hash.c
index 71cd74c..26a031e 100644
--- a/lib/hash.c
+++ b/lib/hash.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,6 +50,198 @@ hash_bytes(const void *p_, size_t n, uint32_t basis)
 return hash_finish(hash, orig_n);
 }
 
+static void mhash128_add(ovs_u128 *hashp, const uint32_t *data)
+{
+const uint32_t c1 = 0x239b961b;
+const uint32_t c2 = 0xab0e9789;
+const uint32_t c3 = 0x38b34ae5;
+const uint32_t c4 = 0xa1e38b93;
+uint32_t k1 = data[0];
+uint32_t k2 = data[1];
+uint32_t k3 = data[2];
+uint32_t k4 = data[3];
+
+ovs_u128 hash = *hashp;
+
+k1 *= c1;
+k1  = hash_rot(k1,15);
+k1 *= c2;
+hash.h[0] ^= k1;
+
+hash.h[0] = hash_rot(hash.h[0],19);
+hash.h[0] += hash.h[1];
+hash.h[0] = hash.h[0]*5+0x561ccd1b;
+
+k2 *= c2;
+k2  = hash_rot(k2,16);
+k2 *= c3;
+hash.h[1] ^= k2;
+
+hash.h[1] = hash_rot(hash.h[1],17);
+hash.h[1] += hash.h[2];
+hash.h[1] = hash.h[1]*5+0x0bcaa747;
+
+k3 *= c3;
+k3  = hash_rot(k3,17);
+k3 *= c4;
+hash.h[2] ^= k3;
+
+hash.h[2] = hash_rot(hash.h[2],15);
+hash.h[2] += hash.h[3];
+hash.h[2] = hash.h[2]*5+0x96cd1c35;
+
+k4 *= c4;
+k4  = hash_rot(k4,18);
+k4 *= c1;
+hash.h[3] ^= k4;
+
+hash.h[3] = hash_rot(hash.h[3],13);
+hash.h[3] += hash.h[0];
+hash.h[3] = hash.h[3]*5+0x32ac3b17;
+
+*hashp = hash;
+}
+
+static void mhash128_add_tail(ovs_u128 *hashp, const uint8_t *data, size_t len)
+{
+const uint32_t c1 = 0x239b961b;
+const uint32_t c2 = 0xab0e9789;
+const uint32_t c3 = 0x38b34ae5;
+const uint32_t c4 = 0xa1e38b93;
+uint32_t k1 = 0;
+uint32_t k2 = 0;
+uint32_t k3 = 0;
+uint32_t k4 = 0;
+ovs_u128 hash = *hashp;
+
+switch(len & 15) {
+case 15:
+k4 ^= data[14] << 16;
+case 14:
+k4 ^= data[13] << 8;
+case 13:
+k4 ^= data[12] << 0;
+k4 *= c4;
+k4  = hash_rot(k4,18);
+k4 *= c1; hash.h[3] ^= k4;
+
+case 12:
+k3 ^= data[11] << 24;
+case 11:
+k3 ^= data[10] << 16;
+case 10:
+k3 ^= data[ 9] << 8;
+case  9:
+k3 ^= data[ 8] << 0;
+k3 *= c3;
+k3  = hash_rot(k3,17);
+k3 *= c4;
+hash.h[2] ^= k3;
+
+case  8:
+k2 ^= data[ 7] << 24;
+case  7:
+k2 ^= data[ 6] << 16;
+case  6:
+k2 ^= data[ 5] << 8;
+case  5:
+k2 ^= data[ 4] << 0;
+k2 *= c2;
+k2  = hash_rot(k2,16);
+k2 *= c3;
+hash.h[1] ^= k2;
+
+case  4:
+k1 ^= data[ 3] << 24;
+case  3:
+k1 ^= data[ 2] << 16;
+case  2:
+k1 ^= data[ 1] << 8;
+case  1:
+k1 ^= data[ 0] << 0;
+k1 *= c1;
+k1  = hash_rot(k1,15);
+k1 *= c2;
+hash.h[0] ^= k1;
+};
+
+*hashp = hash;
+}
+
+static void
+fmix32(ovs_u128 *in, ovs_u128 *out)
+{
+int i, h;
+
+for (i = 0; i < 4; i++) {
+h = in->h[i];
+
+h ^= h >> 16;
+h *= 0x85ebca6b;
+h ^= h >> 13;
+h *= 0xc2b2ae35;
+h ^= h >> 16;
+
+out->h[i] = h;
+}
+}
+
+static void
+mhash128_finish(ovs_u128 *hashp, size_t len)
+{
+ovs_u128 hash = *hashp;
+
+hash.h[0] ^= len;
+hash.h[1] ^= len;
+hash.h[2] ^= len;
+hash.h[3] ^= len;
+
+hash.h[0] += hash.h[1];
+hash.h[0] += hash.h[2];
+hash.h[0] += hash.h[3];
+hash.h[1] += hash.h[0];
+hash.h[2] += hash.h[0];
+hash.h[3] += hash.h[0];
+
+fmix32(&hash, &hash);
+
+hash.h[0] += hash.h[1];
+hash.h[0] += hash.h[2];
+hash.h[0] += hash.h[3];
+hash.h[1] += hash.h[0];
+hash.h[2] += hash.h[0];
+hash.h[3] += hash.h[0];
+
+

[ovs-dev] [RFCv2 10/12] dpif-netdev: Support unique flow identifiers.

[Joe Stringer]

Signed-off-by: Joe Stringer 
---
 lib/dpif-netdev.c   |  210 ---
 lib/flow.h  |6 ++
 tests/dpif-netdev.at|3 +
 tests/ofproto-dpif.at   |   20 +++--
 tests/ofproto-macros.at |1 +
 5 files changed, 165 insertions(+), 75 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 32c738a..6ddfd54 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -228,8 +228,9 @@ struct dp_netdev_flow {
 /* Packet classification. */
 const struct cls_rule cr;   /* In owning dp_netdev's 'cls'. */
 
-/* Hash table index by unmasked flow. */
+/* Hash table index by uid. */
 const struct cmap_node node; /* In owning dp_netdev's 'flow_table'. */
+const ovs_u128 uid;  /* Unique flow identifier. */
 const struct flow flow;  /* The flow that created this entry. */
 
 /* Number of references.
@@ -921,7 +922,7 @@ dp_netdev_remove_flow(struct dp_netdev *dp, struct 
dp_netdev_flow *flow)
 struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow->node);
 
 classifier_remove(&dp->cls, cr);
-cmap_remove(&dp->flow_table, node, flow_hash(&flow->flow, 0));
+cmap_remove(&dp->flow_table, node, flow_uid_hash(&flow->uid, 0));
 
 dp_netdev_flow_unref(flow);
 }
@@ -1042,13 +1043,13 @@ dp_netdev_lookup_flow(const struct dp_netdev *dp, const 
struct miniflow *key)
 }
 
 static struct dp_netdev_flow *
-dp_netdev_find_flow(const struct dp_netdev *dp, const struct flow *flow)
+dp_netdev_find_flow(const struct dp_netdev *dp, const ovs_u128 *uid)
 {
 struct dp_netdev_flow *netdev_flow;
 
-CMAP_FOR_EACH_WITH_HASH (netdev_flow, node, flow_hash(flow, 0),
+CMAP_FOR_EACH_WITH_HASH (netdev_flow, node, flow_uid_hash(uid, 0),
  &dp->flow_table) {
-if (flow_equal(&netdev_flow->flow, flow)) {
+if (!memcmp(&netdev_flow->uid, uid, sizeof *uid)) {
 return netdev_flow;
 }
 }
@@ -1074,30 +1075,82 @@ get_dpif_flow_stats(const struct dp_netdev_flow 
*netdev_flow,
 }
 }
 
+struct dp_netdev_flow_convert_buf {
+struct ofpbuf *key, *mask, *uid;
+bool actions;
+};
+
 static void
-dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
-struct ofpbuf *buffer, struct dpif_flow *flow)
+dp_netdev_init_flow_convert(struct dp_netdev_flow_convert_buf *convert,
+uint32_t flags, struct ofpbuf *key,
+struct ofpbuf *mask, struct ofpbuf *uid)
+{
+memset(convert, 0, sizeof *convert);
+if (!(flags & OVS_UID_F_SKIP_KEY)) {
+convert->key = key;
+}
+if (!(flags & OVS_UID_F_SKIP_MASK)) {
+convert->mask = mask;
+}
+if (!(flags & OVS_UID_F_SKIP_ACTIONS)) {
+convert->actions = true;
+}
+convert->uid = uid;
+}
+
+static void
+dp_netdev_flow_to_dpif_flow__(const struct dp_netdev_flow *netdev_flow,
+  struct dpif_flow *flow,
+  struct dp_netdev_flow_convert_buf *buf)
 {
 struct flow_wildcards wc;
-struct dp_netdev_actions *actions;
+size_t offset;
 
 minimask_expand(&netdev_flow->cr.match.mask, &wc);
-odp_flow_key_from_mask(buffer, &wc.masks, &netdev_flow->flow,
-   odp_to_u32(wc.masks.in_port.odp_port),
-   SIZE_MAX, true);
-flow->mask = ofpbuf_data(buffer);
-flow->mask_len = ofpbuf_size(buffer);
 
-actions = dp_netdev_flow_get_actions(netdev_flow);
-flow->actions = actions->actions;
-flow->actions_len = actions->size;
+memset(flow, 0, sizeof *flow);
+if (buf->key) {
+offset = ofpbuf_size(buf->key);
+flow->key = ofpbuf_tail(buf->key);
+odp_flow_key_from_flow(buf->key, &netdev_flow->flow, &wc.masks,
+   netdev_flow->flow.in_port.odp_port, true);
+flow->key_len = ofpbuf_size(buf->key) - offset;
+}
+if (buf->mask) {
+offset = ofpbuf_size(buf->mask);
+flow->mask = ofpbuf_tail(buf->mask);
+odp_flow_key_from_mask(buf->mask, &wc.masks, &netdev_flow->flow,
+   odp_to_u32(wc.masks.in_port.odp_port),
+   SIZE_MAX, true);
+flow->mask_len = ofpbuf_size(buf->mask) - offset;
+}
+
+if (buf->actions) {
+struct dp_netdev_actions *netdev_actions;
 
-flow->uid = NULL;
-flow->uid_len = 0;
+netdev_actions = dp_netdev_flow_get_actions(netdev_flow);
+flow->actions = netdev_actions->actions;
+flow->actions_len = netdev_actions->size;
+}
 
+offset = ofpbuf_size(buf->uid);
+flow->uid = ofpbuf_tail(buf->uid);
+odp_uid_to_nlattrs(buf->uid, &netdev_flow->uid, 0);
+flow->uid_len = ofpbuf_size(buf->uid) - offset;
 get_dpif_flow_stats(netdev_flow, &flow->stats);
 }
 
+static void
+dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
+   

[ovs-dev] [RFCv2 09/12] upcall: Generate unique flow identifiers.

[Joe Stringer]

This patch modifies the way that ukey lookup is done, to use a 128-bit
hash of the flow key as Netlink attributes. This hash is generated at
flow installation time, and passed down to the dpif so that datapaths
which support UID can index based on this ID rather than the flow.

For backward compatibility, flows that are dumped with no ukey will be
hashed in the same manner as flow setup, so that the flow may be looked
up. For this reason, it is important that two flows with the same UID
are not installed. Handlers enforce this and track conflicts with the
coverage counter 'handler_install_conflict'.

Signed-off-by: Joe Stringer 
---
 ofproto/ofproto-dpif-upcall.c |  198 ++---
 ofproto/ofproto-dpif-upcall.h |3 +
 ofproto/ofproto-dpif.c|  103 +++--
 ofproto/ofproto-dpif.h|2 +
 4 files changed, 267 insertions(+), 39 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index fd3ab60..a96c1f6 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -115,6 +115,7 @@ struct udpif {
 struct dpif_flow_dump *dump;   /* DPIF flow dump state. */
 long long int dump_duration;   /* Duration of the last flow dump. */
 struct seq *dump_seq;  /* Increments each dump iteration. */
+atomic_bool enable_uid;/* If false, dumps flows verbosely.  */
 
 /* There are 'N_UMAPS' maps containing 'struct udpif_key' elements.
  *
@@ -166,6 +167,7 @@ struct upcall {
 bool xout_initialized; /* True if 'xout' must be uninitialized. */
 struct xlate_out xout; /* Result of xlate_actions(). */
 struct ofpbuf put_actions; /* Actions 'put' in the fastapath. */
+ovs_u128 *uid; /* The flow's unique identifier. */
 
 struct dpif_ipfix *ipfix;  /* IPFIX pointer or NULL. */
 struct dpif_sflow *sflow;  /* SFlow pointer or NULL. */
@@ -204,6 +206,7 @@ struct udpif_key {
 const struct nlattr *mask; /* Datapath flow mask. */
 size_t mask_len;   /* Length of 'mask'. */
 struct ofpbuf *actions;/* Datapath flow actions as nlattrs. */
+ovs_u128 uid;  /* Unique flow identifier. */
 uint32_t hash; /* Pre-computed hash for 'key'. */
 
 struct ovs_mutex mutex;   /* Guards the following. */
@@ -228,6 +231,7 @@ struct udpif_key {
 struct ukey_op {
 struct udpif_key *ukey;
 struct dpif_flow_stats stats; /* Stats for 'op'. */
+struct odputil_uidbuf uid;/* Storage for Netlink-formatted uid. */
 struct dpif_op dop;   /* Flow operation. */
 };
 
@@ -253,19 +257,26 @@ static void upcall_unixctl_disable_megaflows(struct 
unixctl_conn *, int argc,
  const char *argv[], void *aux);
 static void upcall_unixctl_enable_megaflows(struct unixctl_conn *, int argc,
 const char *argv[], void *aux);
+static void upcall_unixctl_disable_uid(struct unixctl_conn *, int argc,
+   const char *argv[], void *aux);
+static void upcall_unixctl_enable_uid(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 static void upcall_unixctl_set_flow_limit(struct unixctl_conn *conn, int argc,
 const char *argv[], void *aux);
 static void upcall_unixctl_dump_wait(struct unixctl_conn *conn, int argc,
  const char *argv[], void *aux);
 
-static struct udpif_key *ukey_new(const struct udpif *, struct upcall *);
+static uint32_t get_uid_hash(const ovs_u128 *uid);
+
+static struct udpif_key *ukey_new(struct udpif *, struct upcall *);
 static bool ukey_install_start(struct udpif *, struct udpif_key *ukey);
 static void ukey_install_finish(struct udpif_key *ukey, int error);
 static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
-static struct udpif_key *ukey_lookup(struct udpif *udpif, uint32_t hash,
+static struct udpif_key *ukey_lookup(struct udpif *udpif,
+ const struct nlattr *uid, size_t uid_len,
  const struct nlattr *key, size_t key_len);
-static int ukey_acquire(struct udpif *, const struct dpif_flow *,
-struct udpif_key **result, int *error);
+static struct udpif_key *ukey_lookup_by_uid(struct udpif *udpif,
+const ovs_u128 *uid);
 static void ukey_delete__(struct udpif_key *);
 static void ukey_delete(struct umap *, struct udpif_key *);
 static enum upcall_type classify_upcall(enum dpif_upcall_type type,
@@ -293,6 +304,10 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
  upcall_unixctl_disable_megaflows, NULL);
 unixctl_command_register("upcall/enable-megaflows

[ovs-dev] [RFCv2 12/12] revalidator: Reduce ukey contention.

[Joe Stringer]

When handler threads are installing ukeys, they hold the ukey mutex for
the duration of flow setup. If a revalidator thread dumps this flow
while the handler holds the lock, it will attempt to lock it using
ovs_mutex_trylock(), then if it cannot grab the lock, skip the flow.

Attempting to lock on a ukey that is currently locked is a common
occurrence when:
 - There are many handler threads running, and
 - Large numbers of small flows are passing through OVS.

In these cases, the act of attempting to lock the ukey is quite
expensive. This patch adds a flag 'installed' to the ukey, which is
raised when flow setup is complete. By checking this flag before
attempting to perform a trylock(), we can reduce lock contention between
handler and revalidator threads.

Signed-off-by: Joe Stringer 
---
 ofproto/ofproto-dpif-upcall.c |   29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index a96c1f6..d1e33f1 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -208,14 +208,17 @@ struct udpif_key {
 struct ofpbuf *actions;/* Datapath flow actions as nlattrs. */
 ovs_u128 uid;  /* Unique flow identifier. */
 uint32_t hash; /* Pre-computed hash for 'key'. */
+atomic_bool installed; /* True if the datapath flow has been
+  installed and handlers are finished with
+  this ukey. Used to reduce contention. */
 
 struct ovs_mutex mutex;   /* Guards the following. */
 struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
 long long int created OVS_GUARDED;/* Estimate of creation time. */
 uint64_t dump_seq OVS_GUARDED;/* Tracks udpif->dump_seq. */
 uint64_t reval_seq OVS_GUARDED;   /* Tracks udpif->reval_seq. */
-bool flow_exists OVS_GUARDED; /* Ensures flows are only deleted
- once. */
+bool flow_exists OVS_GUARDED; /* The flow is currently in the
+ datapath. */
 
 struct xlate_cache *xcache OVS_GUARDED;   /* Cache for xlate entries that
* are affected by this ukey.
@@ -1306,6 +1309,7 @@ ukey_new(struct udpif *udpif, struct upcall *upcall)
 ukey->hash = get_uid_hash(&ukey->uid);
 ukey->actions = ofpbuf_clone(&upcall->put_actions);
 
+atomic_init(&ukey->installed, false);
 ovs_mutex_init(&ukey->mutex);
 ukey->dump_seq = upcall->dump_seq;
 ukey->reval_seq = upcall->reval_seq;
@@ -1349,6 +1353,7 @@ ukey_install_finish(struct udpif_key *ukey, int error)
 {
 if (!error) {
 ukey->flow_exists = true;
+atomic_store(&ukey->installed, true);
 }
 ovs_mutex_unlock(&ukey->mutex);
 }
@@ -1364,6 +1369,22 @@ ukey_install(struct udpif *udpif, struct udpif_key *ukey)
 return true;
 }
 
+/* Wrapper for ovs_mutex_trylock() which reduces contention between handler
+ * threads and revalidator threads by not bothering to try locking if the
+ * flow is still being processed. */
+static int
+ukey_trylock(struct udpif_key *ukey)
+OVS_TRY_LOCK(0, ukey->mutex)
+{
+bool installed;
+
+atomic_read(&ukey->installed, &installed);
+if (!installed) {
+return EBUSY;
+}
+return ovs_mutex_trylock(&ukey->mutex);
+}
+
 /* Searches for a ukey in 'udpif->ukeys' that matches 'flow' and attempts to
  * lock the ukey.
  *
@@ -1385,7 +1406,7 @@ ukey_acquire(struct udpif *udpif, const struct dpif_flow 
*flow,
 ukey = ukey_lookup(udpif, flow->uid, flow->uid_len,
flow->key, flow->key_len);
 if (ukey) {
-retval = ovs_mutex_trylock(&ukey->mutex);
+retval = ukey_trylock(ukey);
 } else {
 struct ds ds = DS_EMPTY_INITIALIZER;
 ovs_u128 uid;
@@ -1842,7 +1863,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool 
purge)
 
 /* Handler threads could be holding a ukey lock while it installs a
  * new flow, so don't hang around waiting for access to it. */
-if (ovs_mutex_trylock(&ukey->mutex)) {
+if (ukey_trylock(ukey)) {
 continue;
 }
 flow_exists = ukey->flow_exists;
-- 
1.7.10.4

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

[ovs-dev] [RFCv2 11/12] dpif-linux: Support unique flow identifiers.

[Joe Stringer]

Signed-off-by: Joe Stringer 
---
 lib/dpif-linux.c |   91 +-
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index f4e78a1..a5f4182 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -110,6 +110,8 @@ struct dpif_linux_flow {
 size_t mask_len;
 const struct nlattr *actions;   /* OVS_FLOW_ATTR_ACTIONS. */
 size_t actions_len;
+const struct nlattr *uid;   /* OVS_FLOW_ATTR_UID. */
+size_t uid_len;
 const struct ovs_flow_stats *stats; /* OVS_FLOW_ATTR_STATS. */
 const uint8_t *tcp_flags;   /* OVS_FLOW_ATTR_TCP_FLAGS. */
 const ovs_32aligned_u64 *used;  /* OVS_FLOW_ATTR_USED. */
@@ -1050,25 +1052,16 @@ dpif_linux_port_poll_wait(const struct dpif *dpif_)
 
 static void
 dpif_linux_init_flow_get(const struct dpif_linux *dpif,
- const struct nlattr *key, size_t key_len,
+ const struct dpif_flow_get *get,
  struct dpif_linux_flow *request)
 {
 dpif_linux_flow_init(request);
 request->cmd = OVS_FLOW_CMD_GET;
 request->dp_ifindex = dpif->dp_ifindex;
-request->key = key;
-request->key_len = key_len;
-}
-
-static int
-dpif_linux_flow_get(const struct dpif_linux *dpif,
-const struct nlattr *key, size_t key_len,
-struct dpif_linux_flow *reply, struct ofpbuf **bufp)
-{
-struct dpif_linux_flow request;
-
-dpif_linux_init_flow_get(dpif, key, key_len, &request);
-return dpif_linux_flow_transact(&request, reply, bufp);
+request->key = get->key;
+request->key_len = get->key_len;
+request->uid = get->uid;
+request->uid_len = get->uid_len;
 }
 
 static void
@@ -1085,6 +1078,9 @@ dpif_linux_init_flow_put(struct dpif_linux *dpif, const 
struct dpif_flow_put *pu
 request->key_len = put->key_len;
 request->mask = put->mask;
 request->mask_len = put->mask_len;
+request->uid = put->uid;
+request->uid_len = put->uid_len;
+
 /* Ensure that OVS_FLOW_ATTR_ACTIONS will always be included. */
 request->actions = (put->actions
 ? put->actions
@@ -1105,6 +1101,8 @@ dpif_linux_init_flow_del(struct dpif_linux *dpif, const 
struct dpif_flow_del *de
 request->dp_ifindex = dpif->dp_ifindex;
 request->key = del->key;
 request->key_len = del->key_len;
+request->uid = del->uid;
+request->uid_len = del->uid_len;
 }
 
 struct dpif_linux_flow_dump {
@@ -1120,13 +1118,13 @@ dpif_linux_flow_dump_cast(struct dpif_flow_dump *dump)
 }
 
 static struct dpif_flow_dump *
-dpif_linux_flow_dump_create(const struct dpif *dpif_,
-uint32_t dump_flags OVS_UNUSED)
+dpif_linux_flow_dump_create(const struct dpif *dpif_, uint32_t dump_flags)
 {
 const struct dpif_linux *dpif = dpif_linux_cast(dpif_);
 struct dpif_linux_flow_dump *dump;
 struct dpif_linux_flow request;
-struct ofpbuf *buf;
+struct ofpbuf *buf, uid;
+struct odputil_uidbuf uid_buf;
 
 dump = xmalloc(sizeof *dump);
 dpif_flow_dump_init(&dump->up, dpif_);
@@ -1135,6 +1133,11 @@ dpif_linux_flow_dump_create(const struct dpif *dpif_,
 request.cmd = OVS_FLOW_CMD_GET;
 request.dp_ifindex = dpif->dp_ifindex;
 
+ofpbuf_use_stack(&uid, &uid_buf, sizeof uid_buf);
+odp_uid_to_nlattrs(&uid, NULL, dump_flags);
+request.uid = ofpbuf_data(&uid);
+request.uid_len = ofpbuf_size(&uid);
+
 buf = ofpbuf_new(1024);
 dpif_linux_flow_to_ofpbuf(&request, buf);
 nl_dump_start(&dump->nl_dump, NETLINK_GENERIC, buf);
@@ -1207,8 +1210,8 @@ dpif_linux_flow_to_dpif_flow(struct dpif_flow *dpif_flow,
 dpif_flow->mask_len = linux_flow->mask_len;
 dpif_flow->actions = linux_flow->actions;
 dpif_flow->actions_len = linux_flow->actions_len;
-dpif_flow->uid = NULL;
-dpif_flow->uid_len = 0;
+dpif_flow->uid = linux_flow->uid;
+dpif_flow->uid_len = linux_flow->uid_len;
 dpif_linux_flow_get_stats(linux_flow, &dpif_flow->stats);
 }
 
@@ -1219,7 +1222,6 @@ dpif_linux_flow_dump_next(struct dpif_flow_dump_thread 
*thread_,
 struct dpif_linux_flow_dump_thread *thread
 = dpif_linux_flow_dump_thread_cast(thread_);
 struct dpif_linux_flow_dump *dump = thread->dump;
-struct dpif_linux *dpif = dpif_linux_cast(thread->up.dpif);
 int n_flows;
 
 ofpbuf_delete(thread->nl_actions);
@@ -1244,30 +1246,7 @@ dpif_linux_flow_dump_next(struct dpif_flow_dump_thread 
*thread_,
 break;
 }
 
-if (linux_flow.actions) {
-/* Common case: the flow includes actions. */
-dpif_linux_flow_to_dpif_flow(&flows[n_flows++], &linux_flow);
-} else {
-/* Rare case: the flow does not include actions.  Retrieve this
- * individual flow again to get the actions. */
-error = dpif_linux_flow_get(dpif, linux_flow.key,
-  

Re: [ovs-dev] [PATCH] lib/odp: coverity null check in odp_execute_sample

[Thomas Graf]

On 08/26/14 at 06:00pm, Madhu Challa wrote:
> call odp_execute_actions__ only if there are any subactions to prevent null
> pointer dereference in nl_attr_get().
> 
> Signed-off-by: Madhu Challa 
> ---
>  lib/odp-execute.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index e1e9b57..607af03 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -194,6 +194,9 @@ odp_execute_sample(void *dp, struct dpif_packet
> *packet, bool steal,
>  }
>  }
> 
> +if (subactions == NULL)
> +return;
> +

Or we call odp_execute_actions__() in the OVS_SAMPLE_ATTR_ACTIONS
case directly and avoid the checks in the fast path. OVS is controlling
the attribute ordering and we rely on it in a lot of other places.
A probability attribute after the sub actions should be considered a bug.

An OVS_NOT_REACHED() after the loop can handle the no sub actions case
with an assert without affecting the fast path.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Question about metadata matching

[Thomas Graf]

On 08/26/14 at 03:23pm, Junguk Cho wrote:
> What I thought is to generally use metadata as matching rule in datapath.
> It is not just between tables.

You may be looking for the 'mark' field of the flow. It is known in
the datapath and used across different Linux kernel subsystems.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Open vSwitch details

[Irina Povolotskaya]

Hello,

I'm updating information on Open vSwitch, so I believe you may help me.
I'd like to know, what Neutron releases Open vSwitch is in trunk.

Thank you for your help.

Best regards,
Irina.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [v3 2/4] datapath: Refactor ovs_dp_process_packet()

[Andy Zhou]

Split ovs_dp_packet_flow_lookup() into its own API. In preparation for
the next patch.

Signed-off-by: Andy Zhou 
---
 datapath/datapath.c | 31 ++-
 datapath/datapath.h |  1 +
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index b61e0ae..5bae23e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -252,13 +252,10 @@ void ovs_dp_detach_port(struct vport *p)
 }
 
 /* Must be called with rcu_read_lock. */
-void ovs_dp_process_packet(struct sk_buff *skb)
+struct sw_flow *ovs_dp_packet_flow_lookup(struct datapath *dp, struct sk_buff 
*skb)
 {
-   const struct vport *p = OVS_CB(skb)->input_vport;
struct sw_flow_key *pkt_key = OVS_CB(skb)->pkt_key;
-   struct datapath *dp = p->dp;
struct sw_flow *flow;
-   struct sw_flow_actions *sf_acts;
struct dp_stats_percpu *stats;
u64 *stats_counter;
u32 n_mask_hit;
@@ -266,10 +263,11 @@ void ovs_dp_process_packet(struct sk_buff *skb)
stats = this_cpu_ptr(dp->stats_percpu);
 
/* Look up flow. */
-   flow = ovs_flow_tbl_lookup_stats(&dp->table, pkt_key, skb_get_hash(skb),
-&n_mask_hit);
+   flow = ovs_flow_tbl_lookup_stats(&dp->table, pkt_key,
+skb_get_hash(skb), &n_mask_hit);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
+   const struct vport *p = OVS_CB(skb)->input_vport;
 
upcall.cmd = OVS_PACKET_CMD_MISS;
upcall.userdata = NULL;
@@ -282,9 +280,6 @@ void ovs_dp_process_packet(struct sk_buff *skb)
}
 
ovs_flow_stats_update(flow, pkt_key->tp.flags, skb);
-
-   sf_acts = rcu_dereference(flow->sf_acts);
-   ovs_execute_actions(dp, skb, sf_acts);
stats_counter = &stats->n_hit;
 
 out:
@@ -293,6 +288,24 @@ out:
(*stats_counter)++;
stats->n_mask_hit += n_mask_hit;
u64_stats_update_end(&stats->sync);
+
+   return flow;
+}
+
+/* Must be called with rcu_read_lock. */
+void ovs_dp_process_packet(struct sk_buff *skb)
+{
+   const struct vport *p = OVS_CB(skb)->input_vport;
+   struct datapath *dp = p->dp;
+   struct sw_flow *flow;
+
+   flow =ovs_dp_packet_flow_lookup(dp, skb);
+   if (flow) {
+   struct sw_flow_actions *sf_acts;
+
+   sf_acts = rcu_dereference(flow->sf_acts);
+   ovs_execute_actions(dp, skb, sf_acts);
+   }
 }
 
 int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
diff --git a/datapath/datapath.h b/datapath/datapath.h
index dbe58d4..66fa84f 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -190,6 +190,7 @@ extern struct genl_family dp_vport_genl_family;
 extern struct genl_multicast_group ovs_dp_vport_multicast_group;
 
 void ovs_dp_process_packet(struct sk_buff *c);
+struct sw_flow *ovs_dp_packet_flow_lookup(struct datapath *dp, struct sk_buff 
*c);
 void ovs_dp_detach_port(struct vport *);
 int ovs_dp_upcall(struct datapath *, struct sk_buff *,
  const struct dp_upcall_info *);
-- 
1.9.1

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

[ovs-dev] [v3 3/4] datapath: Implement recirc action without recursion

[Andy Zhou]

Since kernel stack is limited in size, it is not wise to using
recursive function with large stack frames.

This patch provides an alternative implementation of recirc action
without using recursion.

A per CPU fixed sized, 'deferred action stack' is used to store both
recirc and sample actions when executing actions. These actions becomes
'deferred actions".

Deferred actions are only executed after all other actions has been
executed, including the ones triggered by loopback from the kernel
network stack.

The depth of the private stack, currently set to 10, limits the number
of 'deferred actions' can be accumulated for each packet.

Signed-off-by: Andy Zhou 
---
 datapath/actions.c  | 150 +---
 datapath/actions.h  |   4 +-
 datapath/datapath.c |   4 +-
 3 files changed, 112 insertions(+), 46 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 31fb57d..662de9c 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -42,6 +42,7 @@
 #include "actions.h"
 
 static DEFINE_PER_CPU(struct ovs_action_stack, action_stacks);
+DEFINE_PER_CPU(int, ovs_exec_actions_count);
 
 static inline void action_stack_init(struct ovs_action_stack *stack)
 {
@@ -73,6 +74,24 @@ action_stack_push(struct ovs_action_stack *stack)
return &stack->stack[stack->top++];
 }
 
+static inline struct ovs_deferred_action *
+push_actions(struct sk_buff *skb, const struct nlattr *attr, int len)
+{
+   struct ovs_action_stack *stack;
+   struct ovs_deferred_action *da;
+
+   stack = this_cpu_ptr(&(action_stacks));
+   da = action_stack_push(stack);
+
+   if (da) {
+   da->skb = skb;
+   da->actions = attr;
+   da->rem = len;
+   }
+
+   return da;
+}
+
 static void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key)
 {
*new_key = *OVS_CB(skb)->pkt_key;
@@ -173,8 +192,8 @@ static bool is_skb_flow_key_valid(struct sk_buff *skb)
return !!OVS_CB(skb)->pkt_key->eth.type;
 }
 
-static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
- const struct nlattr *attr, int len);
+static void do_execute_actions(struct datapath *dp, struct sk_buff *skb,
+  const struct nlattr *attr, int len);
 
 static int make_writable(struct sk_buff *skb, int write_len)
 {
@@ -718,10 +737,8 @@ static bool last_action(const struct nlattr *a, int rem)
 static int sample(struct datapath *dp, struct sk_buff *skb,
  const struct nlattr *attr)
 {
-   struct sw_flow_key sample_key;
const struct nlattr *acts_list = NULL;
const struct nlattr *a;
-   struct sk_buff *sample_skb;
int rem;
 
for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
@@ -741,33 +758,39 @@ static int sample(struct datapath *dp, struct sk_buff 
*skb,
rem = nla_len(acts_list);
a = nla_data(acts_list);
 
+   if (unlikely(rem == 0))
+   return 0;
+
/* Actions list is either empty or only contains a single user-space
 * action, the latter being a special case as it is the only known
 * usage of the sample action.
 * In these special cases don't clone the skb as there are no
 * side-effects in the nested actions.
 * Otherwise, clone in case the nested actions have side effects. */
-   if (likely(rem == 0 ||
-  (nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
-   last_action(a, rem {
-   sample_skb = skb;
-   skb_get(skb);
+   if (nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
+   last_action(a, rem)) {
+   return output_userspace(dp, skb, a);
} else {
-   sample_skb = skb_clone(skb, GFP_ATOMIC);
-   if (!sample_skb)
-   /* Skip the sample action when out of memory. */
+   struct ovs_deferred_action *da;
+
+   skb = skb_clone(skb, GFP_ATOMIC);
+   if (!skb)
+   /* Out of memory, skip the sample action. */
+   return 0;
+
+   da = push_actions(skb, a, rem);
+   if (!da) {
+   if (net_ratelimit())
+   pr_warn("%s: stack limit reached, drop sample 
action\n",
+   ovs_dp_name(dp));
+   kfree_skb(skb);
return 0;
+   }
 
-   flow_key_clone(skb, &sample_key);
+   flow_key_clone(skb, &da->pkt_key);
}
 
-   /* Note that do_execute_actions() never consumes skb.
-* In the case where skb has been cloned above it is the clone that
-* is consumed.  Otherwise the skb_get(skb) call prevents
-* consumption by do_execute_actions(). Thus, it is safe to simply
-* return the error code and let the caller (also
-* do_execut

[ovs-dev] [v3 1/4] datapath: Add per-CPU recirc stack infrasturcutre

[Andy Zhou]

Future pathces will make use of those functions.

Signed-off-by: Andy Zhou 
---
 datapath/Modules.mk |  1 +
 datapath/actions.c  | 57 +
 datapath/actions.h  | 54 ++
 datapath/datapath.c |  4 
 datapath/datapath.h |  4 +---
 5 files changed, 117 insertions(+), 3 deletions(-)
 create mode 100644 datapath/actions.h

diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 90e158c..2e74f6e 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -23,6 +23,7 @@ openvswitch_sources = \
 
 openvswitch_headers = \
compat.h \
+   actions.h \
datapath.h \
flow.h \
flow_netlink.h \
diff --git a/datapath/actions.c b/datapath/actions.c
index 9ac5f7b..31fb57d 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -39,6 +39,39 @@
 #include "mpls.h"
 #include "vlan.h"
 #include "vport.h"
+#include "actions.h"
+
+static DEFINE_PER_CPU(struct ovs_action_stack, action_stacks);
+
+static inline void action_stack_init(struct ovs_action_stack *stack)
+{
+   stack->top = 0;
+}
+
+static inline bool action_stack_is_empty(struct ovs_action_stack *stack)
+{
+   return (stack->top == 0);
+}
+
+static inline struct ovs_deferred_action *
+action_stack_pop(struct ovs_action_stack *stack)
+{
+   if (stack->top) {
+   stack->top--;
+   return &stack->stack[stack->top];
+   }
+
+   return NULL;
+}
+
+static inline struct ovs_deferred_action *
+action_stack_push(struct ovs_action_stack *stack)
+{
+   if (stack->top >= OVS_ACTION_STACK_LIMIT - 1)
+   return NULL;
+
+   return &stack->stack[stack->top++];
+}
 
 static void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key)
 {
@@ -932,3 +965,27 @@ int ovs_execute_actions(struct datapath *dp, struct 
sk_buff *skb, struct sw_flow
 {
return do_execute_actions(dp, skb, acts->actions, acts->actions_len);
 }
+
+void ovs_action_stacks_init(void)
+{
+   int i;
+
+   for_each_possible_cpu(i) {
+   struct ovs_action_stack *stack;
+
+   stack = &per_cpu(action_stacks, i);
+   action_stack_init(stack);
+   }
+}
+
+void ovs_action_stacks_exit(void)
+{
+   int i;
+
+   for_each_possible_cpu(i) {
+   struct ovs_action_stack *stack;
+
+   stack = &per_cpu(action_stacks, i);
+   BUG_ON(stack->top != 0);
+   }
+}
diff --git a/datapath/actions.h b/datapath/actions.h
new file mode 100644
index 000..5f98ad4
--- /dev/null
+++ b/datapath/actions.h
@@ -0,0 +1,54 @@
+/* Copyright (c) 2007-2014 Nicira, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA
+ */
+
+#ifndef ACTION_H
+#define ACTION_H 1
+
+#include 
+#include 
+#include 
+#include 
+
+#include "compat.h"
+#include "flow.h"
+#include "flow_table.h"
+#include "vlan.h"
+#include "vport.h"
+
+#define OVS_ACTION_STACK_LIMIT 10
+
+struct ovs_deferred_action {
+   struct sk_buff *skb;
+   struct nlattr *actions;
+   int rem;
+
+   /* Store pkt_key clone during recirc */
+   struct sw_flow_key pkt_key;
+};
+
+struct ovs_action_stack {
+   /* Deffered action stack */
+   int top;
+   struct ovs_deferred_action stack[OVS_ACTION_STACK_LIMIT];
+};
+
+void ovs_action_stacks_init(void);
+void ovs_action_stacks_exit(void);;
+int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb, struct 
sw_flow_actions *acts);
+void ovs_process_action_stack(void);
+
+#endif /* actions.h */
diff --git a/datapath/datapath.c b/datapath/datapath.c
index a668222..b61e0ae 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -61,6 +61,7 @@
 #include "vlan.h"
 #include "vport-internal_dev.h"
 #include "vport-netdev.h"
+#include "actions.h"
 
 int ovs_net_id __read_mostly;
 
@@ -2151,6 +2152,8 @@ static int __init dp_init(void)
if (err < 0)
goto error_unreg_notifier;
 
+   ovs_action_stacks_init();
+
return 0;
 
 error_unreg_notifier:
@@ -2173,6 +2176,7 @@ static void dp_cleanup(void)
rcu_barrier();
ovs_vport_exit();
ovs_flow_exit();
+   ovs_action_stacks_exit();
 }
 
 module_init(dp_init);
diff --git a/datapath/datapath.h b/datapath/datapath.h
index eba2fc4..dbe58d4 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -31,6 +

[ovs-dev] [v3 4/4] datapath compact: Use lower network stack recursion limits

[Andy Zhou]

For the out of tree OVS module, the network stack recursion limit are
some times lower than the default value enforced by dev.c.
example that the default

This patch implements a lower limit, than the limit enforced by dev,c,
to accommodate some OVS use cases that involves higher stack usage.
For example, OVS + IPsec.

Signed-off-by: Andy Zhou 
---
 datapath/linux/compat/vport.h | 24 
 datapath/vport.c  |  4 ++--
 datapath/vport.h  |  8 +++-
 3 files changed, 33 insertions(+), 3 deletions(-)
 create mode 100644 datapath/linux/compat/vport.h

diff --git a/datapath/linux/compat/vport.h b/datapath/linux/compat/vport.h
new file mode 100644
index 000..772f2c5
--- /dev/null
+++ b/datapath/linux/compat/vport.h
@@ -0,0 +1,24 @@
+#ifndef __NET_OEPNVSWITCH_VPORT_WRAPPER_H
+#define __NET_OEPNVSWITCH_VPORT_WRAPPER_H
+
+#include_next "vport.h"
+
+DECLARE_PER_CPU(int, ovs_exec_actions_count);
+#define OVS_OUTPUT_LOOP_LIMIT 4
+
+static inline int rpl_ovs_vport_send(struct vport *p, struct sk_buff *skb)
+{
+   if (this_cpu_read(ovs_exec_actions_count) > OVS_OUTPUT_LOOP_LIMIT) {
+   if(net_ratelimit())
+   pr_info("OVS output loop limit reached, drop. \n");
+   kfree_skb(skb);
+   return 0;
+   }
+
+   return ovs_vport_send(p, skb);
+}
+
+#define ovs_vport_send rpl_ovs_vport_send
+
+#endif
+
diff --git a/datapath/vport.c b/datapath/vport.c
index 37e0edc..ac72ee0 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -499,7 +499,7 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff 
*skb,
 }
 
 /**
- * ovs_vport_send - send a packet on a device
+ * ovs_vport_send_packet - send a packet on a device
  *
  * @vport: vport on which to send the packet
  * @skb: skb to send
@@ -507,7 +507,7 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff 
*skb,
  * Sends the given packet and returns the length of data sent.  Either ovs
  * lock or rcu_read_lock must be held.
  */
-int ovs_vport_send(struct vport *vport, struct sk_buff *skb)
+int ovs_vport_send_packet(struct vport *vport, struct sk_buff *skb)
 {
int sent = vport->ops->send(vport, skb);
 
diff --git a/datapath/vport.h b/datapath/vport.h
index 8c3da05..c1d6a80 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -55,7 +55,7 @@ int ovs_vport_set_upcall_portids(struct vport *, struct 
nlattr *pids);
 int ovs_vport_get_upcall_portids(const struct vport *, struct sk_buff *);
 u32 ovs_vport_find_upcall_portid(const struct vport *, struct sk_buff *);
 
-int ovs_vport_send(struct vport *, struct sk_buff *);
+int ovs_vport_send_packet(struct vport * vport, struct sk_buff * skb);
 
 int ovs_tunnel_get_egress_info(struct ovs_tunnel_info *egress_tun_info,
   struct net *net,
@@ -243,4 +243,10 @@ static inline void ovs_skb_postpush_rcsum(struct sk_buff 
*skb,
if (skb->ip_summed == CHECKSUM_COMPLETE)
skb->csum = csum_add(skb->csum, csum_partial(start, len, 0));
 }
+
+static inline int ovs_vport_send(struct vport *vport, struct sk_buff *skb)
+{
+   return ovs_vport_send_packet(vport, skb);
+}
+
 #endif /* vport.h */
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

[Alin Serdean]

Hi Saurabh,

At the end of the day today. Was on short PTO last week.

Alin.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com] 
Trimis: Wednesday, August 27, 2014 2:46 AM
Către: Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hey Alin,
Are you planning on re-spinning this patch?

Saurabh

>This commit adds to the automake build system the full build required 
>by the forwarding extension solution.
>
>It will help a lot in the future CI to check the full build of the 
>project.
>
>Also the documentation was updated.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> INSTALL.Windows | 34 --
> Makefile.am | 10 ++
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>diff --git a/INSTALL.Windows b/INSTALL.Windows index abe665d..8c30c1c 
>100644
>--- a/INSTALL.Windows
>+++ b/INSTALL.Windows
>@@ -1,4 +1,4 @@
>-How to Build the Kernel module & userspace daemons for Windows
>+ How to Build the forwarding extension & userspace binaries for
>Windows
> ==
> 
> Autoconf, Automake and Visual C++:
>@@ -29,8 +29,8 @@ the following entry in /etc/fstab - 'C:/MinGW /mingw'.
> part of Windows' PATH environment variable.
> 
> * You will need at least Visual Studio 2013 to compile userspace 
>binaries. In -addition to that, if you want to compile the kernel 
>module you will also need to -install Windows Driver Kit (WDK) 8.1 
>Update.
>+addition to that, if you want to compile the forwarding extension you
>will also
>+need to install Windows Driver Kit (WDK) 8.1 Update.
> 
> It is important to get the Visual Studio related environment variables 
>and to  have the $PATH inside the bash to point to the proper compiler 
>and linker. One @@ -63,7 +63,8 @@ or from a distribution tar ball.
> --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
> --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
> 
>-* Run make for the ported executables in the top source directory, e.g.:
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>   % make
> 
>@@ -91,32 +92,29 @@ For example,
>   --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
>--enable-ssl \
>   --with-openssl="C:/OpenSSL-Win32"
> 
>-* Run make for the ported executables.
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>-Building the Kernel module
>---
>-We directly use the Visual Studio 2013 IDE to compile the kernel module.
>You can
>-open the extensions.sln file in the IDE and build the solution.
>+  % make
> 
>-Installing the Kernel module
>+Installing the forwarding extension
> 
>-Once you have built the solution, you can copy the following files to 
>the -target Hyper-V machines:
>+You can copy the following files to the target Hyper-V machines:
> 
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.inf
>-./datapath-windows/x64/Win8.1Debug/package/OVSExt.sys
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.cat
>+./datapath-windows/x64/Win8Debug/package/ovsext.inf
>+./datapath-windows/x64/Win8Debug/package/OVSExt.sys
>+./datapath-windows/x64/Win8Debug/package/ovsext.cat
> ./datapath-windows/misc/install.cmd
> ./datapath-windows/misc/uninstall.cmd
> 
>-Steps to install the module
>+Steps to install the forwarding extension
> ---
> 
> 01> Run ./uninstall.cmd to remove the old extension.
> 02> Run ./install.cmd to insert the new one. For this to work you will
>have to
> turn on TESTSIGNING boot option or 'Disable Driver Signature Enforcement'
> during boot.
>-03> In the Virtual Switch Manager configuration you should now see
>"VMWare OVS
>+03> In the Virtual Switch Manager configuration you should now see 
>+03> "Open
>vSwitch
> Extension" under 'Virtual Switch Extensions'. Click the check box to 
>enable the  extension.
> 
>@@ -255,5 +253,5 @@ be brought in.
> 
> * Investigate the working of sFlow on Windows and re-enable the unit 
>tests.
> 
>-* Sign the driver & create an MSI for installing the different 
>OpenvSwitch
>+* Sign the driver & create an MSI for installing the different Open
>vSwitch
> components on windows.
>diff --git a/Makefile.am b/Makefile.am
>index eb58101..339882a 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -276,6 +276,16 @@ manpage-check: $(man_MANS) $(dist_man_MANS)
>$(noinst_man_MANS)
> CLEANFILES += manpage-check
> endif
> 
>+if WIN32
>+ALL_LOCAL += ovsext_make
>+ovsext_make: datapath-windows/ovsext.sln
>+  MSBuild.exe datapath-windows/ovsext.sln /target:Build
>+
>+CLEAN_LOCAL += ovsext_clean
>+ovsext_clean: datapath-windows/ovsext.sln
>+  MSBuild.exe datapath-windows/ovsext.sln /target:Clean endif
>+
> include $(srcdir)/manpages.mk
> $(srcdir)/manpages.mk: $(MAN_ROOTS) b

Re: [ovs-dev] [PATCH] Rename files under datapath-windows

[Alin Serdean]

Hi Saurabh,

Could you please try to do the autoreconf again. 
./boot.sh
./configure

Thanks.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com] 
Trimis: Wednesday, August 27, 2014 3:01 AM
Către: Samuel Ghinet; dev@openvswitch.org
Cc: Alin Serdean; Nithin Raju
Subiect: RE: [ovs-dev] [PATCH] Rename files under datapath-windows

Hi Samuel,

I manually patched the change but I wasn't able to build it. Haven't spent much 
time figuring out why.

link -nologo -DEBUG -out:ovsdb/ovsdb-server.exe ovsdb/ovsdb-server.obj -LIBPATH:
C:/pthread/lib/x86 ovsdb/.libs/libovsdb.lib lib/.libs/libopenvswitch.lib pthread
VC2.lib ws2_32.lib
LINK : ovsdb/ovsdb-server.exe not found or not built by the last incremental lin
k; performing full link
make[2]: *** No rule to make target `datapath-windows/ovsext/Actions.c', needed
by `all-am'.  Stop.
make[2]: Leaving directory `/home/Administrator/work/ro/ovs'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/Administrator/work/ro/ovs'
make: *** [all] Error 2

It would make sense to mention in the coding guide the quirk about using 'Ovs' 
prefix to interfaces that are shared with userspace.

Otherwise the change looks good to me. Thanks for working on this. 

Saurabh

> -Original Message-
> From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com]
> Sent: Tuesday, August 26, 2014 1:04 PM
> To: dev@openvswitch.org
> Cc: Alin Serdean; Saurabh Shah; Nithin Raju
> Subject: [ovs-dev] [PATCH] Rename files under datapath-windows
> 
> 
> This patch includes the file renaming and accommodations needed for the
> file
> renaming to build the forwarding extension for Hyper-V.
> 
> This patch is also a follow-up for the thread:
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/piper
> mail/dev/2014-
> August/044005.html&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjs
> HfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=7Rg7UPq8iQ7
> HcPlOgJ4FvU9sZhZ0GQRhzx1LUZtWmrk%3D%0A&s=310a208afab4c3400497c
> bb3d336923893557324acca90bd4505c1f7c2bfb60e
> 
> Signed-off-by: Samuel Ghinet 
> Co-authored-by: Alin Gabriel Serdean 
> ---
>  build-aux/extract-odp-netlink-windows-dp-h |   2 +-
>  datapath-windows/automake.mk   | 126 
> ++---
>  .../ovsext/{OvsActions.c => Actions.c} |  20 ++--
>  datapath-windows/ovsext/{OvsAtomic.h => Atomic.h}  |   6 +-
>  .../ovsext/{OvsBufferMgmt.c => BufferMgmt.c}   |  12 +-
>  .../ovsext/{OvsBufferMgmt.h => BufferMgmt.h}   |   6 +-
>  .../ovsext/{OvsChecksum.c => Checksum.c}   |   8 +-
>  .../ovsext/{OvsChecksum.h => Checksum.h}   |   6 +-
>  datapath-windows/ovsext/Datapath.c |  20 ++--
>  datapath-windows/ovsext/Datapath.h |   8 +-
>  datapath-windows/ovsext/{OvsDebug.c => Debug.c}|   2 +-
>  datapath-windows/ovsext/{OvsDebug.h => Debug.h}|   6 +-
>  datapath-windows/ovsext/{OvsDriver.c => Driver.c}  |   4 +-
>  datapath-windows/ovsext/{OvsEth.h => Ethernet.h}   |   6 +-
>  datapath-windows/ovsext/{OvsEvent.c => Event.c}|   8 +-
>  datapath-windows/ovsext/{OvsEvent.h => Event.h}|   6 +-
>  datapath-windows/ovsext/{OvsFlow.c => Flow.c}  |  12 +-
>  datapath-windows/ovsext/{OvsFlow.h => Flow.h}  |  12 +-
>  datapath-windows/ovsext/{OvsIoctl.c => Ioctl.c}|  22 ++--
>  datapath-windows/ovsext/{OvsIoctl.h => Ioctl.h}|   6 +-
>  .../ovsext/{OvsIpHelper.c => IpHelper.c}   |   8 +-
>  .../ovsext/{OvsIpHelper.h => IpHelper.h}   |   6 +-
>  datapath-windows/ovsext/{OvsJhash.c => Jhash.c}|   0
>  datapath-windows/ovsext/{OvsJhash.h => Jhash.h}|   6 +-
>  .../ovsext/{OvsNetProto.h => NetProto.h}   |   8 +-
>  datapath-windows/ovsext/Netlink.c  |   2 +-
>  datapath-windows/ovsext/Netlink.h  |   2 +-
>  datapath-windows/ovsext/NetlinkProto.h |   4 +-
>  datapath-windows/ovsext/{OvsOid.c => Oid.c}|  18 +--
>  datapath-windows/ovsext/{OvsOid.h => Oid.h}|   6 +-
>  .../ovsext/{OvsPacketIO.c => PacketIO.c}   |  18 +--
>  .../ovsext/{OvsPacketIO.h => PacketIO.h}   |   6 +-
>  .../ovsext/{OvsPacketParser.c => PacketParser.c}   |   2 +-
>  .../ovsext/{OvsPacketParser.h => PacketParser.h}   |   8 +-
>  datapath-windows/ovsext/{OvsSwitch.c => Switch.c}  |  16 +--
>  datapath-windows/ovsext/{OvsSwitch.h => Switch.h}  |  10 +-
>  datapath-windows/ovsext/{OvsTunnel.c => Tunnel.c}  |  18 +--
>  datapath-windows/ovsext/{OvsTunnel.h => Tunnel.h}  |   6 +-
>  .../ovsext/{OvsTunnelFilter.c => TunnelFilter.c}   |  12 +-
>  .../ovsext/{OvsTunnelIntf.h => TunnelIntf.h}   |   6 +-
>  datapath-windows/ovsext/{OvsTypes.h => Types.h}|   6 +-
>  datapath-windows/ovsext/{OvsUser.c => User.c}  |  20 ++--
>  datapath-windows/ovsext/{OvsUser.h => User.h}  |   6 +-
>  datapath-windows/ovsext/{OvsUtil.c => Util.c}  |   2 +-
>  datapath-windows/ovsext/{OvsUtil.h => Util.h}  |   6 +-
>  dat

Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

[Eitan Eliahu]

Hi Alin,
Should we able to specify Debug or Release build when building with MSBuild.exe?
Thanks,
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Wednesday, August 27, 2014 7:33 AM
To: Saurabh Shah; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Saurabh,

At the end of the day today. Was on short PTO last week.

Alin.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com]
Trimis: Wednesday, August 27, 2014 2:46 AM
Către: Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hey Alin,
Are you planning on re-spinning this patch?

Saurabh

>This commit adds to the automake build system the full build required 
>by the forwarding extension solution.
>
>It will help a lot in the future CI to check the full build of the 
>project.
>
>Also the documentation was updated.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> INSTALL.Windows | 34 --
> Makefile.am | 10 ++
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>diff --git a/INSTALL.Windows b/INSTALL.Windows index abe665d..8c30c1c
>100644
>--- a/INSTALL.Windows
>+++ b/INSTALL.Windows
>@@ -1,4 +1,4 @@
>-How to Build the Kernel module & userspace daemons for Windows
>+ How to Build the forwarding extension & userspace binaries for
>Windows
> ==
> 
> Autoconf, Automake and Visual C++:
>@@ -29,8 +29,8 @@ the following entry in /etc/fstab - 'C:/MinGW /mingw'.
> part of Windows' PATH environment variable.
> 
> * You will need at least Visual Studio 2013 to compile userspace 
>binaries. In -addition to that, if you want to compile the kernel 
>module you will also need to -install Windows Driver Kit (WDK) 8.1 
>Update.
>+addition to that, if you want to compile the forwarding extension you
>will also
>+need to install Windows Driver Kit (WDK) 8.1 Update.
> 
> It is important to get the Visual Studio related environment variables 
>and to  have the $PATH inside the bash to point to the proper compiler 
>and linker. One @@ -63,7 +63,8 @@ or from a distribution tar ball.
> --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
> --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
> 
>-* Run make for the ported executables in the top source directory, e.g.:
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>   % make
> 
>@@ -91,32 +92,29 @@ For example,
>   --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
>--enable-ssl \
>   --with-openssl="C:/OpenSSL-Win32"
> 
>-* Run make for the ported executables.
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>-Building the Kernel module
>---
>-We directly use the Visual Studio 2013 IDE to compile the kernel module.
>You can
>-open the extensions.sln file in the IDE and build the solution.
>+  % make
> 
>-Installing the Kernel module
>+Installing the forwarding extension
> 
>-Once you have built the solution, you can copy the following files to 
>the -target Hyper-V machines:
>+You can copy the following files to the target Hyper-V machines:
> 
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.inf
>-./datapath-windows/x64/Win8.1Debug/package/OVSExt.sys
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.cat
>+./datapath-windows/x64/Win8Debug/package/ovsext.inf
>+./datapath-windows/x64/Win8Debug/package/OVSExt.sys
>+./datapath-windows/x64/Win8Debug/package/ovsext.cat
> ./datapath-windows/misc/install.cmd
> ./datapath-windows/misc/uninstall.cmd
> 
>-Steps to install the module
>+Steps to install the forwarding extension
> ---
> 
> 01> Run ./uninstall.cmd to remove the old extension.
> 02> Run ./install.cmd to insert the new one. For this to work you will
>have to
> turn on TESTSIGNING boot option or 'Disable Driver Signature Enforcement'
> during boot.
>-03> In the Virtual Switch Manager configuration you should now see
>"VMWare OVS
>+03> In the Virtual Switch Manager configuration you should now see 
>+03> "Open
>vSwitch
> Extension" under 'Virtual Switch Extensions'. Click the check box to 
>enable the  extension.
> 
>@@ -255,5 +253,5 @@ be brought in.
> 
> * Investigate the working of sFlow on Windows and re-enable the unit 
>tests.
> 
>-* Sign the driver & create an MSI for installing the different 
>OpenvSwitch
>+* Sign the driver & create an MSI for installing the different Open
>vSwitch
> components on windows.
>diff --git a/Makefile.am b/Makefile.am
>index eb58101..339882a 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -276,6 +276,16 @@ manpage-check: $(man_MANS) $(dist_man_MANS)
>$(noinst_man_MANS)
> CLEANFILES += manpage-check
> endif
> 
>+if WIN32

Re: [ovs-dev] [PATCH] fix nx-match.c and execution.c memory leak find by the tool of fortify

[Ben Pfaff]

On Wed, Aug 27, 2014 at 09:52:54AM +0800, \y00206114 wrote:
> From: yinpeijun 
> 
> Signed-off-by: yinpeijun 

Thanks, I applied this to master.

I changed the commit message to primarily describe the problem fixed and
only secondarily the tool:

From: yinpeijun 
Date: Wed, 27 Aug 2014 09:52:54 +0800
Subject: [PATCH] Fix memory leaks in error paths.

Found by Fortify.

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

Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

[Alin Serdean]

Hi Eithan,

Yes.  Also Win8 or Win8.1

I still to format it accordingly and will send out the patch.

Alin.

-Mesaj original-
De la: Eitan Eliahu [mailto:elia...@vmware.com] 
Trimis: Wednesday, August 27, 2014 5:40 PM
Către: Alin Serdean; Saurabh Shah; dev@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Alin,
Should we able to specify Debug or Release build when building with MSBuild.exe?
Thanks,
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Wednesday, August 27, 2014 7:33 AM
To: Saurabh Shah; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Saurabh,

At the end of the day today. Was on short PTO last week.

Alin.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com]
Trimis: Wednesday, August 27, 2014 2:46 AM
Către: Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hey Alin,
Are you planning on re-spinning this patch?

Saurabh

>This commit adds to the automake build system the full build required 
>by the forwarding extension solution.
>
>It will help a lot in the future CI to check the full build of the 
>project.
>
>Also the documentation was updated.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> INSTALL.Windows | 34 --
> Makefile.am | 10 ++
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>diff --git a/INSTALL.Windows b/INSTALL.Windows index abe665d..8c30c1c
>100644
>--- a/INSTALL.Windows
>+++ b/INSTALL.Windows
>@@ -1,4 +1,4 @@
>-How to Build the Kernel module & userspace daemons for Windows
>+ How to Build the forwarding extension & userspace binaries for
>Windows
> ==
> 
> Autoconf, Automake and Visual C++:
>@@ -29,8 +29,8 @@ the following entry in /etc/fstab - 'C:/MinGW /mingw'.
> part of Windows' PATH environment variable.
> 
> * You will need at least Visual Studio 2013 to compile userspace 
>binaries. In -addition to that, if you want to compile the kernel 
>module you will also need to -install Windows Driver Kit (WDK) 8.1 
>Update.
>+addition to that, if you want to compile the forwarding extension you
>will also
>+need to install Windows Driver Kit (WDK) 8.1 Update.
> 
> It is important to get the Visual Studio related environment variables 
>and to  have the $PATH inside the bash to point to the proper compiler 
>and linker. One @@ -63,7 +63,8 @@ or from a distribution tar ball.
> --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
> --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
> 
>-* Run make for the ported executables in the top source directory, e.g.:
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>   % make
> 
>@@ -91,32 +92,29 @@ For example,
>   --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
>--enable-ssl \
>   --with-openssl="C:/OpenSSL-Win32"
> 
>-* Run make for the ported executables.
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>-Building the Kernel module
>---
>-We directly use the Visual Studio 2013 IDE to compile the kernel module.
>You can
>-open the extensions.sln file in the IDE and build the solution.
>+  % make
> 
>-Installing the Kernel module
>+Installing the forwarding extension
> 
>-Once you have built the solution, you can copy the following files to 
>the -target Hyper-V machines:
>+You can copy the following files to the target Hyper-V machines:
> 
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.inf
>-./datapath-windows/x64/Win8.1Debug/package/OVSExt.sys
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.cat
>+./datapath-windows/x64/Win8Debug/package/ovsext.inf
>+./datapath-windows/x64/Win8Debug/package/OVSExt.sys
>+./datapath-windows/x64/Win8Debug/package/ovsext.cat
> ./datapath-windows/misc/install.cmd
> ./datapath-windows/misc/uninstall.cmd
> 
>-Steps to install the module
>+Steps to install the forwarding extension
> ---
> 
> 01> Run ./uninstall.cmd to remove the old extension.
> 02> Run ./install.cmd to insert the new one. For this to work you will
>have to
> turn on TESTSIGNING boot option or 'Disable Driver Signature Enforcement'
> during boot.
>-03> In the Virtual Switch Manager configuration you should now see
>"VMWare OVS
>+03> In the Virtual Switch Manager configuration you should now see 
>+03> "Open
>vSwitch
> Extension" under 'Virtual Switch Extensions'. Click the check box to 
>enable the  extension.
> 
>@@ -255,5 +253,5 @@ be brought in.
> 
> * Investigate the working of sFlow on Windows and re-enable the unit 
>tests.
> 
>-* Sign the driver & create an MSI for installing the different 
>OpenvSwitch

Re: [ovs-dev] [PATCH] ovsdb: Catch negative timeout values

[Ben Pfaff]

On Wed, Aug 27, 2014 at 12:41:50AM +0200, Thomas Graf wrote:
> Although the check is present already, a missing !error branch
> in the next condition prevents the error from being reported back.
> 
> Signed-off-by: Thomas Graf 

Are you sure?  I think that the only real bug here is that if the
timeout is negative *and* the "until" string is invalid, then there's a
memory leak of the error message for the negative timeout.  We should
still fix that, and the same code change does that, but I think that the
commit message is wrong.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovsdb: Fix leaked row in ovsdb_execute_wait()

[Ben Pfaff]

On Wed, Aug 27, 2014 at 12:48:04AM +0200, Thomas Graf wrote:
> An error returned from ovsdb_row_from_json() currently results in
> leaking of row.
> 
> Signed-off-by: Thomas Graf 

I applied a patch from yinpeijun a few minutes ago that fixed the same
problem.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] lib/odp: coverity null check in odp_execute_sample

[Ben Pfaff]

On Tue, Aug 26, 2014 at 06:00:42PM -0700, Madhu Challa wrote:
> call odp_execute_actions__ only if there are any subactions to prevent null
> pointer dereference in nl_attr_get().
> 
> Signed-off-by: Madhu Challa 

Should not be necessary because we don't generally deal with untrusted
ODP actions in userspace.  We'd need a more comprehensive audit if this
becomes necessary.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] vtep-ctl: coverity - free error string before return from cmd_remove

[Ben Pfaff]

On Tue, Aug 26, 2014 at 06:16:12PM -0700, Madhu Challa wrote:
> error string should be freed in all cases.
> 
> Signed-off-by: Madhu Challa 

This patch was wordwrapped so I applied it by hand.

The "if" isn't necessary so I removed it.

I adjusted the commit message to primarily describe the change and the
reason for it and only secondarily to mention the tool.

So, applied as:

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

From: Madhu Challa 
Date: Tue, 26 Aug 2014 18:16:12 -0700
Subject: [PATCH] vtep-ctl: Free error string before return from cmd_remove().

Error string should be freed in all cases.

Found by Coverity.

Signed-off-by: Madhu Challa 
Signed-off-by: Ben Pfaff 
---
 vtep/vtep-ctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 0b9463a..3576313 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -3199,11 +3199,11 @@ cmd_remove(struct vtep_ctl_context *ctx)
 error = ovsdb_datum_from_string(&rm, &rm_type,
 ctx->argv[i], ctx->symtab);
 if (error && ovsdb_type_is_map(&rm_type)) {
-free(error);
 rm_type.value.type = OVSDB_TYPE_VOID;
 die_if_error(ovsdb_datum_from_string(&rm, &rm_type,
  ctx->argv[i], ctx->symtab));
 }
+free(error);
 ovsdb_datum_subtract(&old, type, &rm, &rm_type);
 ovsdb_datum_destroy(&rm, &rm_type);
 }
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

[Eitan Eliahu]

Thanks Alin. We probably don't need Win8 but definitely Release and Debug 
builds of Win8.1.
Eitan

-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com] 
Sent: Wednesday, August 27, 2014 7:46 AM
To: Eitan Eliahu; Saurabh Shah; dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Eithan,

Yes.  Also Win8 or Win8.1

I still to format it accordingly and will send out the patch.

Alin.

-Mesaj original-
De la: Eitan Eliahu [mailto:elia...@vmware.com]
Trimis: Wednesday, August 27, 2014 5:40 PM
Către: Alin Serdean; Saurabh Shah; dev@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Alin,
Should we able to specify Debug or Release build when building with MSBuild.exe?
Thanks,
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Wednesday, August 27, 2014 7:33 AM
To: Saurabh Shah; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Saurabh,

At the end of the day today. Was on short PTO last week.

Alin.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com]
Trimis: Wednesday, August 27, 2014 2:46 AM
Către: Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hey Alin,
Are you planning on re-spinning this patch?

Saurabh

>This commit adds to the automake build system the full build required 
>by the forwarding extension solution.
>
>It will help a lot in the future CI to check the full build of the 
>project.
>
>Also the documentation was updated.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> INSTALL.Windows | 34 --
> Makefile.am | 10 ++
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>diff --git a/INSTALL.Windows b/INSTALL.Windows index abe665d..8c30c1c
>100644
>--- a/INSTALL.Windows
>+++ b/INSTALL.Windows
>@@ -1,4 +1,4 @@
>-How to Build the Kernel module & userspace daemons for Windows
>+ How to Build the forwarding extension & userspace binaries for
>Windows
> ==
> 
> Autoconf, Automake and Visual C++:
>@@ -29,8 +29,8 @@ the following entry in /etc/fstab - 'C:/MinGW /mingw'.
> part of Windows' PATH environment variable.
> 
> * You will need at least Visual Studio 2013 to compile userspace 
>binaries. In -addition to that, if you want to compile the kernel 
>module you will also need to -install Windows Driver Kit (WDK) 8.1 
>Update.
>+addition to that, if you want to compile the forwarding extension you
>will also
>+need to install Windows Driver Kit (WDK) 8.1 Update.
> 
> It is important to get the Visual Studio related environment variables 
>and to  have the $PATH inside the bash to point to the proper compiler 
>and linker. One @@ -63,7 +63,8 @@ or from a distribution tar ball.
> --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
> --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
> 
>-* Run make for the ported executables in the top source directory, e.g.:
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>   % make
> 
>@@ -91,32 +92,29 @@ For example,
>   --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
>--enable-ssl \
>   --with-openssl="C:/OpenSSL-Win32"
> 
>-* Run make for the ported executables.
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>-Building the Kernel module
>---
>-We directly use the Visual Studio 2013 IDE to compile the kernel module.
>You can
>-open the extensions.sln file in the IDE and build the solution.
>+  % make
> 
>-Installing the Kernel module
>+Installing the forwarding extension
> 
>-Once you have built the solution, you can copy the following files to 
>the -target Hyper-V machines:
>+You can copy the following files to the target Hyper-V machines:
> 
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.inf
>-./datapath-windows/x64/Win8.1Debug/package/OVSExt.sys
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.cat
>+./datapath-windows/x64/Win8Debug/package/ovsext.inf
>+./datapath-windows/x64/Win8Debug/package/OVSExt.sys
>+./datapath-windows/x64/Win8Debug/package/ovsext.cat
> ./datapath-windows/misc/install.cmd
> ./datapath-windows/misc/uninstall.cmd
> 
>-Steps to install the module
>+Steps to install the forwarding extension
> ---
> 
> 01> Run ./uninstall.cmd to remove the old extension.
> 02> Run ./install.cmd to insert the new one. For this to work you will
>have to
> turn on TESTSIGNING boot option or 'Disable Driver Signature Enforcement'
> during boot.
>-03> In the Virtual Switch Manager configuration you should now see
>"VMWare OVS
>+03> In the Virtual Switch Manager conf

Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

[Alin Serdean]

Hi Eithan,

Will add them anyway for the moment.

Alin.

-Mesaj original-
De la: Eitan Eliahu [mailto:elia...@vmware.com] 
Trimis: Wednesday, August 27, 2014 6:08 PM
Către: Alin Serdean; Saurabh Shah; dev@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Thanks Alin. We probably don't need Win8 but definitely Release and Debug 
builds of Win8.1.
Eitan

-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com]
Sent: Wednesday, August 27, 2014 7:46 AM
To: Eitan Eliahu; Saurabh Shah; dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Eithan,

Yes.  Also Win8 or Win8.1

I still to format it accordingly and will send out the patch.

Alin.

-Mesaj original-
De la: Eitan Eliahu [mailto:elia...@vmware.com]
Trimis: Wednesday, August 27, 2014 5:40 PM
Către: Alin Serdean; Saurabh Shah; dev@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Alin,
Should we able to specify Debug or Release build when building with MSBuild.exe?
Thanks,
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Wednesday, August 27, 2014 7:33 AM
To: Saurabh Shah; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hi Saurabh,

At the end of the day today. Was on short PTO last week.

Alin.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com]
Trimis: Wednesday, August 27, 2014 2:46 AM
Către: Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] Add build of ovsext.sln using MSBuild

Hey Alin,
Are you planning on re-spinning this patch?

Saurabh

>This commit adds to the automake build system the full build required 
>by the forwarding extension solution.
>
>It will help a lot in the future CI to check the full build of the 
>project.
>
>Also the documentation was updated.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> INSTALL.Windows | 34 --
> Makefile.am | 10 ++
> 2 files changed, 26 insertions(+), 18 deletions(-)
>
>diff --git a/INSTALL.Windows b/INSTALL.Windows index abe665d..8c30c1c
>100644
>--- a/INSTALL.Windows
>+++ b/INSTALL.Windows
>@@ -1,4 +1,4 @@
>-How to Build the Kernel module & userspace daemons for Windows
>+ How to Build the forwarding extension & userspace binaries for
>Windows
> ==
> 
> Autoconf, Automake and Visual C++:
>@@ -29,8 +29,8 @@ the following entry in /etc/fstab - 'C:/MinGW /mingw'.
> part of Windows' PATH environment variable.
> 
> * You will need at least Visual Studio 2013 to compile userspace 
>binaries. In -addition to that, if you want to compile the kernel 
>module you will also need to -install Windows Driver Kit (WDK) 8.1 
>Update.
>+addition to that, if you want to compile the forwarding extension you
>will also
>+need to install Windows Driver Kit (WDK) 8.1 Update.
> 
> It is important to get the Visual Studio related environment variables 
>and to  have the $PATH inside the bash to point to the proper compiler 
>and linker. One @@ -63,7 +63,8 @@ or from a distribution tar ball.
> --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
> --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
> 
>-* Run make for the ported executables in the top source directory, e.g.:
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>   % make
> 
>@@ -91,32 +92,29 @@ For example,
>   --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
>--enable-ssl \
>   --with-openssl="C:/OpenSSL-Win32"
> 
>-* Run make for the ported executables.
>+* Run make for the ported executables  and the forwarding extension in
>the top
>+source directory, e.g.:
> 
>-Building the Kernel module
>---
>-We directly use the Visual Studio 2013 IDE to compile the kernel module.
>You can
>-open the extensions.sln file in the IDE and build the solution.
>+  % make
> 
>-Installing the Kernel module
>+Installing the forwarding extension
> 
>-Once you have built the solution, you can copy the following files to 
>the -target Hyper-V machines:
>+You can copy the following files to the target Hyper-V machines:
> 
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.inf
>-./datapath-windows/x64/Win8.1Debug/package/OVSExt.sys
>-./datapath-windows/x64/Win8.1Debug/package/ovsext.cat
>+./datapath-windows/x64/Win8Debug/package/ovsext.inf
>+./datapath-windows/x64/Win8Debug/package/OVSExt.sys
>+./datapath-windows/x64/Win8Debug/package/ovsext.cat
> ./datapath-windows/misc/install.cmd
> ./datapath-windows/misc/uninstall.cmd
> 
>-Steps to install the module
>+Steps to install the forwarding extension
> ---
> 
> 01> Run ./uninstall.cmd to remove the old extensio

Re: [ovs-dev] [PATCH v2] netlink-socket.c: fix typo to get_sock_pid_from_kernel()

[Ben Pfaff]

On Tue, Aug 26, 2014 at 08:20:51PM -0700, Nithin Raju wrote:
> A typo crept in while respinning get_sock_pid_from_kernel() in the previous
> patch. Fixing it now. Also, get_sock_pid_from_kernel() doesn't need an OUT
> argument. Fixing that too.
> 
> Signed-off-by: Nithin Raju 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2 v2] netlink-socket.c: add support for nl_lookup_genl_mcgroup()

[Ben Pfaff]

On Tue, Aug 26, 2014 at 08:17:03PM -0700, Nithin Raju wrote:
> While we work out whether nl_sock_join_mcgroup() will be the mechanism
> to support VPORT events, it is easy to add support for
> nl_lookup_genl_mcgroup() and make progress on the other commands.
> 
> In this patch, we implement support for nl_lookup_genl_mcgroup() only
> for the VPORT family though, which is all what dpif-linux.c needs.
> 
> Validation:
> - A ported dpif-linux.c with epoll code commented out went so far as
> to call dp_enumerate! DP Dump commands can be implemented next.
> 
> Signed-off-by: Nithin Raju 

Both applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] lib/flow.h: Improve struct miniflow comment and definition.

[Jarno Rajahalme]

On Aug 26, 2014, at 5:13 PM, Ben Pfaff  wrote:

> On Wed, Aug 27, 2014 at 12:11:34AM +, Saurabh Shah wrote:
>> 
>> I reverted uint8_t -> uint64_t and was able to move past compile flow.h.
> 
> OK, so much for that idea.  Jarno, let's stick with uint64_t then?

Reverted back to uint64_t on master,

  Jarno

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

Re: [ovs-dev] [PATCH] Rename files under datapath-windows

[Ben Pfaff]

On Tue, Aug 26, 2014 at 08:03:47PM +, Samuel Ghinet wrote:
> 
> This patch includes the file renaming and accommodations needed for the file
> renaming to build the forwarding extension for Hyper-V.
> 
> This patch is also a follow-up for the thread:
> http://openvswitch.org/pipermail/dev/2014-August/044005.html
> 
> Signed-off-by: Samuel Ghinet 
> Co-authored-by: Alin Gabriel Serdean 

Attempting to apply this, I get a patch reject in
datapath-windows/automake.mk.  I guess that it must have changed
slightly since you composed the patch.  Will you send a v2?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] datapath-windows: Update netlink family IDs

[Ben Pfaff]

On Tue, Aug 26, 2014 at 08:37:18PM -0700, Nithin Raju wrote:
> I didn't realize earlier while defining OvsDpInterfaceExt.h that
> there are special values defined in netlink-protocol.h for nlmsg_type.
> For Eg. NLMSG_ERROR is defined to be 2. In this patch, we update the
> values of the family IDs to not clash with the special defines.
> I'm using NLMSG_MIN_TYPE as a reference.
> 
> All this points to doing family ID lookup from the kernel rather than
> returning values from netlink-socket.c. We should move to that model
> after we get through the first round of netlink commands.
> 
> Signed-off-by: Nithin Raju 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead of ReadFile/WriteFile

[Nithin Raju]

The Windows datapath supports a READ/WRITE ioctl instead of ReadFile/WriteFile.
In this change, we update the following:
- WriteFile() in nl_sock_send__() to use DeviceIoControl(OVS_IOCTL_WRITE)
- ReadFile() in nl_sock_recv__() to use DeviceIoControl(OVS_IOCTL_READ)

The WriteFile() call in nl_sock_transact_multiple__() has not been touched
since it is not needed yet.

Main motive for this change is to be able to unblock the DP Dump workflow.

Signed-off-by: Nithin Raju 
---
 lib/netlink-socket.c |   46 ++
 1 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index 4d9832f..a6be186 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -426,17 +426,20 @@ nl_sock_send__(struct nl_sock *sock, const struct ofpbuf 
*msg,
 do {
 int retval;
 #ifdef _WIN32
-bool result;
-DWORD last_error = 0;
-result = WriteFile(sock->handle, ofpbuf_data(msg), ofpbuf_size(msg),
-   &retval, NULL);
-last_error = GetLastError();
-if (last_error != ERROR_SUCCESS && !result) {
+DWORD bytes;
+
+if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE,
+ofpbuf_data(msg), ofpbuf_size(msg), NULL, 0,
+&bytes, NULL)) {
 retval = -1;
-errno = EAGAIN;
+/* XXX: Map to a more appropriate error based on GetLastError(). */
+errno = EINVAL;
+} else {
+retval = ofpbuf_size(msg);
 }
 #else
-retval = send(sock->fd, ofpbuf_data(msg), ofpbuf_size(msg), wait ? 0 : 
MSG_DONTWAIT);
+retval = send(sock->fd, ofpbuf_data(msg), ofpbuf_size(msg),
+  wait ? 0 : MSG_DONTWAIT);
 #endif
 error = retval < 0 ? errno : 0;
 } while (error == EINTR);
@@ -488,12 +491,7 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
bool wait)
  * 'tail' to allow Netlink messages to be up to 64 kB long (a reasonable
  * figure since that's the maximum length of a Netlink attribute). */
 struct nlmsghdr *nlmsghdr;
-#ifdef _WIN32
-#define MAX_STACK_LENGTH 81920
-uint8_t tail[MAX_STACK_LENGTH];
-#else
 uint8_t tail[65536];
-#endif
 struct iovec iov[2];
 struct msghdr msg;
 ssize_t retval;
@@ -522,15 +520,23 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
bool wait)
 do {
 nlmsghdr->nlmsg_len = UINT32_MAX;
 #ifdef _WIN32
-boolean result = false;
-DWORD last_error = 0;
-result = ReadFile(sock->handle, tail, MAX_STACK_LENGTH, &retval, NULL);
-last_error = GetLastError();
-if (last_error != ERROR_SUCCESS && !result) {
+DWORD bytes;
+if (!DeviceIoControl(sock->handle, OVS_IOCTL_READ,
+ NULL, 0, tail, sizeof tail, &bytes, NULL)) {
 retval = -1;
-errno = EAGAIN;
+errno = EINVAL;
 } else {
-ofpbuf_put(buf, tail, retval);
+retval = bytes;
+if (retval == 0) {
+retval = -1;
+errno = EAGAIN;
+} else {
+if (retval >= buf->allocated) {
+ofpbuf_reinit(buf, retval);
+}
+memcpy(ofpbuf_data(buf), tail, retval);
+ofpbuf_set_size(buf, retval);
+}
 }
 #else
 retval = recvmsg(sock->fd, &msg, wait ? 0 : MSG_DONTWAIT);
-- 
1.7.4.1

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

Re: [ovs-dev] [PATCH] lib/flow.h: Improve struct miniflow comment and definition.

[Gurucharan Shetty]

On Tue, Aug 26, 2014 at 4:08 PM, Ben Pfaff  wrote:
> On Tue, Aug 26, 2014 at 04:04:09PM -0700, Jarno Rajahalme wrote:
>>
>> On Aug 26, 2014, at 3:53 PM, Ben Pfaff  wrote:
>>
>> > On Tue, Aug 26, 2014 at 03:42:33PM -0700, Jarno Rajahalme wrote:
>> >> Finally, use change the storage type of 'values_inline' to uint8_t, as
>> >> uint64_t looks kind of wide for a boolean, even though we intend the
>> >> bit be carved out from the uint64_t where 'map' resides.
>> >
>> > It can't be type "bool"?
>>
>> From CodingStyle:
>>
>>   Declare bit-fields to be type "unsigned int" or "signed int".  Do
>> *not* declare bit-fields of type "int": C89 allows these to be either
>> signed or unsigned according to the compiler's whim.  (A 1-bit
>> bit-field of type "int" may have a range of -1...0!)  Do not declare
>> bit-fields of type _Bool or enum or any other type, because these are
>> not portable.
>>
>> :-)
>
> This was important when we supported C89 compilers that did not have
> native _Bool (bool), but I think that it is obsolete now, because all
> of the compilers we support have C99 implementations good enough to
> have native _Bool.
>
> Let's find out and update CodingStyle if so.
>
> Guru: does MSVC allow "bool" bit-fields?  I assume so but it's worth a
> test.

You mean something like this, right?  If so, it does work.
#include 
#include 

struct test {
bool a:1;
bool b:1;
};

main()
{
struct test t;
t.a = 1;
t.b = 0;

printf("%d %d", t.a, t.b);
}


>
> Thanks,
>
> Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Rename files under datapath-windows

[Saurabh Shah]

Hi Alin,

I was getting an error after reconf which I now realize was due to missing 
files in the distribution.

make[3]: Entering directory `/home/Administrator/work/temp/ovs/datapath'
make[3]: Leaving directory `/home/Administrator/work/temp/ovs/datapath'
The distribution is missing the following files:
datapath-windows/ovsext/OvsActions.c
datapath-windows/ovsext/OvsAtomic.h
datapath-windows/ovsext/OvsBufferMgmt.c
datapath-windows/ovsext/OvsBufferMgmt.h
datapath-windows/ovsext/OvsChecksum.c
datapath-windows/ovsext/OvsChecksum.h
datapath-windows/ovsext/OvsDebug.c
datapath-windows/ovsext/OvsDebug.h
datapath-windows/ovsext/OvsDriver.c
datapath-windows/ovsext/OvsEth.h
datapath-windows/ovsext/OvsEvent.c
datapath-windows/ovsext/OvsEvent.h
datapath-windows/ovsext/OvsFlow.c
datapath-windows/ovsext/OvsFlow.h
datapath-windows/ovsext/OvsIoctl.c
datapath-windows/ovsext/OvsIoctl.h
datapath-windows/ovsext/OvsIpHelper.c
datapath-windows/ovsext/OvsIpHelper.h
datapath-windows/ovsext/OvsJhash.c
datapath-windows/ovsext/OvsJhash.h
datapath-windows/ovsext/OvsNetProto.h
datapath-windows/ovsext/OvsOid.c
datapath-windows/ovsext/OvsOid.h
datapath-windows/ovsext/OvsPacketIO.c
datapath-windows/ovsext/OvsPacketIO.h
datapath-windows/ovsext/OvsPacketParser.c
datapath-windows/ovsext/OvsPacketParser.h
datapath-windows/ovsext/OvsSwitch.c
datapath-windows/ovsext/OvsSwitch.h
datapath-windows/ovsext/OvsTunnel.c
datapath-windows/ovsext/OvsTunnel.h
datapath-windows/ovsext/OvsTunnelFilter.c
datapath-windows/ovsext/OvsTunnelIntf.h
datapath-windows/ovsext/OvsTypes.h
datapath-windows/ovsext/OvsUser.c
datapath-windows/ovsext/OvsUser.h
datapath-windows/ovsext/OvsUtil.c
datapath-windows/ovsext/OvsUtil.h
datapath-windows/ovsext/OvsVport.c
datapath-windows/ovsext/OvsVport.h
datapath-windows/ovsext/OvsVxlan.c
datapath-windows/ovsext/OvsVxlan.h
make[2]: *** [dist-hook-git] Error 1

Although, for some bizarre reason this went away once I added the new files to 
git.

Saurabh

> -Original Message-
> From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com]
> Sent: Wednesday, August 27, 2014 7:34 AM
> To: Saurabh Shah; Samuel Ghinet; dev@openvswitch.org
> Cc: Nithin Raju
> Subject: RE: [ovs-dev] [PATCH] Rename files under datapath-windows
> 
> Hi Saurabh,
> 
> Could you please try to do the autoreconf again.
> ./boot.sh
> ./configure
> 
> Thanks.
> 
> -Mesaj original-
> De la: Saurabh Shah [mailto:ssaur...@vmware.com]
> Trimis: Wednesday, August 27, 2014 3:01 AM
> Către: Samuel Ghinet; dev@openvswitch.org
> Cc: Alin Serdean; Nithin Raju
> Subiect: RE: [ovs-dev] [PATCH] Rename files under datapath-windows
> 
> Hi Samuel,
> 
> I manually patched the change but I wasn't able to build it. Haven't spent
> much time figuring out why.
> 
> link -nologo -DEBUG -out:ovsdb/ovsdb-server.exe ovsdb/ovsdb-server.obj -
> LIBPATH:
> C:/pthread/lib/x86 ovsdb/.libs/libovsdb.lib lib/.libs/libopenvswitch.lib
> pthread
> VC2.lib ws2_32.lib
> LINK : ovsdb/ovsdb-server.exe not found or not built by the last incremental
> lin
> k; performing full link
> make[2]: *** No rule to make target `datapath-windows/ovsext/Actions.c',
> needed
> by `all-am'.  Stop.
> make[2]: Leaving directory `/home/Administrator/work/ro/ovs'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/home/Administrator/work/ro/ovs'
> make: *** [all] Error 2
> 
> It would make sense to mention in the coding guide the quirk about using
> 'Ovs' prefix to interfaces that are shared with userspace.
> 
> Otherwise the change looks good to me. Thanks for working on this.
> 
> Saurabh
> 
> > -Original Message-
> > From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com]
> > Sent: Tuesday, August 26, 2014 1:04 PM
> > To: dev@openvswitch.org
> > Cc: Alin Serdean; Saurabh Shah; Nithin Raju
> > Subject: [ovs-dev] [PATCH] Rename files under datapath-windows
> >
> >
> > This patch includes the file renaming and accommodations needed for the
> > file
> > renaming to build the forwarding extension for Hyper-V.
> >
> > This patch is also a follow-up for the thread:
> >
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/piper
> > mail/dev/2014-
> >
> August/044005.html&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjs
> >
> HfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=7Rg7UPq8iQ7
> >
> HcPlOgJ4FvU9sZhZ0GQRhzx1LUZtWmrk%3D%0A&s=310a208afab4c3400497c
> > bb3d336923893557324acca90bd4505c1f7c2bfb60e
> >
> > Signed-off-by: Samuel Ghinet 
> > Co-authored-by: Alin Gabriel Serdean
> 
> > ---
> >  build-aux/extract-odp-netlink-windows-dp-h |   2 +-
> >  datapath-windows/automake.mk   | 126 
> > ++---
> >  .../ovsext/{OvsActions.c => Actions.c} |  20 ++--
> >  datapath-windows/ovsext/{OvsAtomic.h => Atomic.h}  |   6 +-
> >  .../ovsext/{OvsBufferMgmt.c => BufferMgmt.c}   |  12 +-
> >  .../ovsext/{OvsBufferMgmt.h => BufferMgmt.h}   |   6 +-
> >  .../ovsext/{OvsChecksum.c => Checksum.c}   |   8 +-
> >  .../ovsex

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

[Samuel Ghinet]

Hello Saurabh,

Thanks for the review!

For future reviews, could you please reduce or mark somehow the patch, so it 
would be easier for me to find your comments?
Something like,

>All NET_BUFFERs of a NET_BUFFER_LIST must go through
>the pipeline: extract, find flow, execute. Previously,
>only the first NET_BUFFER of a NET_BUFFER_LIST was going
>through this pipeline, which was erroneous.

comments here.

It might make the reading clearer, and strengthen my confidence that I did not 
miss something :)

[QUOTE]
>-   OvsIPv4TunnelKey *tunKey)
>+   OvsIPv4TunnelKey *tunKey, VOID* vlanTagValue)

Nit: the parameter should be on a new line.
[/QUOTE]
I think we would also need a coding style update here: any function declaration 
or definition should have each parameter on a separate line.
I am going to fix the modif for this function you pointed.

[QUOTE]
This function has become a bit too long now, can you please refactor it?
One obvious thing you could do is to extract the processing of an single
NB into a separate function and the parent function can take care of
splitting the NBL (if required) & creating the right forwarding ctx.
[/QUOTE]
Perhaps I should refactor OvsStartNBLIngress in a separate commit.
I would not like to combine multiple issues in a single commit. Plus, it would 
look clearer in the history.

[QUOTE]
I believe we can take the lock after initializing the external context.
[/QUOTE]
It was like this in the original, but yes, I think you're right.
If I make a separate commit of the OvsStartNBLIngress refactor, I believe I 
should update it there.

[QUOTE]
I actually prefer the shorter version which would take up only 2 lines. I
am going to suggest this be added to the coding guide as well.
[/QUOTE]
Yes, I think it would be a good idea to have only one style of multi-line 
comments.
Even though, I personally prefer this style:
/*
* comment line
* comment line
*/

Sam


From: Saurabh Shah [ssaur...@vmware.com]
Sent: Saturday, August 23, 2014 7:15 AM
To: Alin Serdean; dev@openvswitch.org; Nithin Raju; Samuel Ghinet
Subject: Re: [ovs-dev]  [PATCH] Create a NBL for each NB when required

Hi Alin,

Thanks for working on it! I am almost done reviewing the change. I have a
few comments below.

>ovs/ovs-issues#15
>
>All NET_BUFFERs of a NET_BUFFER_LIST must go through
>the pipeline: extract, find flow, execute. Previously,
>only the first NET_BUFFER of a NET_BUFFER_LIST was going
>through this pipeline, which was erroneous.
>
>OvsPartialCopyToMultipleNBLs is used to make each NET_BUFFER
>have its own NET_BUFFER_LIST.
>
>Some functions that used to take as argument a NET_BUFFER_LIST
>now take as argument a NET_BUFFER. I have also added a few
>ASSERTs where the NET_BUFFER_LIST is expected to have
>only one NET_BUFFER."
>
>Signed-off-by: Samuel Ghinet 
>Co-authored-by: Alin Gabriel Serdean 
>---
> datapath-windows/ovsext/OvsActions.c  |  20 -
> datapath-windows/ovsext/OvsBufferMgmt.c   |   8 +-
> datapath-windows/ovsext/OvsBufferMgmt.h   |   5 ++
> datapath-windows/ovsext/OvsChecksum.c |   3 +-
> datapath-windows/ovsext/OvsFlow.c |  44 +
> datapath-windows/ovsext/OvsFlow.h |   6 +-
> datapath-windows/ovsext/OvsPacketIO.c | 143
>+++---
> datapath-windows/ovsext/OvsPacketParser.c |  24 ++---
> datapath-windows/ovsext/OvsPacketParser.h |  30 ---
> datapath-windows/ovsext/OvsTunnel.c   |  12 ++-
> datapath-windows/ovsext/OvsUser.c |  19 +++-
> datapath-windows/ovsext/OvsVxlan.c|  37 ++--
> 12 files changed, 221 insertions(+), 130 deletions(-)
>
>diff --git a/datapath-windows/ovsext/OvsActions.c
>b/datapath-windows/ovsext/OvsActions.c
>index 635becc..c5c5116 100644
>--- a/datapath-windows/ovsext/OvsActions.c
>+++ b/datapath-windows/ovsext/OvsActions.c
>@@ -517,6 +517,9 @@ OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx)
> OvsFlow *flow;
> UINT64 hash;
> NDIS_STATUS status;
>+VOID* vlanTagValue;
>+NET_BUFFER* pNb;
>+
> POVS_VPORT_ENTRY vport =
> OvsFindVportByPortNo(ovsFwdCtx->switchContext,
>ovsFwdCtx->srcVportNo);
> if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) {
>@@ -527,12 +530,19 @@ OvsDoFlowLookupOutput(OvsForwardingContext
>*ovsFwdCtx)
> return NDIS_STATUS_SUCCESS;
> }
> ASSERT(vport->nicState == NdisSwitchNicStateConnected);
>+ASSERT(ovsFwdCtx->curNbl->FirstNetBuffer->Next == NULL);
>
> /* Assert that in the Rx direction, key is always setup. */
> ASSERT(ovsFwdCtx->tunnelRxNic == NULL || ovsFwdCtx->tunKey.dst != 0);
>-status = OvsExtractFlow(ovsFwdCtx->curNbl, ovsFwdCtx->srcVportNo,
>-  &key, &ovsFwdCtx->layers,
>ovsFwdCtx->tunKey.dst != 0 ?
>- &ovsFwdCtx->tunKey : NULL);
>+vlanTagValue = NET_BUFFER_LIST_INFO(ovsFwdCtx->curNbl,
>+Ieee8021QNetBufferListIn

Re: [ovs-dev] [PATCH] Rename files under datapath-windows

[Alin Serdean]

You need to commit the patch also. It needs to do the diff between git ls-files 
and the actual files.

-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com] 
Trimis: Wednesday, August 27, 2014 6:59 PM
Către: Alin Serdean; Samuel Ghinet; dev@openvswitch.org
Cc: Nithin Raju
Subiect: RE: [ovs-dev] [PATCH] Rename files under datapath-windows

Hi Alin,

I was getting an error after reconf which I now realize was due to missing 
files in the distribution.

make[3]: Entering directory `/home/Administrator/work/temp/ovs/datapath'
make[3]: Leaving directory `/home/Administrator/work/temp/ovs/datapath'
The distribution is missing the following files:
datapath-windows/ovsext/OvsActions.c
datapath-windows/ovsext/OvsAtomic.h
datapath-windows/ovsext/OvsBufferMgmt.c
datapath-windows/ovsext/OvsBufferMgmt.h
datapath-windows/ovsext/OvsChecksum.c
datapath-windows/ovsext/OvsChecksum.h
datapath-windows/ovsext/OvsDebug.c
datapath-windows/ovsext/OvsDebug.h
datapath-windows/ovsext/OvsDriver.c
datapath-windows/ovsext/OvsEth.h
datapath-windows/ovsext/OvsEvent.c
datapath-windows/ovsext/OvsEvent.h
datapath-windows/ovsext/OvsFlow.c
datapath-windows/ovsext/OvsFlow.h
datapath-windows/ovsext/OvsIoctl.c
datapath-windows/ovsext/OvsIoctl.h
datapath-windows/ovsext/OvsIpHelper.c
datapath-windows/ovsext/OvsIpHelper.h
datapath-windows/ovsext/OvsJhash.c
datapath-windows/ovsext/OvsJhash.h
datapath-windows/ovsext/OvsNetProto.h
datapath-windows/ovsext/OvsOid.c
datapath-windows/ovsext/OvsOid.h
datapath-windows/ovsext/OvsPacketIO.c
datapath-windows/ovsext/OvsPacketIO.h
datapath-windows/ovsext/OvsPacketParser.c
datapath-windows/ovsext/OvsPacketParser.h
datapath-windows/ovsext/OvsSwitch.c
datapath-windows/ovsext/OvsSwitch.h
datapath-windows/ovsext/OvsTunnel.c
datapath-windows/ovsext/OvsTunnel.h
datapath-windows/ovsext/OvsTunnelFilter.c
datapath-windows/ovsext/OvsTunnelIntf.h
datapath-windows/ovsext/OvsTypes.h
datapath-windows/ovsext/OvsUser.c
datapath-windows/ovsext/OvsUser.h
datapath-windows/ovsext/OvsUtil.c
datapath-windows/ovsext/OvsUtil.h
datapath-windows/ovsext/OvsVport.c
datapath-windows/ovsext/OvsVport.h
datapath-windows/ovsext/OvsVxlan.c
datapath-windows/ovsext/OvsVxlan.h
make[2]: *** [dist-hook-git] Error 1

Although, for some bizarre reason this went away once I added the new files to 
git.

Saurabh

> -Original Message-
> From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com]
> Sent: Wednesday, August 27, 2014 7:34 AM
> To: Saurabh Shah; Samuel Ghinet; dev@openvswitch.org
> Cc: Nithin Raju
> Subject: RE: [ovs-dev] [PATCH] Rename files under datapath-windows
> 
> Hi Saurabh,
> 
> Could you please try to do the autoreconf again.
> ./boot.sh
> ./configure
> 
> Thanks.
> 
> -Mesaj original-
> De la: Saurabh Shah [mailto:ssaur...@vmware.com]
> Trimis: Wednesday, August 27, 2014 3:01 AM
> Către: Samuel Ghinet; dev@openvswitch.org
> Cc: Alin Serdean; Nithin Raju
> Subiect: RE: [ovs-dev] [PATCH] Rename files under datapath-windows
> 
> Hi Samuel,
> 
> I manually patched the change but I wasn't able to build it. Haven't spent
> much time figuring out why.
> 
> link -nologo -DEBUG -out:ovsdb/ovsdb-server.exe ovsdb/ovsdb-server.obj -
> LIBPATH:
> C:/pthread/lib/x86 ovsdb/.libs/libovsdb.lib lib/.libs/libopenvswitch.lib
> pthread
> VC2.lib ws2_32.lib
> LINK : ovsdb/ovsdb-server.exe not found or not built by the last incremental
> lin
> k; performing full link
> make[2]: *** No rule to make target `datapath-windows/ovsext/Actions.c',
> needed
> by `all-am'.  Stop.
> make[2]: Leaving directory `/home/Administrator/work/ro/ovs'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/home/Administrator/work/ro/ovs'
> make: *** [all] Error 2
> 
> It would make sense to mention in the coding guide the quirk about using
> 'Ovs' prefix to interfaces that are shared with userspace.
> 
> Otherwise the change looks good to me. Thanks for working on this.
> 
> Saurabh
> 
> > -Original Message-
> > From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com]
> > Sent: Tuesday, August 26, 2014 1:04 PM
> > To: dev@openvswitch.org
> > Cc: Alin Serdean; Saurabh Shah; Nithin Raju
> > Subject: [ovs-dev] [PATCH] Rename files under datapath-windows
> >
> >
> > This patch includes the file renaming and accommodations needed for the
> > file
> > renaming to build the forwarding extension for Hyper-V.
> >
> > This patch is also a follow-up for the thread:
> >
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/piper
> > mail/dev/2014-
> >
> August/044005.html&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjs
> >
> HfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=7Rg7UPq8iQ7
> >
> HcPlOgJ4FvU9sZhZ0GQRhzx1LUZtWmrk%3D%0A&s=310a208afab4c3400497c
> > bb3d336923893557324acca90bd4505c1f7c2bfb60e
> >
> > Signed-off-by: Samuel Ghinet 
> > Co-authored-by: Alin Gabriel Serdean
> 
> > ---
> >  build-aux/extract-odp-netlink-windows-dp-h |   2 +-
> >  datapath-windows/automake.mk   | 126 
> > +

Re: [ovs-dev] [PATCH] lib/flow.h: Improve struct miniflow comment and definition.

[Ben Pfaff]

On Wed, Aug 27, 2014 at 08:54:47AM -0700, Gurucharan Shetty wrote:
> On Tue, Aug 26, 2014 at 4:08 PM, Ben Pfaff  wrote:
> > On Tue, Aug 26, 2014 at 04:04:09PM -0700, Jarno Rajahalme wrote:
> >>
> >> On Aug 26, 2014, at 3:53 PM, Ben Pfaff  wrote:
> >>
> >> > On Tue, Aug 26, 2014 at 03:42:33PM -0700, Jarno Rajahalme wrote:
> >> >> Finally, use change the storage type of 'values_inline' to uint8_t, as
> >> >> uint64_t looks kind of wide for a boolean, even though we intend the
> >> >> bit be carved out from the uint64_t where 'map' resides.
> >> >
> >> > It can't be type "bool"?
> >>
> >> From CodingStyle:
> >>
> >>   Declare bit-fields to be type "unsigned int" or "signed int".  Do
> >> *not* declare bit-fields of type "int": C89 allows these to be either
> >> signed or unsigned according to the compiler's whim.  (A 1-bit
> >> bit-field of type "int" may have a range of -1...0!)  Do not declare
> >> bit-fields of type _Bool or enum or any other type, because these are
> >> not portable.
> >>
> >> :-)
> >
> > This was important when we supported C89 compilers that did not have
> > native _Bool (bool), but I think that it is obsolete now, because all
> > of the compilers we support have C99 implementations good enough to
> > have native _Bool.
> >
> > Let's find out and update CodingStyle if so.
> >
> > Guru: does MSVC allow "bool" bit-fields?  I assume so but it's worth a
> > test.
> 
> You mean something like this, right?  If so, it does work.

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

[ovs-dev] [PATCH] CodingStyle: Relax restrictions on types of bit-fields.

[Ben Pfaff]

C99 only requires compilers to support four types for bit-fields: signed
int, unsigned int, int, and _Bool.  "int" should not be used because it
is implementation-defined whether it is signed.  In practice, we have found
that compilers (in particular, GCC, Clang, and MSVC 2013) support any
integer type.

Signed-off-by: Ben Pfaff 
---
 CodingStyle | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/CodingStyle b/CodingStyle
index ce97258..c9b9989 100644
--- a/CodingStyle
+++ b/CodingStyle
@@ -428,12 +428,10 @@ prints 255 but printf("%u", -1) prints 4294967295.
 network protocol fields or in other circumstances where the exact
 format is important.
 
-  Declare bit-fields to be type "unsigned int" or "signed int".  Do
-*not* declare bit-fields of type "int": C89 allows these to be either
-signed or unsigned according to the compiler's whim.  (A 1-bit
-bit-field of type "int" may have a range of -1...0!)  Do not declare
-bit-fields of type _Bool or enum or any other type, because these are
-not portable.
+  Declare bit-fields to be signed or unsigned integer types or _Bool
+(aka bool).  Do *not* declare bit-fields of type "int": C99 allows
+these to be either signed or unsigned according to the compiler's
+whim.  (A 1-bit bit-field of type "int" may have a range of -1...0!)
 
   Try to order structure members such that they pack well on a system
 with 2-byte "short", 4-byte "int", and 4- or 8-byte "long" and pointer
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

[Ben Pfaff]

On Wed, Aug 27, 2014 at 04:01:17PM +, Samuel Ghinet wrote:
> For future reviews, could you please reduce or mark somehow the patch, so it 
> would be easier for me to find your comments?
> Something like,
> 
> >All NET_BUFFERs of a NET_BUFFER_LIST must go through
> >the pipeline: extract, find flow, execute. Previously,
> >only the first NET_BUFFER of a NET_BUFFER_LIST was going
> >through this pipeline, which was erroneous.
> 
> comments here.

Or you guys could quote with a > prefix, like pretty much everyone else
on the ovs-dev list, and trim irrelevant bits.  That would eliminate a
lot of confusing and hard-to-read messages.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] Proposal for enhanced select groups

[Ben Pfaff]

On Wed, Aug 27, 2014 at 10:26:14AM +0900, Simon Horman wrote:
> On Fri, Aug 22, 2014 at 08:30:08AM -0700, Ben Pfaff wrote:
> > On Fri, Aug 22, 2014 at 09:19:41PM +0900, Simon Horman wrote:
> > > I have been working with Netronome on examining the possibilities of
> > > providing (richer) load balancing facilities in Open vSwitch.
> > > 
> > > It seems to us that the current select group provides for some load
> > > balancing functionality. And that in particular the way that it is
> > > implemented in Open vSwitch provides L2 destination load balancing (it
> > > hashes on the destination ethernet address). Our ideas so fare are as 
> > > follows:
> > > 
> > > 1. Provide a richer and ideally extendible select group in the
> > >form of an OpenFlow extension to groups.
> > > 
> > >* Allow the fields used to be selected.
> > > 
> > >  In the case of a hash this would be the fields that are hashed.
> > > 
> > >  An implication of this is that the pre-requisites of these
> > >  fields would need to be present in the flow's match.
> > >  In masking of the fields would be allowed but not
> > >  required for fields whose TLVs allow masking.
> > > 
> > >* Allow designation of the selection method used.
> > > 
> > >  For example hash.
> > > 
> > >* Allow passing a parameter to the selection method.
> > > 
> > >  For example an initial value key for hashes.
> > 
> > There is an outstanding patch on this topic already:
> > http://patchwork.openvswitch.org/patch/5424/
> 
> I have no particular objections to that change, though I have
> not thought about it deeply. However, I think its more valuable
> in the long run to make select groups configurable rather than
> tweaking what would be the default setting.
> 
> > It sounds reasonable to make select groups configurable.  The way to do
> > that would be to implement the OpenFlow 1.5 (draft) proposal to add
> > properties to groups and group buckets, which is filed in the ONF JIRA
> > as EXT-350.
> 
> I'm happy to look at implementing EXT-350 (which I now have access to :)
> however it seems to me that while it makes groups more configurable
> it does not address the ability to configure the selection method.

Why not?  Part of the configuration can be the selection method.

> What we would like to do is to provide something generally useful
> which may be used as appropriate to:

I'm going to skip past these ideas, which do sound interesting, because
I think that they're more for Pravin and Jesse than for me.  I hope that
they will provide some reactions to them.

> --
> Proposal: Proposal for enhanced select groups
> Version: 0.0.1

The proposal seems reasonable on its own but given that EXT-350 allows a
standardized way to add group configuration extension I am not sure it
makes sense to add a specialized way to do that.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead of ReadFile/WriteFile

[Eitan Eliahu]

+if (!DeviceIoControl(sock->handle, OVS_IOCTL_READ,
+ NULL, 0, tail, sizeof tail, &bytes, NULL))
...
+memcpy(ofpbuf_data(buf), tail, retval);
+ofpbuf_set_size(buf, retval);

We probably want to send down the "struct iovec iov" (rather than the tail)  
parameter so we can use the original and avoid the data copy.
This will require to map the user mode virtual address into the kernel (since 
the kernel executed in the context of the use mode thread we don't have to pin 
this memory).
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Nithin Raju
Sent: Wednesday, August 27, 2014 8:36 AM
To: dev@openvswitch.org; aserd...@cloudbasesolutions.com
Subject: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead of 
ReadFile/WriteFile

The Windows datapath supports a READ/WRITE ioctl instead of ReadFile/WriteFile.
In this change, we update the following:
- WriteFile() in nl_sock_send__() to use DeviceIoControl(OVS_IOCTL_WRITE)
- ReadFile() in nl_sock_recv__() to use DeviceIoControl(OVS_IOCTL_READ)

The WriteFile() call in nl_sock_transact_multiple__() has not been touched 
since it is not needed yet.

Main motive for this change is to be able to unblock the DP Dump workflow.

Signed-off-by: Nithin Raju 
---
 lib/netlink-socket.c |   46 ++
 1 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index 4d9832f..a6be186 
100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -426,17 +426,20 @@ nl_sock_send__(struct nl_sock *sock, const struct ofpbuf 
*msg,
 do {
 int retval;
 #ifdef _WIN32
-bool result;
-DWORD last_error = 0;
-result = WriteFile(sock->handle, ofpbuf_data(msg), ofpbuf_size(msg),
-   &retval, NULL);
-last_error = GetLastError();
-if (last_error != ERROR_SUCCESS && !result) {
+DWORD bytes;
+
+if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE,
+ofpbuf_data(msg), ofpbuf_size(msg), NULL, 0,
+&bytes, NULL)) {
 retval = -1;
-errno = EAGAIN;
+/* XXX: Map to a more appropriate error based on GetLastError(). */
+errno = EINVAL;
+} else {
+retval = ofpbuf_size(msg);
 }
 #else
-retval = send(sock->fd, ofpbuf_data(msg), ofpbuf_size(msg), wait ? 0 : 
MSG_DONTWAIT);
+retval = send(sock->fd, ofpbuf_data(msg), ofpbuf_size(msg),
+  wait ? 0 : MSG_DONTWAIT);
 #endif
 error = retval < 0 ? errno : 0;
 } while (error == EINTR);
@@ -488,12 +491,7 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
bool wait)
  * 'tail' to allow Netlink messages to be up to 64 kB long (a reasonable
  * figure since that's the maximum length of a Netlink attribute). */
 struct nlmsghdr *nlmsghdr;
-#ifdef _WIN32
-#define MAX_STACK_LENGTH 81920
-uint8_t tail[MAX_STACK_LENGTH];
-#else
 uint8_t tail[65536];
-#endif
 struct iovec iov[2];
 struct msghdr msg;
 ssize_t retval;
@@ -522,15 +520,23 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
bool wait)
 do {
 nlmsghdr->nlmsg_len = UINT32_MAX;  #ifdef _WIN32
-boolean result = false;
-DWORD last_error = 0;
-result = ReadFile(sock->handle, tail, MAX_STACK_LENGTH, &retval, NULL);
-last_error = GetLastError();
-if (last_error != ERROR_SUCCESS && !result) {
+DWORD bytes;
+if (!DeviceIoControl(sock->handle, OVS_IOCTL_READ,
+ NULL, 0, tail, sizeof tail, &bytes, NULL)) 
+ {
 retval = -1;
-errno = EAGAIN;
+errno = EINVAL;
 } else {
-ofpbuf_put(buf, tail, retval);
+retval = bytes;
+if (retval == 0) {
+retval = -1;
+errno = EAGAIN;
+} else {
+if (retval >= buf->allocated) {
+ofpbuf_reinit(buf, retval);
+}
+memcpy(ofpbuf_data(buf), tail, retval);
+ofpbuf_set_size(buf, retval);
+}
 }
 #else
 retval = recvmsg(sock->fd, &msg, wait ? 0 : MSG_DONTWAIT);
--
1.7.4.1

___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=pobbphoswUVAlYlGyDNMKifynvHgFWdpDrVvUMLFReg%3D%0A&s=83541ef0d336bd56a4c781e92871129c4339a90888183dd5fc13efa1646b0a37
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] OVS-on-HyperV: Agenda for IRC meeting on 8/27

[Nithin Raju]

hi,
I'm sending out the meeting minutes for future reference.

1. Netlink implementation:
--
Status wise, discussed the following:
 a) Nithin created issues on github/ovs-issues to distribute the work across 
multiple developers.
 b) Ankur committed the patches for the netlink message, attributes get APIs
 c) Ankur is working on the set API, and he'll be sending out the patches in a 
day or two
 d) Nithin worked on the GET_DP command, and a few hurdles before that like 
lookup family etc. GET_DP patch should be out for review today.

Going forward, we agreed that;
 i) Created a milestone called *'netlink-integration-milestone-1'* to track the 
multiple issues we have to track the work distribution. Issues tracked under: 
https://github.com/openvswitch/ovs-issues/milestones/netlink-integration-milestone-1
 ii) #Action-item: Alin to send out patches for dpif-linux.c soon.
 ii) Sam will be working on the vport commands - dump and add to start with, 
and the rest later.
 iii) Ankur will be working on the flow commands - dump, put, flush.
 iv) Created an issue for packet execute (issue #44)

2. netdev for Windows:
--
It was not clear if not having netdev would be a blocker for vswitchd 
functionality, and whether we should have netdev for Windows implemented by 
'netlink-integration-milestone-1' milestone. We decided to table to discussion 
for later. But, this should not fall through the cracks.

3. Events discussion:
-
Eitan led the discussion on events with Alin and Samuel, and we have agreement 
on how we are going to go about it. Eitan will work on some preliminary patches 
in userspace, and hand it off to Alin. After that, Eitan will focus on the 
kernel changes, and work with Alin on getting the integration completed.

4. Packet receive:
--
Packet receive functionality implementation can possibly go along with the 
Events implementation, but we'll wait till Eitan takes the first stab at the 
changes so we don't to re-invent the wheel.

5. Pending patches:
---
We all agreed that pending patches need to be reviewed soon, esp. the multiple 
NB patch.

thanks,
Nithin


On Aug 26, 2014, at 10:20 AM, Nithin Raju 
mailto:nit...@vmware.com>> wrote:

hi Alin/Sam,
These were the items we wanted to discuss:

1. Netlink implementation:
  * status
  * work has been spread out now, and issues have been created for each 
individual task. There should be no confusion now. This was the plan all along, 
BTW. We'll discuss anything that is not clear.

2. Eitan is back from PTO. If we want to discuss (again) the design for the 
Event notification as well as missed packet, this would be a good time.

3. Pending reviews.

Pls. feel free to add on more items.

thanks,
Nithin

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

Re: [ovs-dev] [RFC] Proposal for enhanced select groups

[Jesse Gross]

On Wed, Aug 27, 2014 at 11:51 AM, Ben Pfaff  wrote:
> On Wed, Aug 27, 2014 at 10:26:14AM +0900, Simon Horman wrote:
>> On Fri, Aug 22, 2014 at 08:30:08AM -0700, Ben Pfaff wrote:
>> > On Fri, Aug 22, 2014 at 09:19:41PM +0900, Simon Horman wrote:
>> What we would like to do is to provide something generally useful
>> which may be used as appropriate to:
>
> I'm going to skip past these ideas, which do sound interesting, because
> I think that they're more for Pravin and Jesse than for me.  I hope that
> they will provide some reactions to them.

For the hardware offloading piece in particular, I would take a look
at the discussion that has been going on in the netdev mailing list. I
think the general consensus (to the extent that there is one) is that
the hardware offload interface should be a block outside of OVS and
then OVS (mostly likely from userspace) configures it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead of ReadFile/WriteFile

[Nithin Raju]

On Aug 27, 2014, at 11:19 AM, Eitan Eliahu  wrote:

> 
> +if (!DeviceIoControl(sock->handle, OVS_IOCTL_READ,
> + NULL, 0, tail, sizeof tail, &bytes, NULL))
> ...
> +memcpy(ofpbuf_data(buf), tail, retval);
> +ofpbuf_set_size(buf, retval);
> 
> We probably want to send down the "struct iovec iov" (rather than the tail)  
> parameter so we can use the original and avoid the data copy.
> This will require to map the user mode virtual address into the kernel (since 
> the kernel executed in the context of the use mode thread we don't have to 
> pin this memory).

Eitan,
That is a good point. We'll have to add support for mapping userspace addresses 
in the kernel and using them. It can be done in the future.

Are you fine with the change as-is for the limited functionality it is supposed 
to provide? I want to get this committed to unblock the read/write workflow. 
Alin might have a more formal patch at some point.

thanks,
Nithin

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

Re: [ovs-dev] [PATCH] build: Add travis continuous integration support

[Justin Pettit]

On August 26, 2014 at 8:57:41 AM, Justin Pettit (jpet...@nicira.com) wrote:
>  
> > On Aug 26, 2014, at 8:48 AM, Ben Pfaff wrote:
> >
> >> On Mon, Aug 25, 2014 at 06:13:14PM +0200, Thomas Graf wrote:
> >> Can you setup a mailing list for the reports on openvswitch.org
> >> and set an environment variable in the github settings to contain
> >> the address of the list? [1]
> >
> > Justin, can we create an "ovs-build" or "ovs-ci" mailing list?
>  
> Yes, I can do that later today.

I created a new "build" mailing list to keep our confusing convention of having 
the list have one name and our refering to it another.  Here's how you can 
subsribe:

http://mail.openvswitch.org/mailman/listinfo/build

The new address is bu...@openvswitch.org.  The first post from the CI program 
will get stuck in moderation, but I'll white-list future posts.

--Justin



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

Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead of ReadFile/WriteFile

[Eitan Eliahu]

Yes, I'll open a gerrit issue for that.
Thnks,
Eitan

From: Nithin Raju
Sent: Wednesday, August 27, 2014 1:16 PM
To: Eitan Eliahu
Cc: dev@openvswitch.org; aserd...@cloudbasesolutions.com
Subject: Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead 
ofReadFile/WriteFile

On Aug 27, 2014, at 11:19 AM, Eitan Eliahu  wrote:

>
> +if (!DeviceIoControl(sock->handle, OVS_IOCTL_READ,
> + NULL, 0, tail, sizeof tail, &bytes, NULL))
> ...
> +memcpy(ofpbuf_data(buf), tail, retval);
> +ofpbuf_set_size(buf, retval);
>
> We probably want to send down the "struct iovec iov" (rather than the tail)  
> parameter so we can use the original and avoid the data copy.
> This will require to map the user mode virtual address into the kernel (since 
> the kernel executed in the context of the use mode thread we don't have to 
> pin this memory).

Eitan,
That is a good point. We'll have to add support for mapping userspace addresses 
in the kernel and using them. It can be done in the future.

Are you fine with the change as-is for the limited functionality it is supposed 
to provide? I want to get this committed to unblock the read/write workflow. 
Alin might have a more formal patch at some point.

thanks,
Nithin

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

Re: [ovs-dev] [PATCH] ovsdb: Catch negative timeout values

[Thomas Graf]

On 08/27/14 at 07:48am, Ben Pfaff wrote:
> Are you sure?  I think that the only real bug here is that if the
> timeout is negative *and* the "until" string is invalid, then there's a
> memory leak of the error message for the negative timeout.  We should
> still fix that, and the same code change does that, but I think that the
> commit message is wrong.

You are right Ben, the commit message is misleading. I'll send a v2.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead of ReadFile/WriteFile

[Eitan Eliahu]

Acked-by: Eitan Eliahu 

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Eitan Eliahu
Sent: Wednesday, August 27, 2014 1:47 PM
To: Nithin Raju
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead 
of ReadFile/WriteFile


Yes, I'll open a gerrit issue for that.
Thnks,
Eitan

From: Nithin Raju
Sent: Wednesday, August 27, 2014 1:16 PM
To: Eitan Eliahu
Cc: dev@openvswitch.org; aserd...@cloudbasesolutions.com
Subject: Re: [ovs-dev] [PATCH] netlink-socket.c: use read/write ioctl instead 
ofReadFile/WriteFile

On Aug 27, 2014, at 11:19 AM, Eitan Eliahu  wrote:

>
> +if (!DeviceIoControl(sock->handle, OVS_IOCTL_READ,
> + NULL, 0, tail, sizeof tail, &bytes, NULL))
> ...
> +memcpy(ofpbuf_data(buf), tail, retval);
> +ofpbuf_set_size(buf, retval);
>
> We probably want to send down the "struct iovec iov" (rather than the tail)  
> parameter so we can use the original and avoid the data copy.
> This will require to map the user mode virtual address into the kernel (since 
> the kernel executed in the context of the use mode thread we don't have to 
> pin this memory).

Eitan,
That is a good point. We'll have to add support for mapping userspace addresses 
in the kernel and using them. It can be done in the future.

Are you fine with the change as-is for the limited functionality it is supposed 
to provide? I want to get this committed to unblock the read/write workflow. 
Alin might have a more formal patch at some point.

thanks,
Nithin

___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0A&m=w2J3%2BXT3epx%2FmqiChgH7BLE5reu7HJQaxjBJzpLwDo0%3D%0A&s=664e7a63287c1fde79e0964922eb3485f92d14bea65996c0f391a69128d74c16
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [patch net-next RFC 03/12] net: introduce generic switch devices support

[Cong Wang]

On Thu, Aug 21, 2014 at 9:18 AM, Jiri Pirko  wrote:
> diff --git a/include/linux/switchdev.h b/include/linux/switchdev.h
> new file mode 100644
> index 000..ba77a68
> --- /dev/null
> +++ b/include/linux/switchdev.h

It should be in include/net/ instead, since it never
goes out of networking.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath: remove flow member from struct ovs_skb_cb

[Pravin Shelar]

On Tue, Aug 26, 2014 at 11:43 PM, Joe Stringer  wrote:
> I've been having a bit of trouble with ovs after this patch was applied,
> ovs-vswitchd locks up on Debian 7.6 (VBox VM). Kernel is 3.2.0-4-amd64 #1
> SMP Debian 3.2.60-1+deb7u3 x86_64 GNU/Linux.
>
> I have a fairly typical phy bridge + int bridge setup, with dhclient running
> on br-int:
>
> # ovs-vsctl show
> a9624cfa-2ae8-42ee-9275-3f5b022d09a5
> Bridge "breth0"
> Port "breth0+"
> Interface "breth0+"
> type: patch
> options: {peer="breth0-"}
> Port "eth0"
> Interface "eth0"
> Port "breth0"
> Interface "breth0"
> type: internal
> Bridge br-int
> Port br-int
> Interface br-int
> type: internal
> Port "breth0-"
> Interface "breth0-"
> type: patch
> options: {peer="breth0+"}
> ovs_version: "2.3.90"
>
> From one terminal, I send a bunch of traffic to a neighbour:
> # ovs-appctl upcall/disable-megaflows && hping3 --flood 10.0.2.2
>
> From another terminal, I observe the upcall output:
> # watch -n 0.2 'ovs-appctl upcall/show'
>
> After 10-20 seconds, the output in the second terminal freezes. Around 2-5
> minutes later, dmesg starts complaining about blocked threads:
>
> [9957.804271] openvswitch: Open vSwitch switching datapath 2.3.90, built Aug
> 27 2014 14:15:14
> [10088.874770] device ovs-system entered promiscuous mode
> [10088.883911] openvswitch: netlink: Key attribute has unexpected length
> (type=21, length=8, expected=4).
> [10088.913387] device br-int entered promiscuous mode
> [10088.917780] device eth0 entered promiscuous mode
> [10088.946122] device breth0 entered promiscuous mode
> [10117.536276] ADDRCONF(NETDEV_UP): eth0: link is not ready
> [10117.576174] e1000: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow
> Control: RX
> [10117.576174] ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [10127.808269] eth0: no IPv6 routers present
> [10131.104185] br-int: no IPv6 routers present
> [10133.976474] breth0: no IPv6 routers present
> [10321.389028] INFO: task ovs-vswitchd:7073 blocked for more than 120
> seconds.
> [10321.389028] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
> this message.
> [10321.389028] ovs-vswitchdD 88011fc13780 0  7073  1
> 0x
> [10321.389028]  8800c398e800 0082 00030246
> 8801178610c0
> [10321.389028]  00013780 8800ca8bffd8 8800ca8bffd8
> 8800c398e800
> [10321.389028]  8800c398e800  
> 8165c0f0
> [10321.389028] Call Trace:
> [10321.389028]  [] ? __mutex_lock_common.isra.5+0xff/0x164
> [10321.389028]  [] ? mutex_lock+0x1a/0x2d
> [10321.389028]  [] ? netlink_trim+0x13/0x7a
> [10321.389028]  [] ? genl_rcv+0xe/0x28
> [10321.389028]  [] ? netlink_unicast+0xe6/0x14e
> [10321.389028]  [] ? netlink_sendmsg+0x274/0x2ac
> [10321.389028]  [] ? sock_sendmsg+0xc1/0xde
> [10321.389028]  [] ? sock_recvmsg+0xcd/0xec
> [10321.389028]  [] ? file_update_time+0x30/0x105
> [10321.389028]  [] ? should_resched+0x5/0x23
> [10321.389028]  [] ? _cond_resched+0x7/0x1c
> [10321.389028]  [] ? copy_from_user+0x18/0x30
> [10321.389028]  [] ? should_resched+0x5/0x23
> [10321.389028]  [] ? ___sys_sendmsg+0x209/0x2a9
> [10321.389028]  [] ? __wake_up+0x35/0x46
> [10321.389028]  [] ? _raw_spin_unlock_irqrestore+0xe/0xf
> [10321.389028]  [] ? netlink_insert+0x109/0x130
> [10321.389028]  [] ? should_resched+0x5/0x23
> [10321.389028]  [] ? _cond_resched+0x7/0x1c
> [10321.389028]  [] ? sys_getsockname+0x8e/0xb7
> [10321.389028]  [] ? fget_light+0x67/0x7b
> [10321.389028]  [] ? __sys_sendmsg+0x39/0x58
> [10321.389028]  [] ? system_call_fastpath+0x16/0x1b
> [10321.389028] INFO: task handler10:7112 blocked for more than 120 seconds.
> [10321.389028] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
> this message.
> [10321.389028] handler10   D 8800d2c59800 0  7112  1
> 0x
> [10321.389028]  8800d2c59800 0082 
> 880118292800
> [10321.389028]  00013780 8800d0f73fd8 8800d0f73fd8
> 8800d2c59800
> [10321.389028]  8103661f 0010 0246
> 8165c0f0
> [10321.389028] Call Trace:
> [10321.389028]  [] ? need_resched+0x14/0x18
> [10321.389028]  [] ? __mutex_lock_common.isra.5+0xff/0x164
> [10321.389028]  [] ? mutex_lock+0x1a/0x2d
> [10321.389028]  [] ? netlink_trim+0x13/0x7a
> [10321.389028]  [] ? genl_rcv+0xe/0x28
> [10321.389028]  [] ? netlink_unicast+0xe6/0x14e
> [10321.389028]  [] ? genl_rcv+0x28/0x28
> [10321.389028]  [] ? netlink_rcv_skb+0x53/0x7a
> [10321.389028]  [] ? genl_rcv+0x1f/0x28
> [10321.389028]  [] ? netlink_unicast+0xe6/0x14e
> [10321.389028]  [] ? netlink_sendmsg+0x274/0x2ac
> [10321.389028]  [] ? sock_sendmsg+0xc1/0xde
> [10321.389028]  [] ? _cond_resched+0x7/0x1c
> [10321.389028]  [] ? update

Re: [ovs-dev] [PATCH] datapath: remove flow member from struct ovs_skb_cb

[Joe Stringer]

On 28 August 2014 10:34, Pravin Shelar  wrote:

> 
>
> This is weird. I could not reproduce this on newer kernel. But I found
> related bug. Can you try attached patch? It may be related.
> I will submit formal patch later.
>
> --Pravin.
>

This patch doesn't fix the issue.

Another slight variation I've seen in dmesg:

[19921.489585] INFO: task revalidator11:11397 blocked for more than 120
seconds.
[19921.489596] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
this message.
[19921.489607] revalidator11   D 88011fd13780 0 11397  1
0x
[19921.489621]  88011805f080 0086 8800
88011aec4780
[19921.489635]  00013780 880107b35fd8 880107b35fd8
88011805f080
[19921.489649]  880107b35ab8 00010286 8135049f
8165c0f0
[19921.489663] Call Trace:
[19921.489677]  [] ? _raw_spin_unlock_irqrestore+0xe/0xf
[19921.489691]  [] ? __mutex_lock_common.isra.5+0xff/0x164
[19921.489704]  [] ? mutex_lock+0x1a/0x2d
[19921.489719]  [] ? netlink_trim+0x13/0x7a
[19921.489733]  [] ? genl_rcv+0xe/0x28
[19921.489748]  [] ? netlink_unicast+0xe6/0x14e
[19921.489763]  [] ? netlink_sendmsg+0x274/0x2ac
[19921.489775]  [] ? __pollwait+0xce/0xce
[19921.489789]  [] ? sock_sendmsg+0xc1/0xde
[19921.489802]  [] ? __pollwait+0xce/0xce
[19921.489816]  [] ? current_fs_time+0x31/0x37
[19921.489831]  [] ? file_update_time+0xda/0x105
[19921.489844]  [] ? _cond_resched+0x7/0x1c
[19921.489997]  [] ? should_resched+0x5/0x23
[19921.490015]  [] ? _cond_resched+0x7/0x1c
[19921.490031]  [] ? copy_from_user+0x18/0x30
[19921.490043]  [] ? should_resched+0x5/0x23
[19921.490060]  [] ? ___sys_sendmsg+0x209/0x2a9
[19921.490076]  [] ? do_sync_read+0xb4/0xec
[19921.490091]  [] ? fget_light+0x67/0x7b
[19921.490105]  [] ? __sys_sendmsg+0x39/0x58
[19921.490120]  [] ? system_call_fastpath+0x16/0x1b
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] Proposal for enhanced select groups

[Simon Horman]

On Wed, Aug 27, 2014 at 09:51:59AM -0700, Ben Pfaff wrote:
> On Wed, Aug 27, 2014 at 10:26:14AM +0900, Simon Horman wrote:
> > On Fri, Aug 22, 2014 at 08:30:08AM -0700, Ben Pfaff wrote:
> > > On Fri, Aug 22, 2014 at 09:19:41PM +0900, Simon Horman wrote:
> > > > I have been working with Netronome on examining the possibilities of
> > > > providing (richer) load balancing facilities in Open vSwitch.
> > > > 
> > > > It seems to us that the current select group provides for some load
> > > > balancing functionality. And that in particular the way that it is
> > > > implemented in Open vSwitch provides L2 destination load balancing (it
> > > > hashes on the destination ethernet address). Our ideas so fare are as 
> > > > follows:
> > > > 
> > > > 1. Provide a richer and ideally extendible select group in the
> > > >form of an OpenFlow extension to groups.
> > > > 
> > > >* Allow the fields used to be selected.
> > > > 
> > > >  In the case of a hash this would be the fields that are hashed.
> > > > 
> > > >  An implication of this is that the pre-requisites of these
> > > >  fields would need to be present in the flow's match.
> > > >  In masking of the fields would be allowed but not
> > > >  required for fields whose TLVs allow masking.
> > > > 
> > > >* Allow designation of the selection method used.
> > > > 
> > > >  For example hash.
> > > > 
> > > >* Allow passing a parameter to the selection method.
> > > > 
> > > >  For example an initial value key for hashes.
> > > 
> > > There is an outstanding patch on this topic already:
> > > http://patchwork.openvswitch.org/patch/5424/
> > 
> > I have no particular objections to that change, though I have
> > not thought about it deeply. However, I think its more valuable
> > in the long run to make select groups configurable rather than
> > tweaking what would be the default setting.
> > 
> > > It sounds reasonable to make select groups configurable.  The way to do
> > > that would be to implement the OpenFlow 1.5 (draft) proposal to add
> > > properties to groups and group buckets, which is filed in the ONF JIRA
> > > as EXT-350.
> > 
> > I'm happy to look at implementing EXT-350 (which I now have access to :)
> > however it seems to me that while it makes groups more configurable
> > it does not address the ability to configure the selection method.
> 
> Why not?  Part of the configuration can be the selection method.
>
> > What we would like to do is to provide something generally useful
> > which may be used as appropriate to:
> 
> I'm going to skip past these ideas, which do sound interesting, because
> I think that they're more for Pravin and Jesse than for me.  I hope that
> they will provide some reactions to them.
> 
> > --
> > Proposal: Proposal for enhanced select groups
> > Version: 0.0.1
> 
> The proposal seems reasonable on its own but given that EXT-350 allows a
> standardized way to add group configuration extension I am not sure it
> makes sense to add a specialized way to do that.

Thanks, as discussed off-line, on closer examination I see that our
proposal could fit into EXT-350 as a group property.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] Proposal for enhanced select groups

[Simon Horman]

On Wed, Aug 27, 2014 at 03:03:53PM -0500, Jesse Gross wrote:
> On Wed, Aug 27, 2014 at 11:51 AM, Ben Pfaff  wrote:
> > On Wed, Aug 27, 2014 at 10:26:14AM +0900, Simon Horman wrote:
> >> On Fri, Aug 22, 2014 at 08:30:08AM -0700, Ben Pfaff wrote:
> >> > On Fri, Aug 22, 2014 at 09:19:41PM +0900, Simon Horman wrote:
> >> What we would like to do is to provide something generally useful
> >> which may be used as appropriate to:
> >
> > I'm going to skip past these ideas, which do sound interesting, because
> > I think that they're more for Pravin and Jesse than for me.  I hope that
> > they will provide some reactions to them.
> 
> For the hardware offloading piece in particular, I would take a look
> at the discussion that has been going on in the netdev mailing list. I
> think the general consensus (to the extent that there is one) is that
> the hardware offload interface should be a block outside of OVS and
> then OVS (mostly likely from userspace) configures it.

Thanks, I am now digesting that conversation.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] 2nd International Conference on Economics, Finance and Management Outlooks

[icbsss01.pakrdw]

Dear *Sir/Madam*



The *Asian Economic and Social Society and Pak Publishing Group* are
organizing a 2nd International Conference on Economics, Finance and
Management Outlooks

20-21 December, 2014,* Pearl International Hotel, Kuala Lumpur, Malaysia.*

Website: http://www.pakrdw.com/?ic=details&id=4



We would like to take this opportunity to invite you and your colleagues to
attend this conference on *20-21 December, 2014 Malaysia*. Please share
this call for paper with your friends and colleagues through forwarding
e-mail.



The call for paper find below this e-mail.





*Call for Paper*



The 2nd International Conference on Economics, Finance and Management
Outlooks 20-21 December is organized by the Asian Economic and Social
Society, and Pak Publishing Group. The purpose of this conference is to
bring together researchers from around the globe in order to present and
discuss new trends in the fields of Scientific Research.





*Topics of interest for submission include, but are not limited to:*

Conference Main Theme:  Economics, Finance and Management Outlooks



*Sub-themes*



*Economic Issues*



 Growth and Development Strategies

Fiscal System and Policy

Monetary System and Policy

Income Distribution

Poverty

Unemployment

Inflation

Investment

Consumption and Saving

Human Capital

Trade Policy

FDI

WTO

The History of Economic Thought

Energy and Environment

Financial Issues

Financial Reforms

Finance and Investment

Finance and Saving

International Trade and



*Finance*

 Asset Pricing Theory

Risk Securitization

Derivatives and Structured Financial Products

Commercial Insurance and Reinsurance

Corporate Finance, Mergers and Acquisitions

The Microstructure of the Financial Market

Liquidity and Price mechanism in stock Market



*Management Issues  *

 Business Management

Corporate Governance

Human Resource Management

Business & Market Strategies

Entrepreneurship

E-business Services

Information Technology Management

Production & Operations Strategies

Total Quality Management

Strategic Management

Mutual Fund Management

T-bill, T-bond, and Dividend Policy

Management Research Methods and Managerial Economics

Corporate Social Responsibility

Economic Sustainability and any disciplines concerning the interaction
between management and enterprise sustainable development

 *Proceeding*

All selected papers will be published in a book with an ISBN by Pak
publishing Group. Conference proceedings will be submitted to ISI Thomson
Reuters Web of Science, Google Scholar, and Microsoft Academic Search for
consideration and indexation.



*Journal Publication*

After conference presentation few selected papers will publish in the
special or regular issues of the following journals:



*Asian Economic and Financial Review *
(*Online ISSN:* -6737 - *Print ISSN:* 2305-2147 )
URL: http://www.aessweb.com/journals/5002



*International journal of Asian Social Science *
(*Online ISSN: *2224-4441*- Print ISSN: *2226-5139 )
URL : http://www.aessweb.com/journals/5007


*International Journal of Management and Sustainability *
(*Online ISSN:* 2306-0662* - Print ISSN:* 2306-9856 )
URL : http://www.pakinsight.com/?ic=journal&journal=11

*International Journal of Sustainable Development & World Policy *
(*Online ISSN: *2305-705X* - Print ISSN: *2306-9929)
URL: http://www.pakinsight.com/?ic=journal&journal=26

*International Journal of Sustainable Energy and Environmental Research*
(*Online ISSN: *2306-6253* - Print ISSN: *2312-5764)
URL: http://www.pakinsight.com/?ic=journal&journal=13



*Humanities and Social Sciences Letters ( Online ISSN: 2312-4318 ) URL: *
http://www.pakinsight.com/?ic=journal&journal=73

International Journal of Business, Economics and Management
(Online ISSN: 2312-0916 - Print ISSN: 2312-5772)

*Journal of Empirical Studies *
(Online ISSN: 2312-6248 - Print ISSN: 2312-623X)
URL: http://www.pakinsight.com/?ic=journal&journal=66

Journal of Social Economics Research
(Online ISSN: 2312-6264 - Print ISSN: 2312-6329)
URL: http://www.pakinsight.com/?ic=journal&journal=35


*Review of Knowledge Economy URL: *
http://www.pakinsight.com/?ic=journal&journal=67



* Journal of Tourism Management Research URL:*
http://www.pakinsight.com/?ic=journal&journal=31

Journal of Challenges
*URL:* http://www.pakinsight.com/?ic=journal&journal=85


*The Econometric Reviews URL: *
http://www.pakinsight.com/?ic=journal&journal=88



* International Journal of Public Policy and Administration Research URL: *
http://www.pakinsight.com/?ic=journal&journal=74



*IMPORTANT DATES*



Abstract Submission Date: 27 September, 2014

Decision of Acceptance/Rejection: Within 15 days of submission

Full Paper Submission Date: 14 November, 2014

Early Bird Discount Date: 14 November, 2014

Conference date: 20-21 December, 2014



*Submit your paper: *
*http://www.pakrdw.com/?ic=details&id=4&info=submission*


*Registration: