[ovs-dev] Automatizar y optimizar el trabajo

[Excel: Fórmulas y funciones]

Buenos día 
Quise aprovechar la oportunidad de hacerte una invitación para tomar nuestro 
curso de 9 horas en 3 sesiones de 3 Horas :
 
Nombre: Excel: Fórmulas y funciones.
Fechas y horas: 
Sábado 20 de junio - Sábado 27 de junio y Sábado 4 de julio
Formato: En línea con interacción en vivo.
Precio:
Individual (1 conexión):Por solo 2,695 + IVA 
Empresarial (3 conexiones):Por solo 7,235 + IVA
Lugar: En Vivo desde su computadora
Instructor: José Chabarría

Ese curso te enseña a utilizar las funciones más comunes de Microsoft Excel, la 
creación de fórmulas,
y a automatizar una hoja de cálculo. Al término del curso, el participante será 
capaz de utilizar con 
soltura la hoja de cálculo de Excel.

Al finalizar el curso, el participante será capaz de:

- Escribir fórmulas dinámicas desde cero.
- Utilizar funciones avanzadas.
- Automatizar y optimizar el trabajo en Excel.
- Construir fórmulas para analizar fechas, campos de texto, valores numéricos y 
matrices.

Solicita información respondiendo a este correo con la palabra Excel, junto con 
los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:
Email Alterno:

Números de Atención: 55 15 54 66 30 - 55 30 16 70 85  

Qué tengas un gran día.
Saludos.


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

Re: [ovs-dev] [PATCH] ovsdb-idl.at: Wait all servers to join the cluster.

[Flavio Leitner]

Hi,

It would be nice to have this applied to branch-2.13 as well.
fbl

On Wed, Jun 10, 2020 at 08:45:38PM -0300, Flavio Leitner wrote:
> The test 'Check Python IDL reconnects to leader - Python3
> (leader only)' fails sometimes when the first ovsdb-server
> gets killed before the others had joined the cluster.
> 
> Fix the function ovsdb_cluster_start_idltest to wait them
> to join the cluster.
> 
> Suggested-by: Ilya Maximets 
> Signed-off-by: Flavio Leitner 
> ---
>  tests/ovsdb-idl.at | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index b5cbee7d9..c045e9264 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -29,6 +29,17 @@ ovsdb_cluster_start_idltest () {
>   ovsdb-server -vraft -vconsole:warn --detach --no-chdir 
> --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i --remote=punix:s$i.ovsdb 
> ${2:+--remote=$2} s$i.db || return $?
> done
> on_exit 'kill `cat s*.pid`'
> +   for i in `seq $n`; do
> + for d in `seq 1 "$OVS_CTL_TIMEOUT"`; do
> +   if ovs-appctl -t $(pwd)/s$i cluster/status ${schema_name} | grep -q 
> 'Status: cluster member'; then
> + break
> +   fi
> +   sleep 1
> + done
> + if ! ovs-appctl -t $(pwd)/s$i cluster/status ${schema_name} | grep -q 
> 'Status: cluster member'; then
> +   return 1
> + fi
> +   done
>  }
>  
>  # ovsdb_cluster_leader [REMOTES] [DATABASE]
> -- 
> 2.26.2
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] ctags: Include new annotations to ctags ignore list.

[Flavio Leitner]

Hi,

It would be nice to have this applied to branch-2.13 as well.
fbl

On Wed, Jun 10, 2020 at 04:49:45PM -0300, Flavio Leitner wrote:
> The annotation OVS_NO_THREAD_SAFETY_ANALYSIS and OVS_LOCKABLE are
> not part of the list, so ctags can't find functions using them.
> 
> The annotation list comes from a regex and to include more items
> make the regex more difficult to read and maintain. Convert to a
> static list because it isn't supposed to change much and there
> is no standard names.
> 
> Also add a comment to remind to keep the list up-to-date.
> 
> Signed-off-by: Flavio Leitner 
> ---
>  Makefile.am| 2 +-
>  acinclude.m4   | 6 +++---
>  include/openvswitch/compiler.h | 2 ++
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index b279303d1..27ef9e4b4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -46,7 +46,7 @@ AM_CPPFLAGS += -DNDEBUG
>  AM_CFLAGS += -fomit-frame-pointer
>  endif
>  
> -AM_CTAGSFLAGS = $(OVS_CTAGS_IDENTIFIERS_LIST)
> +AM_CTAGSFLAGS = -I "$(OVS_CTAGS_IDENTIFIERS_LIST)"
>  
>  if WIN32
>  psep=";"
> diff --git a/acinclude.m4 b/acinclude.m4
> index 8847b8145..054ec2e3c 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -1332,11 +1332,11 @@ AC_DEFUN([OVS_ENABLE_SPARSE],
>  
>  dnl OVS_CTAGS_IDENTIFIERS
>  dnl
> -dnl ctags ignores symbols with extras identifiers. This builds a list of
> -dnl specially handled identifiers to be ignored.
> +dnl ctags ignores symbols with extras identifiers. This is a list of
> +dnl specially handled identifiers to be ignored. [ctags(1) -I ].
>  AC_DEFUN([OVS_CTAGS_IDENTIFIERS],
>  AC_SUBST([OVS_CTAGS_IDENTIFIERS_LIST],
> -   [`printf %s '-I "'; sed -n 's/^#define 
> \(OVS_[A-Z_]\+\)(\.\.\.)$/\1+/p' ${srcdir}/include/openvswitch/compiler.h  | 
> tr \\\n ' ' ; printf '"'`] ))
> +   ["OVS_LOCKABLE OVS_NO_THREAD_SAFETY_ANALYSIS OVS_REQ_RDLOCK+ 
> OVS_ACQ_RDLOCK+ OVS_REQ_WRLOCK+ OVS_ACQ_WRLOCK+ OVS_REQUIRES+ OVS_ACQUIRES+ 
> OVS_TRY_WRLOCK+ OVS_TRY_RDLOCK+ OVS_TRY_LOCK+ OVS_GUARDED_BY+ OVS_EXCLUDED+ 
> OVS_RELEASES+ OVS_ACQ_BEFORE+ OVS_ACQ_AFTER+"]))
>  
>  dnl OVS_PTHREAD_SET_NAME
>  dnl
> diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h
> index 5289a70f6..cf009f826 100644
> --- a/include/openvswitch/compiler.h
> +++ b/include/openvswitch/compiler.h
> @@ -113,6 +113,8 @@
>   *OVS_REQUIRES OVS_REQ_RDLOCK   OVS_REQ_WRLOCK
>   *OVS_EXCLUDED OVS_EXCLUDED OVS_EXCLUDED
>   */
> +
> +/* Please keep OVS_CTAGS_IDENTIFIERS up-to-date in acinclude.m4. */
>  #define OVS_LOCKABLE __attribute__((lockable))
>  #define OVS_REQ_RDLOCK(...) 
> __attribute__((shared_locks_required(__VA_ARGS__)))
>  #define OVS_ACQ_RDLOCK(...) 
> __attribute__((shared_lock_function(__VA_ARGS__)))
> -- 
> 2.26.2
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[ovs-dev] [PATCH] ovsdb-idl.at: Wait all servers to join the cluster.

[Flavio Leitner]

The test 'Check Python IDL reconnects to leader - Python3
(leader only)' fails sometimes when the first ovsdb-server
gets killed before the others had joined the cluster.

Fix the function ovsdb_cluster_start_idltest to wait them
to join the cluster.

Suggested-by: Ilya Maximets 
Signed-off-by: Flavio Leitner 
---
 tests/ovsdb-idl.at | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index b5cbee7d9..c045e9264 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -29,6 +29,17 @@ ovsdb_cluster_start_idltest () {
  ovsdb-server -vraft -vconsole:warn --detach --no-chdir --log-file=s$i.log 
--pidfile=s$i.pid --unixctl=s$i --remote=punix:s$i.ovsdb ${2:+--remote=$2} 
s$i.db || return $?
done
on_exit 'kill `cat s*.pid`'
+   for i in `seq $n`; do
+ for d in `seq 1 "$OVS_CTL_TIMEOUT"`; do
+   if ovs-appctl -t $(pwd)/s$i cluster/status ${schema_name} | grep -q 
'Status: cluster member'; then
+ break
+   fi
+   sleep 1
+ done
+ if ! ovs-appctl -t $(pwd)/s$i cluster/status ${schema_name} | grep -q 
'Status: cluster member'; then
+   return 1
+ fi
+   done
 }
 
 # ovsdb_cluster_leader [REMOTES] [DATABASE]
-- 
2.26.2

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

Re: [ovs-dev] [RFC dpdk-latest 1/1] netdev-dpdk: integrate dpdk vhost pmd

[Flavio Leitner]

Hi,

First of all thanks for the patch.

I found some issues that I would like to discuss while I continue
reviewing the patch. I haven't tested yet.

See my comment in line.

Thanks!
fbl

On Tue, May 19, 2020 at 05:19:12PM +0530, Sivaprasad Tummala wrote:
> The vHost PMD brings vHost User port types ('dpdkvhostuser' and
> 'dpdkvhostuserclient') under control of DPDK's librte_ether API, like
> other DPDK netdev types ('dpdk'). In doing so, direct
> calls to DPDK's librte_vhost library are removed and replaced with
> librte_ether API calls, for which most of the infrastructure is already
> in place.
> 
> To enable TSO, specific changes were required in the vhost PMD. The
> patch which enables these is  available on dpdk-master and here:
> https://patches.dpdk.org/patch/66052/
> 
> Signed-off-by: Ciara Loftus 
> Signed-off-by: Sivaprasad Tummala 
> 
> Tested-by: Sunil Pai G 
> ---
>  Documentation/topics/dpdk/vhost-user.rst |3 +
>  NEWS |3 +
>  acinclude.m4 |4 +
>  include/openvswitch/netdev.h |1 +
>  lib/dpdk.c   |   11 +
>  lib/dpdk.h   |2 +
>  lib/netdev-dpdk.c| 1384 --
>  7 files changed, 535 insertions(+), 873 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/vhost-user.rst 
> b/Documentation/topics/dpdk/vhost-user.rst
> index c6c6fd8bd..644598f79 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -34,6 +34,9 @@ User, refer to the `QEMU documentation`_ on same.
> To use any DPDK-backed interface, you must ensure your bridge is 
> configured
> correctly. For more information, refer to :doc:`bridge`.
>  
> +   Maximum number of vHost ports should be less than RTE_MAX_ETHPORTS as
> +   defined in the DPDK configuration.
> +

This needs more clarification saying that now the vhost-user ports
are counted and the maximum is RTE_MAX_ETHPORTS.  This also misses
the name restriction change disallowing commas.



>  Quick Example
>  -
>  
> diff --git a/NEWS b/NEWS
> index 3dbd8ec0e..0071916fb 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,6 +9,9 @@ Post-v2.13.0
> - DPDK:
>   * Deprecated DPDK pdump packet capture support removed.
>   * Deprecated DPDK ring ports (dpdkr) are no longer supported.
> + * Use DPDK's vHost PMD instead of direct library calls. This means the
> +   maximum number of vHost ports is equal to RTE_MAX_ETHPORTS as defined
> +   in the DPDK configuration.


Same here.


> - Linux datapath:
>   * Support for kernel versions up to 5.5.x.
> - AF_XDP:
> diff --git a/acinclude.m4 b/acinclude.m4
> index dabbffd01..80d09f61c 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -367,6 +367,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>AC_DEFINE([VHOST_NUMA], [1], [NUMA Aware vHost support detected in 
> DPDK.])
>  ], [], [[#include ]])
>  
> +AC_CHECK_DECL([RTE_LIBRTE_PMD_VHOST], [], [
> +  AC_MSG_ERROR([RTE_LIBRTE_PMD_VHOST is not defined in rte_config.h])
> +], [[#include ]])
> +
>  AC_CHECK_DECL([RTE_LIBRTE_MLX5_PMD], [dnl found
>OVS_FIND_DEPENDENCY([mnl_attr_put], [mnl], [libmnl])
>AC_CHECK_DECL([RTE_IBVERBS_LINK_DLOPEN], [], [dnl not found
> diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
> index 0c10f7b48..09027e15d 100644
> --- a/include/openvswitch/netdev.h
> +++ b/include/openvswitch/netdev.h
> @@ -84,6 +84,7 @@ struct netdev_stats {
>  uint64_t tx_broadcast_packets;
>  
>  uint64_t rx_undersized_errors;
> +uint64_t rx_undersize_packets;

This doesn't seem to be required for this patchset, correct?. I know
it will improve things but it could be done on a separate patch
to reduce the patch's size and complexity.


>  uint64_t rx_oversize_errors;
>  uint64_t rx_fragmented_errors;
>  uint64_t rx_jabber_errors;
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 31450d470..202965b2a 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -23,12 +23,14 @@
>  #include 
>  
>  #include 
> +#include 


Why this is needed?


>  #include 
>  #include 
>  #include 
>  
>  #include "dirs.h"
>  #include "fatal-signal.h"
> +#include "id-pool.h"
>  #include "netdev-dpdk.h"
>  #include "netdev-offload-provider.h"
>  #include "openvswitch/dynamic-string.h"
> @@ -50,6 +52,7 @@ static bool vhost_postcopy_enabled = false; /* Status of 
> vHost POSTCOPY
>  static bool dpdk_initialized = false; /* Indicates successful initialization
> * of DPDK. */
>  static bool per_port_memory = false; /* Status of per port memory support */
> +static struct id_pool *vhost_driver_ids;  /* Pool of IDs for vHost PMDs. */
>  
>  static int
>  process_vhost_flags(char *flag, const char *default_val, int size,
> @@ -428,6 +431,8 @@ dpdk_init__(const struct smap *ovs_other_config)
>  /* We are called from the 

[ovs-dev] [PATCH] ctags: Include new annotations to ctags ignore list.

[Flavio Leitner]

The annotation OVS_NO_THREAD_SAFETY_ANALYSIS and OVS_LOCKABLE are
not part of the list, so ctags can't find functions using them.

The annotation list comes from a regex and to include more items
make the regex more difficult to read and maintain. Convert to a
static list because it isn't supposed to change much and there
is no standard names.

Also add a comment to remind to keep the list up-to-date.

Signed-off-by: Flavio Leitner 
---
 Makefile.am| 2 +-
 acinclude.m4   | 6 +++---
 include/openvswitch/compiler.h | 2 ++
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b279303d1..27ef9e4b4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,7 +46,7 @@ AM_CPPFLAGS += -DNDEBUG
 AM_CFLAGS += -fomit-frame-pointer
 endif
 
-AM_CTAGSFLAGS = $(OVS_CTAGS_IDENTIFIERS_LIST)
+AM_CTAGSFLAGS = -I "$(OVS_CTAGS_IDENTIFIERS_LIST)"
 
 if WIN32
 psep=";"
diff --git a/acinclude.m4 b/acinclude.m4
index 8847b8145..054ec2e3c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -1332,11 +1332,11 @@ AC_DEFUN([OVS_ENABLE_SPARSE],
 
 dnl OVS_CTAGS_IDENTIFIERS
 dnl
-dnl ctags ignores symbols with extras identifiers. This builds a list of
-dnl specially handled identifiers to be ignored.
+dnl ctags ignores symbols with extras identifiers. This is a list of
+dnl specially handled identifiers to be ignored. [ctags(1) -I ].
 AC_DEFUN([OVS_CTAGS_IDENTIFIERS],
 AC_SUBST([OVS_CTAGS_IDENTIFIERS_LIST],
-   [`printf %s '-I "'; sed -n 's/^#define 
\(OVS_[A-Z_]\+\)(\.\.\.)$/\1+/p' ${srcdir}/include/openvswitch/compiler.h  | tr 
\\\n ' ' ; printf '"'`] ))
+   ["OVS_LOCKABLE OVS_NO_THREAD_SAFETY_ANALYSIS OVS_REQ_RDLOCK+ 
OVS_ACQ_RDLOCK+ OVS_REQ_WRLOCK+ OVS_ACQ_WRLOCK+ OVS_REQUIRES+ OVS_ACQUIRES+ 
OVS_TRY_WRLOCK+ OVS_TRY_RDLOCK+ OVS_TRY_LOCK+ OVS_GUARDED_BY+ OVS_EXCLUDED+ 
OVS_RELEASES+ OVS_ACQ_BEFORE+ OVS_ACQ_AFTER+"]))
 
 dnl OVS_PTHREAD_SET_NAME
 dnl
diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h
index 5289a70f6..cf009f826 100644
--- a/include/openvswitch/compiler.h
+++ b/include/openvswitch/compiler.h
@@ -113,6 +113,8 @@
  *OVS_REQUIRES OVS_REQ_RDLOCK   OVS_REQ_WRLOCK
  *OVS_EXCLUDED OVS_EXCLUDED OVS_EXCLUDED
  */
+
+/* Please keep OVS_CTAGS_IDENTIFIERS up-to-date in acinclude.m4. */
 #define OVS_LOCKABLE __attribute__((lockable))
 #define OVS_REQ_RDLOCK(...) __attribute__((shared_locks_required(__VA_ARGS__)))
 #define OVS_ACQ_RDLOCK(...) __attribute__((shared_lock_function(__VA_ARGS__)))
-- 
2.26.2

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

Re: [ovs-dev] [PATCH] ovs-rcu: Avoid flushing callbacks during postponing.

[Ben Pfaff]

On Wed, Jun 10, 2020 at 09:37:38PM +0200, Ilya Maximets wrote:
> ovsrcu_flush_cbset() call during ovsrcu_postpone() could cause
> use after free in case the caller sets new pointer only after
> postponing free for the old one:

Thanks so much for this! (And Linhaifeng too!)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovs-rcu: Avoid flushing callbacks during postponing.

[Ilya Maximets]

ovsrcu_flush_cbset() call during ovsrcu_postpone() could cause
use after free in case the caller sets new pointer only after
postponing free for the old one:

 --  --  ---
 Thread 1Thread 2RCU Thread
 --  --  ---
 pointer = A

 ovsrcu_quiesce():
  thread->seqno = 30
  global_seqno = 31
  quiesced

 read pointer A
 postpone(free(A)):
   flush cbset
 pop flushed_cbsets
 ovsrcu_synchronize:
   target_seqno = 31
 ovsrcu_quiesce():
  thread->seqno = 31
  global_seqno = 32
  quiesced

 read pointer A
 use pointer A

 ovsrcu_quiesce():
  thread->seqno = 32
  global_seqno = 33
  quiesced

 read pointer A
 pointer = B

 ovsrcu_quiesce():
  thread->seqno = 33
  global_seqno = 34
  quiesced

 target_seqno exceeded
 by all threads
 call cbs to free A
 use pointer A
 (use after free)
 ---

Fix that by using dynamically re-allocated array without flushing
to the global flushed_cbsets until writer enters quiescent state.

Fixes: 0f2ea84841e1 ("ovs-rcu: New library.")
Reported-by: Linhaifeng 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/371265.html
Acked-by: Ben Pfaff 
Signed-off-by: Ilya Maximets 
---

'Reported-at' tag pointed to v2 of the patch from Linhaifeng, since it
contains a main discussion.  Also Linhaifeng added to a list of people
who provided valuable bug reports and suggestions.

This patch is already acked, so I will just test it a little bit more
and apply.

 AUTHORS.rst   |  1 +
 lib/ovs-rcu.c | 17 -
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 3f7eee54f..7a3b12610 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -563,6 +563,7 @@ Krishna Miriyalamiriya...@vmware.com
 Krishna Mohan Elluruelluru.kri.mo...@hpe.com
 László Sürü laszlo.s...@ericsson.com
 Len Gao l...@vmware.com
+Linhaifeng  haifeng@huawei.com
 Logan Rosen logatron...@gmail.com
 Luca Falavigna  dktrkr...@debian.org
 Luiz Henrique Ozaki luiz.oz...@gmail.com
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index ebc8120f0..cde1e925b 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -30,6 +30,8 @@
 
 VLOG_DEFINE_THIS_MODULE(ovs_rcu);
 
+#define MIN_CBS 16
+
 struct ovsrcu_cb {
 void (*function)(void *aux);
 void *aux;
@@ -37,7 +39,8 @@ struct ovsrcu_cb {
 
 struct ovsrcu_cbset {
 struct ovs_list list_node;
-struct ovsrcu_cb cbs[16];
+struct ovsrcu_cb *cbs;
+size_t n_allocated;
 int n_cbs;
 };
 
@@ -310,16 +313,19 @@ ovsrcu_postpone__(void (*function)(void *aux), void *aux)
 cbset = perthread->cbset;
 if (!cbset) {
 cbset = perthread->cbset = xmalloc(sizeof *perthread->cbset);
+cbset->cbs = xmalloc(MIN_CBS * sizeof *cbset->cbs);
+cbset->n_allocated = MIN_CBS;
 cbset->n_cbs = 0;
 }
 
+if (cbset->n_cbs == cbset->n_allocated) {
+cbset->cbs = x2nrealloc(cbset->cbs, >n_allocated,
+sizeof *cbset->cbs);
+}
+
 cb = >cbs[cbset->n_cbs++];
 cb->function = function;
 cb->aux = aux;
-
-if (cbset->n_cbs >= ARRAY_SIZE(cbset->cbs)) {
-ovsrcu_flush_cbset(perthread);
-}
 }
 
 static bool
@@ -341,6 +347,7 @@ ovsrcu_call_postponed(void)
 for (cb = cbset->cbs; cb < >cbs[cbset->n_cbs]; cb++) {
 cb->function(cb->aux);
 }
+free(cbset->cbs);
 free(cbset);
 }
 
-- 
2.25.4

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

Re: [ovs-dev] [PATCH ovn] Add northd and ovn-controller cluster status reset commands.

[Mark Michelson]

I have merged this to master now.

On 5/4/20 12:18 PM, Han Zhou wrote:

Acked-by: Han Zhou mailto:hz...@ovn.org>>

On Fri, May 1, 2020 at 12:15 PM Mark Michelson > wrote:

 >
 > This patch is dependent on OVS commit
 > 
https://patchwork.ozlabs.org/project/openvswitch/patch/20200501191308.94486-1-mmich...@redhat.com/

 >
 > Please do not merge this until the OVS commit has been merged as well.
 >
 > On 5/1/20 3:13 PM, Mark Michelson wrote:
 > > During the course of debugging a clustered DB environment, all members
 > > of the southbound database cluster were destroyed (i.e. the .db files
 > > were removed from disk) and then restarted. Once this happened,
 > > ovn-northd and ovn-controller could not interact with the southbound
 > > database because they both detected all members of the cluster as 
having

 > > "stale" data. The only course of action was to reset ovn-northd and all
 > > ovn-controllers. It is possible to have this happen with the northbound
 > > database as well if it is clustered.
 > >
 > > This patch offers new ovn-appctl commands for ovn-northd and
 > > ovn-controller that allows for it to reset its clustered status. This
 > > allows for it to interact with the database successfully after a 
cluster

 > > teardown and restart.
 > >
 > > Signed-off-by: Mark Michelson >

 > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1829109
 > > ---
 > >   controller/ovn-controller.8.xml | 16 
 > >   controller/ovn-controller.c     | 30 ++---
 > >   northd/ovn-northd.8.xml         | 28 +++
 > >   northd/ovn-northd.c             | 34 
+

 > >   4 files changed, 105 insertions(+), 3 deletions(-)
 > >
 > > diff --git a/controller/ovn-controller.8.xml 
b/controller/ovn-controller.8.xml

 > > index 76bbbdc5f..fe62163fa 100644
 > > --- a/controller/ovn-controller.8.xml
 > > +++ b/controller/ovn-controller.8.xml
 > > @@ -491,6 +491,22 @@
 > >           recomputes are cpu intensive.
 > >         
 > >         
 > > +
 > > +      sb-cluster-state-reset
 > > +      
 > > +      
 > > +        Reset southbound database cluster status when databases 
are destroyed

 > > +        and rebuilt.
 > > +      
 > > +      
 > > +        If all databases in a clustered southbound database are 
removed from
 > > +        disk, then the stored index of all databases will be reset 
to zero.
 > > +        This will cause ovn-controller to be unable to read or 
write to the
 > > +        southbound database, because it will always detect the 
data as stale.
 > > +        In such a case, run this command so that ovn-controller 
will reset its
 > > +        local index so that it can interact with the southbound 
database again.

 > > +      
 > > +      
 > >         
 > >       
 > >
 > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
 > > index 6ff897325..1442accd7 100644
 > > --- a/controller/ovn-controller.c
 > > +++ b/controller/ovn-controller.c
 > > @@ -73,6 +73,7 @@ static unixctl_cb_func extend_table_list;
 > >   static unixctl_cb_func inject_pkt;
 > >   static unixctl_cb_func ovn_controller_conn_show;
 > >   static unixctl_cb_func engine_recompute_cmd;
 > > +static unixctl_cb_func cluster_state_reset_cmd;
 > >
 > >   #define DEFAULT_BRIDGE_NAME "br-int"
 > >   #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 > > @@ -446,7 +447,7 @@ get_ofctrl_probe_interval(struct ovsdb_idl 
*ovs_idl)

 > >    * updates 'sbdb_idl' with that pointer. */
 > >   static void
 > >   update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
 > > -             bool *monitor_all_p)
 > > +             bool *monitor_all_p, bool *reset_ovnsb_idl_min_index)
 > >   {
 > >       const struct ovsrec_open_vswitch *cfg = 
ovsrec_open_vswitch_first(ovs_idl);

 > >       if (!cfg) {
 > > @@ -476,6 +477,12 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct 
ovsdb_idl *ovnsb_idl,

 > >       if (monitor_all_p) {
 > >           *monitor_all_p = monitor_all;
 > >       }
 > > +    if (*reset_ovnsb_idl_min_index) {
 > > +        VLOG_INFO("Resetting southbound database cluster state");
 > > +        engine_set_force_recompute(true);
 > > +        ovsdb_idl_reset_min_index(ovnsb_idl);
 > > +        *reset_ovnsb_idl_min_index = false;
 > > +    }
 > >   }
 > >
 > >   static void
 > > @@ -1936,6 +1943,11 @@ main(int argc, char *argv[])
 > >       unixctl_command_register("recompute", "", 0, 0, 
engine_recompute_cmd,

 > >                                NULL);
 > >
 > > +    bool reset_ovnsb_idl_min_index = false;
 > > +    unixctl_command_register("sb-cluster-state-reset", "", 0, 0,
 > > +                             cluster_state_reset_cmd,
 > > +                             _ovnsb_idl_min_index);
 > > +
 > >       unsigned int ovs_cond_seqno = UINT_MAX;
 > >       unsigned int ovnsb_cond_seqno = UINT_MAX;
 > >
 > > @@ -1957,7 +1969,8 @@ main(int argc, char *argv[])
 > 

[ovs-dev] [RFC ovn 5/6] ovn-northd.c: Remove the use of the REGBIT_SKIP_LOOKUP_NEIGHBOR bit.

[Han Zhou]

In LR ingress stage LOOKUP_NEIGHBOR and LEARN_NEIGHBOR, the flag
REGBIT_SKIP_LOOKUP_NEIGHBOR was used to indicate if mac-binding
lookup can be skipped. This patch avoid using the bit by combining
it with the REGBIT_LOOKUP_NEIGHBOR_RESULT bit, and assigning 1
to REGBIT_LOOKUP_NEIGHBOR_RESULT serves same purpose of skipping
the lookup. There will be a new bit needed in a future patch, and
this change can avoid using too many bits unnecessarily.

Signed-off-by: Han Zhou 
---
 northd/ovn-northd.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d8197ab..9a4e884 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -219,7 +219,6 @@ enum ovn_stage {
 /* Register to store the result of check_pkt_larger action. */
 #define REGBIT_PKT_LARGER"reg9[1]"
 #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
-#define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
 
 /* Register for ECMP bucket selection. */
 #define REG_ECMP_GROUP_ID   "reg8[0..15]"
@@ -7979,14 +7978,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   "lookup_nd(inport, ip6.src, nd.sll); next;");
 
 /* For other packet types, we can skip neighbor learning.
- * So set REGBIT_SKIP_LOOKUP_NEIGHBOR to 1. */
+ * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1",
-  REGBIT_SKIP_LOOKUP_NEIGHBOR" = 1; next;");
+  REGBIT_LOOKUP_NEIGHBOR_RESULT" = 1; next;");
 
 /* Flows for LEARN_NEIGHBOR. */
 /* Skip Neighbor learning if not required. */
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 100,
-  REGBIT_SKIP_LOOKUP_NEIGHBOR" == 1 || "
   REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1", "next;");
 
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
-- 
2.1.0

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

[ovs-dev] [RFC ovn 0/6] Avoid ARP flow explosion.

[Han Zhou]

This patch series addresses the problem discussed in [0].

It is RFC status now for tryout in the environement where the problem was
reported, and what's pending are more documentation and tests, and possible
adjustments depending on test feedbacks.

To avoid flow explosion in the scenario mentioned in [0], please configure
below options for the gateway routers:

options:learn_from_arp_request = false
options:dynamic_neigh_routers = true

[0] - https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049994.html

Han Zhou (6):
  ovn-northd: Support optionally avoid static neighbor flows in routers.
  tests: Fix get_arp/get_nd tests mac-binding table id.
  actions: Rename xxx_lookup_mac to xxx_lookup_mac_bind.
  actions: Implement new actions lookup_arp_ip and lookup_nd_ip.
  ovn-northd.c: Remove the use of the REGBIT_SKIP_LOOKUP_NEIGHBOR bit.
  ovn-northd.c: Support optionally disabling neighbor learning from ARP
request/NS.

 controller/lflow.c  |   4 +-
 include/ovn/actions.h   |  10 
 lib/actions.c   | 133 
 northd/ovn-northd.8.xml |   5 +-
 northd/ovn-northd.c |  90 ++--
 ovn-nb.xml  |  13 +
 tests/ovn.at|  76 +--
 tests/test-ovn.c|   2 +-
 utilities/ovn-trace.c   |  67 
 9 files changed, 358 insertions(+), 42 deletions(-)

-- 
2.1.0

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

[ovs-dev] [RFC ovn 4/6] actions: Implement new actions lookup_arp_ip and lookup_nd_ip.

[Han Zhou]

lookup_arp_ip and lookup_nd_ip are added to lookup if an entry exists
in MAC bindings for a given IP address, for IPv4 and IPv6 respectively.

Signed-off-by: Han Zhou 
---
 controller/lflow.c|   4 +-
 include/ovn/actions.h |  10 +
 lib/actions.c | 112 ++
 tests/ovn.at  |  50 ++
 utilities/ovn-trace.c |  54 ++--
 5 files changed, 226 insertions(+), 4 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 01214a3..e45ed33 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -761,13 +761,15 @@ consider_neighbor_flow(struct ovsdb_idl_index 
*sbrec_port_binding_by_name,
 
 uint64_t stub[1024 / 8];
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
+uint8_t value = 1;
 put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, );
+put_load(, sizeof value, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1,
+ );
 ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
 b->header_.uuid.parts[0], _arp_match,
 , >header_.uuid);
 
 ofpbuf_clear();
-uint8_t value = 1;
 put_load(, sizeof value, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1,
  );
 match_set_dl_src(_arp_match, mac);
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 4a54abe..2b5a63a 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -75,9 +75,11 @@ struct ovn_extend_table;
 OVNACT(GET_ARP,   ovnact_get_mac_bind)\
 OVNACT(PUT_ARP,   ovnact_put_mac_bind)\
 OVNACT(LOOKUP_ARP,ovnact_lookup_mac_bind) \
+OVNACT(LOOKUP_ARP_IP, ovnact_lookup_mac_bind_ip) \
 OVNACT(GET_ND,ovnact_get_mac_bind)\
 OVNACT(PUT_ND,ovnact_put_mac_bind)\
 OVNACT(LOOKUP_ND, ovnact_lookup_mac_bind) \
+OVNACT(LOOKUP_ND_IP,  ovnact_lookup_mac_bind_ip) \
 OVNACT(PUT_DHCPV4_OPTS,   ovnact_put_opts)\
 OVNACT(PUT_DHCPV6_OPTS,   ovnact_put_opts)\
 OVNACT(SET_QUEUE, ovnact_set_queue)   \
@@ -307,6 +309,14 @@ struct ovnact_lookup_mac_bind {
 struct expr_field mac;  /* 48-bit Ethernet address. */
 };
 
+/* OVNACT_LOOKUP_ARP_IP, OVNACT_LOOKUP_ND_IP. */
+struct ovnact_lookup_mac_bind_ip {
+struct ovnact ovnact;
+struct expr_field dst;  /* 1-bit destination field. */
+struct expr_field port; /* Logical port name. */
+struct expr_field ip;   /* 32-bit or 128-bit IP address. */
+};
+
 struct ovnact_gen_option {
 const struct gen_opts_map *option;
 struct expr_constant_set value;
diff --git a/lib/actions.c b/lib/actions.c
index 22c0e76..41301c8 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1953,6 +1953,110 @@ ovnact_lookup_mac_bind_free(
 
 }
 
+
+static void format_lookup_mac_bind_ip(
+const struct ovnact_lookup_mac_bind_ip *lookup_mac,
+struct ds *s, const char *name)
+{
+expr_field_format(_mac->dst, s);
+ds_put_format(s, " = %s(", name);
+expr_field_format(_mac->port, s);
+ds_put_cstr(s, ", ");
+expr_field_format(_mac->ip, s);
+ds_put_cstr(s, ");");
+}
+
+static void
+format_LOOKUP_ARP_IP(const struct ovnact_lookup_mac_bind_ip *lookup_mac,
+ struct ds *s)
+{
+format_lookup_mac_bind_ip(lookup_mac, s, "lookup_arp_ip");
+}
+
+static void
+format_LOOKUP_ND_IP(const struct ovnact_lookup_mac_bind_ip *lookup_mac,
+struct ds *s)
+{
+format_lookup_mac_bind_ip(lookup_mac, s, "lookup_nd_ip");
+}
+
+static void
+encode_lookup_mac_bind_ip(const struct ovnact_lookup_mac_bind_ip *lookup_mac,
+  enum mf_field_id ip_field,
+  const struct ovnact_encode_params *ep,
+  struct ofpbuf *ofpacts)
+{
+const struct arg args[] = {
+{ expr_resolve_field(_mac->port), MFF_LOG_OUTPORT },
+{ expr_resolve_field(_mac->ip), ip_field },
+};
+
+encode_setup_args(args, ARRAY_SIZE(args), ofpacts);
+init_stack(ofpact_put_STACK_PUSH(ofpacts), MFF_ETH_DST);
+
+struct mf_subfield dst = expr_resolve_field(_mac->dst);
+ovs_assert(dst.field);
+
+put_load(0, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1, ofpacts);
+emit_resubmit(ofpacts, ep->mac_bind_ptable);
+
+struct ofpact_reg_move *orm = ofpact_put_REG_MOVE(ofpacts);
+orm->dst = dst;
+orm->src.field = mf_from_id(MFF_LOG_FLAGS);
+orm->src.ofs = MLF_LOOKUP_MAC_BIT;
+orm->src.n_bits = 1;
+
+init_stack(ofpact_put_STACK_POP(ofpacts), MFF_ETH_DST);
+encode_restore_args(args, ARRAY_SIZE(args), ofpacts);
+}
+
+static void
+encode_LOOKUP_ARP_IP(const struct ovnact_lookup_mac_bind_ip *lookup_mac,
+ const struct ovnact_encode_params *ep,
+ struct ofpbuf *ofpacts)
+{
+encode_lookup_mac_bind_ip(lookup_mac, MFF_REG0, ep, ofpacts);
+}
+
+static void
+encode_LOOKUP_ND_IP(const struct ovnact_lookup_mac_bind_ip 

[ovs-dev] [RFC ovn 1/6] ovn-northd: Support optionally avoid static neighbor flows in routers.

[Han Zhou]

Support option:dynamic_neigh_routers for logical routers, so that in
particular use cases static neighbor flows are not prepopulated IP
addresses belonging to neighbor router ports, to avoid flow exploding
problem reported for ovn-kubernetes large scale setup.

Reported-by: Girish Moodalbail 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
Signed-off-by: Han Zhou 
---
 northd/ovn-northd.8.xml |  5 -
 northd/ovn-northd.c |  6 ++
 ovn-nb.xml  | 13 +
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index dc56de2..87e90d1 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2659,7 +2659,10 @@ outport = P;
   Logical_Switch_Port table.  For router ports
   connected to other logical routers, MAC bindings can be known
   statically from the mac and networks
-  column in the Logical_Router_Port table.
+  column in the Logical_Router_Port table.  (Note: the
+  flow is NOT installed for the IP addresses that belong to a neighbor
+  logical router port if the current router has the
+  options:dynamic_neigh_routers set to true)
 
 
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index eb78f31..d8197ab 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -9971,6 +9971,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+if (peer->od->nbr &&
+smap_get_bool(>od->nbr->options,
+  "dynamic_neigh_routers", false)) {
+continue;
+}
+
 for (size_t i = 0; i < op->od->n_router_ports; i++) {
 const char *router_port_name = smap_get(
 >od->router_ports[i]->nbsp->options,
diff --git a/ovn-nb.xml b/ovn-nb.xml
index acf5648..c0222ca 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1845,6 +1845,19 @@
   connected to the logical router. Default: False.
 
   
+  
+
+  If set to true, the router will resolve neighbor
+  routers' MAC addresses only by dynamic ARP/ND, instead of
+  prepopulating static mappings for all neighbor routers in the ARP/ND
+  Resolution stage.  This reduces number of flows, but requires ARP/ND
+  messages to resolve the IP-MAC bindings when needed.  It is
+  false by default.  It is recommended to set to
+  true when a large number of logical routers are
+  connected to the same logical switch but most of them never need to
+  send traffic between each other.
+
+  
 
 
 
-- 
2.1.0

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

[ovs-dev] [RFC ovn 3/6] actions: Rename xxx_lookup_mac to xxx_lookup_mac_bind.

[Han Zhou]

For the functions related to lookup_arp/lookup_nd, renaming them to
avoid confusion, because those functions checks both mac and ip in
mac-bindings. This patch renames them so that a future patch can
add a function that only looks up by ip without confusing names.

This patch also removes the unnecessary OVS_UNUSED for the function
execute_lookup_mac() in ovn-trace.c.

Signed-off-by: Han Zhou 
---
 lib/actions.c | 21 +++--
 utilities/ovn-trace.c | 13 +++--
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/lib/actions.c b/lib/actions.c
index c506151..22c0e76 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1847,8 +1847,9 @@ ovnact_put_mac_bind_free(struct ovnact_put_mac_bind 
*put_mac OVS_UNUSED)
 {
 }
 
-static void format_lookup_mac(const struct ovnact_lookup_mac_bind *lookup_mac,
-  struct ds *s, const char *name)
+static void format_lookup_mac_bind(
+const struct ovnact_lookup_mac_bind *lookup_mac,
+struct ds *s, const char *name)
 {
 expr_field_format(_mac->dst, s);
 ds_put_format(s, " = %s(", name);
@@ -1864,21 +1865,21 @@ static void
 format_LOOKUP_ARP(const struct ovnact_lookup_mac_bind *lookup_mac,
  struct ds *s)
 {
-format_lookup_mac(lookup_mac, s, "lookup_arp");
+format_lookup_mac_bind(lookup_mac, s, "lookup_arp");
 }
 
 static void
 format_LOOKUP_ND(const struct ovnact_lookup_mac_bind *lookup_mac,
 struct ds *s)
 {
-format_lookup_mac(lookup_mac, s, "lookup_nd");
+format_lookup_mac_bind(lookup_mac, s, "lookup_nd");
 }
 
 static void
-encode_lookup_mac(const struct ovnact_lookup_mac_bind *lookup_mac,
-  enum mf_field_id ip_field,
-  const struct ovnact_encode_params *ep,
-  struct ofpbuf *ofpacts)
+encode_lookup_mac_bind(const struct ovnact_lookup_mac_bind *lookup_mac,
+   enum mf_field_id ip_field,
+   const struct ovnact_encode_params *ep,
+   struct ofpbuf *ofpacts)
 {
 const struct arg args[] = {
 { expr_resolve_field(_mac->port), MFF_LOG_INPORT },
@@ -1908,7 +1909,7 @@ encode_LOOKUP_ARP(const struct ovnact_lookup_mac_bind 
*lookup_mac,
   const struct ovnact_encode_params *ep,
   struct ofpbuf *ofpacts)
 {
-encode_lookup_mac(lookup_mac, MFF_REG0, ep, ofpacts);
+encode_lookup_mac_bind(lookup_mac, MFF_REG0, ep, ofpacts);
 }
 
 static void
@@ -1916,7 +1917,7 @@ encode_LOOKUP_ND(const struct ovnact_lookup_mac_bind 
*lookup_mac,
 const struct ovnact_encode_params *ep,
 struct ofpbuf *ofpacts)
 {
-encode_lookup_mac(lookup_mac, MFF_XXREG0, ep, ofpacts);
+encode_lookup_mac_bind(lookup_mac, MFF_XXREG0, ep, ofpacts);
 }
 
 static void
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index d7251e7..146560c 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -1745,10 +1745,10 @@ execute_get_mac_bind(const struct ovnact_get_mac_bind 
*bind,
 }
 
 static void
-execute_lookup_mac(const struct ovnact_lookup_mac_bind *bind OVS_UNUSED,
-   const struct ovntrace_datapath *dp OVS_UNUSED,
-   struct flow *uflow OVS_UNUSED,
-   struct ovs_list *super OVS_UNUSED)
+execute_lookup_mac_bind(const struct ovnact_lookup_mac_bind *bind,
+const struct ovntrace_datapath *dp,
+struct flow *uflow,
+struct ovs_list *super)
 {
 /* Get logical port number.*/
 struct mf_subfield port_sf = expr_resolve_field(>port);
@@ -2208,11 +2208,12 @@ trace_actions(const struct ovnact *ovnacts, size_t 
ovnacts_len,
 break;
 
 case OVNACT_LOOKUP_ARP:
-execute_lookup_mac(ovnact_get_LOOKUP_ARP(a), dp, uflow, super);
+execute_lookup_mac_bind(ovnact_get_LOOKUP_ARP(a), dp, uflow,
+super);
 break;
 
 case OVNACT_LOOKUP_ND:
-execute_lookup_mac(ovnact_get_LOOKUP_ND(a), dp, uflow, super);
+execute_lookup_mac_bind(ovnact_get_LOOKUP_ND(a), dp, uflow, super);
 break;
 
 case OVNACT_PUT_DHCPV4_OPTS:
-- 
2.1.0

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

[ovs-dev] [RFC ovn 2/6] tests: Fix get_arp/get_nd tests mac-binding table id.

[Han Zhou]

The table id used in test is not the same as the one used in
real implementation. Although it doesn't affect correctness, it
may cause confusion when people are studying test cases.

Signed-off-by: Han Zhou 
---
 tests/ovn.at | 8 
 tests/test-ovn.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index 15b40ca..b7976c6 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1149,10 +1149,10 @@ arp { };
 
 # get_arp
 get_arp(outport, ip4.dst);
-encodes as 
push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[]
+encodes as 
push:NXM_NX_REG0[],push:NXM_OF_IP_DST[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[]
 has prereqs eth.type == 0x800
 get_arp(inport, reg0);
-encodes as 
push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
+encodes as 
push:NXM_NX_REG15[],push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],pop:NXM_NX_REG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG0[],pop:NXM_NX_REG15[]
 
 get_arp;
 Syntax error at `;' expecting `('.
@@ -1253,10 +1253,10 @@ nd_na_router { eth.src = 12:34:56:78:9a:bc; nd.tll = 
12:34:56:78:9a:bc; outport
 
 # get_nd
 get_nd(outport, ip6.dst);
-encodes as 
push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_XXREG0[]
+encodes as 
push:NXM_NX_XXREG0[],push:NXM_NX_IPV6_DST[],pop:NXM_NX_XXREG0[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_XXREG0[]
 has prereqs eth.type == 0x86dd
 get_nd(inport, xxreg0);
-encodes as 
push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,65),pop:NXM_NX_REG15[]
+encodes as 
push:NXM_NX_REG15[],push:NXM_NX_REG14[],pop:NXM_NX_REG15[],set_field:00:00:00:00:00:00->eth_dst,resubmit(,66),pop:NXM_NX_REG15[]
 get_nd;
 Syntax error at `;' expecting `('.
 get_nd();
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index a77d2f1..72b2985 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -1335,7 +1335,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx 
OVS_UNUSED)
 .ingress_ptable = 8,
 .egress_ptable = 40,
 .output_ptable = 64,
-.mac_bind_ptable = 65,
+.mac_bind_ptable = 66,
 .mac_lookup_ptable = 67,
 };
 struct ofpbuf ofpacts;
-- 
2.1.0

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

[ovs-dev] [RFC ovn 6/6] ovn-northd.c: Support optionally disabling neighbor learning from ARP request/NS.

[Han Zhou]

Support a new logical router option "learn_from_arp_request" that controls
behavior when handling ARP requests or IPv4 ND-NS packets.

"true" - Always learn the MAC/IP binding and add a new MAC_Binding entry
(default behavior)

"false" - If there is a MAC_binding for that IP and the MAC is different, or,
if TPA of ARP request belongs to any router port on this router, then
update/add that MAC/IP binding. Otherwise, don't update/add entries.

Reported-by: Girish Moodalbail 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
Signed-off-by: Han Zhou 
---
 northd/ovn-northd.c | 78 -
 tests/ovn.at| 18 +
 2 files changed, 84 insertions(+), 12 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 9a4e884..8a1f490 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -219,6 +219,7 @@ enum ovn_stage {
 /* Register to store the result of check_pkt_larger action. */
 #define REGBIT_PKT_LARGER"reg9[1]"
 #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
+#define REGBIT_LOOKUP_NEIGHBOR_IP_RESULT "reg9[3]"
 
 /* Register for ECMP bucket selection. */
 #define REG_ECMP_GROUP_ID   "reg8[0..15]"
@@ -7964,18 +7965,33 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
  * */
 
 /* Flows for LOOKUP_NEIGHBOR. */
+bool learn_from_arp_request = smap_get_bool(>nbr->options,
+"learn_from_arp_request",
+true);
+ds_clear();
+ds_put_format(, REGBIT_LOOKUP_NEIGHBOR_RESULT
+  " = lookup_arp(inport, arp.spa, arp.sha); %snext;",
+  learn_from_arp_request? "":
+  REGBIT_LOOKUP_NEIGHBOR_IP_RESULT" = 1; ");
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100,
-  "arp.op == 2",
-  REGBIT_LOOKUP_NEIGHBOR_RESULT" = "
-  "lookup_arp(inport, arp.spa, arp.sha); next;");
+  "arp.op == 2", ds_cstr());
 
+ds_clear();
+ds_put_format(, REGBIT_LOOKUP_NEIGHBOR_RESULT
+  " = lookup_nd(inport, nd.target, nd.tll); %snext;",
+  learn_from_arp_request? "":
+  REGBIT_LOOKUP_NEIGHBOR_IP_RESULT" = 1; ");
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100, "nd_na",
-  REGBIT_LOOKUP_NEIGHBOR_RESULT" = "
-  "lookup_nd(inport, nd.target, nd.tll); next;");
+  ds_cstr());
 
+ds_clear();
+ds_put_format(, REGBIT_LOOKUP_NEIGHBOR_RESULT
+  " = lookup_nd(inport, ip6.src, nd.sll); %snext;",
+  learn_from_arp_request? "":
+  REGBIT_LOOKUP_NEIGHBOR_IP_RESULT
+  " = lookup_nd_ip(inport, ip6.src); ");
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100, "nd_ns",
-  REGBIT_LOOKUP_NEIGHBOR_RESULT" = "
-  "lookup_nd(inport, ip6.src, nd.sll); next;");
+  ds_cstr());
 
 /* For other packet types, we can skip neighbor learning.
  * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
@@ -7984,8 +8000,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 
 /* Flows for LEARN_NEIGHBOR. */
 /* Skip Neighbor learning if not required. */
+ds_clear();
+ds_put_format(, REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1%s",
+  learn_from_arp_request? "":
+  " || "REGBIT_LOOKUP_NEIGHBOR_IP_RESULT" == 0");
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 100,
-  REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1", "next;");
+  ds_cstr(), "next;");
 
 ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
   "arp", "put_arp(inport, arp.spa, arp.sha); next;");
@@ -8002,8 +8022,38 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+bool learn_from_arp_request = smap_get_bool(>od->nbr->options,
+"learn_from_arp_request",
+true);
+
 /* Check if we need to learn mac-binding from ARP requests. */
 for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+if (!learn_from_arp_request) {
+/* ARP request to this address should always get learned,
+ * so add a priority-110 flow to set
+ * REGBIT_LOOKUP_NEIGHBOR_IP_RESULT to 1. */
+ds_clear();
+ds_put_format(,
+  "inport == %s && arp.spa == %s/%u && "
+  "arp.tpa == %s && arp.op == 1",
+  

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Ilya Maximets]

On 6/10/20 8:41 PM, Ben Pfaff wrote:
> On Wed, Jun 10, 2020 at 11:40:14AM -0700, Ben Pfaff wrote:
>> On Wed, Jun 03, 2020 at 04:33:28PM +0200, Ilya Maximets wrote:
>>> On 6/3/20 1:08 PM, Linhaifeng wrote:


> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> Sent: Wednesday, June 3, 2020 6:50 PM
> To: Linhaifeng ; Ben Pfaff 
> Cc: i.maxim...@ovn.org; d...@openvswitch.org; Lilijun (Jerry)
> ; Lichunhe ; nd
> ; chenchanghu 
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
>
> On 6/3/20 9:04 AM, Linhaifeng wrote:
>>
>>
>>> -Original Message-
>>> From: Ben Pfaff [mailto:b...@ovn.org]
>>> Sent: Wednesday, June 3, 2020 1:26 PM
>>> To: Linhaifeng 
>>> Cc: Yanqin Wei ; d...@openvswitch.org; nd
>>> ; Lilijun (Jerry) ; chenchanghu
>>> ; Lichunhe 
>>> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
>>>
>>> On Wed, Jun 03, 2020 at 01:22:52AM +, Linhaifeng wrote:


 -Original Message-
 From: Ben Pfaff [mailto:b...@ovn.org]
 Sent: Wednesday, June 3, 2020 1:28 AM
 To: Linhaifeng 
 Cc: Yanqin Wei ; d...@openvswitch.org; nd
 ; Lilijun (Jerry) ;
 chenchanghu ; Lichunhe
> 
 Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

 On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> We should update rcu pointer first then use ovsrcu_postpone to free
> otherwise maybe cause use-after-free.
> e.g.,reader indicates momentary quiescent and access old pointer
> after writer postpone free old pointer and before setting new pointer.
>
> Signed-off-by: Linhaifeng 

 I don't see how that's possible, since the writer hasn't quiesced.

 I think the logic is as follow, Could you help me find out where is 
 incorrect?

 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4
 ->
 3.3 -> 2.2(use after free)

 wirter:
 1.1 use postone to free old pointer
 1.2 flush cbsets to flushed_cbsets
 1.3 update new pointer
 1.4 quiesced

 Read:
 2.1. read pointer
 2.2. use pointer
 2.3. quiesced

 Rcu:
 3.1 pop flushed_cbsets
 3.2 ovsrcu_synchronize
 3.3 call all cb to free
>>>
>>> So you're saying this:
>>>
>>> 1.1 use postone to free old pointer (A)
>>> 1.2 flush cbsets to flushed_cbsets
>>>
>>> 3.1 pop flushed_cbsets
>>> 3.2 ovsrcu_synchronize
>>>
>>> 2.1. read pointer (A)
>>> 2.2. use pointer (A)
>>> 2.3. quiesced
>>>
>>> 2.1. read pointer (A)
>>>
>>> 1.3 update new pointer (B)
>>> 1.4 quiesced
>>>
>>> 3.3 call all cb to free (A)
>>>
>>> 2.2. use pointer (A)
>>>
>>> Wow, you are absolutely right.  This had never occurred to me.  Thank
> you!
>>> I'll review your patch.
>>
>> Yes, it's really hard to happen. If it happened it's also hard to find 
>> the reason
> so I suggest it can be a rule for using rcu.
>
> I agree that there is an issue here, but I think that we should not force 
> users to
> call ovsrcu_set() before ovsrcu_postpone().  Current users doesn't do
> anything illegal since pointer must not be freed before the next grace 
> period
> from their point of view.
>
> For me it looks like the main issue is existence of point 1.2, i.e. 
> flushing cbsets
> while writer is not quiesced yet.  And we need to fix this inside rcu 
> library itself.
> For example, we could avoid flushing inside
> ovsrcu_postpone() by making cbs[16] a dynamically allocated array and 
> using
> x2nrealloc instead of flushing.
>
> Thoughts?
>
 Hi, Ilya Maximets

 May be this is a good idea therefor the users not need to think about call 
 ovsrcu_set() first or ovsrcu_postpone().
 How about you think, ben? May be you can send a patch to modify the 
 ovsrcu_postpone() not to flush cbsets to
 replace of my patches.
>>>
>>> The change could look like this:
>>>
>>> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
>>> index ebc8120f0..cde1e925b 100644
>>> --- a/lib/ovs-rcu.c
>>> +++ b/lib/ovs-rcu.c
>>> @@ -30,6 +30,8 @@
>>>  
>>>  VLOG_DEFINE_THIS_MODULE(ovs_rcu);
>>>  
>>> +#define MIN_CBS 16
>>> +
>>>  struct ovsrcu_cb {
>>>  void (*function)(void *aux);
>>>  void *aux;
>>> @@ -37,7 +39,8 @@ struct ovsrcu_cb {
>>>  
>>>  struct ovsrcu_cbset {
>>>  struct ovs_list list_node;
>>> -struct ovsrcu_cb cbs[16];
>>> +struct ovsrcu_cb *cbs;
>>> +size_t n_allocated;
>>>  int n_cbs;
>>>  };
>>>  
>>> @@ -310,16 +313,19 @@ 

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Ben Pfaff]

On Wed, Jun 10, 2020 at 11:40:14AM -0700, Ben Pfaff wrote:
> On Wed, Jun 03, 2020 at 04:33:28PM +0200, Ilya Maximets wrote:
> > On 6/3/20 1:08 PM, Linhaifeng wrote:
> > > 
> > > 
> > >> -Original Message-
> > >> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> > >> Sent: Wednesday, June 3, 2020 6:50 PM
> > >> To: Linhaifeng ; Ben Pfaff 
> > >> Cc: i.maxim...@ovn.org; d...@openvswitch.org; Lilijun (Jerry)
> > >> ; Lichunhe ; nd
> > >> ; chenchanghu 
> > >> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> > >>
> > >> On 6/3/20 9:04 AM, Linhaifeng wrote:
> > >>>
> > >>>
> >  -Original Message-
> >  From: Ben Pfaff [mailto:b...@ovn.org]
> >  Sent: Wednesday, June 3, 2020 1:26 PM
> >  To: Linhaifeng 
> >  Cc: Yanqin Wei ; d...@openvswitch.org; nd
> >  ; Lilijun (Jerry) ; chenchanghu
> >  ; Lichunhe 
> >  Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> > 
> >  On Wed, Jun 03, 2020 at 01:22:52AM +, Linhaifeng wrote:
> > >
> > >
> > > -Original Message-
> > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > Sent: Wednesday, June 3, 2020 1:28 AM
> > > To: Linhaifeng 
> > > Cc: Yanqin Wei ; d...@openvswitch.org; nd
> > > ; Lilijun (Jerry) ;
> > > chenchanghu ; Lichunhe
> > >> 
> > > Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> > >
> > > On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> > >> We should update rcu pointer first then use ovsrcu_postpone to free
> > >> otherwise maybe cause use-after-free.
> > >> e.g.,reader indicates momentary quiescent and access old pointer
> > >> after writer postpone free old pointer and before setting new 
> > >> pointer.
> > >>
> > >> Signed-off-by: Linhaifeng 
> > >
> > > I don't see how that's possible, since the writer hasn't quiesced.
> > >
> > > I think the logic is as follow, Could you help me find out where is 
> > > incorrect?
> > >
> > > 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4
> > > ->
> > > 3.3 -> 2.2(use after free)
> > >
> > > wirter:
> > > 1.1 use postone to free old pointer
> > > 1.2 flush cbsets to flushed_cbsets
> > > 1.3 update new pointer
> > > 1.4 quiesced
> > >
> > > Read:
> > > 2.1. read pointer
> > > 2.2. use pointer
> > > 2.3. quiesced
> > >
> > > Rcu:
> > > 3.1 pop flushed_cbsets
> > > 3.2 ovsrcu_synchronize
> > > 3.3 call all cb to free
> > 
> >  So you're saying this:
> > 
> >  1.1 use postone to free old pointer (A)
> >  1.2 flush cbsets to flushed_cbsets
> > 
> >  3.1 pop flushed_cbsets
> >  3.2 ovsrcu_synchronize
> > 
> >  2.1. read pointer (A)
> >  2.2. use pointer (A)
> >  2.3. quiesced
> > 
> >  2.1. read pointer (A)
> > 
> >  1.3 update new pointer (B)
> >  1.4 quiesced
> > 
> >  3.3 call all cb to free (A)
> > 
> >  2.2. use pointer (A)
> > 
> >  Wow, you are absolutely right.  This had never occurred to me.  Thank
> > >> you!
> >  I'll review your patch.
> > >>>
> > >>> Yes, it's really hard to happen. If it happened it's also hard to find 
> > >>> the reason
> > >> so I suggest it can be a rule for using rcu.
> > >>
> > >> I agree that there is an issue here, but I think that we should not 
> > >> force users to
> > >> call ovsrcu_set() before ovsrcu_postpone().  Current users doesn't do
> > >> anything illegal since pointer must not be freed before the next grace 
> > >> period
> > >> from their point of view.
> > >>
> > >> For me it looks like the main issue is existence of point 1.2, i.e. 
> > >> flushing cbsets
> > >> while writer is not quiesced yet.  And we need to fix this inside rcu 
> > >> library itself.
> > >> For example, we could avoid flushing inside
> > >> ovsrcu_postpone() by making cbs[16] a dynamically allocated array and 
> > >> using
> > >> x2nrealloc instead of flushing.
> > >>
> > >> Thoughts?
> > >>
> > > Hi, Ilya Maximets
> > > 
> > > May be this is a good idea therefor the users not need to think about 
> > > call ovsrcu_set() first or ovsrcu_postpone().
> > > How about you think, ben? May be you can send a patch to modify the 
> > > ovsrcu_postpone() not to flush cbsets to
> > > replace of my patches.
> > 
> > The change could look like this:
> > 
> > diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> > index ebc8120f0..cde1e925b 100644
> > --- a/lib/ovs-rcu.c
> > +++ b/lib/ovs-rcu.c
> > @@ -30,6 +30,8 @@
> >  
> >  VLOG_DEFINE_THIS_MODULE(ovs_rcu);
> >  
> > +#define MIN_CBS 16
> > +
> >  struct ovsrcu_cb {
> >  void (*function)(void *aux);
> >  void *aux;
> > @@ -37,7 +39,8 @@ struct ovsrcu_cb {
> >  
> >  struct ovsrcu_cbset {
> >  struct ovs_list list_node;
> > -struct ovsrcu_cb cbs[16];
> > +struct ovsrcu_cb *cbs;
> > 

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Ben Pfaff]

On Wed, Jun 03, 2020 at 04:33:28PM +0200, Ilya Maximets wrote:
> On 6/3/20 1:08 PM, Linhaifeng wrote:
> > 
> > 
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> >> Sent: Wednesday, June 3, 2020 6:50 PM
> >> To: Linhaifeng ; Ben Pfaff 
> >> Cc: i.maxim...@ovn.org; d...@openvswitch.org; Lilijun (Jerry)
> >> ; Lichunhe ; nd
> >> ; chenchanghu 
> >> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> >>
> >> On 6/3/20 9:04 AM, Linhaifeng wrote:
> >>>
> >>>
>  -Original Message-
>  From: Ben Pfaff [mailto:b...@ovn.org]
>  Sent: Wednesday, June 3, 2020 1:26 PM
>  To: Linhaifeng 
>  Cc: Yanqin Wei ; d...@openvswitch.org; nd
>  ; Lilijun (Jerry) ; chenchanghu
>  ; Lichunhe 
>  Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
>  On Wed, Jun 03, 2020 at 01:22:52AM +, Linhaifeng wrote:
> >
> >
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Wednesday, June 3, 2020 1:28 AM
> > To: Linhaifeng 
> > Cc: Yanqin Wei ; d...@openvswitch.org; nd
> > ; Lilijun (Jerry) ;
> > chenchanghu ; Lichunhe
> >> 
> > Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> >
> > On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> >> We should update rcu pointer first then use ovsrcu_postpone to free
> >> otherwise maybe cause use-after-free.
> >> e.g.,reader indicates momentary quiescent and access old pointer
> >> after writer postpone free old pointer and before setting new pointer.
> >>
> >> Signed-off-by: Linhaifeng 
> >
> > I don't see how that's possible, since the writer hasn't quiesced.
> >
> > I think the logic is as follow, Could you help me find out where is 
> > incorrect?
> >
> > 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4
> > ->
> > 3.3 -> 2.2(use after free)
> >
> > wirter:
> > 1.1 use postone to free old pointer
> > 1.2 flush cbsets to flushed_cbsets
> > 1.3 update new pointer
> > 1.4 quiesced
> >
> > Read:
> > 2.1. read pointer
> > 2.2. use pointer
> > 2.3. quiesced
> >
> > Rcu:
> > 3.1 pop flushed_cbsets
> > 3.2 ovsrcu_synchronize
> > 3.3 call all cb to free
> 
>  So you're saying this:
> 
>  1.1 use postone to free old pointer (A)
>  1.2 flush cbsets to flushed_cbsets
> 
>  3.1 pop flushed_cbsets
>  3.2 ovsrcu_synchronize
> 
>  2.1. read pointer (A)
>  2.2. use pointer (A)
>  2.3. quiesced
> 
>  2.1. read pointer (A)
> 
>  1.3 update new pointer (B)
>  1.4 quiesced
> 
>  3.3 call all cb to free (A)
> 
>  2.2. use pointer (A)
> 
>  Wow, you are absolutely right.  This had never occurred to me.  Thank
> >> you!
>  I'll review your patch.
> >>>
> >>> Yes, it's really hard to happen. If it happened it's also hard to find 
> >>> the reason
> >> so I suggest it can be a rule for using rcu.
> >>
> >> I agree that there is an issue here, but I think that we should not force 
> >> users to
> >> call ovsrcu_set() before ovsrcu_postpone().  Current users doesn't do
> >> anything illegal since pointer must not be freed before the next grace 
> >> period
> >> from their point of view.
> >>
> >> For me it looks like the main issue is existence of point 1.2, i.e. 
> >> flushing cbsets
> >> while writer is not quiesced yet.  And we need to fix this inside rcu 
> >> library itself.
> >> For example, we could avoid flushing inside
> >> ovsrcu_postpone() by making cbs[16] a dynamically allocated array and using
> >> x2nrealloc instead of flushing.
> >>
> >> Thoughts?
> >>
> > Hi, Ilya Maximets
> > 
> > May be this is a good idea therefor the users not need to think about call 
> > ovsrcu_set() first or ovsrcu_postpone().
> > How about you think, ben? May be you can send a patch to modify the 
> > ovsrcu_postpone() not to flush cbsets to
> > replace of my patches.
> 
> The change could look like this:
> 
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> index ebc8120f0..cde1e925b 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -30,6 +30,8 @@
>  
>  VLOG_DEFINE_THIS_MODULE(ovs_rcu);
>  
> +#define MIN_CBS 16
> +
>  struct ovsrcu_cb {
>  void (*function)(void *aux);
>  void *aux;
> @@ -37,7 +39,8 @@ struct ovsrcu_cb {
>  
>  struct ovsrcu_cbset {
>  struct ovs_list list_node;
> -struct ovsrcu_cb cbs[16];
> +struct ovsrcu_cb *cbs;
> +size_t n_allocated;
>  int n_cbs;
>  };
>  
> @@ -310,16 +313,19 @@ ovsrcu_postpone__(void (*function)(void *aux), void 
> *aux)
>  cbset = perthread->cbset;
>  if (!cbset) {
>  cbset = perthread->cbset = xmalloc(sizeof *perthread->cbset);
> +cbset->cbs = xmalloc(MIN_CBS * sizeof *cbset->cbs);
> +cbset->n_allocated = MIN_CBS;
>

Re: [ovs-dev] [PATCH ovn] ovn-controller: Fix I-P for SB Port_Binding and OVS Interface.

[Numan Siddique]

On Wed, Jun 10, 2020 at 10:16 PM Dumitru Ceara  wrote:

> The commit that introduced incremental processing for Port_Binding and
> OVS Interface in the I-P runtime_data node covered most cases but two
> were missed:
>
> 1. If a Port_Binding was already claimed by the local hypervisor when
> ovn-controller starts, binding_handle_port_binding_changes doesn't
> correctly set the "changed" variable causing en_runtime_data node to
> go to EN_VALID instead of EN_UPDATED. Due to this update_sb_monitors()
> is skipped in that run and ovn-controller does not register for
> updates regarding the datapath containing the Port_Binding.
>
> 2. If a Port_Binding was already claimed by the local hypervisor when
> ovn-controller starts, but the underlying OVS interface was removed in
> the meantime, handle_updated_vif_lport() would fail the assertion that a
> local_binding should exist in memory.
>
> To address the first issue, we now explicitly track changes to the binding
> context local_lport and local_lport_ids sets. If these change during
> incremental processing of the runtime_data OVS_Interface and
> SB_Port_Binding input nodes then the runtime_data node should change
> state to EN_UPDATED.
>
> For the second issue, we now allow the case when a stale port_binding is
> released.
>
> Also, added an explicit non_vif_ports_changed variable to
> binding_ctx_out to track if other types of Port_Bindings
> have been changed in the current run. This kind of update should also
> cause runtime_data to move to EN_UPDATED such that update_sb_monitors()
> gets executed.
>
>
Thanks Dumitru  for fixing this. I applied this patch to master and
branch-20.06.

When other types of Port_Bindings get updated, I don't think there is a
need to
call update_sb_monitors(). Because we don't do conditional monitoring on
other type of Port_Bindings and the fact that 'non_vif_ports_changed' gets
set
to true, indicate that the ovn-controller received the updates for other
type of
Port_Bindings.

I'll rebase my I-P patch series and submit v12.

Thanks
Numan



> The commit also adds two test cases to cover the above scenarios and
> changes the way unit tests attach hypervisors in such way that a unit
> test can first configure br-int interfaces, even if ovn-controller
> hasn't started yet.
>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/371499.html
> CC: Numan Siddique 
> Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS
> interface in runtime_data.")
> Signed-off-by: Dumitru Ceara 
>



> ---
>  controller/binding.c| 122
> ++--
>  controller/binding.h|  21 ++--
>  controller/ovn-controller.c |  16 +++---
>  tests/ovn-macros.at |   2 +-
>  tests/ovn.at|  56 
>  5 files changed, 154 insertions(+), 63 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index e79220e..06ecb93 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -471,24 +471,57 @@ update_ld_localnet_port(const struct
> sbrec_port_binding *binding_rec,
>  ld->localnet_port = binding_rec;
>  }
>
> +/* Add an interface ID (usually taken from port_binding->name or
> + * ovs_interface->external_ids:iface-id) to the set of local lports.
> + * Also track if the set has changed.
> + */
> +static void
> +update_local_lports(struct binding_ctx_out *b_ctx, const char *iface_id)
> +{
> +if (sset_add(b_ctx->local_lports, iface_id) != NULL) {
> +b_ctx->local_lports_changed = true;
> +}
> +}
> +
> +/* Remove an interface ID from the set of local lports. Also track if the
> + * set has changed.
> + */
>  static void
> -update_local_lport_ids(struct sset *local_lport_ids,
> +remove_local_lports(struct binding_ctx_out *b_ctx, const char *iface_id)
> +{
> +if (sset_find_and_delete(b_ctx->local_lports, iface_id)) {
> +b_ctx->local_lports_changed = true;
> +}
> +}
> +
> +/* Add a port binding ID (of the form "dp-key"_"port-key") to the set of
> local
> + * lport IDs. Also track if the set has changed.
> + */
> +static void
> +update_local_lport_ids(struct binding_ctx_out *b_ctx,
> const struct sbrec_port_binding *pb)
>  {
>  char buf[16];
>  snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>   pb->datapath->tunnel_key, pb->tunnel_key);
> -sset_add(local_lport_ids, buf);
> +if (sset_add(b_ctx->local_lport_ids, buf) != NULL) {
> +b_ctx->local_lport_ids_changed = true;
> +}
>  }
>
> +/* Remove a port binding id from the set of local lport IDs. Also track if
> + * the set has changed.
> + */
>  static void
> -remove_local_lport_ids(const struct sbrec_port_binding *pb,
> -   struct sset *local_lport_ids)
> +remove_local_lport_ids(struct binding_ctx_out *b_ctx,
> +   const struct sbrec_port_binding *pb)
>  {
>  char buf[16];
>  snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>  

Re: [ovs-dev] [PATCH ovn] Honour router_preference for solicited RA

[Numan Siddique]

On Wed, Jun 10, 2020 at 3:01 PM Gabriele Cerami  wrote:

> Thanks for the review!
>
> On 10 Jun, Numan Siddique wrote:
>
> > I think test cases are fine. The action parsing test case makes sure that
> > the
> > action is encoded properly.
>
>
> I'm pretty sure they are not. But in production they end up being ok.
> For example checking the tcpdump output at the bottom of the
> description in https://bugzilla.redhat.com/show_bug.cgi?id=1804576
> you can see the RA flags are [none] (so it's using slaac) but the prefix
> info flags are [onlink, auto] and the value is c0, not 80.
>

The action parsing test case doesn't validate that the encoded value is as
per RFC.

While encoding an action, I can deliberately encode a wrong value and
in the action parsing test case in ovn.at I can add the same wrong value as
expected
and the test would pass.

So the developer working on it needs to make sure that it is encoded
properly.
One way to test is run an actual setup and make sure that when the VMs
interface
comes up, it gets configured with proper IPv6 addressed as per the
configuration - i.e slaac, dhcpv6 etc.

The tests in ovn.at use dummy datapath so we need to inject a packet and
then validate
that the response was correct.

So if the existing IPv6 RA encoding was wrong, please fix it in actions.c
and also fix the test case.



> Thing is, I'm not sure why this happens with my patch.
> Do you have time for a counter analysis ?
>
> This was my v2 yesterday, that is getting the incorrect test result
>
>
> diff --git a/lib/actions.c b/lib/actions.c
> index c50615177..7066d597e 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -2535,6 +2535,12 @@ parse_put_nd_ra_opts(struct action_context *ctx,
> const struct expr_field *dst,
>  }
>  break;
>
> +case ND_RA_FLAG_PRF:
> +ok = (c->string && (!strcmp(c->string, "MEDIUM") ||
> +!strcmp(c->string, "HIGH") ||
> +!strcmp(c->string, "LOW")));
> +break;
> +
>  case ND_OPT_SOURCE_LINKADDR:
>  ok = c->format == LEX_F_ETHERNET;
>  slla_present = true;
> @@ -2580,18 +2586,29 @@ format_PUT_ND_RA_OPTS(const struct ovnact_put_opts
> *po,
>
>  static void
>  encode_put_nd_ra_option(const struct ovnact_gen_option *o,
> -struct ofpbuf *ofpacts, ptrdiff_t ra_offset)
> +struct ofpbuf *ofpacts, struct ovs_ra_msg *ra)
>  {
>  const union expr_constant *c = o->value.values;
>
>  switch (o->option->code) {
>  case ND_RA_FLAG_ADDR_MODE:
>  {
> -struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
>  if (!strcmp(c->string, "dhcpv6_stateful")) {
> -ra->mo_flags = IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
> +ra->mo_flags |= IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
>  } else if (!strcmp(c->string, "dhcpv6_stateless")) {
> -ra->mo_flags = IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
> +ra->mo_flags |= IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
> +}
> +break;
> +}
> +
> +case ND_RA_FLAG_PRF:
> +{
> +if (!strcmp(c->string, "LOW")) {
> +ra->mo_flags |= IPV6_ND_RA_OPT_PRF_LOW;
> +} else if (!strcmp(c->string, "HIGH")) {
> +ra->mo_flags |= IPV6_ND_RA_OPT_PRF_HIGH;
> +} else {
> +ra->mo_flags |= IPV6_ND_RA_OPT_PRF_NORMAL;
>  }
>  break;
>  }
> @@ -2622,7 +2639,6 @@ encode_put_nd_ra_option(const struct
> ovnact_gen_option *o,
>  struct ovs_nd_prefix_opt *prefix_opt =
>  ofpbuf_put_uninit(ofpacts, sizeof *prefix_opt);
>  uint8_t prefix_len = ipv6_count_cidr_bits(>mask.ipv6);
> -struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
>  prefix_opt->type = ND_OPT_PREFIX_INFORMATION;
>  prefix_opt->len = 4;
>  prefix_opt->prefix_len = prefix_len;
> @@ -2640,6 +2656,12 @@ encode_put_nd_ra_option(const struct
> ovnact_gen_option *o,
>  break;
>  }
>  }
> +
> +/* RFC4191 section 2.2 */
> +if (ntohs(ra->router_lifetime) == 0x0) {
> +ra->mo_flags &= IPV6_ND_RA_OPT_PRF_RESET_MASK;
> +}
> +
>  }
>
>  static void
> @@ -2660,7 +2682,6 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts
> *po,
>   * pinctrl module receives the ICMPv6 Router Solicitation packet
>   * it can copy the userdata field AS IS and resume the packet.
>   */
> -size_t ra_offset = ofpacts->size;
>  struct ovs_ra_msg *ra = ofpbuf_put_zeros(ofpacts, sizeof *ra);
>  ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
>  ra->cur_hop_limit = IPV6_ND_RA_CUR_HOP_LIMIT;
> @@ -2669,7 +2690,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts
> *po,
>
>  for (const struct ovnact_gen_option *o = po->options;
>   o < >options[po->n_options]; o++) {
> -encode_put_nd_ra_option(o, ofpacts, ra_offset);
> +

Re: [ovs-dev] [PATCH ovn v10 3/8] ovn-controller: I-P for datapath binding

[Dumitru Ceara]

On 6/10/20 12:41 PM, Numan Siddique wrote:
> 
> 
> On Wed, Jun 10, 2020 at 3:31 PM Numan Siddique  > wrote:
> 
> 
> 
> On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara  > wrote:
> 
> On 6/9/20 7:10 PM, Numan Siddique wrote:
> >
> >
> > On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara
> mailto:dce...@redhat.com>
> > >> wrote:
> >
> >     On 6/8/20 3:50 PM, num...@ovn.org 
> > wrote:
> >     > From: Numan Siddique    >>
> >     >
> >     > This patch adds partial support of incremental processing of
> >     datapath binding.
> >     > If a datapath is deleted, then a full recompute is
> triggered if that
> >     > datapath is present in the 'local_datapaths' hmap of
> runtime data.
> >     >
> >     > Acked-by: Mark Michelson  
> >     >>
> >     > Acked-by: Han Zhou mailto:hz...@ovn.org>
> >>
> >     > Signed-off-by: Numan Siddique    >>
> >
> >     Looks good to me.
> >
> >     Acked-by: Dumitru Ceara    >>
> >
> >
> >
> > Thanks Dumitru, Han and Mark  for the reviews. 
> >
> > I applied the first 3 patches of this series (addressing the
> review
> > comments) to master and also applied to branch-20.06.
> >
> > @Han - If you have any additional comments on these patches
> please let
> > me know. I'll have follow up patches.
> >
> > I'll update v11 of this series addressing the review comments
> from Dumitru.
> >
> > Thanks
> > Numan
> >
> 
> Hi Numan,
> 
> I spotted a bug introduced by these 3 patches. The following
> scenario is
> now broken:
> 
> ovn-nbctl lr-add rtr
> ovn-nbctl lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24
> 
> ovn-nbctl ls-add ls
> ovn-nbctl lsp-add ls ls-rtr
> ovn-nbctl lsp-set-addresses ls-rtr 00:00:00:00:01:00
> ovn-nbctl lsp-set-type ls-rtr router
> ovn-nbctl lsp-set-options ls-rtr router-port=rtr-ls
> ovn-nbctl lsp-add ls vm1
> ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01
> 
> ovn-nbctl lsp-add ls vm2
> ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02
> 
> ip netns add vm1
> ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
> ip link set vm1 netns vm1
> ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
> ip netns exec vm1 ip addr add 42.42.42.2/24
>  dev vm1
> ip netns exec vm1 ip link set vm1 up
> ovs-vsctl set Interface vm1 external_ids:iface-id=vm1
> 
> ip netns add vm2
> ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
> ip link set vm2 netns vm2
> ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
> ip netns exec vm2 ip addr add 42.42.42.3/24
>  dev vm2
> ip netns exec vm2 ip link set vm2 up
> ovs-vsctl set Interface vm2 external_ids:iface-id=vm2
> 
> # Works
> ip netns exec vm1 ping 42.42.42.3 -c 1
> 
> # Restart ovn-controller
> ovn-ctl restart_controller
> 
> # Doesn't work
> ip netns exec vm1 ping 42.42.42.3 -c 1
> 
> # Delete port bindings
> ovn-sbctl destroy port_binding vm1
> ovn-sbctl destroy port_binding vm2
> 
> # Works
> ip netns exec vm1 ping 42.42.42.3 -c 1
> 
> 
> Oops. Thanks for reporting this Dumitru.
> 
> So when we restart, a full recompute should have been triggered.
> Looks like full recompute is not  triggered, after the IDL contents
> are received.
> 
> 
> As we discussed offline and the reason you pointed out that
> binding_handle_port_binding_changes()
> is not returning true when it processes for the port binding initial
> dump it received and hence not
> calling update_sb_monitors().
>  
> The issue is not seen with the v11 of the I-P series because it does
> return true. And also the issue is not
> seen with ovn-monitor-all is set.
> 
> But of course we should first fix this issue. Thanks for looking into it.
> 

Hi Numan,

I sent a patch to fix 

[ovs-dev] [PATCH ovn] ovn-controller: Fix I-P for SB Port_Binding and OVS Interface.

[Dumitru Ceara]

The commit that introduced incremental processing for Port_Binding and
OVS Interface in the I-P runtime_data node covered most cases but two
were missed:

1. If a Port_Binding was already claimed by the local hypervisor when
ovn-controller starts, binding_handle_port_binding_changes doesn't
correctly set the "changed" variable causing en_runtime_data node to
go to EN_VALID instead of EN_UPDATED. Due to this update_sb_monitors()
is skipped in that run and ovn-controller does not register for
updates regarding the datapath containing the Port_Binding.

2. If a Port_Binding was already claimed by the local hypervisor when
ovn-controller starts, but the underlying OVS interface was removed in
the meantime, handle_updated_vif_lport() would fail the assertion that a
local_binding should exist in memory.

To address the first issue, we now explicitly track changes to the binding
context local_lport and local_lport_ids sets. If these change during
incremental processing of the runtime_data OVS_Interface and
SB_Port_Binding input nodes then the runtime_data node should change
state to EN_UPDATED.

For the second issue, we now allow the case when a stale port_binding is
released.

Also, added an explicit non_vif_ports_changed variable to
binding_ctx_out to track if other types of Port_Bindings
have been changed in the current run. This kind of update should also
cause runtime_data to move to EN_UPDATED such that update_sb_monitors()
gets executed.

The commit also adds two test cases to cover the above scenarios and
changes the way unit tests attach hypervisors in such way that a unit
test can first configure br-int interfaces, even if ovn-controller
hasn't started yet.

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/371499.html
CC: Numan Siddique 
Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface 
in runtime_data.")
Signed-off-by: Dumitru Ceara 
---
 controller/binding.c| 122 ++--
 controller/binding.h|  21 ++--
 controller/ovn-controller.c |  16 +++---
 tests/ovn-macros.at |   2 +-
 tests/ovn.at|  56 
 5 files changed, 154 insertions(+), 63 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index e79220e..06ecb93 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -471,24 +471,57 @@ update_ld_localnet_port(const struct sbrec_port_binding 
*binding_rec,
 ld->localnet_port = binding_rec;
 }
 
+/* Add an interface ID (usually taken from port_binding->name or
+ * ovs_interface->external_ids:iface-id) to the set of local lports.
+ * Also track if the set has changed.
+ */
+static void
+update_local_lports(struct binding_ctx_out *b_ctx, const char *iface_id)
+{
+if (sset_add(b_ctx->local_lports, iface_id) != NULL) {
+b_ctx->local_lports_changed = true;
+}
+}
+
+/* Remove an interface ID from the set of local lports. Also track if the
+ * set has changed.
+ */
 static void
-update_local_lport_ids(struct sset *local_lport_ids,
+remove_local_lports(struct binding_ctx_out *b_ctx, const char *iface_id)
+{
+if (sset_find_and_delete(b_ctx->local_lports, iface_id)) {
+b_ctx->local_lports_changed = true;
+}
+}
+
+/* Add a port binding ID (of the form "dp-key"_"port-key") to the set of local
+ * lport IDs. Also track if the set has changed.
+ */
+static void
+update_local_lport_ids(struct binding_ctx_out *b_ctx,
const struct sbrec_port_binding *pb)
 {
 char buf[16];
 snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
  pb->datapath->tunnel_key, pb->tunnel_key);
-sset_add(local_lport_ids, buf);
+if (sset_add(b_ctx->local_lport_ids, buf) != NULL) {
+b_ctx->local_lport_ids_changed = true;
+}
 }
 
+/* Remove a port binding id from the set of local lport IDs. Also track if
+ * the set has changed.
+ */
 static void
-remove_local_lport_ids(const struct sbrec_port_binding *pb,
-   struct sset *local_lport_ids)
+remove_local_lport_ids(struct binding_ctx_out *b_ctx,
+   const struct sbrec_port_binding *pb)
 {
 char buf[16];
 snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
  pb->datapath->tunnel_key, pb->tunnel_key);
-sset_find_and_delete(local_lport_ids, buf);
+if (sset_find_and_delete(b_ctx->local_lport_ids, buf)) {
+b_ctx->local_lport_ids_changed = true;
+}
 }
 
 /* Local bindings. binding.c module binds the logical port (represented by
@@ -876,7 +909,7 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
b_ctx_in->sbrec_port_binding_by_name,
pb->datapath, false,
b_ctx_out->local_datapaths);
-update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
+update_local_lport_ids(b_ctx_out, pb);
 if (lbinding->iface && qos_map && 

[ovs-dev] Dear Friend,

[mr green]

Dear Friend,

Have you received your fund since last year? I saw your payment file
that is why I decided to contact you. You have suffered for nothing
without receiving your fund due to over greediness by the officials.
You would have received your fund since last year but your problem is
over greediness that cost you a lot of money and still yet, you have
never received $1 into your account. Reply to my email
address:(fredgreen...@gmail.com)
for more details.you are to send us.
your full names.
your contact address
your personal telephone number..
your Date of birth..
your Marital Status.
your Occupation.
YOUR NEXT OF KINGS INFORMATIONS
Thanks Mr FRED GREEN
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Do you need a quick loan?

[noperes]

Do you need a quick loan? we process your loan request within the shortest 
possible time and Interest rates are convinent. Reply this email for more 
detials on how to apply for a business or personal loans. 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Make polling fds persistent

[0-day Robot]

Bleep bloop.  Greetings Anton Ivanov, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 88 characters long (recommended limit is 79)
#124 FILE: lib/poll-loop-unix.c:64:
struct pollfd * watched;/* list of descriptors and event masks passed 
to poll */

WARNING: Line is 82 characters long (recommended limit is 79)
#189 FILE: lib/poll-loop-unix.c:129:
resized_watched = xzalloc(sizeof(struct pollfd) * 
loop->watched_size);

WARNING: Line is 101 characters long (recommended limit is 79)
#190 FILE: lib/poll-loop-unix.c:130:
memcpy(resized_watched, loop->watched, sizeof(struct pollfd) * 
(loop->watched_size - 1));

WARNING: Line is 84 characters long (recommended limit is 79)
#417 FILE: lib/poll-loop-unix.c:357:
loop->watched[i] = loop->watched[hmap_count(>poll_nodes) 
- 1];

Lines checked: 564, Warnings: 4, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

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

Re: [ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[0-day Robot]

Bleep bloop.  Greetings weili zhang, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Inappropriate bracing around statement
#24 FILE: lib/netdev-native-tnl.c:159:
if (padding)

Lines checked: 31, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

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

Re: [ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[0-day Robot]

Bleep bloop.  Greetings weili zhang, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Inappropriate bracing around statement
#24 FILE: lib/netdev-native-tnl.c:159:
if (padding)

Lines checked: 31, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

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

Re: [ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[0-day Robot]

Bleep bloop.  Greetings weili zhang, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Martin Zhang 
ERROR: Inappropriate bracing around statement
#25 FILE: lib/netdev-native-tnl.c:159:
if (padding)

Lines checked: 32, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

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

Re: [ovs-dev] [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces

[Gaëtan Rivet]

On 27/05/20 17:49 +0200, Ilya Maximets wrote:
> On 4/20/20 9:26 AM, Roni Bar Yanai wrote:
> > Hi Ben, Ilya
> > 
> > Going back to this thread. We've tried app-ctl approach and it fails on 
> > consistency
> > problem. Orchestrator can configure on full system init, but now executing 
> > local 
> > restart will lose the configuration (again for bifurcated driver it is not 
> > problem).
> > 
> > The requirement for setting VF mac through the representor, comes from 
> > different
> > use cases such as Open Stack and Kubernetes. VF is used with pass-through 
> > and
> > untrusted user VM/Pod/Container . The MAC address is set by the orchestrator
> > only. For Linux use case there is ip tool for doing that, and Linux takes 
> > care of the
> > consistency. For OVS-DPDK with port representor, there is no way to 
> > configure VF MAC
> > address (except of for bifurcated drivers).
> > 
> > The requirement is to enable orchestrator configuration of VF MAC address 
> > in DPDK,
> > when working with port representor.
> > The solution should handle the generic use case and not bifurcated drivers 
> > only.
> > The MAC should be kept and configured in case of OVS restart. 
> > 
> > Maybe we can go back to the first solution and open set MAC to port 
> > representors
> > only. We treat the solution as a generic solution and ignore bifurcated 
> > drivers. 
> > When user configure the representor MAC (which is a reflection of the VF), 
> > the 
> > VF MAC is configured. The MAC address is saved in the DB. In case of 
> > returning back
> > to kernel there is no problem because the DB MAC applies only for DPDK 
> > representor.
> > For bifurcated driver, user can cause in consistency, but this is a unique 
> > case and 
> > we cannot defend against misuse of bifurcated drivers.
> > 
> > Any thoughts?
> 
> This is a DPDK specific issue.  Since appctl commnad is not suitable and 
> common
> 'mac' configuration via database is a controversial solution, I'd suggest 
> having
> a scary-named netdev-dpdk specific knob to avoid abusing it for any other 
> usecase.
> For example, something like other_config:dpdk-vf-mac in the interface table.  
> And
> the code should, probably, reject attempts to change mac address on 
> non-representors.
> Probably, the name of the knob should reflect that somehow.
> 
> What do you think?

Hello Ilya,

I tried to read back the history about this subject but it's a little
scattered so apologies if I am repeating past comments.

I understand that you want to confine this issue to the specific case of
VF MAC.  Your solution addresses it but does not mitigate the split-brain
potential of bifurcated drivers.

The split-brain issue is inherent to those drivers.  I believe making a
feature more narrow and more difficult to find to side-step an intractable
problem will only muddy the water.

The confusion for bifurcated drivers is already there for other
interface states, such as MTU.  At this point wouldn't it be better to
be consistent in the risk involved, and expect users not to contradict
themselves when using their ports in OvS?

> 
> One more note here: You mentioned untrusted VFs and containers. Last time I 
> checked
> DPDK sources I didn't see any "mac address administratively set" concept like 
> it
> done in kernel drivers, so the untrusted VM or container could easily change 
> the
> configured mac address. i.e. configuration from the OVS side is only part of 
> the
> solution that is required.
> 
> Best regards, Ilya Maximets.
> 

I have looked at the VF specific configurations in DPDK, this concept is
currently addressed by vendor-specific APIs for a few drivers only.
I will propose a standard API for VF trusted mode (among others of those
VF APIs) to allow integration in upper layers, so that vendors can align
themselves.

Best regards,
-- 
Gaëtan
Mellanox
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[0-day Robot]

Bleep bloop.  Greetings weili zhang, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author zhangweilizhangweili  needs 
to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Martin Zhang , Weili Zhang 
<305753...@qq.com>
ERROR: Inappropriate bracing around statement
#25 FILE: lib/netdev-native-tnl.c:159:
if (padding)

Lines checked: 32, Warnings: 1, Errors: 2


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

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

[ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[305753229]

From: Weili Zhang <305753...@qq.com>

We need strip the inner L2 padding bytes, before enapcasulate a packet.

Signed-off-by: Weili Zhang <305753...@qq.com>
---
 lib/netdev-native-tnl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0acc87953..945cca3d5 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -153,8 +153,11 @@ netdev_tnl_push_ip_header(struct dp_packet *packet,
 struct eth_header *eth;
 struct ip_header *ip;
 struct ovs_16aligned_ip6_hdr *ip6;
+int padding = dp_packet_l2_pad_size(packet);
 
 eth = dp_packet_push_uninit(packet, size);
+if (padding)
+dp_packet_set_size(packet, dp_packet_size(packet) - padding);
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
-- 
2.24.2 (Apple Git-127)

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

[ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[305753229]

From: Weili Zhang <305753...@qq.com>

We need strip the inner L2 padding bytes, before enapcasulate a packet.

Signed-off-by: Weili Zhang <305753...@qq.com>
---
 lib/netdev-native-tnl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0acc87953..945cca3d5 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -153,8 +153,11 @@ netdev_tnl_push_ip_header(struct dp_packet *packet,
 struct eth_header *eth;
 struct ip_header *ip;
 struct ovs_16aligned_ip6_hdr *ip6;
+int padding = dp_packet_l2_pad_size(packet);
 
 eth = dp_packet_push_uninit(packet, size);
+if (padding)
+dp_packet_set_size(packet, dp_packet_size(packet) - padding);
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
-- 
2.24.2 (Apple Git-127)

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

[ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[305753229]

From: Weili Zhang <305753...@qq.com>

We need strip the inner L2 padding bytes, before enapcasulate a packet.

Signed-off-by: Martin Zhang 
Signed-off-by: Weili Zhang <305753...@qq.com>
---
 lib/netdev-native-tnl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0acc87953..945cca3d5 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -153,8 +153,11 @@ netdev_tnl_push_ip_header(struct dp_packet *packet,
 struct eth_header *eth;
 struct ip_header *ip;
 struct ovs_16aligned_ip6_hdr *ip6;
+int padding = dp_packet_l2_pad_size(packet);
 
 eth = dp_packet_push_uninit(packet, size);
+if (padding)
+dp_packet_set_size(packet, dp_packet_size(packet) - padding);
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
-- 
2.24.2 (Apple Git-127)

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

[ovs-dev] [PATCH] netdev-native-tnl: strip padding bytes of inner L2.

[305753229]

From: zhangweilizhangweili 

We need strip the inner L2 padding bytes, before enapcasulate a packet.

Signed-off-by: Martin Zhang 
Signed-off-by: Weili Zhang <305753...@qq.com>
---
 lib/netdev-native-tnl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0acc87953..945cca3d5 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -153,8 +153,11 @@ netdev_tnl_push_ip_header(struct dp_packet *packet,
 struct eth_header *eth;
 struct ip_header *ip;
 struct ovs_16aligned_ip6_hdr *ip6;
+int padding = dp_packet_l2_pad_size(packet);
 
 eth = dp_packet_push_uninit(packet, size);
+if (padding)
+dp_packet_set_size(packet, dp_packet_size(packet) - padding);
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
-- 
2.24.2 (Apple Git-127)

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

Re: [ovs-dev] [PATCH v3 6/7] dpif-lookup: add avx512 gather implementation

[0-day Robot]

Bleep bloop.  Greetings Harry van Haaren, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


build:
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/dirs.lo -MD -MP -MF lib/.deps/dirs.Tpo -c 
lib/dirs.c -o lib/dirs.o
depbase=`echo lib/ovsdb-server-idl.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/ovsdb-server-idl.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/ovsdb-server-idl.lo lib/ovsdb-server-idl.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/ovsdb-server-idl.lo -MD -MP -MF 
lib/.deps/ovsdb-server-idl.Tpo -c lib/ovsdb-server-idl.c -o 
lib/ovsdb-server-idl.o
depbase=`echo lib/vswitch-idl.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/vswitch-idl.lo -MD -MP -MF $depbase.Tpo -c -o lib/vswitch-idl.lo 
lib/vswitch-idl.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/vswitch-idl.lo -MD -MP -MF 
lib/.deps/vswitch-idl.Tpo -c lib/vswitch-idl.c -o lib/vswitch-idl.o
/bin/sh ./libtool  --tag=CC   --mode=link gcc -std=gnu99 -Wstrict-prototypes 
-Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -o lib/libopenvswitchcore.la -rpath /usr/local/lib lib/aes128.lo 
lib/backtrace.lo lib/bfd.lo lib/bundle.lo lib/byteq.lo lib/cfm.lo 
lib/classifier.lo lib/ccmap.lo lib/cmap.lo lib/colors.lo lib/command-line.lo 
lib/connectivity.lo lib/conntrack-icmp.lo lib/conntrack-tcp.lo 
lib/conntrack-tp.lo lib/conntrack-other.lo lib/conntrack.lo lib/coverage.lo 
lib/crc32c.lo lib/csum.lo lib/ct-dpif.lo lib/daemon.lo lib/db-ctl-base.lo 
lib/dummy.lo lib/dpctl.lo lib/dp-packet.lo lib/dpif-netdev-lookup.lo 
lib/dpif-netdev-lookup-generic.lo lib/dpif-netdev-lookup-autovalidator.lo 
lib/dpif-netdev.lo lib/dpif-netdev-perf.lo lib/dpif.lo lib/heap.
 lo lib/dynamic-string.lo lib/entropy.lo lib/fat-rwlock.lo lib/fatal-signal.lo 
lib/flow.lo lib/guarded-list.lo lib/hash.lo lib/hindex.lo lib/hmap.lo 
lib/hmapx.lo lib/id-pool.lo lib/if-notifier-manual.lo lib/ipf.lo lib/jhash.lo 
lib/json.lo lib/jsonrpc.lo lib/lacp.lo lib/learn.lo lib/learning-switch.lo 
lib/lockfile.lo lib/mac-learning.lo lib/match.lo lib/mcast-snooping.lo 
lib/memory.lo lib/meta-flow.lo lib/multipath.lo lib/namemap.lo 
lib/netdev-dummy.lo lib/netdev-offload.lo lib/netdev-vport.lo lib/netdev.lo 
lib/netlink.lo lib/nx-match.lo lib/object-collection.lo lib/odp-execute.lo 
lib/odp-util.lo lib/ofp-actions.lo lib/ofp-bundle.lo lib/ofp-connection.lo 
lib/ofp-ed-props.lo lib/ofp-errors.lo lib/ofp-flow.lo lib/ofp-group.lo 
lib/ofp-ipfix.lo lib/ofp-match.lo lib/ofp-meter.lo lib/ofp-monitor.lo 
lib/ofp-msgs.lo lib/ofp-packet.lo lib/ofp-parse.lo lib/ofp-port.lo 
lib/ofp-print.lo 

[ovs-dev] [PATCH] Make polling fds persistent

[anton . ivanov]

From: Anton Ivanov 

Saves on:

1. Allocation and disposal of a hash map per iteration in all threads
2. Re-population of the hashmap with all fds per iteration
3. Walking of the hashmap to construct a pollfd array per iteration
4. Allocating/deallocating the pollfd array per iteration
5. Decreases costs on various lookups

Compared to older attempts to do this, this emulates strictly the old
behaviour and is 100% backwards compatible with the old approach.

Unix only - the unix poll loop has been pulled to a new file.

Signed-off-by: Anton Ivanov 
---
 lib/automake.mk  |   3 +-
 lib/poll-loop-unix.c | 415 +++
 lib/poll-loop.c  |  19 +-
 3 files changed, 418 insertions(+), 19 deletions(-)
 create mode 100644 lib/poll-loop-unix.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 86940ccd2..39ff70650 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -242,7 +242,6 @@ lib_libopenvswitch_la_SOURCES = \
lib/perf-counter.c \
lib/stopwatch.h \
lib/stopwatch.c \
-   lib/poll-loop.c \
lib/process.c \
lib/process.h \
lib/pvector.c \
@@ -349,6 +348,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/route-table-stub.c \
lib/if-notifier-stub.c \
lib/stream-windows.c \
+   lib/poll-loop.c \
lib/strsep.c
 else
 lib_libopenvswitch_la_SOURCES += \
@@ -357,6 +357,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/signals.c \
lib/signals.h \
lib/socket-util-unix.c \
+   lib/poll-loop-unix.c \
lib/stream-unix.c
 endif
 
diff --git a/lib/poll-loop-unix.c b/lib/poll-loop-unix.c
new file mode 100644
index 0..0fb137855
--- /dev/null
+++ b/lib/poll-loop-unix.c
@@ -0,0 +1,415 @@
+/*
+ * Copyright (c) 2020 Red Hat Inc
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * 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 "openvswitch/poll-loop.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "coverage.h"
+#include "openvswitch/dynamic-string.h"
+#include "fatal-signal.h"
+#include "openvswitch/list.h"
+#include "ovs-thread.h"
+#include "seq.h"
+#include "socket-util.h"
+#include "timeval.h"
+#include "openvswitch/vlog.h"
+#include "openvswitch/hmap.h"
+#include "hash.h"
+
+VLOG_DEFINE_THIS_MODULE(poll_loop);
+
+COVERAGE_DEFINE(poll_create_node);
+COVERAGE_DEFINE(poll_zero_timeout);
+
+#define POLLFD_INCREMENT 16;
+
+/* The poll_node structures are used solely as metadata for
+ * the pollfd array associated with the loop. That pollfd
+ * array is persistent and does not need to be regenerated
+ * on every iteration.
+ */
+
+struct poll_node {
+struct hmap_node hmap_node;
+int index;  /* index in the pollfd array */
+const char *where;  /* Where poll_node was created. */
+};
+
+struct poll_loop {
+/* All active poll waiters. */
+struct hmap poll_nodes;
+
+/* Time at which to wake up the next call to poll_block(), LLONG_MIN to
+ * wake up immediately, or LLONG_MAX to wait forever. */
+long long int timeout_when; /* In msecs as returned by time_msec(). */
+const char *timeout_where;  /* Where 'timeout_when' was set. */
+struct pollfd * watched;/* list of descriptors and event masks passed 
to poll */
+int watched_size;   /* size of the watched allocation */
+};
+
+static struct poll_loop *poll_loop(void);
+
+/* Look up the node with same fd or wevent. */
+static struct poll_node *
+find_poll_node(struct poll_loop *loop, int fd)
+{
+struct poll_node *node;
+
+HMAP_FOR_EACH_WITH_HASH (node, hmap_node,
+ hash_2words(fd, 0),
+ >poll_nodes) {
+if (fd && loop->watched[node->index].fd == fd) {
+return node;
+}
+}
+return NULL;
+}
+
+/* On Unix based systems:
+ *
+ * Registers 'fd' as waiting for the specified 'events' (which should be
+ * POLLIN or POLLOUT or POLLIN | POLLOUT).  The following call to
+ * poll_block() will wake up when 'fd' becomes ready for one or more of the
+ * requested events. The 'fd's are given to poll() function later.
+ *
+ * The event registration is one-shot: only the following call to
+ * poll_block() is affected.  The event will need to be re-registered after
+ * poll_block() is called if it is to persist.
+ *
+ * ('where' is used in debug logging.  Commonly one would use 

[ovs-dev] [PATCH v3 5/7] lib/automake: split build multiple static library

[Harry van Haaren]

This commit changes the way the core lib/* code is built.
Before this commit, the lib/libopenvswitch_la target contains
all the code, and is directly linked against by executable targets
like ovs-vswitchd, ovsdb, tests etc.

This commit splits the building of the code and the linking to
that code into two seperate static libraries, providing more
flexibility in building of each individual static library.

A new library lib/libopenvswitchcore_la represents the lib/*
code. The previous library lib/libopenvswitch_la remains intact,
and is used by executable targets to link against. The core
library is listed as a dependency for the linked against library.

This approach requires no changes for executable targets, and
provides the required flexibility for future ISA optimized static
libraries to be built individually, and later combined into a
single static library.

Signed-off-by: Harry van Haaren 
---
 lib/automake.mk | 50 ++---
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/lib/automake.mk b/lib/automake.mk
index 9dbc2bbc5..19e454c4b 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -5,9 +5,19 @@
 # notice and this notice are preserved.  This file is offered as-is,
 # without warranty of any kind.
 
+# libopenvswitch.la is the library to link against for binaries like vswitchd.
+# The code itself is built as two seperate static libraries;
+# - core: Core files, always compiled with distro provided CFLAGS
 lib_LTLIBRARIES += lib/libopenvswitch.la
+lib_LTLIBRARIES += lib/libopenvswitchcore.la
 
-lib_libopenvswitch_la_LIBADD = $(SSL_LIBS)
+# Dummy library to link against doesn't have any sources, but does
+# depend on libopenvswitchcore static library
+lib_libopenvswitch_la_SOURCES =
+lib_libopenvswitch_la_LIBADD = lib/libopenvswitchcore.la
+
+# Dummy library continues to depend on external libraries as before
+lib_libopenvswitch_la_LIBADD += $(SSL_LIBS)
 lib_libopenvswitch_la_LIBADD += $(CAPNG_LDADD)
 lib_libopenvswitch_la_LIBADD += $(LIBBPF_LDADD)
 
@@ -18,9 +28,11 @@ endif
 lib_libopenvswitch_la_LDFLAGS = \
 $(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/lib/libopenvswitch.sym \
+$(lib_libopenvswitchcore_la_LIBS) \
 $(AM_LDFLAGS)
 
-lib_libopenvswitch_la_SOURCES = \
+# Build core vswitch libraries as before
+lib_libopenvswitchcore_la_SOURCES = \
lib/aes128.c \
lib/aes128.h \
lib/async-append.h \
@@ -344,7 +356,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/lldp/lldpd-structs.h
 
 if WIN32
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/daemon-windows.c \
lib/getopt_long.c \
lib/getrusage-windows.c \
@@ -354,7 +366,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/stream-windows.c \
lib/strsep.c
 else
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/daemon-unix.c \
lib/latch-unix.c \
lib/signals.c \
@@ -367,13 +379,13 @@ EXTRA_DIST += \
lib/stdio.h.in \
lib/string.h.in
 
-nodist_lib_libopenvswitch_la_SOURCES = \
+nodist_lib_libopenvswitchcore_la_SOURCES = \
lib/dirs.c \
lib/ovsdb-server-idl.c \
lib/ovsdb-server-idl.h \
lib/vswitch-idl.c \
lib/vswitch-idl.h
-CLEANFILES += $(nodist_lib_libopenvswitch_la_SOURCES)
+CLEANFILES += $(nodist_lib_libopenvswitchcore_la_SOURCES)
 
 lib_LTLIBRARIES += lib/libsflow.la
 lib_libsflow_la_LDFLAGS = \
@@ -397,7 +409,7 @@ lib_libsflow_la_CFLAGS += -Wno-unused-parameter
 endif
 
 if LINUX
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/dpif-netlink.c \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.c \
@@ -423,7 +435,7 @@ lib_libopenvswitch_la_SOURCES += \
 endif
 
 if HAVE_AF_XDP
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/netdev-afxdp-pool.c \
lib/netdev-afxdp-pool.h \
lib/netdev-afxdp.c \
@@ -431,17 +443,17 @@ lib_libopenvswitch_la_SOURCES += \
 endif
 
 if DPDK_NETDEV
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/dpdk.c \
lib/netdev-dpdk.c \
lib/netdev-offload-dpdk.c
 else
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/dpdk-stub.c
 endif
 
 if WIN32
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \
lib/dpif-netlink.c \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.h \
@@ -458,13 +470,13 @@ lib_libopenvswitch_la_SOURCES += \
 endif
 
 if HAVE_POSIX_AIO
-lib_libopenvswitch_la_SOURCES += lib/async-append-aio.c
+lib_libopenvswitchcore_la_SOURCES += lib/async-append-aio.c
 else
-lib_libopenvswitch_la_SOURCES += lib/async-append-null.c
+lib_libopenvswitchcore_la_SOURCES += lib/async-append-null.c
 endif
 
 if HAVE_IF_DL
-lib_libopenvswitch_la_SOURCES += \
+lib_libopenvswitchcore_la_SOURCES += \

[ovs-dev] [PATCH v3 7/7] docs/dpdk/bridge: add datapath performance section

[Harry van Haaren]

This commit adds a section to the dpdk/bridge.rst netdev documentation,
detailing the added DPCLS functionality. The newely added commands are
documented, and sample output is provided.

Signed-off-by: Harry van Haaren 
---
 Documentation/topics/dpdk/bridge.rst | 63 
 1 file changed, 63 insertions(+)

diff --git a/Documentation/topics/dpdk/bridge.rst 
b/Documentation/topics/dpdk/bridge.rst
index f0ef42ecc..2ada76571 100644
--- a/Documentation/topics/dpdk/bridge.rst
+++ b/Documentation/topics/dpdk/bridge.rst
@@ -137,3 +137,66 @@ currently turned off by default.
 To turn on SMC::
 
 $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=true
+
+Datapath Classifier Performance
+---
+
+The datapath classifier (dpcls) performs wildcard rule matching, a compute
+intensive process of matching a packet ``miniflow`` to a rule ``miniflow``. The
+code that does this compute work impacts datapath performance, and optimizing
+it can provide higher switching performance.
+
+Modern CPUs provide extensive SIMD instructions which can be used to get higher
+performance. The CPU OVS is being deployed on must be capable of running these
+SIMD instructions in order to take advantage of the performance benefits.
+In OVS v2.14 runtime CPU detection was introduced to enable identifing if these
+CPU ISA additions are available, and to allow the user to enable them.
+
+OVS provides multiple implementations of dpcls. The following command enables
+the user to check what implementations are available in a running instance ::
+
+$ ovs-appctl dpif-netdev/subtable-lookup-get
+Available lookup functions (priority : name)
+0 : autovalidator
+1 : generic
+0 : avx512_gather
+
+To set the priority of a lookup function, run the ``prio-set`` command ::
+
+$ ovs-appctl dpif-netdev/subtable-lookup-prio-set avx512_gather 5
+Lookup priority change affected 1 dpcls ports and 1 subtables.
+
+The highest priority lookup function is used for classification, and the output
+above indicates that one subtable of one DPCLS port is has changed its lookup
+function due to the command being run. To verify the prioritization, re-run the
+get command, note the updated priority of the ``avx512_gather`` function ::
+
+$ ovs-appctl dpif-netdev/subtable-lookup-get
+Available lookup functions (priority : name)
+0 : autovalidator
+1 : generic
+5 : avx512_gather
+
+If two lookup functions have the same priority, the first one in the list is
+chosen, and the 2nd occurance of that priority is not used. Put in logical
+terms, a subtable is chosen if its priority is greater than the previous
+best candidate.
+
+CPU ISA Testing and Validation
+~~
+
+As multiple versions of DPCLS can co-exist, each with different CPU ISA
+optimizations, it is important to validate that they all give the exact same
+results. To easily test all DPCLS implementations, an ``autovalidator``
+implementation of the DPCLS exists. This implementation runs all other
+available DPCLS implementations, and verifies that the results are identical.
+
+Running the OVS unit tests with the autovalidator enabled ensures all
+implementations provide the same results. Note that the performance of the
+autovalidator is lower than all other implementations, as it tests the scalar
+implementation against itself, and against all other enabled DPCLS
+implementations.
+
+To adjust the DPCLS autovalidator priority, use this command ::
+
+$ ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 7
-- 
2.17.1

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

[ovs-dev] [PATCH v3 6/7] dpif-lookup: add avx512 gather implementation

[Harry van Haaren]

This commit adds an AVX-512 dpcls lookup implementation.
It uses the AVX-512 SIMD ISA to perform multiple miniflow
operations in parallel.

To run this implementation, the "avx512f" and "bmi2" ISAs are
required. These ISA checks are performed at runtime while
probing the subtable implementation. If a CPU does not provide
both "avx512f" and "bmi2", then this code does not execute.

The avx512 code is built as a seperate static library, with added
CFLAGS to enable the required ISA features. By building only this
static library with avx512 enabled, it is ensured that the main OVS
core library is *not* using avx512, and that OVS continues to run
as before on CPUs that do not support avx512.

The approach taken in this implementation is to use the
gather instruction to access the packet miniflow, allowing
any miniflow blocks to be loaded into an AVX-512 register.
This maximises the usefulness of the register, and hence this
implementation handles any subtable with up to miniflow 8 bits.

Note that specialization of these avx512 lookup routines
still provides performance value, as the hashing of the
resulting data is performed in scalar code, and compile-time
loop unrolling occurs when specialized to miniflow bits.

Signed-off-by: Harry van Haaren 

---

v3:
- Improve function name for _any subtable lookup
- Use "" include not <> for immintrin.h
- Add checks for SSE42 instructions in core OVS for CRC32 based hashing
  If not available, disable AVX512 lookup implementation as it requires
  uses CRC32 for hashing, and the hashing algorithm must match core OVS.
  Issue a #warning when building x86_64 without SSE42 for core OVS.
- Rework ovs_asserts() into function selection time check
- Add #define for magic number 8, number of u64 blocks in AVX512 register
- Add #if CHECKER around AVX code, sparse doesn't like checking it
- Remove #warning if SSE42 isn't available. There is now no message if
  the AVX512 routines are not being compiled into the binary, however
  the "subtable-lookup-get" command will not return it in the list.

hvh: comment #warning for crc32 sse42 isa

Signed-off-by: Harry van Haaren 

hvh: avx512 add #if __CHECKER__

Signed-off-by: Harry van Haaren 
---
 lib/automake.mk|  16 ++
 lib/dpif-netdev-lookup-avx512-gather.c | 265 +
 lib/dpif-netdev-lookup.c   |  15 ++
 lib/dpif-netdev-lookup.h   |   7 +
 lib/dpif-netdev.c  |   4 +
 5 files changed, 307 insertions(+)
 create mode 100644 lib/dpif-netdev-lookup-avx512-gather.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 19e454c4b..d8a05b384 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -8,13 +8,16 @@
 # libopenvswitch.la is the library to link against for binaries like vswitchd.
 # The code itself is built as two seperate static libraries;
 # - core: Core files, always compiled with distro provided CFLAGS
+# - lookupavx512: ISA optimized routines that require CPUID checks at runtime
 lib_LTLIBRARIES += lib/libopenvswitch.la
 lib_LTLIBRARIES += lib/libopenvswitchcore.la
+lib_LTLIBRARIES += lib/libopenvswitchlookupavx512.la
 
 # Dummy library to link against doesn't have any sources, but does
 # depend on libopenvswitchcore static library
 lib_libopenvswitch_la_SOURCES =
 lib_libopenvswitch_la_LIBADD = lib/libopenvswitchcore.la
+lib_libopenvswitch_la_LIBADD += lib/libopenvswitchlookupavx512.la
 
 # Dummy library continues to depend on external libraries as before
 lib_libopenvswitch_la_LIBADD += $(SSL_LIBS)
@@ -31,6 +34,19 @@ lib_libopenvswitch_la_LDFLAGS = \
 $(lib_libopenvswitchcore_la_LIBS) \
 $(AM_LDFLAGS)
 
+
+# Build lookupavx512 library with extra CFLAGS enabled. This allows the
+# compiler to use the ISA features required for the ISA optimized code-paths.
+lib_libopenvswitchlookupavx512_la_CFLAGS = \
+   -mavx512f \
+   -mavx512bw \
+   -mavx512dq \
+   -mbmi2 \
+   $(AM_CFLAGS)
+lib_libopenvswitchlookupavx512_la_SOURCES = \
+   lib/dpif-netdev-lookup-avx512-gather.c
+
+
 # Build core vswitch libraries as before
 lib_libopenvswitchcore_la_SOURCES = \
lib/aes128.c \
diff --git a/lib/dpif-netdev-lookup-avx512-gather.c 
b/lib/dpif-netdev-lookup-avx512-gather.c
new file mode 100644
index 0..754cd0e3c
--- /dev/null
+++ b/lib/dpif-netdev-lookup-avx512-gather.c
@@ -0,0 +1,265 @@
+/*
+ * Copyright (c) 2020, Intel Corperation.
+ *
+ * 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.
+ */
+

[ovs-dev] [PATCH v3 2/7] dpif-netdev: add subtable lookup set command

[Harry van Haaren]

This commit adds a command for the dpif-netdev to set a specific
lookup function to a particular priority level. The command enables
runtime switching of the dpcls subtable lookup implementation.

Selection is performed based on a priority. Higher priorities take
precedence, eg; priotity 5 will be selected instead of a priority 3.

The two options available are 'autovalidator' and 'generic'.
The below command will set a new priority for the given function:
$ ovs-appctl dpif-netdev/subtable-lookup-set generic 2

The autovalidator implementation can be selected at runtime now:
$ ovs-appctl dpif-netdev/subtable-lookup-set autovalidator 5

Signed-off-by: Harry van Haaren 

---

v3
- Add automatic reprobe after changing priorities
--- Refactored from previous 1-second timeout based reprobe WIP-hack
- Add VLOG entries for changed dpcls and subtable counts
--- Also return the updated counts to the issuing command for visibility
- Clarify command by adding "prio" to the name
--- New command name is "dpif-netdev/subtable-lookup-prio-set"
--- Please note this new command change - previous command is now invalid
---
 lib/dpif-netdev.c | 121 ++
 1 file changed, 121 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5e101e054..30806af16 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -258,6 +258,7 @@ struct dp_packet_flow_map {
 static void dpcls_init(struct dpcls *);
 static void dpcls_destroy(struct dpcls *);
 static void dpcls_sort_subtable_vector(struct dpcls *);
+static uint32_t dpcls_subtable_lookup_reprobe(struct dpcls *cls);
 static void dpcls_insert(struct dpcls *, struct dpcls_rule *,
  const struct netdev_flow_key *mask);
 static void dpcls_remove(struct dpcls *, struct dpcls_rule *);
@@ -860,6 +861,9 @@ dpif_netdev_xps_revalidate_pmd(const struct 
dp_netdev_pmd_thread *pmd,
bool purge);
 static int dpif_netdev_xps_get_tx_qid(const struct dp_netdev_pmd_thread *pmd,
   struct tx_port *tx);
+static inline struct dpcls *
+dp_netdev_pmd_lookup_dpcls(struct dp_netdev_pmd_thread *pmd,
+   odp_port_t in_port);
 
 static inline bool emc_entry_alive(struct emc_entry *ce);
 static void emc_clear_entry(struct emc_entry *ce);
@@ -1260,6 +1264,97 @@ sorted_poll_thread_list(struct dp_netdev *dp,
 *n = k;
 }
 
+static void
+dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
+const char *argv[], void *aux OVS_UNUSED)
+{
+/* This function requires 2 parameters (argv[1] and argv[2]) to execute.
+ *   argv[1] is subtable name
+ *   argv[2] is priority
+ *   argv[3] is the datapath name (optional if only 1 datapath exists)
+ */
+const char *func_name = argv[1];
+
+errno = 0;
+char *err_char;
+uint32_t new_prio = strtoul(argv[2], _char, 10);
+if (errno != 0 || new_prio > UINT8_MAX) {
+unixctl_command_reply_error(conn,
+"error converting priority, use integer in range 0-255\n");
+return;
+}
+
+int32_t err = dpcls_subtable_set_prio(func_name, new_prio);
+if (err) {
+unixctl_command_reply_error(conn,
+"error, subtable lookup function not found\n");
+return;
+}
+
+/* argv[3] is optional datapath instance. If no datapath name is provided
+ * and only one datapath exists, the one existing datapath is reprobed.
+ */
+ovs_mutex_lock(_netdev_mutex);
+struct dp_netdev *dp = NULL;
+
+if (argc == 4) {
+dp = shash_find_data(_netdevs, argv[3]);
+} else if (shash_count(_netdevs) == 1) {
+dp = shash_first(_netdevs)->data;
+}
+
+if (!dp) {
+ovs_mutex_unlock(_netdev_mutex);
+unixctl_command_reply_error(conn,
+"please specify an existing datapath");
+return;
+}
+
+/* Get PMD threads list, required to get DPCLS instances */
+size_t n;
+uint32_t lookup_dpcls_changed = 0;
+uint32_t lookup_subtable_changed = 0;
+struct dp_netdev_pmd_thread **pmd_list;
+sorted_poll_thread_list(dp, _list, );
+
+/* take port mutex as HMAP iters over them */
+ovs_mutex_lock(>port_mutex);
+
+for (size_t i = 0; i < n; i++) {
+struct dp_netdev_pmd_thread *pmd = pmd_list[i];
+if (pmd->core_id == NON_PMD_CORE_ID) {
+continue;
+}
+
+struct dp_netdev_port *port = NULL;
+HMAP_FOR_EACH (port, node, >ports) {
+odp_port_t in_port = port->port_no;
+struct dpcls *cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
+if (!cls) {
+continue;
+}
+uint32_t subtbl_changes = dpcls_subtable_lookup_reprobe(cls);
+if (subtbl_changes) {
+lookup_dpcls_changed++;
+lookup_subtable_changed += subtbl_changes;
+}
+}
+ 

[ovs-dev] [PATCH v3 1/7] dpif: implement subtable lookup validation

[Harry van Haaren]

This commit refactors the existing dpif subtable function pointer
infrastructure, and implements an autovalidator component.

The refactoring of the existing dpcls subtable lookup function
handling, making it more generic, and cleaning up how to enable
more implementations in future.

In order to ensure all implementations provide identical results,
the autovalidator is added. The autovalidator itself implements
the subtable lookup function prototype, but internally iterates
over all other available implementations. The end result is that
testing of each implementation becomes automatic, when the auto-
validator implementation is selected.

Signed-off-by: Harry van Haaren 

v3:
- Fix compile error by adding errno.h include (William Tu)
- Improve vlog prints by using hex not int for bitmasks
- Update license years adding 2020
- Fix 0 used as NULL pointer
---
 lib/automake.mk|   3 +
 lib/dpif-netdev-lookup-autovalidator.c | 106 +
 lib/dpif-netdev-lookup-generic.c   |   9 ++-
 lib/dpif-netdev-lookup.c   |  96 ++
 lib/dpif-netdev-lookup.h   |  75 +
 lib/dpif-netdev-private.h  |  15 
 lib/dpif-netdev.c  |  13 ++-
 7 files changed, 293 insertions(+), 24 deletions(-)
 create mode 100644 lib/dpif-netdev-lookup-autovalidator.c
 create mode 100644 lib/dpif-netdev-lookup.c
 create mode 100644 lib/dpif-netdev-lookup.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 86940ccd2..9dbc2bbc5 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -81,7 +81,10 @@ lib_libopenvswitch_la_SOURCES = \
lib/dp-packet.h \
lib/dp-packet.c \
lib/dpdk.h \
+   lib/dpif-netdev-lookup.h \
+   lib/dpif-netdev-lookup.c \
lib/dpif-netdev-lookup-generic.c \
+   lib/dpif-netdev-lookup-autovalidator.c \
lib/dpif-netdev.c \
lib/dpif-netdev.h \
lib/dpif-netdev-private.h \
diff --git a/lib/dpif-netdev-lookup-autovalidator.c 
b/lib/dpif-netdev-lookup-autovalidator.c
new file mode 100644
index 0..0b759a5b9
--- /dev/null
+++ b/lib/dpif-netdev-lookup-autovalidator.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2020 Intel Corporation.
+ *
+ * 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 "dpif-netdev.h"
+#include "dpif-netdev-private.h"
+#include "dpif-netdev-lookup.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(dpif_lookup_autovalidator);
+
+/* This file implements an automated validator for subtable search
+ * implementations. It compares the results of the generic scalar search result
+ * with ISA optimized implementations.
+ *
+ * Note the goal is *NOT* to test the *specialized* versions of subtables, as
+ * the compiler performs the specialization - and we rely on the correctness of
+ * the compiler to not break those specialized variantes.
+ *
+ * The goal is to ensure identical results of the different implementations,
+ * despite that the implementations may have different methods to get those
+ * results.
+ *
+ * Example: AVX-512 ISA uses different instructions and algorithm to the scalar
+ * implementation, however the results (rules[] output) must be the same.
+ */
+
+static uint32_t
+dpcls_subtable_autovalidator(struct dpcls_subtable *subtable,
+ uint32_t keys_map,
+ const struct netdev_flow_key *keys[],
+ struct dpcls_rule **rules_good)
+{
+const uint32_t u0_bit_count = subtable->mf_bits_set_unit0;
+const uint32_t u1_bit_count = subtable->mf_bits_set_unit1;
+
+/* Scalar generic - the "known correct" version */
+dpcls_subtable_lookup_func lookup_good;
+lookup_good = dpcls_subtable_generic_probe(u0_bit_count, u1_bit_count);
+
+/* Run actual scalar implemenation to get known good results */
+uint32_t matches_good = lookup_good(subtable, keys_map, keys, rules_good);
+
+/* Now compare all other implementations against known good results.
+ * Note we start iterating from array[2], as 0 is autotester, and 1 is
+ * the known-good scalar implementation.
+ */
+/* TODO: use BUILD_BUG_ON to check for i = 2 being correct? */
+
+struct dpcls_subtable_lookup_info_t *lookup_funcs;
+int32_t lookup_func_count = dpcls_subtable_lookup_info_get(_funcs);
+if (lookup_func_count < 0) {
+VLOG_ERR("failed to get lookup subtable function 

[ovs-dev] [PATCH v3 4/7] dpcls: enable cpu feature detection

[Harry van Haaren]

This commit implements a method to retrieve the CPU ISA capabilities.
These ISA capabilities can be used in OVS to select a function
implementation that uses the best ISA available on the CPU being used.

Signed-off-by: Harry van Haaren 
---
 lib/dpdk-stub.c | 13 +
 lib/dpdk.c  | 27 +++
 lib/dpdk.h  |  2 ++
 3 files changed, 42 insertions(+)

diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
index c332c217c..9935f3d2b 100644
--- a/lib/dpdk-stub.c
+++ b/lib/dpdk-stub.c
@@ -79,6 +79,19 @@ print_dpdk_version(void)
 {
 }
 
+int
+dpdk_get_cpu_has_isa(const char *arch OVS_UNUSED,
+ const char *feature OVS_UNUSED)
+{
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+if (ovsthread_once_start()) {
+VLOG_ERR("DPDK not supported in this version of Open vSwitch, "
+ "cannot use CPU flag based optimizations");
+ovsthread_once_done();
+}
+return 0;
+}
+
 void
 dpdk_status(const struct ovsrec_open_vswitch *cfg)
 {
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 31450d470..3bea65859 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -513,6 +514,32 @@ print_dpdk_version(void)
 puts(rte_version());
 }
 
+#define CHECK_CPU_FEATURE(feature, name_str, RTE_CPUFLAG)   \
+do {\
+if (strncmp(feature, name_str, strlen(name_str)) == 0) {\
+int has_isa = rte_cpu_get_flag_enabled(RTE_CPUFLAG);\
+VLOG_DBG("CPU flag %s, available %s\n", name_str,   \
+  has_isa ? "yes" : "no");  \
+return has_isa; \
+}   \
+} while (0)
+
+int
+dpdk_get_cpu_has_isa(const char *arch, const char *feature)
+{
+/* Ensure Arch is x86_64 */
+if (strncmp(arch, "x86_64", 6) != 0) {
+return 0;
+}
+
+CHECK_CPU_FEATURE(feature, "avx512f", RTE_CPUFLAG_AVX512F);
+CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2);
+
+VLOG_WARN("Unknown CPU arch,feature: %s,%s. Returning not supported.\n",
+  arch, feature);
+return 0;
+}
+
 void
 dpdk_status(const struct ovsrec_open_vswitch *cfg)
 {
diff --git a/lib/dpdk.h b/lib/dpdk.h
index 736a64279..818dfcbba 100644
--- a/lib/dpdk.h
+++ b/lib/dpdk.h
@@ -44,4 +44,6 @@ bool dpdk_per_port_memory(void);
 bool dpdk_available(void);
 void print_dpdk_version(void);
 void dpdk_status(const struct ovsrec_open_vswitch *);
+int dpdk_get_cpu_has_isa(const char * arch, const char *feature);
+
 #endif /* dpdk.h */
-- 
2.17.1

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

[ovs-dev] [PATCH v3 3/7] dpif-netdev: add subtable-lookup-get command for usability

[Harry van Haaren]

This commit introduces a new command, "dpif-netdev/subtable-lookup-get"
which prints the avaiable sutable lookup functions available in this OVS
binary. Example output from the command:

Available lookup functions (priority : name)
0 : autovalidator
1 : generic

Signed-off-by: Harry van Haaren 
---
 lib/dpif-netdev.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 30806af16..cd4e1dbb1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1264,6 +1264,30 @@ sorted_poll_thread_list(struct dp_netdev *dp,
 *n = k;
 }
 
+static void
+dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[] OVS_UNUSED,
+void *aux OVS_UNUSED)
+{
+/* Get a list of all lookup functions */
+struct dpcls_subtable_lookup_info_t *lookup_funcs = NULL;
+int32_t count = dpcls_subtable_lookup_info_get(_funcs);
+if (count < 0) {
+unixctl_command_reply_error(conn, "error getting lookup names");
+return;
+}
+
+/* Add all lookup functions to reply string */
+struct ds reply = DS_EMPTY_INITIALIZER;
+ds_put_cstr(, "Available lookup functions (priority : name)\n");
+for (int i = 0; i < count; i++) {
+ds_put_format(, "\t%d : %s\n", lookup_funcs[i].prio,
+  lookup_funcs[i].name);
+}
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+}
+
 static void
 dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
 const char *argv[], void *aux OVS_UNUSED)
@@ -1528,6 +1552,9 @@ dpif_netdev_init(void)
 "[lookup_func] [prio] [dp]",
  2, 3, dpif_netdev_subtable_lookup_set,
  NULL);
+unixctl_command_register("dpif-netdev/subtable-lookup-get", "",
+ 0, 0, dpif_netdev_subtable_lookup_get,
+ NULL);
 return 0;
 }
 
-- 
2.17.1

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

[ovs-dev] [PATCH v3 0/7] DPCLS Subtable ISA Optimization

[Harry van Haaren]

v3 Changes Summary:
- Added new "subtable lookup get" command for ease of use
- Changed set command to include "prio" aligning with other commands
- Improved output of "subtable lookup prio set" command
- Added documentation
- Minor code cleanups, #defines for magic numbers, typos etc
- Implement fix for hash-mismatch issue (reported by William Tu)

v4 Planned work:
- Add NEWS section
- Investigate/fix --enable-shared builds link-time issues
- Enable autovalidator to run with unit-tests without recompilation
  (Already works now, but requires manual priority change at compile time)
- Address other feedback on v3


This patchset implements the changes as proposed during the
OVS Conf '19, in the talk "Next steps for SW Datapath".
Youtube link: https://youtu.be/x0bOpojnpmU

The talk raises 3 main requirements for CPU ISA Optimizations,
each of which is addressed in some of the patches below.
- Test & Validation (video @ 2:20)
- Usabiliity & Debug (video @ 6:00)
- Package & Deploy (video @ 8:45)

Patch 1/7:
The test and validation requirements proposed above are implemented,
with the refactor of the subtable function pointer registration,
and the autovalidator implementation is added.

Patch 2 & 3 / 7:
Adds the commands for usability & debug. Now improved with a "get" and
"set" command. Get returns current priorities and a list of each lookup
implementation. Set provides feedback to the user as to the number of
DPCLS ports/subtables that have new lookup functions due to the command
that was executed.

Patch 4/7:
Enable CPU ISA detection at runtime, providing information for future
ISA optimized functions.

Patch 5/7:
Build system changes to enable the Package & Deploy requirements,
allowing a single OVS binary to run on all CPUs, but also gain best
performance from CPU specific ISA optimizations.

Patch 6/7:
Actual AVX-512 implementation for DPCLS subtable search. This is the
actual SIMD vector code, which performs DPCLS miniflow iteration in
parallel.

Patch 7/7:
Add section in dpdk/bridges.rst on how to use the DPCLS commands, and
what they can be used for. Testing and validation using autovalidator
concept introduced, and command to set its priority is provided.


Thanks for reading, any questions please let me know.
Regards, -Harry


Harry van Haaren (7):
  dpif: implement subtable lookup validation
  dpif-netdev: add subtable lookup set command
  dpif-netdev: add subtable-lookup-get command for usability
  dpcls: enable cpu feature detection
  lib/automake: split build multiple static library
  dpif-lookup: add avx512 gather implementation
  docs/dpdk/bridge: add datapath performance section

 Documentation/topics/dpdk/bridge.rst   |  63 ++
 lib/automake.mk|  69 +--
 lib/dpdk-stub.c|  13 ++
 lib/dpdk.c |  27 +++
 lib/dpdk.h |   2 +
 lib/dpif-netdev-lookup-autovalidator.c | 106 ++
 lib/dpif-netdev-lookup-avx512-gather.c | 265 +
 lib/dpif-netdev-lookup-generic.c   |   9 +-
 lib/dpif-netdev-lookup.c   | 111 +++
 lib/dpif-netdev-lookup.h   |  82 
 lib/dpif-netdev-private.h  |  15 --
 lib/dpif-netdev.c  | 165 ++-
 12 files changed, 884 insertions(+), 43 deletions(-)
 create mode 100644 lib/dpif-netdev-lookup-autovalidator.c
 create mode 100644 lib/dpif-netdev-lookup-avx512-gather.c
 create mode 100644 lib/dpif-netdev-lookup.c
 create mode 100644 lib/dpif-netdev-lookup.h

-- 
2.17.1

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

Re: [ovs-dev] [PATCH ovn v10 3/8] ovn-controller: I-P for datapath binding

[Numan Siddique]

On Wed, Jun 10, 2020 at 3:31 PM Numan Siddique  wrote:

>
>
> On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara  wrote:
>
>> On 6/9/20 7:10 PM, Numan Siddique wrote:
>> >
>> >
>> > On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara > > > wrote:
>> >
>> > On 6/8/20 3:50 PM, num...@ovn.org  wrote:
>> > > From: Numan Siddique mailto:num...@ovn.org>>
>> > >
>> > > This patch adds partial support of incremental processing of
>> > datapath binding.
>> > > If a datapath is deleted, then a full recompute is triggered if
>> that
>> > > datapath is present in the 'local_datapaths' hmap of runtime data.
>> > >
>> > > Acked-by: Mark Michelson > > >
>> > > Acked-by: Han Zhou mailto:hz...@ovn.org>>
>> > > Signed-off-by: Numan Siddique > num...@ovn.org>>
>> >
>> > Looks good to me.
>> >
>> > Acked-by: Dumitru Ceara > dce...@redhat.com>>
>> >
>> >
>> >
>> > Thanks Dumitru, Han and Mark  for the reviews.
>> >
>> > I applied the first 3 patches of this series (addressing the review
>> > comments) to master and also applied to branch-20.06.
>> >
>> > @Han - If you have any additional comments on these patches please let
>> > me know. I'll have follow up patches.
>> >
>> > I'll update v11 of this series addressing the review comments from
>> Dumitru.
>> >
>> > Thanks
>> > Numan
>> >
>>
>> Hi Numan,
>>
>> I spotted a bug introduced by these 3 patches. The following scenario is
>> now broken:
>>
>> ovn-nbctl lr-add rtr
>> ovn-nbctl lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24
>> ovn-nbctl ls-add ls
>> ovn-nbctl lsp-add ls ls-rtr
>> ovn-nbctl lsp-set-addresses ls-rtr 00:00:00:00:01:00
>> ovn-nbctl lsp-set-type ls-rtr router
>> ovn-nbctl lsp-set-options ls-rtr router-port=rtr-ls
>> ovn-nbctl lsp-add ls vm1
>> ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01
>>
>> ovn-nbctl lsp-add ls vm2
>> ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02
>>
>> ip netns add vm1
>> ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
>> ip link set vm1 netns vm1
>> ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
>> ip netns exec vm1 ip addr add 42.42.42.2/24 dev vm1
>> ip netns exec vm1 ip link set vm1 up
>> ovs-vsctl set Interface vm1 external_ids:iface-id=vm1
>>
>> ip netns add vm2
>> ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
>> ip link set vm2 netns vm2
>> ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
>> ip netns exec vm2 ip addr add 42.42.42.3/24 dev vm2
>> ip netns exec vm2 ip link set vm2 up
>> ovs-vsctl set Interface vm2 external_ids:iface-id=vm2
>>
>> # Works
>> ip netns exec vm1 ping 42.42.42.3 -c 1
>>
>> # Restart ovn-controller
>> ovn-ctl restart_controller
>>
>> # Doesn't work
>> ip netns exec vm1 ping 42.42.42.3 -c 1
>>
>> # Delete port bindings
>> ovn-sbctl destroy port_binding vm1
>> ovn-sbctl destroy port_binding vm2
>>
>> # Works
>> ip netns exec vm1 ping 42.42.42.3 -c 1
>>
>
> Oops. Thanks for reporting this Dumitru.
>
> So when we restart, a full recompute should have been triggered.
> Looks like full recompute is not  triggered, after the IDL contents are
> received.
>

As we discussed offline and the reason you pointed out that
binding_handle_port_binding_changes()
is not returning true when it processes for the port binding initial dump
it received and hence not
calling update_sb_monitors().

The issue is not seen with the v11 of the I-P series because it does return
true. And also the issue is not
seen with ovn-monitor-all is set.

But of course we should first fix this issue. Thanks for looking into it.

Thanks
Numan


> Thanks
> Numan
>
>
>> Regards,
>> Dumitru
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v10 3/8] ovn-controller: I-P for datapath binding

[Numan Siddique]

On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara  wrote:

> On 6/9/20 7:10 PM, Numan Siddique wrote:
> >
> >
> > On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara  > > wrote:
> >
> > On 6/8/20 3:50 PM, num...@ovn.org  wrote:
> > > From: Numan Siddique mailto:num...@ovn.org>>
> > >
> > > This patch adds partial support of incremental processing of
> > datapath binding.
> > > If a datapath is deleted, then a full recompute is triggered if
> that
> > > datapath is present in the 'local_datapaths' hmap of runtime data.
> > >
> > > Acked-by: Mark Michelson  > >
> > > Acked-by: Han Zhou mailto:hz...@ovn.org>>
> > > Signed-off-by: Numan Siddique  num...@ovn.org>>
> >
> > Looks good to me.
> >
> > Acked-by: Dumitru Ceara mailto:dce...@redhat.com
> >>
> >
> >
> >
> > Thanks Dumitru, Han and Mark  for the reviews.
> >
> > I applied the first 3 patches of this series (addressing the review
> > comments) to master and also applied to branch-20.06.
> >
> > @Han - If you have any additional comments on these patches please let
> > me know. I'll have follow up patches.
> >
> > I'll update v11 of this series addressing the review comments from
> Dumitru.
> >
> > Thanks
> > Numan
> >
>
> Hi Numan,
>
> I spotted a bug introduced by these 3 patches. The following scenario is
> now broken:
>
> ovn-nbctl lr-add rtr
> ovn-nbctl lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24
> ovn-nbctl ls-add ls
> ovn-nbctl lsp-add ls ls-rtr
> ovn-nbctl lsp-set-addresses ls-rtr 00:00:00:00:01:00
> ovn-nbctl lsp-set-type ls-rtr router
> ovn-nbctl lsp-set-options ls-rtr router-port=rtr-ls
> ovn-nbctl lsp-add ls vm1
> ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01
>
> ovn-nbctl lsp-add ls vm2
> ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02
>
> ip netns add vm1
> ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
> ip link set vm1 netns vm1
> ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
> ip netns exec vm1 ip addr add 42.42.42.2/24 dev vm1
> ip netns exec vm1 ip link set vm1 up
> ovs-vsctl set Interface vm1 external_ids:iface-id=vm1
>
> ip netns add vm2
> ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
> ip link set vm2 netns vm2
> ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
> ip netns exec vm2 ip addr add 42.42.42.3/24 dev vm2
> ip netns exec vm2 ip link set vm2 up
> ovs-vsctl set Interface vm2 external_ids:iface-id=vm2
>
> # Works
> ip netns exec vm1 ping 42.42.42.3 -c 1
>
> # Restart ovn-controller
> ovn-ctl restart_controller
>
> # Doesn't work
> ip netns exec vm1 ping 42.42.42.3 -c 1
>
> # Delete port bindings
> ovn-sbctl destroy port_binding vm1
> ovn-sbctl destroy port_binding vm2
>
> # Works
> ip netns exec vm1 ping 42.42.42.3 -c 1
>

Oops. Thanks for reporting this Dumitru.

So when we restart, a full recompute should have been triggered.
Looks like full recompute is not  triggered, after the IDL contents are
received.

Thanks
Numan


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

Re: [ovs-dev] [PATCH v2] netdev-offload-dpdk: Support offload of VLAN PUSH/POP actions

[Sriharsha Basavapatna via dev]

Just a gentle reminder on this patch. It has already been ack'd by Eli.
Thanks,
-Harsha

On Tue, Jun 2, 2020 at 10:16 PM Sriharsha Basavapatna
 wrote:
>
> If there are no other comments, can this be applied to master ?
>
> Thanks,
> -Harsha
>
> On Mon, Jun 1, 2020 at 7:54 PM Eli Britstein  wrote:
> >
> > Acked-by: Eli Britstein 
> >
> > On 5/29/2020 9:33 AM, Sriharsha Basavapatna wrote:
> > > Parse VLAN PUSH/POP OVS datapath actions and add respective RTE actions.
> > >
> > > Signed-off-by: Sriharsha Basavapatna 
> > > ---
> > > v1->v2:
> > > * Updated dump_flow_action() to print VLAN Push/Pop actions
> > > * Updated NEWS, Documentation/howto/dpdk.rst files
> > > ---
> > >
> > >   Documentation/howto/dpdk.rst |  1 +
> > >   NEWS |  1 +
> > >   lib/netdev-offload-dpdk.c| 64 
> > >   3 files changed, 66 insertions(+)
> > >
> > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> > > index be950d7ce..c40fcafcb 100644
> > > --- a/Documentation/howto/dpdk.rst
> > > +++ b/Documentation/howto/dpdk.rst
> > > @@ -395,6 +395,7 @@ Supported actions for hardware offload are:
> > >   - Modification of Ethernet (mod_dl_src/mod_dl_dst).
> > >   - Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
> > >   - Modification of TCP/UDP (mod_tp_src/mod_tp_dst).
> > > +- VLAN Push/Pop (push_vlan/pop_vlan).
> > >
> > >   Further Reading
> > >   ---
> > > diff --git a/NEWS b/NEWS
> > > index 3dbd8ec0e..c1311e366 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -9,6 +9,7 @@ Post-v2.13.0
> > >  - DPDK:
> > >* Deprecated DPDK pdump packet capture support removed.
> > >* Deprecated DPDK ring ports (dpdkr) are no longer supported.
> > > + * Add hardware offload support for VLAN Push/Pop actions 
> > > (experimental).
> > >  - Linux datapath:
> > >* Support for kernel versions up to 5.5.x.
> > >  - AF_XDP:
> > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > > index f8c46bbaa..c57586a48 100644
> > > --- a/lib/netdev-offload-dpdk.c
> > > +++ b/lib/netdev-offload-dpdk.c
> > > @@ -420,6 +420,36 @@ dump_flow_action(struct ds *s, const struct 
> > > rte_flow_action *actions)
> > >   } else {
> > >   ds_put_format(s, "  Set-%s-tcp/udp-port = null\n", dirstr);
> > >   }
> > > +} else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN) {
> > > +const struct rte_flow_action_of_push_vlan *rte_push_vlan =
> > > +actions->conf;
> > > +ds_put_cstr(s, "rte flow push-vlan action:\n");
> > > +if (rte_push_vlan) {
> > > +ds_put_format(s, "  Push-vlan: 0x%"PRIx16"\n",
> > > +  ntohs(rte_push_vlan->ethertype));
> > > +} else {
> > > +ds_put_format(s, "  Push-vlan = null\n");
> > > +}
> > > +} else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP) {
> > > +struct rte_flow_action_of_set_vlan_pcp *rte_vlan_pcp = 
> > > actions->conf;
> > > +ds_put_cstr(s, "rte flow set-vlan-pcp action:\n");
> > > +if (rte_vlan_pcp) {
> > > +ds_put_format(s, "  Set-vlan-pcp: %"PRIu8"\n",
> > > +  rte_vlan_pcp->vlan_pcp);
> > > +} else {
> > > +ds_put_format(s, "  Set-vlan-pcp = null\n");
> > > +}
> > > +} else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID) {
> > > +struct rte_flow_action_of_set_vlan_vid *rte_vlan_vid = 
> > > actions->conf;
> > > +ds_put_cstr(s, "rte flow set-vlan-vid action:\n");
> > > +if (rte_vlan_vid) {
> > > +ds_put_format(s, "  Set-vlan-vid: %"PRIu16"\n",
> > > +  ntohs(rte_vlan_vid->vlan_vid));
> > > +} else {
> > > +ds_put_format(s, "  Set-vlan-vid = null\n");
> > > +}
> > > +} else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_POP_VLAN) {
> > > +ds_put_cstr(s, "rte flow pop-vlan action\n");
> > >   } else {
> > >   ds_put_format(s, "unknown rte flow action (%d)\n", 
> > > actions->type);
> > >   }
> > > @@ -970,6 +1000,33 @@ parse_set_actions(struct flow_actions *actions,
> > >   return 0;
> > >   }
> > >
> > > +static int
> > > +parse_vlan_push_action(struct flow_actions *actions,
> > > +   const struct ovs_action_push_vlan *vlan_push)
> > > +{
> > > +struct rte_flow_action_of_push_vlan *rte_push_vlan;
> > > +struct rte_flow_action_of_set_vlan_pcp *rte_vlan_pcp;
> > > +struct rte_flow_action_of_set_vlan_vid *rte_vlan_vid;
> > > +
> > > +rte_push_vlan = xzalloc(sizeof *rte_push_vlan);
> > > +rte_push_vlan->ethertype = vlan_push->vlan_tpid;
> > > +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN,
> > > +rte_push_vlan);
> > > +
> > > +rte_vlan_pcp = xzalloc(sizeof *rte_vlan_pcp);
> > > +rte_vlan_pcp->vlan_pcp = 

Re: [ovs-dev] [PATCH ovn v10 3/8] ovn-controller: I-P for datapath binding

[Dumitru Ceara]

On 6/9/20 7:10 PM, Numan Siddique wrote:
> 
> 
> On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara  > wrote:
> 
> On 6/8/20 3:50 PM, num...@ovn.org  wrote:
> > From: Numan Siddique mailto:num...@ovn.org>>
> >
> > This patch adds partial support of incremental processing of
> datapath binding.
> > If a datapath is deleted, then a full recompute is triggered if that
> > datapath is present in the 'local_datapaths' hmap of runtime data.
> >
> > Acked-by: Mark Michelson  >
> > Acked-by: Han Zhou mailto:hz...@ovn.org>>
> > Signed-off-by: Numan Siddique mailto:num...@ovn.org>>
> 
> Looks good to me.
> 
> Acked-by: Dumitru Ceara mailto:dce...@redhat.com>>
> 
> 
> 
> Thanks Dumitru, Han and Mark  for the reviews. 
> 
> I applied the first 3 patches of this series (addressing the review
> comments) to master and also applied to branch-20.06.
> 
> @Han - If you have any additional comments on these patches please let
> me know. I'll have follow up patches.
> 
> I'll update v11 of this series addressing the review comments from Dumitru.
> 
> Thanks
> Numan
> 

Hi Numan,

I spotted a bug introduced by these 3 patches. The following scenario is
now broken:

ovn-nbctl lr-add rtr
ovn-nbctl lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24
ovn-nbctl ls-add ls
ovn-nbctl lsp-add ls ls-rtr
ovn-nbctl lsp-set-addresses ls-rtr 00:00:00:00:01:00
ovn-nbctl lsp-set-type ls-rtr router
ovn-nbctl lsp-set-options ls-rtr router-port=rtr-ls
ovn-nbctl lsp-add ls vm1
ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01

ovn-nbctl lsp-add ls vm2
ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02

ip netns add vm1
ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
ip link set vm1 netns vm1
ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
ip netns exec vm1 ip addr add 42.42.42.2/24 dev vm1
ip netns exec vm1 ip link set vm1 up
ovs-vsctl set Interface vm1 external_ids:iface-id=vm1

ip netns add vm2
ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
ip link set vm2 netns vm2
ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
ip netns exec vm2 ip addr add 42.42.42.3/24 dev vm2
ip netns exec vm2 ip link set vm2 up
ovs-vsctl set Interface vm2 external_ids:iface-id=vm2

# Works
ip netns exec vm1 ping 42.42.42.3 -c 1

# Restart ovn-controller
ovn-ctl restart_controller

# Doesn't work
ip netns exec vm1 ping 42.42.42.3 -c 1

# Delete port bindings
ovn-sbctl destroy port_binding vm1
ovn-sbctl destroy port_binding vm2

# Works
ip netns exec vm1 ping 42.42.42.3 -c 1

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn] Honour router_preference for solicited RA

[Gabriele Cerami]

Thanks for the review!

On 10 Jun, Numan Siddique wrote:

> I think test cases are fine. The action parsing test case makes sure that
> the
> action is encoded properly.


I'm pretty sure they are not. But in production they end up being ok.
For example checking the tcpdump output at the bottom of the
description in https://bugzilla.redhat.com/show_bug.cgi?id=1804576
you can see the RA flags are [none] (so it's using slaac) but the prefix
info flags are [onlink, auto] and the value is c0, not 80.

Thing is, I'm not sure why this happens with my patch.
Do you have time for a counter analysis ?

This was my v2 yesterday, that is getting the incorrect test result


diff --git a/lib/actions.c b/lib/actions.c
index c50615177..7066d597e 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2535,6 +2535,12 @@ parse_put_nd_ra_opts(struct action_context *ctx, const 
struct expr_field *dst,
 }
 break;
 
+case ND_RA_FLAG_PRF:
+ok = (c->string && (!strcmp(c->string, "MEDIUM") ||
+!strcmp(c->string, "HIGH") ||
+!strcmp(c->string, "LOW")));
+break;
+
 case ND_OPT_SOURCE_LINKADDR:
 ok = c->format == LEX_F_ETHERNET;
 slla_present = true;
@@ -2580,18 +2586,29 @@ format_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
 
 static void
 encode_put_nd_ra_option(const struct ovnact_gen_option *o,
-struct ofpbuf *ofpacts, ptrdiff_t ra_offset)
+struct ofpbuf *ofpacts, struct ovs_ra_msg *ra)
 {
 const union expr_constant *c = o->value.values;
 
 switch (o->option->code) {
 case ND_RA_FLAG_ADDR_MODE:
 {
-struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
 if (!strcmp(c->string, "dhcpv6_stateful")) {
-ra->mo_flags = IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
+ra->mo_flags |= IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
 } else if (!strcmp(c->string, "dhcpv6_stateless")) {
-ra->mo_flags = IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
+ra->mo_flags |= IPV6_ND_RA_FLAG_OTHER_ADDR_CONFIG;
+}
+break;
+}
+
+case ND_RA_FLAG_PRF:
+{
+if (!strcmp(c->string, "LOW")) {
+ra->mo_flags |= IPV6_ND_RA_OPT_PRF_LOW;
+} else if (!strcmp(c->string, "HIGH")) {
+ra->mo_flags |= IPV6_ND_RA_OPT_PRF_HIGH;
+} else {
+ra->mo_flags |= IPV6_ND_RA_OPT_PRF_NORMAL;
 }
 break;
 }
@@ -2622,7 +2639,6 @@ encode_put_nd_ra_option(const struct ovnact_gen_option *o,
 struct ovs_nd_prefix_opt *prefix_opt =
 ofpbuf_put_uninit(ofpacts, sizeof *prefix_opt);
 uint8_t prefix_len = ipv6_count_cidr_bits(>mask.ipv6);
-struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
 prefix_opt->type = ND_OPT_PREFIX_INFORMATION;
 prefix_opt->len = 4;
 prefix_opt->prefix_len = prefix_len;
@@ -2640,6 +2656,12 @@ encode_put_nd_ra_option(const struct ovnact_gen_option 
*o,
 break;
 }
 }
+
+/* RFC4191 section 2.2 */
+if (ntohs(ra->router_lifetime) == 0x0) {
+ra->mo_flags &= IPV6_ND_RA_OPT_PRF_RESET_MASK;
+}
+
 }
 
 static void
@@ -2660,7 +2682,6 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
  * pinctrl module receives the ICMPv6 Router Solicitation packet
  * it can copy the userdata field AS IS and resume the packet.
  */
-size_t ra_offset = ofpacts->size;
 struct ovs_ra_msg *ra = ofpbuf_put_zeros(ofpacts, sizeof *ra);
 ra->icmph.icmp6_type = ND_ROUTER_ADVERT;
 ra->cur_hop_limit = IPV6_ND_RA_CUR_HOP_LIMIT;
@@ -2669,7 +2690,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
 
 for (const struct ovnact_gen_option *o = po->options;
  o < >options[po->n_options]; o++) {
-encode_put_nd_ra_option(o, ofpacts, ra_offset);
+encode_put_nd_ra_option(o, ofpacts, ra);
 }
 
 encode_finish_controller_op(oc_offset, ofpacts);

***

> Also you need to add few tests in ovn.at for action parsing and also enhance

ah, sure !

> the test - AT_SETUP([ovn -- IPv6 ND Router Solicitation responder]) in
> ovn.at

enhance in what way? I'm covering all the cases using existing tests ... I'd
wish to cover the flags reset on 0 lifetime, but the lifetime value is
hardcoded as infinity so I'm not sure how to test that.

Thanks!

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

Re: [ovs-dev] [PATCH V2 00/12] netdev datapath offload: Support IPv6 and VXLAN encap

[Eli Britstein]

Besides Harsha's comment to improve the commit message of [PATCH V2 
07/12] netdev-offload-dpdk: Support offload of clone tnl_push/output actions


https://patchwork.ozlabs.org/project/openvswitch/patch/20200527160112.28005-8-el...@mellanox.com/ 
(Thanks Harsha).


Any other comments?

Thanks,

Eli


On 5/27/2020 7:01 PM, Eli Britstein wrote:

This patch set includes additional offloads - IPv6 and VXLAN encap, and
enhanced logging to increase debugability.

Patches #1-#8:   Add support for offloads of IPv6 patterns, partial
  TCP/UDP ports, set IPv6 and encap actions
  (clone/output).
Patch #9:Bug fix of partial offloads.
Patches #10-#11: Enhance log prints for debugability.
Patch #12:   Fix Ethernet matching for type only.

v2-v1:
- Removed redundant out label.
- Added a patch to fix dl_type match only.

Travis:
v1: 
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F688413350data=02%7C01%7Celibr%40mellanox.com%7C085f3c6e8e1942c3532108d80257565f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637261921416296969sdata=37V5aK0iVtlMaujDfSM2Aim%2BEwcYy0rOR01eekUyu5Q%3Dreserved=0
v2: 
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.org%2Fgithub%2Felibritstein%2FOVS%2Fbuilds%2F691375847data=02%7C01%7Celibr%40mellanox.com%7C085f3c6e8e1942c3532108d80257565f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637261921416306963sdata=ixnZsvJoT2VN3j5dQBtGjSpUt8aDVKWfOIqtMETCJpc%3Dreserved=0


Eli Britstein (10):
   netdev-offload-dpdk: Remove pre-validate of patterns function
   netdev-offload-dpdk: Add IPv6 pattern matching
   netdev-offload-dpdk: Support offload of set IPv6 actions
   netdev-offload-dpdk: Support partial TCP/UDP port matching
   netdev-offload-dpdk: Support offload of clone tnl_push/output actions
   netdev-offload-dpdk: Support tnl/push using vxlan encap attribute
   dpif-netdev: Don't use zero flow mark
   dpif-netdev: Add mega ufid in flow add log
   netdev-offload-dpdk: Add testpmd log commands
   netdev-offload-dpdk: Fix Ethernet matching for type only

Ilya Maximets (2):
   netdev: Allow storing dpif type into netdev structure.
   netdev-offload: Use dpif type instead of class.

  Documentation/howto/dpdk.rst  |   4 +-
  NEWS  |   3 +
  lib/dpif-netdev.c |  26 +-
  lib/dpif-netlink.c|  23 +-
  lib/dpif.c|  21 +-
  lib/netdev-offload-dpdk.c | 649 +++---
  lib/netdev-offload-tc.c   |   3 +-
  lib/netdev-offload.c  |  51 ++--
  lib/netdev-offload.h  |  16 +-
  lib/netdev-provider.h |   3 +-
  lib/netdev.c  |  16 ++
  lib/netdev.h  |   2 +
  ofproto/ofproto-dpif-upcall.c |   5 +-
  tests/dpif-netdev.at  |  20 +-
  tests/ofproto-macros.at   |   3 +-
  15 files changed, 657 insertions(+), 188 deletions(-)


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

Re: [ovs-dev] [PATCH ovn] Honour router_preference for solicited RA

[Numan Siddique]

On Tue, Jun 9, 2020 at 5:29 PM Gabriele Cerami  wrote:

> On 09 Jun, Gabriele Cerami wrote:
> > Problem is, the rest of the options assume mo_flags contains only
> addr_mode,
> > so there's a bit more to rework to make everything pass again.
>
> I got it to work reusing a single ra pointer, but I'm getting weird
> results for the test 029 - ovn -- action parsing


> I attached the testsuite.log but there's something I don't get about the
> expectations for this test: All test cases expect prefix_opt->la_flags to
> be 0x80 (on-link, stateful) even for stateless addr_modes
>
> The code that is responsible for prefix_opt->la_flags is in
> lib/actions.c:2645
>
> prefix_opt->la_flags = IPV6_ND_RA_OPT_PREFIX_ON_LINK;
> if (!(ra->mo_flags & IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG)) {
> prefix_opt->la_flags |= IPV6_ND_RA_OPT_PREFIX_AUTONOMOUS;
> }
>
> If I'm reading this correctly, the la_flags starts at least with 0x80.
> ra->mo_flags & IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG is nonzero if M flag is
> set (addr_mode is dhcpv6_stateful), zero otherwise.
>
> That means that the IPV6_ND_RA_OPT_PREFIX_AUTONOMOUS la_flag is set if
> ra M in not set, that is when addr_mode is not stateful
>
> This is correct under RFC4861 section 4.6.2
>
> But in the failing test cases the M flag is not set, yet they expect
> IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG to not be set. Reading:
>
> tests/ovn.at:1383
> addr_mode 0x00 -> la_flags 0x80 (0x00 slaac is stateless, la_flag should
> be 0xc0)
> tests/ovn.at:1389
> addr_mode 0x40 -> la_flags 0x80 (0x40 dhcpv6_stateless, la_flag should be
> 0xc0)
>
> The addr_mode should be the first value immediately before the
> ra->lifetime 0x so ff.80.ff.ff and ff.40.ff.ff
> The la_flags should be the byte just before the long .ff.ff.ff.ff.ff
> instead
>
Am I missing something ? If my analysis is correct, I'm not sure how
> tests passed before. There's a lot going on there, I would have found
> smaller tests useful in this case.
>

I think test cases are fine. The action parsing test case makes sure that
the
action is encoded properly.

With the below changes on top of your patch, the test passes for me.

*
diff --git a/lib/actions.c b/lib/actions.c
index 38a3c0ef0..c11e6aeb4 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2589,11 +2589,10 @@ encode_put_nd_ra_option(const struct
ovnact_gen_option *o,
 struct ofpbuf *ofpacts, ptrdiff_t ra_offset)
 {
 const union expr_constant *c = o->value.values;
-struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
-
 switch (o->option->code) {
 case ND_RA_FLAG_ADDR_MODE:
 {
+struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
 if (!strcmp(c->string, "dhcpv6_stateful")) {
 ra->mo_flags |= IPV6_ND_RA_FLAG_MANAGED_ADDR_CONFIG;
 } else if (!strcmp(c->string, "dhcpv6_stateless")) {
@@ -2604,6 +2603,7 @@ encode_put_nd_ra_option(const struct
ovnact_gen_option *o,

 case ND_RA_FLAG_PRF:
 {
+struct ovs_ra_msg *ra = ofpbuf_at(ofpacts, ra_offset, sizeof *ra);
 if (!strcmp(c->string, "LOW")) {
 ra->mo_flags |= IPV6_ND_RA_OPT_PRF_LOW;
 } else if (!strcmp(c->string, "HIGH")) {
@@ -2658,12 +2658,6 @@ encode_put_nd_ra_option(const struct
ovnact_gen_option *o,
 break;
 }
 }
-
-/* RFC4191 section 2.2 */
-if (ntohs(ra->router_lifetime) == 0x0) {
-ra->mo_flags &= IPV6_ND_RA_OPT_PRF_RESET_MASK;
-}
-
 }

 static void
@@ -2696,6 +2690,11 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts
*po,
 encode_put_nd_ra_option(o, ofpacts, ra_offset);
 }

+/* RFC4191 section 2.2 */
+if (ntohs(ra->router_lifetime) == 0x0) {
+ra->mo_flags &= IPV6_ND_RA_OPT_PRF_RESET_MASK;
+}
+
 encode_finish_controller_op(oc_offset, ofpacts);
 }



Also you need to add few tests in ovn.at for action parsing and also enhance
the test - AT_SETUP([ovn -- IPv6 ND Router Solicitation responder]) in
ovn.at


Thanks
Numan

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

Re: [ovs-dev] [PATCH ovn 2/2] Prepare for 20.03.2

[Numan Siddique]

On Wed, Jun 10, 2020 at 12:53 AM Mark Michelson  wrote:

> Signed-off-by: Mark Michelson 
>

Acked-by: Numan Siddique  for both the patches of the
series.

 Numan

---
>  NEWS | 3 +++
>  configure.ac | 2 +-
>  debian/changelog | 6 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index f21713432..b18d52263 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,6 @@
> +OVN v20.03.2 - xx xxx 
> +--
> +
>  OVN v20.03.1 - 09 Jun 2020
>  --
>
> diff --git a/configure.ac b/configure.ac
> index 1e378c14f..ee8703743 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>
>  AC_PREREQ(2.63)
> -AC_INIT(ovn, 20.03.1, b...@openvswitch.org)
> +AC_INIT(ovn, 20.03.2, b...@openvswitch.org)
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> diff --git a/debian/changelog b/debian/changelog
> index 8b65aa36f..3ed742674 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +OVN (20.03.2-1) unstable; urgency=low
> +
> +   * New upstream version
> +
> + -- OVN team   Tue, 09 Jun 2020 03:13:59 -0500
> +
>  OVN (20.03.1-1) unstable; urgency=low
>
> * New upstream version
> --
> 2.25.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] northd: Fix IPAM IPv4 start address calculation.

[Numan Siddique]

On Tue, Jun 9, 2020 at 12:51 PM Numan Siddique  wrote:

>
>
> On Wed, Jun 3, 2020 at 12:31 AM Mark Michelson 
> wrote:
>
>> IPAM assumes the other_config:subnet of the logical switch is a network
>> address, which can result in unusual address assignments.
>>
>> As an example, consider the following configuration:
>>
>> ovn-nbctl set logical_switch ls other_config:subnet=172.16.1.254/29
>>
>> 172.16.1.254 is not a network address of a /29 network, but ovn-northd
>> doesn't care. ovn-northd starts IP address allocation at 172.16.1.254,
>> with 7
>> assignable addresses in the subnet. The first address (172.16.1.255) is
>> reserved for router port use. The first IP addresses to a logical switch
>> port is 172.16.2.0, then 172.16.2.1, and so on.
>>
>> This patch changes the behavior by using the provided netmask to change
>> the starting IP address to the network address of the subnet. In the
>> previous example, the provided 172.16.1.254/29 would be converted
>> internally to 172.16.1.248/29 . Therefore, the first IP address
>> allocated to a switch port would be 172.16.1.250. Further allocations
>> would
>> continue up until 172.16.1.254.
>>
>> Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1823287
>>
>> Signed-off-by: Mark Michelson 
>>
>
> Acked-by: Numan Siddique \
>

I applied this patch to master.

Thanks
Numan


>
> Thanks
> Numan
>
>
>> ---
>>  northd/ovn-northd.c |  2 +-
>>  tests/ovn.at| 11 +++
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index eb78f317e..668c6c2f9 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -728,7 +728,7 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
>>  return;
>>  }
>>
>> -od->ipam_info.start_ipv4 = ntohl(subnet) + 1;
>> +od->ipam_info.start_ipv4 = ntohl(subnet & mask) + 1;
>>  od->ipam_info.total_ipv4s = ~ntohl(mask);
>>  od->ipam_info.allocated_ipv4s =
>>  bitmap_allocate(od->ipam_info.total_ipv4s);
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 15b40ca1e..25b47fdff 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -7098,6 +7098,17 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p102
>> dynamic_addresses], [0],
>>  ["00:11:22:a8:6e:0b 192.168.110.10 ae01::2"
>>  ])
>>
>> +# Configure subnet using address from middle of the subnet and ensure
>> +# address is allocated from the beginning.
>> +
>> +ovn-nbctl ls-add sw11
>> +ovn-nbctl --wait=sb set Logical-Switch sw11 other_config:subnet=
>> 172.16.1.254/29
>> +ovn-nbctl  --wait=sb lsp-add sw11
>> p103 -- lsp-set-addresses p103 "22:33:44:55:66:77 dynamic"
>> +
>> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0],
>> +["22:33:44:55:66:77 172.16.1.250"
>> +])
>> +
>>  as ovn-sb
>>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>
>> --
>> 2.25.4
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn v11 4/6] ovn-controller: Use the tracked runtime data changes for flow calculation.

[numans]

From: Venkata Anil 

This patch processes the logical flows of tracked datapaths
and tracked logical ports. To handle the tracked logical port
changes, reference of logical flows to port bindings is maintained.

Co-Authored-by: Numan Siddique 
Signed-off-by: Venkata Anil 
Signed-off-by: Numan Siddique 
---
 controller/lflow.c  |  86 +
 controller/lflow.h  |  12 +++-
 controller/ovn-controller.c | 107 ++--
 tests/ovn-performance.at|  12 ++--
 4 files changed, 156 insertions(+), 61 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 01214a3a6..eb6be0100 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -59,6 +59,10 @@ struct condition_aux {
 struct ovsdb_idl_index *sbrec_port_binding_by_name;
 const struct sbrec_chassis *chassis;
 const struct sset *active_tunnels;
+const struct sbrec_logical_flow *lflow;
+/* Resource reference to store the port name referenced
+ * in is_chassis_resident() to lhe logicl flow. */
+struct lflow_resource_ref *lfrr;
 };
 
 static bool
@@ -68,6 +72,8 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,
   struct controller_event_options *controller_event_opts,
   struct lflow_ctx_in *l_ctx_in,
   struct lflow_ctx_out *l_ctx_out);
+static void lflow_resource_add(struct lflow_resource_ref *, enum ref_type,
+   const char *ref_name, const struct uuid *);
 
 static bool
 lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
@@ -120,6 +126,14 @@ is_chassis_resident_cb(const void *c_aux_, const char 
*port_name)
 if (!pb) {
 return false;
 }
+
+/* Store the port_name to lflow reference. */
+int64_t dp_id = pb->datapath->tunnel_key;
+char buf[16];
+snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, pb->tunnel_key);
+lflow_resource_add(c_aux->lfrr, REF_TYPE_PORTBINDING, buf,
+   _aux->lflow->header_.uuid);
+
 if (strcmp(pb->type, "chassisredirect")) {
 /* for non-chassisredirect ports */
 return pb->chassis && pb->chassis == c_aux->chassis;
@@ -594,6 +608,8 @@ consider_logical_flow(const struct sbrec_logical_flow 
*lflow,
 .sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name,
 .chassis = l_ctx_in->chassis,
 .active_tunnels = l_ctx_in->active_tunnels,
+.lflow = lflow,
+.lfrr = l_ctx_out->lfrr
 };
 expr = expr_simplify(expr, is_chassis_resident_cb, _aux);
 expr = expr_normalize(expr);
@@ -649,6 +665,8 @@ consider_logical_flow(const struct sbrec_logical_flow 
*lflow,
 int64_t dp_id = lflow->logical_datapath->tunnel_key;
 char buf[16];
 snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, 
port_id);
+lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf,
+   >header_.uuid);
 if (!sset_contains(l_ctx_in->local_lport_ids, buf)) {
 VLOG_DBG("lflow "UUID_FMT
  " port %s in match is not local, skip",
@@ -847,3 +865,71 @@ lflow_destroy(void)
 expr_symtab_destroy();
 shash_destroy();
 }
+
+bool
+lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp,
+ struct lflow_ctx_in *l_ctx_in,
+ struct lflow_ctx_out *l_ctx_out)
+{
+bool handled = true;
+struct hmap dhcp_opts = HMAP_INITIALIZER(_opts);
+struct hmap dhcpv6_opts = HMAP_INITIALIZER(_opts);
+const struct sbrec_dhcp_options *dhcp_opt_row;
+SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
+   l_ctx_in->dhcp_options_table) {
+dhcp_opt_add(_opts, dhcp_opt_row->name, dhcp_opt_row->code,
+ dhcp_opt_row->type);
+}
+
+
+const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
+SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
+ l_ctx_in->dhcpv6_options_table) {
+   dhcp_opt_add(_opts, dhcpv6_opt_row->name, dhcpv6_opt_row->code,
+dhcpv6_opt_row->type);
+}
+
+struct hmap nd_ra_opts = HMAP_INITIALIZER(_ra_opts);
+nd_ra_opts_init(_ra_opts);
+
+struct controller_event_options controller_event_opts;
+controller_event_opts_init(_event_opts);
+
+struct sbrec_logical_flow *lf_row = sbrec_logical_flow_index_init_row(
+l_ctx_in->sbrec_logical_flow_by_logical_datapath);
+sbrec_logical_flow_index_set_logical_datapath(lf_row, dp);
+
+const struct sbrec_logical_flow *lflow;
+SBREC_LOGICAL_FLOW_FOR_EACH_EQUAL (
+lflow, lf_row, l_ctx_in->sbrec_logical_flow_by_logical_datapath) {
+/* Remove the lflow from flow_table if present before processing it. */
+ofctrl_remove_flows(l_ctx_out->flow_table, 

[ovs-dev] [PATCH ovn v11 6/6] Add an util function get_unique_lport_key() for generating unique lport key.

[numans]

From: Numan Siddique 

Suggested-by: Dumitru Ceara 
Signed-off-by: Numan Siddique 
---
 controller/binding.c | 8 
 controller/lflow.c   | 8 
 lib/ovn-util.h   | 8 
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index 3da19a219..27f9dbdc3 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -497,8 +497,8 @@ update_local_lport_ids(struct sset *local_lport_ids,
struct hmap *tracked_datapaths)
 {
 char buf[16];
-snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
- pb->datapath->tunnel_key, pb->tunnel_key);
+get_unique_lport_key(pb->datapath->tunnel_key, pb->tunnel_key,
+ buf, sizeof(buf));
 bool added = !!sset_add(local_lport_ids, buf);
 if (added && tracked_datapaths) {
 /* Add the 'pb' to the tracked_datapaths. */
@@ -512,8 +512,8 @@ remove_local_lport_ids(const struct sbrec_port_binding *pb,
struct hmap *tracked_datapaths)
 {
 char buf[16];
-snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
- pb->datapath->tunnel_key, pb->tunnel_key);
+get_unique_lport_key(pb->datapath->tunnel_key, pb->tunnel_key,
+ buf, sizeof(buf));
 bool deleted = sset_find_and_delete(local_lport_ids, buf);
 if (deleted && tracked_datapaths) {
 /* Add the 'pb' to the tracked_datapaths. */
diff --git a/controller/lflow.c b/controller/lflow.c
index eb6be0100..546658c60 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -130,7 +130,7 @@ is_chassis_resident_cb(const void *c_aux_, const char 
*port_name)
 /* Store the port_name to lflow reference. */
 int64_t dp_id = pb->datapath->tunnel_key;
 char buf[16];
-snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, pb->tunnel_key);
+get_unique_lport_key(dp_id, pb->tunnel_key, buf, sizeof(buf));
 lflow_resource_add(c_aux->lfrr, REF_TYPE_PORTBINDING, buf,
_aux->lflow->header_.uuid);
 
@@ -664,7 +664,7 @@ consider_logical_flow(const struct sbrec_logical_flow 
*lflow,
 if (port_id) {
 int64_t dp_id = lflow->logical_datapath->tunnel_key;
 char buf[16];
-snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, 
port_id);
+get_unique_lport_key(dp_id, port_id, buf, sizeof(buf));
 lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf,
>header_.uuid);
 if (!sset_contains(l_ctx_in->local_lport_ids, buf)) {
@@ -927,8 +927,8 @@ lflow_handle_flows_for_lport(const struct 
sbrec_port_binding *pb,
 {
 int64_t dp_id = pb->datapath->tunnel_key;
 char pb_ref_name[16];
-snprintf(pb_ref_name, sizeof(pb_ref_name), "%"PRId64"_%"PRId64,
- dp_id, pb->tunnel_key);
+get_unique_lport_key(dp_id, pb->tunnel_key, pb_ref_name,
+ sizeof(pb_ref_name));
 bool changed = true;
 return lflow_handle_changed_ref(REF_TYPE_PORTBINDING, pb_ref_name,
 l_ctx_in, l_ctx_out, );
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index e13cf4d78..eba2948ff 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -114,6 +114,14 @@ bool ovn_tnlid_in_use(const struct hmap *set, uint32_t 
tnlid);
 uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
 uint32_t max, uint32_t *hint);
 
+static inline void
+get_unique_lport_key(uint64_t dp_tunnel_key, uint64_t lport_tunnel_key,
+ char *buf, size_t buf_size)
+{
+snprintf(buf, buf_size, "%"PRId64"_%"PRId64, dp_tunnel_key,
+ lport_tunnel_key);
+}
+
 char *ovn_chassis_redirect_name(const char *port_name);
 void ovn_set_pidfile(const char *name);
 
-- 
2.26.2

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

[ovs-dev] [PATCH ovn v11 5/6] tests: Enhance ovn-performance testing by adding gw router port.

[numans]

From: Numan Siddique 

This covers the scenario of setting up/deleting of BFD tunnels
for HA.

Tested-by: Dumitru Ceara 
Signed-off-by: Numan Siddique 
---
 tests/ovn-performance.at | 104 +++
 1 file changed, 104 insertions(+)

diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at
index a12757e18..2a15cb473 100644
--- a/tests/ovn-performance.at
+++ b/tests/ovn-performance.at
@@ -239,6 +239,16 @@ for i in 1 2; do
 ovn_attach n1 br-phys 192.168.0.$i
 done
 
+for i in 1 2 3; do
+sim_add gw$i
+as gw$i
+ovs-vsctl add-br br-phys
+ovs-vsctl add-br br-ex
+ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex"
+j=$((i + 2))
+ovn_attach n1 br-phys 192.168.0.$j
+done
+
 # Wait for the tunnel ports to be created and up.
 # Otherwise this may affect the lflow_run count.
 
@@ -399,6 +409,100 @@ OVN_CONTROLLER_EXPECT_NO_HIT(
 [ovn-nbctl --wait=hv acl-add pg1 to-lport 1001 'outport == @pg1 && ip4.src 
== $pg1_ip4' allow]
 )
 
+# Create a public logical switch and attach the router to it.
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv ls-add public]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-add public public-lr1]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-set-type public-lr1 router]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-set-addresses public-lr1 router]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-set-options public-lr1 router-port=lr1-public]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lrp-add lr1 lr1-public 00:00:20:20:12:13 
172.168.0.100/24]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-add public ln-public]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-set-type ln-public localnet]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-set-addresses ln-public unknown]
+)
+
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2], [lflow_run],
+[ovn-nbctl --wait=hv lsp-set-options ln-public network_name=public]
+)
+
+OVN_CONTROLLER_EXPECT_HIT_COND(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run], [=0 =0 >0 =0 =0],
+[ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw1 30]
+)
+
+# After this, BFD should be enabled from hv1 and hv2 to gw1.
+# So there should be lflow_run hits in hv1, hv2, gw1 and gw2
+OVN_CONTROLLER_EXPECT_HIT_COND(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run], [>0 >0 >0 >0 =0],
+[ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw2 20]
+)
+
+OVN_CONTROLLER_EXPECT_HIT(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run],
+[ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw3 10]
+)
+
+# Make gw2 master.
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run],
+[ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw2 40]
+)
+
+# Delete gw2 from gateway chassis
+OVN_CONTROLLER_EXPECT_HIT(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run],
+[ovn-nbctl --wait=hv lrp-del-gateway-chassis lr1-public gw2]
+)
+
+# Delete gw1 from gateway chassis
+# After this, the BFD should be disabled entirely as gw3 is the
+# only gateway chassis.
+OVN_CONTROLLER_EXPECT_HIT_COND(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run],  [>0 >0 >0 =0 >0],
+[ovn-nbctl --wait=hv lrp-del-gateway-chassis lr1-public gw1]
+)
+
+# Delete gw3 from gateway chassis. There should be no lflow_run.
+OVN_CONTROLLER_EXPECT_NO_HIT(
+[hv1 hv2 gw1 gw2 gw3], [lflow_run],
+[ovn-nbctl --wait=hv lrp-del-gateway-chassis lr1-public gw3]
+)
+
 for i in 1 2; do
 j=$((i%2 + 1))
 lp=lp$i
-- 
2.26.2

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

[ovs-dev] [PATCH ovn v11 3/6] ovn-controller: Handle runtime data changes in flow output engine

[numans]

From: Numan Siddique 

In order to handle runtime data changes incrementally, the flow outut
runtime data handle should know the changed runtime data.
Runtime data now tracks the changed data for any OVS interface
and SB port binding changes. The tracked data contains a hmap
of tracked datapaths (which changed during runtime data processing.

The flow outout runtime_data handler in this patch doesn't do much
with the tracked data. It returns false if there is tracked data available
so that flow_output run is called. If no tracked data is available
then there is no need for flow computation and the handler returns true.

Next patch in the series processes the tracked data incrementally.

Co-Authored-by: Venkata Anil 
Signed-off-by: Venkata Anil 
Signed-off-by: Numan Siddique 
---
 controller/binding.c| 291 
 controller/binding.h|  31 +++-
 controller/ovn-controller.c | 151 +--
 tests/ovn-performance.at|  20 +--
 4 files changed, 406 insertions(+), 87 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index 61cdc8dbc..3da19a219 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -69,13 +69,25 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 ovsdb_idl_add_column(ovs_idl, _qos_col_type);
 }
 
+static struct tracked_binding_datapath *tracked_binding_datapath_create(
+const struct sbrec_datapath_binding *,
+bool is_new, struct hmap *tracked_dps);
+static struct tracked_binding_datapath *tracked_binding_datapath_find(
+struct hmap *, const struct sbrec_datapath_binding *);
+static void tracked_binding_datapath_lport_add(
+const struct sbrec_port_binding *, struct hmap *tracked_datapaths);
+static void update_lport_tracking(const struct sbrec_port_binding *pb,
+  bool old_claim, bool new_claim,
+  struct hmap *tracked_dp_bindings);
+
 static void
 add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
  struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
  struct ovsdb_idl_index *sbrec_port_binding_by_name,
  const struct sbrec_datapath_binding *datapath,
  bool has_local_l3gateway, int depth,
- struct hmap *local_datapaths)
+ struct hmap *local_datapaths,
+ struct hmap *tracked_datapaths)
 {
 uint32_t dp_key = datapath->tunnel_key;
 struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key);
@@ -92,6 +104,11 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
 ld->localnet_port = NULL;
 ld->has_local_l3gateway = has_local_l3gateway;
 
+if (tracked_datapaths &&
+!tracked_binding_datapath_find(tracked_datapaths, datapath)) {
+tracked_binding_datapath_create(datapath, true, tracked_datapaths);
+}
+
 if (depth >= 100) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 VLOG_WARN_RL(, "datapaths nested too deep");
@@ -124,7 +141,8 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
  sbrec_port_binding_by_datapath,
  sbrec_port_binding_by_name,
  peer->datapath, false,
- depth + 1, local_datapaths);
+ depth + 1, local_datapaths,
+ tracked_datapaths);
 }
 ld->n_peer_ports++;
 if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
@@ -147,12 +165,14 @@ add_local_datapath(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
struct ovsdb_idl_index *sbrec_port_binding_by_name,
const struct sbrec_datapath_binding *datapath,
-   bool has_local_l3gateway, struct hmap *local_datapaths)
+   bool has_local_l3gateway, struct hmap *local_datapaths,
+   struct hmap *tracked_datapaths)
 {
 add_local_datapath__(sbrec_datapath_binding_by_key,
  sbrec_port_binding_by_datapath,
  sbrec_port_binding_by_name,
- datapath, has_local_l3gateway, 0, local_datapaths);
+ datapath, has_local_l3gateway, 0, local_datapaths,
+ tracked_datapaths);
 }
 
 static void
@@ -473,22 +493,45 @@ update_ld_localnet_port(const struct sbrec_port_binding 
*binding_rec,
 
 static void
 update_local_lport_ids(struct sset *local_lport_ids,
-   const struct sbrec_port_binding *pb)
+   const struct sbrec_port_binding *pb,
+  

[ovs-dev] [PATCH ovn v11 1/6] I-P engine: Provide the option for an engine to clear tracked engine data in every run.

[numans]

From: Numan Siddique 

A new function is added in the engine node called - clear_tracked_data() to
clear any engine data which was tracked during the engine run. This tracked data
has to be part of the engine 'data'. engine_init_run() calls 
clear_tracked_data()
and each engine node interested in tracking the data needs to implement the
en_clear_tracked_data() function.

With this patch, an engine node can store any changes done to the engine data
separately in the engine change handlers. The parent of this engine node
can use this tracked data for incrementally processing the changes. Upcoming
patches in the series will make use of this.

Acked-by: Dumitru Ceara 
Signed-off-by: Numan Siddique 
---
 lib/inc-proc-eng.c | 8 
 lib/inc-proc-eng.h | 9 +
 2 files changed, 17 insertions(+)

diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
index 9b1479a1c..8b56cbaec 100644
--- a/lib/inc-proc-eng.c
+++ b/lib/inc-proc-eng.c
@@ -121,6 +121,10 @@ void
 engine_cleanup(void)
 {
 for (size_t i = 0; i < engine_n_nodes; i++) {
+if (engine_nodes[i]->clear_tracked_data) {
+engine_nodes[i]->clear_tracked_data(engine_nodes[i]->data);
+}
+
 if (engine_nodes[i]->cleanup) {
 engine_nodes[i]->cleanup(engine_nodes[i]->data);
 }
@@ -260,6 +264,10 @@ engine_init_run(void)
 VLOG_DBG("Initializing new run");
 for (size_t i = 0; i < engine_n_nodes; i++) {
 engine_set_node_state(engine_nodes[i], EN_STALE);
+
+if (engine_nodes[i]->clear_tracked_data) {
+engine_nodes[i]->clear_tracked_data(engine_nodes[i]->data);
+}
 }
 }
 
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 8606a360b..e25bcb29c 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -149,6 +149,10 @@ struct engine_node {
  * doesn't store pointers to DB records it's still safe to use).
  */
 bool (*is_valid)(struct engine_node *);
+
+/* Method to clear up tracked data maintained by the engine node in the
+ * engine 'data'. It may be NULL. */
+void (*clear_tracked_data)(void *tracked_data);
 };
 
 /* Initialize the data for the engine nodes. It calls each node's
@@ -282,6 +286,7 @@ void engine_ovsdb_node_add_index(struct engine_node *, 
const char *name,
 .run = en_##NAME##_run, \
 .cleanup = en_##NAME##_cleanup, \
 .is_valid = en_##NAME##_is_valid, \
+.clear_tracked_data = NULL, \
 };
 
 #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \
@@ -291,6 +296,10 @@ void engine_ovsdb_node_add_index(struct engine_node *, 
const char *name,
 static bool (*en_##NAME##_is_valid)(struct engine_node *node) = NULL; \
 ENGINE_NODE_DEF(NAME, NAME_STR)
 
+#define ENGINE_NODE_WITH_CLEAR_TRACK_DATA(NAME, NAME_STR) \
+ENGINE_NODE(NAME, NAME_STR) \
+en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data;
+
 /* Macro to define member functions of an engine node which represents
  * a table of OVSDB */
 #define ENGINE_FUNC_OVSDB(DB_NAME, TBL_NAME) \
-- 
2.26.2

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

[ovs-dev] [PATCH ovn v11 0/6] Incremental processing improvements.

[numans]

From: Numan Siddique 

This patch series handles port binding, datapath binding, ovs interface changes,
runtime data changes, sb chassis changes incrementally.


Below are the results of some testing I did with ovn-fake-multinode
setup

Test setup
--
 1. ovn-central fake node running OVN dbs and 2 compute nodes running
ovn-controller.

 2. Before running the tests, used an existing OVN db with the below
resources
   No of logical switches - 53
   No of logical ports- 1256
   No of logical routers  - 9
   No of logical router ports - 56
   No of port groups  - 152
   No of logical flows- 45447

   Port bindings on compute-1 -  19
   Port bindings on compute-2 -  18
   No of OF flows on compute-1 - 84996
   No of OF flows on compute-2 - 84901

 3. The test does the following
- Creates 2 logical switches (one for each compute node) and connect to a
  logical router for each compute node.
- 100 logical ports are created (50 per lswitch), a simple ACL is added and 
the address
  set is created for each port.
- Each port is bound on the respective compute node and the test
  pings the IP of the port (from another port belonging to the same
  lswitch created earlier).


Below are the results with OVN master

+-+
|  Response Times (sec) 
  |
+--+---++++++-+---+
| action   | min   | median | 90%ile | 95%ile | max
| avg| success | count |
+--+---++++++-+---+
| ovn.create_or_update_address_set | 0.491 | 0.519  | 0.542  | 0.548  | 0.558  
| 0.521  | 100.0%  | 100   |
| ovn.create_or_update_port_group  | 0.0   | 0.0| 0.0| 0.0| 0.001  
| 0.0| 100.0%  | 100   |
| ovn.create_port_group_acls   | 0.966 | 1.037  | 1.065  | 1.069  | 1.07   
| 1.037  | 50.0%   | 100   |
| ovn_network.bind_port| 1.242 | 1.341  | 1.397  | 1.409  | 1.443  
| 1.348  | 100.0%  | 100   |
| ovn.bind_ovs_vm  | 0.413 | 0.469  | 0.49   | 0.494  | 0.523  
| 0.469  | 100.0%  | 100   |
| ovn.bind_internal_vm | 0.804 | 0.875  | 0.921  | 0.935  | 0.95   
| 0.88   | 100.0%  | 100   |
| ovn_network.wait_port_ping   | 6.695 | 7.788  | 7.903  | 11.63  | 16.124 
| 7.997  | 100.0%  | 100   |
| total| 9.271 | 10.318 | 11.269 | 14.047 | 18.509 
| 10.871 | 100.0%  | 100   |
+--+---++++++-+---+
Load duration: 1087.5742933750153
Full duration: 1089.151035308838


Below are the results with these patches

+---+
| Response Times (sec)  
|
+--+---++++---+---+-+---+
| action   | min   | median | 90%ile | 95%ile | max   | 
avg   | success | count |
+--+---++++---+---+-+---+
| ovn.create_or_update_address_set | 0.484 | 0.506  | 0.53   | 0.536  | 0.551 | 
0.509 | 100.0%  | 100   |
| ovn.create_or_update_port_group  | 0.0   | 0.0| 0.0| 0.0| 0.0   | 
0.0   | 100.0%  | 100   |
| ovn.create_port_group_acls   | 0.966 | 1.006  | 1.032  | 1.036  | 1.059 | 
1.006 | 50.0%   | 100   |
| ovn_network.bind_port| 1.255 | 1.352  | 1.421  | 1.444  | 1.516 | 
1.352 | 100.0%  | 100   |
| ovn.bind_ovs_vm  | 0.411 | 0.455  | 0.472  | 0.476  | 0.5   | 
0.456 | 100.0%  | 100   |
| ovn.bind_internal_vm | 0.806 | 0.893  | 0.968  | 0.989  | 1.043 | 
0.896 | 100.0%  | 100   |
| ovn_network.wait_port_ping   | 0.226 | 0.253  | 0.325  | 0.329  | 0.347 | 
0.267 | 100.0%  | 100   |
| total| 2.517 | 3.137  | 3.718  | 3.749  | 3.797 | 
3.135 | 100.0%  | 100   |
+--+---++++---+---+-+---+
Load duration: 313.99292826652527
Full duration: 315.29931354522705

I ran same tests but with 1000 lports and below are the results with
these patches

+---+
| Response Times (sec)  
|
+--+---++++---+---+-+---+
| action   | min   | median | 90%ile | 95%ile | max   | 
avg   | success | count |

[ovs-dev] [PATCH ovn v11 2/6] ovn-controller: I-P for ct zone and OVS interface changes in flow output stage.

[numans]

From: Numan Siddique 

This patch handles ct zone changes and OVS interface changes incrementally
in the flow output stage.

Any changes to ct zone can be handled by running physical_run() instead of 
running
flow_output_run(). And any changes to OVS interfaces can be either handled
incrementally (for OVS interfaces representing VIFs) or just running
physical_run() (for tunnel and other types of interfaces).

To better handle this, a new engine node 'physical_flow_changes' is added which
handles changes to ct zone and OVS interfaces.

Signed-off-by: Numan Siddique 
---
 controller/binding.c|  23 +-
 controller/binding.h|  24 +-
 controller/ovn-controller.c | 145 +++-
 controller/physical.c   |  51 +
 controller/physical.h   |   5 +-
 5 files changed, 223 insertions(+), 25 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index e79220ed5..61cdc8dbc 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -502,7 +502,7 @@ remove_local_lport_ids(const struct sbrec_port_binding *pb,
  * 'struct local_binding' is used. A shash of these local bindings is
  * maintained with the 'external_ids:iface-id' as the key to the shash.
  *
- * struct local_binding has 3 main fields:
+ * struct local_binding (defined in binding.h) has 3 main fields:
  *- type
  *- OVS interface row object
  *- Port_Binding row object
@@ -553,21 +553,6 @@ remove_local_lport_ids(const struct sbrec_port_binding *pb,
  *   - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its parent
  * is bound to this chassis.
  */
-enum local_binding_type {
-BT_VIF,
-BT_CONTAINER,
-BT_VIRTUAL
-};
-
-struct local_binding {
-char *name;
-enum local_binding_type type;
-const struct ovsrec_interface *iface;
-const struct sbrec_port_binding *pb;
-
-/* shash of 'struct local_binding' representing children. */
-struct shash children;
-};
 
 static struct local_binding *
 local_binding_create(const char *name, const struct ovsrec_interface *iface,
@@ -589,12 +574,6 @@ local_binding_add(struct shash *local_bindings, struct 
local_binding *lbinding)
 shash_add(local_bindings, lbinding->name, lbinding);
 }
 
-static struct local_binding *
-local_binding_find(struct shash *local_bindings, const char *name)
-{
-return shash_find_data(local_bindings, name);
-}
-
 static void
 local_binding_destroy(struct local_binding *lbinding)
 {
diff --git a/controller/binding.h b/controller/binding.h
index f10c92bf9..e3d1f07de 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -18,6 +18,7 @@
 #define OVN_BINDING_H 1
 
 #include 
+#include "openvswitch/shash.h"
 
 struct hmap;
 struct ovsdb_idl;
@@ -32,7 +33,6 @@ struct sbrec_chassis;
 struct sbrec_port_binding_table;
 struct sset;
 struct sbrec_port_binding;
-struct shash;
 
 struct binding_ctx_in {
 struct ovsdb_idl_txn *ovnsb_idl_txn;
@@ -64,6 +64,28 @@ struct binding_ctx_out {
 struct smap *local_iface_ids;
 };
 
+enum local_binding_type {
+BT_VIF,
+BT_CONTAINER,
+BT_VIRTUAL
+};
+
+struct local_binding {
+char *name;
+enum local_binding_type type;
+const struct ovsrec_interface *iface;
+const struct sbrec_port_binding *pb;
+
+/* shash of 'struct local_binding' representing children. */
+struct shash children;
+};
+
+static inline struct local_binding *
+local_binding_find(struct shash *local_bindings, const char *name)
+{
+return shash_find_data(local_bindings, name);
+}
+
 void binding_register_ovs_idl(struct ovsdb_idl *);
 void binding_run(struct binding_ctx_in *, struct binding_ctx_out *);
 bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index bb82b15dc..a6bee1f76 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1361,8 +1361,13 @@ static void init_physical_ctx(struct engine_node *node,
 
 ovs_assert(br_int && chassis);
 
+struct ovsrec_interface_table *iface_table =
+(struct ovsrec_interface_table *)EN_OVSDB_GET(
+engine_get_input("OVS_interface", node));
+
 struct ed_type_ct_zones *ct_zones_data =
 engine_get_input_data("ct_zones", node);
+
 struct simap *ct_zones = _zones_data->current;
 
 p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name;
@@ -1370,12 +1375,14 @@ static void init_physical_ctx(struct engine_node *node,
 p_ctx->mc_group_table = multicast_group_table;
 p_ctx->br_int = br_int;
 p_ctx->chassis_table = chassis_table;
+p_ctx->iface_table = iface_table;
 p_ctx->chassis = chassis;
 p_ctx->active_tunnels = _data->active_tunnels;
 p_ctx->local_datapaths = _data->local_datapaths;
 p_ctx->local_lports = _data->local_lports;
 p_ctx->ct_zones = ct_zones;
 p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve;
+p_ctx->local_bindings = _data->local_bindings;
 }
 
 static