Re: [ovs-dev] ovs + dpdk vhost-user match flows but cannot execute actions

[Mauricio Vásquez]

Hello lifuqiong,

I faced the same problem some days ago (
http://openvswitch.org/pipermail/dev/2016-March/068282.html), the bug is
already fixed.
Where are you downloading OVS from?, it appears that the bug is still
present in the version on
http://openvswitch.org/releases/openvswitch-2.5.0.tar.gz, please download
ovs from git and switch to branch-2.5.

Mauricio Vasquez,

On Thu, Apr 14, 2016 at 4:28 AM, lifuqiong  wrote:

> I want to test dpdk vhost-user port on ovs to follow
> https://software.intel.com/en-us/blogs/2015/06/09/building-vhost-user-for-ovs-today-using-dpdk-200
> ;
> I create ovs+dpdk environment followed INSTALL.DPDK.md; and create 2 VM2,
> try to ping each other but show me “Destination Host Unreachable”;
> Dump-flows shows packets matched the flow, but can’t output to port 4, why
> ? I can’t get any useful error or warning info from ovs-vswitchd.log.
> While ping from vm1 to vm2, statistics on vm1 shows that eth1 RX_packet
> keeps zero, TX_PACKET keeps increasing.
> 1.
> OVS: 2.5.0
> Dpdk: 2.2.0
> Qemu: 2.2.1
>
> 2. ovs-ofctl dump-flows ovsbr0
> NXST_FLOW reply (xid=0x4):
> cookie=0x0, duration=836.946s, table=0, n_packets=628, n_bytes=26376,
> idle_age=0, in_port=3 actions=output:4
> cookie=0x0, duration=831.458s, table=0, n_packets=36, n_bytes=1512,
> idle_age=770, in_port=4 actions=output:3
>
> 3. root@host152:/usr/local/var/run/openvswitch# ovs-vsctl show
> 03ae6f7d-3b71-45e3-beb0-09fa11292eaa
> Bridge "ovsbr0"
> Port "vhost-user-1"
> Interface "vhost-user-1"
> type: dpdkvhostuser
> Port "ovsbr0"
> Interface "ovsbr0"
> type: internal
> Port "dpdk1"
> Interface "dpdk1"
> type: dpdk
> Port "vhost-user-0"
> Interface "vhost-user-0"
> type: dpdkvhostuser
> Port "dpdk0"
> Interface "dpdk0"
> type: dpdk
>
> 4. Start VM info:
> qemu-system-x86_64 -m 1024 -smp 2 -hda /root/vm11.qcow2 -boot c
> -enable-kvm -vnc 0.0.0.0:1 -chardev
> socket,id=char1,path=/usr/local/var/run/openvswitch/vhost-user-0 -netdev
> type=vhost-user,id=mynet1,chardev=char1,vhostforce -device
> virtio-net-pci,mac=00:00:00:00:01:12,netdev=mynet1 -object
> memory-backend-file,id=mem,size=1024M,mem-path=/dev/hugepages,share=on
> -numa node,memdev=mem -mem-prealloc -d exec
>
> qemu-system-x86_64: -netdev
> type=vhost-user,id=mynet1,chardev=char1,vhostforce: chardev "char1" went up
>
>
>
> 5. My build command as follows:
> #!/bin/bash
>
>  config and compile dpdk   # cd dpdk #
> make config T=x86_64-native-linuxapp-gcc # make install
> T=x86_64-native-linuxapp-gcc
> 
>
>  config and compile ovs  # cd ovs #
> ./boot.sh # ./configure --localstatedir=/var
> --with-dpdk=/root/workplane/dpdk/x86_64-native-linuxapp-gcc
> # make
> # make install
> 
>
>  config and compile qemu  # cd qemu #
> ./configure # make # make install
> 
>
> ## set hugepage number, use boot cmdline or procfs echo 8 >
> /proc/sys/vm/nr_hugepages
>
> ## insert the kernel modules
> modprobe uio
> insmod $DPDK_BUILD/kmod/igb_uio.ko
> insmod $DPDK_BUILD/kmod/rte_kni.ko
> insmod $DPDK_DIR/lib/librte_vhost/eventfd_link/eventfd_link.ko
>
> # unbind the dpdk interface
>
> $DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio 01:00.0
> $DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio 01:00.1
>
> #Mount hugetable
> mkdir -p /dev/hugepages
> mount -t hugetlbfs -o pagesize=1G none /dev/hugepages
>
> #first time
> #ovsdb-tool create /usr/local/etc/openvswitch/conf.db
> /usr/local/share/openvswitch/vswitch.ovsschema
>
> ovsdb-server --remote=punix:/usr/local/var/run/openvswitch/db.sock
> --remote=db:Open_vSwitch,Open_vSwitch,manager_options --pidfile --detach
> --log-file
>
> #firsttime only
> #ovs-vsctl --no-wait init
> ovs-vswitchd --dpdk -c 0x77 -n 2 --socket-mem 2048,0 -- unix:$DB_SOCK
> --pidfile --detach --log-file
>
>
> ##
> ## Add brige
> /usr/local/bin/ovs-vsctl add-br ovsbr0 -- set bridge ovsbr0
> datapath_type=netdev
>
> ## Add dpdk port
> /usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk0 -- set Interface dpdk0
> type=dpdk /usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk1 -- set Interface
> dpdk1 type=dpdk # /usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk2 -- set
> Interface dpdk2 type=dpdk # /usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk3
> -- set Interface dpdk3 type=dpdk
>
> ## Add vhost-user port
> /usr/local/bin/ovs-vsctl add-port ovsbr0 vhost-user-0 -- set Interface
> vhost-user-0 type=dpdkvhostuser /usr/local/bin/ovs-vsctl add-port ovsbr0
> vhost-user-1 -- set Interface vhost-user-1 type=dpdkvhostuser
>
>
>
> __

Re: [ovs-dev] [PATCH v14 2/6] Change encaps_run to work incrementally

[Kei Nohguchi]

Hi Ryan,

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

Re: [ovs-dev] [PATCH v2] ovn: Add software l2 gateway.

[Ben Pfaff]

On Mon, Apr 04, 2016 at 05:58:28AM -0700, Russell Bryant wrote:
> This patch implements one approach to using ovn-controller to implement
> a software l2 gateway between logical and physical networks.
> 
> A new logical port type called "gateway" is introduced here.  It is very
> close to how localnet ports work, with the following exception:
> 
> - A localnet port makes OVN use the physical network as the
>   transport between hypervisors instead of tunnels. A gateway port still
>   uses tunnels between all hypervisors, and packets only go to/from the
>   specified physical network as needed via the chassis the gateway port
>   is bound to.
> 
> - A gateway port also gets bound to a chassis while a localnet port does
>   not.  This binding is not done by ovn-controller.  It is left as an
>   administrative function.  In the case of OpenStack, the Neutron plugin
>   will do this.
> 
> Signed-off-by: Russell Bryant 

This seems reasonable to me.  Thanks for working on it.

There's a stray " * " in the middle of the second line here:
> + * ovn-controller does not bind gateway ports itself.
> + * Choosing a chassis * for a gateway port is left
> + * up to an entity external to OVN. */
> +sset_add(&all_lports, binding_rec->logical_port);
> +add_local_datapath(local_datapaths, binding_rec);
>  } else if (binding_rec->chassis == chassis_rec) {
>  if (ctx->ovnsb_idl_txn) {
>  VLOG_INFO("Releasing lport %s from this chassis.",

The treatment of the "chassis" column in the Port_Binding database is
quite different based on the type of the port:

* For localnet ports, it's always empty.

* For gateway ports, ovn-northd (or whoever adds the port, I
  guess) must set the chassis correctly.

* For other ports, ovn-controller sets (and un-sets) the
  chassis.

I don't think these differences can be gleaned from the documentation.
I think that it should be made clear in ovn-sb.xml.

I guess that you may have comments from others that you should address,
but for me:
Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 1/2] ovn-controller: Warn if system-id is missing.

[Ben Pfaff]

On Thu, Apr 07, 2016 at 11:39:32AM -0400, Russell Bryant wrote:
> If 'system-id' is missing from the Open_vSwitch database, ovn-controller
> will not work.  Log a warning if that happens to make it clear that
> configuration is incomplete.
> 
> Signed-off-by: Russell Bryant 
> ---
>  ovn/controller/ovn-controller.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> v3:
>  - This patch is new in v3, as suggested by Han Zhou.

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

[ovs-dev] [PATCH 2/2] ofproto-dpif: Do not count resubmit to later tables against limit.

[Ben Pfaff]

Open vSwitch must ensure that flow translation takes a finite amount of
time.  Until now it has implemented this by limiting the depth of
recursion.  The initial limit, in version 1.0.1, was no recursion at all,
and then over the years it has increased to 8 levels, then 16, then 32,
and 64 for the last few years.  Now reports are coming in that 64 levels
are inadequate for some OVN setups.  The natural inclination would be to
double the limit again to 128 levels.

This commit attempts another approach.  Instead of increasing the limit,
it reduces the class of resubmits that count against the limit.  Since the
goal for the depth limit is to prevent an infinite amount of work, it's
not necessary to count resubmits that can't lead to infinite work.  In
particular, a resubmit from a table numbered x to a table y > x cannot do
this, because any OpenFlow switch has a finite number of tables.  Because
in fact a resubmit (or goto_table) from one table to a later table is the
most common form of an OpenFlow pipeline, I suspect that this will greatly
alleviate the pressure to increase the depth limit.

Reported-by: Guru Shetty 
Signed-off-by: Ben Pfaff 
---
 lib/ofp-actions.c| 19 +++--
 ofproto/ofproto-dpif-xlate.c | 64 +++-
 ofproto/ofproto-dpif-xlate.h | 23 +---
 ofproto/ofproto-dpif.c   |  5 ++--
 ofproto/ofproto-dpif.h   |  3 ++-
 tests/ofproto-dpif.at| 41 +++-
 utilities/ovs-ofctl.8.in | 28 ---
 7 files changed, 151 insertions(+), 32 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 106ebc8..323bee1 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -3738,8 +3738,23 @@ format_FIN_TIMEOUT(const struct ofpact_fin_timeout *a, 
struct ds *s)
  *
  * Resubmit actions may be used any number of times within a set of actions.
  *
- * Resubmit actions may nest to an implementation-defined depth.  Beyond this
- * implementation-defined depth, further resubmit actions are simply ignored.
+ * Resubmit actions may nest.  To prevent infinite loops and excessive resource
+ * use, the implementation may limit nesting depth and the total number of
+ * resubmits:
+ *
+ *- Open vSwitch 1.0.1 and earlier did not support recursion.
+ *
+ *- Open vSwitch 1.0.2 and 1.0.3 limited recursion to 8 levels.
+ *
+ *- Open vSwitch 1.1 and 1.2 limited recursion to 16 levels.
+ *
+ *- Open vSwitch 1.2 through 1.8 limited recursion to 32 levels.
+ *
+ *- Open vSwitch 1.9 through 2.0 limited recursion to 64 levels.
+ *
+ *- Open vSwitch 2.1 through 2.5 limited recursion to 64 levels and impose
+ *  a total limit of 4,096 resubmits per flow translation (earlier versions
+ *  did not impose any total limit).
  *
  * NXAST_RESUBMIT ignores 'table' and 'pad'.  NXAST_RESUBMIT_TABLE requires
  * 'pad' to be all-bits-zero.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8cf6ea2..47556ed 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -67,14 +67,22 @@ COVERAGE_DEFINE(xlate_actions_too_many_output);
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate);
 
 /* Maximum depth of flow table recursion (due to resubmit actions) in a
- * flow translation. */
-#define MAX_RESUBMIT_RECURSION 64
-#define MAX_INTERNAL_RESUBMITS 1   /* Max resbmits allowed using rules in
-  internal table. */
+ * flow translation.
+ *
+ * The goal of limiting the depth of resubmits is to ensure that flow
+ * translation eventually terminates.  Only resubmits to the same table or an
+ * earlier table count against the maximum depth.  This is because resubmits to
+ * strictly monotonically increasing table IDs will eventually terminate, since
+ * any OpenFlow switch has a finite number of tables.  OpenFlow tables are most
+ * commonly traversed in numerically increasing order, so this limit has little
+ * effect on conventionally designed OpenFlow pipelines.
+ *
+ * Outputs to patch ports and to groups also count against the depth limit. */
+#define MAX_DEPTH 64
 
 /* Maximum number of resubmit actions in a flow translation, whether they are
  * recursive or not. */
-#define MAX_RESUBMITS (MAX_RESUBMIT_RECURSION * MAX_RESUBMIT_RECURSION)
+#define MAX_RESUBMITS (MAX_DEPTH * MAX_DEPTH)
 
 struct xbridge {
 struct hmap_node hmap_node;   /* Node in global 'xbridges' map. */
@@ -195,8 +203,31 @@ struct xlate_ctx {
  * wants actions. */
 struct ofpbuf *odp_actions;
 
-/* Resubmit statistics, via xlate_table_action(). */
-int indentation;/* Current resubmit nesting depth. */
+/* Statistics maintained by xlate_table_action().
+ *
+ * 'indentation' is the nesting level for resubmits.  It is used to indent
+ * the output of resubmit_hook (e.g. for the "ofproto/trace" feature).
+ *
+ * The other statistics limit the amount of work that a single flow
+ * transl

[ovs-dev] [PATCH 1/2] ofproto-dpif: Rename "recurse" to "indentation".

[Ben Pfaff]

The "recurse" member of struct xlate_in and struct xlate_ctx is used for
two purposes: to determine the amount of indentation in "ofproto/trace"
output and to limit the depth of recursion.  An upcoming commit will
separate these tasks, and so in preparation this commit renames "recurse"
to "indentation".

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-xlate.c | 24 
 ofproto/ofproto-dpif-xlate.h | 15 ---
 ofproto/ofproto-dpif.c   | 43 ++-
 ofproto/ofproto-dpif.h   |  2 +-
 4 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a02dc24..8cf6ea2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -196,7 +196,7 @@ struct xlate_ctx {
 struct ofpbuf *odp_actions;
 
 /* Resubmit statistics, via xlate_table_action(). */
-int recurse;/* Current resubmit nesting depth. */
+int indentation;/* Current resubmit nesting depth. */
 int resubmits;  /* Total number of resubmits. */
 bool in_group;  /* Currently translating ofgroup, if true. */
 bool in_action_set; /* Currently translating action_set, if true. 
*/
@@ -583,7 +583,7 @@ xlate_report(struct xlate_ctx *ctx, const char *format, ...)
 va_list args;
 
 va_start(args, format);
-ctx->xin->report_hook(ctx->xin, ctx->recurse, format, args);
+ctx->xin->report_hook(ctx->xin, ctx->indentation, format, args);
 va_end(args);
 }
 }
@@ -2805,7 +2805,7 @@ compose_table_xlate(struct xlate_ctx *ctx, const struct 
xport *out_dev,
 
 return ofproto_dpif_execute_actions__(xbridge->ofproto, &flow, NULL,
   &output.ofpact, sizeof output,
-  ctx->recurse, ctx->resubmits, 
packet);
+  ctx->indentation, ctx->resubmits, 
packet);
 }
 
 static void
@@ -3222,20 +3222,20 @@ xlate_recursively(struct xlate_ctx *ctx, struct 
rule_dpif *rule)
 }
 
 ctx->resubmits++;
-ctx->recurse++;
+ctx->indentation++;
 ctx->rule = rule;
 ctx->rule_cookie = rule_dpif_get_flow_cookie(rule);
 actions = rule_dpif_get_actions(rule);
 do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);
 ctx->rule_cookie = old_cookie;
 ctx->rule = old_rule;
-ctx->recurse--;
+ctx->indentation--;
 }
 
 static bool
 xlate_resubmit_resource_check(struct xlate_ctx *ctx)
 {
-if (ctx->recurse >= MAX_RESUBMIT_RECURSION + MAX_INTERNAL_RESUBMITS) {
+if (ctx->indentation >= MAX_RESUBMIT_RECURSION + MAX_INTERNAL_RESUBMITS) {
 XLATE_REPORT_ERROR(ctx, "resubmit actions recursed over %d times",
MAX_RESUBMIT_RECURSION);
 ctx->error = XLATE_RECURSION_TOO_DEEP;
@@ -3274,7 +3274,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t 
in_port, uint8_t table_id,
may_packet_in, honor_table_miss);
 
 if (OVS_UNLIKELY(ctx->xin->resubmit_hook)) {
-ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse + 1);
+ctx->xin->resubmit_hook(ctx->xin, rule, ctx->indentation + 1);
 }
 
 if (rule) {
@@ -3289,7 +3289,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t 
in_port, uint8_t table_id,
 entry->u.rule = rule;
 rule_dpif_ref(rule);
 }
-xlate_recursively(ctx, rule);
+xlate_recursively(ctx, rule, table_id <= old_table_id);
 }
 
 ctx->table_id = old_table_id;
@@ -3323,9 +3323,9 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct 
ofputil_bucket *bucket)
 struct flow old_flow = ctx->xin->flow;
 
 ofpacts_execute_action_set(&action_list, &action_set);
-ctx->recurse++;
+ctx->indentation++;
 do_xlate_actions(action_list.data, action_list.size, ctx);
-ctx->recurse--;
+ctx->indentation--;
 
 ofpbuf_uninit(&action_list);
 
@@ -4824,7 +4824,7 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif 
*ofproto,
 xin->resubmit_hook = NULL;
 xin->report_hook = NULL;
 xin->resubmit_stats = NULL;
-xin->recurse = 0;
+xin->indentation = 0;
 xin->resubmits = 0;
 xin->wc = wc;
 xin->odp_actions = odp_actions;
@@ -5089,7 +5089,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out 
*xout)
 .wc = xin->wc ? xin->wc : &scratch_wc,
 .odp_actions = xin->odp_actions ? xin->odp_actions : &scratch_actions,
 
-.recurse = xin->recurse,
+.indentation = xin->indentation,
 .resubmits = xin->resubmits,
 .in_group = false,
 .in_action_set = false,
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index cf9932c..4453072 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -83,18 +83,19 @@ struct xla

[ovs-dev] ovs + dpdk vhost-user match flows but cannot execute actions

[lifuqiong]

I want to test dpdk vhost-user port on ovs to follow 
https://software.intel.com/en-us/blogs/2015/06/09/building-vhost-user-for-ovs-today-using-dpdk-200;
I create ovs+dpdk environment followed INSTALL.DPDK.md; and create 2 VM2, try 
to ping each other but show me “Destination Host Unreachable”;
Dump-flows shows packets matched the flow, but can’t output to port 4, why ? I 
can’t get any useful error or warning info from ovs-vswitchd.log.
While ping from vm1 to vm2, statistics on vm1 shows that eth1 RX_packet keeps 
zero, TX_PACKET keeps increasing.
1.
OVS: 2.5.0 
Dpdk: 2.2.0
Qemu: 2.2.1

2. ovs-ofctl dump-flows ovsbr0
NXST_FLOW reply (xid=0x4):
cookie=0x0, duration=836.946s, table=0, n_packets=628, n_bytes=26376, 
idle_age=0, in_port=3 actions=output:4
cookie=0x0, duration=831.458s, table=0, n_packets=36, n_bytes=1512, 
idle_age=770, in_port=4 actions=output:3

3. root@host152:/usr/local/var/run/openvswitch# ovs-vsctl show
03ae6f7d-3b71-45e3-beb0-09fa11292eaa
Bridge "ovsbr0"
Port "vhost-user-1"
Interface "vhost-user-1"
type: dpdkvhostuser
Port "ovsbr0"
Interface "ovsbr0"
type: internal
Port "dpdk1"
Interface "dpdk1"
type: dpdk
Port "vhost-user-0"
Interface "vhost-user-0"
type: dpdkvhostuser
Port "dpdk0"
Interface "dpdk0"
type: dpdk

4. Start VM info:
qemu-system-x86_64 -m 1024 -smp 2 -hda /root/vm11.qcow2 -boot c -enable-kvm 
-vnc 0.0.0.0:1 -chardev 
socket,id=char1,path=/usr/local/var/run/openvswitch/vhost-user-0 -netdev 
type=vhost-user,id=mynet1,chardev=char1,vhostforce -device 
virtio-net-pci,mac=00:00:00:00:01:12,netdev=mynet1 -object 
memory-backend-file,id=mem,size=1024M,mem-path=/dev/hugepages,share=on -numa 
node,memdev=mem -mem-prealloc -d exec

qemu-system-x86_64: -netdev type=vhost-user,id=mynet1,chardev=char1,vhostforce: 
chardev "char1" went up



5. My build command as follows:
#!/bin/bash

 config and compile dpdk   # cd dpdk # make 
config T=x86_64-native-linuxapp-gcc # make install T=x86_64-native-linuxapp-gcc 


 config and compile ovs  # cd ovs # ./boot.sh # 
./configure --localstatedir=/var 
--with-dpdk=/root/workplane/dpdk/x86_64-native-linuxapp-gcc
# make
# make install


 config and compile qemu  # cd qemu # 
./configure # make # make install 


## set hugepage number, use boot cmdline or procfs echo 8 > 
/proc/sys/vm/nr_hugepages

## insert the kernel modules
modprobe uio
insmod $DPDK_BUILD/kmod/igb_uio.ko
insmod $DPDK_BUILD/kmod/rte_kni.ko
insmod $DPDK_DIR/lib/librte_vhost/eventfd_link/eventfd_link.ko

# unbind the dpdk interface

$DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio 01:00.0 
$DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio 01:00.1

#Mount hugetable
mkdir -p /dev/hugepages
mount -t hugetlbfs -o pagesize=1G none /dev/hugepages

#first time
#ovsdb-tool create /usr/local/etc/openvswitch/conf.db 
/usr/local/share/openvswitch/vswitch.ovsschema

ovsdb-server --remote=punix:/usr/local/var/run/openvswitch/db.sock 
--remote=db:Open_vSwitch,Open_vSwitch,manager_options --pidfile --detach 
--log-file

#firsttime only
#ovs-vsctl --no-wait init
ovs-vswitchd --dpdk -c 0x77 -n 2 --socket-mem 2048,0 -- unix:$DB_SOCK --pidfile 
--detach --log-file

##
## Add brige
/usr/local/bin/ovs-vsctl add-br ovsbr0 -- set bridge ovsbr0 datapath_type=netdev

## Add dpdk port
/usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk0 -- set Interface dpdk0 type=dpdk 
/usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk1 -- set Interface dpdk1 type=dpdk 
# /usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk2 -- set Interface dpdk2 
type=dpdk # /usr/local/bin/ovs-vsctl add-port ovsbr0 dpdk3 -- set Interface 
dpdk3 type=dpdk

## Add vhost-user port
/usr/local/bin/ovs-vsctl add-port ovsbr0 vhost-user-0 -- set Interface 
vhost-user-0 type=dpdkvhostuser /usr/local/bin/ovs-vsctl add-port ovsbr0 
vhost-user-1 -- set Interface vhost-user-1 type=dpdkvhostuser



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

[ovs-dev] [PATCH 4/4] classifier: Avoid inserting duplicates to cmaps.

[Jarno Rajahalme]

Staged lookup indices assumed that cmap is efficient fealing with
duplicates.  Duplicates are implemented as linked lists, however,
causing removal of rules to become (O^2) and highly cache-inefficient
with large number of duplicates.

This was problematic especially when many rules shared the same match
in packet metadata (e.g., a port number, but nothing else).

This patch fixes the problem by introducing a new 'count' variant of
the cmap (ccmap), which can be efficiently used to keep counts of
inserted hash values provided by the caller.  This does not require a
node in the user data structure, so this makes the classifier
implementation a bit more memory efficient, too.

Reported-by: Alok Kumar Maurya 
Signed-off-by: Jarno Rajahalme 
---
 lib/automake.mk  |   2 +
 lib/ccmap.c  | 574 +++
 lib/ccmap.h  |  64 ++
 lib/classifier-private.h |   6 +-
 lib/classifier.c |  42 +---
 5 files changed, 654 insertions(+), 34 deletions(-)
 create mode 100644 lib/ccmap.c
 create mode 100644 lib/ccmap.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 7b829d0..639a87e 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -38,6 +38,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/classifier.c \
lib/classifier.h \
lib/classifier-private.h \
+   lib/ccmap.c \
+   lib/ccmap.h \
lib/cmap.c \
lib/cmap.h \
lib/colors.c \
diff --git a/lib/ccmap.c b/lib/ccmap.c
new file mode 100644
index 000..83f3bb0
--- /dev/null
+++ b/lib/ccmap.c
@@ -0,0 +1,574 @@
+/*
+ * Copyright (c) 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.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include "ccmap.h"
+#include "coverage.h"
+#include "bitmap.h"
+#include "hash.h"
+#include "ovs-rcu.h"
+#include "random.h"
+#include "util.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(ccmap);
+
+COVERAGE_DEFINE(ccmap_expand);
+COVERAGE_DEFINE(ccmap_shrink);
+
+/* A count-only version of the cmap. */
+
+/* An entry is an 32-bit hash and a 32-bit count. */
+#define CCMAP_ENTRY_SIZE (4 + 4)
+
+/* Number of entries per bucket: 8 */
+#define CCMAP_K (CACHE_LINE_SIZE / CCMAP_ENTRY_SIZE)
+
+/* Pad to make a bucket a full cache line in size: 4 on 32-bit, 0 on 64-bit. */
+#define CCMAP_PADDING ((CACHE_LINE_SIZE - 4) - (CCMAP_K * CCMAP_ENTRY_SIZE))
+
+typedef ATOMIC(uint64_t) ccmap_node_t;
+
+static inline uint64_t ccmap_node(uint32_t count, uint32_t hash)
+{
+return (uint64_t)count << 32 | hash;
+}
+
+static inline uint32_t ccmap_node_hash(uint64_t node)
+{
+return node;
+}
+
+static inline uint32_t ccmap_node_count(uint64_t node)
+{
+return node >> 32;
+}
+
+/* A cuckoo hash bucket.  Designed to be cache-aligned and exactly one cache
+ * line long. */
+struct ccmap_bucket {
+/* Each node incudes both the hash (low 32-bits) and the count (high
+ * 32-bits), allowing readers always getting a consistent pair. */
+ccmap_node_t nodes[CCMAP_K];
+};
+BUILD_ASSERT_DECL(sizeof(struct ccmap_bucket) == CACHE_LINE_SIZE);
+
+/* Default maximum load factor (as a fraction of UINT32_MAX + 1) before
+ * enlarging a ccmap.  Reasonable values lie between about 75% and 93%.  
Smaller
+ * values waste memory; larger values increase the average insertion time. */
+#define CCMAP_MAX_LOAD ((uint32_t) (UINT32_MAX * .85))
+
+/* Default minimum load factor (as a fraction of UINT32_MAX + 1) before
+ * shrinking a ccmap.  Currently, the value is chosen to be 20%, this
+ * means ccmap will have a 40% load factor after shrink. */
+#define CCMAP_MIN_LOAD ((uint32_t) (UINT32_MAX * .20))
+
+/* The implementation of a concurrent hash map. */
+struct ccmap_impl {
+unsigned int n; /* Number of in-use elements. */
+unsigned int max_n; /* Max elements before enlarging. */
+unsigned int min_n; /* Min elements before shrinking. */
+uint32_t mask;  /* Number of 'buckets', minus one. */
+uint32_t basis; /* Basis for rehashing client's hash values. */
+
+/* Padding to make ccmap_impl exactly one cache line long. */
+uint8_t pad[CACHE_LINE_SIZE - sizeof(unsigned int) * 5];
+
+struct ccmap_bucket buckets[];
+};
+BUILD_ASSERT_DECL(sizeof(struct ccmap_impl) == CACHE_LINE_SIZE);
+
+static struct ccmap_impl *ccmap_rehash(struct ccmap *, uint32_t mask);
+
+/* Explicit inline keywords in utility functions seem to be necessary
+ * to prevent performance reg

[ovs-dev] [PATCH 1/4] classifier: Remove redundant index.

[Jarno Rajahalme]

The test for figuring out if the last index had the same fields as the
actual rules map as broken, resulting into keeping an unnecessary
index around.

Signed-off-by: Jarno Rajahalme 
---
 lib/classifier.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index 8bc6353..4861672 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1490,11 +1490,11 @@ insert_subtable(struct classifier *cls, const struct 
minimask *mask)
 /* Map for the final stage. */
 *CONST_CAST(struct flowmap *, &subtable->index_maps[index])
 = miniflow_get_map_in_range(&mask->masks, prev, FLOW_U64S);
-/* Check if the final stage adds any bits,
- * and remove the last index if it doesn't. */
+/* Check if the final stage adds any bits. */
 if (index > 0) {
-if (flowmap_equal(subtable->index_maps[index],
-  subtable->index_maps[index - 1])) {
+if (flowmap_is_empty(subtable->index_maps[index])) {
+/* Remove the last index, as it has the same fields as the rules
+ * map. */
 --index;
 cmap_destroy(&subtable->indices[index]);
 }
-- 
2.1.4

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

[ovs-dev] [PATCH 2/4] classifier: Remove logging.

[Jarno Rajahalme]

The only vlog line was a left over from debugging.

Signed-off-by: Jarno Rajahalme 
---
 lib/classifier.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index 4861672..1fbaa0c 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -25,9 +25,6 @@
 #include "ofp-util.h"
 #include "packets.h"
 #include "util.h"
-#include "openvswitch/vlog.h"
-
-VLOG_DEFINE_THIS_MODULE(classifier);
 
 struct trie_ctx;
 
@@ -2252,7 +2249,6 @@ trie_remove_prefix(rcu_trie_ptr *root, const ovs_be32 
*prefix, int mlen)
 }
 /* Cannot go deeper. This should never happen, since only rules
  * that actually exist in the classifier are ever removed. */
-VLOG_WARN("Trying to remove non-existing rule from a prefix trie.");
 }
 
 
-- 
2.1.4

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

[ovs-dev] [PATCH 3/4] classifier: Remove rare optimization case.

[Jarno Rajahalme]

This optimization applied when a staged lookup index would narrow down
to a single rule, which happens sometimes is simple test cases, but
presumably less often in more populated flow tables.  The result of
this optimization allowed a bit more general megaflows, but the bit
patterns produced were sometimes cryptic.  Finally, a later fix to a
more important performance problem does not allow for this
optimization any more, so remove it now.

Signed-off-by: Jarno Rajahalme 
---
 lib/classifier.c  | 75 +--
 tests/classifier.at   | 10 +++
 tests/ofproto-dpif.at | 14 +-
 3 files changed, 13 insertions(+), 86 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index 1fbaa0c..ebd31fb 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1649,54 +1649,6 @@ find_match(const struct cls_subtable *subtable, 
cls_version_t version,
 return NULL;
 }
 
-/* Returns true if 'target' satisifies 'flow'/'mask', that is, if each bit
- * for which 'flow', for which 'mask' has a bit set, specifies a particular
- * value has the correct value in 'target'.
- *
- * This function is equivalent to miniflow_and_mask_matches_flow() but this
- * version fills in the mask bits in 'wc'. */
-static inline bool
-miniflow_and_mask_matches_flow_wc(const struct miniflow *flow,
-  const struct minimask *mask,
-  const struct flow *target,
-  struct flow_wildcards *wc)
-{
-const uint64_t *flowp = miniflow_get_values(flow);
-const uint64_t *maskp = miniflow_get_values(&mask->masks);
-const uint64_t *target_u64 = (const uint64_t *)target;
-uint64_t *wc_u64 = (uint64_t *)&wc->masks;
-uint64_t diff;
-size_t idx;
-map_t map;
-
-FLOWMAP_FOR_EACH_MAP (map, mask->masks.map) {
-MAP_FOR_EACH_INDEX(idx, map) {
-uint64_t msk = *maskp++;
-
-diff = (*flowp++ ^ target_u64[idx]) & msk;
-if (diff) {
-goto out;
-}
-
-/* Fill in the bits that were looked at. */
-wc_u64[idx] |= msk;
-}
-target_u64 += MAP_T_BITS;
-wc_u64 += MAP_T_BITS;
-}
-return true;
-
-out:
-/* Only unwildcard if none of the differing bits is already
- * exact-matched. */
-if (!(wc_u64[idx] & diff)) {
-/* Keep one bit of the difference.  The selected bit may be
- * different in big-endian v.s. little-endian systems. */
-wc_u64[idx] |= rightmost_1bit(diff);
-}
-return false;
-}
-
 static const struct cls_match *
 find_match_wc(const struct cls_subtable *subtable, cls_version_t version,
   const struct flow *flow, struct trie_ctx trie_ctx[CLS_MAX_TRIES],
@@ -1715,8 +1667,6 @@ find_match_wc(const struct cls_subtable *subtable, 
cls_version_t version,
 
 /* Try to finish early by checking fields in segments. */
 for (i = 0; i < subtable->n_indices; i++) {
-const struct cmap_node *inode;
-
 if (check_tries(trie_ctx, n_tries, subtable->trie_plen,
 subtable->index_maps[i], flow, wc)) {
 /* 'wc' bits for the trie field set, now unwildcard the preceding
@@ -1731,32 +1681,9 @@ find_match_wc(const struct cls_subtable *subtable, 
cls_version_t version,
subtable->index_maps[i],
&mask_offset, &basis);
 
-inode = cmap_find(&subtable->indices[i], hash);
-if (!inode) {
+if (!cmap_find(&subtable->indices[i], hash)) {
 goto no_match;
 }
-
-/* If we have narrowed down to a single rule already, check whether
- * that rule matches.  Either way, we're done.
- *
- * (Rare) hash collisions may cause us to miss the opportunity for this
- * optimization. */
-if (!cmap_node_next(inode)) {
-const struct cls_match *head;
-
-ASSIGN_CONTAINER(head, inode - i, index_nodes);
-if (miniflow_and_mask_matches_flow_wc(&head->flow, &subtable->mask,
-  flow, wc)) {
-/* Return highest priority rule that is visible. */
-CLS_MATCH_FOR_EACH (rule, head) {
-if (OVS_LIKELY(cls_match_visible_in_version(rule,
-version))) {
-return rule;
-}
-}
-}
-return NULL;
-}
 }
 /* Trie check for the final range. */
 if (check_tries(trie_ctx, n_tries, subtable->trie_plen,
diff --git a/tests/classifier.at b/tests/classifier.at
index b110508..e56ba3a 100644
--- a/tests/classifier.at
+++ b/tests/classifier.at
@@ -49,7 +49,7 @@ Datapath actions: 1
 ])
 AT_CHECK([ovs-appctl ofproto/trace br0 
'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:0

Re: [ovs-dev] [PATCH v6 06/12] dpif-netdev: Wait an RCU grace period before freeing ports.

[Daniele Di Proietto]

On 10/04/2016 12:23, "Ben Pfaff"  wrote:

>On Fri, Apr 08, 2016 at 03:12:59AM +, Daniele Di Proietto wrote:
>> 
>> 
>> On 01/04/2016 09:52, "Jarno Rajahalme"  wrote:
>> 
>> >
>> >> On Mar 30, 2016, at 8:08 PM, Daniele Di Proietto
>> >> wrote:
>> >> 
>> >> 
>> >> On 30/03/2016 16:01, "Ben Pfaff"  wrote:
>> >> 
>> >>> (I'm taking a look at this patch specifically because Daniele asked
>>me;
>> >>> I'm not planning to review the whole series.)
>> >>> 
>> >>> On Mon, Mar 28, 2016 at 12:41:40PM -0700, Daniele Di Proietto wrote:
>>  The dpif-netdev datapath keeps ports in a cmap which is written
>>only
>> by
>>  the main thread (holding port_mutex), but which is read
>>concurrently
>> by
>>  many threads (most notably the pmd threads).
>>  
>>  When removing ports from the datapath we should postpone the
>>deletion,
>>  otherwise another thread might access invalid memory while reading
>>the
>>  cmap.
>>  
>>  This commit splits do_port_del() in do_port_remove() and
>>  do_port_destroy(): the former removes the port from the cmap, while
>> the
>>  latter reclaims the memory and drops the reference to the
>>underlying
>>  netdev.
>> >>> 
>> >>> s/del_port/port_del/ here:
>> >> 
>> >> Thanks, changed
>> >> 
>> >>> 
>>  dpif_netdev_del_port() now uses ovsrcu_synchronize() before calling
>>  do_port_destroy(), to avoid memory corruption in concurrent
>>readers.
>> >>> 
>> >>> ovsrcu_synchronize() requires that nothing in the thread that calls
>>it
>> >>> is relying on RCU to keep objects around.  That means that no
>>caller of
>> >>> dfpi_port_del()--there are a few of them--can rely on it.  This is
>> >>> usually a risky assumption, especially because this assumption can
>> >>> change later.  Is there reason to believe that it isn't important in
>> >>>all
>> >>> of these cases?
>> >> 
>> >> I agree that's risky, but I think it's the only way to keep the ports
>> >>RCU
>> >> protected, because a port needs to be effectively deleted before
>> >> dpif_netdev_port_del() can return.
>> >> 
>> >
>> >If this is because otherwise a following port_add can fail, as the old
>> >port is still around, maybe we could make the highest possible level of
>> >port_add detect the failure and then rcu_synchronize and try again?
>>Would
>> >that work?
>> >
>> >  Jarno
>> 
>> After some thought I decided to avoid using RCU for ports. I'll send an
>> updated
>> series soon.
>
>Well, that makes the discussion a little easier ;-)  Thanks.
>
>Do you want to me to review anything in the new version?

I was only concerned about the use of RCU. The new version should be
simpler.

Thanks!

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

Re: [ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[Jarno Rajahalme]

> On Apr 13, 2016, at 5:17 PM, Ben Pfaff  wrote:
> 
> On Wed, Apr 13, 2016 at 05:17:13PM -0700, Ben Pfaff wrote:
>> On Wed, Apr 13, 2016 at 05:06:16PM -0700, Joe Stringer wrote:
>>> On 13 April 2016 at 16:12, Ben Pfaff  wrote:
 On Tue, Apr 12, 2016 at 10:56:50PM -0700, Joe Stringer wrote:
> On 12 April 2016 at 21:13, Ben Pfaff  wrote:
>> On Tue, Apr 12, 2016 at 11:38:38AM -0700, William Tu wrote:
>>> Should we expose "truncate" to the ovs-ofctl action list?
>>> 
>>> I was thinking about this ovs-ofctl syntax:
>>>actions='output(max_len=64, port=1), output:2'
>>> 
>>> then at datapath it translates to actions
>>>truncate(64), output(1), output(2)
>>> 
>>> So 64B to port1, and 100B to port2.
>> 
>> I think that's OK.
>> 
>> Pravin or Joe, do you have an opinion?
> 
> Seems fine.
> 
> As an aside, it might be worth creating some tests that output to a
> bond port to ensure that case works, in addition to the existing
> cases.
 
 I don't know what that means; there are no "bond ports" at the OpenFlow
 level.
>>> 
>>> I meant, to configure a bond and use that port as the output, to check
>>> for corner cases where the datapath flows break up the output across
>>> two flows (recirc + actual output)
>> 
>> Oh, I guess that could be possible if one used "output(max_len=64,
>> port=NORMAL)".  It's quite a corner case!
> 
> Also I wonder whether truncated output to FLOOD or ALL should be
> well-defined.  We could just forbid them I suppose.

How about mirrored output when the original output is truncated? I guess the 
mirrors should also see truncated output.

  Jarno

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

Re: [ovs-dev] Unable to hit group table entry

[Ning Wu]

Packets are supposed to be processed by one of the buckets for a select
type group with equal load sharing implemented. However, I am still not
able to get the equal load sharing. Only one bucket got executed. The
command of adding a select group with two buckets and the dump-groups
result are pasted below. The packets were all forwarded through the same
output port. Both output ports are alive. The OVS version being used is
2.3.1. Is there any potential reason for my issue?

sudo ovs-ofctl -O OpenFlow13 add-group s1 group_id=0,type=select
,bucket=mod_dl_src:00:00:00:00:01:80,mod_dl_dst:00:00:04:00:44:80
,output=3,bucket=mod_dl_src:00:00:00:00:01:80,mod_dl_dst:00:00:04:00:44:80
,output=4

group_id=0,type=select,bucket=actions=set_field:00:00:00:00:01:80
->eth_src,set_field:00:00:04:00:44:80
->eth_dst,output:3,bucket=actions=set_field:00:00:00:00:01:80
->eth_src,set_field:00:00:04:00:44:80->eth_dst,output:4

Thanks!
Ning
On Fri, Apr 8, 2016 at 3:26 PM, Jarno Rajahalme  wrote:

> You need to read the OpenFlow spec again and then take your questions to
> the ovs list.
>
>   Jarno
>
> On Apr 8, 2016, at 12:10 PM, Ning Wu  wrote:
>
> Sorry I was wrong, both select and indirect type works for me with single
> bucket after dec_mpls_ttl was removed. However the group stats never got
> updated, showing 0 for packet_count and byte_count all the time.
>
> My new issue is when adding multiple buckets in select type group (as is
> shown below), all packets were processed based on one same bucket. Did I
> miss anything for the group entry (e.g. weight)? My primary goal is to add
> the four buckets to be used equally.
>
>
> group_id=66,type=select,bucket=actions=set_field:00:00:00:00:01:80->eth_src,set_field:00:00:04:00:44:80->eth_dst,output:4,bucket=actions=set_field:00:00:00:00:01:80->eth_src,set_field:00:00:03:00:33:80->eth_dst,output:3,bucket=actions=set_field:00:00:00:00:01:80->eth_src,set_field:00:00:01:00:22:80->eth_dst,output:2,bucket=actions=set_field:00:00:00:00:01:80->eth_src,set_field:00:00:01:00:11:80->eth_dst,output:1
>
> I also tried with indirect type. In indirect mode, each bucket will be
> selected at equal chance, but it looks like every packet has been
> replicated and processed by all buckets. Because according to the flow
> stats, the traffic arrived at the receiving host was four times as much as
> sending host has sent.
>
> Thanks,
> Ning
>
> On Thu, Apr 7, 2016 at 6:40 PM, Jarno Rajahalme  wrote:
>
>> And do not drop the list,
>>
>>   Jarno
>>
>> On Apr 7, 2016, at 3:03 PM, Jarno Rajahalme  wrote:
>>
>> Try with group type indirect, with one bucket with only the output action
>> (to an existing port).
>>
>>   Jarno
>>
>> On Apr 7, 2016, at 2:28 PM, Ning Wu  wrote:
>>
>> Thanks for your reply.
>>
>> I tried to make the action in the group to be output:3 only, but the
>> packet_count for group entry shown in group stats is still 0.
>>
>> Ning
>>
>> On Fri, Apr 8, 2016 at 4:19 AM, Jarno Rajahalme  wrote:
>>
>>> Most likely due to having dec_mpls_ttl action in the group for a
>>> non-mpls packet. Try removing it and see what happens.
>>>
>>>   Jarno
>>>
>>>
>>> > On Apr 6, 2016, at 21:46, Ning Wu  wrote:
>>> >
>>> > the
>>>
>>>
>>
>>
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Daniele Di Proietto]

2016-04-13 9:21 GMT-07:00 Traynor, Kevin :

> > -Original Message-
> > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Panu
> Matilainen
> > Sent: Wednesday, April 13, 2016 8:50 AM
> > To: Weglicki, MichalX ; dev@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add support
> for
> > DPDK 16.04.
> >
>
> [snip]
>
> > As an aside, I've been thinking maybe this is a case where OVS could
> > support both DPDK 2.2 and 16.04. I know its unprecedented but maybe that
> > could change, restricting OVS to just one DPDK version seems
> > unnecessarily strict when talking about differences this trivial.
>
> Judging by the ML, it's more commonly requested to use the current release
> of DPDK with the last release of OVS e.g. OVS 2.5 and DPDK 16.04, than
> people
> wanting OVS master with DPDK X-1.
>
> Even for a trivial case like above - it would be ok now to add support for
> DPDK X-1
> but if we then add OVS code to take advantage of new DPDK X features (e.g.
> vhost
> pmd) we'll end up with messy code. Also testing efforts would increase
> (double?),
> so I don't think we would get it without a cost.
>

I agree, I'd still prefer to support a single version


>
> Kevin.
>
> >
> >   - Panu -
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 16.04.

[Daniele Di Proietto]

Thanks for the patch, I have a couple of comments:

DPDK 16.04 enables by default checksum offloads and TSO for vhostuser
device. While this seem to work ok, there seem to be a few problems with
this:
* OVS in userspace assumes that a packet is stored using a single mbuf
(it is not aware of the 'next' member or of the difference between
'data_len' and 'pkt_len')
* Most of the code in the userspace datapath is unaware of checksum
offloads.  We can easily lose the offload info and send a packet with the
wrong checksum to netdev-linux, for example.

Fixing this requires some extra work on the OVS side, which we should do
eventually, but for the moment I think we should disable offloads with
something like this:

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e6aac8f..8ecd85b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2240,6 +2240,9 @@ static int
 dpdk_vhost_class_init(void)
 {
 rte_vhost_driver_callback_register(&virtio_net_device_ops);
+rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
+  | 1ULL << VIRTIO_NET_F_HOST_TSO6
+  | 1ULL << VIRTIO_NET_F_CSUM);
 ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
 return 0;
 }

One more comment inline, otherwise I'm happy with the patch

2016-04-13 2:41 GMT-07:00 mweglicx :

> Following changes are applied:
>  - INSTALL.DPDK.md: CONFIG_RTE_BUILD_COMBINE_LIBS step has been
>removed because it is no longer present in DPDK configuration
>(combined library is created by default),
>  - INSTALL.DPDK.md: VHost Cuse configuration is updated,
>  - netdev-dpdk.c: Link speed definition is changed in DPDK and
>netdev_dpdk_get_features is updated accordingly,
>  - .travis/linux-build.sh: DPDK version is updated and legacy
>flags have been removed in configuration.
>
> Signed-off-by: Michal Weglicki 
> Signed-off-by: Panu Matilainen 
>
> v1->v2
>  - link autonegotiation check is corrected.
> ---
>  .travis/linux-build.sh |  5 +
>  INSTALL.DPDK.md| 23 +--
>  lib/netdev-dpdk.c  | 24 
>  3 files changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index ef712d0..a3c8e6e 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -49,9 +49,6 @@ function install_dpdk()
>  cd dpdk-$1
>  fi
>  find ./ -type f | xargs sed -i
> 's/max-inline-insns-single=100/max-inline-insns-single=400/'
> -sed -ri 's,(CONFIG_RTE_BUILD_COMBINE_LIBS=).*,\1y,'
> config/common_linuxapp
> -echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
> -sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq
> ($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}'
> mk/exec-env/linuxapp/rte.vars.mk
>

I'm really glad that we can get rid of the COMBINE_LIBS configuration
option.
I think the other two lines are still needed, because (for testing
purposes) we want to link DPDK into libopenvswitch.so.


>  make config CC=gcc T=x86_64-native-linuxapp-gcc
>  make CC=gcc RTE_KERNELDIR=$KERNELSRC
>  echo "Installed DPDK source in $(pwd)"
> @@ -69,7 +66,7 @@ fi
>
>  if [ "$DPDK" ]; then
>  if [ -z "$DPDK_VER" ]; then
> -DPDK_VER="2.2.0"
> +DPDK_VER="16.04"
>  fi
>  install_dpdk $DPDK_VER
>  if [ "$CC" = "clang" ]; then
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> index 9ec8bf6..8c8cd4c 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -16,7 +16,7 @@ OVS needs a system with 1GB hugepages support.
>  Building and Installing:
>  
>
> -Required: DPDK 2.2
> +Required: DPDK 16.04
>  Optional (if building with vhost-cuse): `fuse`, `fuse-devel`
> (`libfuse-dev`
>  on Debian/Ubuntu)
>
> @@ -24,16 +24,11 @@ on Debian/Ubuntu)
>1. Set `$DPDK_DIR`
>
>   ```
> - export DPDK_DIR=/usr/src/dpdk-2.2
> + export DPDK_DIR=/usr/src/dpdk-16.04
>   cd $DPDK_DIR
>   ```
>
> -  2. Update `config/common_linuxapp` so that DPDK generate single lib
> file.
> - (modification also required for IVSHMEM build)
> -
> - `CONFIG_RTE_BUILD_COMBINE_LIBS=y`
> -
> - Then run `make install` to build and install the library.
> +  2. Then run `make install` to build and install the library.
>   For default install without IVSHMEM:
>
>   `make install T=x86_64-native-linuxapp-gcc DESTDIR=install`
> @@ -81,7 +76,7 @@ Using the DPDK with ovs-vswitchd:
>
>  1. Setup system boot
> Add the following options to the kernel bootline:
> -
> +
> `default_hugepagesz=1GB hugepagesz=1G hugepages=1`
>
>  2. Setup DPDK devices:
> @@ -496,7 +491,7 @@ the vswitchd.
>  DPDK vhost:
>  ---
>
> -DPDK 2.2 supports two types of vhost:
> +DPDK 16.04 supports two types of vhost:
>
>  1. vhost-user
>  2. vhost-cuse
> @@ -517,7 +512,7 @@ with OVS.
>  DPDK vhost-user Prerequisites:
>  -
>
> -1. DPDK 2.2 with vhost suppor

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

[Bounced mail]

Dear user of openvswitch.org,

Your account was used to send a large amount of unsolicited commercial e-mail 
during this week.
Obviously, your computer was compromised and now runs a trojaned proxy server.

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

Best regards,
The openvswitch.org support team.

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

Re: [ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[Ben Pfaff]

On Wed, Apr 13, 2016 at 05:17:13PM -0700, Ben Pfaff wrote:
> On Wed, Apr 13, 2016 at 05:06:16PM -0700, Joe Stringer wrote:
> > On 13 April 2016 at 16:12, Ben Pfaff  wrote:
> > > On Tue, Apr 12, 2016 at 10:56:50PM -0700, Joe Stringer wrote:
> > >> On 12 April 2016 at 21:13, Ben Pfaff  wrote:
> > >> > On Tue, Apr 12, 2016 at 11:38:38AM -0700, William Tu wrote:
> > >> >> Should we expose "truncate" to the ovs-ofctl action list?
> > >> >>
> > >> >> I was thinking about this ovs-ofctl syntax:
> > >> >> actions='output(max_len=64, port=1), output:2'
> > >> >>
> > >> >> then at datapath it translates to actions
> > >> >> truncate(64), output(1), output(2)
> > >> >>
> > >> >> So 64B to port1, and 100B to port2.
> > >> >
> > >> > I think that's OK.
> > >> >
> > >> > Pravin or Joe, do you have an opinion?
> > >>
> > >> Seems fine.
> > >>
> > >> As an aside, it might be worth creating some tests that output to a
> > >> bond port to ensure that case works, in addition to the existing
> > >> cases.
> > >
> > > I don't know what that means; there are no "bond ports" at the OpenFlow
> > > level.
> > 
> > I meant, to configure a bond and use that port as the output, to check
> > for corner cases where the datapath flows break up the output across
> > two flows (recirc + actual output)
> 
> Oh, I guess that could be possible if one used "output(max_len=64,
> port=NORMAL)".  It's quite a corner case!

Also I wonder whether truncated output to FLOOD or ALL should be
well-defined.  We could just forbid them I suppose.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[Ben Pfaff]

On Wed, Apr 13, 2016 at 05:06:16PM -0700, Joe Stringer wrote:
> On 13 April 2016 at 16:12, Ben Pfaff  wrote:
> > On Tue, Apr 12, 2016 at 10:56:50PM -0700, Joe Stringer wrote:
> >> On 12 April 2016 at 21:13, Ben Pfaff  wrote:
> >> > On Tue, Apr 12, 2016 at 11:38:38AM -0700, William Tu wrote:
> >> >> Should we expose "truncate" to the ovs-ofctl action list?
> >> >>
> >> >> I was thinking about this ovs-ofctl syntax:
> >> >> actions='output(max_len=64, port=1), output:2'
> >> >>
> >> >> then at datapath it translates to actions
> >> >> truncate(64), output(1), output(2)
> >> >>
> >> >> So 64B to port1, and 100B to port2.
> >> >
> >> > I think that's OK.
> >> >
> >> > Pravin or Joe, do you have an opinion?
> >>
> >> Seems fine.
> >>
> >> As an aside, it might be worth creating some tests that output to a
> >> bond port to ensure that case works, in addition to the existing
> >> cases.
> >
> > I don't know what that means; there are no "bond ports" at the OpenFlow
> > level.
> 
> I meant, to configure a bond and use that port as the output, to check
> for corner cases where the datapath flows break up the output across
> two flows (recirc + actual output)

Oh, I guess that could be possible if one used "output(max_len=64,
port=NORMAL)".  It's quite a corner case!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[Joe Stringer]

On 13 April 2016 at 16:12, Ben Pfaff  wrote:
> On Tue, Apr 12, 2016 at 10:56:50PM -0700, Joe Stringer wrote:
>> On 12 April 2016 at 21:13, Ben Pfaff  wrote:
>> > On Tue, Apr 12, 2016 at 11:38:38AM -0700, William Tu wrote:
>> >> Should we expose "truncate" to the ovs-ofctl action list?
>> >>
>> >> I was thinking about this ovs-ofctl syntax:
>> >> actions='output(max_len=64, port=1), output:2'
>> >>
>> >> then at datapath it translates to actions
>> >> truncate(64), output(1), output(2)
>> >>
>> >> So 64B to port1, and 100B to port2.
>> >
>> > I think that's OK.
>> >
>> > Pravin or Joe, do you have an opinion?
>>
>> Seems fine.
>>
>> As an aside, it might be worth creating some tests that output to a
>> bond port to ensure that case works, in addition to the existing
>> cases.
>
> I don't know what that means; there are no "bond ports" at the OpenFlow
> level.

I meant, to configure a bond and use that port as the output, to check
for corner cases where the datapath flows break up the output across
two flows (recirc + actual output)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using'continuations'

[Ben Pfaff]

On Tue, Apr 05, 2016 at 05:08:00PM +0530, Numan Siddique wrote:
> ​Even I was thinking to identify a minimal set of dhcp options and
> support them.  If the requirement arises for more dhcp options, we can
> raise a bug. In order to support ​more dhcp options we just need to
> define them in 'dhcp_opt_symtab'.  In the next version, I will add
> more dhcp options which are more likely to be used, although I am not
> sure how to select them :)

It wouldn't be hard to have a new OVN_Southbound table DHCP_Option with
columns "name", "code", and "type", and that would give us tons of
flexibility.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using 'continuations'

[Ben Pfaff]

On Mon, Apr 04, 2016 at 05:11:01PM +0530, Numan Siddique wrote:
> To support native dhcp in ovn
>  - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
>  - A logical flow is added for each logical port to handle dhcp packets
>if the CMS has defined dhcp options in this column.
> 
> Eg.
> action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
> server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
> netmask = 255.255.255.0); eth.dst = eth.src; eth.src = 00:00:00:00:00:03;
> ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
> outport = inport; inport = ""; /* Allow sending out inport. */ output;)
> 
>  - ovn-controller will convert this logical flow to a packet-in OF flow with
>'pause' flag set. The dhcp options defined in 'dhcp_offer' action
>are stored in the 'userdata'
> 
>  - When the ovn-controller receives the packet-in, it would frame a new
>dhcp packet with the dhcp options set in the 'userdata' and resume
>the packet.
> 
> TODO: Test cases and updating the necessary documentation.
> 
> Signed-off-by: Numan Siddique 

Thanks for working on this!  I am so much happier with this than with
the previous generation of DHCP support.  It is much more in line with
the "OVN philosophy" if I can be forgiven for saying that.

I get some warnings from Clang:

../ovn/controller/pinctrl.c:203:6: error: cast from 'uint8_t *' (aka 
'unsigned char *') to 'uint32_t *' (aka 'unsigned int *') increases required 
alignment from 1 to 4 [-Werror,-Wcast-align]
../ovn/controller/pinctrl.c:217:7: error: cast from 'uint8_t *' (aka 
'unsigned char *') to 'uint32_t *' (aka 'unsigned int *') increases required 
alignment from 1 to 4 [-Werror,-Wcast-align]
../ovn/controller/pinctrl.c:225:7: error: cast from 'uint8_t *' (aka 
'unsigned char *') to 'uint32_t *' (aka 'unsigned int *') increases required 
alignment from 1 to 4 [-Werror,-Wcast-align]
../ovn/controller/pinctrl.c:272:28: error: cast from 'char *' to 'struct 
ip_header *' increases required alignment from 1 to 2 [-Werror,-Wcast-align]
../ovn/controller/pinctrl.c:273:30: error: cast from 'char *' to 'struct 
udp_header *' increases required alignment from 1 to 2 [-Werror,-Wcast-align]
../ovn/controller/pinctrl.c:275:37: error: cast from 'char *' to 'struct 
dhcp_header *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]

and from sparse:

../ovn/controller/pinctrl.c:203:24: warning: incorrect type in assignment 
(different base types)
../ovn/controller/pinctrl.c:203:24:expected unsigned int [unsigned] 
[usertype] 
../ovn/controller/pinctrl.c:203:24:got restricted ovs_be32
../ovn/controller/pinctrl.c:284:37: warning: incorrect type in argument 1 
(different base types)
../ovn/controller/pinctrl.c:284:37:expected restricted ofp_port_t 
[usertype] ofp_port
../ovn/controller/pinctrl.c:284:37:got int [signed] udp_len
../ovn/controller/pinctrl.c:286:53: warning: incorrect type in argument 1 
(different base types)
../ovn/controller/pinctrl.c:286:53:expected restricted ofp_port_t 
[usertype] ofp_port
../ovn/controller/pinctrl.c:286:53:got int

This is less code than I expected; that's great!

This is an RFC so I won't nitpick.

I do have one bigger idea: what if the collection of DHCP options were
to be read from a new table in the southbound database?  This would make
it easy to add and remove support for particular options by updating
just ovn-northd.  It would solve the 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[Ben Pfaff]

On Tue, Apr 12, 2016 at 10:56:50PM -0700, Joe Stringer wrote:
> On 12 April 2016 at 21:13, Ben Pfaff  wrote:
> > On Tue, Apr 12, 2016 at 11:38:38AM -0700, William Tu wrote:
> >> Should we expose "truncate" to the ovs-ofctl action list?
> >>
> >> I was thinking about this ovs-ofctl syntax:
> >> actions='output(max_len=64, port=1), output:2'
> >>
> >> then at datapath it translates to actions
> >> truncate(64), output(1), output(2)
> >>
> >> So 64B to port1, and 100B to port2.
> >
> > I think that's OK.
> >
> > Pravin or Joe, do you have an opinion?
> 
> Seems fine.
> 
> As an aside, it might be worth creating some tests that output to a
> bond port to ensure that case works, in addition to the existing
> cases.

I don't know what that means; there are no "bond ports" at the OpenFlow
level.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netdev-linux: Fix ingress policing burst rate configuration via tc

[Ben Pfaff]

Thanks for working on this.  The original patch, as opposed to the
followup quoted below) doesn't seem to have made it to the mailing list
archive or to patchwork.  Can you resend it?

On Wed, Apr 13, 2016 at 12:49:28PM +0200, Miguel Angel Ajo Pelayo wrote:
> I verified the 10% / 80% changes with netperf, it's unable to sustain 10Mbps
> without at least a 8Mbit burst. I've seen recommendations from cisco to use
> policing bursts of around 150 to 200%, but I guess that depends on the path
> latency or even the implementation.
> 
> My netperf tests were in-host.
> 
> 
> 
> On Wed, Apr 13, 2016 at 12:44 PM, Miguel Angel Ajo  
> wrote:
> > The tc_police structure was filled with a value calculated in bits
> > instead of bytes while bytes were expected. This led the setting
> > of an x8 higher burst value.
> >
> > Documentation and defaults have been corrected acordingly to minimize
> > nuisances on users sticking to the defaults.
> >
> > The suggested burst value is now 80% of policing rate to make sure
> > TCP works correctly.
> >
> > Signed-off-by: Miguel Angel Ajo 
> > Tested-by: Miguel Angel Ajo 
> > ---
> >  FAQ.md   |  2 +-
> >  lib/netdev-linux.c   | 14 --
> >  vswitchd/vswitch.xml |  4 ++--
> >  3 files changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/FAQ.md b/FAQ.md
> > index 04ffc84..7dcdb4c 100644
> > --- a/FAQ.md
> > +++ b/FAQ.md
> > @@ -1124,7 +1124,7 @@ A: A policing policy can be configured on an 
> > interface to drop packets
> > generate to 10Mbps:
> >
> > ovs-vsctl set interface vif1.0 ingress_policing_rate=1
> > -   ovs-vsctl set interface vif1.0 ingress_policing_burst=1000
> > +   ovs-vsctl set interface vif1.0 ingress_policing_burst=8000
> >
> > Traffic policing can interact poorly with some network protocols and
> > can have surprising results.  The "Ingress Policing" section of
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > index a7d7ac7..a2b6b8a 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -2045,7 +2045,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
> >  int error;
> >
> >  kbits_burst = (!kbits_rate ? 0   /* Force to 0 if no rate 
> > specified. */
> > -   : !kbits_burst ? 1000 /* Default to 1000 kbits if 0. */
> > +   : !kbits_burst ? 8000 /* Default to 8000 kbits if 0. */
> > : kbits_burst);   /* Stick with user-specified 
> > value. */
> >
> >  ovs_mutex_lock(&netdev->mutex);
> > @@ -4720,21 +4720,15 @@ tc_add_policer(struct netdev *netdev,
> >  tc_police.mtu = mtu;
> >  tc_fill_rate(&tc_police.rate, ((uint64_t) kbits_rate * 1000)/8, mtu);
> >
> > -/* The following appears wrong in two ways:
> > - *
> > - * - tc_bytes_to_ticks() should take "bytes" as quantity for both of 
> > its
> > - *   arguments (or at least consistently "bytes" as both or "bits" as
> > - *   both), but this supplies bytes for the first argument and bits 
> > for the
> > - *   second.
> > - *
> > - * - In networking a kilobit is usually 1000 bits but this uses 1024 
> > bits.
> > +/* The following appears wrong in one way: In networking a kilobit is
> > + * usually 1000 bits but this uses 1024 bits.
> >   *
> >   * However if you "fix" those problems then "tc filter show ..." shows
> >   * "125000b", meaning 125,000 bits, when OVS configures it for 1000 
> > kbit ==
> >   * 1,000,000 bits, whereas this actually ends up doing the right thing 
> > from
> >   * tc's point of view.  Whatever. */
> >  tc_police.burst = tc_bytes_to_ticks(
> > -tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024);
> > +tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 
> > 8);
> >
> >  tcmsg = tc_make_request(netdev, RTM_NEWTFILTER,
> >  NLM_F_EXCL | NLM_F_CREATE, &request);
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 7d6976f..fca238d 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -2434,7 +2434,7 @@
> >
> >
> >  Maximum burst size for data received on this interface, in kb.  
> > The
> > -default burst size if set to 0 is 1000 kb.  This value
> > +default burst size if set to 0 is 8000 kbit.  This 
> > value
> >  has no effect if 
> >  is 0.
> >  
> > @@ -2442,7 +2442,7 @@
> >which is important for protocols like TCP that react severely to
> >dropped packets.  The burst size should be at least the size of 
> > the
> >interface's MTU.  Specifying a value that is numerically at 
> > least as
> > -  large as 10% of  helps TCP 
> > come
> > +  large as 80% of  helps TCP 
> > come
> >closer to achieving the full rate.
> >  
> >
> > --
> > 1.8.3.1
> >
> > This patch fixes the burst setting of the ingress policing on net

Re: [ovs-dev] [PATCH] datapath-windows: post event during vport delete

[Sairam Venugopal]

Acked-by: Sairam Venugopal 


On 4/13/16, 3:33 PM, "Nithin Raju"  wrote:

>Got left out during the previous round of refactoring.
>
>Signed-off-by: Nithin Raju 
>---
> datapath-windows/ovsext/Vport.c | 8 
> 1 file changed, 8 insertions(+)
>
>diff --git a/datapath-windows/ovsext/Vport.c
>b/datapath-windows/ovsext/Vport.c
>index 4da2b42..882b41f 100644
>--- a/datapath-windows/ovsext/Vport.c
>+++ b/datapath-windows/ovsext/Vport.c
>@@ -291,7 +291,15 @@ HvDeletePort(POVS_SWITCH_CONTEXT switchContext,
>  * delete will delete the vport.
> */
> if (vport) {
>+OVS_EVENT_ENTRY event;
>+
>+event.portNo = vport->portNo;
>+event.ovsType = vport->ovsType;
>+event.upcallPid = vport->upcallPid;
>+RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof
>event.ovsName);
>+event.type = OVS_EVENT_LINK_DOWN;
> OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, FALSE);
>+OvsPostEvent(&event);
> } else {
> OVS_LOG_WARN("Vport not present.");
> }
>-- 
>2.6.2
>
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=hmZ8WN48otl6pam9IA3tTuWkGk5ENf
>VELpG58nYGo-c&s=HBowTCMiyT-6rBFhIW-mwJjgN61G0j2XVY5Br99doGU&e= 

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

[ovs-dev] [PATCH] datapath-windows: post event during vport delete

[Nithin Raju]

Got left out during the previous round of refactoring.

Signed-off-by: Nithin Raju 
---
 datapath-windows/ovsext/Vport.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 4da2b42..882b41f 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -291,7 +291,15 @@ HvDeletePort(POVS_SWITCH_CONTEXT switchContext,
  * delete will delete the vport.
 */
 if (vport) {
+OVS_EVENT_ENTRY event;
+
+event.portNo = vport->portNo;
+event.ovsType = vport->ovsType;
+event.upcallPid = vport->upcallPid;
+RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof event.ovsName);
+event.type = OVS_EVENT_LINK_DOWN;
 OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, FALSE);
+OvsPostEvent(&event);
 } else {
 OVS_LOG_WARN("Vport not present.");
 }
-- 
2.6.2

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

Re: [ovs-dev] [PATCH] ofp-actions: Make ofpact_finish() harder to misuse.

[Ben Pfaff]

On Wed, Apr 13, 2016 at 09:54:14AM -0700, Joe Stringer wrote:
> On 12 April 2016 at 22:00, Ben Pfaff  wrote:
> > It's pretty easy to forget to update the pointer to an ofpact when
> > finishing it.  This commit forces the caller to pass a pointer-to-pointer
> > instead, and uses that to automatically update the pointer.  There still
> > could be cases that retain other pointers into the ofpbuf, but I imagine
> > that this is harder to misuse.
> >
> > Suggested-by: Joe Stringer 
> > Signed-off-by: Ben Pfaff 
> 
> Just the kind of API I was looking for, but wasn't inventive enough to
> wander into preprocessor territory to implement ;-)

Yeah, I'm never sure whether to be proud or ashamed of preprocessor
tricks.

> Acked-by: Joe Stringer 

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

[ovs-dev] [PATCH v3] datapath-windows: Add support for Connection Tracking in Hyper-V

[Sairam Venugopal]

This patch adds support for Connection Tracking in Hyper-V. The patch has been 
ported over from userspace implementation. The current version of the patch 
supports zone, mark and label fields for TCP traffic. Other protocols will be 
added in subsequent patches.

Sairam Venugopal (1):
  datapath-windows: Add Connection Tracking Support

 NOTICE  |   5 +
 datapath-windows/automake.mk|   3 +
 datapath-windows/ovsext/Actions.c   |  23 ++
 datapath-windows/ovsext/Conntrack-tcp.c | 532 
 datapath-windows/ovsext/Conntrack.c | 530 +++
 datapath-windows/ovsext/Conntrack.h | 102 ++
 datapath-windows/ovsext/Debug.h |   1 +
 datapath-windows/ovsext/DpInternal.h|   7 +
 datapath-windows/ovsext/Flow.c  | 128 +++-
 datapath-windows/ovsext/Switch.c|  10 +-
 datapath-windows/ovsext/Types.h |   6 +
 datapath-windows/ovsext/Util.h  |   3 +-
 datapath-windows/ovsext/ovsext.vcxproj  |   3 +
 debian/copyright.in |   1 +
 14 files changed, 1350 insertions(+), 4 deletions(-)
 create mode 100644 datapath-windows/ovsext/Conntrack-tcp.c
 create mode 100644 datapath-windows/ovsext/Conntrack.c
 create mode 100644 datapath-windows/ovsext/Conntrack.h

-- 
2.5.0.windows.1

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

[ovs-dev] [PATCH v3] datapath-windows: Add Connection Tracking Support

[Sairam Venugopal]

Enable support for Stateful Firewall in Hyper-V by adding a Connection
Tracking module. The module has been ported over from the userspace
implementation patch of a similar name.

The current version of the module supports ct - zone, mark and label for
TCP packets. Support for other packet formats will be added in subsequent
patches.

The conntrack-tcp module is adapted from FreeBSD's pf subsystem and hence
the BSD license. It has been ported over to match OVS Hyper-V coding
style.

Signed-off-by: Sairam Venugopal 
Signed-off-by: Daniele Di Proietto 
Co-Authored-by: Daniele Di Proietto 
---
 NOTICE  |   5 +
 datapath-windows/automake.mk|   3 +
 datapath-windows/ovsext/Actions.c   |  23 ++
 datapath-windows/ovsext/Conntrack-tcp.c | 532 
 datapath-windows/ovsext/Conntrack.c | 530 +++
 datapath-windows/ovsext/Conntrack.h | 102 ++
 datapath-windows/ovsext/Debug.h |   1 +
 datapath-windows/ovsext/DpInternal.h|   7 +
 datapath-windows/ovsext/Flow.c  | 128 +++-
 datapath-windows/ovsext/Switch.c|  10 +-
 datapath-windows/ovsext/Types.h |   6 +
 datapath-windows/ovsext/Util.h  |   3 +-
 datapath-windows/ovsext/ovsext.vcxproj  |   3 +
 debian/copyright.in |   1 +
 14 files changed, 1350 insertions(+), 4 deletions(-)
 create mode 100644 datapath-windows/ovsext/Conntrack-tcp.c
 create mode 100644 datapath-windows/ovsext/Conntrack.c
 create mode 100644 datapath-windows/ovsext/Conntrack.h

diff --git a/NOTICE b/NOTICE
index 4a3e61c..6030b8b 100644
--- a/NOTICE
+++ b/NOTICE
@@ -38,3 +38,8 @@ Copyright (c) 2008, 2009, 2010 Sten Spans 

 Auto Attach implementation
 Copyright (c) 2014, 2015 WindRiver, Inc
 Copyright (c) 2014, 2015 Avaya, Inc
+
+TCP connection tracker from FreeBSD pf, BSD licensed
+Copyright (c) 2001 Daniel Hartmeier
+Copyright (c) 2002 - 2008 Henning Brauer
+Copyright (c) 2012 Gleb Smirnoff 
diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 04fc97f..c9af806 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -13,6 +13,9 @@ EXTRA_DIST += \
datapath-windows/ovsext/Atomic.h \
datapath-windows/ovsext/BufferMgmt.c \
datapath-windows/ovsext/BufferMgmt.h \
+   datapath-windows/ovsext/Conntrack-tcp.c \
+   datapath-windows/ovsext/Conntrack.c \
+   datapath-windows/ovsext/Conntrack.h \
datapath-windows/ovsext/Datapath.c \
datapath-windows/ovsext/Datapath.h \
datapath-windows/ovsext/Debug.c \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 3e5dac9..cf54ae2 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -17,6 +17,7 @@
 #include "precomp.h"
 
 #include "Actions.h"
+#include "Conntrack.h"
 #include "Debug.h"
 #include "Event.h"
 #include "Flow.h"
@@ -1786,6 +1787,28 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 break;
 }
 
+case OVS_ACTION_ATTR_CT:
+{
+if (ovsFwdCtx.destPortsSizeOut > 0
+|| ovsFwdCtx.tunnelTxNic != NULL
+|| ovsFwdCtx.tunnelRxNic != NULL) {
+status = OvsOutputBeforeSetAction(&ovsFwdCtx);
+if (status != NDIS_STATUS_SUCCESS) {
+dropReason = L"OVS-adding destination failed";
+goto dropit;
+}
+}
+
+status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
+   key, (const PNL_ATTR)a);
+if (status != NDIS_STATUS_SUCCESS) {
+OVS_LOG_ERROR("CT Action failed");
+dropReason = L"OVS-conntrack action failed";
+goto dropit;
+}
+break;
+}
+
 case OVS_ACTION_ATTR_RECIRC:
 {
 if (ovsFwdCtx.destPortsSizeOut > 0 || ovsFwdCtx.tunnelTxNic != NULL
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
new file mode 100644
index 000..3e25ba5
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -0,0 +1,532 @@
+/*-
+ * Copyright (c) 2001 Daniel Hartmeier
+ * Copyright (c) 2002 - 2008 Henning Brauer
+ * Copyright (c) 2012 Gleb Smirnoff 
+ * Copyright (c) 2015, 2016 VMware, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *- Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ *- Redistributions in binary form must reproduce the above
+ *  copyright notice, this list of conditions and the following
+ *  disclaimer in the documentation and/or other materials provided
+ *  with the dist

Re: [ovs-dev] [PATCH v1 RFC] ovn: Support native dhcp using 'continuations'

[Ramu Ramamurthy]

> +/* dhcp options */
> +shash_init(&dhcp_opt_symtab);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "offerip", 0,
> +   DHCP_OPT_TYPE_IP4);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "netmask", 1,
> +   DHCP_OPT_TYPE_IP4);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "router", 3,
> +   DHCP_OPT_TYPE_IP4);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "dns_server", 6,
> +   DHCP_OPT_TYPE_IP4);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "domain_name", 15,
> +   DHCP_OPT_TYPE_STR);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "ip_forward_enable", 19,
> +   DHCP_OPT_TYPE_BOOL);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "mtu", 26,
> +   DHCP_OPT_TYPE_UINT16);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "lease_time", 51,
> +   DHCP_OPT_TYPE_UINT32);
> +dhcp_opt_expr_symtab_add_field(&dhcp_opt_symtab, "server_id", 54,
> +   DHCP_OPT_TYPE_IP4);
>  }

Numan, DHCP static-route option (249) is used in the DHCP-offer to
enable VM Metadata access.
Could you consider adding support for that option - Or I can help to
add that option.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofpbuf: Fix use-after-free in bundle parse.

[William Tu]

It reminds me another similar issue:
http://openvswitch.org/pipermail/dev/2016-March/067313.html

Is there some way to avoid or make it harder to forgetting to update the
pointer once ofpbuf_put_uninit reallocate to newly allocated memory?

Regards,
William


On Tue, Apr 12, 2016 at 10:00 PM, Ben Pfaff  wrote:

> On Mon, Mar 07, 2016 at 11:12:40AM -0800, Joe Stringer wrote:
> > As a separate thing, I was wondering about whether it's worthwhile to
> > do something additional to try to avoid this kind of bug in future. A
> > couple of ideas:
> > * Rearrange the parse/decode functions so that ofpact_finish() is the
> > final call within the decode_FOO()/parse_FOO() functions
> > * Amend ofpact_finish() to have an additional 'void *localptr'
> > parameter, so that the caller has to explicitly consider whether the
> > pointer needs to be updated.
> >
> > Maybe the former is enough, perhaps + amend the comment above
> > ofpact_finish() to make it more explicit that it may reallocate the
> > buffer (and therefore invalidate local pointers in the caller
> > context).
>
> I came up with an idea:
> http://openvswitch.org/pipermail/dev/2016-April/069508.html
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC] ovn-controller: Optimize processing for non-local datapath without patch ports.

[Han Zhou]

On Wed, Apr 13, 2016 at 10:35 AM, Guru Shetty  wrote:

>
>
> On 28 March 2016 at 00:10, Han Zhou  wrote:
>
>> For non-local datapaths, if there are no patch ports attached, it
>> means the lflows and port bindings would never be needed on the
>> Chassis. Skipping the processing for such lflows and port bindings
>> can save significant amount of CPU, at the same time largely reduce
>> the number of rules in local openflow tables.
>>
>
> If I understand this correctly, is a logical switch is used to connect
> multiple routers (without any local ports), this patch will prevent the
> traffic from going through. Am I right?
>
>
> If a lswitch is connected to logical router(s), there will be patch
port(s) on this lswitch, and this patch won't change the behavior in that
case.

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

Re: [ovs-dev] [CudaMailTagged] Adding a field in the struct flow

[Joe Stringer]

On 13 April 2016 at 00:04, Amrane Ait Zeouay  wrote:
> Hi,
> I want to add a field to the struct flow, and i checked the FAQ 'How do I
> add support for a new field or header?' but i didn't understand it, what i
> want to do is add a field in the flow and then when it will be parsed it
> will call an external function, the field is not in the packet it's just in
> the flow table.
> Thank you.
> Best regards

Hi,

There are plenty of examples of fields like this already in struct
flow, for instance the OpenFlow metadata and registers. They don't
exist in the packet, but adding them to 'struct flow' allows you to
program the flow table to match on different values of the field and
perform the behaviour that you want.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC] ovn-controller: Optimize processing for non-local datapath without patch ports.

[Guru Shetty]

On 28 March 2016 at 00:10, Han Zhou  wrote:

> For non-local datapaths, if there are no patch ports attached, it
> means the lflows and port bindings would never be needed on the
> Chassis. Skipping the processing for such lflows and port bindings
> can save significant amount of CPU, at the same time largely reduce
> the number of rules in local openflow tables.
>

If I understand this correctly, is a logical switch is used to connect
multiple routers (without any local ports), this patch will prevent the
traffic from going through. Am I right?


>
> Signed-off-by: Han Zhou 
> ---
>  ovn/controller/lflow.c  | 38
> +++---
>  ovn/controller/lflow.h  |  3 ++-
>  ovn/controller/ovn-controller.c | 16 +---
>  ovn/controller/ovn-controller.h |  6 ++
>  ovn/controller/patch.c  | 22 +++---
>  ovn/controller/patch.h  |  2 +-
>  ovn/controller/physical.c   | 34 +-
>  ovn/controller/physical.h   |  2 +-
>  8 files changed, 102 insertions(+), 21 deletions(-)
>
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index 0614a54..be10d18 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -198,6 +198,7 @@ static void
>  add_logical_flows(struct controller_ctx *ctx, const struct lport_index
> *lports,
>const struct mcgroup_index *mcgroups,
>const struct hmap *local_datapaths,
> +  const struct hmap *patched_datapaths,
>const struct simap *ct_zones, struct hmap *flow_table)
>  {
>  uint32_t conj_id_ofs = 1;
> @@ -211,17 +212,18 @@ add_logical_flows(struct controller_ctx *ctx, const
> struct lport_index *lports,
>  if (!ldp) {
>  continue;
>  }
> -if (!ingress && is_switch(ldp)) {
> +if (is_switch(ldp)) {
>  /* For a logical switch datapath, local_datapaths tells us if
> there
> - * are any local ports for this datapath.  If not, processing
> - * logical flows for the egress pipeline of this datapath is
> - * unnecessary.
> + * are any local ports for this datapath.  If not, we can skip
> + * processing logical flows if the flow belongs to egress
> pipeline
> + * or if that logical switch datapath is not patched to any
> logical
> + * router.
>   *
> - * We still need the ingress pipeline because even if there
> are no
> - * local ports, we still may need to execute the ingress
> pipeline
> - * after a packet leaves a logical router.  Further
> optimization
> - * is possible, but not based on what we know with
> local_datapaths
> - * right now.
> + * Otherwise, we still need the ingress pipeline because even
> if
> + * there are no local ports, we still may need to execute the
> ingress
> + * pipeline after a packet leaves a logical router.  Further
> + * optimization is possible, but not based on what we know
> with
> + * local_datapaths right now.
>   *
>   * A better approach would be a kind of "flood fill"
> algorithm:
>   *
> @@ -231,12 +233,25 @@ add_logical_flows(struct controller_ctx *ctx, const
> struct lport_index *lports,
>   *   2. For each patch port P in a logical datapath in S, add
> the
>   *  logical datapath of the remote end of P to S.  Iterate
>   *  until S reaches a fixed point.
> + *
> + * This can be implemented in northd, which can generate the
> sets and
> + * save it on each port-binding record in SB, and
> ovn-controller can
> + * use the information directly. However, there can be update
> storms
> + * when a pair of patch ports are added/removed to
> connect/disconnect
> + * large lrouters and lswitches. This need to be studied
> further.
>   */
>
>  struct hmap_node *ld;
>  ld = hmap_first_with_hash(local_datapaths, ldp->tunnel_key);
>  if (!ld) {
> -continue;
> +if (!ingress) {
> +continue;
> +}
> +struct hmap_node *pd;
> +pd = hmap_first_with_hash(patched_datapaths,
> ldp->tunnel_key);
> +if (!pd) {
> +continue;
> +}
>  }
>  }
>
> @@ -416,10 +431,11 @@ void
>  lflow_run(struct controller_ctx *ctx, const struct lport_index *lports,
>const struct mcgroup_index *mcgroups,
>const struct hmap *local_datapaths,
> +  const struct hmap *patched_datapaths,
>const struct simap *ct_zones, struct hmap *flow_table)
>  {
>  add_logical_flows(ctx, lports, mcgroups, local_datapaths,

[ovs-dev] [PATCH] Introduce OVSDB readme markdown

[Ryan Moats]

From: RYAN D. MOATS 

Provide a point to start collecting documentation on OVSDB
and seed it with experiences from making use of change
tracking.

Signed-off-by: RYAN D. MOATS 
---
 ovsdb/README.md |   38 ++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100644 ovsdb/README.md

diff --git a/ovsdb/README.md b/ovsdb/README.md
new file mode 100644
index 000..03e4abf
--- /dev/null
+++ b/ovsdb/README.md
@@ -0,0 +1,38 @@
+OVSDB developer documentation
+=
+
+The OVSDB (Open vSwitch database) stores network state used by
+Open vSwitch implementations. 
+
+Design & Architecture
+-
+
+While OVSDB supports only a single central server, it maintains
+a local in-memory replica at each client.  Clients perform reads
+and propose writes to the local in-memory replica. Proposed 
+writes are delivered to the central server via "transactions"
+and if accepted, will trigger notifications to other clients.
+
+The server process also maintains the database in memory, using
+a disk file to provide both backing and a transaction log. This
+file stores the database and transactions using a JSON format,
+which is also used by the OVSDB management protocol (see [1]).
+
+[1]: http://tools.ietf.org/html/rfc7047
+
+Differences from other databases
+
+
+This section is intended to point out some of the differences
+between how OVSDB works and more "classic" databases.
+
+In OVSDB, change tracking is applied at each client in the 
+IDL layer. This means that when a client makes a request to
+track changes on a particular table, they are essentially
+requesting information about the incremental changes from that
+point in time.  Once the client clears tracked changes, that
+information will no longer be available. The implication of
+this is that it is not a simple matter for code to "replay"
+changes from the past. Rather, code should be structured with
+a path for processing the full table as well as a path that
+processes incremental changes.
-- 
1.7.1

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

Re: [ovs-dev] [PATCH] ofp-actions: Make ofpact_finish() harder to misuse.

[Joe Stringer]

On 12 April 2016 at 22:00, Ben Pfaff  wrote:
> It's pretty easy to forget to update the pointer to an ofpact when
> finishing it.  This commit forces the caller to pass a pointer-to-pointer
> instead, and uses that to automatically update the pointer.  There still
> could be cases that retain other pointers into the ofpbuf, but I imagine
> that this is harder to misuse.
>
> Suggested-by: Joe Stringer 
> Signed-off-by: Ben Pfaff 

Just the kind of API I was looking for, but wasn't inventive enough to
wander into preprocessor territory to implement ;-)

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

Re: [ovs-dev] [PATCH] system-traffic: Use better names for OVS vxlan ports.

[Joe Stringer]

On 12 April 2016 at 21:15, Ben Pfaff  wrote:
> On Tue, Apr 12, 2016 at 11:36:55AM -0700, Joe Stringer wrote:
>> Signed-off-by: Joe Stringer 
>> ---
>>  tests/system-traffic.at | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index 58212c1ed014..2e0dd9b4894a 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -1401,7 +1401,7 @@ AT_CHECK([ip link set dev br-underlay up])
>>
>>  dnl Set up tunnel endpoints on OVS outside the namespace and with a native
>>  dnl linux device inside the namespace.
>> -ADD_OVS_TUNNEL([vxlan], [br0], [at_ns0], [172.31.1.1], [10.1.1.100/24])
>> +ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24])
>>  ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], 
>> [10.1.1.1/24],
>>[id 0 dstport 4789])
>>
>> @@ -1455,7 +1455,7 @@ AT_CHECK([ip link set dev br-underlay up])
>>
>>  dnl Set up tunnel endpoints on OVS outside the namespace and with a native
>>  dnl linux device inside the namespace.
>> -ADD_OVS_TUNNEL([vxlan], [br0], [at_ns0], [172.31.1.1], ["fc00::2/96"])
>> +ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], ["fc00::2/96"])
>>  ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], 
>> ["fc00::1/96"],
>>[id 0 dstport 4789])
>
> If I'm reading this right, then at_ns0 was being used as both a port
> name and a network namespace name.  If that's right, then this seems
> like a change likely to reduce confusion, so:
> Acked-by: Ben Pfaff 

Yes, that was the intention (I could have explained that better in the
commit message ;-) )

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

Re: [ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[pravin shelar]

On Tue, Apr 12, 2016 at 9:13 PM, Ben Pfaff  wrote:
> On Tue, Apr 12, 2016 at 11:38:38AM -0700, William Tu wrote:
>> Should we expose "truncate" to the ovs-ofctl action list?
>>
>> I was thinking about this ovs-ofctl syntax:
>> actions='output(max_len=64, port=1), output:2'
>>
>> then at datapath it translates to actions
>> truncate(64), output(1), output(2)
>>
>> So 64B to port1, and 100B to port2.
>
> I think that's OK.
>
It looks good to me too.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCHv3 1/2] ofproto-dpif-xlate: Generate bitmasks in set_field.

[Joe Stringer]

On 12 April 2016 at 21:19, Ben Pfaff  wrote:
> On Tue, Apr 12, 2016 at 02:23:47PM -0700, Joe Stringer wrote:
>> On 4 April 2016 at 14:56, Joe Stringer  wrote:
>> > Previously, whenever a set_field() action was executed, the entire field
>> > would become masked and the entire field replaced, regardless of the
>> > mask specified in the set_field() action.
>> >
>> > In most cases this is fine, although it may lead to more specific
>> > wildcards than strictly necessary. However, in a particular case with
>> > connection tracking actions it could lead to the wrong behaviour.
>> >
>> > Unlike most OpenFlow fields, the ct_{mark,labels} fields are typically
>> > unknown until the ct(...,recirc_table=N,...) action is executed however
>> > the packet may actually belong to a connection which has a nonzero value
>> > for one of these fields. This can lead to the wrong behaviour with flows
>> > such as the following:
>> >
>> > in_port=1,ip,actions=ct(commit,exec(set_field(0x1/0x1->ct_mark))),2
>> > in_port=2,ip,actions=ct(commit,exec(set_field(0x2/0x2->ct_mark))),1
>> >
>> > Connections flowing through these actions will always update the ct_mark
>> > field stored within the conntrack table. However, rather than modifying
>> > only the specified bits (0x1 in one direction, 0x2 in the other), the
>> > entire ct_mark field will be replaced. Such connections will constantly
>> > toggle the value of ct_mark between 0x1 and 0x2, rather than becoming
>> > 0x3 and keeping that value.
>> >
>> > This commit fixes the issue by ensuring that set_field actions only
>> > modify the modified bits in the wildcards, rather than masking the
>> > entire field.
>> >
>> > Fixes: 8e53fe8cf7a1 ("Add connection tracking mark support.")
>> > Fixes: 9daf23484fb1 ("Add connection tracking label support.")
>> > Signed-off-by: Joe Stringer 
>>
>> A couple of questions came up around this offline, which I looked into:
>>
>> * How does this work for OpenFlow versions that don't support masked 
>> set_field?
>
> Is this a real issue?  Open vSwitch effectively supports masked
> set_field on older versions of OpenFlow by encoding them as an
> equivalent series of one or more NXAST_REG_LOAD operations.

Not really; I think the concern was more: if a set_field mask is not
expressible in the OF protocol version, then is the mask populated
correctly with this change - ie full mask? (Yes, it is).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] ipv6 tunneling support question for branch-2.5

[Richukov Kes]

Hi, 

We are researching with IPv6 tunneling support on openvswitch. We can only use 
a released version of openvswitch (not the experimental
master branch). We saw that IPv6 tunnel creation fails in branch-2.5, which 
dates back to a commit and log suggests that there were some issues
that could not be debugged before 2.5 release and hence functionality was 
disabled before release. Can anyone tell us what patches are needed
to make IPv6 tunneling work on branch-2.5? If these patches are compatible, can 
they be ported into branch-2.5 by any chance?

Also, is there a dependency on the kernel that we should be using? Will Centos 
6.6 or Centos 7.1 work?

Thanking you in advance,
Richukov Kes 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Traynor, Kevin]

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Panu Matilainen
> Sent: Wednesday, April 13, 2016 8:50 AM
> To: Weglicki, MichalX ; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for
> DPDK 16.04.
> 

[snip]

> As an aside, I've been thinking maybe this is a case where OVS could
> support both DPDK 2.2 and 16.04. I know its unprecedented but maybe that
> could change, restricting OVS to just one DPDK version seems
> unnecessarily strict when talking about differences this trivial.

Judging by the ML, it's more commonly requested to use the current release
of DPDK with the last release of OVS e.g. OVS 2.5 and DPDK 16.04, than people
wanting OVS master with DPDK X-1.

Even for a trivial case like above - it would be ok now to add support for DPDK 
X-1
but if we then add OVS code to take advantage of new DPDK X features (e.g. vhost
pmd) we'll end up with messy code. Also testing efforts would increase 
(double?),
so I don't think we would get it without a cost.

Kevin.

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

Re: [ovs-dev] [PATCH v11 2/8] util: Add a path canonicalizer

[Aaron Conole]

Hi Ben,

I have rebased (due to conflicts) the series and am set to resubmit;
however I'd like to get closure on this issue before I do.

I have no strong feelings either way - I can manually scan the string
for .. or use the realpath code I've put here. Since this only happens
at initialization time, I figured it didn't matter how heavy it was,
but that may not be the case (especially due to where initialization
occurs).

Please advise what you would rather see - I can make the changes and test
with them.

Thanks :-)
-Aaron

Aaron Conole  writes:

> Hi Ben,
>
> Ben Pfaff  writes:
>
>> (Yow, that's a lot of CCs.)
>
> Lots of cooks in the kitchen on this one.
>
>> On Fri, Apr 01, 2016 at 11:31:31AM -0400, Aaron Conole wrote:
>>> This commit adds a new function (ovs_realpath) to perform the role of
>>> realpath on various operating systems. The purpose is to ensure that a
>>> given path to file exists, and to return a completely resolved path (sans
>>> '.' and '..').
>>> 
>>> Signed-off-by: Aaron Conole 
>>
>> Path canonicalization is a pretty big hammer.  In other cases where OVS
>> relies on an absolute path, it uses textual comparison on a prefix of
>> the name (representing a directory) and rejects use of ".." following
>> the prefix.  This is pretty easy to get right, and it is not as
>> heavyweight, and it does not have to actually do file system operations
>> (stat, readlink, ...), and its verdict can't change as a result of
>> changes to the file system (e.g. new or modified or deleted symlinks,
>> NFS servers that are down), and so on.
>>
>> Do you think we really need path canonicalization?
>
> I was nervous about a user putting escapes in the code. Unlike with,
> say, vhost user filenames (where we just blanket deny '/' because the
> semantic is of a file) this is not a file specification, but a directory
> specification. That implies that we would have to keep state and test
> for /../, and ../ (at the beginning of the string), at the least.
>
> If you think it's safe to merely test for the presence of these and
> bail, I'm okay with it, but I didn't want to leave any possibility that
> a malicious DB user could escape out of the rundir when changing the
> vhost-socket dir.
>
> I do agree it's heavy.
>
>> Thanks,
>
> Thanks for the review!
>
>> Ben.
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] tunneling: Fix for concomitant IPv4 and IPv6 tunnels

[Ryan Moats]

> -Original Message-
> From patchwork Fri Apr  1 13:06:05 2016
> Content-Type: text/plain; charset="utf-8"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Subject: [ovs-dev] tunneling: Fix for concomitant IPv4 and IPv6 tunnels
> From: Thadeu Lima de Souza Cascardo 
> X-Patchwork-Id: 604801
> Message-Id: <1459515965-31727-1-git-send-email-casca...@redhat.com>
> To: dev@openvswitch.org
> Date: Fri,  1 Apr 2016 10:06:05 -0300
>
> When using an IPv6 tunnel on the same bridge as an IPv4 tunnel, the flow
> received from the IPv6 tunnel would have an IPv4 address added to it,
causing
> problems when trying to put or execute the action on Linux datapath.
>
> Clearing the IPv6 address when we have a valid IPv4 address fixes this
problem.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo 

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

[ovs-dev] ovn: Issue in ACL stage when the dhcp reply packet is resumed

[Numan Siddique]

Hi,

I am seeing an issue while testing the ovn native dhcp implementation.

When the dhcp reply packet from ovn-controller is resumed by ovs-vswitchd,
the packet is getting dropped in the egress pipeline - ACL stage.

I see the below messages in the vswitchd.log

--
2016-04-13T14:14:03.813Z|2|ofproto_dpif_upcall(handler1)|INFO|received
packet on unassociated datapath port 0
2016-04-13T14:14:03.860Z|1|ofproto_dpif_upcall(revalidator2)|WARN|Failed
to acquire udpif_key corresponding to unexpected flow (Invalid argument):
ufid:a33e557f-369f-404e-aefb-2e3da678b09a



In the function upcall_receive(), I could see that xlate_lookup() is
returning error.

Also I could see that in the function odp_flow_key_to_flow__()
L5191,  nl_attr_get_odp_port(attrs[OVS_KEY_ATTR_IN_PORT]) is returning 0.


Please let me know if you need further information.

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

[ovs-dev] Mail System Error - Returned Mail

[Mail Administrator]

This Message was undeliverable due to the following reason:

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

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

Your message was not delivered within 8 days:
Host 38.133.241.234 is not responding.

The following recipients did not receive this message:


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



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

[ovs-dev] [PATCH v1 2/2] netdev-dpdk.c: Add ingress-policing functionality.

[Ian Stokes]

This patch provides the modifications required in netdev-dpdk.c and
vswitch.xml to enable ingress policing for DPDK interfaces.

This patch implements the necessary netdev functions to netdev-dpdk.c as
well as various helper functions required for ingress policing.

The vswitch.xml has been modified to explain the expected parameters and
behaviour when using ingress policing.

The INSTALL.DPDK.md guide has been modified to provide an example
configuration of ingress policing.

Signed-off-by: Ian Stokes 
---

v1:

*INSTALL.DPDK.md
- Add ingress_policing usage example.

*NEWS
- Add support for ingress_policing to DPDK section.

*netdev-dpdk.c
- Add ingress_policer struct.
- Modify struct netdev-dpdk to include a ovsrcu pointer to an ingress
  policer.
- Modify netdev_dpdk_init() to initialise ingress policer to null.
- Add function 'ingress_policer_run()'to process packets with a given
  netdev, rte_mbuf and packet count.
- Modified 'netdev_dpdk_vhost_update_rx_counters()' to accept new
  parameter int dropped representing number of packets dropped.
- Modified 'netdev_dpdk_vhost_update_rx_counters()' to update number of
  dropped packets in stats->rx_dropped.
- Modified 'netdev_dpdk_vhost_rxq_recv()' to check and call
  ingress_policing functionality with 'ingress_policer_run()'.
- Modified 'netdev_dpdk_rxq_recv()' to check and call
  ingress_policing functionality with 'ingress_policer_run()'.
- Modified 'netdev_dpdk_rxq_recv()' to update rx dropped stats.
- Modified 'netdev_dpdk_vhost_get_stats()' to add support for rx_dropped.
- Add function 'netdev_dpdk_policer_construct__()' to create an
  ingress_policer.
- Add function 'netdev_dpdk_policer_destruct__()' to destroy an ingress
  policer.
- Add function 'netdev_dpdk_set_policing()' responsible for calling
  helper functions to create and destroy an ingress policer for the
  device.
- Add function 'netdev_dpdk_get_ingress_policer()' to return a pointer
  to the ingress_policer using the ovsrcu_get function.

*vswitch.xml
- Modify ingress policing section to include support in OVS with DPDK.
---
 INSTALL.DPDK.md  |   19 ++
 NEWS |1 +
 lib/netdev-dpdk.c|  173 --
 vswitchd/vswitch.xml |4 +-
 4 files changed, 190 insertions(+), 7 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 9ec8bf6..df2c6cd 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -227,6 +227,25 @@ Using the DPDK with ovs-vswitchd:
For more details regarding egress-policer parameters please refer to the
vswitch.xml.
 
+9. Ingress Policing Example
+
+   Assuming you have a vhost-user port receiving traffic consisting of
+   packets of size 64 bytes, the following command would limit the reception
+   rate of the port to ~1,000,000 packets per second:
+
+   `ovs-vsctl set interface vhost-user0 ingress_policing_rate=368000
+ingress_policing_burst=1000`
+
+   To examine the ingress policer configuration of the port:
+
+   `ovs-vsctl list interface vhost-user0`
+
+   To clear the ingress policer configuration from the port use the following:
+
+   `ovs-vsctl set interface vhost-user0 ingress_policing_rate=0`
+
+   For more details regarding ingress-policer see the vswitch.xml.
+
 Performance Tuning:
 ---
 
diff --git a/NEWS b/NEWS
index ea7f3a1..d0283fa 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.5.0
assignment.
  * Type of log messages from PMD threads changed from INFO to DBG.
  * QoS functionality with sample egress-policer implementation.
+ * Add ingress policing functionality.
- ovs-benchmark: This utility has been removed due to lack of use and
  bitrot.
- ovs-appctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index fe9c9cb..2d955d3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -297,6 +297,12 @@ struct dpdk_ring {
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
 
+struct ingress_policer {
+struct rte_meter_srtcm_params app_srtcm_params;
+struct rte_meter_srtcm in_policer;
+rte_spinlock_t policer_lock;
+};
+
 struct netdev_dpdk {
 struct netdev up;
 int port_id;
@@ -342,6 +348,10 @@ struct netdev_dpdk {
 struct qos_conf *qos_conf;
 rte_spinlock_t qos_lock;
 
+/* Ingress Policer */
+OVSRCU_TYPE(struct ingress_policer *) ingress_policer;
+uint32_t policer_rate;
+uint32_t policer_burst;
 };
 
 struct netdev_rxq_dpdk {
@@ -355,6 +365,9 @@ static int netdev_dpdk_construct(struct netdev *);
 
 struct virtio_net * netdev_dpdk_get_virtio(const struct netdev_dpdk *dev);
 
+struct ingress_policer *
+netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
+
 static bool
 is_dpdk_class(const struct netdev_class *class)
 {
@@ -721,6 +734,11 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int 
port_no,
 dev->qos_conf = NULL;
 rte_spinlock_init(&dev->qos_lock);
 
+/* Initialise rcu pointer for ingress policer to NULL */
+ovsrcu_init

[ovs-dev] [PATCH v1 1/2] netdev-dpdk.c: Add generic policer functions.

[Ian Stokes]

Add generic policer functions to avoid code duplication.

Policing can be implemented on both egress and ingress paths.
Currently the QoS egress-policer implementation uses it's own specific run
and packet handle policer functions. This patch makes the policer functions
generic so that they can be used regardless of whether the policer is egress
or ingress by just requiring a pointer to the rte_meter used for policing
to be passed.

Signed-off-by: Ian Stokes 
---

v1:

*netdev-dpdk.c
- Add netdev_dpdk_policer_pkt_handle__() function.
- Add netdev_dpdk_policer_run__() function.
- Modify egress_policer_run() to call netdev_dpdk_policer_run__().
---
 lib/netdev-dpdk.c |   65 ++--
 1 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c7217ea..fe9c9cb 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1117,6 +1117,41 @@ dpdk_queue_flush(struct netdev_dpdk *dev, int qid)
 dpdk_queue_flush__(dev, qid);
 }
 
+static inline bool
+netdev_dpdk_policer_pkt_handle__(struct rte_meter_srtcm *meter,
+struct rte_mbuf *pkt, uint64_t time)
+{
+uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct ether_hdr);
+
+return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) ==
+e_RTE_METER_GREEN;
+}
+
+static int
+netdev_dpdk_policer_run__(struct rte_meter_srtcm *meter,
+struct rte_mbuf **pkts, int pkt_cnt)
+{
+int i = 0;
+int cnt = 0;
+struct rte_mbuf *pkt = NULL;
+uint64_t current_time = rte_rdtsc();
+
+for (i = 0; i < pkt_cnt; i++) {
+pkt = pkts[i];
+/* Handle current packet */
+if (netdev_dpdk_policer_pkt_handle__(meter, pkt, current_time)) {
+if (cnt != i) {
+pkts[cnt] = pkt;
+}
+cnt++;
+} else {
+rte_pktmbuf_free(pkt);
+}
+}
+
+return cnt;
+}
+
 static bool
 is_vhost_running(struct virtio_net *virtio_dev)
 {
@@ -2611,39 +2646,13 @@ egress_policer_qos_set(struct netdev *netdev, const 
struct smap *details)
 return err;
 }
 
-static inline bool
-egress_policer_pkt_handle__(struct rte_meter_srtcm *meter,
-struct rte_mbuf *pkt, uint64_t time)
-{
-uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct ether_hdr);
-
-return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) ==
-e_RTE_METER_GREEN;
-}
-
 static int
-egress_policer_run(struct netdev *netdev, struct rte_mbuf **pkts,
-int pkt_cnt)
+egress_policer_run(struct netdev *netdev, struct rte_mbuf **pkts, int pkt_cnt)
 {
-int i = 0;
 int cnt = 0;
 struct egress_policer *policer = egress_policer_get__(netdev);
-struct rte_mbuf *pkt = NULL;
-uint64_t current_time = rte_rdtsc();
 
-for(i = 0; i < pkt_cnt; i++) {
-pkt = pkts[i];
-/* Handle current packet */
-if (egress_policer_pkt_handle__(&policer->egress_meter, pkt,
-current_time)) {
-if (cnt != i) {
-pkts[cnt] = pkt;
-}
-cnt++;
-} else {
-rte_pktmbuf_free(pkt);
-}
-}
+cnt = netdev_dpdk_policer_run__(&policer->egress_meter, pkts, pkt_cnt);
 
 return cnt;
 }
-- 
1.7.4.1

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

[ovs-dev] Add ingress policer functionality to OVS with DPDK.

[Ian Stokes]

This patchset adds generic policer functions and ingress_policing functionality 
to OVS with DPDK.

Patch 1 introduces generic policer functions to avoid code duplication when 
processing either ingress or egress traffic with a policer.
Patch 2 adds ingress policing functionality and details to OVS with DPDK. This 
patch depends on the changes introduced in patch 1.

[PATCH 1/2] netdev-dpdk: Add generic policer functions.
[PATCH 2/2] netdev-dpdk.c: Add ingress-policing functionality.

 INSTALL.DPDK.md  |  19 ++
 NEWS |   1 +
 lib/netdev-dpdk.c| 233 
+++-
 vswitchd/vswitch.xml |   4 +--
 4 files changed, 222 insertions(+), 35 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] tunneling: Improving tunneling performance using DPDK Rx checksum offloading feature.

[Sugesh Chandran]

Optimizing tunneling performance in userspace datapath by offloading the
rx checksum validation on tunnel packets to the NIC when it is supported.

This patch improves the bidirectional VxLAN tunneling performance by 8% and
decapsulation performance by 24%. However it introduces 1% performance drop in
PHY-PHY case due to the overhead of validating invalid checksum flag reported
by NIC.

Signed-off-by: Sugesh Chandran 
---
 lib/dpif-netdev.c | 40 
 lib/netdev-dpdk.c | 39 +--
 lib/netdev-provider.h |  3 +++
 lib/netdev-vport.c| 25 +++--
 lib/netdev.c  |  1 +
 lib/netdev.h  |  5 +
 lib/packets.h |  1 +
 7 files changed, 82 insertions(+), 32 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 2870951..5de5c6a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -70,6 +70,7 @@
 #include "util.h"
 
 #include "openvswitch/vlog.h"
+#include "netdev-provider.h"
 
 VLOG_DEFINE_THIS_MODULE(dpif_netdev);
 
@@ -479,7 +480,8 @@ static void dp_netdev_execute_actions(struct 
dp_netdev_pmd_thread *pmd,
   const struct nlattr *actions,
   size_t actions_len);
 static void dp_netdev_input(struct dp_netdev_pmd_thread *,
-struct dp_packet **, int cnt, odp_port_t port_no);
+  struct dp_packet **, int cnt,
+  struct dp_netdev_port *port);
 static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *,
   struct dp_packet **, int cnt);
 
@@ -2572,7 +2574,7 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread 
*pmd,
 *recirc_depth_get() = 0;
 
 cycles_count_start(pmd);
-dp_netdev_input(pmd, packets, cnt, port->port_no);
+dp_netdev_input(pmd, packets, cnt, port);
 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);
@@ -3394,6 +3396,20 @@ dp_netdev_queue_batches(struct dp_packet *pkt,
 packet_batch_update(batch, pkt, mf);
 }
 
+static inline bool
+is_checksum_valid(struct dp_packet *packet) {
+#ifdef DPDK_NETDEV
+if (packet->mbuf.ol_flags & (PKT_RX_IP_CKSUM_BAD |
+ PKT_RX_L4_CKSUM_BAD)) {
+return 0;
+}
+packet->md.ol_flags = NETDEV_RX_CHECKSUM_OFFLOAD;
+return 1;
+#else
+return 0;
+#endif
+}
+
 /* Try to process all ('cnt') the 'packets' using only the exact match cache
  * 'pmd->flow_cache'. If a flow is not found for a packet 'packets[i]', the
  * miniflow is copied into 'keys' and the packet pointer is moved at the
@@ -3409,11 +3425,13 @@ static inline size_t
 emc_processing(struct dp_netdev_pmd_thread *pmd, struct dp_packet **packets,
size_t cnt, struct netdev_flow_key *keys,
struct packet_batch batches[], size_t *n_batches,
-   bool md_is_valid, odp_port_t port_no)
+   bool md_is_valid, struct dp_netdev_port *port)
 {
 struct emc_cache *flow_cache = &pmd->flow_cache;
 struct netdev_flow_key *key = &keys[0];
 size_t i, n_missed = 0, n_dropped = 0;
+bool rx_cksm_ol_enable = port && (port->netdev->ol_flags &
+NETDEV_RX_CHECKSUM_OFFLOAD);
 
 for (i = 0; i < cnt; i++) {
 struct dp_netdev_flow *flow;
@@ -3425,6 +3443,12 @@ emc_processing(struct dp_netdev_pmd_thread *pmd, struct 
dp_packet **packets,
 continue;
 }
 
+if (OVS_UNLIKELY(rx_cksm_ol_enable && !is_checksum_valid(packet))) {
+dp_packet_delete(packet);
+n_dropped++;
+continue;
+}
+
 if (i != cnt - 1) {
 /* Prefetch next packet data and metadata. */
 OVS_PREFETCH(dp_packet_data(packets[i+1]));
@@ -3432,7 +3456,7 @@ emc_processing(struct dp_netdev_pmd_thread *pmd, struct 
dp_packet **packets,
 }
 
 if (!md_is_valid) {
-pkt_metadata_init(&packet->md, port_no);
+pkt_metadata_init(&packet->md, port->port_no);
 }
 miniflow_extract(packet, &key->mf);
 key->len = 0; /* Not computed yet. */
@@ -3606,7 +3630,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
 static void
 dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
   struct dp_packet **packets, int cnt,
-  bool md_is_valid, odp_port_t port_no)
+  bool md_is_valid, struct dp_netdev_port *port)
 {
 #if !defined(__CHECKER__) && !defined(_WIN32)
 const size_t PKT_ARRAY_SIZE = cnt;
@@ -3621,7 +3645,7 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
 
 n_batches = 0;
 newcnt = emc_processing(pmd, packets, cnt, keys, batches, &n_batches,
-md_is_valid, port_no);
+

Re: [ovs-dev] [PATCH] tunneling: Improving VxLAN tunneling performance using DPDK Rx checksum offloading feature.

[Chandran, Sugesh]

Hi Jesse,
Thank you for looking into the patch. I will send out v2 patch after 
incorporating your comments.

Regards
_Sugesh

> -Original Message-
> From: Jesse Gross [mailto:je...@kernel.org]
> Sent: Monday, April 11, 2016 5:33 PM
> To: Chandran, Sugesh 
> Cc: ovs dev 
> Subject: Re: [ovs-dev] [PATCH] tunneling: Improving VxLAN tunneling
> performance using DPDK Rx checksum offloading feature.
> 
> On Mon, Apr 11, 2016 at 2:52 AM, Sugesh Chandran
>  wrote:
> > Optimizing VxLAN tunneling performance in userspace datapath by
> > offloading the rx checksum validation on tunnel packets to the NIC when it
> is supported.
> >
> > Signed-off-by: Sugesh Chandran 
> 
> Thanks for working on this. I think there is a lot of room for OVS DPDK to 
> take
> advantage of common NIC offloads.
> 
> I have some high level comments:
> 
> * I noticed that you've been following up you patches with a second message
> with performance numbers. I think it would be better to just include this in
> the patch description since that's what will become the commit message.
> Even if the numbers might change over time, it's useful to have a reference
> to what the expected performance benefit was at the time the patch was
> authored. Related to this, it would also be useful to have more context on
> the benchmark numbers that you are providing. For example, I'm guessing
> that the test was run using VXLAN with outer UDP checksums? What NIC was
> used?
[Sugesh] Yes, its VxLAN with outer UDP checksums. I have used Intel Niantic 10G 
NICs.
The test setup is as follows
NIC <--> OVS<-->NIC

64 Byte UDP stream (14 million) -->
114 Byte VxLAN stream(9.3 Million) <--

Results :
(Unidirectional, on DECAP side)
6.53 Million(Before Patch)
8.17 Million(After Patch)

Bidirectional
4.14 Million (Before patch, on each side)
4.43 Million(After the patch, on each side)


> 
> * I think we need to have a clearer way to pass around checksum state rather
> than as an argument, it's just too invasive. A natural place that most OSs use
> is in metadata associate with the packet itself, which would be struct
> dp_packet in this case. It's also important to consider how this will be
> expanded over time. For example, offloading of transmit checksums would
> likely want to use the same fields.
> 
> * Please find a way to do this without duplicating the tunnel header
> extraction code. Having two copies will surely result in bugs over time.
> Potentially the previous point may help with this.
> 
> * Please try to generalize your code where possible. In this case it looks 
> like
> IPv6, Geneve, and GRE (at least the IP checksum) could easily be supported
> using the same mechanism.
[Sugesh] Modified the code to use the packet metadata and same header 
extraction functions. However we notice 1-2% of less in performance improvement
due to the overhead of packet metadata handling  and validations in the 
extraction code.
And also in both case we noticed a~1% of performance drop in PHY-PHY 
bidirectional test.
This is because of the new invalid checksum flag check on every packet 
from the NIC.


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

[ovs-dev] [PATCH v3] netdev-dpdk: add hotplug support

[Mauricio Vasquez B]

In order to use dpdk ports in ovs they have to be bound to a DPDK
compatible driver before ovs is started.

This patch adds the possibility to hotplug (or hot-unplug) a device
after ovs has been started. The implementation adds an appctl command:
netdev-dpdk/port-clt

After the user attaches a new device, it has to be added to a bridge
using the to use the add-port command, similarly, before detaching a
device, it has to be removed using the del-port command.

Signed-off-by: Mauricio Vasquez B 
---
 INSTALL.DPDK.md   |  26 +-
 NEWS  |   1 +
 lib/netdev-dpdk.c | 102 ++
 3 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 9ec8bf6..4095402 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -81,7 +81,7 @@ Using the DPDK with ovs-vswitchd:
 
 1. Setup system boot
Add the following options to the kernel bootline:
-   
+
`default_hugepagesz=1GB hugepagesz=1G hugepages=1`
 
 2. Setup DPDK devices:
@@ -227,6 +227,29 @@ Using the DPDK with ovs-vswitchd:
For more details regarding egress-policer parameters please refer to the
vswitch.xml.
 
+9. Port Hotplug
+
+   ovs supports port hotplugging, it allows to use ports that were not bound
+   to DPDK when vswitchd was started.
+   In order to attach a port, it has to be bound to DPDK using the
+   dpdk_nic_bind.py script:
+
+   `$DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio :01:00.0`
+
+   Then it can be attached to OVS:
+
+   `ovs-appctl netdev-dpdk/port-ctl attach :01:00.0`
+
+   At this point, the user can create a ovs port using the add-port command.
+
+   It is also possible to detach a port from ovs, the user has to remove the
+   port using the del-port command, then it can be detached using:
+
+   `ovs-appctl netdev-dpdk/port-ctl detach dpdk0`
+
+   This feature is not supported by all the NICs, please refer to the
+   [DPDK Port Hotplug Framework] in order to get more information.
+
 Performance Tuning:
 ---
 
@@ -959,3 +982,4 @@ Please report problems to b...@openvswitch.org.
 [INSTALL.md]:INSTALL.md
 [DPDK Linux GSG]: 
http://www.dpdk.org/doc/guides/linux_gsg/build_dpdk.html#binding-and-unbinding-network-ports-to-from-the-igb-uioor-vfio-modules
 [DPDK Docs]: http://dpdk.org/doc
+[DPDK Port Hotplug Framework]: 
http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html
diff --git a/NEWS b/NEWS
index ea7f3a1..2ba8659 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.5.0
assignment.
  * Type of log messages from PMD threads changed from INFO to DBG.
  * QoS functionality with sample egress-policer implementation.
+ * Port Hotplug is now supported.
- ovs-benchmark: This utility has been removed due to lack of use and
  bitrot.
- ovs-appctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..cd98a2b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -599,7 +599,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) 
OVS_REQUIRES(dpdk_mutex)
 int diag;
 int n_rxq, n_txq;
 
-if (dev->port_id < 0 || dev->port_id >= rte_eth_dev_count()) {
+if (!rte_eth_dev_is_valid_port(dev->port_id)) {
 return ENODEV;
 }
 
@@ -1985,6 +1985,89 @@ netdev_dpdk_set_admin_state(struct unixctl_conn *conn, 
int argc,
 unixctl_command_reply(conn, "OK");
 }
 
+static int
+dpdk_port_attach(const char * port, char * response, size_t size)
+OVS_REQUIRES(dpdk_mutex)
+{
+int ret;
+uint8_t port_id;
+
+ret = rte_eth_dev_attach(port, &port_id);
+if (ret < 0) {
+snprintf(response, size, "Error attaching device '%s'", port);
+return -1;
+}
+
+snprintf(response, size,
+ "Device '%s' has been attached as 'dpdk%d'", port, port_id);
+return 0;
+}
+
+static int
+dpdk_port_detach(const char * port, char * response, size_t size)
+OVS_REQUIRES(dpdk_mutex)
+{
+int ret;
+unsigned int parsed_port;
+uint8_t port_id;
+char devname[RTE_ETH_NAME_MAX_LEN];
+
+ret = dpdk_dev_parse_name(port, "dpdk", &parsed_port);
+if (ret) {
+snprintf(response, size, "'%s' is not a valid dpdk device", port);
+return -1;
+}
+
+port_id = parsed_port;
+
+struct netdev *netdev = netdev_from_name(port);
+if (netdev) {
+netdev_close(netdev);
+snprintf(response, size,
+ "Port '%s' is being used. Remove it before detaching", port);
+return -1;
+}
+
+rte_eth_dev_close(port_id);
+
+ret = rte_eth_dev_detach(port_id, devname);
+if (ret < 0) {
+snprintf(response, size, "Port '%s' can not be detached", port);
+return -1;
+}
+
+snprintf(response, size, "Port '%s' has been detached", port);
+return 0;
+}
+
+static void
+netdev_dpdk_port_ctl(struct unixctl_conn *conn, int argc OVS_UNUSED,
+ const char *argv[], void *aux OVS_UNUSED)
+{
+int ret;
+char response[512]

[ovs-dev] [PATCH v14 1/6] More updates to ovn test output

[Ryan Moats]

Adding more detail that helps find what went wrong.

Signed-off-by: Ryan Moats 
---
 tests/ovn.at | 8 
 1 file changed, 8 insertions(+)

diff --git a/tests/ovn.at b/tests/ovn.at
index 22121e1..786479f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1177,14 +1177,17 @@ ovn-sbctl show
 
 echo "-- hv1 dump --"
 as hv1 ovs-vsctl show
+as hv1 ovs-ofctl show br-int
 as hv1 ovs-ofctl -O OpenFlow13 dump-flows br-int
 
 echo "-- hv2 dump --"
 as hv2 ovs-vsctl show
+as hv2 ovs-ofctl show br-int
 as hv2 ovs-ofctl -O OpenFlow13 dump-flows br-int
 
 echo "-- hv3 dump --"
 as hv3 ovs-vsctl show
+as hv3 ovs-ofctl show br-int
 as hv3 ovs-ofctl -O OpenFlow13 dump-flows br-int
 
 # Now check the packets actually received against the ones expected.
@@ -1942,14 +1945,17 @@ ovn-sbctl dump-flows -- list multicast_group
 
 echo "-- hv1 dump --"
 as hv1 ovs-vsctl show
+as hv1 ovs-ofctl show br-int
 as hv1 ovs-ofctl -O OpenFlow13 dump-flows br-int
 
 echo "-- hv2 dump --"
 as hv2 ovs-vsctl show
+as hv2 ovs-ofctl show br-int
 as hv2 ovs-ofctl -O OpenFlow13 dump-flows br-int
 
 echo "-- hv3 dump --"
 as hv3 ovs-vsctl show
+as hv3 ovs-ofctl show br-int
 as hv3 ovs-ofctl -O OpenFlow13 dump-flows br-int
 
 # Now check the packets actually received against the ones expected.
@@ -2107,8 +2113,10 @@ ovn-sbctl list port_binding
 echo "-"
 
 echo "-- hv1 dump --"
+as hv1 ovs-ofctl show br-int
 as hv1 ovs-ofctl dump-flows br-int
 echo "-- hv2 dump --"
+as hv2 ovs-ofctl show br-int
 as hv2 ovs-ofctl dump-flows br-int
 
 # Packet to Expect
-- 
1.9.1

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

[ovs-dev] [PATCH v14 2/6] Change encaps_run to work incrementally

[Ryan Moats]

As a side effect, tunnel context is persisted.

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

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

[ovs-dev] [PATCH v14 3/6] Convert binding_run to incremental processing.

[Ryan Moats]

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

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

Signed-off-by: Ryan Moats 
---
 ovn/controller/binding.c| 140 +---
 ovn/controller/binding.h|   1 +
 ovn/controller/encaps.c |   7 +-
 ovn/controller/ovn-controller.c |  26 ++--
 ovn/controller/ovn-controller.h |   2 +
 ovn/controller/patch.c  |   3 +-
 6 files changed, 117 insertions(+), 62 deletions(-)

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

[ovs-dev] [PATCH v14 0/6] Incremental Processing

[Ryan Moats]

Prior versions of the patch set used change tracking
incorrectly - this change set fixes that, but it
results in larger patchsets, as persisting structures
and adding incremental processing end up combined
into the same patch.

Specific processing steps havs been extracted into
helper methods to minimize duplicating code between
the full and incremental processing paths.

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

 ovn/controller/binding.c| 143 +--
 ovn/controller/binding.h|   1 +
 ovn/controller/encaps.c | 163 ---
 ovn/controller/lflow.c  | 191 ++---
 ovn/controller/lflow.h  |   6 +-
 ovn/controller/lport.c  | 216 --
 ovn/controller/lport.h  |  22 +-
 ovn/controller/ofctrl.c | 258 
 ovn/controller/ofctrl.h |  18 +-
 ovn/controller/ovn-controller.c |  53 +--
 ovn/controller/ovn-controller.h |   2 +
 ovn/controller/patch.c  |   7 +-
 ovn/controller/physical.c   | 911 ++--
 ovn/controller/physical.h   |   4 +-
 tests/ovn.at|   8 +
 15 files changed, 1306 insertions(+), 697 deletions(-)

-- 
1.9.1

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

[ovs-dev] [PATCH v14 6/6] Add incremental proessing to lflow_run and physical_run

[Ryan Moats]

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

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

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

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

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

[ovs-dev] [PATCH v14 5/6] Persist ovn flow tables.

[Ryan Moats]

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

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

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

[ovs-dev] [PATCH v14 4/6] Persist lport_index and mcgroup_index structures

[Ryan Moats]

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

Signed-off-by: Ryan Moats 
---
 ovn/controller/lport.c  | 216 +---
 ovn/controller/lport.h  |  22 +++-
 ovn/controller/ovn-controller.c |  14 +--
 3 files changed, 208 insertions(+), 44 deletions(-)

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

[ovs-dev] You can use it to make cash anytime you need it.

[dev]

Good day!

We considered your resume to be very attractive and we thought the vacant 
position in our company could be interesting for you.

We cooperate with different countries and currently we have many clients in the 
world.Part-time and full-time employment are both currently important.

We offer a flat wage from ~ $2000 up to ~ $6000 per month.

The job offers a good salary so, interested candidates please registration on 
the 
our site

Respectively submitted
Personnel department
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC] netdev-dpdk: Expand RSS hash calculation by MAC addresses

[Wojciechowicz, RobertX]

Hi,

> -Original Message-
> From: Jesse Gross [mailto:je...@kernel.org]
> Sent: Friday, March 25, 2016 7:57 PM
> To: Wojciechowicz, RobertX 
> Cc: ovs dev 
> Subject: Re: [ovs-dev] [PATCH RFC] netdev-dpdk: Expand RSS hash
> calculation by MAC addresses
> 
> On Fri, Mar 25, 2016 at 7:45 AM, Wojciechowicz, RobertX
>  wrote:
> > Hi Jesse,
> >
> > please find my answers inline.
> >
> >> -Original Message-
> >> From: Jesse Gross [mailto:je...@kernel.org]
> >> Sent: Thursday, March 24, 2016 11:20 PM
> >> To: Wojciechowicz, RobertX 
> >> Cc: ovs dev 
> >> Subject: Re: [ovs-dev] [PATCH RFC] netdev-dpdk: Expand RSS hash
> >> calculation by MAC addresses
> >>
> >> On Thu, Mar 24, 2016 at 7:30 AM, Robert Wojciechowicz
> >>  wrote:
> >> > This patch configures RSS hash filter in DPDK in order to include
> >> > source and destination MAC addresses into RSS hash calculation.
> >> > This configuration considerably improves performance for flows
> >> > with overlapping IP address spaces and non-overlapping Ethernet
> >> > address spaces.
> >> > Here is the example test result (throughput in frames per second)
> >> > I have done for 16 flows (fixed IP addresses and changing source
> >> > MAC addresses) on X710 for 10GbE SFP+:
> >> >
> >> > 1. original hash calculation:4870667 fps
> >> > 2. expanded hash calculation (with this patch): 12740258 fps
> >> >
> >> > Signed-off-by: Robert Wojciechowicz
> 
> >>
> >> This is somewhat of an unusual configuration - I guess as evidenced by
> >> the fact that it is not supported by ixgbe.
> >>
> >> How much of a difference does this make in non-synthetic benchmarks? I
> >> would expect in most cases that the use of L4 ports would provide
> >> sufficient entropy even in cases where there are overlapping IP
> >> addresses.
> >>
> >
> > In most cases you're probably right, but there are use cases where
> > this overlapping exists. For example in openstack configurations
> > with many tenants.
> > The idea of this patch is to make this configuration possible
> > for people who really need it.
> 
> I understand that that there can be configurations where there are
> overlapping IP addresses, however, I'm not sure that really means that
> the L4 ports aren't able to still spread the load. Have you seen real
> world situations where this patch makes a difference in practice, as
> opposed to just in theory?
> 

We contacted our customer regarding this real world test, but unfortunately
we're still waiting for response. However synthetic benchmarks could be useful
also, e.g. for comparison with other vswitches.
I discussed this issue also with openstack team in more detail. 
It seems that overlapping ip and mac address spaces can happen for vlan 
or other tunnels they use a lot in openstack setups. So there is suggestion 
to expand this hash by tunnel id instead of mac addresses. 
I will try to test this scenario also.

> The reason why some of the RSS modes you are enabling here aren't on
> by default is usually because they can introduce reordering. So I'm
> not really eager to go down this path unless you can show that there
> are specific reasons why the assumptions behind the default
> configuration don't apply here.
> 

To be honest I'm not sure how expanding rss hash may introduce reordering.
My understanding is that hash filter is a mechanism to statistically distribute 
received 
packets into several receive queues. By default there is used 5-tuple (src_addr,
dst_addr, protocol, src_port, dst_port). This patch tries to expand
this tuple by mac or tunnel_id in order to reduce collisions for some traffic 
flows
and improve packets distribution into queues.
Could you please explain why it may introduce reordering?

> >> Two other concerns:
> >>  * This appears to enable hashing for a wide range of packets,
> >> including IP fragments, which would typically allow the possibility of
> >> reordering. I think we probably want to be a little more careful in
> >> considering what to include.
> >>  * I think we need to more gracefully handle NICs where this
> >> configuration is not supported. I don't like that we should what looks
> >> like an error message in normal conditions.
> >
> > Unfortunately DPDK returns EINVAL error also for unsupported operations,
> > so I couldn't handle it in more reasonable way. I wanted to make it
> > minimally invasive for other hardware than XL710, so I report all errors
> > as info messages.
> > Basically I'm not sure if there is some recommended approach
> > for providing functionality to OVS, which is hardware specific?
> > If there is any I could of course reimplement this patch appropriately.
> 
> I don't think that we have any hardware specific configuration in OVS.
> The best way to handle this is to fix DPDK or expose an API that
> abstracts out the hardware specific pieces.

Ok, so DPDK patch could be the next step in this work.

Br,
Robert
___
dev mailing list
dev@openvswi

Re: [ovs-dev] [PATCH] debian: Exclude 'control' from .gitignore

[Shmulik Ladkani]

Hi,

On Wed, 13 Apr 2016 08:37:21 +0300, shmulik.ladk...@ravellosystems.com wrote:
> debian/control is git controlled, no reason for it to be in the git
> ignore list.

Please ignore; Seems debian/control gets over-written when building
openvswitch-datapath-module using debian/rules.modules.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netdev-linux: Fix ingress policing burst rate configuration via tc

[Miguel Angel Ajo Pelayo]

I verified the 10% / 80% changes with netperf, it's unable to sustain 10Mbps
without at least a 8Mbit burst. I've seen recommendations from cisco to use
policing bursts of around 150 to 200%, but I guess that depends on the path
latency or even the implementation.

My netperf tests were in-host.



On Wed, Apr 13, 2016 at 12:44 PM, Miguel Angel Ajo  wrote:
> The tc_police structure was filled with a value calculated in bits
> instead of bytes while bytes were expected. This led the setting
> of an x8 higher burst value.
>
> Documentation and defaults have been corrected acordingly to minimize
> nuisances on users sticking to the defaults.
>
> The suggested burst value is now 80% of policing rate to make sure
> TCP works correctly.
>
> Signed-off-by: Miguel Angel Ajo 
> Tested-by: Miguel Angel Ajo 
> ---
>  FAQ.md   |  2 +-
>  lib/netdev-linux.c   | 14 --
>  vswitchd/vswitch.xml |  4 ++--
>  3 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/FAQ.md b/FAQ.md
> index 04ffc84..7dcdb4c 100644
> --- a/FAQ.md
> +++ b/FAQ.md
> @@ -1124,7 +1124,7 @@ A: A policing policy can be configured on an interface 
> to drop packets
> generate to 10Mbps:
>
> ovs-vsctl set interface vif1.0 ingress_policing_rate=1
> -   ovs-vsctl set interface vif1.0 ingress_policing_burst=1000
> +   ovs-vsctl set interface vif1.0 ingress_policing_burst=8000
>
> Traffic policing can interact poorly with some network protocols and
> can have surprising results.  The "Ingress Policing" section of
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index a7d7ac7..a2b6b8a 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2045,7 +2045,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>  int error;
>
>  kbits_burst = (!kbits_rate ? 0   /* Force to 0 if no rate specified. 
> */
> -   : !kbits_burst ? 1000 /* Default to 1000 kbits if 0. */
> +   : !kbits_burst ? 8000 /* Default to 8000 kbits if 0. */
> : kbits_burst);   /* Stick with user-specified value. 
> */
>
>  ovs_mutex_lock(&netdev->mutex);
> @@ -4720,21 +4720,15 @@ tc_add_policer(struct netdev *netdev,
>  tc_police.mtu = mtu;
>  tc_fill_rate(&tc_police.rate, ((uint64_t) kbits_rate * 1000)/8, mtu);
>
> -/* The following appears wrong in two ways:
> - *
> - * - tc_bytes_to_ticks() should take "bytes" as quantity for both of its
> - *   arguments (or at least consistently "bytes" as both or "bits" as
> - *   both), but this supplies bytes for the first argument and bits for 
> the
> - *   second.
> - *
> - * - In networking a kilobit is usually 1000 bits but this uses 1024 
> bits.
> +/* The following appears wrong in one way: In networking a kilobit is
> + * usually 1000 bits but this uses 1024 bits.
>   *
>   * However if you "fix" those problems then "tc filter show ..." shows
>   * "125000b", meaning 125,000 bits, when OVS configures it for 1000 kbit 
> ==
>   * 1,000,000 bits, whereas this actually ends up doing the right thing 
> from
>   * tc's point of view.  Whatever. */
>  tc_police.burst = tc_bytes_to_ticks(
> -tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024);
> +tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8);
>
>  tcmsg = tc_make_request(netdev, RTM_NEWTFILTER,
>  NLM_F_EXCL | NLM_F_CREATE, &request);
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 7d6976f..fca238d 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2434,7 +2434,7 @@
>
>
>  Maximum burst size for data received on this interface, in kb.  
> The
> -default burst size if set to 0 is 1000 kb.  This value
> +default burst size if set to 0 is 8000 kbit.  This value
>  has no effect if 
>  is 0.
>  
> @@ -2442,7 +2442,7 @@
>which is important for protocols like TCP that react severely to
>dropped packets.  The burst size should be at least the size of the
>interface's MTU.  Specifying a value that is numerically at least 
> as
> -  large as 10% of  helps TCP 
> come
> +  large as 80% of  helps TCP 
> come
>closer to achieving the full rate.
>  
>
> --
> 1.8.3.1
>
> This patch fixes the burst setting of the ingress policing on netdev-linux, 
> and
> corrects the documentation recommendations accordingly.
>
> Before the patch we had:
>
>
> # ovs-vsctl -- --may-exist add-port br-int test-port -- set Interface 
> test-port type=internal
> # ovs-vsctl set interface test-port ingress_policing_rate=1
> # ovs-vsctl set interface test-port ingress_policing_burst=1000
> # tc filter show dev test-port parent :
> filter protocol all pref 49 basic
> filter protocol all pref 49 basic handle 0x1
>  police 0x1 rate 1Kbit burst 1000Kb mt

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Panu Matilainen]

On 04/13/2016 11:38 AM, Panu Matilainen wrote:

On 04/13/2016 11:33 AM, Panu Matilainen wrote:

On 04/13/2016 11:16 AM, Weglicki, MichalX wrote:

-Original Message-
From: Panu Matilainen [mailto:pmati...@redhat.com]
Sent: Wednesday, April 13, 2016 8:50 AM
To: Weglicki, MichalX ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add
support for DPDK 16.04.

On 04/12/2016 05:05 PM, mweglicx wrote:
[...]

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..2295e53 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev
*netdev_,
   link = dev->link;
   ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
+if (link.link_duplex == ETH_LINK_AUTONEG) {


This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having
autonegotiation end up with half-duplex these days isn't that likely.

[MW] Thank you, your're right, just to be sure, you mean that I should
check autoneg through
link_autoneg flag, like this:
 if (link.link_autoneg) {
 *current = NETDEV_F_AUTONEG;
 } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
 if (link.link_speed == ETH_SPEED_NUM_10M) {
 *current = NETDEV_F_10MB_HD;
 }
 if (link.link_speed == ETH_SPEED_NUM_100M) {
 *current = NETDEV_F_100MB_HD;
 }
 if (link.link_speed == ETH_SPEED_NUM_1G) {
 *current = NETDEV_F_1GB_HD;
 }
 } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
 if (link.link_speed == ETH_SPEED_NUM_10M) {
 *current = NETDEV_F_10MB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_100M) {
 *current = NETDEV_F_100MB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_1G) {
 *current = NETDEV_F_1GB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_10G) {
 *current = NETDEV_F_10GB_FD;
 }
 }

Based on the comments in header files this should work. Thanks in
advance.


Yup, that's what I meant. However turns out that isn't right either.

Looking at lib/netdev-linux.c, *current is a bitfield with current speed
set, AND if its autonegotiated then NETDEV_F_AUTONEG is *also* set. I
think this code was never quite correct to begin with.


To clarify, I think the DPDK code for this was never quite correct to
begin with.


So I had a quick look whether 2.5 and earlier need some kind of fix in 
this area. AFAICS the deal is that the autoneg case is a "can't happen" 
situation here: when initializing a port, value of zero indicates 
"autonegotiate this, please" but the driver then sets the fields to the 
negotiated value and there's no way to tell whether it was 
autonegotiated or not after the fact.


In other words, this could be applied to OVS 2.5 (and older) and nothing 
would actually change:


--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1736,11 +1736,7 @@ netdev_dpdk_get_features(const struct netdev *netdev,
 link = dev->link;
 ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
-*current = NETDEV_F_AUTONEG;
-}
-} else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
+if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
 if (link.link_speed == ETH_LINK_SPEED_10) {
 *current = NETDEV_F_10MB_HD;
 }

But since its just a NOP and now fixed for newer versions... probably 
not worth the trouble.


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

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

[antonio . fischetti]

The purpose of this implementation is to improve the performance
of wildcard matching in user-space.
This RFC patch shows the basic functionality, some aspects were not
covered yet.

I would like to get some feedback on whether people think integrating
the DPDK ACL table in this manner is potentially a good solution or not.

DPDK ACL tables show a better performance on lookup operations than the
Classifier.  However their insertion time for new rules is unacceptable.
This solution attempts to combine the better performance of ACL lookups
with the lower insertion latency of the Classifier.

One ACL and one Classifier are being used and are interchangeable.
One table at a time is operating, the other one is kept in stand-by.
Lookups are performed on the operating table only.
Both tables must have the same content.

Whenever we need to insert a new batch of rules we first do that into
the Classifier, which becomes the operating table.
In the meantime the ACL gets updated by a separate thread.  When
it's ready, the ACL will become the operating table and all subsequent
lookups will be performed on the ACL.

EMC table is bypassed for debugging and testing purposes, so each
incoming packet will hit the wildcarded table (see define DEBUG_BYPASS_EMC).

PLEASE NOTE:

Some aspects were not implemented yet, so this RFC Patch has the
following limitations.

- ACL Rules are not removed after a Flow deletion.
  That is due to a limitation of ACL tables.
  A solution for a future implementation can be: when we need to delete
  an ACL rule the dpcls becomes the operating table.  In the background
  the ACL gets populated from scratch and then rebuilt.

- Flows can be installed with Rules on fields like:
  Protocol Type, IP src, IP dest, Port Src, Port Dest.
  In a later implementation all other fields will be added.

- Works with 1 PMD only. Will be extended in a later implementation.

- Builds with DPDK only.

- The ACL builder thread model will be changed, so that the thread will be
  created once at the startup and will run all the time. More details
  in comments inside dpacl_build() and acl_builder_thread() functions.

Signed-off-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 634 +-
 1 file changed, 633 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 2870951..3d12fe8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -69,6 +69,12 @@
 #include "unixctl.h"
 #include "util.h"
 
+/* FIXME Other compiler guards like the following one
+ * will need to be added. */
+#ifdef DPDK_NETDEV
+#include "rte_acl.h"
+#endif /* DPDK_NETDEV */
+
 #include "openvswitch/vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(dpif_netdev);
@@ -78,6 +84,60 @@ VLOG_DEFINE_THIS_MODULE(dpif_netdev);
 #define MAX_RECIRC_DEPTH 5
 DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
 
+/* The next define is just for debugging/testing purposes, will be removed.
+ * When enabled, it forces to skip the EMC search and pretends it fails.
+ * So that any packet will be checked against the ACL/Classifier. */
+#define DEBUG_BYPASS_EMC
+
+/* Unique name for the ACL table. */
+#define ACL_NAME "OVS-DPDK"
+/* Setup nr of categories for ACL. */
+#define ACL_NR_CATEGORIES 1
+/* Max elements in the wait queue. */
+#define ACL_WAIT_QUEUE_MAX_LEN 100
+/* Number of elements in the intermediate table. */
+#define ACL_DPCLS_RULES_MAX 100
+
+/* The following offsets and bit postitions are hard-coded. They were
+ * calculated on the received miniflows values. */
+#define ACL_OFFSET_MASK_MAC_DST 32
+#define ACL_FIELD_IS_PRESENT_BIT_MAC 0x40
+#define ACL_FIELD_IS_PRESENT_BIT_IP 0x04
+
+#define ACL_OFFSET_IP_SRC 40
+#define ACL_OFFSET_IP_DST 44
+#define ACL_OFFSET_PROTO 55
+#define ACL_OFFSET_PORT_SRC 64
+#define ACL_OFFSET_PORT_DST (ACL_OFFSET_PORT_SRC + 2)
+
+/*
+ * Finite State Machine to keep track and manage ACL table setup, rule
+ * insertions and the swapping between the ACL and the Classifier.
+ */
+enum acl_state {
+S_ACL_INIT, /* Initial state.
+ * The Classifier is the operating tbl.
+ * No ACL has been created yet. */
+S_ACL_STANDBY,  /* ACL contains no entry.
+ * The Classifier is the operating tbl.
+ * At the startup, after the ACL is created we enter
+ * this state.  We can't perform lookups on ACL when
+ * it's empty.  Rule insertions are allowed. */
+S_ACL_BUILDING, /* New rules were inserted and ACL building is in progress.
+ * The Classifier is the operating tbl.
+ * ACL insertions are not allowed now, so any new rule
+ * will be stored in a wait queue. */
+S_ACL_OPERATING,/* ACL contains rules and was built.
+ * The ACL is the operating table.
+ * We keep staying in this

[ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 16.04.

[mweglicx]

Following changes are applied:
 - INSTALL.DPDK.md: CONFIG_RTE_BUILD_COMBINE_LIBS step has been
   removed because it is no longer present in DPDK configuration
   (combined library is created by default),
 - INSTALL.DPDK.md: VHost Cuse configuration is updated,
 - netdev-dpdk.c: Link speed definition is changed in DPDK and
   netdev_dpdk_get_features is updated accordingly,
 - .travis/linux-build.sh: DPDK version is updated and legacy
   flags have been removed in configuration.

Signed-off-by: Michal Weglicki 
Signed-off-by: Panu Matilainen 

v1->v2
 - link autonegotiation check is corrected.
---
 .travis/linux-build.sh |  5 +
 INSTALL.DPDK.md| 23 +--
 lib/netdev-dpdk.c  | 24 
 3 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index ef712d0..a3c8e6e 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -49,9 +49,6 @@ function install_dpdk()
 cd dpdk-$1
 fi
 find ./ -type f | xargs sed -i 
's/max-inline-insns-single=100/max-inline-insns-single=400/'
-sed -ri 's,(CONFIG_RTE_BUILD_COMBINE_LIBS=).*,\1y,' config/common_linuxapp
-echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
-sed -ri '/EXECENV_CFLAGS  = -pthread -fPIC/{s/$/\nelse ifeq 
($(CONFIG_RTE_BUILD_FPIC),y)/;s/$/\nEXECENV_CFLAGS  = -pthread -fPIC/}' 
mk/exec-env/linuxapp/rte.vars.mk
 make config CC=gcc T=x86_64-native-linuxapp-gcc
 make CC=gcc RTE_KERNELDIR=$KERNELSRC
 echo "Installed DPDK source in $(pwd)"
@@ -69,7 +66,7 @@ fi
 
 if [ "$DPDK" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="2.2.0"
+DPDK_VER="16.04"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 9ec8bf6..8c8cd4c 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -16,7 +16,7 @@ OVS needs a system with 1GB hugepages support.
 Building and Installing:
 
 
-Required: DPDK 2.2
+Required: DPDK 16.04
 Optional (if building with vhost-cuse): `fuse`, `fuse-devel` (`libfuse-dev`
 on Debian/Ubuntu)
 
@@ -24,16 +24,11 @@ on Debian/Ubuntu)
   1. Set `$DPDK_DIR`
 
  ```
- export DPDK_DIR=/usr/src/dpdk-2.2
+ export DPDK_DIR=/usr/src/dpdk-16.04
  cd $DPDK_DIR
  ```
 
-  2. Update `config/common_linuxapp` so that DPDK generate single lib file.
- (modification also required for IVSHMEM build)
-
- `CONFIG_RTE_BUILD_COMBINE_LIBS=y`
-
- Then run `make install` to build and install the library.
+  2. Then run `make install` to build and install the library.
  For default install without IVSHMEM:
 
  `make install T=x86_64-native-linuxapp-gcc DESTDIR=install`
@@ -81,7 +76,7 @@ Using the DPDK with ovs-vswitchd:
 
 1. Setup system boot
Add the following options to the kernel bootline:
-   
+
`default_hugepagesz=1GB hugepagesz=1G hugepages=1`
 
 2. Setup DPDK devices:
@@ -496,7 +491,7 @@ the vswitchd.
 DPDK vhost:
 ---
 
-DPDK 2.2 supports two types of vhost:
+DPDK 16.04 supports two types of vhost:
 
 1. vhost-user
 2. vhost-cuse
@@ -517,7 +512,7 @@ with OVS.
 DPDK vhost-user Prerequisites:
 -
 
-1. DPDK 2.2 with vhost support enabled as documented in the "Building and
+1. DPDK 16.04 with vhost support enabled as documented in the "Building and
Installing section"
 
 2. QEMU version v2.1.0+
@@ -635,10 +630,10 @@ with OVS.
 DPDK vhost-cuse Prerequisites:
 -
 
-1. DPDK 2.2 with vhost support enabled as documented in the "Building and
+1. DPDK 16.04 with vhost support enabled as documented in the "Building and
Installing section"
As an additional step, you must enable vhost-cuse in DPDK by setting the
-   following additional flag in `config/common_linuxapp`:
+   following additional flag in `config/common_base`:
 
`CONFIG_RTE_LIBRTE_VHOST_USER=n`
 
@@ -938,7 +933,7 @@ Restrictions:
 this with smaller page sizes.
 
   Platform and Network Interface:
-  - By default with DPDK 2.2, a maximum of 64 TX queues can be used with an
+  - By default with DPDK 16.04, a maximum of 64 TX queues can be used with an
 Intel XL710 Network Interface on a platform with more than 64 logical
 cores. If a user attempts to add an XL710 interface as a DPDK port type to
 a system as described above, an error will be reported that initialization
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..adf1d13 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1740,35 +1740,35 @@ netdev_dpdk_get_features(const struct netdev *netdev_,
 link = dev->link;
 ovs_mutex_unlock(&dev->mutex);
 
-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
-*current = NETDEV_F_AUTONEG;
-}
-} else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_10) {
+if (link.link_

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Weglicki, MichalX]

-Original Message-
From: Panu Matilainen [mailto:pmati...@redhat.com] 
Sent: Wednesday, April 13, 2016 9:38 AM
To: Weglicki, MichalX ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for 
DPDK 16.04.

On 04/13/2016 11:33 AM, Panu Matilainen wrote:
> On 04/13/2016 11:16 AM, Weglicki, MichalX wrote:
>> -Original Message-
>> From: Panu Matilainen [mailto:pmati...@redhat.com]
>> Sent: Wednesday, April 13, 2016 8:50 AM
>> To: Weglicki, MichalX ; dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add
>> support for DPDK 16.04.
>>
>> On 04/12/2016 05:05 PM, mweglicx wrote:
>> [...]
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index e09b471..2295e53 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev
>>> *netdev_,
>>>link = dev->link;
>>>ovs_mutex_unlock(&dev->mutex);
>>>
>>> -if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
>>> +if (link.link_duplex == ETH_LINK_AUTONEG) {
>>
>> This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or
>> ETH_LINK_FULL_DUPLEX. It sort of happens to work because both
>> ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having
>> autonegotiation end up with half-duplex these days isn't that likely.
>>
>> [MW] Thank you, your're right, just to be sure, you mean that I should
>> check autoneg through
>> link_autoneg flag, like this:
>>  if (link.link_autoneg) {
>>  *current = NETDEV_F_AUTONEG;
>>  } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
>>  if (link.link_speed == ETH_SPEED_NUM_10M) {
>>  *current = NETDEV_F_10MB_HD;
>>  }
>>  if (link.link_speed == ETH_SPEED_NUM_100M) {
>>  *current = NETDEV_F_100MB_HD;
>>  }
>>  if (link.link_speed == ETH_SPEED_NUM_1G) {
>>  *current = NETDEV_F_1GB_HD;
>>  }
>>  } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
>>  if (link.link_speed == ETH_SPEED_NUM_10M) {
>>  *current = NETDEV_F_10MB_FD;
>>  }
>>  if (link.link_speed == ETH_SPEED_NUM_100M) {
>>  *current = NETDEV_F_100MB_FD;
>>  }
>>  if (link.link_speed == ETH_SPEED_NUM_1G) {
>>  *current = NETDEV_F_1GB_FD;
>>  }
>>  if (link.link_speed == ETH_SPEED_NUM_10G) {
>>  *current = NETDEV_F_10GB_FD;
>>  }
>>  }
>>
>> Based on the comments in header files this should work. Thanks in
>> advance.
>
> Yup, that's what I meant. However turns out that isn't right either.
>
> Looking at lib/netdev-linux.c, *current is a bitfield with current speed
> set, AND if its autonegotiated then NETDEV_F_AUTONEG is *also* set. I
> think this code was never quite correct to begin with.

To clarify, I think the DPDK code for this was never quite correct to 
begin with.

- Panu -
Yes, you are right, I double checked netdev-linux and this is exactly how it is 
implemented there. 

Ok, I will apply v2 shortly, thank you. 

Br, 
Michal. 
--
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Panu Matilainen]

On 04/13/2016 11:33 AM, Panu Matilainen wrote:

On 04/13/2016 11:16 AM, Weglicki, MichalX wrote:

-Original Message-
From: Panu Matilainen [mailto:pmati...@redhat.com]
Sent: Wednesday, April 13, 2016 8:50 AM
To: Weglicki, MichalX ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add
support for DPDK 16.04.

On 04/12/2016 05:05 PM, mweglicx wrote:
[...]

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..2295e53 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev
*netdev_,
   link = dev->link;
   ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
+if (link.link_duplex == ETH_LINK_AUTONEG) {


This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having
autonegotiation end up with half-duplex these days isn't that likely.

[MW] Thank you, your're right, just to be sure, you mean that I should
check autoneg through
link_autoneg flag, like this:
 if (link.link_autoneg) {
 *current = NETDEV_F_AUTONEG;
 } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
 if (link.link_speed == ETH_SPEED_NUM_10M) {
 *current = NETDEV_F_10MB_HD;
 }
 if (link.link_speed == ETH_SPEED_NUM_100M) {
 *current = NETDEV_F_100MB_HD;
 }
 if (link.link_speed == ETH_SPEED_NUM_1G) {
 *current = NETDEV_F_1GB_HD;
 }
 } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
 if (link.link_speed == ETH_SPEED_NUM_10M) {
 *current = NETDEV_F_10MB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_100M) {
 *current = NETDEV_F_100MB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_1G) {
 *current = NETDEV_F_1GB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_10G) {
 *current = NETDEV_F_10GB_FD;
 }
 }

Based on the comments in header files this should work. Thanks in
advance.


Yup, that's what I meant. However turns out that isn't right either.

Looking at lib/netdev-linux.c, *current is a bitfield with current speed
set, AND if its autonegotiated then NETDEV_F_AUTONEG is *also* set. I
think this code was never quite correct to begin with.


To clarify, I think the DPDK code for this was never quite correct to 
begin with.


- Panu -

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

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Panu Matilainen]

On 04/13/2016 11:16 AM, Weglicki, MichalX wrote:

-Original Message-
From: Panu Matilainen [mailto:pmati...@redhat.com]
Sent: Wednesday, April 13, 2016 8:50 AM
To: Weglicki, MichalX ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for 
DPDK 16.04.

On 04/12/2016 05:05 PM, mweglicx wrote:
[...]

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..2295e53 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev *netdev_,
   link = dev->link;
   ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
+if (link.link_duplex == ETH_LINK_AUTONEG) {


This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having
autonegotiation end up with half-duplex these days isn't that likely.

[MW] Thank you, your're right, just to be sure, you mean that I should check 
autoneg through
link_autoneg flag, like this:
 if (link.link_autoneg) {
 *current = NETDEV_F_AUTONEG;
 } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
 if (link.link_speed == ETH_SPEED_NUM_10M) {
 *current = NETDEV_F_10MB_HD;
 }
 if (link.link_speed == ETH_SPEED_NUM_100M) {
 *current = NETDEV_F_100MB_HD;
 }
 if (link.link_speed == ETH_SPEED_NUM_1G) {
 *current = NETDEV_F_1GB_HD;
 }
 } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
 if (link.link_speed == ETH_SPEED_NUM_10M) {
 *current = NETDEV_F_10MB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_100M) {
 *current = NETDEV_F_100MB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_1G) {
 *current = NETDEV_F_1GB_FD;
 }
 if (link.link_speed == ETH_SPEED_NUM_10G) {
 *current = NETDEV_F_10GB_FD;
 }
 }

Based on the comments in header files this should work. Thanks in advance.


Yup, that's what I meant. However turns out that isn't right either.

Looking at lib/netdev-linux.c, *current is a bitfield with current speed 
set, AND if its autonegotiated then NETDEV_F_AUTONEG is *also* set. I 
think this code was never quite correct to begin with.


So what the whole thing *really* should be is:

if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
if (link.link_speed == ETH_SPEED_NUM_10M) {
*current = NETDEV_F_10MB_HD;
}
if (link.link_speed == ETH_SPEED_NUM_100M) {
*current = NETDEV_F_100MB_HD;
}
if (link.link_speed == ETH_SPEED_NUM_1G) {
*current = NETDEV_F_1GB_HD;
}
} else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
if (link.link_speed == ETH_SPEED_NUM_10M) {
*current = NETDEV_F_10MB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_100M) {
*current = NETDEV_F_100MB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_1G) {
*current = NETDEV_F_1GB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_10G) {
*current = NETDEV_F_10GB_FD;
}
}
if (link.autoneg == ETH_LINK_AUTONEG)
*current |= NETDEV_F_AUTONEG;


- Panu -

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

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Weglicki, MichalX]

-Original Message-
From: Panu Matilainen [mailto:pmati...@redhat.com] 
Sent: Wednesday, April 13, 2016 8:50 AM
To: Weglicki, MichalX ; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for 
DPDK 16.04.

On 04/12/2016 05:05 PM, mweglicx wrote:
[...]
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index e09b471..2295e53 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev *netdev_,
>   link = dev->link;
>   ovs_mutex_unlock(&dev->mutex);
>
> -if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
> +if (link.link_duplex == ETH_LINK_AUTONEG) {

This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or 
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both 
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having 
autonegotiation end up with half-duplex these days isn't that likely.

[MW] Thank you, your're right, just to be sure, you mean that I should check 
autoneg through 
link_autoneg flag, like this: 
if (link.link_autoneg) {
*current = NETDEV_F_AUTONEG;
} else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
if (link.link_speed == ETH_SPEED_NUM_10M) {
*current = NETDEV_F_10MB_HD;
}
if (link.link_speed == ETH_SPEED_NUM_100M) {
*current = NETDEV_F_100MB_HD;
}
if (link.link_speed == ETH_SPEED_NUM_1G) {
*current = NETDEV_F_1GB_HD;
}
} else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
if (link.link_speed == ETH_SPEED_NUM_10M) {
*current = NETDEV_F_10MB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_100M) {
*current = NETDEV_F_100MB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_1G) {
*current = NETDEV_F_1GB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_10G) {
*current = NETDEV_F_10GB_FD;
}
}

Based on the comments in header files this should work. Thanks in advance. 

>   if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
>   *current = NETDEV_F_AUTONEG;
>   }

Additionally link_speed in DPDK 16.04 reflects the actual negotiated 
speed and is never ETH_LINK_SPEED_AUTONEG, which is a bitmap flag 
relevant to link_speeds field.

The autoneg case should be something like this:

@@ -1573,31 +1573,29 @@ netdev_dpdk_get_features(const struct netdev 
*netdev_,
  link = dev->link;
  ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
-*current = NETDEV_F_AUTONEG;
-}
+if (link.link_autoneg) {
+   *current = NETDEV_F_AUTONEG;
  } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {


>   } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
> -if (link.link_speed == ETH_LINK_SPEED_10) {
> +if (link.link_speed == ETH_SPEED_NUM_10M) {
>   *current = NETDEV_F_10MB_HD;
>   }
> -if (link.link_speed == ETH_LINK_SPEED_100) {
> +if (link.link_speed == ETH_SPEED_NUM_100M) {
>   *current = NETDEV_F_100MB_HD;
>   }
> -if (link.link_speed == ETH_LINK_SPEED_1000) {
> +if (link.link_speed == ETH_SPEED_NUM_1G) {
>   *current = NETDEV_F_1GB_HD;
>   }
>   } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
> -if (link.link_speed == ETH_LINK_SPEED_10) {
> +if (link.link_speed == ETH_SPEED_NUM_10M) {
>   *current = NETDEV_F_10MB_FD;
>   }
> -if (link.link_speed == ETH_LINK_SPEED_100) {
> +if (link.link_speed == ETH_SPEED_NUM_100M) {
>   *current = NETDEV_F_100MB_FD;
>   }
> -if (link.link_speed == ETH_LINK_SPEED_1000) {
> +if (link.link_speed == ETH_SPEED_NUM_1G) {
>   *current = NETDEV_F_1GB_FD;
>   }
> -if (link.link_speed == ETH_LINK_SPEED_1) {
> +if (link.link_speed == ETH_SPEED_NUM_10G) {
>   *current = NETDEV_F_10GB_FD;
>   }
>   }

The rest looks ok.

As an aside, I've been thinking maybe this is a case where OVS could 
support both DPDK 2.2 and 16.04. I know its unprecedented but maybe that 
could change, restricting OVS to just one DPDK version seems 
unnecessarily strict when talking about differences this trivial.

- Panu -
--
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
_

Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

[Panu Matilainen]

On 04/12/2016 05:05 PM, mweglicx wrote:
[...]

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..2295e53 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev *netdev_,
  link = dev->link;
  ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
+if (link.link_duplex == ETH_LINK_AUTONEG) {


This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or 
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both 
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having 
autonegotiation end up with half-duplex these days isn't that likely.




  if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
  *current = NETDEV_F_AUTONEG;
  }


Additionally link_speed in DPDK 16.04 reflects the actual negotiated 
speed and is never ETH_LINK_SPEED_AUTONEG, which is a bitmap flag 
relevant to link_speeds field.


The autoneg case should be something like this:

@@ -1573,31 +1573,29 @@ netdev_dpdk_get_features(const struct netdev 
*netdev_,

 link = dev->link;
 ovs_mutex_unlock(&dev->mutex);

-if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
-*current = NETDEV_F_AUTONEG;
-}
+if (link.link_autoneg) {
+   *current = NETDEV_F_AUTONEG;
 } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {



  } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_10) {
+if (link.link_speed == ETH_SPEED_NUM_10M) {
  *current = NETDEV_F_10MB_HD;
  }
-if (link.link_speed == ETH_LINK_SPEED_100) {
+if (link.link_speed == ETH_SPEED_NUM_100M) {
  *current = NETDEV_F_100MB_HD;
  }
-if (link.link_speed == ETH_LINK_SPEED_1000) {
+if (link.link_speed == ETH_SPEED_NUM_1G) {
  *current = NETDEV_F_1GB_HD;
  }
  } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
-if (link.link_speed == ETH_LINK_SPEED_10) {
+if (link.link_speed == ETH_SPEED_NUM_10M) {
  *current = NETDEV_F_10MB_FD;
  }
-if (link.link_speed == ETH_LINK_SPEED_100) {
+if (link.link_speed == ETH_SPEED_NUM_100M) {
  *current = NETDEV_F_100MB_FD;
  }
-if (link.link_speed == ETH_LINK_SPEED_1000) {
+if (link.link_speed == ETH_SPEED_NUM_1G) {
  *current = NETDEV_F_1GB_FD;
  }
-if (link.link_speed == ETH_LINK_SPEED_1) {
+if (link.link_speed == ETH_SPEED_NUM_10G) {
  *current = NETDEV_F_10GB_FD;
  }
  }


The rest looks ok.

As an aside, I've been thinking maybe this is a case where OVS could 
support both DPDK 2.2 and 16.04. I know its unprecedented but maybe that 
could change, restricting OVS to just one DPDK version seems 
unnecessarily strict when talking about differences this trivial.


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

[ovs-dev] [CudaMailTagged] Adding a field in the struct flow

[Amrane Ait Zeouay]

Hi,
I want to add a field to the struct flow, and i checked the FAQ 'How do I
add support for a new field or header?' but i didn't understand it, what i
want to do is add a field in the flow and then when it will be parsed it
will call an external function, the field is not in the packet it's just in
the flow table.
Thank you.
Best regards

-- 

Amrane Ait Zeouay

Engineer Student in The Development of Software and Systems

University of Western Brittany

Tel:  +33 7 62 25 56 03 <+33+7+62+25+56+03>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev