Re: [ovs-dev] [PATCH] selinux: allow dpdkvhostuserclient sockets with newer libvirt

[Guoshuai Li]

diff --git a/selinux/openvswitch-custom.te.in b/selinux/openvswitch-custom.te.in
index c1a774f0e..7b9c1c7a0 100644
--- a/selinux/openvswitch-custom.te.in
+++ b/selinux/openvswitch-custom.te.in
@@ -14,6 +14,7 @@ require {
  type hugetlbfs_t;
  type kernel_t;
  type svirt_image_t;

Is missing type svirt_t; ?

The compilation failed:
openvswitch-custom.te:53:ERROR 'unknown type svirt_t' at token ';' on 
line 1042:
allow openvswitch_t svirt_tmpfs_t:sock_file { read write append getattr 
open };
allow openvswitch_t svirt_t:unix_stream_socket { connectto read write 
getattr sendto recvfrom setopt };


Thanks ~!

+type svirt_tmpfs_t;
  type vfio_device_t;
  @end_dpdk@



+allow openvswitch_t svirt_t:unix_stream_socket { connectto read write getattr 
sendto recvfrom setopt };
  allow openvswitch_t vfio_device_t:chr_file { read write open ioctl getattr };

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

[ovs-dev] [PATCH] datapath-windows: On Debug builds, dump NBL info based on OVS_DBG_DEFAULT macro

[Anand Kumar]

Currently nbl information is getting dumped whenever a nbl is copied or
allocated, since OVS_DBG_DEFAULT is set to OVS_DBG_INFO for debug builds,
which affects the ovs performance. Instead dump nbl information only when
OVS_DBG_DEFAULT is set to OVS_LOG_LOUD

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/BufferMgmt.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/BufferMgmt.c 
b/datapath-windows/ovsext/BufferMgmt.c
index 03470d7..d51dafd 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -412,9 +412,11 @@ OvsAllocateFixSizeNBL(PVOID ovsContext,
 
 #ifdef DBG
 InterlockedIncrement((LONG volatile *)>fixNBLCount);
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNetBufferList(nbl);
 OvsDumpForwardingDetails(nbl);
 #endif
+#endif
 
 ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(nbl);
 ASSERT(ctx);
@@ -525,9 +527,11 @@ OvsAllocateVariableSizeNBL(PVOID ovsContext,
 
 #ifdef DBG
 InterlockedIncrement((LONG volatile *)>zeroNBLCount);
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNetBufferList(nbl);
 OvsDumpForwardingDetails(nbl);
 #endif
+#endif
 
 ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(nbl);
 
@@ -574,7 +578,9 @@ OvsInitExternalNBLContext(PVOID ovsContext,
 return NULL;
 }
 #ifdef DBG
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNBLContext(nbl);
+#endif
 InterlockedIncrement((LONG volatile *)>ovsPool.sysNBLCount);
 #endif
 flags = isRecv ? OVS_BUFFER_RECV_BUFFER : OVS_BUFFER_SEND_BUFFER;
@@ -809,12 +815,14 @@ OvsPartialCopyNBL(PVOID ovsContext,
 InterlockedIncrement((LONG volatile *)>refCount);
 
 #ifdef DBG
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNetBufferList(nbl);
 OvsDumpForwardingDetails(nbl);
 
 OvsDumpNetBufferList(newNbl);
 OvsDumpForwardingDetails(newNbl);
 #endif
+#endif
 
 OVS_LOG_LOUD("Partial Copy new NBL: %p", newNbl);
 return newNbl;
@@ -942,9 +950,11 @@ OvsCopySinglePacketNBL(PVOID ovsContext,
 dstCtx->flags |= srcCtx->flags & (OVS_BUFFER_RECV_BUFFER |
   OVS_BUFFER_SEND_BUFFER);
 #ifdef DBG
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNetBufferList(newNbl);
 OvsDumpForwardingDetails(newNbl);
 #endif
+#endif
 OVS_LOG_LOUD("Copy single nb to new NBL: %p", newNbl);
 return newNbl;
 }
@@ -1064,8 +1074,10 @@ OvsFullCopyNBL(PVOID ovsContext,
   OVS_DPPORT_NUMBER_INVALID);
 
 #ifdef DBG
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNetBufferList(nbl);
 OvsDumpForwardingDetails(nbl);
+#endif
 InterlockedIncrement((LONG volatile *)>nblOnlyCount);
 #endif
 OVS_LOG_LOUD("newNbl: %p", newNbl);
@@ -1466,13 +1478,14 @@ OvsFragmentNBL(PVOID ovsContext,
 InterlockedIncrement((LONG volatile *)>refCount);
 #ifdef DBG
 InterlockedIncrement((LONG volatile *)>fragNBLCount);
-
+#if OVS_DBG_DEFAULT >= OVS_DBG_LOUD
 OvsDumpNetBufferList(nbl);
 OvsDumpForwardingDetails(nbl);
 
 OvsDumpNetBufferList(newNbl);
 OvsDumpForwardingDetails(newNbl);
 #endif
+#endif
 OVS_LOG_TRACE("Fragment nbl %p to newNbl: %p", nbl, newNbl);
 return newNbl;
 
-- 
2.9.3.windows.1

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

[ovs-dev] El Arte de Negociar en Compras

[La Negociación en Acción]

 

En línea y en Vivo / Para todo su Equipo con una sola Conexión 

El Arte de Negociar en Compras
14 de marzo - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00Hrs   
 
Hoy en día es imprescindible que los responsables de compras adquieran 
habilidades, técnicas y herramientas prácticas que les permitan profesionalizar 
sus negociaciones y relaciones con proveedores; además de garantizar acuerdos 
duraderos y mutuamente benéficos, podrán asegurar el abastecimiento, disminuir 
los costos y contribuir con el logro de los objetivos y la satisfacción de los 
intereses de su empresa. 

TEMARIO: 

1. Conceptos triunfadores en la negociación. 

2. El proceso de la negociación en compras. 

3. Estrategias y tácticas. 

4. La negociación en acción. 

- Y mucho más. 




 
¿Requiere la información a la Brevedad?
responda este email con la palabra: Compras.
Junto con los siguientes datos:
Nombre:
Teléfono:
Empresa:
centro telefónico: 018002129393


Lic. Arturo López
Líder de Proyecto


 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA..
 

 

 



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

[ovs-dev] [PATCH] ofproto: fix comment about ofproto-dpif

[William Tu]

"dpif-netlink" is the dpif to communicate with kernel.
So replace "dpif-linux" with "dpif-netlink".

Signed-off-by: William Tu 
---
 ofproto/ofproto-dpif.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 2433a548c6d0..90432fa2678b 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -19,7 +19,7 @@
  *
  * ofproto-dpif provides an ofproto implementation for those platforms which
  * implement the netdev and dpif interface defined in netdev.h and dpif.h.  The
- * most important of which is the Linux Kernel Module (dpif-linux), but
+ * most important of which is the Linux Kernel Module (dpif-netlink), but
  * alternatives are supported such as a userspace only implementation
  * (dpif-netdev), and a dummy implementation used for unit testing.
  *
-- 
2.7.4

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

Re: [ovs-dev] [PATCHv4] DNS: Add basic support for asynchronous DNS resolving

[Yifeng Sun]

Hi Ben,

Thanks a lot for your comment. I will check on the things you mentioned.

Best,
Yifeng

On Mon, Feb 26, 2018 at 2:59 PM, Ben Pfaff  wrote:

> On Mon, Jan 22, 2018 at 09:20:36AM -0800, Yifeng Sun wrote:
> > This patch is a simple implementation for the proposal discussed in
> > https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html
> and
> > https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/340013.html.
> >
> > It enables ovs-vswitchd and other utilities to use DNS names when
> specifying
> > OpenFlow and OVSDB remotes.
>
> Thanks a lot for figuring this out.  I have a few minor suggestions,
> which I'm appending in the form of a patch.  The additional #includes
> are there because the BSDs require them (and building with "sparse"
> reported this); the others are just style preferences.
>
> Another suggestion: I think that the documentation should mention, in
> the places where we document that hostnames can be used, that OVS has to
> be built with "unbound" to enable that.
>
> I think that we probably need to update .travis.yml to get the unbound
> library on travis.  Can you check for that?
>
> Does this patch count the TTL from the time resolution finishes, or from
> the time that resolution starts?  It wasn't obvious to me.  If it counts
> it from the time that resolution starts, then it could be that in the
> case of slow DNS resolution that we end up resolving DNS names forever
> and never actually using them.  Can you check on that?
>
> Thanks,
>
> Ben.
>
> --8<--cut here-->8--
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index b225b168fb0d..bd16f724baae 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -455,14 +455,11 @@ else
>  lib_libopenvswitch_la_SOURCES += lib/stream-nossl.c
>  endif
>
> +lib_libopenvswitch_la_SOURCES += lib/dns-resolve.h
>  if HAVE_UNBOUND
> -lib_libopenvswitch_la_SOURCES += \
> -   lib/dns-resolve.h \
> -   lib/dns-resolve.c
> +lib_libopenvswitch_la_SOURCES += lib/dns-resolve.c
>  else
> -lib_libopenvswitch_la_SOURCES += \
> -   lib/dns-resolve.h \
> -   lib/dns-resolve-stub.c
> +lib_libopenvswitch_la_SOURCES += lib/dns-resolve-stub.c
>  endif
>
>  pkgconfig_DATA += \
> diff --git a/lib/dns-resolve.c b/lib/dns-resolve.c
> index b94a5266c33d..391a8f48872b 100644
> --- a/lib/dns-resolve.c
> +++ b/lib/dns-resolve.c
> @@ -16,6 +16,8 @@
>
>  #include 
>  #include "dns-resolve.h"
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -91,7 +93,8 @@ dns_resolve_init(bool is_daemon)
>  /* Handles '/etc/hosts' on Linux and 'WINDIR/etc/hosts' on Windows. */
>  retval = ub_ctx_hosts(ub_ctx__, NULL);
>  if (retval != 0) {
> -VLOG_WARN_RL(, "Failed to read etc/hosts: %s",
> ub_strerror(retval));
> +VLOG_WARN_RL(, "Failed to read /etc/hosts: %s",
> + ub_strerror(retval));
>  }
>
>  ub_ctx_async(ub_ctx__, true);
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv4] DNS: Add basic support for asynchronous DNS resolving

[Ben Pfaff]

On Mon, Jan 22, 2018 at 09:20:36AM -0800, Yifeng Sun wrote:
> This patch is a simple implementation for the proposal discussed in   
>   
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html and
>   
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/340013.html.  
>   
>   
>   
> It enables ovs-vswitchd and other utilities to use DNS names when specifying  
>  
> OpenFlow and OVSDB remotes.   
>   

Thanks a lot for figuring this out.  I have a few minor suggestions,
which I'm appending in the form of a patch.  The additional #includes
are there because the BSDs require them (and building with "sparse"
reported this); the others are just style preferences.

Another suggestion: I think that the documentation should mention, in
the places where we document that hostnames can be used, that OVS has to
be built with "unbound" to enable that.

I think that we probably need to update .travis.yml to get the unbound
library on travis.  Can you check for that?

Does this patch count the TTL from the time resolution finishes, or from
the time that resolution starts?  It wasn't obvious to me.  If it counts
it from the time that resolution starts, then it could be that in the
case of slow DNS resolution that we end up resolving DNS names forever
and never actually using them.  Can you check on that?

Thanks,

Ben.

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

diff --git a/lib/automake.mk b/lib/automake.mk
index b225b168fb0d..bd16f724baae 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -455,14 +455,11 @@ else
 lib_libopenvswitch_la_SOURCES += lib/stream-nossl.c
 endif
 
+lib_libopenvswitch_la_SOURCES += lib/dns-resolve.h
 if HAVE_UNBOUND
-lib_libopenvswitch_la_SOURCES += \
-   lib/dns-resolve.h \
-   lib/dns-resolve.c
+lib_libopenvswitch_la_SOURCES += lib/dns-resolve.c
 else
-lib_libopenvswitch_la_SOURCES += \
-   lib/dns-resolve.h \
-   lib/dns-resolve-stub.c
+lib_libopenvswitch_la_SOURCES += lib/dns-resolve-stub.c
 endif
 
 pkgconfig_DATA += \
diff --git a/lib/dns-resolve.c b/lib/dns-resolve.c
index b94a5266c33d..391a8f48872b 100644
--- a/lib/dns-resolve.c
+++ b/lib/dns-resolve.c
@@ -16,6 +16,8 @@
 
 #include 
 #include "dns-resolve.h"
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -91,7 +93,8 @@ dns_resolve_init(bool is_daemon)
 /* Handles '/etc/hosts' on Linux and 'WINDIR/etc/hosts' on Windows. */
 retval = ub_ctx_hosts(ub_ctx__, NULL);
 if (retval != 0) {
-VLOG_WARN_RL(, "Failed to read etc/hosts: %s", ub_strerror(retval));
+VLOG_WARN_RL(, "Failed to read /etc/hosts: %s",
+ ub_strerror(retval));
 }
 
 ub_ctx_async(ub_ctx__, true);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.8] tunnel: Fix deletion of datapath tunnel ports in case of reconfiguration

[Ben Pfaff]

On Mon, Feb 26, 2018 at 09:39:47AM +, Balazs Nemeth wrote:
> There is an issue in OVS with tunnel deletion during the
> reconfiguration of OF tunnels. If the dst_port value is changed, the
> old tunnel map entry will not be deleted, because the tp_port
> argument of tnl_port_map_delete() has the new dst_port setting, hence
> the tunnel cannot be found in the list of tnl_port structures.
> 
> The patch corrects this mechanism by adding a new argument,
> 'old_odp_port' to tnl_port_reconfigure(). This value is used to
> identify the datapath tunnel port which is being reconfigured. In
> connection with this fix, to unify the tunnel port map handling,
> odp_port value is used to search the proper port to insert and delete
> tunnel map entries as well. This variable can be used instead of
> tp_port, as it is unique for all datapath tunnel ports, and there is
> no need to reach dst_port from netdev_tunnel_config structure.
> 
> This patch also adds a printout to check the reference counter of
> a tnl_port structure in tnl-port.c. Extending OVS unit test cases to
> have ref_cnt values in the expected dump. Adding new test cases to
> check if packet receiving is still working in the case of OF tunnel
> port deletion. Adding new test cases to check the reference counter
> in case of OF tunnel deletion or reconfiguration.
> 
> Signed-off-by: Balazs Nemeth 
> Signed-off-by: Jan Scheurich 
> Co-authored-by: Jan Scheurich 

Applied to branch-2.8, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.8] tests: Make packet-type-aware.at hash independent

[Ben Pfaff]

On Mon, Feb 26, 2018 at 09:17:57AM +, Balazs Nemeth wrote:
> When compiling with -msse4.2 a test case of packet-type-aware.at will
> fail due to the CRC32 based hash function is different from mhash.
> Fix this issue with parsing the port statistics one-by-one.
> 
> Signed-off-by: Balazs Nemeth 
> CC: Jan Scheurich 
> CC: Zoltan Balogh 
> Fixes: 0d87cb5b6757 ("xlate: fix xport lookup for recirc")

I applied this to branch-2.8.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Administrar Adecuadamente su Empresa

[Empresas Familiares]

 

 
Gestión de empresas familiares 
Marzo 02 - webinar Interactivo

Objetivo:

Descubrir cómo administrar adecuadamente su empresa a pesar y a través de la 
problemática del negocio familiar. 

Temas:

-  Conozca y aplique estrategias de institucionalización de la organización que 
se traducen en una mejor estructura a largo plazo.
- Analice las decisiones trascendentales que debe tomar el fundador.
- Estudie casos probados de éxito y su aplicación a su propio negocio.
- Aprenda a tomar decisiones antes que la vida las tome por usted.  
 
Temario e Inscripciones:

Respondiendo por este medio "Familiar"+TELÉFONO + NOMBRE o marcando al:

045 + 5515546630  



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

Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages

[Ben Pfaff]

On Mon, Feb 26, 2018 at 02:04:02PM -0600, Mark Michelson wrote:
> The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
> their "Database Commands" section when it comes to referring to what
> database tables exist. This commit amends this by making each *ctl
> manpage reference the corresponding database manpage instead.
> 
> To aid in having a more handy list, the --help text of ovn-nbctl,
> ovn-sbctl, and ovs-vsctl have been modified to list the available
> tables. This is also referenced in the manpages for those applications.
> 
> Signed-off-by: Mark Michelson 
> Signed-off-by: Ben Pfaff 

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

[ovs-dev] Reply

[Julie Leach]

Money was donated to you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] compat: Fix RHEL 7 build warnings

[Greg Rose]

A prior commit to fix up netdev_master_upper_dev_link for recent
kernels caused a compile warning on RHEL 7 builds.

Fixes: 36d3520b5f ("datapath: Fix netdev_master_upper_dev_link for 4.14")
Signed-off-by: Greg Rose 
---
 datapath/linux/compat/include/linux/netdevice.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/datapath/linux/compat/include/linux/netdevice.h 
b/datapath/linux/compat/include/linux/netdevice.h
index 052689f..9d3b249 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -116,6 +116,7 @@ static inline int rpl_netdev_master_upper_dev_link(struct 
net_device *dev,
return netdev_master_upper_dev_link(dev, upper_dev,
upper_priv, upper_info);
 }
+#undef netdev_master_upper_dev_link
 #define netdev_master_upper_dev_link rpl_netdev_master_upper_dev_link
 #endif /* #else HAVE_NETDEV_MASTER_UPPER_DEV_LINK_RH */
 #else  /* #ifndef HAVE_NETDEV_MASTER_UPPER_DEV_LINK_PRIV */
-- 
1.8.3.1

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

[ovs-dev] [PATCH 1/2] compat: Fix RHEL 7 compile

[Greg Rose]

frag_percpu_counter_batch is a variable, not a define, so checking if
it is defined is an error and causes warning messages during compile
on RHEL 7 (or other 3.10 based) builds.  Use a compat #define from
acinclude.m4 instead.

Fixes: 64d8cb7295 ("compat:inet_frag.h: Check for frag_percpu_counter_batch")
Signed-off-by: Greg Rose 
---
 acinclude.m4  | 4 
 datapath/linux/compat/include/net/inet_frag.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index b5f62cc..d61e37a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -811,6 +811,10 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/timekeeping.h],
   [ktime_get_ns],
   [OVS_DEFINE([HAVE_KTIME_GET_NS])])
