[ovs-dev] [PATCH 3/3] Makefile.am: Avoid including internal library in public header files.

2017-02-17 Thread Yi-Hung Wei 
Add a build check that public openvswitch header file should not include
internal library.

Suggested-by: Joe Stringer 
Suggested-by: Daniele Di Proietto 
Signed-off-by: Yi-Hung Wei 
---
 Makefile.am | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 4b1b48d..7126ebe 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -230,6 +230,13 @@ config-h-check:
  echo "every C source file must #include ."; \
  exit 1; \
fi
+   if grep '#include' include/openvswitch/*.h | \
+   grep -vE '(<.*>)|("openvswitch)|("openflow)'; \
+   then \
+ echo "See above for list of violations of the rule that"; \
+ echo "public openvswitch header file should not include internal 
library."; \
+ exit 1; \
+   fi
 .PHONY: config-h-check
 
 # Check for printf() type modifiers that MSVC doesn't support.
-- 
2.7.4

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

[ovs-dev] [PATCH 2/3] meta-flow: Remove dependency of cmap from meta-flow.h.

2017-02-17 Thread Yi-Hung Wei 
Previous patch 04f48a68 ("ofp-actions: Fix variable length meta-flow OXMs.")
introduced dependency of an internal library (cmap.h) to ovs public
interface (meta-flow.h) that may cause potential building problem. In this
patch, we remove cmap from struct mf_field, and provide a wrapper struct
vl_mff_map that resolve the dependency problem.

Fixes: 04f48a68 ("ofp-actions: Fix variable length meta-flow OXMs.")
Suggested-by: Joe Stringer 
Suggested-by: Daniele Di Proietto 
Signed-off-by: Yi-Hung Wei 
---
 build-aux/extract-ofp-fields  |  2 +-
 include/openvswitch/meta-flow.h   | 24 +
 include/openvswitch/ofp-actions.h |  2 ++
 include/openvswitch/ofp-util.h|  1 +
 lib/automake.mk   |  1 +
 lib/meta-flow.c   | 54 +++
 lib/nx-match.c|  1 +
 lib/ofp-actions.c |  1 +
 lib/vl-mff-map.h  | 41 +
 ofproto/ofproto-provider.h|  2 +-
 10 files changed, 82 insertions(+), 47 deletions(-)
 create mode 100644 lib/vl-mff-map.h

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 40f1bb2..498b887 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -386,7 +386,7 @@ def make_meta_flow(meta_flow_h):
 else:
 output += ["-1, /* not usable for prefix lookup */"]
 
-output += ["{OVSRCU_INITIALIZER(NULL)},},"]
+output += ["},"]
 for oline in output:
 print(oline)
 
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index d5c0971..309d5e3 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -23,16 +23,15 @@
 #include 
 #include 
 #include 
-#include "cmap.h"
 #include "openvswitch/flow.h"
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/packets.h"
-#include "openvswitch/thread.h"
 #include "openvswitch/util.h"
 
 struct ds;
 struct match;
 struct ofputil_tlv_table_mod;
+struct vl_mff_map;
 
 /* Open vSwitch fields
  * ===
@@ -1774,9 +1773,6 @@ struct mf_field {
 
 int flow_be32ofs;  /* Field's be32 offset in "struct flow", if prefix tree
 * lookup is supported for the field, or -1. */
-
-/* For variable length mf_fields only. In ofproto->vl_mff_map->cmap. */
-struct cmap_node cmap_node;
 };
 
 /* The representation of a field's value. */
@@ -1853,14 +1849,6 @@ union mf_subvalue {
 };
 BUILD_ASSERT_DECL(sizeof(union mf_value) == sizeof (union mf_subvalue));
 
-/* Variable length mf_fields mapping map. This is a single writer,
- * multiple-reader hash table that a writer must hold the following mutex
- * to access this map. */
-struct vl_mff_map {
-struct cmap cmap;   /* Contains 'struct mf_field' */
-struct ovs_mutex mutex;
-};
-
 bool mf_subvalue_intersect(const union mf_subvalue *a_value,
const union mf_subvalue *a_mask,
const union mf_subvalue *b_value,
@@ -1987,14 +1975,4 @@ void mf_format_subvalue(const union mf_subvalue 
*subvalue, struct ds *s);
 /* Field Arrays. */
 void field_array_set(enum mf_field_id id, const union mf_value *,
  struct field_array *);
-
-/* Variable length fields. */
-void mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
-OVS_REQUIRES(vl_mff_map->mutex);
-enum ofperr mf_vl_mff_map_mod_from_tun_metadata(
-struct vl_mff_map *vl_mff_map, const struct ofputil_tlv_table_mod *)
-OVS_REQUIRES(vl_mff_map->mutex);
-const struct mf_field * mf_get_vl_mff(const struct mf_field *,
-  const struct vl_mff_map *);
-bool mf_vl_mff_invalid(const struct mf_field *, const struct vl_mff_map *);
 #endif /* meta-flow.h */
diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index a3783c2..88f573d 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -26,6 +26,8 @@
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/types.h"
 
+struct vl_mff_map;
+
 /* List of OVS abstracted actions.
  *
  * This macro is used directly only internally by this header, but the list is
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index dd254e6..0c3a10a 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -36,6 +36,7 @@
 struct ofpbuf;
 union ofp_action;
 struct ofpact_set_field;
+struct vl_mff_map;
 
 /* Port numbers. */
 enum ofperr ofputil_port_from_ofp11(ovs_be32 ofp11_port,
diff --git a/lib/automake.mk b/lib/automake.mk
index abc9d0d..b266af1 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -284,6 +284,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/vconn-stream.c \
lib/vconn.c \
lib/versions.h \
+   lib/vl-mff-map.h \
lib/vlan-bitmap.c \
lib/vlan-bitmap.h \

[ovs-dev] [PATCH 1/3] ofp-msgs: Remove unnecessary #include.

2017-02-17 Thread Yi-Hung Wei 
Remove unnecessary #include in ofp-msgs.h so that make will not complain.

Signed-off-by: Yi-Hung Wei 
---
 include/openvswitch/ofp-msgs.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index ffb88b3..07fd8aa 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -40,7 +40,6 @@
 
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/types.h"
-#include "util.h"
 
 struct ovs_list;
 
-- 
2.7.4

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

[ovs-dev] Manejo, administración y uso

2017-02-17 Thread Auditoría Financiera + 11 temas 
Manejo, administración y uso de los recursos financieros.

Auditoría Financiera

Consiste en el examen y evaluación de los documentos, operaciones, registros y 
estados financieros de la Compañía, para determinar si estos reflejan 
razonablemente su situación financiera y los resultados de sus operaciones, así 
como el cumplimiento de las disposiciones económico-financieras.

Si desea que le adjuntemos el temario Sin compromiso, responda este correo con 
la palabra: Info-Auditoría, juntos con los datos solicitados.

Nombre:
Teléfono:
Correo:

y le enviaremos la información completa de este tema que está incluido en 
nuestro Plan Integral de Capacitación de Auditoria y Control Interno.
centro telefónico: 018002129393


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

Re: [ovs-dev] [PATCH ovs V3 13/25] netdev-tc-offloads: Add flower mask to priority map

2017-02-17 Thread Marcelo Ricardo Leitner 
On Wed, Feb 08, 2017 at 05:29:26PM +0200, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Flower classifer requires a different priority per mask,
> so we hash the mask and generate a new priority for
> each new mask used.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> ---
>  lib/netdev-tc-offloads.c | 38 ++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index f88e7ce..48e452a 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -176,6 +176,44 @@ add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int 
> handle,
>  return replace;
>  }
>  
> +struct prio_map_data {
> +struct hmap_node node;
> +struct tc_flower_key mask;
> +ovs_be16 protocol;
> +uint16_t prio;
   
> +};
> +
> +static uint16_t
  
> +get_prio_for_tc_flower(struct tc_flower *flower)
> +{
> +static struct hmap prios = HMAP_INITIALIZER();
> +static struct ovs_mutex prios_lock = OVS_MUTEX_INITIALIZER;
> +static int last_prio = 0;
  ^^^
> +size_t key_len = sizeof(struct tc_flower_key);
> +size_t hash = hash_bytes(>mask, key_len,
> + (OVS_FORCE uint32_t) flower->key.eth_type);
> +struct prio_map_data *data;
> +struct prio_map_data *new_data;
> +
> +ovs_mutex_lock(_lock);
> +HMAP_FOR_EACH_WITH_HASH(data, node, hash, ) {
> +if (!memcmp(>mask, >mask, key_len)
> +&& data->protocol == flower->key.eth_type) {
> +ovs_mutex_unlock(_lock);
> +return data->prio;
> +}
> +}
> +
> +new_data = xzalloc(sizeof *new_data);
> +memcpy(_data->mask, >mask, key_len);
> +new_data->prio = ++last_prio;
 ^^
I don't think this type difference is an issue, but it would be nice if
it's avoided.

More importantly, last_prio will overflow sooner or later here, and I
don't see any check on whether the new value is actually free to use. As
you explained on the other email, if there is a mask with another
protocol already using this priority, flower will reject it.

Considering the useful range is only 16 bit wide, this may happen quite
often I'm afraid.

  Marcelo

> +new_data->protocol = flower->key.eth_type;
> +hmap_insert(, _data->node, hash);
> +ovs_mutex_unlock(_lock);
> +
> +return new_data->prio;
> +}
> +
>  int
>  netdev_tc_flow_flush(struct netdev *netdev)
>  {
> -- 
> 2.7.4
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto/bond: Fix bond/show when all interfaces are disabled

2017-02-17 Thread Andy Zhou 
On Thu, Feb 16, 2017 at 9:39 PM, nickcooper-zhangtonghao
 wrote:
>
> On Feb 17, 2017, at 4:49 AM, Andy Zhou  wrote:
>
> Without this patch, when all slaves are disabled, the 'bond/show'
> command still shows the mac address of last active slave in
> 'active slave mac' output. This patch clears them to zeros.
>
> Signed-off-by: Andy Zhou 

>
> looks good to me.
>
>
I took it as ask, and push to master and branch-2.5 2.6 and 2.7.
Thanks for the review.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] windows: Broken internal netdevs

2017-02-17 Thread Nithin Raju 
Thanks for the fix.

Acked-by: Nithin Raju 



From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Serdean 
Sent: Friday, February 17, 2017 3:10 AM
To: d...@openvswitch.org
Subject: [ovs-dev] [PATCH] windows: Broken internal netdevs

Commit fa07525f9cf3fa698ebc23ea09da477d3d881a87 fixed error logging for
for regular netdevs, however it overlooked "internal" netdevs.

This patch allows "internal" netdev objects to be created and passed to
dpif_port_add().

Reported-by: Nithin Raju 
Signed-off-by: Alin Gabriel Serdean 
---
Intended for branch-2.5,branch-2.6,branch-2.7,master
---
 lib/netdev-windows.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
index b22bd09..375cb32 100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -159,7 +159,10 @@ netdev_windows_system_construct(struct netdev *netdev_)

 /* Query the attributes and runtime status of the netdev. */
 ret = query_netdev(netdev_get_name(>up), , );
-if (ret) {
+/* "Internal" netdevs do not exist in the kernel yet.  They need to be
+ * transformed into a netdev object and passed to dpif_port_add(), which
+ * will add them to the kernel.  */
+if (strcmp(netdev_get_type(>up), "internal") && ret) {
 return ret;
 }
 ofpbuf_delete(buf);
--
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=-xl6DPE_Y3uQD-mpZD7osBo2iL4s3jwdmSjTlGgjlsQ=P92jgkw_e8yXcobASp3agdOOn_TbmMcTLXwVjH037sY=z1OeB_nNoNtLKaQeFUZ-JfP_-aZiU4CmjF5VVblMHuc=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/1] python: Fix logic resulting in too many unexpected replies Fix logic resulting in too many unexpected replies

2017-02-17 Thread Russell Bryant 
On Fri, Feb 17, 2017 at 12:27 PM, Terry Wilson  wrote:

> Since __txn_process_reply always returns None, the existing code
> will always hit the final else for replies and log a debug message
> about receiving an unexpected reply. In the C version,
> ovsdb_idl_txn_process_reply returns true any time the txn is found,
> so that behavior is duplicated here.
>
> Signed-off-by: Terry Wilson 
> ---
>  python/ovs/db/idl.py | 1 +
>  1 file changed, 1 insertion(+)
>

Thanks for the patch.  I fixed up the first line of the commit message and
then applied this to master, branch-2.7, and branch-2.6.

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

Re: [ovs-dev] [PATCH] Move lib/cmap.h to include/openvswitch directory.

2017-02-17 Thread Joe Stringer 
On 17 February 2017 at 11:21, Yi-Hung Wei  wrote:
> Fixes: 04f48a68 ("ofp-actions: Fix variable length meta-flow OXMs.")
> VMWare-BZ: 1815360
> Signed-off-by: Yi-Hung Wei 
> ---

I have some concern about exporting this; unlike the other data
structures like lists and hmaps which provide no locking semantics and
it's up to the user to provide guarantees about read/write
consistency, cmap builds in a bunch of requirements around RCU
quiescing and such. Mechanically this patch is simple but the
implications of exporting this in libopenvswitch are quite wide and it
needs thinking through.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Move lib/cmap.h to include/openvswitch directory.

2017-02-17 Thread Yi-Hung Wei 
Fixes: 04f48a68 ("ofp-actions: Fix variable length meta-flow OXMs.")
VMWare-BZ: 1815360
Signed-off-by: Yi-Hung Wei 
---
 include/openvswitch/automake.mk |   1 +
 include/openvswitch/cmap.h  | 279 
 include/openvswitch/meta-flow.h |   2 +-
 lib/automake.mk |   1 -
 lib/classifier-private.h|   2 +-
 lib/classifier.h|   2 +-
 lib/cmap.c  |   4 +-
 lib/cmap.h  | 279 
 lib/dpif-netdev.c   |   2 +-
 lib/netdev.c|   2 +-
 lib/tnl-neigh-cache.c   |   2 +-
 ofproto/ofproto-dpif-rid.h  |   2 +-
 ofproto/ofproto-dpif-upcall.c   |   2 +-
 tests/test-cmap.c   |   2 +-
 14 files changed, 291 insertions(+), 291 deletions(-)
 create mode 100644 include/openvswitch/cmap.h
 delete mode 100644 lib/cmap.h

diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index c0e276f..d9025ff 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -1,5 +1,6 @@
 openvswitchincludedir = $(includedir)/openvswitch
 openvswitchinclude_HEADERS = \
+   include/openvswitch/cmap.h \
include/openvswitch/compiler.h \
include/openvswitch/dynamic-string.h \
include/openvswitch/hmap.h \
diff --git a/include/openvswitch/cmap.h b/include/openvswitch/cmap.h
new file mode 100644
index 000..18105c1
--- /dev/null
+++ b/include/openvswitch/cmap.h
@@ -0,0 +1,279 @@
+/*
+ * Copyright (c) 2014, 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef OPENVSWITCH_CMAP_H
+#define OPENVSWITCH_CMAP_H 1
+
+#include 
+#include 
+#include "ovs-rcu.h"
+#include "util.h"
+
+/* Concurrent hash map
+ * ===
+ *
+ * A single-writer, multiple-reader hash table that efficiently supports
+ * duplicates.
+ *
+ *
+ * Thread-safety
+ * =
+ *
+ * The general rules are:
+ *
+ *- Only a single thread may safely call into cmap_insert(),
+ *  cmap_remove(), or cmap_replace() at any given time.
+ *
+ *- Any number of threads may use functions and macros that search or
+ *  iterate through a given cmap, even in parallel with other threads
+ *  calling cmap_insert(), cmap_remove(), or cmap_replace().
+ *
+ *  There is one exception: cmap_find_protected() is only safe if no thread
+ *  is currently calling cmap_insert(), cmap_remove(), or cmap_replace().
+ *  (Use ordinary cmap_find() if that is not guaranteed.)
+ *
+ *- See "Iteration" below for additional thread safety rules.
+ *
+ * Writers must use special care to ensure that any elements that they remove
+ * do not get freed or reused until readers have finished with them.  This
+ * includes inserting the element back into its original cmap or a different
+ * one.  One correct way to do this is to free them from an RCU callback with
+ * ovsrcu_postpone().
+ */
+
+/* A concurrent hash map node, to be embedded inside the data structure being
+ * mapped.
+ *
+ * All nodes linked together on a chain have exactly the same hash value. */
+struct cmap_node {
+OVSRCU_TYPE(struct cmap_node *) next; /* Next node with same hash. */
+};
+
+static inline struct cmap_node *
+cmap_node_next(const struct cmap_node *node)
+{
+return ovsrcu_get(struct cmap_node *, >next);
+}
+
+static inline struct cmap_node *
+cmap_node_next_protected(const struct cmap_node *node)
+{
+return ovsrcu_get_protected(struct cmap_node *, >next);
+}
+
+/* Concurrent hash map. */
+struct cmap {
+OVSRCU_TYPE(struct cmap_impl *) impl;
+};
+
+/* Initializer for an empty cmap. */
+#define CMAP_INITIALIZER {  \
+.impl = OVSRCU_INITIALIZER((struct cmap_impl *) _cmap)\
+}
+extern OVS_ALIGNED_VAR(CACHE_LINE_SIZE) const struct cmap_impl empty_cmap;
+
+/* Initialization. */
+void cmap_init(struct cmap *);
+void cmap_destroy(struct cmap *);
+
+/* Count. */
+size_t cmap_count(const struct cmap *);
+bool cmap_is_empty(const struct cmap *);
+
+/* Insertion and deletion.  Return the current count after the operation. */
+size_t cmap_insert(struct cmap *, struct cmap_node *, uint32_t hash);
+static inline size_t cmap_remove(struct cmap *, struct cmap_node *,
+ uint32_t hash);
+size_t cmap_replace(struct cmap *, struct cmap_node *old_node,
+

Re: [ovs-dev] [PATCH ovs V3 12/25] dpif-netlink: Use netdev flow put api to insert a flow

2017-02-17 Thread Marcelo Ricardo Leitner 
On Wed, Feb 15, 2017 at 03:44:30PM +0200, Roi Dayan wrote:
> 
> 
> On 14/02/2017 01:55, Chandran, Sugesh wrote:
> > 
> > 
> > Regards
> > _Sugesh
> > 
> > 
> > > -Original Message-
> > > From: Roi Dayan [mailto:r...@mellanox.com]
> > > Sent: Wednesday, February 8, 2017 3:29 PM
> > > To: d...@openvswitch.org
> > > Cc: Paul Blakey ; Or Gerlitz
> > > ; Hadar Hen Zion ; Shahar
> > > Klein ; Mark Bloch ; Rony
> > > Efraim ; Fastabend, John R
> > > ; Joe Stringer ; Andy
> > > Gospodarek ; Lance Richardson
> > > ; Marcelo Ricardo Leitner ;
> > > Simon Horman ; Jiri Pirko
> > > ; Chandran, Sugesh 
> > > Subject: [PATCH ovs V3 12/25] dpif-netlink: Use netdev flow put api to 
> > > insert
> > > a flow
> > > 
> > > From: Paul Blakey 
> > > 
> > > +
> > > +// XXX: missing ofpbuf_uninit?
> > > +return 0;
> > > +}
> > > +
> > > +static int
> > > +parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) {
> > > +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> > > +struct match match;
> > > +odp_port_t in_port;
> > > +const struct nlattr *nla;
> > > +size_t left;
> > > +int outputs = 0;
> > > +struct netdev *dev;
> > > +struct offload_info info;
> > > +ovs_be16 dst_port = 0;
> > > +int err = 0;
> > > +
> > > +if (put->flags & DPIF_FP_PROBE) {
> > > +return EOPNOTSUPP;
> > > +}
> > > +
> > > +err = parse_key_and_mask_to_match(put->key, put->key_len, put-
> > > > mask,
> > > +  put->mask_len, );
> > > +if (err) {
> > > +return err;
> > > +}
> > > +
> > > +/* Get tunnel dst port and count outputs */
> > > +NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) {
> > > +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
> > > +const struct netdev_tunnel_config *tnl_cfg;
> > > +struct netdev *outdev;
> > > +odp_port_t out_port;
> > > +
> > [Sugesh]  Just wondering , is it the right place to do these checks. In 
> > fact these are limited by the hardware(Correct me if I am wrong here). So 
> > this has to under the specific netdev_flow_put than here?
> 
> right. it was done here for convenience. should probably move it. thanks.
> 
> > > +outputs++;
> > > +if (outputs > 1) {
> > > +VLOG_WARN_RL(, "offloading multiple ports isn't 
> > > supported");

Also, you may want to consider reducing the log level of this message.
Seems I'm hitting it for all broadcasts on the subnet.

2017-02-17T17:28:35.025Z|03238|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:28:45.437Z|03239|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:28:50.315Z|03240|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:28:55.376Z|00649|dpif_netlink(handler20)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:29:17.834Z|03241|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:29:39.952Z|00650|dpif_netlink(handler20)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:29:54.321Z|03242|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:29:59.498Z|03243|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:30:14.300Z|03244|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:30:22.338Z|03245|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:30:24.896Z|00651|dpif_netlink(handler20)|WARN|offloading
multiple ports isn't supported 
2017-02-17T17:30:43.072Z|03246|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported 


for flows like:
recirc_id(0),in_port(2),eth(src=XX:XX:XX:cb:76:af,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0800),ipv4(frag=no),
 packets:2, bytes:692, used:8.821s, actions:3,1

And/or you plan to support it?

> > > +err = EOPNOTSUPP;
> > > +goto err_out;
> > > +}
> > > +
> > > +out_port = nl_attr_get_odp_port(nla);
> > > +outdev = netdev_hmap_port_get(out_port, DPIF_HMAP_KEY(
> > > > dpif));
> > > +tnl_cfg = netdev_get_tunnel_config(outdev);
> > > +if (tnl_cfg

Re: [ovs-dev] manpages in rst?

2017-02-17 Thread Ben Pfaff 
On Fri, Feb 17, 2017 at 07:34:10AM -0500, Stephen Finucane wrote:
> On Thu, 2017-02-16 at 09:24 -0800, Ben Pfaff wrote:
> > Currently, we have some manpages written directly in nroff.  This is
> > an
> > awful format, that is difficult to read and difficult to
> > write.  Other
> > manpages are written in a custom XML format that, while it is easier
> > to
> > read and write, isn't any standard format and so we can't expect
> > anyone
> > else (person or program) to understand it.  This is not ideal.  It's
> > difficult to include either format in the readthedocs documentation,
> > too.
> 
> I started on a converter for that XML format but I haven't had a chance
> to finish it. The biggest problem I encountered was that rST is
> slightly lossy - it's not possible to nest elements the way you can
> nest XML or HTML so you have to drop some stuff. For example,
> converting '' is impossible, since rST doesn't have markup for
> bold AND italic. Figuring out what to drop proved tougher than I
> thought. I basically needed to build a parser rather than hacking
> something together with regexes :(

Hmm.  I think that the XML format, in nroff translation, effectively
ends up with whatever is the innermost markup.  I don't think it does
anything smarter than that.  (I don't even know how to produce
bold-italic in nroff.)

> > I'm thinking about starting to write manpages in REstructured Text
> > (rst).  This would make it much easier to include them in the
> > readthedocs pages, and ReST seems to convert pretty well to nroff for
> > installing as real manpages.  For example, try fetching
> > http://docutils.sourceforge.net/sandbox/manpage-writer/input/test.txt
> > ,
> > which is a rst file, and then running "rst2man test.txt > test.man"
> > and
> > viewing test.man with "man -l" or "groffer".  The output looks fine.
> > 
> > I think that all we'd need for this is a build dependency on
> > python-docutils to ensure that rst2man is available at build time.
> 
> Pure rST would be my recommendation. Sphinx (python-sphinx) supports
> man page output [2] and could be an even better option that rst2man. We
> already have this (optional) dependency for the docs.

Can you help me understand the trade-off?  That is, how is one better
than the other?

> I do note that Django includes the generated man pages in the repo
> itself [1]. While the general policy for version control is not to
> include anything that's autogenerated, this could make packaging and
> installation from source slightly easier by avoiding adding another
> dependency. Then again, Sphinx is widely enough used that we could be
> pretty certain it would be available. I'll leave this to you to decide.

We already have a build dependency on Python, so I don't think that
depending on python-docutils would add much to that; it seems like it's
close to being core Python in any case.  I don't know whether adding
sphinx is a big deal.

> I have a couple of the man pages already converted to rST so, if you're
> happy to wait until after the OpenStack PTG, I could submit these. Also
> happy to review if someone else would like to work on it.

It'd be great to see what you've converted, so that we'd have a head
start.

I'm going to be out of town through next Wednesday, so I'll probably be
unresponsive until then.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/1] python: Fix logic resulting in too many unexpected replies Fix logic resulting in too many unexpected replies

2017-02-17 Thread Terry Wilson 
Since __txn_process_reply always returns None, the existing code
will always hit the final else for replies and log a debug message
about receiving an unexpected reply. In the C version,
ovsdb_idl_txn_process_reply returns true any time the txn is found,
so that behavior is duplicated here.

Signed-off-by: Terry Wilson 
---
 python/ovs/db/idl.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index e43457c..b7b5e1c 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -649,6 +649,7 @@ class Idl(object):
 txn = self._outstanding_txns.pop(msg.id, None)
 if txn:
 txn._process_reply(msg)
+return True
 
 
 def _uuid_to_row(atom, base):
-- 
2.9.3

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

Re: [ovs-dev] [PATCH v3] dpif-netdev: Change definitions of 'idle' & 'processing' cycles

2017-02-17 Thread Kevin Traynor 
On 02/17/2017 10:39 AM, Ciara Loftus wrote:
> Instead of counting all polling cycles as processing cycles, only count
> the cycles where packets were received from the polling.
> 
> Signed-off-by: Georg Schmuecking 
> Signed-off-by: Ciara Loftus 
> Co-authored-by: Georg Schmuecking 
> Acked-by: Kevin Traynor 
> ---
> v3:
> - Updated acks & co-author tags
> - Removed unnecessary PMD_CYCLES_POLLING counter type
> - Explain terms 'idle' and 'processing' cycles in
>   vswitchd/ovs-vswitchd.8.in
> v2:
> - Rebase
> 
>  lib/dpif-netdev.c  | 57 
> ++
>  vswitchd/ovs-vswitchd.8.in |  5 +++-
>  2 files changed, 47 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 30907b7..6bf6809 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -279,8 +279,9 @@ enum dp_stat_type {
>  };
>  
>  enum pmd_cycles_counter_type {
> -PMD_CYCLES_POLLING, /* Cycles spent polling NICs. */
> -PMD_CYCLES_PROCESSING,  /* Cycles spent processing packets */
> +PMD_CYCLES_IDLE,/* Cycles spent idle or unsuccessful polling 
> */
> +PMD_CYCLES_PROCESSING,  /* Cycles spent successfully polling and
> + * processing polled packets */
>  PMD_N_CYCLES
>  };
>  
> @@ -755,10 +756,10 @@ pmd_info_show_stats(struct ds *reply,
>  }
>  
>  ds_put_format(reply,
> -  "\tpolling cycles:%"PRIu64" (%.02f%%)\n"
> +  "\tidle cycles:%"PRIu64" (%.02f%%)\n"
>"\tprocessing cycles:%"PRIu64" (%.02f%%)\n",
> -  cycles[PMD_CYCLES_POLLING],
> -  cycles[PMD_CYCLES_POLLING] / (double)total_cycles * 100,
> +  cycles[PMD_CYCLES_IDLE],
> +  cycles[PMD_CYCLES_IDLE] / (double)total_cycles * 100,
>cycles[PMD_CYCLES_PROCESSING],
>cycles[PMD_CYCLES_PROCESSING] / (double)total_cycles * 
> 100);
>  
> @@ -2953,30 +2954,43 @@ cycles_count_end(struct dp_netdev_pmd_thread *pmd,
>  non_atomic_ullong_add(>cycles.n[type], interval);
>  }
>  
> -static void
> +/* Calculate the intermediate cycle result and add to the counter 'type' */
> +static inline void
> +cycles_count_intermediate(struct dp_netdev_pmd_thread *pmd,
> +  enum pmd_cycles_counter_type type)
> +OVS_NO_THREAD_SAFETY_ANALYSIS
> +{
> +unsigned long long new_cycles = cycles_counter();
> +unsigned long long interval = new_cycles - pmd->last_cycles;
> +pmd->last_cycles = new_cycles;
> +
> +non_atomic_ullong_add(>cycles.n[type], interval);
> +}
> +
> +static int
>  dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
> struct netdev_rxq *rx,
> odp_port_t port_no)
>  {
>  struct dp_packet_batch batch;
>  int error;
> +int batch_cnt = 0;
>  
>  dp_packet_batch_init();
> -cycles_count_start(pmd);
>  error = netdev_rxq_recv(rx, );
> -cycles_count_end(pmd, PMD_CYCLES_POLLING);
>  if (!error) {
>  *recirc_depth_get() = 0;
>  
> -cycles_count_start(pmd);
> +batch_cnt = batch.count;
>  dp_netdev_input(pmd, , port_no);
> -cycles_count_end(pmd, PMD_CYCLES_PROCESSING);
>  } else if (error != EAGAIN && error != EOPNOTSUPP) {
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>  
>  VLOG_ERR_RL(, "error receiving data from %s: %s",
>  netdev_rxq_get_name(rx), ovs_strerror(error));
>  }
> +
> +return batch_cnt;
>  }
>  
>  static struct tx_port *
> @@ -3438,21 +3452,27 @@ dpif_netdev_run(struct dpif *dpif)
>  struct dp_netdev *dp = get_dp_netdev(dpif);
>  struct dp_netdev_pmd_thread *non_pmd;
>  uint64_t new_tnl_seq;
> +int process_packets = 0;
>  
>  ovs_mutex_lock(>port_mutex);
>  non_pmd = dp_netdev_get_pmd(dp, NON_PMD_CORE_ID);
>  if (non_pmd) {
>  ovs_mutex_lock(>non_pmd_mutex);
> +cycles_count_start(non_pmd);
>  HMAP_FOR_EACH (port, node, >ports) {
>  if (!netdev_is_pmd(port->netdev)) {
>  int i;
>  
>  for (i = 0; i < port->n_rxq; i++) {
> -dp_netdev_process_rxq_port(non_pmd, port->rxqs[i].rx,
> -   port->port_no);
> +process_packets +=
> +dp_netdev_process_rxq_port(non_pmd,
> +   port->rxqs[i].rx,
> +   port->port_no);
>  }
>  }
>  }
> +cycles_count_end(non_pmd, process_packets ? PMD_CYCLES_PROCESSING
> +  : PMD_CYCLES_IDLE);
>

[ovs-dev] ASO Boost Services That Rank Your App in Top Charts within 48 Hours

2017-02-17 Thread ASMBee 
 HI APP DEVELOPER,

 Hope all is fine to you!

 Do you want your app be listed on the top of Apple app store Top
Charts?

 With ASMBee ASO Boost services, we can rank your app in targed Top
Charts & Search Results, within 48 hours, and stay there for 72 - 168
hours.

 Contact us or see more details: ASMBee ASO Boost Services
http://wizz.asmranker.com/index.php/campaigns/hj102ont7gc8b/track-url/te411v0oxwb00/5068ce42a1772d35b49d5b1a732c98831a09f43c

 
http://wizz.asmranker.com/index.php/campaigns/hj102ont7gc8b/track-url/te411v0oxwb00/5068ce42a1772d35b49d5b1a732c98831a09f43c

 I'm sorry if I bother you, but it could be very helpful if your app
is on the top of search result and Top Charts.

 Feel free to contact us if you need any further information and
suggestions for ASO Boost.

 Thanks with regards.

 _ASMBEE TEAM_

   
Very sorry if I'm botherring you

 This Email is delievered to d...@openvswitch.org , if this is not you,
or you don't want to see email from us:

 Unsubscribe You from The List
http://wizz.asmranker.com/index.php/campaigns/hj102ont7gc8b/track-url/te411v0oxwb00/6ef4be2662af9ddbc6afeffc95e230897b6a7398
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] datapath-windows: geneve option header is in BE

2017-02-17 Thread Alin Serdean 
At the moment on Windows we suport only LE platforms.

The option header is currently defined in BE.

Found while testing.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Geneve.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Geneve.h b/datapath-windows/ovsext/Geneve.h
index be8a834..019c0dd 100644
--- a/datapath-windows/ovsext/Geneve.h
+++ b/datapath-windows/ovsext/Geneve.h
@@ -71,10 +71,10 @@ typedef struct GeneveOptionHdr {
 UINT32   optionClass:16;
 /* Format of data contained in the option. */
 UINT32   type:8;
-/* Reserved. */
-UINT32   reserved:3;
 /* Length of option in int32 excluding the option header. */
 UINT32   length:5;
+/* Reserved. */
+UINT32   reserved:3;
 } GeneveOptionHdr;
 
 #define GENEVE_CRIT_OPT_TYPE (1 << 7)
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] windows: Crash when the handle communication device cannot be found

2017-02-17 Thread Alin Serdean 
When trying to uninstall/disable the OVS extension the driver will
fail to unload properly(require reboot)/hang until ovs-vswitchd is closed.

The root cause of this behavior is because the handles from ovs-vswitchd
to the kernel communication devices are still opened although the
actual device was removed from the kernel.

Trying to close the handles will also fail because they do not exist.

The remaining option is to cause a crash and rely on the service manager
to restart ovs-vswitchd.

Reported-at: https://github.com/openvswitch/ovs-issues/issues/27
Reported-by: Alin Gabriel Serdean 
Signed-off-by: Alin Gabriel Serdean 
---
 lib/netlink-socket.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
index e45914c..4f56c60 100644
--- a/lib/netlink-socket.c
+++ b/lib/netlink-socket.c
@@ -60,6 +60,20 @@ static void log_nlmsg(const char *function, int error,
 #ifdef _WIN32
 static int get_sock_pid_from_kernel(struct nl_sock *sock);
 static int set_sock_property(struct nl_sock *sock);
+
+/* In the case DeviceIoControl failed and GetLastError returns with
+ * ERROR_NOT_FOUND means we lost communication with the kernel device.
+ * CloseHandle will fail because the handle in 'theory' does not exist.
+ * The only remaining option is to crash and allow the service to be restarted
+ * via service manager.  This is the only way to close the handle from both
+ * userspace and kernel. */
+void
+lost_communication(DWORD last_err)
+{
+if (last_err == ERROR_NOT_FOUND) {
+ovs_fatal(0, "lost communication with the kernel device");
+}
+}
 #endif
 
 /* Netlink sockets. */
@@ -278,6 +292,7 @@ get_sock_pid_from_kernel(struct nl_sock *sock)
 if (!DeviceIoControl(sock->handle, OVS_IOCTL_GET_PID,
  NULL, 0, , sizeof(pid),
  , NULL)) {
+lost_communication(GetLastError());
 retval = EINVAL;
 } else {
 if (bytes < sizeof(pid)) {
@@ -535,11 +550,12 @@ nl_sock_send__(struct nl_sock *sock, const struct ofpbuf 
*msg,
 if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE,
  msg->data, msg->size, NULL, 0,
  , NULL)) {
+lost_communication(GetLastError());
 retval = -1;
 /* XXX: Map to a more appropriate error based on GetLastError(). */
 errno = EINVAL;
 VLOG_DBG_RL(, "fatal driver failure in write: %s",
-ovs_lasterror_to_string());
+ovs_lasterror_to_string());
 } else {
 retval = msg->size;
 }
@@ -629,6 +645,7 @@ nl_sock_recv__(struct nl_sock *sock, struct ofpbuf *buf, 
bool wait)
 DWORD bytes;
 if (!DeviceIoControl(sock->handle, sock->read_ioctl,
  NULL, 0, tail, sizeof tail, , NULL)) {
+lost_communication(GetLastError());
 VLOG_DBG_RL(, "fatal driver failure in transact: %s",
 ovs_lasterror_to_string());
 retval = -1;
@@ -879,6 +896,7 @@ nl_sock_transact_multiple__(struct nl_sock *sock,
 }
 } else if (!ret) {
 /* XXX: Map to a more appropriate error. */
+lost_communication(GetLastError());
 error = EINVAL;
 VLOG_DBG_RL(, "fatal driver failure: %s",
 ovs_lasterror_to_string());
@@ -1275,6 +1293,7 @@ pend_io_request(struct nl_sock *sock)
 error = GetLastError();
 /* Check if the I/O got pended */
 if (error != ERROR_IO_INCOMPLETE && error != ERROR_IO_PENDING) {
+lost_communication(error);
 VLOG_ERR("nl_sock_wait failed - %s\n", ovs_format_message(error));
 retval = EINVAL;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] manpages in rst?

2017-02-17 Thread Stephen Finucane 
On Thu, 2017-02-16 at 09:24 -0800, Ben Pfaff wrote:
> Currently, we have some manpages written directly in nroff.  This is
> an
> awful format, that is difficult to read and difficult to
> write.  Other
> manpages are written in a custom XML format that, while it is easier
> to
> read and write, isn't any standard format and so we can't expect
> anyone
> else (person or program) to understand it.  This is not ideal.  It's
> difficult to include either format in the readthedocs documentation,
> too.

I started on a converter for that XML format but I haven't had a chance
to finish it. The biggest problem I encountered was that rST is
slightly lossy - it's not possible to nest elements the way you can
nest XML or HTML so you have to drop some stuff. For example,
converting '' is impossible, since rST doesn't have markup for
bold AND italic. Figuring out what to drop proved tougher than I
thought. I basically needed to build a parser rather than hacking
something together with regexes :(

> I'm thinking about starting to write manpages in REstructured Text
> (rst).  This would make it much easier to include them in the
> readthedocs pages, and ReST seems to convert pretty well to nroff for
> installing as real manpages.  For example, try fetching
> http://docutils.sourceforge.net/sandbox/manpage-writer/input/test.txt
> ,
> which is a rst file, and then running "rst2man test.txt > test.man"
> and
> viewing test.man with "man -l" or "groffer".  The output looks fine.
> 
> I think that all we'd need for this is a build dependency on
> python-docutils to ensure that rst2man is available at build time.

Pure rST would be my recommendation. Sphinx (python-sphinx) supports
man page output [2] and could be an even better option that rst2man. We
already have this (optional) dependency for the docs.

I do note that Django includes the generated man pages in the repo
itself [1]. While the general policy for version control is not to
include anything that's autogenerated, this could make packaging and
installation from source slightly easier by avoiding adding another
dependency. Then again, Sphinx is widely enough used that we could be
pretty certain it would be available. I'll leave this to you to decide.

I have a couple of the man pages already converted to rST so, if you're
happy to wait until after the OpenStack PTG, I could submit these. Also
happy to review if someone else would like to work on it.

Cheers,
Stephen

[1] https://github.com/django/django/tree/master/docs/man
[2] http://www.sphinx-doc.org/en/stable/builders.html#sphinx.builders.m
anpage.ManualPageBuilder
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] msi: add ovs-vswitchd trigger info This patch changes the service ovs-vswitchd from "auto" execution to "demand" start.

2017-02-17 Thread Alin Serdean 
This patch also introduces a custom action for the ovs-vswitchd service
in which the following command will be executed before the service startup:

sc triggerinfo ovs-vswitchd \
start/strcustom/6066F867-7CA1-4418-85FD-36E3F9C0600C/VmmsWmiEventProvider

The above command is a service trigger available since Windows 7.
More on the topic:
https://msdn.microsoft.com/en-us/library/windows/desktop/dd405513%28v=vs.85%29.aspx

In out case we will wait until Microsoft-Windows-Hyper-V-VMMS has triggered
that the WMI provider: VmmsWmiEventProvider has started.

The change is needed because the network service inside VMMS starts slower than
ovs-vswitchd, which will cause a race condition because we check if the OVS
extension is enabled on a single switch.

Signed-off-by: Alin Gabriel Serdean 
---
Intended for branch-2.7, master.
---
 windows/ovs-windows-installer/CustomActions.wxs | 6 ++
 windows/ovs-windows-installer/Product.wxs   | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/windows/ovs-windows-installer/CustomActions.wxs 
b/windows/ovs-windows-installer/CustomActions.wxs
index bce9455..422f951 100644
--- a/windows/ovs-windows-installer/CustomActions.wxs
+++ b/windows/ovs-windows-installer/CustomActions.wxs
@@ -63,5 +63,11 @@
 
+
+
   
 
diff --git a/windows/ovs-windows-installer/Product.wxs 
b/windows/ovs-windows-installer/Product.wxs
index 0cdc9a8..23bc880 100644
--- a/windows/ovs-windows-installer/Product.wxs
+++ b/windows/ovs-windows-installer/Product.wxs
@@ -83,6 +83,8 @@
   
   
 
+  
+  
   
   
   
@@ -207,7 +209,7 @@
 Name="ovs-vswitchd"
 DisplayName="Open vSwitch Service"
 Description="Open vSwitch Service"
-Start="auto"
+Start="demand"
 Account="LocalSystem"
 ErrorControl="ignore"
 Interactive="no">
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] windows: Broken internal netdevs

2017-02-17 Thread Alin Serdean 
Commit fa07525f9cf3fa698ebc23ea09da477d3d881a87 fixed error logging for
for regular netdevs, however it overlooked "internal" netdevs.

This patch allows "internal" netdev objects to be created and passed to
dpif_port_add().

Reported-by: Nithin Raju 
Signed-off-by: Alin Gabriel Serdean 
---
Intended for branch-2.5,branch-2.6,branch-2.7,master
---
 lib/netdev-windows.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
index b22bd09..375cb32 100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -159,7 +159,10 @@ netdev_windows_system_construct(struct netdev *netdev_)
 
 /* Query the attributes and runtime status of the netdev. */
 ret = query_netdev(netdev_get_name(>up), , );
-if (ret) {
+/* "Internal" netdevs do not exist in the kernel yet.  They need to be
+ * transformed into a netdev object and passed to dpif_port_add(), which
+ * will add them to the kernel.  */
+if (strcmp(netdev_get_type(>up), "internal") && ret) {
 return ret;
 }
 ofpbuf_delete(buf);
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] dpif-netdev: Change definitions of 'idle' & 'processing' cycles

2017-02-17 Thread Ciara Loftus 
Instead of counting all polling cycles as processing cycles, only count
the cycles where packets were received from the polling.

Signed-off-by: Georg Schmuecking 
Signed-off-by: Ciara Loftus 
Co-authored-by: Georg Schmuecking 
Acked-by: Kevin Traynor 
---
v3:
- Updated acks & co-author tags
- Removed unnecessary PMD_CYCLES_POLLING counter type
- Explain terms 'idle' and 'processing' cycles in
  vswitchd/ovs-vswitchd.8.in
v2:
- Rebase

 lib/dpif-netdev.c  | 57 ++
 vswitchd/ovs-vswitchd.8.in |  5 +++-
 2 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 30907b7..6bf6809 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -279,8 +279,9 @@ enum dp_stat_type {
 };
 
 enum pmd_cycles_counter_type {
-PMD_CYCLES_POLLING, /* Cycles spent polling NICs. */
-PMD_CYCLES_PROCESSING,  /* Cycles spent processing packets */
+PMD_CYCLES_IDLE,/* Cycles spent idle or unsuccessful polling */
+PMD_CYCLES_PROCESSING,  /* Cycles spent successfully polling and
+ * processing polled packets */
 PMD_N_CYCLES
 };
 
@@ -755,10 +756,10 @@ pmd_info_show_stats(struct ds *reply,
 }
 
 ds_put_format(reply,
-  "\tpolling cycles:%"PRIu64" (%.02f%%)\n"
+  "\tidle cycles:%"PRIu64" (%.02f%%)\n"
   "\tprocessing cycles:%"PRIu64" (%.02f%%)\n",
-  cycles[PMD_CYCLES_POLLING],
-  cycles[PMD_CYCLES_POLLING] / (double)total_cycles * 100,
+  cycles[PMD_CYCLES_IDLE],
+  cycles[PMD_CYCLES_IDLE] / (double)total_cycles * 100,
   cycles[PMD_CYCLES_PROCESSING],
   cycles[PMD_CYCLES_PROCESSING] / (double)total_cycles * 100);
 
@@ -2953,30 +2954,43 @@ cycles_count_end(struct dp_netdev_pmd_thread *pmd,
 non_atomic_ullong_add(>cycles.n[type], interval);
 }
 
-static void
+/* Calculate the intermediate cycle result and add to the counter 'type' */
+static inline void
+cycles_count_intermediate(struct dp_netdev_pmd_thread *pmd,
+  enum pmd_cycles_counter_type type)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+unsigned long long new_cycles = cycles_counter();
+unsigned long long interval = new_cycles - pmd->last_cycles;
+pmd->last_cycles = new_cycles;
+
+non_atomic_ullong_add(>cycles.n[type], interval);
+}
+
+static int
 dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
struct netdev_rxq *rx,
odp_port_t port_no)
 {
 struct dp_packet_batch batch;
 int error;
+int batch_cnt = 0;
 
 dp_packet_batch_init();
-cycles_count_start(pmd);
 error = netdev_rxq_recv(rx, );
-cycles_count_end(pmd, PMD_CYCLES_POLLING);
 if (!error) {
 *recirc_depth_get() = 0;
 
-cycles_count_start(pmd);
+batch_cnt = batch.count;
 dp_netdev_input(pmd, , port_no);
-cycles_count_end(pmd, PMD_CYCLES_PROCESSING);
 } else if (error != EAGAIN && error != EOPNOTSUPP) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
 VLOG_ERR_RL(, "error receiving data from %s: %s",
 netdev_rxq_get_name(rx), ovs_strerror(error));
 }
+
+return batch_cnt;
 }
 
 static struct tx_port *
@@ -3438,21 +3452,27 @@ dpif_netdev_run(struct dpif *dpif)
 struct dp_netdev *dp = get_dp_netdev(dpif);
 struct dp_netdev_pmd_thread *non_pmd;
 uint64_t new_tnl_seq;
+int process_packets = 0;
 
 ovs_mutex_lock(>port_mutex);
 non_pmd = dp_netdev_get_pmd(dp, NON_PMD_CORE_ID);
 if (non_pmd) {
 ovs_mutex_lock(>non_pmd_mutex);
+cycles_count_start(non_pmd);
 HMAP_FOR_EACH (port, node, >ports) {
 if (!netdev_is_pmd(port->netdev)) {
 int i;
 
 for (i = 0; i < port->n_rxq; i++) {
-dp_netdev_process_rxq_port(non_pmd, port->rxqs[i].rx,
-   port->port_no);
+process_packets +=
+dp_netdev_process_rxq_port(non_pmd,
+   port->rxqs[i].rx,
+   port->port_no);
 }
 }
 }
+cycles_count_end(non_pmd, process_packets ? PMD_CYCLES_PROCESSING
+  : PMD_CYCLES_IDLE);
 dpif_netdev_xps_revalidate_pmd(non_pmd, time_msec(), false);
 ovs_mutex_unlock(>non_pmd_mutex);
 
@@ -3577,6 +3597,7 @@ pmd_thread_main(void *f_)
 bool exiting;
 int poll_cnt;
 int i;
+int process_packets = 0;
 
 poll_list = NULL;
 
@@ -3603,10 +3624,12 @@ reload:
 lc = UINT_MAX;
 }
 
+

Re: [ovs-dev] [PATCH] netdev-dpdk: Add support for DPDK 17.02

2017-02-17 Thread Loftus, Ciara 
> 
> Ciara Loftus  writes:
> 
> > This commit announces support for DPDK 17.02. Compatibility with DPDK
> > v16.11 is not broken yet thanks to no code changes being needed for the
> > upgrade.
> >
> > Signed-off-by: Ciara Loftus 
> > ---
> 
> Is it possible to make this reflect an either 16.11 or 17.02 support
> instead of a hard requirement?  17.02 is not a long-term supported
> version, but IIRC 16.11 is.
> 
> What do you think?

Good point. Would like to hear more opinions. This could be a good topic for 
the bi-weekly call next Thursday.

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

Re: [ovs-dev] [PATCH ovs V3 06/25] dpif: Save added ports in a port map for netdev flow api use

2017-02-17 Thread Simon Horman 
On Wed, Feb 15, 2017 at 10:46:20AM +0200, Roi Dayan wrote:
> 
> 
> On 14/02/2017 17:52, Simon Horman wrote:
> >On Wed, Feb 08, 2017 at 05:29:19PM +0200, Roi Dayan wrote:
> >>From: Paul Blakey 
> >>
> >>To use netdev flow offloading api, dpifs needs to iterate over
> >>added ports. This addition inserts the added dpif ports in a hash map,
> >>The map will also be used to translate dpif ports to netdevs.
> >>
> >>Signed-off-by: Paul Blakey 
> >>Reviewed-by: Roi Dayan 
> >>---
> >> lib/dpif.c   |  25 
> >> lib/dpif.h   |   2 +
> >> lib/netdev.c | 121 
> >> +++
> >> lib/netdev.h |  15 
> >> 4 files changed, 163 insertions(+)
> >>
> >>diff --git a/lib/dpif.c b/lib/dpif.c
> >>index 57aa3c6..d4e4c0a 100644
> >>--- a/lib/dpif.c
> >>+++ b/lib/dpif.c
> >
> >...
> >
> >>@@ -545,6 +560,14 @@ dpif_port_add(struct dpif *dpif, struct netdev 
> >>*netdev, odp_port_t *port_nop)
> >> if (!error) {
> >> VLOG_DBG_RL(_rl, "%s: added %s as port %"PRIu32,
> >> dpif_name(dpif), netdev_name, port_no);
> >>+
> >>+/* temp dpif_port, will be cloned in netdev_hmap_port_add */
> >>+struct dpif_port dpif_port;
> >>+
> >>+dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev));
> >>+dpif_port.name = CONST_CAST(char *, netdev_name);
> >>+dpif_port.port_no = port_no;
> >>+netdev_hmap_port_add(netdev, DPIF_HMAP_KEY(dpif), _port);
> >
> >I wonder if it would be cleaner to get the dpif_port from the dpif API,
> >for example by providing a optionally NULL struct dpif_port * parameter to
> >dpif_class->port_add.
> >
> 
> I'm not sure I'm following you here. how would this be done?
> we want to same dpif_port and netdev resolution so we can use it later.
> right that we use it in dpif_class it self but other classes would want
> maybe to use this in the future so it's being handles in dpif_port_add.

I was just suggesting a minor cleanup as it seems a bit untidy to have
struct dpif_port data marshalled inline. My assumption was that
dpif->dpif_class->port_add(), which is called just above the code in
question, was doing the same marshalling and it could be leveraged.  I no
longer think that is the case.

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

Re: [ovs-dev] [PATCH ovs V3 05/25] other-config: Add tc-policy switch to control tc flower flag

2017-02-17 Thread Simon Horman 
On Wed, Feb 15, 2017 at 01:48:37PM +0200, Roi Dayan wrote:
> On 15/02/2017 13:16, Chandran, Sugesh wrote:
> >>From: Roi Dayan [mailto:r...@mellanox.com]
...
> >>On 14/02/2017 01:53, Chandran, Sugesh wrote:
> From: Roi Dayan [mailto:r...@mellanox.com]
...
> +tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
> +   TC_POLICY_DEFAULT));
> +
> >>>[Sugesh] I feel we better call this policy as hw-ofld-policy than 
> >>>tc-policy. As
> >>far as I know tc is one of the way to program the hardware. In the dpdk dpif
> >>there must be be another flow programming interface that can be used to
> >>program the offload. It would be nice if we can use the same configuration
> >>option  for all the deployments.
> >>
> >>currently the options used in tc-policy are somewhat tc specific.
> >>the options could be a lot different for other interfaces.
> >>maybe leave this like this for now?
> >>if another interface will have a policy then we can revisit this if the same
> >>options apply or if we can merge those options or maybe we'll see we'll need
> >>an entirely different [interface]-policy option?
> >[Sugesh] Ok, I am fine with leaving it .  But out of curiosity, just 
> >wondering what would be other possible policy options you are expecting for 
> >other type of interfaces??  I assume the policy is more or less the way 
> >hardware get programmed. It has nothing to do with what approach used to 
> >program it, which can be tc or any other APIs.
> >
> >
> 
> I'm not sure. for example tc use negative approach (skip_sw/skip_hw) and if
> you leave it empty it's "both".
> Other interface could use a positive approach like offload_hw, offload_sw,
> offload_both..
> Though it very well could be it would just be hw-offload-policy later..

I think another consideration when naming options is that policy could be
implemented in different places.

In the case of the option above it affects policy implemented by the kernel
regarding where it places a flow in the context of TC with the flower
classifier: not in sw, not in hw, or an implementation-defined default.

The last option is, as far as I understand:
* Add to sw and;
* Add to hw if an offload callback is provided by the driver

But there is also a policy present in OvS, currently in the DPIF code in
this patchset, which is as follows:

* Try to add flow using offload callback
* If that fails - e.g. due to the callback not being present then
  - Try to add flow to OvS datapath

In a sense the above is already configurable by the "hw-offload" option
provided by this patchset which allows skipping trying to add the flow
using the offload callback.

However, there are other possibilities for the policy implemented in the
DPIF code. In particular I would like to see the following supported:

* Try to add flow using offload callback
* Regardless of if that fails or not
  - Try to add flow to OvS datapath

We can debate the merit of the last policy above but my point is not that
specific policy but rather that policy exists in two places - the kernel
and the DPIF - and that both could plausibly be called "hw-ofld-policy".

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

Re: [ovs-dev] [PATCH ovs V3 05/25] other-config: Add tc-policy switch to control tc flower flag

2017-02-17 Thread Simon Horman 
On Wed, Feb 08, 2017 at 05:29:18PM +0200, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Add a new configuration tc-policy option that controls tc
> flower flag. Possible options are none, skip_sw, skip_hw.
> The default is none which is to insert the rule both to sw and hw.
> This option is only relevant if hw-offload is enabled.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> ---
>  lib/netdev.c |  6 ++
>  lib/tc.c | 43 ++-
>  lib/tc.h |  1 +
>  vswitchd/vswitch.xml | 17 +
>  4 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/netdev.c b/lib/netdev.c
> index ca727a5..d5935c3 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -54,6 +54,9 @@
>  #include "openvswitch/vlog.h"
>  #include "flow.h"
>  #include "util.h"
> +#ifdef __linux__
> +#include "tc.h"
> +#endif
>  
>  VLOG_DEFINE_THIS_MODULE(netdev);
>  
> @@ -2115,6 +2118,9 @@ netdev_set_flow_api_enabled(const struct smap 
> *ovs_other_config)
>  
>  VLOG_INFO("netdev: Flow API Enabled");
>  
> +tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
> +   TC_POLICY_DEFAULT));
> +
>  ovsthread_once_done();
>  }
>  }
> diff --git a/lib/tc.c b/lib/tc.c
> index 431242b..b2e3d21 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -38,6 +38,14 @@ VLOG_DEFINE_THIS_MODULE(tc);
>  
>  static struct vlog_rate_limit parse_err = VLOG_RATE_LIMIT_INIT(5, 5);
>  
> +enum tc_offload_policy {
> +TC_POLICY_NONE,
> +TC_POLICY_SKIP_SW,
> +TC_POLICY_SKIP_HW
> +};
> +
> +static enum tc_offload_policy tc_policy = TC_POLICY_NONE;
> +
>  /* Returns tc handle 'major':'minor'. */
>  unsigned int
>  tc_make_handle(unsigned int major, unsigned int minor)
> @@ -719,6 +727,18 @@ tc_get_flower(int ifindex, int prio, int handle, struct 
> tc_flower *flower)
>  return error;
>  }
>  
> +static int
> +tc_get_tc_cls_policy(enum tc_offload_policy policy)
> +{
> +if (policy == TC_POLICY_SKIP_HW) {
> +return TCA_CLS_FLAGS_SKIP_HW;
> +} else if (policy == TC_POLICY_SKIP_SW) {
> +return TCA_CLS_FLAGS_SKIP_SW;
> +}
> +
> +return 0;
> +}
> +
>  static void
>  nl_msg_put_act_push_vlan(struct ofpbuf *request, uint16_t vid, uint8_t prio)
>  {
> @@ -989,7 +1009,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
> tc_flower *flower)
>  }
>  }
>  
> -nl_msg_put_u32(request, TCA_FLOWER_FLAGS, 0);
> +nl_msg_put_u32(request, TCA_FLOWER_FLAGS, 
> tc_get_tc_cls_policy(tc_policy));
>  
>  if (flower->tunnel.tunnel) {
>  nl_msg_put_flower_tunnel(request, flower);
> @@ -1033,3 +1053,24 @@ tc_replace_flower(int ifindex, uint16_t prio, uint32_t 
> handle,
>  
>  return error;
>  }
> +
> +void
> +tc_set_policy(const char *policy)
> +{
> +if (!policy) {
> +return;
> +}
> +
> +if (!strcmp(policy, "skip_sw")) {
> +tc_policy = TC_POLICY_SKIP_SW;
> +} else if (!strcmp(policy, "skip_hw")) {
> +tc_policy = TC_POLICY_SKIP_HW;
> +} else if (!strcmp(policy, "none")) {
> +tc_policy = TC_POLICY_NONE;
> +} else {
> +VLOG_WARN("tc: Invalid policy '%s'", policy);
> +return;
> +}
> +
> +VLOG_INFO("tc: Using policy '%s'", policy);
> +}
> diff --git a/lib/tc.h b/lib/tc.h
> index 5ca6c55..6f6cc09 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -115,5 +115,6 @@ int tc_flush(int ifindex);
>  int tc_dump_flower_start(int ifindex, struct nl_dump *dump);
>  int parse_netlink_to_tc_flower(struct ofpbuf *reply,
> struct tc_flower *flower);
> +void tc_set_policy(const char *policy);
>  
>  #endif /* tc.h */
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 942e68f..8b9bdcb 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -183,6 +183,23 @@
>  
>
>  
> +   +  type='{"type": "string"}'>
> +
> +Specified the policy used with HW offloading.
> +Options:
> +none- Add software rule and offload rule to 
> HW.
> +skip_sw - Offload rule to HW only.
> +skip_hw - Add software rule without offloading 
> rule to HW.
> +
> +
> +This is only relevant if HW offloading is enabled (hw-offload).
> +
> +
> +  The default value is false.

netdev_set_flow_api_enabled() seems to implement a default value of
none.



> +
> +  
> +
>type='{"type": "boolean"}'>
>  
> -- 
> 2.7.4
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev