Re: [ovs-dev] [PATCH] tests: Don't include a newline in ovs_fatal() calls.

[Justin Pettit]

> On Feb 9, 2018, at 5:25 PM, Ben Pfaff  wrote:
> 
> On Fri, Feb 09, 2018 at 05:03:16PM -0800, Justin Pettit wrote:
>> Signed-off-by: Justin Pettit 
> 
> Acked-by: Ben Pfaff 

Thanks.  I pushed this to master.

--Justin


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

Re: [ovs-dev] [PATCH] tests: Don't include a newline in ovs_fatal() calls.

[Ben Pfaff]

On Fri, Feb 09, 2018 at 05:03:16PM -0800, Justin Pettit wrote:
> Signed-off-by: Justin Pettit 

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

Re: [ovs-dev] [PATCH v3 2/2] Measure performance of ovn-controller loop.

[Han Zhou]

Looks a great tool! Just some minor comments:

On Fri, Feb 9, 2018 at 3:00 PM, Mark Michelson  wrote:
>
> This modifies ovn-controller to measure the amount of time it takes to
> detect a change in the southbound database, generate the resulting flow
> table, and write the flow table down to vswitchd.

The comment is a little inaccurate. The change cannot guarantee measurement
of "write the flow table down to vswitchd", since sending the flow-adding
messages to vswitchd could be performed in multiple iterations of the main
loop, depending on the buffering status of vconn_send(). If it can't be
completed in one round, the message sendings will continue in next loops in
ofctrl_run().

>
> The statistics can be queried using:
>
> ovs-appctl -t ovn-controller performance/show ovn-controller-loop
>
> Signed-off-by: Mark Michelson 
> ---
>  ovn/controller/ovn-controller.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> index 7592bda25..b9f8950d4 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -57,6 +57,9 @@
>  #include "stream.h"
>  #include "unixctl.h"
>  #include "util.h"
> +#include "timeval.h"
> +#include "timer.h"
> +#include "performance.h"
>
>  VLOG_DEFINE_THIS_MODULE(main);
>
> @@ -67,6 +70,8 @@ static unixctl_cb_func inject_pkt;
>  #define DEFAULT_BRIDGE_NAME "br-int"
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>
> +#define CONTROLLER_LOOP_PERFORMANCE_NAME "ovn-controller-loop"
> +
>  static void update_probe_interval(struct controller_ctx *,
>const char *ovnsb_remote);
>  static void parse_options(int argc, char *argv[]);
> @@ -639,8 +644,10 @@ main(int argc, char *argv[])
>  unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
>   _pkt);
>
> +performance_create(CONTROLLER_LOOP_PERFORMANCE_NAME, PERF_MS);
>  /* Main loop. */
>  exiting = false;
> +unsigned int our_seqno = 0;
>  while (!exiting) {
>  /* Check OVN SB database. */
>  char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
> @@ -659,6 +666,12 @@ main(int argc, char *argv[])
>  .ovnsb_idl_txn = ovsdb_idl_loop_run(_idl_loop),
>  };
>
> +if (our_seqno != ovsdb_idl_get_seqno(ctx.ovnsb_idl)) {

Shall we measure when there is no SB change, too? E.g. the loop can be
triggered by local ovs-idl change, or by pin-ctrl messages, and these
changes will trigger all the flow computations, which contributes a
significant cost in ovn-controller, so I think we'd better measure them as
well. I am working on an incremental processing framework which would avoid
this kind of unnecessary recompute. I hope with this tool the improvements
can be measured accurately :)

> +performance_start_sample(CONTROLLER_LOOP_PERFORMANCE_NAME,
> + time_msec());
> +our_seqno = ovsdb_idl_get_seqno(ctx.ovnsb_idl);
> +}
> +
>  update_probe_interval(, ovnsb_remote);
>
>  update_ssl_config(ctx.ovs_idl);
> @@ -728,6 +741,9 @@ main(int argc, char *argv[])
>  ofctrl_put(_table, _ct_zones,
> get_nb_cfg(ctx.ovnsb_idl));
>
> +
 performance_end_sample(CONTROLLER_LOOP_PERFORMANCE_NAME,
> +   time_msec());
> +
>  hmap_destroy(_table);
>  }
>  if (ctx.ovnsb_idl_txn) {
> @@ -792,6 +808,7 @@ main(int argc, char *argv[])
>  ofctrl_wait();
>  pinctrl_wait();
>  }
> +
>  ovsdb_idl_loop_commit_and_wait(_idl_loop);
>
>  if (ovsdb_idl_loop_commit_and_wait(_idl_loop) == 1) {
> --
> 2.13.6
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[ovs-dev] [PATCH] tests: Don't include a newline in ovs_fatal() calls.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 tests/test-rstp.c | 2 +-
 tests/test-stp.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test-rstp.c b/tests/test-rstp.c
index 7bcff83f3ab6..01aeaf847838 100644
--- a/tests/test-rstp.c
+++ b/tests/test-rstp.c
@@ -467,7 +467,7 @@ test_rstp_main(int argc, char *argv[])
 vlog_set_levels(NULL, VLF_SYSLOG, VLL_OFF);
 
 if (argc != 2) {
-ovs_fatal(0, "usage: test-rstp INPUT.RSTP\n");
+ovs_fatal(0, "usage: test-rstp INPUT.RSTP");
 }
 file_name = argv[1];
 
diff --git a/tests/test-stp.c b/tests/test-stp.c
index c072108c7e12..c85c99d67385 100644
--- a/tests/test-stp.c
+++ b/tests/test-stp.c
@@ -446,7 +446,7 @@ test_stp_main(int argc, char *argv[])
 vlog_set_levels(NULL, VLF_SYSLOG, VLL_OFF);
 
 if (argc != 2) {
-ovs_fatal(0, "usage: test-stp INPUT.STP\n");
+ovs_fatal(0, "usage: test-stp INPUT.STP");
 }
 file_name = argv[1];
 
-- 
2.7.4

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

[ovs-dev] Top 50 February

[Bonesca Mailing]

Deze email nieuwsbrief werd in grafisch HTML formaat verzonden.
Als u deze tekstversie ziet, verkiest uw email programma "gewone tekst" emails.
U kan de originele nieuwsbrief online bekijken:
http://ymlptrack5.com/zl1V8C



Good afternoon, Gutentag, Goedemiddag, Bonjour,

GB:  In the attachment you can find our top 50 February offer!
DE: Im Anlage finden sie unsere Top 50 Februar Angebot!
NL: In de bijlage vind u onze top 50 februari aanbieding!
FR: Dans l'attachement vous pouvez trouver notre top 50 février
offre!

Kind regards, Mit freundlichen grüssen, Met vriendelijke groet,
Cordialement,

Bonesca Import en Export BV

Bertus Brouwer
Director/Purchase - Dutch / English / German
Tel.: +31 (0) 527 68 07 81
Mail: ber...@bonesca.nl

Nedal Muhtaseb
Sales - Dutch / Arab / English / German
Tel: +31 (0) 527 68 07 83
Mail: ne...@bonesca.nl

Marianne Kramer
Sales - Dutch / French / English / German
Tel: +31 (0) 527 68 07 82
Mail: maria...@bonesca.nl

Henry Bakker
Administration/Planning
Tel: +31 (0) 527 68 07 85
Mail: he...@bonesca.nl; i...@bonesca.nl;invo...@bonesca.nl

Thu Hanh
Sales Vietnamese / Thai / Laothian / German / English
Tel: +49 (0) 32221097676
Tel: +31 (0) 527 68 07 88
Mail: t...@bonesca.nl

Ramesh Raja
Sales Tamil / Dutch / English
Tel: +31 (0) 527 68 07 84
Mail: r...@bonesca.nl

Policarpo J. Olivas
Sales Spanish, French, Portugese, Italian, English
Tel Spain: +34 (0) 649 566 367
Tel Rep. Dominican: +1 (0) 809 778 4040
Mail: poli...@bonesca.nl

Cornelis Ras
Office support
Dutch / English
Tel: +31 (0) 527 70 10 63 option 1
Mail: st...@bonesca.nl

Jacob Cornelis Hakvoort
Accounting Dutch / English
Tel: +31 (0) 527 68 07 86
Mail: st...@bonesca.nl

Warehouse - Albert Korf / Willem Bakker / Fokke Korf
Tel: +31 (0) 527 68 07 89
Mail: expedi...@bonesca.nl

_
Sign out / Change E-mailadress: 
http://ymlptrack5.com/ughbywyjgsgubbebbgjmumgghewbs
Powered door YourMailingListProvider

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

[ovs-dev] [PATCH] ovsdb-tool: Indicate "db" and "schema" are optional in man page.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 ovsdb/ovsdb-tool.1.in | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/ovsdb/ovsdb-tool.1.in b/ovsdb/ovsdb-tool.1.in
index 7b89ffeec8bf..40eb71581f3f 100644
--- a/ovsdb/ovsdb-tool.1.in
+++ b/ovsdb/ovsdb-tool.1.in
@@ -31,9 +31,9 @@ ovsdb\-tool \- Open vSwitch database management utility
 .IP "Other commands:"
 \fBovsdb\-tool \fR[\fIoptions\fR] \fBcompact \fR[\fIdb\fR [\fItarget\fR]]
 .br
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBquery \fR[\fIdb\fR] \fItransaction\fR
+\fBovsdb\-tool \fR[\fIoptions\fR] [\fB\-\-rbac\-role=\fIrole\fR] \fBquery 
\fR[\fIdb\fR] \fItransaction\fR
 .br
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBtransact \fR[\fIdb\fR] \fItransaction\fR
+\fBovsdb\-tool \fR[\fIoptions\fR] [\fB\-\-rbac\-role=\fIrole\fR] \fBtransact 
\fR[\fIdb\fR] \fItransaction\fR
 .br
 \fBovsdb\-tool \fR[\fIoptions\fR] [\fB\-m\fR | \fB\-\-more\fR]... \fBshow\-log 
\fR[\fIdb\fR]
 .br
@@ -53,6 +53,11 @@ running Open vSwitch database servers (instead, use
 For an introduction to OVSDB and its implementation in Open vSwitch,
 see \fBovsdb\fR(7).
 .PP
+Each command that takes an optional \fIdb\fR or \fIschema\fR argument
+has a default file location if it is not specified..  The default
+\fIdb\fR is \fB@DBDIR@/conf.db\fR.  The default \fIschema\fR is
+\fB@pkgdatadir@/vswitch.ovsschema\fR.
+.PP
 This OVSDB implementation supports standalone and active-backup
 database service models with a common on-disk format  For a
 specification of this format, see \fBovsdb\fR(5).  For more
@@ -64,7 +69,7 @@ This command creates a new OVSDB database file.
 It will not overwrite an existing database file.  To
 replace an existing database with a new one, first delete the old one.
 .
-.IP "\fBcreate\fI db schema\fR"
+.IP "\fBcreate \fR[\fIdb\fR [\fIschema\fR]]"
 Use this command to create the database for controlling
 \fBovs\-vswitchd\fR or another standalone or active-backup database.
 It creates database file \fIdb\fR with the given \fIschema\fR, which
@@ -78,7 +83,7 @@ initially empty.
 These commands work with different versions of OVSDB schemas and
 databases.
 .
-.IP "\fBconvert\fI db schema \fR[\fItarget\fR]"
+.IP "\fBconvert \fR[\fIdb\fR [\fIschema \fR[\fItarget\fR]]]"
 Reads \fIdb\fR, translating it into to the schema specified in
 \fIschema\fR, and writes out the new interpretation.  If \fItarget\fR
 is specified, the translated version is written as a new file named
@@ -102,21 +107,21 @@ example, converting a database from a schema that has a 
given column
 or table to one that does not will delete all data in that column or
 table.  Back up critical databases before converting them.
 .IP
-.IP "\fBneeds\-conversion\fI db schema\fR"
+.IP "\fBneeds\-conversion \fR[\fIdb\fR [\fIschema\fR]]"
 Reads the schema embedded in \fIdb\fR and the JSON schema from
 \fIschema\fR and compares them.  If the schemas are the same, prints
 \fBno\fR on stdout; if they differ, prints \fByes\fR.
 .IP
-.IP "\fBdb\-version\fI db\fR"
-.IQ "\fBschema\-version\fI schema\fR"
+.IP "\fBdb\-version \fR[\fIdb\fR]"
+.IQ "\fBschema\-version \fR[\fIschema\fR]"
 Prints the version number in the schema embedded within the database
 \fIdb\fR or in the JSON schema \fIschema\fR on stdout.
 If \fIschema\fR or \fIdb\fR was created before schema versioning was
 introduced, then it will not have a version number and this command
 will print a blank line.
 .IP
-.IP "\fBdb\-cksum\fI db\fR"
-.IQ "\fBschema\-cksum\fI schema\fR"
+.IP "\fBdb\-cksum \fR[\fIdb\fR]"
+.IQ "\fBschema\-cksum \fR[\fIschema\fR]"
 Prints the checksum in the schema embedded within the database
 \fIdb\fR or of the JSON schema \fIschema\fR on stdout.
 If \fIschema\fR or \fIdb\fR was created before schema checksums were
@@ -125,7 +130,7 @@ will print a blank line.
 .IP
 .SS "Other Commands"
 .
-.IP "\fBcompact\fI db \fR[\fItarget\fR]"
+.IP "\fBcompact \fR[\fIdb\fR [\fItarget\fR]]"
 Reads \fIdb\fR and writes a compacted version.  If \fItarget\fR is
 specified, the compacted version is written as a new file named
 \fItarget\fR, which must not already exist.  If \fItarget\fR is
@@ -139,7 +144,7 @@ This command does not work if \fIdb\fR is currently being 
served by
 another process.  Instead, send the \fBovsdb\-server/compact\fR
 command to \fBovsdb\-server\fR, via \fBovs\-appctl\fR).
 .
-.IP "[\fB\-\-rbac\-role=\fIrole\fR] \fBquery\fI db transaction\fR"
+.IP "[\fB\-\-rbac\-role=\fIrole\fR] \fBquery \fR[\fIdb\fR] \fItransaction\fR"
 Opens \fIdb\fR, executes \fItransaction\fR on it, and prints the
 results.  The \fItransaction\fR must be a JSON array in the format of
 the \fBparams\fR array for the JSON-RPC \fBtransact\fR method, as
@@ -154,7 +159,7 @@ may specify database modifications, but these will have no 
effect on
 By default, the transaction is executed using the ``superuser'' RBAC
 role.  Use \fB\-\-rbac\-role\fR to specify a different role.
 .
-.IP "[\fR\-\-rbac\-role=\fIrole\fR] \fBtransact\fI db 

[ovs-dev] Prevención de Lavado de Dinero

[Evite que su empresa sea víctima de este delito]

Conozca las obligaciones de las entidades financieras y sujetos obligados 
contenidas en la Ley 

Prevención de Lavado de Dinero y Financiamiento al Terrorismo
16 de Febrero- Lic. Guillermo Ruiz Ramírez o Lic. Juan Carlos Cervantes Córdova 
- 9am- 6pm

El lavado de dinero, como cualquier otro delito, es un fenómeno que evoluciona 
y se manifiesta de nuevas formas. Actualmente el reto para México y otros 
países contra el combate a esta actividad, es el uso de las nuevas tecnologías. 
La banca electrónica, las operaciones de pago en línea e incluso la moneda 
virtual llamada ‘bitcoin’ son aspectos que, debido a su inmediatez y facilidad, 
representan uno de los principales desafíos en materia de prevención de lavado 
de dinero. 

BENEFICIOS DE ASISTIR: 

- Aprenderá la tipificación y penas aplicables de acuerdo con la legislación 
nacional. 
- Conocerá las obligaciones de las entidades financieras y sujetos obligados 
contenidas en las Leyes y Disposiciones de carácter general aplicables.
 - Analizará las funciones del Oficial de Cumplimiento y del Comité de 
Comunicación y Control.
 - Obtendrá herramientas de identificación, medición y mitigación del riesgo.
 - Profundizará en el proceso de Enfoque Basado en Riesgo utilizando guías y 
normas internacionales. 

¿Requiere la información a la Brevedad? responda este email con la palabra: 
Lavado + nombre - teléfono - correo.


centro telefónico:018002120744 


 


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

[ovs-dev] [PATCH v3 2/2] Measure performance of ovn-controller loop.

[Mark Michelson]

This modifies ovn-controller to measure the amount of time it takes to
detect a change in the southbound database, generate the resulting flow
table, and write the flow table down to vswitchd.

The statistics can be queried using:

ovs-appctl -t ovn-controller performance/show ovn-controller-loop

Signed-off-by: Mark Michelson 
---
 ovn/controller/ovn-controller.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 7592bda25..b9f8950d4 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -57,6 +57,9 @@
 #include "stream.h"
 #include "unixctl.h"
 #include "util.h"
+#include "timeval.h"
+#include "timer.h"
+#include "performance.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -67,6 +70,8 @@ static unixctl_cb_func inject_pkt;
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 
+#define CONTROLLER_LOOP_PERFORMANCE_NAME "ovn-controller-loop"
+
 static void update_probe_interval(struct controller_ctx *,
   const char *ovnsb_remote);
 static void parse_options(int argc, char *argv[]);
@@ -639,8 +644,10 @@ main(int argc, char *argv[])
 unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
  _pkt);
 
+performance_create(CONTROLLER_LOOP_PERFORMANCE_NAME, PERF_MS);
 /* Main loop. */
 exiting = false;
+unsigned int our_seqno = 0;
 while (!exiting) {
 /* Check OVN SB database. */
 char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
@@ -659,6 +666,12 @@ main(int argc, char *argv[])
 .ovnsb_idl_txn = ovsdb_idl_loop_run(_idl_loop),
 };
 
+if (our_seqno != ovsdb_idl_get_seqno(ctx.ovnsb_idl)) {
+performance_start_sample(CONTROLLER_LOOP_PERFORMANCE_NAME,
+ time_msec());
+our_seqno = ovsdb_idl_get_seqno(ctx.ovnsb_idl);
+}
+
 update_probe_interval(, ovnsb_remote);
 
 update_ssl_config(ctx.ovs_idl);
@@ -728,6 +741,9 @@ main(int argc, char *argv[])
 ofctrl_put(_table, _ct_zones,
get_nb_cfg(ctx.ovnsb_idl));
 
+performance_end_sample(CONTROLLER_LOOP_PERFORMANCE_NAME,
+   time_msec());
+
 hmap_destroy(_table);
 }
 if (ctx.ovnsb_idl_txn) {
@@ -792,6 +808,7 @@ main(int argc, char *argv[])
 ofctrl_wait();
 pinctrl_wait();
 }
+
 ovsdb_idl_loop_commit_and_wait(_idl_loop);
 
 if (ovsdb_idl_loop_commit_and_wait(_idl_loop) == 1) {
-- 
2.13.6

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

[ovs-dev] [PATCH v3 1/2] Add performance measuring API

[Mark Michelson]

This is similar to the existing coverage and perf-counter APIs in OVS.
However, rather than keeping counters, this is aimed at timing how long
operations take to perform. "Operations" in this case can be anything
from a loop iteration, to a function, to something more complex.

The library will keep track of how long it takes to perform the
particular operations and will maintain statistics of those running
times.

Statistics for a particular operation can be queried from the command
line by using ovs-appctl -t  performance/show .

The API is designed to be pretty small. The expected steps are as
follows:

1) Create a performance measurement, providing a unique name, using
performance_create()
2) Add calls to start_sample() and end_sample() to mark the start and
stop of the operation you wish to measure.

A background thread will keep track of the provided measurements and
periodically remove old measurements.

Signed-off-by: Mark Michelson 
---
 lib/automake.mk   |   2 +
 lib/performance.c | 497 ++
 lib/performance.h |  41 +
 3 files changed, 540 insertions(+)
 create mode 100644 lib/performance.c
 create mode 100644 lib/performance.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 38d2a999d..60a830715 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -207,6 +207,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/pcap-file.h \
lib/perf-counter.h \
lib/perf-counter.c \
+   lib/performance.h \
+   lib/performance.c \
lib/poll-loop.c \
lib/process.c \
lib/process.h \
diff --git a/lib/performance.c b/lib/performance.c
new file mode 100644
index 0..f842375cf
--- /dev/null
+++ b/lib/performance.c
@@ -0,0 +1,497 @@
+/* Copyright (c) 2017 Red Hat, 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 "performance.h"
+#include "timer.h"
+#include "timeval.h"
+#include "openvswitch/shash.h"
+#include "openvswitch/vlog.h"
+#include "unixctl.h"
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/poll-loop.h"
+#include "ovs-thread.h"
+#include 
+#include "socket-util.h"
+
+VLOG_DEFINE_THIS_MODULE(performance);
+
+struct sample {
+unsigned long long start_time; /* Time when we started this sample */
+unsigned long long end_time;   /* Time when we ended this sample */
+unsigned long long elapsed;/* Elapsed time: end_time - start_time */
+};
+
+struct sample_vec {
+struct sample *samples; /* Dynamic array of samples */
+size_t n_samples;   /* Number of samples */
+size_t capacity;/* Number of allocated samples */
+size_t max_size;
+};
+
+struct stats {
+unsigned long long min; /* Minimum measurement (ms) */
+unsigned long long max; /* Maximum measurement (ms) */
+double average; /* Average measurement (ms) */
+unsigned long long percentile;  /* 95th percentile measurement (ms) */
+unsigned long long num_samples; /* Total number of measurements */
+};
+
+struct performance {
+struct sample_vec vec;
+struct timer timer;
+struct sample cur_sample;
+enum performance_units units;
+};
+
+static struct shash performances = SHASH_INITIALIZER();
+static struct ovs_mutex performances_lock = OVS_MUTEX_INITIALIZER;
+
+static int performance_pipe[2];
+static pthread_t performance_thread_id;
+
+#define PERFORMANCE_INTERVAL 1
+
+/* Maximum size the sample vector can grow to.
+ * These are generous sizes. In practice, even
+ * with very quick operations going constantly,
+ * it's not likely to exceed a fifth of the
+ * maximum size.*/
+const size_t max_performance_size[] = {
+[PERF_MS] = 12,
+[PERF_US] = 200,
+};
+
+/* Oldest sample that can exist in the sample vector */
+const unsigned long long performance_oldest[] = {
+[PERF_MS] = 6llu,
+[PERF_US] = 100llu,
+};
+
+const unsigned long long one_min[] = {
+[PERF_MS] = 6llu,
+[PERF_US] = 6000llu,
+};
+
+const unsigned long long thirty_sec[] = {
+[PERF_MS] = 3llu,
+[PERF_US] = 3000llu,
+};
+
+const unsigned long long ten_sec[] = {
+[PERF_MS] = 1llu,
+[PERF_US] = 1000llu,
+};
+
+const char *unit_name[] = {
+[PERF_MS] = "msec",
+[PERF_US] = "usec",
+};
+
+static void
+add_sample(struct sample_vec *vec, struct sample *new_sample)
+{
+if (vec->capacity == vec->n_samples) {
+if 

[ovs-dev] [PATCH v3 0/2] Add Performance Measurement Library

[Mark Michelson]

This set of commits adds a new library for OVS that allows for measuring
the performance of operations in OVS and compiling statistics from these
measurements.

For developers, this can provide a measurement of something that is
either finer or coarser-grained than what is easily measured with a
profiler.

v2 -> v3:
* A background thread has been introduced that maintains the performance
  structures.
* To reduce contention in the threads that are reporting performance
  measurements, they no longer acquire a mutex. Instead, they pass
  information over a pipe to the background thread.
* To reduce memory usage, the sample vectors have a maximum size they
  can reach. In addition, the measured intervals are now smaller. Rather
  than one minute, five minute, and ten minute measurements, we now do
  ten second, thirty second, and one minute measurements.

v1 -> v2:
In version 1, there was a patch included that would save statistics to
the OVS database. Based on feedback from that, I decided to forgo
database support for the time being. If database support were to be
added, using a time series database rather than the OVS database would
be the way to go.

Removal of the database patch had a snowball effect that has reduced the
overall size of the patchset.

Mark Michelson (2):
  Add performance measuring API
  Measure performance of ovn-controller loop.

 lib/automake.mk |   2 +
 lib/performance.c   | 497 
 lib/performance.h   |  41 
 ovn/controller/ovn-controller.c |  17 ++
 4 files changed, 557 insertions(+)
 create mode 100644 lib/performance.c
 create mode 100644 lib/performance.h

-- 
2.13.6

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

[ovs-dev] [PATCH 1/2] ofp-util: Remove prototypes for unimplemented functions.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 include/openvswitch/ofp-util.h | 12 
 1 file changed, 12 deletions(-)

diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 5dd1b34c216b..876ca6ed280a 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -39,7 +39,6 @@ extern "C" {
 #endif
 
 struct ofpbuf;
-union ofp_action;
 struct ofpact_set_field;
 struct vl_mff_map;
 
@@ -1190,17 +1189,6 @@ struct ofpbuf *make_echo_reply(const struct ofp_header 
*rq);
 
 struct ofpbuf *ofputil_encode_barrier_request(enum ofp_version);
 
-/* Actions. */
-
-bool action_outputs_to_port(const union ofp_action *, ovs_be16 port);
-
-enum ofperr ofputil_pull_actions(struct ofpbuf *, unsigned int actions_len,
- union ofp_action **, size_t *);
-
-bool ofputil_actions_equal(const union ofp_action *a, size_t n_a,
-   const union ofp_action *b, size_t n_b);
-union ofp_action *ofputil_actions_clone(const union ofp_action *, size_t n);
-
 /* Handy utility for parsing flows and actions. */
 bool ofputil_parse_key_value(char **stringp, char **keyp, char **valuep);
 
-- 
2.15.1

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

Re: [ovs-dev] [patch v5 00/11] Userspace datapath: Add fragmentation support.

[Aaron Conole]

Hi Darrell,

Darrell Ball  writes:

> Fragmentation support for userspace datapath conntrack is added; both
> v4 and v6 are supported. See the patches for additional details.

Very pumped about this!

I went to start reviewing this, but found out that 04/ didn't apply
correctly.  No problem, I'll proceed - just figured I'd let you know
that:

  1. I'm looking at it :)
  2. it'll need at least one more spin (but no need to rush that since
 I'll move myself past the error)

Thanks,
-Aaron

> v4->v5: Added a sub-feature to optionally dump fragmentation lists.
> This is useful for DOS forensics and debugging.
>
> The testing coverage was also extended including checking
> more counters and frag list occupancies.
>
> Fixed a few bugs: 
> 1/ Handle dpdk mempool source restrictions for a batch of
>packets from multiple sources; this also brings in a purge
>frag list function to handle pathological cases of stuck frags.
> 2/ ipf_destroy was missing packet frees for frag lists.
> 3/ A setting of CS_INVALID was missing for expired packets -
>I mentioned this earlier for version 4.
>
> Some enhancements and coding standards changes for Patch 3.   
>
> v3->v4: Add V6 support to the patches.
> Fix possible race cleanup bug when the user disables
>fragmentation and there are list occupancies, not cleaned up
>yet.
> Add missed orig tuple fields for copy from reassembled packet
> to fragments.
> Fix an fragment list increment check - shoiuld have been "> 0"
> rather then "!= 0".
> Fix max frags calculation in case of theoretical corner case.
> Add proper lock annotations.
> Made some other improvements while adding V6 support.
>
> v2->v3: Patch 2 was updated:
> Remove "XXX" todo items by implementing the ones needed,
> including realloc frag_list contexts to save memory.
> Fix related bug with max_frag_list_size when min_frag_size is
> reconfigured.
>
> Tighten ip_tot_len sanity check for reassembled packets which
> was more loose than intended.
>
> Add another sanity check for fragment ip_tot_len; even though
> it be redundant, add for completeness.
>
> v1->v2: Few fixes, improvements and cleanups.
>
> Darrell Ball (11):
>   dp-packet: Add const qualifiers for checksum apis.
>   flow: Enhance parse_ipv6_ext_hdrs.
>   Userspace datapath: Add fragmentation handling.
>   conntrack: Support fragmentation.
>   ipf: Add command to enable fragmentation handling.
>   ipf: Add set minimum fragment size command.
>   ipf: Add set maximum fragments supported command.
>   ipf: Add command to get fragmentation handling status.
>   ipf: Enhance ipf_get_status.
>   tests: Add missed local stack checks.
>   tests: Enable fragmentation for userspace datapath.
>
>  NEWS |   10 +
>  include/sparse/netinet/ip6.h |1 +
>  lib/automake.mk  |2 +
>  lib/conntrack.c  |   10 +-
>  lib/ct-dpif.c|   69 ++
>  lib/ct-dpif.h|   13 +
>  lib/dp-packet.h  |4 +-
>  lib/dpctl.c  |  216 ++
>  lib/dpctl.man|   32 +
>  lib/dpif-netdev.c|   83 +++
>  lib/dpif-netlink.c   |7 +
>  lib/dpif-provider.h  |   18 +
>  lib/flow.c   |   23 +-
>  lib/flow.h   |3 +-
>  lib/ipf.c| 1390 
> ++
>  lib/ipf.h|   86 +++
>  tests/system-kmod-macros.at  |   30 +-
>  tests/system-traffic.at  |   45 +-
>  tests/system-userspace-macros.at |  125 +++-
>  19 files changed, 2129 insertions(+), 38 deletions(-)
>  create mode 100644 lib/ipf.c
>  create mode 100644 lib/ipf.h
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] xlate: auto ofproto trace when recursion too deep

[William Tu]

Usually ofproto/trace is used to debug the flow translation error.
When translation error such as recursion too deep or too many resubmit,
the issue might happen momentary; flows causing the recursion expire
when users try to debug it.  This patch enables the ofproto trace
automatically when recursion is too deep or too many resubmit, by
invoking the translation again, and log the ofproto trace as warnings.

VMWare-BZ: #2054659
Signed-off-by: William Tu 
---
 ofproto/ofproto-dpif-trace.c  |  7 +--
 ofproto/ofproto-dpif-trace.h  |  6 ++
 ofproto/ofproto-dpif-upcall.c | 19 ++-
 tests/ofproto-dpif.at | 22 ++
 4 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 3811e03b106d..25b018d48207 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -24,11 +24,6 @@
 #include "openvswitch/ofp-parse.h"
 #include "unixctl.h"
 
-static void ofproto_trace(struct ofproto_dpif *, const struct flow *,
-  const struct dp_packet *packet,
-  const struct ofpact[], size_t ofpacts_len,
-  struct ovs_list *next_ct_states,
-  struct ds *);
 static void oftrace_node_destroy(struct oftrace_node *);
 
 /* Creates a new oftrace_node, populates it with the given 'type' and a copy of
@@ -662,7 +657,7 @@ ofproto_trace__(struct ofproto_dpif *ofproto, const struct 
flow *flow,
  *
  * If 'ofpacts' is nonnull then its 'ofpacts_len' bytes specify the actions to
  * trace, otherwise the actions are determined by a flow table lookup. */
-static void
+void
 ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
   const struct dp_packet *packet,
   const struct ofpact ofpacts[], size_t ofpacts_len,
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index 5e51771b19b0..ea6cb3fc01ac 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -28,6 +28,8 @@
  * each action (OFT_ACTION) executed in the table.
  */
 
+#include "openvswitch/dynamic-string.h"
+#include "ofproto/ofproto-dpif.h"
 #include "openvswitch/compiler.h"
 #include "openvswitch/list.h"
 #include "flow.h"
@@ -79,6 +81,10 @@ struct oftrace_next_ct_state {
 };
 
 void ofproto_dpif_trace_init(void);
+void ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
+  const struct dp_packet *packet,
+  const struct ofpact *, size_t ofpacts_len,
+  struct ovs_list *next_ct_states, struct ds *output);
 
 struct oftrace_node *oftrace_report(struct ovs_list *, enum oftrace_node_type,
 const char *text);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 035233d5adc8..05064c8b85fa 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -34,6 +34,7 @@
 #include "ofproto-dpif-sflow.h"
 #include "ofproto-dpif-xlate.h"
 #include "ofproto-dpif-xlate-cache.h"
+#include "ofproto-dpif-trace.h"
 #include "ovs-rcu.h"
 #include "packets.h"
 #include "openvswitch/poll-loop.h"
@@ -1128,7 +1129,9 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
  struct ofpbuf *odp_actions, struct flow_wildcards *wc)
 {
 struct dpif_flow_stats stats;
+enum xlate_error xerr;
 struct xlate_in xin;
+struct ds output;
 
 stats.n_packets = 1;
 stats.n_bytes = dp_packet_size(upcall->packet);
@@ -1163,7 +1166,21 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
 upcall->dump_seq = seq_read(udpif->dump_seq);
 upcall->reval_seq = seq_read(udpif->reval_seq);
 
-xlate_actions(, >xout);
+xerr = xlate_actions(, >xout);
+
+/* Translate again and log the ofproto trace for
+ * these two error types. */
+if (xerr == XLATE_RECURSION_TOO_DEEP ||
+xerr == XLATE_TOO_MANY_RESUBMITS) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+ds_init();
+ofproto_trace(upcall->ofproto, upcall->flow,
+  upcall->packet, NULL, 0, NULL, );
+VLOG_WARN_RL(, "%s", ds_cstr());
+ds_destroy();
+}
+
 if (wc) {
 /* Convert the input port wildcard from OFP to ODP format. There's no
  * real way to do this for arbitrary bitmasks since the numbering 
spaces
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 600afdda8528..776c5d34fae2 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -8473,6 +8473,28 @@ AT_CHECK([grep -c 'over max translation depth 64' 
stdout],
 OVS_VSWITCHD_STOP(["/resubmit actions recursed/d"])
 AT_CLEANUP
 
+dnl Without using ofproto/trace, make sure the
+dnl ofproto trace is still logged
+AT_SETUP([ofproto-dpif - backward resubmit without trace])
+OVS_VSWITCHD_START
+(echo "table=0, actions=resubmit(,66)"
+ for i in `seq 2 66`; do
+j=`expr $i 

Re: [ovs-dev] [RFC 3/3] OVN: add acl reject rule support using icmp4 action

[Lorenzo Bianconi]

> On Fri, Feb 09, 2018 at 11:13:09AM +0100, Lorenzo Bianconi wrote:
>> On Jan 23, Ben Pfaff wrote:
>> > On Wed, Jan 10, 2018 at 06:59:01PM +0100, Lorenzo Bianconi wrote:
>> > > Whenever the acl reject rule is hit send back an ICMPv4 destination
>> > > unreachable packet and do not handle reject rule as drop one
>> > >
>> > > Signed-off-by: Lorenzo Bianconi 
>> >
>> > It's nice to finally get this right!  Thank you.
>> >
>> > I wonder about the treatment for TCP connections.  A connection attempt
>> > to a TCP port that is not listening ordinarily yields a TCP RST
>> > response.  I do not know whether an ICMP reply is acceptable.  Do you
>> > have any thoughts on that?
>> >
>>
>> I agree, we need to add tcp feature, I was thinking to send a different 
>> patchset adding tcp stuff.
>> Do you prefer to squash tcp action to this patchset or repin it with your 
>> comments?
>
> It's OK with me to do TCP in a different patch set.  It takes extra work
> to write code to generate TCP RSTs.  I don't want to delay these patches
> by requiring that extra work now.  I would like to see the TCP work
> done, however.

ack, I will send a new patchset soon

>
> For this patch set, do you think it is better to send ICMP for TCP or to
> continue treating reject as drop for TCP?
>

I guess we can maintain the standard 'drop' action for TCP connections
for the moment

> Thanks,
>
> Ben.

Thanks.
Regards,

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

Re: [ovs-dev] [RFC PATCH 1/1]: Enhance conjunctive match support in OVN

[Numan Siddique]

On Feb 9, 2018 11:19 PM, "Ben Pfaff"  wrote:

Oh, and Numan, would you mind including these results or a least a link
to them in the archives in the next version of the patch?  It's OK if
they aren't completely up-to-date, the idea is to make it clear from
someone reading the commit log later that there was a big benefit.


Yeah. Sure. I will do that.

Thanks Mark for testing it out and sharing the results.

Thanks
Numan


On Fri, Feb 09, 2018 at 09:41:57AM -0800, Ben Pfaff wrote:
> Those are fantastic results, thanks for passing them along!
>
> On Thu, Feb 08, 2018 at 05:11:26PM -0600, Mark Michelson wrote:
> > Hi Numan,
> >
> > I did a test with this where I created two address sets with ~1000
addresses
> > in them. Then I created a series of ACLs that used these two address
sets in
> > ACLs like so:
> >
> > ovn-nbctl acl-add ls0 to-lport 1000 "outport == \"lsp${COUNT}\" &&
ip4.src
> > == \$set1 && ip4.dst == \$set2" allow
> >
> > The ${COUNT} variable would increase with each loop iteration.
> >
> > Without your patch, after adding 3 ACLs, it took multiple minutes to
add the
> > next ACL and ground my system to a halt.
> >
> > With your patch, I was able to add hundreds of ACLs. While I did see
> > processing slow down, it was several orders of magnitude better than
without
> > your patch.
> >
> > It makes sense. Without your patch, each ACL results in ~1 million flows
> > being generated. With your patch, each ACL results in ~2000 flows being
> > generated.
> >
> > Definitely an improvement! If you make a v2, I'll test it as well.
> >
> > Tested-by: Mark Michelson 
> >
> >
> > On 02/02/2018 09:36 AM, nusid...@redhat.com wrote:
> > >From: Numan Siddique 
> > >
> > >Presently, if a logical flow has possible conjunctive matches, OVN
expression
> > >parser has support for that. But certain fields like ip4.src, ip4.dst
are not
> > >considered as dimensions in the conjunctive matches.
> > >
> > >In order to support all the possible fields as dimensions, this patch
has added
> > >a new expression type 'EXPR_T_CONJ'. After a expression is simplified
by
> > >expr_simplify(), before calling expr_normalize(), a new function
> > >expr_eval_conj() is called, which evaluates for possible dimensions for
> > >conjunctive matches.
> > >
> > >For example if the match expression is
> > >"ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6} &&
> > >  ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}"
> > >
> > >expr_simplify() would have generated the expression as -
> > >
> > >AND(CMP(IP4),
> > > OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
> > > CMP(ip4.src == 10.0.0.6)),
> > > OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.src == 20.0.0.5),
> > > CMP(ip4.src == 20.0.0.6))).
> > >
> > >expr_eval_conj() would return a new expression something like
> > >
> > >CONJ(AND(CMP(IP4),
> > >  OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
> > >  CMP(ip4.src == 10.0.0.6))),
> > >  AND(CMP(IP4),
> > >  OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.dst == 20.0.0.5),
> > >  CMP(ip4.dst == 20.0.0.6
> > >
> > >expr_normalize() would normalize each individual 'AND' clause in the
CONJ and
> > >expr_to_matches() would add the necessary conjunctive matches.
> > >
> > >TODO: If the proposed approach is reasonable, then test cases and
necessary
> > >code comments needs to be added.
> > >
> > >Signed-off-by: Numan Siddique 
> > >---
> > >  include/ovn/expr.h |   7 ++-
> > >  ovn/controller/lflow.c |   2 +
> > >  ovn/lib/expr.c | 166 ++
+++
> > >  tests/test-ovn.c   |   2 +-
> > >  4 files changed, 175 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> > >index 711713e08..4259e5938 100644
> > >--- a/include/ovn/expr.h
> > >+++ b/include/ovn/expr.h
> > >@@ -295,6 +295,7 @@ enum expr_type {
> > >  EXPR_T_CONDITION,   /* Conditional to be evaluated in the
> > >   * controller during expr_simplify(),
> > >   * prior to constructing OpenFlow
matches. */
> > >+EXPR_T_CONJ,
> > >  };
> > >  /* Expression condition type. */
> > >@@ -366,12 +367,16 @@ struct expr {
> > >  /* XXX Should arguments for conditions be generic? */
> > >  char *string;
> > >  } cond;
> > >+
> > >+/* EXPR_T_CONJ. */
> > >+struct ovs_list conj;
> > >  };
> > >  };
> > >  struct expr *expr_create_boolean(bool b);
> > >  struct expr *expr_create_andor(enum expr_type);
> > >  struct expr *expr_combine(enum expr_type, struct expr *a, struct
expr *b);
> > >+struct expr *expr_create_conj(enum expr_type);
> > >  static inline struct expr *
> > >  expr_from_node(const struct ovs_list *node)
> > >@@ -500,5 +505,5 @@ void expr_addr_sets_add(struct shash *addr_sets,
const char *name,
> > >  

Re: [ovs-dev] [PATCH] ovn: Allow DNS lookups over IPv6

[Ben Pfaff]

On Fri, Feb 09, 2018 at 11:45:17AM -0600, Mark Michelson wrote:
> On 02/09/2018 11:35 AM, Ben Pfaff wrote:
> >On Fri, Feb 09, 2018 at 09:11:00AM -0600, Mark Michelson wrote:
> >>There was a bug in DNS request handling where the incoming packet was
> >>assumed to be IPv4.
> >>
> >>The result was that for the outgoing packet, we would attempt to write
> >>the IPv4 checksum and total length into what was actually an IPv6
> >>header. This resulted in the source IPv6 address getting corrupted.
> >>Later, the source and destination IPv6 addresses would get swapped,
> >>resulting in the DNS response being sent to a nonsense destination.
> >>
> >>With this change, we check the ethertype of the packet to determine what
> >>l3 information to write, and where to write it. A test is also included
> >>that verifies that this works as expected.
> >>
> >>Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1539608
> >>Signed-off-by: Mark Michelson 
> >
> >Thank you for the fix and the test!  I applied this to master and
> >branch-2.9.  Let me know if you want it backported further.
> >
> >I folded in the following nonessential correction pointed out by
> >"sparse":
> >
> >--8<--cut here-->8--
> >
> >diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> >index 6654d7f49396..14c95ff547ff 100644
> >--- a/ovn/controller/pinctrl.c
> >+++ b/ovn/controller/pinctrl.c
> >@@ -917,7 +917,7 @@ pinctrl_handle_dns_lookup(
> >  out_udp->udp_csum = 0;
> >  struct eth_header *eth = dp_packet_data(_out);
> >-if (eth->eth_type == ntohs(ETH_TYPE_IP)) {
> >+if (eth->eth_type == htons(ETH_TYPE_IP)) {
> >  struct ip_header *out_ip = dp_packet_l3(_out);
> >  out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs
> > + new_l4_size);
> >
> 
> Thanks Ben. This error is present in 2.8 as well.

OK, I backported it there too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

[Ben Pfaff]

On Tue, Feb 06, 2018 at 05:17:19PM +0530, SatyaValli wrote:
> From: SatyaValli 
> 
> This Patch provides implementation Existing flow entry statistics are
> redefined as standard OXS(OpenFlow Extensible Statistics) fields for
> displaying the arbitrary flow stats.The existing Flow Stats were renamed
> as Flow Description.

Thanks for the updated patch.

After a small amount of reading, I agree with Jan Scheurich.  "ovs-ofctl
dump-flows" should provide similar results for all versions of OpenFlow.
In OpenFlow 1.5, that means that it should use a "flow desc" request
rather than a "flow stats" request.

Thanks,

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

Re: [ovs-dev] [PATCH] ovn: Allow DNS lookups over IPv6

[Mark Michelson]

On 02/09/2018 11:35 AM, Ben Pfaff wrote:

On Fri, Feb 09, 2018 at 09:11:00AM -0600, Mark Michelson wrote:

There was a bug in DNS request handling where the incoming packet was
assumed to be IPv4.

The result was that for the outgoing packet, we would attempt to write
the IPv4 checksum and total length into what was actually an IPv6
header. This resulted in the source IPv6 address getting corrupted.
Later, the source and destination IPv6 addresses would get swapped,
resulting in the DNS response being sent to a nonsense destination.

With this change, we check the ethertype of the packet to determine what
l3 information to write, and where to write it. A test is also included
that verifies that this works as expected.

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


Thank you for the fix and the test!  I applied this to master and
branch-2.9.  Let me know if you want it backported further.

I folded in the following nonessential correction pointed out by
"sparse":

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

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 6654d7f49396..14c95ff547ff 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -917,7 +917,7 @@ pinctrl_handle_dns_lookup(
  out_udp->udp_csum = 0;
  
  struct eth_header *eth = dp_packet_data(_out);

-if (eth->eth_type == ntohs(ETH_TYPE_IP)) {
+if (eth->eth_type == htons(ETH_TYPE_IP)) {
  struct ip_header *out_ip = dp_packet_l3(_out);
  out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs
 + new_l4_size);



Thanks Ben. This error is present in 2.8 as well.
Mark
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 1/1]: Enhance conjunctive match support in OVN

[Ben Pfaff]

Those are fantastic results, thanks for passing them along!

On Thu, Feb 08, 2018 at 05:11:26PM -0600, Mark Michelson wrote:
> Hi Numan,
> 
> I did a test with this where I created two address sets with ~1000 addresses
> in them. Then I created a series of ACLs that used these two address sets in
> ACLs like so:
> 
> ovn-nbctl acl-add ls0 to-lport 1000 "outport == \"lsp${COUNT}\" && ip4.src
> == \$set1 && ip4.dst == \$set2" allow
> 
> The ${COUNT} variable would increase with each loop iteration.
> 
> Without your patch, after adding 3 ACLs, it took multiple minutes to add the
> next ACL and ground my system to a halt.
> 
> With your patch, I was able to add hundreds of ACLs. While I did see
> processing slow down, it was several orders of magnitude better than without
> your patch.
> 
> It makes sense. Without your patch, each ACL results in ~1 million flows
> being generated. With your patch, each ACL results in ~2000 flows being
> generated.
> 
> Definitely an improvement! If you make a v2, I'll test it as well.
> 
> Tested-by: Mark Michelson 
> 
> 
> On 02/02/2018 09:36 AM, nusid...@redhat.com wrote:
> >From: Numan Siddique 
> >
> >Presently, if a logical flow has possible conjunctive matches, OVN expression
> >parser has support for that. But certain fields like ip4.src, ip4.dst are not
> >considered as dimensions in the conjunctive matches.
> >
> >In order to support all the possible fields as dimensions, this patch has 
> >added
> >a new expression type 'EXPR_T_CONJ'. After a expression is simplified by
> >expr_simplify(), before calling expr_normalize(), a new function
> >expr_eval_conj() is called, which evaluates for possible dimensions for
> >conjunctive matches.
> >
> >For example if the match expression is
> >"ip4 && ip4.src == {10.0.0.4, 10.0.0.5, 10.0.0.6} &&
> >  ip4.dst == {20.0.0.4, 20.0.0.5, 20.0.0.6}"
> >
> >expr_simplify() would have generated the expression as -
> >
> >AND(CMP(IP4),
> > OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
> > CMP(ip4.src == 10.0.0.6)),
> > OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.src == 20.0.0.5),
> > CMP(ip4.src == 20.0.0.6))).
> >
> >expr_eval_conj() would return a new expression something like
> >
> >CONJ(AND(CMP(IP4),
> >  OR((CMP(ip4.src == 10.0.0.4), CMP(ip4.src == 10.0.0.5),
> >  CMP(ip4.src == 10.0.0.6))),
> >  AND(CMP(IP4),
> >  OR((CMP(ip4.dst == 20.0.0.4), CMP(ip4.dst == 20.0.0.5),
> >  CMP(ip4.dst == 20.0.0.6
> >
> >expr_normalize() would normalize each individual 'AND' clause in the CONJ and
> >expr_to_matches() would add the necessary conjunctive matches.
> >
> >TODO: If the proposed approach is reasonable, then test cases and necessary
> >code comments needs to be added.
> >
> >Signed-off-by: Numan Siddique 
> >---
> >  include/ovn/expr.h |   7 ++-
> >  ovn/controller/lflow.c |   2 +
> >  ovn/lib/expr.c | 166 
> > +
> >  tests/test-ovn.c   |   2 +-
> >  4 files changed, 175 insertions(+), 2 deletions(-)
> >
> >diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> >index 711713e08..4259e5938 100644
> >--- a/include/ovn/expr.h
> >+++ b/include/ovn/expr.h
> >@@ -295,6 +295,7 @@ enum expr_type {
> >  EXPR_T_CONDITION,   /* Conditional to be evaluated in the
> >   * controller during expr_simplify(),
> >   * prior to constructing OpenFlow matches. 
> > */
> >+EXPR_T_CONJ,
> >  };
> >  /* Expression condition type. */
> >@@ -366,12 +367,16 @@ struct expr {
> >  /* XXX Should arguments for conditions be generic? */
> >  char *string;
> >  } cond;
> >+
> >+/* EXPR_T_CONJ. */
> >+struct ovs_list conj;
> >  };
> >  };
> >  struct expr *expr_create_boolean(bool b);
> >  struct expr *expr_create_andor(enum expr_type);
> >  struct expr *expr_combine(enum expr_type, struct expr *a, struct expr *b);
> >+struct expr *expr_create_conj(enum expr_type);
> >  static inline struct expr *
> >  expr_from_node(const struct ovs_list *node)
> >@@ -500,5 +505,5 @@ void expr_addr_sets_add(struct shash *addr_sets, const 
> >char *name,
> >  const char * const *values, size_t n_values);
> >  void expr_addr_sets_remove(struct shash *addr_sets, const char *name);
> >  void expr_addr_sets_destroy(struct shash *addr_sets);
> >-
> >+struct expr *expr_eval_conj(struct expr *expr);
> >  #endif /* ovn/expr.h */
> >diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> >index 1e79a5355..13413c77d 100644
> >--- a/ovn/controller/lflow.c
> >+++ b/ovn/controller/lflow.c
> >@@ -289,6 +289,7 @@ consider_logical_flow(struct controller_ctx *ctx,
> >  expr = expr_combine(EXPR_T_AND, expr, prereqs);
> >  prereqs = NULL;
> >  }
> >+
> >  expr = expr_annotate(expr, , );
> >  }
> >  

Re: [ovs-dev] [PATCH] ovn: Allow DNS lookups over IPv6

[Ben Pfaff]

On Fri, Feb 09, 2018 at 09:11:00AM -0600, Mark Michelson wrote:
> There was a bug in DNS request handling where the incoming packet was
> assumed to be IPv4.
> 
> The result was that for the outgoing packet, we would attempt to write
> the IPv4 checksum and total length into what was actually an IPv6
> header. This resulted in the source IPv6 address getting corrupted.
> Later, the source and destination IPv6 addresses would get swapped,
> resulting in the DNS response being sent to a nonsense destination.
> 
> With this change, we check the ethertype of the packet to determine what
> l3 information to write, and where to write it. A test is also included
> that verifies that this works as expected.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1539608
> Signed-off-by: Mark Michelson 

Thank you for the fix and the test!  I applied this to master and
branch-2.9.  Let me know if you want it backported further.

I folded in the following nonessential correction pointed out by
"sparse":

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

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 6654d7f49396..14c95ff547ff 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -917,7 +917,7 @@ pinctrl_handle_dns_lookup(
 out_udp->udp_csum = 0;
 
 struct eth_header *eth = dp_packet_data(_out);
-if (eth->eth_type == ntohs(ETH_TYPE_IP)) {
+if (eth->eth_type == htons(ETH_TYPE_IP)) {
 struct ip_header *out_ip = dp_packet_l3(_out);
 out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs
+ new_l4_size);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 3/3] OVN: add acl reject rule support using icmp4 action

[Ben Pfaff]

On Fri, Feb 09, 2018 at 11:13:09AM +0100, Lorenzo Bianconi wrote:
> On Jan 23, Ben Pfaff wrote:
> > On Wed, Jan 10, 2018 at 06:59:01PM +0100, Lorenzo Bianconi wrote:
> > > Whenever the acl reject rule is hit send back an ICMPv4 destination
> > > unreachable packet and do not handle reject rule as drop one
> > > 
> > > Signed-off-by: Lorenzo Bianconi 
> > 
> > It's nice to finally get this right!  Thank you.
> > 
> > I wonder about the treatment for TCP connections.  A connection attempt
> > to a TCP port that is not listening ordinarily yields a TCP RST
> > response.  I do not know whether an ICMP reply is acceptable.  Do you
> > have any thoughts on that?
> > 
> 
> I agree, we need to add tcp feature, I was thinking to send a different 
> patchset adding tcp stuff.
> Do you prefer to squash tcp action to this patchset or repin it with your 
> comments?

It's OK with me to do TCP in a different patch set.  It takes extra work
to write code to generate TCP RSTs.  I don't want to delay these patches
by requiring that extra work now.  I would like to see the TCP work
done, however.

For this patch set, do you think it is better to send ICMP for TCP or to
continue treating reject as drop for TCP?

Thanks,

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

[ovs-dev] [PATCH] ovn: Allow DNS lookups over IPv6

[Mark Michelson]

There was a bug in DNS request handling where the incoming packet was
assumed to be IPv4.

The result was that for the outgoing packet, we would attempt to write
the IPv4 checksum and total length into what was actually an IPv6
header. This resulted in the source IPv6 address getting corrupted.
Later, the source and destination IPv6 addresses would get swapped,
resulting in the DNS response being sent to a nonsense destination.

With this change, we check the ethertype of the packet to determine what
l3 information to write, and where to write it. A test is also included
that verifies that this works as expected.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1539608
Signed-off-by: Mark Michelson 
---
 ovn/controller/pinctrl.c | 17 +++-
 tests/ovn.at | 51 
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index faeb3f932..6654d7f49 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -916,11 +916,18 @@ pinctrl_handle_dns_lookup(
 out_udp->udp_len = htons(new_l4_size);
 out_udp->udp_csum = 0;
 
-struct ip_header *out_ip = dp_packet_l3(_out);
-out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs + new_l4_size);
-/* Checksum needs to be initialized to zero. */
-out_ip->ip_csum = 0;
-out_ip->ip_csum = csum(out_ip, sizeof *out_ip);
+struct eth_header *eth = dp_packet_data(_out);
+if (eth->eth_type == ntohs(ETH_TYPE_IP)) {
+struct ip_header *out_ip = dp_packet_l3(_out);
+out_ip->ip_tot_len = htons(pkt_out.l4_ofs - pkt_out.l3_ofs
+   + new_l4_size);
+/* Checksum needs to be initialized to zero. */
+out_ip->ip_csum = 0;
+out_ip->ip_csum = csum(out_ip, sizeof *out_ip);
+} else {
+struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(_out);
+nh->ip6_plen = htons(new_l4_size);
+}
 
 pin->packet = dp_packet_data(_out);
 pin->packet_len = dp_packet_size(_out);
diff --git a/tests/ovn.at b/tests/ovn.at
index 1632a981b..00d26e757 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6865,6 +6865,38 @@ test_dns() {
 as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $request
 }
 
+test_dns6() {
+local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 dns_reply=$6
+local dns_query_data=$7
+shift; shift; shift; shift; shift; shift; shift;
+# Packet size => UDP header (8) +
+#DNS data (header + query)
+ip_len=`expr 8 + ${#dns_query_data} / 2`
+udp_len=$ip_len
+ip_len=$(printf "%x" $ip_len)
+udp_len=$(printf "%x" $udp_len)
+local 
request=${dst_mac}${src_mac}86dd60${ip_len}11ff${src_ip}${dst_ip}
+request=${request}9234003500${udp_len}
+#dns data
+request=${request}${dns_query_data}
+
+if test $dns_reply != 0; then
+local dns_reply=$1
+ip_len=`expr 8 + ${#dns_reply} / 2`
+udp_len=$ip_len
+ip_len=$(printf "%x" $ip_len)
+udp_len=$(printf "%x" $udp_len)
+local 
reply=${src_mac}${dst_mac}86dd60${ip_len}11ff${dst_ip}${src_ip}
+reply=${reply}0035923400${udp_len}${dns_reply}
+echo $reply >> $inport.expected
+else
+for outport; do
+echo $request >> $outport.expected
+done
+fi
+as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $request
+}
+
 AT_CAPTURE_FILE([ofctl_monitor0.log])
 as hv1 ovs-ofctl monitor br-int resume --detach --no-chdir \
 --pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log
@@ -7060,6 +7092,25 @@ reset_pcap_file hv1-vif2 hv1/vif2
 rm -f 1.expected
 rm -f 2.expected
 
+# Try DNS query over IPv6
+set_dns_params vm1
+src_ip=aef4
+dst_ip=aef1
+dns_reply=1
+test_dns6 1 f001 f0f0 $src_ip $dst_ip $dns_reply $dns_req_data 
$dns_resp_data
+
+# NXT_RESUMEs should be 9.
+OVS_WAIT_UNTIL([test 9 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets
+cat 1.expected > expout
+AT_CHECK([cat 1.packets], [0], [expout])
+
+reset_pcap_file hv1-vif1 hv1/vif1
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 1.expected
+rm -f 2.expected
+
 as hv1
  OVS_APP_EXIT_AND_WAIT([ovn-controller])
 OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
-- 
2.13.6

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

[ovs-dev] [PATCH v2 1/1] netdev-vport: reject concomitant incompatible tunnels

[Eelco Chaudron]

This patch will make sure VXLAN tunnels with and without the group
based policy (GBP) option enabled can not coexist on the same
destination UDP port.

In theory, VXLAN tunnel with and without GBP enables can be
multiplexed on the same UDP port as long as different VNI's are
used. However currently OVS does not support this, hence this patch to
check for this condition.

v1->v2
  Updated commit message as its now clear that the OVS  implementation
  does not support VNI multiplexing on the same UDP port.

Signed-off-by: Eelco Chaudron 
---
 lib/netdev-vport.c | 97 --
 tests/tunnel.at| 29 
 2 files changed, 108 insertions(+), 18 deletions(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 52aa12d79..bc8226d62 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -66,11 +66,13 @@ static uint64_t rt_change_seqno;
 static int get_patch_config(const struct netdev *netdev, struct smap *args);
 static int get_tunnel_config(const struct netdev *, struct smap *args);
 static bool tunnel_check_status_change__(struct netdev_vport *);
+static const struct vport_class *netdev_vport_get_class_by_name(const char *);
 
 struct vport_class {
 const char *dpif_port;
 struct netdev_class netdev_class;
 };
+static const struct vport_class vport_classes[];
 
 bool
 netdev_vport_is_vport_class(const struct netdev_class *class)
@@ -421,6 +423,43 @@ default_pt_mode(enum tunnel_layers layers)
 return layers == TNL_L3 ? NETDEV_PT_LEGACY_L3 : NETDEV_PT_LEGACY_L2;
 }
 
+static bool
+is_concomitant_vxlan_tunnel_present(struct netdev_vport *dev,
+const struct netdev_tunnel_config 
*tnl_cfg) {
+bool is_present = false;
+struct shash device_shash;
+struct shash_node *node;
+const char *type = netdev_get_type(>up);
+const struct vport_class *vport_class;
+
+if (strcmp(type, "vxlan")) {
+return false;
+}
+
+shash_init(_shash);
+vport_class = netdev_vport_get_class_by_name("vxlan");
+ovs_assert(vport_class);
+
+netdev_get_devices(_class->netdev_class, _shash);
+
+SHASH_FOR_EACH (node, _shash) {
+struct netdev *netdev_ = node->data;
+struct netdev_vport *netdev = netdev_vport_cast(netdev_);
+
+if (netdev != dev &&
+tnl_cfg->dst_port == netdev->tnl_cfg.dst_port &&
+(tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GBP)) !=
+(netdev->tnl_cfg.exts & (1 << OVS_VXLAN_EXT_GBP))) {
+is_present = true;
+}
+netdev_close(netdev_);
+}
+
+shash_destroy(_shash);
+
+return is_present;
+}
+
 static int
 set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp)
 {
@@ -614,6 +653,15 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
_cfg.out_key_present,
_cfg.out_key_flow);
 
+if (is_concomitant_vxlan_tunnel_present(dev, _cfg)) {
+ds_put_format(, "%s: VXLAN-GBP, and non-VXLAN-GBP "
+  "tunnels can't be configured on the same "
+  "dst_port\n",
+  name);
+err = EEXIST;
+goto out;
+}
+
 ovs_mutex_lock(>mutex);
 if (memcmp(>tnl_cfg, _cfg, sizeof tnl_cfg)) {
 dev->tnl_cfg = tnl_cfg;
@@ -959,27 +1007,40 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
   BUILD_HEADER, PUSH_HEADER, POP_HEADER,   
\
   GET_IFINDEX) }}
 
+/* The name of the dpif_port should be short enough to accomodate adding
+ * a port number to the end if one is necessary. */
+static const struct vport_class vport_classes[] = {
+TUNNEL_CLASS("geneve", "genev_sys", netdev_geneve_build_header,
+netdev_tnl_push_udp_header,
+netdev_geneve_pop_header,
+NETDEV_VPORT_GET_IFINDEX),
+TUNNEL_CLASS("gre", "gre_sys", netdev_gre_build_header,
+   netdev_gre_push_header,
+   netdev_gre_pop_header,
+   NULL),
+TUNNEL_CLASS("vxlan", "vxlan_sys", netdev_vxlan_build_header,
+   netdev_tnl_push_udp_header,
+   netdev_vxlan_pop_header,
+   NETDEV_VPORT_GET_IFINDEX),
+TUNNEL_CLASS("lisp", "lisp_sys", NULL, NULL, NULL, NULL),
+TUNNEL_CLASS("stt", "stt_sys", NULL, NULL, NULL, NULL),
+};
+
+static const struct vport_class *
+netdev_vport_get_class_by_name(const char *name) {
+int i;
+
+for (i = 0; i < ARRAY_SIZE(vport_classes); i++) {
+if (!strcmp(vport_classes[i].netdev_class.type, name)) {
+return _classes[i];
+}
+}
+return NULL;
+}
+
 void
 

Re: [ovs-dev] question about dp_packet lifetime

[Vincenzo Maffione]

Hi Ilya,
  I see, thanks. So we'll need to take this into account.

Cheers,
  Vincenzo

2018-02-09 12:30 GMT+01:00 Ilya Maximets :

> On 08.02.2018 17:21, Vincenzo Maffione wrote:
> > Hi Ilya,
> >
> >> Hi,
> >
> >
> > Hi, Alessandro.
> >
> > >
> > >   My name is Alessandro Rosetti, and I'm currently adding netmap
> support to
> > > ovs, following an approach similar to DPDK.
> >
> > Good to know that someone started to work on this. IMHO, it's a good
> idea.
> > I also wanted to try to implement this someday, but had no much time.
> >
> > >
> > > I've created a new netdev: netdev_netmap that uses the pmd
> infrastructure.
> > > The prototype I have seems to work fine (I still need to tune
> performance,
> > > test optional features, and test more complex topologies.)
> >
> > Cool. Looking forward for your RFC patch-set.
> >
> > >
> > > I have a question about the lifetime of dp_packets.
> > > Is there any guarantee that the dp_packets allocated in a receive
> callback
> > > (e.g. netdev_netmap_rxq_recv) are consumed by OVS (e.g. dropped,
> cloned, or
> > > sent to other ports) **before** a subsequent call to the receive
> callback
> > > (on the same port)?
> > > Or is it possible for dp_packets to be stored somewhere (e.g. in
> an OVS
> > > internal queue) and live across subsequent invocations of the
> receive
> > > callback that allocated them?
> >
> > I think that there was never such a guarantee, but recent changes in
> userspace
> > datapath completely ruined this assumption. I mean output packet
> batching support.
> >
> > Please refer the following commits for details:
> > 009e003 2017-12-14 | dpif-netdev: Output packet batching.
> > c71ea3c 2018-01-15 | dpif-netdev: Time based output batching.
> > 00adb8d 2018-01-15 | docs: Describe output packet batching in DPDK
> guide.
> >
> >
> > Thanks a lot for for the quick reply. So we will add a dp_packet
> allocator for netmap.
> > Is there is any guarantee that a dp_packet allocated by a PMD thread
> will be deallocated
> > by the same thread and only the same thread? At first glance it seems
> this is the case...
>
> Hi Vincenzo,
>
> I would prefer that you do not make such assumptions. Even if it works now,
> netdev abstraction layer doesn't guarantee that. It states that
> 'rxq_recv()'
> function of netdev-provider "stores pointers to up to NETDEV_MAX_BURST
> dp_packets into the array, transferring ownership of the packets to the
> caller".
> This means that caller is able to do anything with received dp_packets
> including
> but not limited to queueing or transferring to another thread.
>
> >
> > Thanks,
> >   Vincenzo
> >
> >
> >
> > >
> > > I need to know if this is the case to check that my current
> prototype is
> > > safe.
> > > I use per-port pre-allocation of dp_packets, for maximum
> performance. I've
> > > seen that DPDK uses its internal allocator to allocate and
> deallocate
> > > dp_packets, but netmap does not expose one.
> > > Each packet received with netmap is created as a new type
> dp_packet:
> > > DPBUF_NETMAP. The data points to a netmap buffer (preallocated by
> the
> > > kernel).
> > > When I receive data (netdev_netmap_rxq_recv) I reuse the
> dp_packets,
> > > updating the internal pointer and a couple of additional
> informations
> > > stored inside the dp_packet.
> > > When I have to send data I use zero copy if dp_packet is
> DPBUF_NETMAP and
> > > copy if it's not.
> > >
> > > Thanks for the help!
> > > Alessandro.
> >
> >
> >
> >
> >
> >
> > --
> > Vincenzo Maffione
>



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

[ovs-dev] SAP SuccessFactors Users List

[Katelyn Smith]

Hi,

I would like to know if you are interested in acquiring SAP SuccessFactors 
Users List for your marketing campaigns.

These are the information fields that we provide for each contacts: Names, 
Title, Email, Phone, Company Name, Company URL, and Company physical address, 
SIC Code, Industry and Company Size (Revenue and Employee).

Let me know if you have any other target criteria and I will get back to you 
with the counts and pricing.

Regards,
Katelyn Smith
Marketing Executive
to opt out, please reply with Leave Out in the Subject Line.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Donation For The Work Of God

[Mrs Vivian James via dev]

 Donation For The Work Of God

MY GREETINGS TO YOU AND YOUR FAMILY I AM Vivian James IF YOU WOULDN,T MIND I 
HAVE SOMETHING I WILL LIKE TO DISCAUSE WITH YOU I HAVE DECEIDED TO MAKE A 
PLEDGE OF DONATION TO SUPPORT THE LESS PRIVILLEGE UNDER YOUR SUPERVISION.

IF YOU ARE INTERESTED CONTACT ME FOR MORE DETAILS vivian0...@gmail.com.

GOD BLESS US AS I WAIT TO HEAR FROM YOU.

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

Re: [ovs-dev] [RFC 1/3] OVN: add icmp4{} action support

[Jakub Sitnicki]

Hi Lorenzo,

On Wed, Jan 10, 2018 at 05:58 PM GMT, Lorenzo Bianconi wrote:
> icmp4 action is used to replace the IPv4 packet been processed with
> an ICMPv4 packet initialized based on incoming IPv4 one.
> Ethernet and IPv4 fields not listed are not changed:
> - ip.proto = 1
> - ip.frag = 0
> - icmp4.type = 3
> - icmp4.code = 1
> Prerequisite: ip4
>
> Signed-off-by: Lorenzo Bianconi 
> ---
>  include/ovn/actions.h|  7 +++
>  ovn/controller/pinctrl.c | 51 
> 
>  ovn/lib/actions.c| 22 +
>  3 files changed, 80 insertions(+)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 85a484ffa..027562c57 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -63,6 +63,7 @@ struct simap;
>  OVNACT(CT_CLEAR,  ovnact_null)\
>  OVNACT(CLONE, ovnact_nest)\
>  OVNACT(ARP,   ovnact_nest)\
> +OVNACT(ICMP,  ovnact_nest)\
>  OVNACT(ND_NA, ovnact_nest)\
>  OVNACT(GET_ARP,   ovnact_get_mac_bind)\
>  OVNACT(PUT_ARP,   ovnact_put_mac_bind)\
> @@ -438,6 +439,12 @@ enum action_opcode {
>   * The actions, in OpenFlow 1.3 format, follow the action_header.
>   */
>  ACTION_OPCODE_ND_NS,
> +
> +/* "icmp4 { ...actions... }".
> + *
> + * The actions, in OpenFlow 1.3 format, follow the action_header.
> + */
> +ACTION_OPCODE_ICMP,
>  };
>
>  /* Header. */
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 7542db3f4..bae804eff 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -218,6 +218,53 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
> struct match *md,
>  }
>
>  static void
> +pinctrl_handle_icmp(const struct flow *ip_flow, const struct match *md,
> +struct ofpbuf *userdata)
> +{
> +/* This action only works for IP packets, and the switch should only send
> + * us IP packets this way, but check here just to be sure. */
> +if (ip_flow->dl_type != htons(ETH_TYPE_IP)) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +VLOG_WARN_RL(, "ICMP action on non-IP packet (Ethertype 
> %"PRIx16")",
> + ntohs(ip_flow->dl_type));
> +return;
> +}
> +
> +uint64_t packet_stub[128 / 8];
> +struct dp_packet packet;
> +
> +dp_packet_use_stub(, packet_stub, sizeof packet_stub);
> +dp_packet_clear();
> +packet.packet_type = htonl(PT_ETH);
> +
> +struct eth_header *eh = dp_packet_put_zeros(, sizeof *eh);
> +eh->eth_dst = ip_flow->dl_dst;
> +eh->eth_src = ip_flow->dl_src;
> +eh->eth_type = htons(ETH_TYPE_IP);
> +
> +struct ip_header *nh = dp_packet_put_zeros(, sizeof *nh);
> +dp_packet_set_l3(, nh);
> +nh->ip_ihl_ver = 0x45;

Consider using IP_IHL_VER(5, 4) to avoid a magic number and also for
consistency with the rest of the code base.

> +nh->ip_tot_len = htons(sizeof(struct ip_header) +
> +   sizeof(struct icmp_header));
> +nh->ip_proto = 1;

Consider using IPPROTO_ICMP for the same reasons as above.

> +nh->ip_frag_off = 0x40;

Ben has already pointed out that this is a 16-bit value so byte order
matters. I only want to add that there is a IP_DF constant that you
could use for better readability.

> +packet_set_ipv4(, ip_flow->nw_src, ip_flow->nw_dst, 0, 255);
> +
> +struct icmp_header *ih = dp_packet_put_zeros(, sizeof *ih);
> +dp_packet_set_l4(, ih);
> +packet_set_icmp(, 3, 1);

You might want to use ICMP4_DST_UNREACH for type.

> +
> +if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) {
> +eth_push_vlan(, htons(ETH_TYPE_VLAN_8021Q),
> +  ip_flow->vlans[0].tci);
> +}
> +
> +set_actions_and_enqueue_msg(, md, userdata);
> +dp_packet_uninit();
> +}
> +
> +static void
>  pinctrl_handle_put_dhcp_opts(
>  struct dp_packet *pkt_in, struct ofputil_packet_in *pin,
>  struct ofpbuf *userdata, struct ofpbuf *continuation)
> @@ -1013,6 +1060,10 @@ process_packet_in(const struct ofp_header *msg, struct 
> controller_ctx *ctx)
>  pinctrl_handle_nd_ns(, _metadata, );
>  break;
>
> +case ACTION_OPCODE_ICMP:
> +pinctrl_handle_icmp(, _metadata, );
> +break;
> +
>  default:
>  VLOG_WARN_RL(, "unrecognized packet-in opcode %"PRIu32,
>   ntohl(ah->opcode));
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 69a2172a8..66362ad37 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -1186,6 +1186,12 @@ parse_ARP(struct action_context *ctx)
>  }
>
>  static void
> +parse_ICMP(struct action_context *ctx)
> +{
> +parse_nested_action(ctx, OVNACT_ICMP, "ip4");
> +}
> +
> +static void
>  parse_ND_NA(struct action_context *ctx)
>  {
>  

Re: [ovs-dev] question about dp_packet lifetime

[Ilya Maximets]

On 08.02.2018 17:21, Vincenzo Maffione wrote:
> Hi Ilya,
> 
>> Hi,
> 
> 
> Hi, Alessandro.
> 
> >
> >   My name is Alessandro Rosetti, and I'm currently adding netmap 
> support to
> > ovs, following an approach similar to DPDK.
> 
> Good to know that someone started to work on this. IMHO, it's a good idea.
> I also wanted to try to implement this someday, but had no much time.
> 
> >
> > I've created a new netdev: netdev_netmap that uses the pmd 
> infrastructure.
> > The prototype I have seems to work fine (I still need to tune 
> performance,
> > test optional features, and test more complex topologies.)
> 
> Cool. Looking forward for your RFC patch-set.
> 
> >
> > I have a question about the lifetime of dp_packets.
> > Is there any guarantee that the dp_packets allocated in a receive 
> callback
> > (e.g. netdev_netmap_rxq_recv) are consumed by OVS (e.g. dropped, 
> cloned, or
> > sent to other ports) **before** a subsequent call to the receive 
> callback
> > (on the same port)?
> > Or is it possible for dp_packets to be stored somewhere (e.g. in an OVS
> > internal queue) and live across subsequent invocations of the receive
> > callback that allocated them?
> 
> I think that there was never such a guarantee, but recent changes in 
> userspace
> datapath completely ruined this assumption. I mean output packet batching 
> support.
> 
> Please refer the following commits for details:
> 009e003 2017-12-14 | dpif-netdev: Output packet batching.
> c71ea3c 2018-01-15 | dpif-netdev: Time based output batching.
> 00adb8d 2018-01-15 | docs: Describe output packet batching in DPDK guide.
> 
> 
> Thanks a lot for for the quick reply. So we will add a dp_packet allocator 
> for netmap.
> Is there is any guarantee that a dp_packet allocated by a PMD thread will be 
> deallocated
> by the same thread and only the same thread? At first glance it seems this is 
> the case...

Hi Vincenzo,

I would prefer that you do not make such assumptions. Even if it works now,
netdev abstraction layer doesn't guarantee that. It states that 'rxq_recv()'
function of netdev-provider "stores pointers to up to NETDEV_MAX_BURST
dp_packets into the array, transferring ownership of the packets to the caller".
This means that caller is able to do anything with received dp_packets including
but not limited to queueing or transferring to another thread.

> 
> Thanks,
>   Vincenzo
>  
> 
> 
> >
> > I need to know if this is the case to check that my current prototype is
> > safe.
> > I use per-port pre-allocation of dp_packets, for maximum performance. 
> I've
> > seen that DPDK uses its internal allocator to allocate and deallocate
> > dp_packets, but netmap does not expose one.
> > Each packet received with netmap is created as a new type dp_packet:
> > DPBUF_NETMAP. The data points to a netmap buffer (preallocated by the
> > kernel).
> > When I receive data (netdev_netmap_rxq_recv) I reuse the dp_packets,
> > updating the internal pointer and a couple of additional informations
> > stored inside the dp_packet.
> > When I have to send data I use zero copy if dp_packet is DPBUF_NETMAP 
> and
> > copy if it's not.
> >
> > Thanks for the help!
> > Alessandro.
> 
> 
> 
> 
> 
> 
> -- 
> Vincenzo Maffione
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 3/3] OVN: add acl reject rule support using icmp4 action

[Lorenzo Bianconi]

On Jan 23, Ben Pfaff wrote:
> On Wed, Jan 10, 2018 at 06:59:01PM +0100, Lorenzo Bianconi wrote:
> > Whenever the acl reject rule is hit send back an ICMPv4 destination
> > unreachable packet and do not handle reject rule as drop one
> > 
> > Signed-off-by: Lorenzo Bianconi 
> 
> It's nice to finally get this right!  Thank you.
> 
> I wonder about the treatment for TCP connections.  A connection attempt
> to a TCP port that is not listening ordinarily yields a TCP RST
> response.  I do not know whether an ICMP reply is acceptable.  Do you
> have any thoughts on that?
> 

I agree, we need to add tcp feature, I was thinking to send a different 
patchset adding tcp stuff.
Do you prefer to squash tcp action to this patchset or repin it with your 
comments?

> I think that this should add an item to NEWS that describes the new
> feature.
> 
> Thanks,
> 
> Ben.

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

Re: [ovs-dev] [RFC 1/3] OVN: add icmp4{} action support

[Lorenzo Bianconi]

On Jan 23, Ben Pfaff wrote:
> On Wed, Jan 10, 2018 at 06:58:59PM +0100, Lorenzo Bianconi wrote:
> > icmp4 action is used to replace the IPv4 packet been processed with
> > an ICMPv4 packet initialized based on incoming IPv4 one.
> > Ethernet and IPv4 fields not listed are not changed:
> > - ip.proto = 1
> > - ip.frag = 0
> > - icmp4.type = 3
> > - icmp4.code = 1
> > Prerequisite: ip4
> > 
> > Signed-off-by: Lorenzo Bianconi 
> 
> Thanks a lot for working on this!  I'd really like to have more complete
> support for this, for the OVN router.
> 
> This patch should update ovn-sb.xml to reflect the new details and that
> the action is actually implemented.

ack, I will do in the following patchset

> 
> "sparse" reports the following:
> 
> ../ovn/controller/pinctrl.c:251:21: error: incorrect type in assignment 
> (different base types)
> ../ovn/controller/pinctrl.c:251:21:expected restricted ovs_be16 
> [usertype] ip_frag_off
> ../ovn/controller/pinctrl.c:251:21:got int
> 
> and I think it's right, htons() should be used in this assignment:
> 
> nh->ip_frag_off = 0x40;
> 

ack

> There's also this new warning:
> 
> ../ovn/utilities/ovn-trace.c: In function ‘trace_actions’:
> ../ovn/utilities/ovn-trace.c:1762:9: error: enumeration value
> ‘OVNACT_ICMP’ not handled in switch [-Werror=switch]
> 
> That's fixed in patch 2/3, but probably it's better to squash patch 2
> into 1.
> 

ack

> Thanks,
> 
> Ben.


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