+  OVS_GREP_IFELSE([$KSRC/include/net/inet_frag.h],
+  frag_percpu_counter_batch[],
+  [OVS_DEFINE([HAVE_FRAG_PERCPU_COUNTER_BATCH])])
+
 
   if cmp -s datapath/linux/kcompat.h.new \
 datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/linux/compat/include/net/inet_frag.h 
b/datapath/linux/compat/include/net/inet_frag.h
index 4090886..124c8be 100644
--- a/datapath/linux/compat/include/net/inet_frag.h
+++ b/datapath/linux/compat/include/net/inet_frag.h
@@ -30,7 +30,7 @@ static inline bool inet_frag_evicting(struct inet_frag_queue 
*q)
 #endif
 
 #ifndef HAVE_SUB_FRAG_MEM_LIMIT_ARG_STRUCT_NETNS_FRAGS
-#ifdef frag_percpu_counter_batch
+#ifdef HAVE_FRAG_PERCPU_COUNTER_BATCH
 static inline void rpl_sub_frag_mem_limit(struct netns_frags *nf, int i)
 {
__percpu_counter_add(>mem, -i, frag_percpu_counter_batch);
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCHv2 1/2] xlate: auto ofproto trace when recursion too deep

[William Tu]

On Wed, Feb 14, 2018 at 3:03 PM, Ben Pfaff  wrote:
> On Wed, Feb 14, 2018 at 10:56:08AM -0800, William Tu wrote:
>> Usually ofproto/trace is used to debug the flow translation error.
>> When translation error such as recursion too deep or too many resubmit,
>> the issue might happen momentary; flows causing the recursion expire
>> when users try to debug it.  This patch enables the ofproto trace
>> automatically when recursion is too deep or too many resubmit, by
>> invoking the translation again, and log the ofproto trace as warnings.
>> Since the log will be huge, rate limit to one per minute.
>>
>> VMWare-BZ: #2054659
>> Signed-off-by: William Tu 
>> Tested-by: Greg Rose 
>> Reviewed-by: Greg Rose 
>
> Thanks for working on this!
>
> Some of the data passed to ofproto_trace() is also passed to the
> xlate_actions() call, indirectly.  Did you check whether that data is
> possibly modified by xlate_actions()?  If it is, then we might have to
> reconsider this approach, because flow data, etc. is very large and I
> don't think that we can afford to always make a copy of it in advance on
> the chance that the original might be needed for tracing later.

We pass the "const struct flow *" and "const struct dp_packet *" to
the ofproto_trace(),
so I think these two data is unmodified and xlate_in_init() actually make a copy
of the "struct flow", so xlate_actions() modifies a separate copy of the flow.

>
> I think that VLOG_WARN is a very high log level for this data.  I would
> tend to use DBG.
>
I worried that most of the time DBG is not on, and asking people
to turn it on and reproduce the issue is hard.  Can we keep VLOG_WARN and
rate limit to very low frequent, "VLOG_RATE_LIMIT_INIT(1, 5);",?

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

Re: [ovs-dev] [PATCH] ofproto: Make ofproto_port_open_type() faster.

[aginwala]

Sweet!

On Mon, Feb 26, 2018 at 11:42 AM, Ben Pfaff  wrote:

> Thanks a lot for the testing!  I added your test results to the commit
> message and applied this to master.
>
> On Sat, Feb 24, 2018 at 04:07:23PM +, aginwala wrote:
> > Hi :
> >
> > The results are super awesome!  Performance have drastically improved by
> > ~60%. The test for 10k lports, 40 LSs and 8 LRs and 1k HVs just got
> > completed in 3 hours 39 min vs 8+ hours for branch-2.9. Cpu utilization
> > graph of a farm comparing Ben's ofproto patch vs branch-2.9 is available
> @
> > https://raw.githubusercontent.com/noah8713/ovn-scale-test/
> scale_results/results/ovs_2.9_vs_ben_ofproto.png
> >
> >
> > Thanks a ton Ben for the new patch. I give it a +1 for merge untill
> someone
> > is suggesting any other improvements/comments.
> >
> >
> > On Fri, Feb 23, 2018 at 3:32 PM, aginwala  wrote:
> >
> > > This is great! I will re-run the test with this patch and send over the
> > > results soon!
> > >
> > > On Fri, Feb 23, 2018 at 3:25 PM, Han Zhou  wrote:
> > >
> > >> On Fri, Feb 23, 2018 at 2:03 PM, Ben Pfaff  wrote:
> > >> >
> > >> > ofproto_port_open_type() was surprisingly slow because it called the
> > >> > function ofproto_class_find__(), which itself was surprisingly slow
> > >> because
> > >> > it actually creates a set of strings and enumerates all of the
> available
> > >> > classes.
> > >> >
> > >> > This patch improves performance by eliminating the call to
> > >> > ofproto_class_find__() from ofproto_port_open_type().  In turn that
> > >> > required changing a parameter type and updating all the callers.
> > >> >
> > >> > Possibly it would be worth making ofproto_class_find__() itself
> faster,
> > >> > but it doesn't look like any of its other callers would be used in
> inner
> > >> > loops.
> > >> >
> > >> > A different patch that was also meant to speed this up reported the
> > >> > following performance improvements.  That patch just eliminated
> half of
> > >> the
> > >> > calls to ofproto_class_find__() in inner loops, whereas this one
> > >> eliminates
> > >> > all of them and should therefore perform even better.
> > >> >
> > >> > This patch arises as a result of testing done by Ali Ginwala
> and Han
> > >> > Zhou. Their test showed that commit 2d4beba resulted in slower
> > >> > performance of ovs-vswitchd than was seen in previous versions
> of
> > >> OVS.
> > >> >
> > >> > With this patch, Ali retested and reported that this patch had
> > >> nearly
> > >> > the same effect on performance as reverting 2d4beba.
> > >> >
> > >> > The test was to create 1 logical switch ports using 20 farm
> > >> nodes,
> > >> > each running 50 HV sandboxes. "Base" in the tests below is OVS
> > >> master
> > >> > with Han Zhou's ovn-controller incremental processing patch
> applied.
> > >> >
> > >> > base: Test completed in 8 hours
> > >> > base with 2d4beba reverted: Test completed in 5 hours 28 minutes
> > >> > base with this patch: Test completed in 5 hours 30 minutes
> > >> >
> > >> > Reported-by: Mark Michelson 
> > >> > Signed-off-by: Ben Pfaff 
> > >> > ---
> > >> >  ofproto/in-band.c |  2 +-
> > >> >  ofproto/ofproto.c | 30 ++
> > >> >  ofproto/ofproto.h |  2 +-
> > >> >  vswitchd/bridge.c | 12 
> > >> >  4 files changed, 16 insertions(+), 30 deletions(-)
> > >> >
> > >> > diff --git a/ofproto/in-band.c b/ofproto/in-band.c
> > >> > index 849b1cedaff0..82d8dfa14774 100644
> > >> > --- a/ofproto/in-band.c
> > >> > +++ b/ofproto/in-band.c
> > >> > @@ -428,7 +428,7 @@ in_band_create(struct ofproto *ofproto, const
> char
> > >> *local_name,
> > >> >  struct in_band *in_band;
> > >> >  struct netdev *local_netdev;
> > >> >  int error;
> > >> > -const char *type = ofproto_port_open_type(ofproto->type,
> > >> "internal");
> > >> > +const char *type = ofproto_port_open_type(ofproto,
> "internal");
> > >> >
> > >> >  *in_bandp = NULL;
> > >> >  error = netdev_open(local_name, type, _netdev);
> > >> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > >> > index f28bb896eee9..a982de9d8db4 100644
> > >> > --- a/ofproto/ofproto.c
> > >> > +++ b/ofproto/ofproto.c
> > >> > @@ -1966,27 +1966,18 @@ ofproto_port_dump_done(struct
> ofproto_port_dump
> > >> *dump)
> > >> >  return dump->error == EOF ? 0 : dump->error;
> > >> >  }
> > >> >
> > >> > -/* Returns the type to pass to netdev_open() when a datapath of
> type
> > >> > - * 'datapath_type' has a port of type 'port_type', for a few
> special
> > >> > - * cases when a netdev type differs from a port type.  For example,
> > >> when
> > >> > - * using the userspace datapath, a port of type "internal" needs
> to be
> > >> > - * opened as "tap".
> > >> > +/* Returns the type to pass to netdev_open() when 'ofproto' has a
> port
> > >> of type
> > >> > + * 'port_type', for a few 

Re: [ovs-dev] [PATCH] Use the correct logical datapath UUID as input for logical flow hash.

[Ben Pfaff]

On Mon, Feb 26, 2018 at 11:47:08AM +0100, Jakub Sitnicki wrote:
> On Fri, Feb 23, 2018 at 09:05 PM GMT, Ben Pfaff wrote:
> > On Thu, Feb 22, 2018 at 10:54:05PM +0100, Jakub Sitnicki wrote:
> >> Use the logical switch/router UUID as hash input, instead of the UUID of
> >> the Datapath_Binding row, when calculating the hash value for lflows in
> >> the SBDB.
> >>
> >> Otherwise the hash value will never match the one computed from NBDB
> >> contents, which will force ovn-northd to constantly drop and attempt to
> >> re-insert all the lflows.
> >
> > Thanks a lot for analyzing the problem and fixing it.
> >
> > There are basically two options here: use Datapath_Binding UUID
> > consistently or use Logical_Switch/Logical_Router UUID consistently.
> > This patch takes the latter approach.  I'd prefer to take the former
> > approach because it is less dependent on the structure of the northbound
> > database (which seems desirable for something that is in the southbound
> > database) and because it doesn't rely on the external-ids being correct.
> >
> > I sent a commit that takes the Datapath_Binding UUID approach:
> > https://patchwork.ozlabs.org/patch/877306/
> >
> > What do you think?
> 
> In general - works for me. I'm wondering about a couple of things:
> 
> 1) build_datapaths() -> join_datapaths() detects and removes duplicated
>datapath bindings, AFAIU. I'm not sure how we can end up with
>duplicated bindings but if it happens, can it lead to removal and
>re-insertion of logical flows for a datapath?

I guess it would, if there were duplicate datpath bindings.  It's
ovn-northd that actually creates the datapath bindings, though, so we
would want to fix the problem in ovn-northd in that case.

> 2) If I wanted to precalculate an lflow hash and cache it in a synthetic
>column in NB DB, ACLs being the potential candidate for that, then it
>won't be possible with this approach. That said, I don't know ATM if
>this would give a significant gain.

If this turns out to be important to performance, I'm more than willing
to reconsider the approach.

For now, I applied my patch to master.  Thanks a lot!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Tagging OVS 2.7.4

[Justin Pettit]

Hi, Miguel Angel.  I pushed a release to the website, and Ben tagged "v2.7.4".  
Please let us know if you need anything else.

--Justin


> On Feb 21, 2018, at 2:44 AM, Miguel Angel Ajo Pelayo  
> wrote:
> 
> Hi, I have realized that deployment of openstack with ml2/networking-ovn
> fails on RDO
> due to the lack of some patches in the 2.7.x branch. It's trying to set the
> inactivity probe
> of the pacemaker OCF scripts, but that parameter doesn't exist in 2.7.3, it
> does in a
> patch that Numan introduced and that it's backported on 2.7.
> 
> Ben/anyone. Can we please get a tag for 2.7.4? That'd get us a rebuild of
> the package
> in fedora / Centos CBS / RDO.
> 
> Best regards,
> Miguel Angel.
> 
> On Wed, Jan 31, 2018 at 2:15 PM Miguel Angel Ajo Pelayo 
> wrote:
> 
>> Does it make sense tagging ovs 2.7.4 stable?.
>> 
>> There are several patches which are important for ovn/ networking-ovn
>> since the release
>> of ovs 2.7.3. And I'd be glad to have them available on the upstream
>> builds, so people
>> can start doing more serious testing over RDO/openstack/networking-ovn .
>> 
>> Best regards,
>> Miguel Ángel
>> 
> ___
> 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] [branch-2.7 1/2] Set release date for 2.7.4.

[Ben Pfaff]

On Mon, Feb 26, 2018 at 11:57:17AM -0800, Justin Pettit wrote:
> 
> > On Feb 26, 2018, at 11:51 AM, Ben Pfaff  wrote:
> > 
> > On Sat, Feb 24, 2018 at 11:08:38AM -0800, Justin Pettit wrote:
> >> Signed-off-by: Justin Pettit 
> > 
> > Acked-by: Ben Pfaff 
> 
> Can you tag commit 6542e817601 for this patch as "v2.7.4"?

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

Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages

[Mark Michelson]

On 02/26/2018 01:54 PM, Ben Pfaff wrote:

On Mon, Feb 26, 2018 at 08:30:45AM -0600, Mark Michelson wrote:

On 02/23/2018 06:07 PM, Ben Pfaff wrote:

On Thu, Feb 15, 2018 at 04:49:57PM -0600, Mark Michelson wrote:

The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
their "Database Commands" section when it comes to referring to what
database tables exist. This commit amends this by making each *ctl
manpage reference the corresponding database manpage instead.

To aid in having a more handy list, the --help text of ovn-nbctl,
ovn-sbctl, and ovs-vsctl have been modified to list the available
tables. This is also referenced in the manpages for those applications.

Signed-off-by: Mark Michelson 


Thanks.  I was hoping for more explanation in the list of tables for how
users can refer to the tables.  Here is a version that adds more
information.  What do you think?


Thanks for the update Ben. I like listing methods of referring to records in
each table. I found the formatting to be a bit overwhelming in some cases.
For instance, from `ovn-nbctl -h` the DHCP_Options table has this text:

   DHCP_Options: by UUID, via "dhcpv4_options" of Logical_Switch_Port with
matching "name", via "dhcpv4_options" of Logical_Switch_Port with matching
"external_ids:neutron:port_name", via "dhcpv6_options" of
Logical_Switch_Port with matching "name", or via "dhcpv6_options" of
Logical_Switch_Port with matching "external_ids:neutron:port_name"

Keep in mind that there is no column-limiting on this, so it stretches
across the entire terminal. It makes it hard to visually parse the options.
I made a adjustment to the formatting so that it looks more like this:

   DHCP_Options:
 by UUID
 via "dhcpv4_options" of Logical_Switch_Port with matching "name"
 via "dhcpv4_options" of Logical_Switch_Port with matching
"external_ids:neutron:port_name"
 via "dhcpv6_options" of Logical_Switch_Port with matching "name"
 via "dhcpv6_options" of Logical_Switch_Port with matching
"external_ids:neutron:port_name"

Separating the methods by line makes it easier to read IMHO. Here's the
change


Thanks.  This came through word-wrapped, though.  Can you re-send?

Thanks,

Ben.



Sorry about that. I sent out a v2 of the patch: 
https://patchwork.ozlabs.org/patch/878063/


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

[ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages

[Mark Michelson]

The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
their "Database Commands" section when it comes to referring to what
database tables exist. This commit amends this by making each *ctl
manpage reference the corresponding database manpage instead.

To aid in having a more handy list, the --help text of ovn-nbctl,
ovn-sbctl, and ovs-vsctl have been modified to list the available
tables. This is also referenced in the manpages for those applications.

Signed-off-by: Mark Michelson 
Signed-off-by: Ben Pfaff 
Please enter the commit message for your changes. Lines starting
---
 lib/db-ctl-base.c | 69 ---
 lib/db-ctl-base.h | 14 -
 ovn/lib/ovn-util.h|  5 
 ovn/utilities/ovn-nbctl.8.xml | 61 +++---
 ovn/utilities/ovn-nbctl.c |  5 ++--
 ovn/utilities/ovn-sbctl.8.in  |  5 ++--
 ovn/utilities/ovn-sbctl.c |  6 ++--
 utilities/ovs-vsctl.8.in  | 51 ++--
 utilities/ovs-vsctl.c |  7 +++--
 vtep/vtep-ctl.c   |  7 +++--
 10 files changed, 103 insertions(+), 127 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 9fec6fa0d..ae7b6cde2 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -35,6 +35,7 @@
 #include "ovsdb-idl-provider.h"
 #include "openvswitch/shash.h"
 #include "sset.h"
+#include "svec.h"
 #include "string.h"
 #include "table.h"
 #include "util.h"
@@ -61,6 +62,9 @@ static const struct cmd_show_table *cmd_show_tables;
 static void (*ctl_exit_func)(int status) = NULL;
 OVS_NO_RETURN static void ctl_exit(int status);
 
+/* IDL class. */
+static const struct ovsdb_idl_class *idl_class;
+
 /* Two arrays with 'n_classes' elements, which represent the tables in this
  * database and how the user can refer to their rows. */
 static const struct ctl_table_class *ctl_classes;
@@ -2132,15 +2136,15 @@ ctl_register_commands(const struct ctl_command_syntax 
*commands)
 
 /* Registers the 'db_ctl_commands' to 'all_commands'. */
 void
-ctl_init__(const struct ovsdb_idl_table_class *idl_classes_,
+ctl_init__(const struct ovsdb_idl_class *idl_class_,
const struct ctl_table_class *ctl_classes_,
-   size_t n_classes_,
const struct cmd_show_table cmd_show_tables_[],
void (*ctl_exit_func_)(int status))
 {
-idl_classes = idl_classes_;
+idl_class = idl_class_;
+idl_classes = idl_class_->tables;
 ctl_classes = ctl_classes_;
-n_classes = n_classes_;
+n_classes = idl_class->n_tables;
 ctl_exit_func = ctl_exit_func_;
 ctl_register_commands(db_ctl_commands);
 
@@ -2170,6 +2174,63 @@ ctl_get_db_cmd_usage(void)
 Potentially unsafe database commands require --force option.\n";
 }
 
+const char *
+ctl_list_db_tables_usage(void)
+{
+static struct ds s = DS_EMPTY_INITIALIZER;
+if (s.length) {
+return ds_cstr();
+}
+
+ds_put_cstr(, "Database commands may reference a row in each table in 
the following ways:\n");
+for (int i = 0; i < n_classes; i++) {
+struct svec options = SVEC_EMPTY_INITIALIZER;
+
+svec_add(, "by UUID");
+if (idl_classes[i].is_singleton) {
+svec_add(, "as \".\"");
+}
+
+for (int j = 0; j < ARRAY_SIZE(ctl_classes[i].row_ids); j++) {
+const struct ctl_row_id *id = _classes[i].row_ids[j];
+if (!id->name_column) {
+continue;
+}
+
+struct ds o = DS_EMPTY_INITIALIZER;
+if (id->uuid_column) {
+ds_put_format(, "via \"%s\"", id->uuid_column->name);
+const struct ovsdb_idl_table_class *referrer
+= ovsdb_idl_table_class_from_column(idl_class,
+id->uuid_column);
+if (referrer != _classes[i]) {
+ds_put_format(, " of %s", referrer->name);
+}
+if (id->key) {
+ds_put_format(, " with matching \"%s:%s\"",
+  id->name_column->name, id->key);
+} else {
+ds_put_format(, " with matching \"%s\"", 
id->name_column->name);
+}
+} else if (id->key) {
+ds_put_format(, "by \"%s:%s\"", id->name_column->name, 
id->key);
+} else {
+ds_put_format(, "by \"%s\"", id->name_column->name);
+}
+svec_add_nocopy(, ds_steal_cstr());
+}
+
+ds_put_format(, "  %s:", idl_classes[i].name);
+for (int j = 0; j < options.n; j++) {
+ds_put_format(, "\n%s", options.names[j]);
+}
+ds_put_char(, '\n');
+svec_destroy();
+}
+
+return ds_cstr();
+}
+
 /* Initializes 'ctx' from 'command'. */
 void
 ctl_context_init_command(struct ctl_context *ctx,
diff --git a/lib/db-ctl-base.h 

Re: [ovs-dev] [PATCH v2] python: Fix decoding error when the received data is larger than 4096.

[Ben Pfaff]

On Mon, Feb 26, 2018 at 02:02:05PM +0800, Guoshuai Li wrote:
> >On Sun, Feb 18, 2018 at 08:51:16PM +0800, Guoshuai Li wrote:
> >>It can only receive 4096 bytes of data each time in jsonrpc,
> >>when there are similar and Chinese characters occupy multiple bytes,
> >>it may receive half a character, this time the decoding will be abnormal.
> >>We need to receive the completed character to decode.
> >>
> >>Signed-off-by: Guoshuai Li 
> >Thanks for finding and fixing the problem.
> >
> >I am worried that, with this patch, any genuine encoding errors will be
> >ignored.  For example, bytes 0xc0 and 0xc1 never appear in a valid UTF-8
> >sequence, yet I believe that this code will silently ignore them, along
> >with whatever other data appears along with them.  Do you have an idea
> >for how to deal with that?
> >
> >Thanks,
> >
> >Ben.
> 
> Hello Ben
> 
> I sent a new version:
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344705.html
> But I am not satisfied with it.
> This patch can not solve the received data length is exactly a multiple of
> 4096, and with 0xc0 and 0xc1.
> I do not know how to judge that I have received all the data.
> I think I need more people's help.

It looks like we should be using IncrementalDecoder:
https://docs.python.org/3/library/codecs.html?highlight=decode#codecs.IncrementalDecoder
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ofproto-dpif-upcall: Fix using uninitialized fitness.

[Ben Pfaff]

On Mon, Feb 26, 2018 at 11:10:11AM +0300, Ilya Maximets wrote:
> 'upcall_xlate()' makes a decision to compose slow path actions
> by checking the 'upcall->fitness', which is not initialized in
> case of calling from the 'upcall_cb()'.
> 
> 'upcall_cb()' receives the real flow, so the fitness should be
> initialized as perfect.
> 
> Fixes following tests on travis:
> 
> ofproto-dpif.at: ofproto-dpif megaflow - disabled - pmd
> ofproto-dpif.at: ofproto-dpif - conntrack - output action
> 
> CC: Ben Pfaff 
> Fixes: 687bafbb8a79 ("ofproto-dpif-upcall: Slow path flows that
>   datapath can't fully match.")
> Signed-off-by: Ilya Maximets 

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

Re: [ovs-dev] [branch-2.7 2/2] Prepare for 2.7.5.

[Justin Pettit]

> On Feb 26, 2018, at 11:52 AM, Ben Pfaff  wrote:
> 
> On Sat, Feb 24, 2018 at 11:08:39AM -0800, Justin Pettit wrote:
>> Signed-off-by: Justin Pettit 
> 
> Acked-by: Ben Pfaff 

Thanks.  I pushed this series to branch-2.7.

--Justin


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

Re: [ovs-dev] [branch-2.7 1/2] Set release date for 2.7.4.

[Justin Pettit]

> On Feb 26, 2018, at 11:51 AM, Ben Pfaff  wrote:
> 
> On Sat, Feb 24, 2018 at 11:08:38AM -0800, Justin Pettit wrote:
>> Signed-off-by: Justin Pettit 
> 
> Acked-by: Ben Pfaff 

Can you tag commit 6542e817601 for this patch as "v2.7.4"?

Thanks!

--Justin


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

Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages

[Ben Pfaff]

On Mon, Feb 26, 2018 at 08:30:45AM -0600, Mark Michelson wrote:
> On 02/23/2018 06:07 PM, Ben Pfaff wrote:
> >On Thu, Feb 15, 2018 at 04:49:57PM -0600, Mark Michelson wrote:
> >>The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
> >>their "Database Commands" section when it comes to referring to what
> >>database tables exist. This commit amends this by making each *ctl
> >>manpage reference the corresponding database manpage instead.
> >>
> >>To aid in having a more handy list, the --help text of ovn-nbctl,
> >>ovn-sbctl, and ovs-vsctl have been modified to list the available
> >>tables. This is also referenced in the manpages for those applications.
> >>
> >>Signed-off-by: Mark Michelson 
> >
> >Thanks.  I was hoping for more explanation in the list of tables for how
> >users can refer to the tables.  Here is a version that adds more
> >information.  What do you think?
> 
> Thanks for the update Ben. I like listing methods of referring to records in
> each table. I found the formatting to be a bit overwhelming in some cases.
> For instance, from `ovn-nbctl -h` the DHCP_Options table has this text:
> 
>   DHCP_Options: by UUID, via "dhcpv4_options" of Logical_Switch_Port with
> matching "name", via "dhcpv4_options" of Logical_Switch_Port with matching
> "external_ids:neutron:port_name", via "dhcpv6_options" of
> Logical_Switch_Port with matching "name", or via "dhcpv6_options" of
> Logical_Switch_Port with matching "external_ids:neutron:port_name"
> 
> Keep in mind that there is no column-limiting on this, so it stretches
> across the entire terminal. It makes it hard to visually parse the options.
> I made a adjustment to the formatting so that it looks more like this:
> 
>   DHCP_Options:
> by UUID
> via "dhcpv4_options" of Logical_Switch_Port with matching "name"
> via "dhcpv4_options" of Logical_Switch_Port with matching
> "external_ids:neutron:port_name"
> via "dhcpv6_options" of Logical_Switch_Port with matching "name"
> via "dhcpv6_options" of Logical_Switch_Port with matching
> "external_ids:neutron:port_name"
> 
> Separating the methods by line makes it easier to read IMHO. Here's the
> change

Thanks.  This came through word-wrapped, though.  Can you re-send?

Thanks,

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

Re: [ovs-dev] [PATCH] vlog: fix the incorrect zero padding in format_log_message

[Ben Pfaff]

Thanks Mark and zhangliping.  I applied this to master.

On Mon, Feb 26, 2018 at 09:08:53AM -0600, Mark Michelson wrote:
> Acked-by: Mark Michelson 
> Tested-by: Mark Michelson 
> 
> On 02/23/2018 09:30 PM, zhangliping wrote:
> >From: zhangliping 
> >
> >If the format specifier does not have the 0 flag, we should pad with
> >blanks instead of zeroes.
> >
> >Signed-off-by: zhangliping 
> >---
> >  lib/vlog.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/vlog.c b/lib/vlog.c
> >index f28695043..0e862a773 100644
> >--- a/lib/vlog.c
> >+++ b/lib/vlog.c
> >@@ -952,7 +952,7 @@ format_log_message(const struct vlog_module *module, 
> >enum vlog_level level,
> >  for (p = pattern; *p != '\0'; ) {
> >  const char *subprogram_name;
> >  enum { LEFT, RIGHT } justify = RIGHT;
> >-int pad = '0';
> >+int pad = ' ';
> >  size_t length, field, used;
> >  if (*p != '%') {
> >
> 
> ___
> 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] [branch-2.7 2/2] Prepare for 2.7.5.

[Ben Pfaff]

On Sat, Feb 24, 2018 at 11:08:39AM -0800, Justin Pettit wrote:
> Signed-off-by: Justin Pettit 

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

Re: [ovs-dev] [branch-2.7 1/2] Set release date for 2.7.4.

[Ben Pfaff]

On Sat, Feb 24, 2018 at 11:08:38AM -0800, Justin Pettit wrote:
> Signed-off-by: Justin Pettit 

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

Re: [ovs-dev] [PATCH] ofproto: Make ofproto_port_open_type() faster.

[Ben Pfaff]

Thanks a lot for the testing!  I added your test results to the commit
message and applied this to master.

On Sat, Feb 24, 2018 at 04:07:23PM +, aginwala wrote:
> Hi :
> 
> The results are super awesome!  Performance have drastically improved by
> ~60%. The test for 10k lports, 40 LSs and 8 LRs and 1k HVs just got
> completed in 3 hours 39 min vs 8+ hours for branch-2.9. Cpu utilization
> graph of a farm comparing Ben's ofproto patch vs branch-2.9 is available @
> https://raw.githubusercontent.com/noah8713/ovn-scale-test/scale_results/results/ovs_2.9_vs_ben_ofproto.png
> 
> 
> Thanks a ton Ben for the new patch. I give it a +1 for merge untill someone
> is suggesting any other improvements/comments.
> 
> 
> On Fri, Feb 23, 2018 at 3:32 PM, aginwala  wrote:
> 
> > This is great! I will re-run the test with this patch and send over the
> > results soon!
> >
> > On Fri, Feb 23, 2018 at 3:25 PM, Han Zhou  wrote:
> >
> >> On Fri, Feb 23, 2018 at 2:03 PM, Ben Pfaff  wrote:
> >> >
> >> > ofproto_port_open_type() was surprisingly slow because it called the
> >> > function ofproto_class_find__(), which itself was surprisingly slow
> >> because
> >> > it actually creates a set of strings and enumerates all of the available
> >> > classes.
> >> >
> >> > This patch improves performance by eliminating the call to
> >> > ofproto_class_find__() from ofproto_port_open_type().  In turn that
> >> > required changing a parameter type and updating all the callers.
> >> >
> >> > Possibly it would be worth making ofproto_class_find__() itself faster,
> >> > but it doesn't look like any of its other callers would be used in inner
> >> > loops.
> >> >
> >> > A different patch that was also meant to speed this up reported the
> >> > following performance improvements.  That patch just eliminated half of
> >> the
> >> > calls to ofproto_class_find__() in inner loops, whereas this one
> >> eliminates
> >> > all of them and should therefore perform even better.
> >> >
> >> > This patch arises as a result of testing done by Ali Ginwala and Han
> >> > Zhou. Their test showed that commit 2d4beba resulted in slower
> >> > performance of ovs-vswitchd than was seen in previous versions of
> >> OVS.
> >> >
> >> > With this patch, Ali retested and reported that this patch had
> >> nearly
> >> > the same effect on performance as reverting 2d4beba.
> >> >
> >> > The test was to create 1 logical switch ports using 20 farm
> >> nodes,
> >> > each running 50 HV sandboxes. "Base" in the tests below is OVS
> >> master
> >> > with Han Zhou's ovn-controller incremental processing patch applied.
> >> >
> >> > base: Test completed in 8 hours
> >> > base with 2d4beba reverted: Test completed in 5 hours 28 minutes
> >> > base with this patch: Test completed in 5 hours 30 minutes
> >> >
> >> > Reported-by: Mark Michelson 
> >> > Signed-off-by: Ben Pfaff 
> >> > ---
> >> >  ofproto/in-band.c |  2 +-
> >> >  ofproto/ofproto.c | 30 ++
> >> >  ofproto/ofproto.h |  2 +-
> >> >  vswitchd/bridge.c | 12 
> >> >  4 files changed, 16 insertions(+), 30 deletions(-)
> >> >
> >> > diff --git a/ofproto/in-band.c b/ofproto/in-band.c
> >> > index 849b1cedaff0..82d8dfa14774 100644
> >> > --- a/ofproto/in-band.c
> >> > +++ b/ofproto/in-band.c
> >> > @@ -428,7 +428,7 @@ in_band_create(struct ofproto *ofproto, const char
> >> *local_name,
> >> >  struct in_band *in_band;
> >> >  struct netdev *local_netdev;
> >> >  int error;
> >> > -const char *type = ofproto_port_open_type(ofproto->type,
> >> "internal");
> >> > +const char *type = ofproto_port_open_type(ofproto, "internal");
> >> >
> >> >  *in_bandp = NULL;
> >> >  error = netdev_open(local_name, type, _netdev);
> >> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> >> > index f28bb896eee9..a982de9d8db4 100644
> >> > --- a/ofproto/ofproto.c
> >> > +++ b/ofproto/ofproto.c
> >> > @@ -1966,27 +1966,18 @@ ofproto_port_dump_done(struct ofproto_port_dump
> >> *dump)
> >> >  return dump->error == EOF ? 0 : dump->error;
> >> >  }
> >> >
> >> > -/* Returns the type to pass to netdev_open() when a datapath of type
> >> > - * 'datapath_type' has a port of type 'port_type', for a few special
> >> > - * cases when a netdev type differs from a port type.  For example,
> >> when
> >> > - * using the userspace datapath, a port of type "internal" needs to be
> >> > - * opened as "tap".
> >> > +/* Returns the type to pass to netdev_open() when 'ofproto' has a port
> >> of type
> >> > + * 'port_type', for a few special cases when a netdev type differs from
> >> a port
> >> > + * type.  For example, when using the userspace datapath, a port of
> >> type
> >> > + * "internal" needs to be opened as "tap".
> >> >   *
> >> >   * Returns either 'type' itself or a string literal, which must not be
> >> >   * freed. */
> >> >  const 

Re: [ovs-dev] [PATCH] ofproto: Make ofproto_port_open_type() faster.

[Ben Pfaff]

Thanks for the review, I applied this to master.

On Mon, Feb 26, 2018 at 09:22:59AM -0600, Mark Michelson wrote:
> Yes this is much better than the patch I submitted. Thank you very much Ben.
> 
> Acked-by: Mark Michelson 
> 
> On 02/23/2018 04:03 PM, Ben Pfaff wrote:
> >ofproto_port_open_type() was surprisingly slow because it called the
> >function ofproto_class_find__(), which itself was surprisingly slow because
> >it actually creates a set of strings and enumerates all of the available
> >classes.
> >
> >This patch improves performance by eliminating the call to
> >ofproto_class_find__() from ofproto_port_open_type().  In turn that
> >required changing a parameter type and updating all the callers.
> >
> >Possibly it would be worth making ofproto_class_find__() itself faster,
> >but it doesn't look like any of its other callers would be used in inner
> >loops.
> >
> >A different patch that was also meant to speed this up reported the
> >following performance improvements.  That patch just eliminated half of the
> >calls to ofproto_class_find__() in inner loops, whereas this one eliminates
> >all of them and should therefore perform even better.
> >
> > This patch arises as a result of testing done by Ali Ginwala and Han
> > Zhou. Their test showed that commit 2d4beba resulted in slower
> > performance of ovs-vswitchd than was seen in previous versions of OVS.
> >
> > With this patch, Ali retested and reported that this patch had nearly
> > the same effect on performance as reverting 2d4beba.
> >
> > The test was to create 1 logical switch ports using 20 farm nodes,
> > each running 50 HV sandboxes. "Base" in the tests below is OVS master
> > with Han Zhou's ovn-controller incremental processing patch applied.
> >
> > base: Test completed in 8 hours
> > base with 2d4beba reverted: Test completed in 5 hours 28 minutes
> > base with this patch: Test completed in 5 hours 30 minutes
> >
> >Reported-by: Mark Michelson 
> >Signed-off-by: Ben Pfaff 
> >---
> >  ofproto/in-band.c |  2 +-
> >  ofproto/ofproto.c | 30 ++
> >  ofproto/ofproto.h |  2 +-
> >  vswitchd/bridge.c | 12 
> >  4 files changed, 16 insertions(+), 30 deletions(-)
> >
> >diff --git a/ofproto/in-band.c b/ofproto/in-band.c
> >index 849b1cedaff0..82d8dfa14774 100644
> >--- a/ofproto/in-band.c
> >+++ b/ofproto/in-band.c
> >@@ -428,7 +428,7 @@ in_band_create(struct ofproto *ofproto, const char 
> >*local_name,
> >  struct in_band *in_band;
> >  struct netdev *local_netdev;
> >  int error;
> >-const char *type = ofproto_port_open_type(ofproto->type, "internal");
> >+const char *type = ofproto_port_open_type(ofproto, "internal");
> >  *in_bandp = NULL;
> >  error = netdev_open(local_name, type, _netdev);
> >diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> >index f28bb896eee9..a982de9d8db4 100644
> >--- a/ofproto/ofproto.c
> >+++ b/ofproto/ofproto.c
> >@@ -1966,27 +1966,18 @@ ofproto_port_dump_done(struct ofproto_port_dump 
> >*dump)
> >  return dump->error == EOF ? 0 : dump->error;
> >  }
> >-/* Returns the type to pass to netdev_open() when a datapath of type
> >- * 'datapath_type' has a port of type 'port_type', for a few special
> >- * cases when a netdev type differs from a port type.  For example, when
> >- * using the userspace datapath, a port of type "internal" needs to be
> >- * opened as "tap".
> >+/* Returns the type to pass to netdev_open() when 'ofproto' has a port of 
> >type
> >+ * 'port_type', for a few special cases when a netdev type differs from a 
> >port
> >+ * type.  For example, when using the userspace datapath, a port of type
> >+ * "internal" needs to be opened as "tap".
> >   *
> >   * Returns either 'type' itself or a string literal, which must not be
> >   * freed. */
> >  const char *
> >-ofproto_port_open_type(const char *datapath_type, const char *port_type)
> >+ofproto_port_open_type(const struct ofproto *ofproto, const char *port_type)
> >  {
> >-const struct ofproto_class *class;
> >-
> >-datapath_type = ofproto_normalize_type(datapath_type);
> >-class = ofproto_class_find__(datapath_type);
> >-if (!class) {
> >-return port_type;
> >-}
> >-
> >-return (class->port_open_type
> >-? class->port_open_type(datapath_type, port_type)
> >+return (ofproto->ofproto_class->port_open_type
> >+? ofproto->ofproto_class->port_open_type(ofproto->type, 
> >port_type)
> >  : port_type);
> >  }
> >@@ -2728,10 +2719,9 @@ init_ports(struct ofproto *p)
> >  static bool
> >  ofport_is_internal_or_patch(const struct ofproto *p, const struct ofport 
> > *port)
> >  {
> >-return !strcmp(netdev_get_type(port->netdev),
> >-   ofproto_port_open_type(p->type, "internal")) ||
> >-   !strcmp(netdev_get_type(port->netdev),
> >-   

Re: [ovs-dev] [PATCH] ofproto: Make ofproto_port_open_type() faster.

[Ben Pfaff]

On Fri, Feb 23, 2018 at 03:25:40PM -0800, Han Zhou wrote:
> On Fri, Feb 23, 2018 at 2:03 PM, Ben Pfaff  wrote:
> >
> > ofproto_port_open_type() was surprisingly slow because it called the
> > function ofproto_class_find__(), which itself was surprisingly slow
> because
> > it actually creates a set of strings and enumerates all of the available
> > classes.
> >
> > This patch improves performance by eliminating the call to
> > ofproto_class_find__() from ofproto_port_open_type().  In turn that
> > required changing a parameter type and updating all the callers.
> >
> > Possibly it would be worth making ofproto_class_find__() itself faster,
> > but it doesn't look like any of its other callers would be used in inner
> > loops.
> >
> > A different patch that was also meant to speed this up reported the
> > following performance improvements.  That patch just eliminated half of
> the
> > calls to ofproto_class_find__() in inner loops, whereas this one
> eliminates
> > all of them and should therefore perform even better.
> >
> > This patch arises as a result of testing done by Ali Ginwala and Han
> > Zhou. Their test showed that commit 2d4beba resulted in slower
> > performance of ovs-vswitchd than was seen in previous versions of OVS.
> >
> > With this patch, Ali retested and reported that this patch had nearly
> > the same effect on performance as reverting 2d4beba.
> >
> > The test was to create 1 logical switch ports using 20 farm nodes,
> > each running 50 HV sandboxes. "Base" in the tests below is OVS master
> > with Han Zhou's ovn-controller incremental processing patch applied.
> >
> > base: Test completed in 8 hours
> > base with 2d4beba reverted: Test completed in 5 hours 28 minutes
> > base with this patch: Test completed in 5 hours 30 minutes
> >
> > Reported-by: Mark Michelson 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  ofproto/in-band.c |  2 +-
> >  ofproto/ofproto.c | 30 ++
> >  ofproto/ofproto.h |  2 +-
> >  vswitchd/bridge.c | 12 
> >  4 files changed, 16 insertions(+), 30 deletions(-)
> >
> > diff --git a/ofproto/in-band.c b/ofproto/in-band.c
> > index 849b1cedaff0..82d8dfa14774 100644
> > --- a/ofproto/in-band.c
> > +++ b/ofproto/in-band.c
> > @@ -428,7 +428,7 @@ in_band_create(struct ofproto *ofproto, const char
> *local_name,
> >  struct in_band *in_band;
> >  struct netdev *local_netdev;
> >  int error;
> > -const char *type = ofproto_port_open_type(ofproto->type, "internal");
> > +const char *type = ofproto_port_open_type(ofproto, "internal");
> >
> >  *in_bandp = NULL;
> >  error = netdev_open(local_name, type, _netdev);
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index f28bb896eee9..a982de9d8db4 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -1966,27 +1966,18 @@ ofproto_port_dump_done(struct ofproto_port_dump
> *dump)
> >  return dump->error == EOF ? 0 : dump->error;
> >  }
> >
> > -/* Returns the type to pass to netdev_open() when a datapath of type
> > - * 'datapath_type' has a port of type 'port_type', for a few special
> > - * cases when a netdev type differs from a port type.  For example, when
> > - * using the userspace datapath, a port of type "internal" needs to be
> > - * opened as "tap".
> > +/* Returns the type to pass to netdev_open() when 'ofproto' has a port
> of type
> > + * 'port_type', for a few special cases when a netdev type differs from
> a port
> > + * type.  For example, when using the userspace datapath, a port of type
> > + * "internal" needs to be opened as "tap".
> >   *
> >   * Returns either 'type' itself or a string literal, which must not be
> >   * freed. */
> >  const char *
> > -ofproto_port_open_type(const char *datapath_type, const char *port_type)
> > +ofproto_port_open_type(const struct ofproto *ofproto, const char
> *port_type)
> >  {
> > -const struct ofproto_class *class;
> > -
> > -datapath_type = ofproto_normalize_type(datapath_type);
> > -class = ofproto_class_find__(datapath_type);
> > -if (!class) {
> > -return port_type;
> > -}
> > -
> > -return (class->port_open_type
> > -? class->port_open_type(datapath_type, port_type)
> > +return (ofproto->ofproto_class->port_open_type
> > +? ofproto->ofproto_class->port_open_type(ofproto->type,
> port_type)
> >  : port_type);
> >  }
> >
> > @@ -2728,10 +2719,9 @@ init_ports(struct ofproto *p)
> >  static bool
> >  ofport_is_internal_or_patch(const struct ofproto *p, const struct ofport
> *port)
> >  {
> > -return !strcmp(netdev_get_type(port->netdev),
> > -   ofproto_port_open_type(p->type, "internal")) ||
> > -   !strcmp(netdev_get_type(port->netdev),
> > -   ofproto_port_open_type(p->type, "patch"));
> > +const char *netdev_type = netdev_get_type(port->netdev);
> > +

Re: [ovs-dev] [RFC 4/4] dpif-netdev.c: Add indirect table

[Wang, Yipeng1]

Thanks for the comments, reply inlined:

>-Original Message-
>From: Bodireddy, Bhanuprakash
>Sent: Friday, February 23, 2018 1:06 PM
>To: Wang, Yipeng1 ; d...@openvswitch.org; 
>jan.scheur...@ericsson.com
>Cc: Tai, Charlie 
>Subject: RE: [ovs-dev] [RFC 4/4] dpif-netdev.c: Add indirect table
>
>Hi Yipeng,
>
>>If we store pointers in DFC, then the memory requirement is large. When
>>there are VMs or multiple PMDs running on the same platform, they will
>>compete the shared cache. So we want DFC to be as memory efficient as
>>possible.
>
>>
>>Indirect table is a simple hash table that map the DFC's result to the
>>dp_netdev_flow's pointer. This is to reduce the memory size of the DFC
>>cache, assuming that the megaflow count is much smaller than the exact
>>match flow count. With this commit, we could reduce the 8-byte pointer to a
>>2-byte index in DFC cache so that the memory/cache requirement is almost
>>halved. Another approach we plan to try is to use the flow_table as the
>>indirect table.
>
>I assume this patch is only aimed at reducing the DFC cache memory foot print 
>and doesn't introduce any new functionality ?
>
[Wang, Yipeng] Yes, by introducing an indirect table. We have another solution 
to use the "flow_table" we will post later. But there will be pros/cons.

>With this I see the dfc_bucket size is at 32 bytes from earlier 80bytes in 3/4 
>and the buckets now will be aligned to cache lines.
>Also the dfc_cache size reduced to ~8MB from ~12MB in 1/4 and ~14Mb in 3/4 
>patches.
>
[Wang, Yipeng]  With 1M entry, the size is as below:
1/4: 8MB
3/4: 10MB
4/4: 4MB + indirect table size, which is 512k to contain 64k rules.


>I am guessing there might be some performance improvement with this patch due 
>to buckets aligning to cache lines apart of reduced
>memory footprint. Do you see any such advantage here in your benchmarks?
>
[Wang, Yipeng] Actually due to the indirect table (1 more memory jump), we saw 
1-2% throughput drop comparing to 3/4 in simple tests. 
But as you said this does not consider the cache efficiency effect. There will 
be performance improvement with cache contention. So we believe
this is more scalable.

>Regards,
>Bhanuprakash.
>
>>
>>The indirect table size is a fixed constant for now.
>>
>>Signed-off-by: Yipeng Wang 
>>---
>> lib/dpif-netdev.c | 69 +++
>>
>> 1 file changed, 44 insertions(+), 25 deletions(-)
>>
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 50a1d25..35197d3
>>100644
>>--- a/lib/dpif-netdev.c
>>+++ b/lib/dpif-netdev.c
>>@@ -151,6 +151,12 @@ struct netdev_flow_key {
>>
>> #define DFC_MASK_LEN 20
>> #define DFC_ENTRY_PER_BUCKET 8
>>+
>>+/* For now we fix the Indirect table size, ideally it should be sized
>>+according
>>+ * to max megaflow count but less than 2^16  */ #define
>>+INDIRECT_TABLE_SIZE (1u << 12) #define INDIRECT_TABLE_MASK
>>+(INDIRECT_TABLE_SIZE - 1)
>> #define DFC_ENTRIES (1u << DFC_MASK_LEN)  #define DFC_BUCKET_CNT
>>(DFC_ENTRIES / DFC_ENTRY_PER_BUCKET)  #define DFC_MASK
>>(DFC_BUCKET_CNT - 1) @@ -175,13 +181,14 @@ struct emc_cache {
>>
>> struct dfc_bucket {
>> uint16_t sig[DFC_ENTRY_PER_BUCKET];
>>-struct dp_netdev_flow *flow[DFC_ENTRY_PER_BUCKET];
>>+uint16_t index[DFC_ENTRY_PER_BUCKET];
>> };
>>
>> struct dfc_cache {
>> struct emc_cache emc_cache;
>> struct dfc_bucket buckets[DFC_BUCKET_CNT];
>> int sweep_idx;
>>+struct dp_netdev_flow *indirect_table[INDIRECT_TABLE_SIZE];
>> };
>>
>>
>
>>@@ -754,7 +761,7 @@ static int dpif_netdev_xps_get_tx_qid(const struct
>>dp_netdev_pmd_thread *pmd,
>>
>> static inline bool dfc_entry_alive(struct dp_netdev_flow *flow);  static void
>>emc_clear_entry(struct emc_entry *ce); -static void dfc_clear_entry(struct
>>dfc_bucket *b, int idx);
>>+static void dfc_clear_entry(struct dp_netdev_flow **flow, struct
>>+dfc_bucket *b, int idx);
>>
>> static void dp_netdev_request_reconfigure(struct dp_netdev *dp);
>>
>>@@ -782,9 +789,12 @@ dfc_cache_init(struct dfc_cache *flow_cache)
>> emc_cache_init(_cache->emc_cache);
>> for (i = 0; i < DFC_BUCKET_CNT; i++) {
>> for (j = 0; j < DFC_ENTRY_PER_BUCKET; j++) {
>>-flow_cache->buckets[i].flow[j] = NULL;
>>+flow_cache->buckets[i].sig[j] = 0;
>> }
>> }
>>+for (i = 0; i < INDIRECT_TABLE_SIZE; i++) {
>>+flow_cache->indirect_table[i] = NULL;
>>+}
>> flow_cache->sweep_idx = 0;
>> }
>>
>>@@ -805,7 +815,7 @@ dfc_cache_uninit(struct dfc_cache *flow_cache)
>>
>> for (i = 0; i < DFC_BUCKET_CNT; i++) {
>> for (j = 0; j < DFC_ENTRY_PER_BUCKET; j++) {
>>-dfc_clear_entry(&(flow_cache->buckets[i]), j);
>>+dfc_clear_entry(flow_cache->indirect_table,
>>+ &(flow_cache->buckets[i]), j);
>> }
>> }
>> emc_cache_uninit(_cache->emc_cache);
>>@@ -2259,7 +2269,7 @@ dfc_entry_get(struct dfc_cache *cache, const

Re: [ovs-dev] [RFC 3/4] dpif-netdev: Use way-associative cache

[Wang, Yipeng1]

Thanks for the comments. Reply inlined:

>-Original Message-
>From: Bodireddy, Bhanuprakash
>Sent: Friday, February 23, 2018 12:56 PM
>To: Wang, Yipeng1 ; d...@openvswitch.org; 
>jan.scheur...@ericsson.com
>Cc: Tai, Charlie 
>Subject: RE: [ovs-dev] [RFC 3/4] dpif-netdev: Use way-associative cache
>
>Hi Yipeng,
>
>Thanks for the patch. Some high level questions/comments.
>
>(1)  Am I right in understanding that this patch *only* introduces a new cache 
>approach in to DFC to reduce the collisions?
>
[Wang, Yipeng] Yes, including set-associative structure and the signatures.

>(2)  Why the number of entries per Bucket is set to '8'?  With this each 
>dfc_bucket  size is 80 bytes (16 + 64).
>If the number of entries set to '6', the dfc_bucket size will be 60 
> bytes and can fit in to a cache line.
>I assume 'DFC_ENTRY_PER_BUCKET' isn't a random picked number. Was it 
> picked due to any benchmarks?
>
[Wang, Yipeng] Next commit (4/4) will solve this issue. Without 4/4,  I agree 
that cache alignment is important and 6 makes more sense.

>(3) A 2 byte signature is introduced in a bucket and is used to insert or 
>retrieve flows in to the bucket.
>3a. Due to the introduction of 2 byte signature the size of dfc_cache 
> increased by 2MB per PMD thread.
[Wang, Yipeng] Yes, the next commit will alleviate the memory issue.

>3b. Every time we insert or retrieve a flow, we have to match the 
> packet signature(upper 16 bit RSS hash) with each entry of the
>bucket. Wondering if that slow down the operations?
[Wang, Yipeng] Yes, I saw 1-4%  throughput penalty. However, the cache 
collision reduced a lot and performance generally improved for most cases.
https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343029.html here I 
shown some results.  Note that if there is only one rule, then signature does 
not
help but this case is less realistic.
>
>(4)  The number of buckets depends on the number of entries per bucket.  Which 
>of this plays an important role in reducing the
>collisions?
>i.e Would higher number of entries per bucket reduce the collisions?
>
[Wang, Yipeng] Generally yes, but there will be diminishing returns on the 
collision rate reduction with more than 8 entries/bucket. Also, comparing 
signatures will be more costly with more entries each bucket. 

>(5) What is the performance delta observed with this new Cache implementation 
>over 1/4 approach?
>
[Wang, Yipeng]  Please check out these numbers I generated before: 
https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343029.html
>Some more minor comments below.
>
>>This commit uses a way-associative cache (CD) rather than a simple single
>>entry hash table for DFC. Experiments show that this design generally has
>>much higher hit rate.
>>
>>@@ -774,11 +777,13 @@ emc_cache_init(struct emc_cache *emc)  static void
>>dfc_cache_init(struct dfc_cache *flow_cache)  {
>>-int i;
>>+int i, j;
>>
>> emc_cache_init(_cache->emc_cache);
>>-for (i = 0; i < ARRAY_SIZE(flow_cache->entries); i++) {
>>-flow_cache->entries[i].flow = NULL;
>>+for (i = 0; i < DFC_BUCKET_CNT; i++) {
>>+for (j = 0; j < DFC_ENTRY_PER_BUCKET; j++) {
>>+flow_cache->buckets[i].flow[j] = NULL;
>
>[BHANU] How about initializing the signature?
>
[Wang, Yipeng] Thanks for pointing out. Functionally it should be fine since I 
use pointer to check entry valid or not anyway. But I will do in next version.

>> }
>> }
>>@@ -2288,10 +2302,25 @@ dfc_insert(struct dp_netdev_pmd_thread *pmd,
>>struct dp_netdev_flow *flow)  {
>> struct dfc_cache *cache = >flow_cache;
>>-struct dfc_entry *current_entry;
>>
>>-current_entry = dfc_entry_get(cache, key->hash);
>>-dfc_change_entry(current_entry, flow);
>>+struct dfc_bucket *bucket = >buckets[key->hash & DFC_MASK];
>>+uint16_t sig = key->hash >> 16;
>
>[BHANU]  Am I right in assuming the below logic is to replace an already 
>existing entry in bucket?
[Wang, Yipeng] Yes, to overwrite an entry with same signature.
>
>>+for (int i = 0; i < DFC_ENTRY_PER_BUCKET; i++) {
>>+if(bucket->sig[i] == sig) {
>>+dfc_change_entry(bucket, i, flow);
>>+return;
>>+}
>>+}
>
>[BHANU] The below happens if inserting in to empty bucket?
>
[Wang, Yipeng] Yes, if there is empty entry we insert there.

>+for (int i = 0; i < DFC_ENTRY_PER_BUCKET; i++) {
>+if(bucket->flow[i] == NULL) {
>+bucket->sig[i] = sig;
>+dfc_change_entry(bucket, i, flow);
>+return;
>+}
>+}

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

Re: [ovs-dev] [RFC 1/4] dpif-netdev: Refactor datapath flow cache

[Wang, Yipeng1]

Bhanu, thanks for the comment. Please see my comments inlined.

And Jan please feel free to add more. 

>-Original Message-
>From: Bodireddy, Bhanuprakash
>Sent: Tuesday, February 20, 2018 1:10 PM
>To: Wang, Yipeng1 ; d...@openvswitch.org; 
>jan.scheur...@ericsson.com
>Cc: Tai, Charlie 
>Subject: RE: [ovs-dev] [RFC 1/4] dpif-netdev: Refactor datapath flow cache
>
>Hi Yipeng,
>
>Thanks for the RFC series. This patch series need to be rebased.
>I applied this on an older commit to do initial testing. Some comments below.
>
>I see that DFC cache is implemented in similar lines of EMC cache except that 
>it holds
>Million entries and uses more bits of RSS hash to index in to the Cache. 
[Wang, Yipeng] 
Conceptually, DFC/CD is much memory efficient than EMC since it does not store 
the full key.

>DPCLS lookup is expensive and consumes 30% of total cycles in some test cases 
>and DFC
>Cache will definitely reduce some pain there.
>
>On the memory foot print:
>
>On Master,
>EMC  entry size = 592 bytes
>   8k entries = ~4MB.
>
>With this patch,
> EMC entry size = 256 bytes
>  16k entries = ~4MB.
>
>I like the above reduction in flow key size, keeping the entry size to 
>multiple of cache line and still keeping the overall EMC size to
>~4MB with more EMC entries.
>
>However my concern is DFC cache size. As DFC cache is million entries and 
>consumes ~12 MB for each PMD thread, it might not fit in to
>L3 cache. Also note that in newer platforms L3 cache is shrinking and L2 is 
>slightly increased (eg: Skylake has only 1MB L2 and 19MB L3
>cache).
>
[Wang, Yipeng] 
Yes, this is also our concern. The following (4/4) patch introduces indirect 
table for this reason. 

>Inspite of the memory footprint I still think DFC cache improves switching 
>performance as it is lot less expensive than invoking
>dpcls_lookup() as the later involves more expensive hash computation and 
>subtable traversal. It would be nice if there is more testing
>done with real VNFs to see that this patch doesn't cause cache thrashing and 
>suffer from memory bottlenecks.
>
[Wang, Yipeng] 
I don't have real VNF benchmarking set, but will it be useful if we do test 
with some synthetic cache pirating application to emulate the effect? 
 
>
>[BHANU]
>I am not sure if this is good idea to simplify EMC by using 1-way associative 
>instead of current 2 way associative implementation.
>I prefer to leave the current approach as-is unless we have strong data to 
>prove it otherwise.
>This comment applies to below code changes w.r.t to EMC lookup and insert.
>
[Wang, Yipeng] 
I have synthetic tests showing simpler EMC works much better, while I also have 
other sets of synthetic tests showing the 2way EMC works much better.
It eventually depends on the use cases.  We had some discussion at this thread: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/342197.html

>>The maximum size of the EMC flow key is limited to 256 bytes to reduce the
>>memory footprint. This should be sufficient to hold most real life packet flow
>>keys. Larger flows are not installed in the EMC.
>
>+1
>
>>
>> reload:
>> pmd_alloc_static_tx_qid(pmd);
>>
>>@@ -4166,8 +4282,7 @@ reload:
>> }
>>
>> if (lc++ > 1024) {
>>-bool reload;
>>-
>>+dfc_slow_sweep(>flow_cache);
>[BHANU]  I need to better understand the usage of RCUs. But I am wondering why 
>the sweep function
>Isn't under the below !rcu_try_quiesce() condition?

[Wang, Yipeng] 
The condition is to see if this pmd can be successfully quiesced once in this 
iteration. And sweeping will evict EMC entries and register rcu callbacks to 
free megaflow entry later.  Then putting this function under the condition 
means that we sweep and register new call back only if this PMD successfully 
quiesced this round.
I think functionally it should be fine either put sweep inside or outside the 
condition. But if outside the condition, there may be callbacks accumulated and 
cannot be called since this PMD may never be able to enter quiesced state once. 
 So, I think we may want to change the code to keep the sweep under the 
condition.
Others may know better of this, please comment.
>
>>@@ -5039,7 +5154,7 @@ emc_processing(struct dp_netdev_pmd_thread
>>*pmd,
>> }
>> miniflow_extract(packet, >mf);
>> key->len = 0; /* Not computed yet. */
>>-/* If EMC is disabled skip hash computation and emc_lookup */
>>+/* If DFC is disabled skip hash computation and DFC lookup */
>[BHANU] Why would DFC ever be disabled by user? This was done earlier for EMC 
>as in EMC saturation case, there was penalty doing
>emc insertion
>and emc_lookup() (that involved costly memcmp()) and disabling EMC showed some 
>10% advantage for some use cases.
>
>This might not apply for DFC and I assume DFC shouldn't be configurable option?

[Wang, Yipeng] 
Yes, this part of code should be tweaked a 

[ovs-dev] Thank you so much,

[Mr. Joong-hyun Kim]

Thank you so much,

I 'm happy to inform you about my successed with the funds transferred under 
the co operation of a new business partner from Paraguay.

Presently I'm in Paraguay for treatment and investment in the country 
meanwhile,I left your compensation Atm card with Mr John Ladi ask him for your 
Atm card, Name: Via Email: (pastorjohnl...@live.com ) Ask him to send you the 
Atm card containing the total of ($800.000.00.USD) Which I kept for you.

Best Regards,
Mr.Joong.Hunkim.

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

Re: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length

[Wang, Yipeng1]

Thanks Bhanu and Jan,

For this proposed simple fix, should the exact match flow key->len not equal to 
megaflow length from cr.flow.len?  Since one megaflow could match many exact 
match flows...

Thanks
Yipeng

>-Original Message-
>From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
>Sent: Wednesday, February 21, 2018 12:09 AM
>To: Bodireddy, Bhanuprakash ; Wang, Yipeng1 
>;
>d...@openvswitch.org
>Cc: Tai, Charlie 
>Subject: RE: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>
>I think there is an even simpler fix for the problem:
>
>+/* Found a match in DFC. Insert into EMC for subsequent lookups.
>+ * We use probabilistic insertion here so that mainly elephant
>+ * flows enter EMC.
>+ * Initialize the key's length from the matched flow. */
>+key->len = flow->cr.flow.len;
>+emc_probabilistic_insert(>emc_cache, key, flow);
>+*exact_match = false;
>+return flow;
>+} else {
>
>Please use that in the next version of 1/4.
>
>BR, Jan
>
>> -Original Message-
>> From: Bodireddy, Bhanuprakash [mailto:bhanuprakash.bodire...@intel.com]
>> Sent: Tuesday, 20 February, 2018 22:14
>> To: Wang, Yipeng1 ; d...@openvswitch.org; Jan 
>> Scheurich 
>> Cc: Tai, Charlie 
>> Subject: RE: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>>
>> This fix is needed and can  be included in 1/4 in next revision.
>>
>> - Bhanuprakash.
>>
>> >-Original Message-
>> >From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>> >boun...@openvswitch.org] On Behalf Of Yipeng Wang
>> >Sent: Thursday, January 18, 2018 6:20 PM
>> >To: d...@openvswitch.org; jan.scheur...@ericsson.com
>> >Cc: Tai, Charlie 
>> >Subject: [ovs-dev] [RFC 2/4] dpif-netdev: Fix EMC key length
>> >
>> >EMC's key length is not initialized when insertion. Initialize the key 
>> >length
>> >before insertion.
>> >
>> >The code might be put in another place, for now I just put it in dfc_lookup.
>> >
>> >Signed-off-by: Yipeng Wang 
>> >---
>> > lib/dpif-netdev.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index b9f4b6d..3e87992
>> >100644
>> >--- a/lib/dpif-netdev.c
>> >+++ b/lib/dpif-netdev.c
>> >@@ -2295,7 +2295,7 @@ dfc_insert(struct dp_netdev_pmd_thread *pmd,  }
>> >
>> > static inline struct dp_netdev_flow *
>> >-dfc_lookup(struct dfc_cache *cache, const struct netdev_flow_key *key,
>> >+dfc_lookup(struct dfc_cache *cache, struct netdev_flow_key *key,
>> >bool *exact_match)
>> > {
>> > struct dp_netdev_flow *flow;
>> >@@ -2317,6 +2317,7 @@ dfc_lookup(struct dfc_cache *cache, const struct
>> >netdev_flow_key *key,
>> > /* Found a match in DFC. Insert into EMC for subsequent lookups.
>> >  * We use probabilistic insertion here so that mainly elephant
>> >  * flows enter EMC. */
>> >+key->len = netdev_flow_key_size(miniflow_n_values(>mf));
>> > emc_probabilistic_insert(>emc_cache, key, flow);
>> > *exact_match = false;
>> > return flow;
>> >--
>> >2.7.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 v3] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics CLI Changes

[Satyavalli Rama]

Hi Ben and Jan,

Below are the logs of the changed OXS Commands please have a look on them once 
and provide your valuable inputs if any...

# ovs-ofctl -O OpenFlow15 dump-flows br0 
 cookie=0x0, duration=9407.750s, table=0, n_packets=61, n_bytes=7320, 
idle_age=94, ip actions=NORMAL 
 cookie=0x0, duration=14615.659s, table=0, n_packets=354, n_bytes=36862, 
idle_age=9547, priority=0 actions=NORMAL 

  
# ovs-ofctl -O OpenFlow15 dump-flows br0 --oxs-stats=duration 
 duration=9391.712s   
 duration=14599.621s   

  
# ovs-ofctl -O OpenFlow15 dump-flows br0 --oxs-stats=duration,packet_count 
 duration=9401.845s  n_packets=61   
 duration=14609.754s  n_packets=354   

  

  
# ovs-ofctl -O OpenFlow14 dump-flows br0 --oxs-stats=duration,packet_count 
ovs-ofctl: None of the enabled OpenFlow versions (OpenFlow14)  supports oxs 
(OXM-OpenFlow15,OXM-OpenFlow16).(Use -O to enable  additional OpenFlow 
versions) 

  
---
  
# ovs-ofctl -O OpenFlow15 dump-aggregate br0 --oxs-stats=byte,packet_count 
 n_packets=415 n_bytes=44182   

# ovs-ofctl -O OpenFlow15 dump-aggregate br0 
OFPST_AGGREGATE reply (OF1.5) (xid=0x2): packet_count=415 byte_count=44182 
flow_count=2 

  

  
#ovs-ofctl -O OpenFlow14 dump-aggregate br0 --oxs-stats=packet_count,byte_count 
ovs-ofctl: None of the enabled OpenFlow versions (OpenFlow14)  supports oxs 
(OXM-OpenFlow15,OXM-OpenFlow16).(Use -O to enable  additional OpenFlow versions)
  


 
We will send this patch in a day or two.
 
Thanks & Regards
 Satya Valli
 Tata Consultancy Services
 Mailto: satyavalli.r...@tcs.com
 Website: http://www.tcs.com
 
 Experience certainty.  IT Services
Business Solutions
Consulting
 
 

-Ben Pfaff  wrote: -
To: Satyavalli Rama 
From: Ben Pfaff 
Date: 02/23/2018 11:55PM
Cc: Jan Scheurich , "d...@openvswitch.org" 
, SatyaValli , 
manasa.cherukupa...@tcs.com, muttamsetty.su...@tcs.com, p.pava...@tcs.com, 
Harivelam Lavanya 
Subject: Re: [ovs-dev] [PATCH v3] OF1.5/EXT-334 OXS/Extensible Flow Entry 
Statistics Support

On Thu, Feb 22, 2018 at 08:29:11PM +0530, Satyavalli Rama wrote:
> Jan and Ben we requesting you both kindly provide your valuable inputs with 
> respect new enhancement i.e  adding  --oxs-stats option to the existing 
> commands. 
> If everything mentioned above is fine then we will share the updated patch 
> soon.

Seems reasonable, please send the revised patch.
=-=-=
Notice: The information contained in this e-mail
message and/or attachments to it may contain 
confidential or privileged information. If you are 
not the intended recipient, any dissemination, use, 
review, distribution, printing or copying of the 
information contained in this e-mail message 
and/or attachments to it are strictly prohibited. If 
you have received this communication in error, 
please notify us by reply e-mail or telephone and 
immediately and permanently delete the message 
and any attachments. Thank you


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

Re: [ovs-dev] [PATCH] lib/cmap: fix a typo

[Mark Michelson]

Acked-by: Mark Michelson 

On 02/24/2018 01:34 AM, zhangliping wrote:

From: zhangliping 

cmap_find_locked should be cmap_find_protected.

Signed-off-by: zhangliping 
---
  lib/cmap.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cmap.h b/lib/cmap.h
index d390923c8..5a72b65be 100644
--- a/lib/cmap.h
+++ b/lib/cmap.h
@@ -140,7 +140,7 @@ size_t cmap_replace(struct cmap *, struct cmap_node 
*old_node,
  #define CMAP_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, CMAP)   \
  CMAP_NODE_FOR_EACH(NODE, MEMBER, cmap_find(CMAP, HASH))
  #define CMAP_FOR_EACH_WITH_HASH_PROTECTED(NODE, MEMBER, HASH, CMAP) \
-CMAP_NODE_FOR_EACH_PROTECTED(NODE, MEMBER, cmap_find_locked(CMAP, HASH))
+CMAP_NODE_FOR_EACH_PROTECTED(NODE, MEMBER, cmap_find_protected(CMAP, HASH))
  
  const struct cmap_node *cmap_find(const struct cmap *, uint32_t hash);

  struct cmap_node *cmap_find_protected(const struct cmap *, uint32_t hash);



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

Re: [ovs-dev] [PATCH RFC 0/2] tunnel: add erspan support.

[William Tu]

On Sat, Feb 24, 2018 at 2:33 PM, Gregory Rose  wrote:
> On 2/21/2018 2:19 PM, William Tu wrote:
>>
>> The RFC patch adds erspan support for OVS to use the
>> erspan feature in its kernel datapath.  Currently the
>> patch only works on kernel 4.15 thus I mark it as RFC.
>> The first patch fixes the compile error on 4.15, and
>> the second patch introduces erspan tunnel protocol.
>>
>> William Tu (2):
>>erspan: fix compile error for kernel 4.15
>>tunnel: add erspan support for kernel datapath.
>>
>>   datapath/linux/compat/include/linux/openvswitch.h |   2 +
>>   datapath/linux/compat/include/net/ip6_fib.h   |   4 +-
>>   include/openvswitch/flow.h|   4 +-
>>   include/openvswitch/match.h   |   8 ++
>>   include/openvswitch/meta-flow.h   |  56 
>>   include/openvswitch/packets.h |   6 +-
>>   lib/dpif-netlink-rtnl.c   |   5 +
>>   lib/dpif-netlink.c|   5 +
>>   lib/flow.c|  32 +++--
>>   lib/flow.h|   2 +-
>>   lib/match.c   |  66 +-
>>   lib/meta-flow.c   |  77 +++
>>   lib/meta-flow.xml |  86 
>>   lib/netdev-vport.c|  23 
>>   lib/netdev.h  |   5 +
>>   lib/nx-match.c|  13 +-
>>   lib/odp-util.c| 151
>> ++
>>   lib/odp-util.h|   2 +-
>>   lib/ofp-match.c   |   2 +-
>>   lib/packets.h |  91 +
>>   ofproto/ofproto-dpif-rid.h|   2 +-
>>   ofproto/ofproto-dpif-xlate.c  |   3 +-
>>   ofproto/tunnel.c  |  13 ++
>>   tests/odp.at  |  25 +++-
>>   tests/ofproto.at  |   6 +-
>>   tests/system-common-macros.at |   5 +
>>   tests/system-traffic.at   |  70 ++
>>   tests/tunnel.at   | 105 +++
>>   vswitchd/vswitch.xml  |  34 +
>>   29 files changed, 881 insertions(+), 22 deletions(-)
>>
>
> Thanks for adding the system tests.  I should have a chance in the next few
> days to test it out
> on the erspan backport patches I'm working on.
>
> Regards,
>
> - Greg

Thanks Greg.

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

Re: [ovs-dev] OVS-DPDK public meeting

[Chandran, Sugesh]

Please find below for the latest update on the OVS-DPDK full offload status.

Regards
_Sugesh


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Kevin Traynor
> Sent: Wednesday, February 21, 2018 6:48 PM
> To: d...@openvswitch.org
> Subject: Re: [ovs-dev] OVS-DPDK public meeting
> 
> 21st February 2018
> Next meeting: 7th March @ 17:00 UTC
> 
> Attendees: Eelco, Michael, Ian, Thomas, Anders, Billy, Harem (sp?), Simon,
> Johann, Yipeng, Frikkie, Sameh, Pradeep, Kevin.
> 
> ===
> GENERAL
> ===
> OVS 2.9 Release (Kevin)
> -- Thanks to Ian for helping get the patches onto dpdk_merge branch and pull
> requests
> 
> OVS 2.9 testing (Kevin)
> -- Nothing blocking so far in Red Hat tests
> 
> Updated status roadmap (Ian)
> -- Jan has put a list of items that Ericsson intend to work on
> -- Pushed discussion until next meeting
> https://docs.google.com/spreadsheets/d/1FilGq46vQePFKoehADWWsDvDCZSN
> sMU7PrpPr3EM3lU/edit
> 
> Meeting
> -- Back to usual bi-weekly cadence
> -- Ping me if anyone wants to be added to the calendar invite
> 
> 
> PERFORMANCE/FEATURES
> 
> 
> Mempools (Ian)
> -- Going to kick off discussion upstream about how to rework to cater for 
> shared
> and per port mempools
> 
> 
> HARDWARE OFFLOAD
> 
> Classification offload
> 
>  (Thomas, Kevin)
> -- There was some comments (on ML I think?) about possibly modularizing it
> away from existing code
> -- A few people previously indicated they would like to review
> -- It's on v7 - so would like to get any remaining comments in over next 
> couple
> of weeks so it can be merged early in the 2.10 release cycle
> 
> 
> Full hardware offload (Sugesh)
> -- Sugesh has some patches in a tree, details here
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344606.html
> -- There is a separate meeting where details about rte_flow hardware offload 
> is
> being discussed - info on ML or ping Sugesh
> -- Modifications needed to DPDK in areas of rte_flow, port representor and
> capability discovery
> -- May not be in DPDK until DPDK18.11. Thomas suggested to aim for
> DPDK18.08 in order to stabilize API for DPDK 18.11 LTS.
[Sugesh] After the discussion with intel DPDK team, we expect basic DPDK
full acceleration apis  will be ready in DPDK-18.05 and we can leverage  them 
in OVS
for the initial release. This means we can have some early enablement of 
OVS-DPDK full acceleration
by 2.10

So lets plan full acceleration enablement for 
> 
> On 07/25/2017 02:25 PM, Kevin Traynor wrote:
> > Hi All,
> >
> > The OVS-DPDK public meetings have moved to Wednesday's at the same time.
> > Everyone is welcome, it's a chance to share status/plans etc.
> >
> > It's scheduled for every 2 weeks starting 26th July. Meeting time is
> > currently @ 4pm UTC.
> >
> > You can connect through Bluejeans or through dial in:
> >
> > https://bluejeans.com/139318596
> >
> > US: +1.408.740.7256 
> > UK: +44.203.608.5256 
> > Germany: +49.32.221.091256 
> > Ireland: +353.1.697.1256  Other numbers at
> > https://www.bluejeans.com/numbers Meeting ID: 139318596
> >
> > thanks,
> > Kevin.
> >
> 
> ___
> 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] ofproto: Make ofproto_port_open_type() faster.

[Mark Michelson]

Yes this is much better than the patch I submitted. Thank you very much Ben.

Acked-by: Mark Michelson 

On 02/23/2018 04:03 PM, Ben Pfaff wrote:

ofproto_port_open_type() was surprisingly slow because it called the
function ofproto_class_find__(), which itself was surprisingly slow because
it actually creates a set of strings and enumerates all of the available
classes.

This patch improves performance by eliminating the call to
ofproto_class_find__() from ofproto_port_open_type().  In turn that
required changing a parameter type and updating all the callers.

Possibly it would be worth making ofproto_class_find__() itself faster,
but it doesn't look like any of its other callers would be used in inner
loops.

A different patch that was also meant to speed this up reported the
following performance improvements.  That patch just eliminated half of the
calls to ofproto_class_find__() in inner loops, whereas this one eliminates
all of them and should therefore perform even better.

 This patch arises as a result of testing done by Ali Ginwala and Han
 Zhou. Their test showed that commit 2d4beba resulted in slower
 performance of ovs-vswitchd than was seen in previous versions of OVS.

 With this patch, Ali retested and reported that this patch had nearly
 the same effect on performance as reverting 2d4beba.

 The test was to create 1 logical switch ports using 20 farm nodes,
 each running 50 HV sandboxes. "Base" in the tests below is OVS master
 with Han Zhou's ovn-controller incremental processing patch applied.

 base: Test completed in 8 hours
 base with 2d4beba reverted: Test completed in 5 hours 28 minutes
 base with this patch: Test completed in 5 hours 30 minutes

Reported-by: Mark Michelson 
Signed-off-by: Ben Pfaff 
---
  ofproto/in-band.c |  2 +-
  ofproto/ofproto.c | 30 ++
  ofproto/ofproto.h |  2 +-
  vswitchd/bridge.c | 12 
  4 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 849b1cedaff0..82d8dfa14774 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -428,7 +428,7 @@ in_band_create(struct ofproto *ofproto, const char 
*local_name,
  struct in_band *in_band;
  struct netdev *local_netdev;
  int error;
-const char *type = ofproto_port_open_type(ofproto->type, "internal");
+const char *type = ofproto_port_open_type(ofproto, "internal");
  
  *in_bandp = NULL;

  error = netdev_open(local_name, type, _netdev);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f28bb896eee9..a982de9d8db4 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1966,27 +1966,18 @@ ofproto_port_dump_done(struct ofproto_port_dump *dump)
  return dump->error == EOF ? 0 : dump->error;
  }
  
-/* Returns the type to pass to netdev_open() when a datapath of type

- * 'datapath_type' has a port of type 'port_type', for a few special
- * cases when a netdev type differs from a port type.  For example, when
- * using the userspace datapath, a port of type "internal" needs to be
- * opened as "tap".
+/* Returns the type to pass to netdev_open() when 'ofproto' has a port of type
+ * 'port_type', for a few special cases when a netdev type differs from a port
+ * type.  For example, when using the userspace datapath, a port of type
+ * "internal" needs to be opened as "tap".
   *
   * Returns either 'type' itself or a string literal, which must not be
   * freed. */
  const char *
-ofproto_port_open_type(const char *datapath_type, const char *port_type)
+ofproto_port_open_type(const struct ofproto *ofproto, const char *port_type)
  {
-const struct ofproto_class *class;
-
-datapath_type = ofproto_normalize_type(datapath_type);
-class = ofproto_class_find__(datapath_type);
-if (!class) {
-return port_type;
-}
-
-return (class->port_open_type
-? class->port_open_type(datapath_type, port_type)
+return (ofproto->ofproto_class->port_open_type
+? ofproto->ofproto_class->port_open_type(ofproto->type, port_type)
  : port_type);
  }
  
@@ -2728,10 +2719,9 @@ init_ports(struct ofproto *p)

  static bool
  ofport_is_internal_or_patch(const struct ofproto *p, const struct ofport 
*port)
  {
-return !strcmp(netdev_get_type(port->netdev),
-   ofproto_port_open_type(p->type, "internal")) ||
-   !strcmp(netdev_get_type(port->netdev),
-   ofproto_port_open_type(p->type, "patch"));
+const char *netdev_type = netdev_get_type(port->netdev);
+return !strcmp(netdev_type, ofproto_port_open_type(p, "internal")) ||
+   !strcmp(netdev_type, ofproto_port_open_type(p, "patch"));
  }
  
  /* If 'port' is internal or patch and if the user didn't explicitly specify an

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 1e48e1952aa2..8c85bbf7fb26 100644
--- a/ofproto/ofproto.h

Re: [ovs-dev] [PATCH] vlog: fix the incorrect zero padding in format_log_message

[Mark Michelson]

Acked-by: Mark Michelson 
Tested-by: Mark Michelson 

On 02/23/2018 09:30 PM, zhangliping wrote:

From: zhangliping 

If the format specifier does not have the 0 flag, we should pad with
blanks instead of zeroes.

Signed-off-by: zhangliping 
---
  lib/vlog.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vlog.c b/lib/vlog.c
index f28695043..0e862a773 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -952,7 +952,7 @@ format_log_message(const struct vlog_module *module, enum 
vlog_level level,
  for (p = pattern; *p != '\0'; ) {
  const char *subprogram_name;
  enum { LEFT, RIGHT } justify = RIGHT;
-int pad = '0';
+int pad = ' ';
  size_t length, field, used;
  
  if (*p != '%') {




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

Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages

[Mark Michelson]

On 02/23/2018 06:07 PM, Ben Pfaff wrote:

On Thu, Feb 15, 2018 at 04:49:57PM -0600, Mark Michelson wrote:

The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
their "Database Commands" section when it comes to referring to what
database tables exist. This commit amends this by making each *ctl
manpage reference the corresponding database manpage instead.

To aid in having a more handy list, the --help text of ovn-nbctl,
ovn-sbctl, and ovs-vsctl have been modified to list the available
tables. This is also referenced in the manpages for those applications.

Signed-off-by: Mark Michelson 


Thanks.  I was hoping for more explanation in the list of tables for how
users can refer to the tables.  Here is a version that adds more
information.  What do you think?


Thanks for the update Ben. I like listing methods of referring to 
records in each table. I found the formatting to be a bit overwhelming 
in some cases. For instance, from `ovn-nbctl -h` the DHCP_Options table 
has this text:


  DHCP_Options: by UUID, via "dhcpv4_options" of Logical_Switch_Port 
with matching "name", via "dhcpv4_options" of Logical_Switch_Port with 
matching "external_ids:neutron:port_name", via "dhcpv6_options" of 
Logical_Switch_Port with matching "name", or via "dhcpv6_options" of 
Logical_Switch_Port with matching "external_ids:neutron:port_name"


Keep in mind that there is no column-limiting on this, so it stretches 
across the entire terminal. It makes it hard to visually parse the 
options. I made a adjustment to the formatting so that it looks more 
like this:


  DHCP_Options:
by UUID
via "dhcpv4_options" of Logical_Switch_Port with matching "name"
via "dhcpv4_options" of Logical_Switch_Port with matching 
"external_ids:neutron:port_name"

via "dhcpv6_options" of Logical_Switch_Port with matching "name"
via "dhcpv6_options" of Logical_Switch_Port with matching 
"external_ids:neutron:port_name"


Separating the methods by line makes it easier to read IMHO. Here's the 
change


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

From: Mark Michelson 
Date: Mon, 26 Feb 2018 08:28:49 -0600
Subject: [PATCH] Refer to database manpages in *ctl manpages

The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
their "Database Commands" section when it comes to referring to what
database tables exist. This commit amends this by making each *ctl
manpage reference the corresponding database manpage instead.

To aid in having a more handy list, the --help text of ovn-nbctl,
ovn-sbctl, and ovs-vsctl have been modified to list the available
tables. This is also referenced in the manpages for those applications.

Signed-off-by: Mark Michelson 
Signed-off-by: Ben Pfaff 
Please enter the commit message for your changes. Lines starting
---
 lib/db-ctl-base.c | 69 
---

 lib/db-ctl-base.h | 14 -
 ovn/lib/ovn-util.h|  5 
 ovn/utilities/ovn-nbctl.8.xml | 61 +++---
 ovn/utilities/ovn-nbctl.c |  5 ++--
 ovn/utilities/ovn-sbctl.8.in  |  5 ++--
 ovn/utilities/ovn-sbctl.c |  6 ++--
 utilities/ovs-vsctl.8.in  | 51 ++--
 utilities/ovs-vsctl.c |  7 +++--
 vtep/vtep-ctl.c   |  7 +++--
 10 files changed, 103 insertions(+), 127 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 9fec6fa0d..ae7b6cde2 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -35,6 +35,7 @@
 #include "ovsdb-idl-provider.h"
 #include "openvswitch/shash.h"
 #include "sset.h"
+#include "svec.h"
 #include "string.h"
 #include "table.h"
 #include "util.h"
@@ -61,6 +62,9 @@ static const struct cmd_show_table *cmd_show_tables;
 static void (*ctl_exit_func)(int status) = NULL;
 OVS_NO_RETURN static void ctl_exit(int status);

+/* IDL class. */
+static const struct ovsdb_idl_class *idl_class;
+
 /* Two arrays with 'n_classes' elements, which represent the tables in 
this

  * database and how the user can refer to their rows. */
 static const struct ctl_table_class *ctl_classes;
@@ -2132,15 +2136,15 @@ ctl_register_commands(const struct 
ctl_command_syntax *commands)


 /* Registers the 'db_ctl_commands' to 'all_commands'. */
 void
-ctl_init__(const struct ovsdb_idl_table_class *idl_classes_,
+ctl_init__(const struct ovsdb_idl_class *idl_class_,
const struct ctl_table_class *ctl_classes_,
-   size_t n_classes_,
const struct cmd_show_table cmd_show_tables_[],
void (*ctl_exit_func_)(int status))
 {
-idl_classes = idl_classes_;
+idl_class = idl_class_;
+idl_classes = idl_class_->tables;
 ctl_classes = ctl_classes_;
-n_classes = n_classes_;
+n_classes = idl_class->n_tables;
 ctl_exit_func = ctl_exit_func_;
 ctl_register_commands(db_ctl_commands);

@@ -2170,6 +2174,63 @@ 

Re: [ovs-dev] [PATCH] Use the correct logical datapath UUID as input for logical flow hash.

[Jakub Sitnicki]

On Fri, Feb 23, 2018 at 09:05 PM GMT, Ben Pfaff wrote:
> On Thu, Feb 22, 2018 at 10:54:05PM +0100, Jakub Sitnicki wrote:
>> Use the logical switch/router UUID as hash input, instead of the UUID of
>> the Datapath_Binding row, when calculating the hash value for lflows in
>> the SBDB.
>>
>> Otherwise the hash value will never match the one computed from NBDB
>> contents, which will force ovn-northd to constantly drop and attempt to
>> re-insert all the lflows.
>
> Thanks a lot for analyzing the problem and fixing it.
>
> There are basically two options here: use Datapath_Binding UUID
> consistently or use Logical_Switch/Logical_Router UUID consistently.
> This patch takes the latter approach.  I'd prefer to take the former
> approach because it is less dependent on the structure of the northbound
> database (which seems desirable for something that is in the southbound
> database) and because it doesn't rely on the external-ids being correct.
>
> I sent a commit that takes the Datapath_Binding UUID approach:
> https://patchwork.ozlabs.org/patch/877306/
>
> What do you think?

In general - works for me. I'm wondering about a couple of things:

1) build_datapaths() -> join_datapaths() detects and removes duplicated
   datapath bindings, AFAIU. I'm not sure how we can end up with
   duplicated bindings but if it happens, can it lead to removal and
   re-insertion of logical flows for a datapath?

2) If I wanted to precalculate an lflow hash and cache it in a synthetic
   column in NB DB, ACLs being the potential candidate for that, then it
   won't be possible with this approach. That said, I don't know ATM if
   this would give a significant gain.

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

[ovs-dev] [PATCH branch-2.8] tunnel: Fix deletion of datapath tunnel ports in case of reconfiguration

[Balazs Nemeth]

There is an issue in OVS with tunnel deletion during the
reconfiguration of OF tunnels. If the dst_port value is changed, the
old tunnel map entry will not be deleted, because the tp_port
argument of tnl_port_map_delete() has the new dst_port setting, hence
the tunnel cannot be found in the list of tnl_port structures.

The patch corrects this mechanism by adding a new argument,
'old_odp_port' to tnl_port_reconfigure(). This value is used to
identify the datapath tunnel port which is being reconfigured. In
connection with this fix, to unify the tunnel port map handling,
odp_port value is used to search the proper port to insert and delete
tunnel map entries as well. This variable can be used instead of
tp_port, as it is unique for all datapath tunnel ports, and there is
no need to reach dst_port from netdev_tunnel_config structure.

This patch also adds a printout to check the reference counter of
a tnl_port structure in tnl-port.c. Extending OVS unit test cases to
have ref_cnt values in the expected dump. Adding new test cases to
check if packet receiving is still working in the case of OF tunnel
port deletion. Adding new test cases to check the reference counter
in case of OF tunnel deletion or reconfiguration.

Signed-off-by: Balazs Nemeth 
Signed-off-by: Jan Scheurich 
Co-authored-by: Jan Scheurich 
---
 lib/tnl-ports.c   |  9 +
 lib/tnl-ports.h   |  4 ++--
 ofproto/ofproto-dpif.c|  8 +---
 ofproto/tunnel.c  | 37 -
 ofproto/tunnel.h  |  7 ---
 tests/tunnel-push-pop-ipv6.at | 33 ++---
 tests/tunnel-push-pop.at  | 35 ---
 7 files changed, 98 insertions(+), 35 deletions(-)

diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 777ed4d..04d2b3f 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -195,7 +195,7 @@ tnl_port_map_insert(odp_port_t port, ovs_be16 tp_port,

 ovs_mutex_lock();
 LIST_FOR_EACH(p, node, _list) {
-if (tp_port == p->tp_port && p->nw_proto == nw_proto) {
+if (p->port == port && p->nw_proto == nw_proto) {
 ovs_refcount_ref(>ref_cnt);
 goto out;
 }
@@ -255,7 +255,7 @@ ipdev_map_delete(struct ip_device *ip_dev, ovs_be16 
tp_port, uint8_t nw_proto)
 }

 void
-tnl_port_map_delete(ovs_be16 tp_port, const char type[])
+tnl_port_map_delete(odp_port_t port, const char type[])
 {
 struct tnl_port *p, *next;
 struct ip_device *ip_dev;
@@ -265,7 +265,7 @@ tnl_port_map_delete(ovs_be16 tp_port, const char type[])

 ovs_mutex_lock();
 LIST_FOR_EACH_SAFE(p, next, node, _list) {
-if (p->tp_port == tp_port && p->nw_proto == nw_proto &&
+if (p->port == port && p->nw_proto == nw_proto &&
 ovs_refcount_unref_relaxed(>ref_cnt) == 1) {
 ovs_list_remove(>node);
 LIST_FOR_EACH(ip_dev, node, _list) {
@@ -348,7 +348,8 @@ tnl_port_show(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 }

 LIST_FOR_EACH(p, node, _list) {
-ds_put_format(, "%s (%"PRIu32")\n", p->dev_name, p->port);
+ds_put_format(, "%s (%"PRIu32") ref_cnt=%u\n", p->dev_name, p->port,
+  ovs_refcount_read(>ref_cnt));
 }

 out:
diff --git a/lib/tnl-ports.h b/lib/tnl-ports.h
index 58b048a..61ca0f8 100644
--- a/lib/tnl-ports.h
+++ b/lib/tnl-ports.h
@@ -26,10 +26,10 @@

 odp_port_t tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc);

-void tnl_port_map_insert(odp_port_t port, ovs_be16 udp_port,
+void tnl_port_map_insert(odp_port_t, ovs_be16 tp_port,
  const char dev_name[], const char type[]);

-void tnl_port_map_delete(ovs_be16 udp_port, const char type[]);
+void tnl_port_map_delete(odp_port_t, const char type[]);
 void tnl_port_map_insert_ipdev(const char dev[]);
 void tnl_port_map_delete_ipdev(const char dev[]);
 void tnl_port_map_run(void);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b37630f..2481859 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -378,6 +378,7 @@ type_run(const char *type)
 HMAP_FOR_EACH (iter, up.hmap_node, >up.ports) {
 char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
 const char *dp_port;
+odp_port_t old_odp_port;

 if (!iter->is_tunnel) {
 continue;
@@ -385,6 +386,7 @@ type_run(const char *type)

 dp_port = netdev_vport_get_dpif_port(iter->up.netdev,
  namebuf, sizeof namebuf);
+old_odp_port = iter->odp_port;
 node = simap_find(_backers, dp_port);
 if (node) {
 simap_put(>tnl_backers, dp_port, node->data);
@@ -406,7 +408,7 @@ type_run(const char *type)

 iter->odp_port = node ? 

[ovs-dev] [PATCH branch-2.8] tests: Make packet-type-aware.at hash independent

[Balazs Nemeth]

When compiling with -msse4.2 a test case of packet-type-aware.at will
fail due to the CRC32 based hash function is different from mhash.
Fix this issue with parsing the port statistics one-by-one.

Signed-off-by: Balazs Nemeth 
CC: Jan Scheurich 
CC: Zoltan Balogh 
Fixes: 0d87cb5b6757 ("xlate: fix xport lookup for recirc")
---
 tests/packet-type-aware.at | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
index f43095c..c289bb4 100644
--- a/tests/packet-type-aware.at
+++ b/tests/packet-type-aware.at
@@ -1027,10 +1027,15 @@ 
tunnel(src=20.0.0.2,dst=20.0.0.1,flags(-df-csum)),recirc_id(0x1),in_port(gre_sys
 ovs-appctl time/warp 1000

 AT_CHECK([
-ovs-ofctl dump-ports int-br
-], [0], [OFPST_PORT reply (xid=0x2): 2 ports
+ovs-ofctl dump-ports int-br | grep -A1 LOCAL:
+], [0], [dnl
   port LOCAL: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
tx pkts=4, bytes=392, drop=?, errs=?, coll=?
+])
+
+AT_CHECK([
+ovs-ofctl dump-ports int-br | grep -A1 2:
+], [0], [dnl
   port  2: rx pkts=4, bytes=352, drop=?, errs=?, frame=?, over=?, crc=?
tx pkts=0, bytes=0, drop=?, errs=?, coll=?
 ])
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ofproto-dpif-upcall: Fix using uninitialized fitness.

[Ilya Maximets]

'upcall_xlate()' makes a decision to compose slow path actions
by checking the 'upcall->fitness', which is not initialized in
case of calling from the 'upcall_cb()'.

'upcall_cb()' receives the real flow, so the fitness should be
initialized as perfect.

Fixes following tests on travis:

ofproto-dpif.at: ofproto-dpif megaflow - disabled - pmd
ofproto-dpif.at: ofproto-dpif - conntrack - output action

CC: Ben Pfaff 
Fixes: 687bafbb8a79 ("ofproto-dpif-upcall: Slow path flows that
  datapath can't fully match.")
Signed-off-by: Ilya Maximets 
---

Version 2:
* Struct initializer replaced with a simple assignment
  for performance reasons. (Ben Pfaff)

 ofproto/ofproto-dpif-upcall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5eb20f7..23e459b 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1266,6 +1266,7 @@ upcall_cb(const struct dp_packet *packet, const struct 
flow *flow, ovs_u128 *ufi
 return error;
 }
 
+upcall.fitness = ODP_FIT_PERFECT;
 error = process_upcall(udpif, , actions, wc);
 if (error) {
 goto out;
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix using uninitialized fitness.

[Ilya Maximets]

On 24.02.2018 01:13, Ben Pfaff wrote:
> On Wed, Feb 21, 2018 at 03:09:00PM +0300, Ilya Maximets wrote:
>> 'upcall_xlate()' makes a decision to compose slow path actions
>> by checking the 'upcall->fitness', which is not initialized in
>> case of calling from the 'upcall_cb()'.
>>
>> 'upcall_cb()' receives the real flow, so the fitness should be
>> initialized as perfect.
>>
>> Fixes following tests on travis:
>>
>> ofproto-dpif.at: ofproto-dpif megaflow - disabled - pmd
>> ofproto-dpif.at: ofproto-dpif - conntrack - output action
>>
>> CC: Ben Pfaff 
>> Fixes: 687bafbb8a79 ("ofproto-dpif-upcall: Slow path flows that
>>   datapath can't fully match.")
>> Signed-off-by: Ilya Maximets 
> 
> Thanks a lot for the fix.
> 
> "struct upcall" is a large data structure (almost 2 kB on x86-32), so
> this one-line fix is going to be very expensive.  To avoid a performance
> regression, I suggest using an assignment statement rather than an
> initializer, which necessarily initializes the entire structure.

Good point. Thanks.

> 
> Would you mind respinning it that way?

Sure, I'll send v2 with this change soon.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev