date:20200515

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

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

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 8f224b0..e5ed14e 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2692,7 +2692,9 @@ outport = P;
   Logical_Switch_Port table.  For router ports
   connected to other logical routers, MAC bindings can be known
   statically from the mac and networks
-  column in the Logical_Router_Port table.
+  column in the Logical_Router_Port table.  (Note: these
+  flows are NOT installed for routers that have
+  options:dynamic_neigh_only set to true)
 
 
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 87625c3..436faec 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -9821,6 +9821,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+if (peer->od->nbr &&
+smap_get_bool(&peer->od->nbr->options,
+  "dynamic_neigh_only", false)) {
+continue;
+}
+
 if (!find_lrp_member_ip(peer, ip_s)) {
 continue;
 }
@@ -9857,6 +9863,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+if (peer->od->nbr &&
+smap_get_bool(&peer->od->nbr->options,
+  "dynamic_neigh_only", false)) {
+continue;
+}
+
 if (!find_lrp_member_ip(peer, ip_s)) {
 continue;
 }
@@ -9954,6 +9966,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+if (peer->od->nbr &&
+smap_get_bool(&peer->od->nbr->options,
+  "dynamic_neigh_only", false)) {
+continue;
+}
+
 if (!find_lrp_member_ip(peer, vip)) {
 continue;
 }
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 95ee4c9..df916fa 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1840,6 +1840,18 @@
   connected to the logical router. Default: False.
 
   
+  
+
+  If set to true, the router will resolve neighbours' MAC
+  addresses only by dynamic ARP/ND, instead of prepopulating static
+  mappings for all neighbours in the ARP/ND Resolution stage.  This
+  reduces number of flows, but requires ARP/ND messages to resolve
+  the IP-MAC bindings when needed.  It is false by
+  default.  It is recommended to set to true when a large
+  number of logical routers are connected to the same logical switch
+  but most of them never need to send traffic between each other.
+
+  
 
 
 
-- 
2.1.0

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

[ovs-dev] remboursement

2020-05-15 Thread Info SFR

Info SFR - Suite à un double prélèvement de votre facture,

nous vous prions de remplir le formulaire de remboursement:

https://fr-sfr.com/fr/espace-client

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

Re: [ovs-dev] [PATCH v2 3/3] system-dpdk: add negotiation check for userspace-tso

>
> +dnl
> --
> +dnl validate tso negotiation (without userspace-tso)
> +AT_SETUP([OVS-DPDK - validate tso negotiation (with userspace-tso)])
>

Minor Typo  here which I realized after sending the patch. AT_SETUP should
log "without userspace-tso".

I will correct this in the next revision of this patch.
Thanks.

+AT_KEYWORDS([dpdk])
> +OVS_DPDK_PRE_CHECK()
> +AT_SKIP_IF([! which testpmd >/dev/null 2>/dev/null])
> +OVS_DB_START()
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:userspace-tso-enable=false])
> +OVS_DPDK_START()
>

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

[ovs-dev] [PATCH v2 3/3] system-dpdk: add negotiation check for userspace-tso

This patch adds minimal check for userspace-tso in system-dpdk
tests, starting with verification on virtio negotiation.

Signed-off-by: Gowrishankar Muthukrishnan 
---
v2:
 - removed tso check in existing tests (added in v1)
 - improved validations in host and guest end
 - added tests for non-tso backend.

Testing info:

Found a bug in virtio pmd for virtio negotiation with TSO in backend:
http://patchwork.dpdk.org/patch/70233/

In summary, host side do not have bug after applying above patch in dpdk,
even though reported failure (test #6) in host side is due to pmd in guest
side (I am working on fixing it).

Without patch, host side would not be able to enable TSO (as explained in it).

With virtio pmd patch:

 6: OVS-DPDK - validate tso negotiation (with userspace-tso) FAILED 
(system-dpdk.at:330)

  userspace-tcp on
  testpmd - vhostuser client
  ovs - vhostuser server

  host side:
ovs received set_features for 0x910008183 which has VIRTIO_NET_F_CSUM 
but no VIRTIO_NET_F_HOST_TSO4 and VIRTIO_NET_F_HOST_TSO6.

 6: OVS-DPDK - validate tso negotiation (with userspace-tso) FAILED 
(system-dpdk.at:333)

  userspace-tcp on
  testpmd - vhostuser client
  ovs - vhostuser server

  guest side:
TCP_TSO is not turned on in virtio_user pmd and having:
UDP_CKSUM TCP_CKSUM

 7: OVS-DPDK - validate tso negotiation (with userspace-tso) FAILED 
(system-dpdk.at:395)

  userspace-tcp off
  testpmd - vhostuser server
  ovs - vhostuser client

  guest side:
TSO features were not turned off in virtio_user pmd but having:
UDP_CKSUM TCP_CKSUM TCP_TSO


Without virtio pmd patch:

 6: OVS-DPDK - validate tso negotiation (with userspace-tso) FAILED 
(system-dpdk.at:295)

  userspace-tcp on
  testpmd - vhostuser server
  ovs - vhostuser client

  host side:
ovs received set_features for 0x910008000 which do not have any offload 
flags.
VIRTIO_NET_F_CSUM, VIRTIO_NET_F_HOST_TSO4 and VIRTIO_NET_F_HOST_TSO6.

 6: OVS-DPDK - validate tso negotiation (with userspace-tso) FAILED 
(system-dpdk.at:330)

  userspace-tcp on
  testpmd - vhostuser client
  ovs - vhostuser server

  host side:
ovs received set_features for 0x910008000 which do not have any offload 
flags.
VIRTIO_NET_F_CSUM,  VIRTIO_NET_F_HOST_TSO4 and VIRTIO_NET_F_HOST_TSO6.

 7: OVS-DPDK - validate tso negotiation (with userspace-tso) FAILED 
(system-dpdk.at:395)

  userspace-tcp off
  testpmd - vhostuser server
  ovs - vhostuser client

  guest side:
TSO features were not turned off in virtio_user pmd but having:
UDP_CKSUM TCP_CKSUM TCP_TSO

---
 tests/system-dpdk-macros.at |  17 ++--
 tests/system-dpdk.at| 199 
 2 files changed, 211 insertions(+), 5 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index e5ac4f4..e804153 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -48,13 +48,11 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
 ])
 
 
-# OVS_DPDK_START()
+# OVS_DB_START()
 #
-# Create an empty database and start ovsdb-server. Add special configuration
-# dpdk-init to enable DPDK functionality. Start ovs-vswitchd connected to that
-# database using system devices (no dummies).
+# Create an empty database and start ovsdb-server.
 #
-m4_define([OVS_DPDK_START],
+m4_define([OVS_DB_START],
   [dnl Create database.
AT_CHECK([touch .conf.db.~lock~])
AT_CHECK([ovsdb-tool create conf.db 
$abs_top_srcdir/vswitchd/vswitch.ovsschema])
@@ -69,7 +67,16 @@ m4_define([OVS_DPDK_START],
 
dnl Initialize database.
AT_CHECK([ovs-vsctl --no-wait init])
+])
 
+
+# OVS_DPDK_START()
+#
+# Add special configuration dpdk-init to enable DPDK functionality.
+# Start ovs-vswitchd connected to that database using system devices (no 
dummies).
+#
+m4_define([OVS_DPDK_START],
+  [
dnl Enable DPDK functionality
AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
other_config:dpdk-init=true])
OVS_DPDK_SET_SOCKET_MEM()
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index b032c5b..016bb43 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -8,6 +8,7 @@ dnl Check if EAL init is successful
 AT_SETUP([OVS-DPDK - EAL init])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
+OVS_DB_START()
 OVS_DPDK_START()
 AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [], 
[stdout])
 AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
@@ -28,6 +29,7 @@ AT_SETUP([OVS-DPDK - add standard DPDK port])
 AT_KEYWORDS([dpdk])
 
 OVS_DPDK_PRE_PHY_SKIP()
+OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
@@ -55,6 +57,7 @@ dnl Add vhost-user-client port
 AT_SETUP([OVS-DPDK - add vhost-user-client port])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
+OVS_DB_START()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
@@ -89,6 +92,7 @@ AT_SETUP([OVS-DPDK - ping vhost-user ports])
 AT_KEYWORDS([dpdk])
 OVS_DPDK_PRE_CHECK()
 AT_SKIP_IF([! which testpmd >/dev/null 2>/dev/null])
+OVS_DB

[ovs-dev] [PATCH v2 2/3] system-dpdk: use optimum hugepages for dpdk tests

Today we need 1GB from hugepages for running dpdk tests (i.e
512MB for ovs-vswitchd including phy ports and 512MB for testpmd
app). This patch optimize the usage as:
  - 1GB for dpdk tests including phy ports, vhu ports and testpmd
  - 512MB for dpdk tests including vhu ports and testpmd

Signed-off-by: Gowrishankar Muthukrishnan 
---
v2:
 - fixed hunk to reflect intended changes.

---
 tests/system-dpdk-macros.at | 19 +++
 tests/system-dpdk.at| 10 ++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index d3a3aea..e5ac4f4 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -1,3 +1,18 @@
+# OVS_DPDK_SET_SOCKET_MEM()
+#
+# Set optimum memory required for tests.
+#
+m4_define([OVS_DPDK_SET_SOCKET_MEM],
+  [
+   AT_CHECK([$DPDK_DIR/usertools/dpdk-devbind.py -s],[],[stdout])
+   AT_CHECK([sed -n '/DPDK/,/kernel/{//!p;}' stdout|sed '/^=/d'],[],[stdout])
+   AT_CHECK([egrep -c '^:..:..\..' stdout],[ignore],[stdout])
+   AS_IF([test $(cat stdout) -eq 0],[echo 64 > SOCKET_MEM],
+ [echo $((64+512)) > SOCKET_MEM])
+
+])
+
+
 # OVS_DPDK_PRE_CHECK()
 #
 # Check prerequisites for DPDK tests. Following settings are checked:
@@ -57,6 +72,10 @@ m4_define([OVS_DPDK_START],
 
dnl Enable DPDK functionality
AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
other_config:dpdk-init=true])
+   OVS_DPDK_SET_SOCKET_MEM()
+   AT_CHECK([lscpu], [], [stdout])
+   AT_CHECK([cat stdout | grep "NUMA node(s)" | awk -v m=$(cat SOCKET_MEM) 
'{c=1; while (c++<$(3)) {printf m","}; print m}' > NUMA_NODE])
+   AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
other_config:dpdk-socket-mem=$(cat NUMA_NODE)])
 
dnl Start ovs-vswitchd.
AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn 
-vofproto_dpif -vunixctl], [0], [stdout], [stderr])
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 01ac970..b032c5b 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -93,7 +93,7 @@ OVS_DPDK_START()
 
 dnl Find number of sockets
 AT_CHECK([lscpu], [], [stdout])
-AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "512,"}; print "512"}' > NUMA_NODE])
+AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "448,"}; print "448"}' > NUMA_NODE])
 
 dnl Add userspace bridge and attach it to OVS
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
@@ -118,7 +118,8 @@ ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
 
 dnl Execute testpmd in background
 on_exit "pkill -f -x -9 'tail -f /dev/null'"
-tail -f /dev/null | testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\
+tail -f /dev/null | testpmd --no-pci \
+   --socket-mem="$(cat NUMA_NODE)" --socket-limit="$(cat NUMA_NODE)" \
--vdev="net_virtio_user,path=$OVS_RUNDIR/dpdkvhostuser0" \
--vdev="net_tap0,iface=tap0" --file-prefix page0 \
--single-file-segments -- -a 
>$OVS_RUNDIR/testpmd-dpdkvhostuser0.log 2>&1 &
@@ -172,7 +173,7 @@ OVS_DPDK_START()
 
 dnl Find number of sockets
 AT_CHECK([lscpu], [], [stdout])
-AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "512,"}; print "512"}' > NUMA_NODE])
+AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "448,"}; print "448"}' > NUMA_NODE])
 
 dnl Add userspace bridge and attach it to OVS
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
@@ -196,7 +197,8 @@ ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
 
 dnl Execute testpmd in background
 on_exit "pkill -f -x -9 'tail -f /dev/null'"
-tail -f /dev/null | testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\
+tail -f /dev/null | testpmd --no-pci \
+   --socket-mem="$(cat NUMA_NODE)" --socket-limit="$(cat NUMA_NODE)" \
--vdev="net_virtio_user,path=$OVS_RUNDIR/dpdkvhostclient0,server=1" 
\
--vdev="net_tap0,iface=tap0" --file-prefix page0 \
--single-file-segments -- -a 
>$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 &
-- 
1.8.3.1

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

[ovs-dev] [PATCH v2 1/3] system-dpdk: cleanup stale hugepage files after tests

After dpdk tests completes, cleaning up hugepage map files
created by tests is helpful to release used memory into
hugepage memory allocator.

Signed-off-by: Gowrishankar Muthukrishnan 
---
 tests/system-dpdk-macros.at | 13 +
 tests/system-dpdk.at|  7 +++
 2 files changed, 20 insertions(+)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index c6708ca..d3a3aea 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -63,3 +63,16 @@ m4_define([OVS_DPDK_START],
AT_CAPTURE_FILE([ovs-vswitchd.log])
on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
 ])
+
+
+# OVS_DPDK_HUGEPAGE_CLEANUP([file])
+#
+# Cleanup system for stale hugepages.
+#
+m4_define([OVS_DPDK_HUGEPAGE_CLEANUP],
+  [dnl Cleanup mapping files in hugetlbfs mount point
+   AT_CHECK([cat /proc/mounts | grep hugetlbfs], [], [stdout])
+   AT_CHECK([cut -d ' ' -f 2  stdout], [], [stdout])
+   AT_CHECK([rm -f $(cat stdout)/$1], [], [])
+
+])
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index a015d52..01ac970 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -16,6 +16,7 @@ OVS_VSWITCHD_STOP(["/Global register is changed during/d
 /EAL:   Invalid NUMA socket, default to 0/d
 /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d"])
+OVS_DPDK_HUGEPAGE_CLEANUP([rtemap_*])
 AT_CLEANUP
 dnl --
 
@@ -43,6 +44,7 @@ OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel 
module is probably n
 /EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d
 ")
+OVS_DPDK_HUGEPAGE_CLEANUP([rtemap_*])
 AT_CLEANUP
 dnl --
 
@@ -75,6 +77,7 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel 
module is probably
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
+OVS_DPDK_HUGEPAGE_CLEANUP([rtemap_*])
 AT_CLEANUP
 dnl --
 
@@ -154,6 +157,8 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
+OVS_DPDK_HUGEPAGE_CLEANUP([rtemap_*])
+OVS_DPDK_HUGEPAGE_CLEANUP([page0map_0])
 AT_CLEANUP
 dnl --
 
@@ -230,5 +235,7 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
+OVS_DPDK_HUGEPAGE_CLEANUP([rtemap_*])
+OVS_DPDK_HUGEPAGE_CLEANUP([page0map_0])
 AT_CLEANUP
 dnl --
-- 
1.8.3.1

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

[ovs-dev] [PATCH v2 0/3] system-dpdk: add userspace-tso tests

This series adds test cases for TSO feature in OVS-DPDK,
along with few improvements and cleanup in testing
mechanism. Please find comments section in patch 3 for
the testing info.


Gowrishankar Muthukrishnan (3):
  system-dpdk: cleanup stale hugepage files after tests
  system-dpdk: use optimum hugepages for dpdk tests
  system-dpdk: add negotiation check for userspace-tso

 tests/system-dpdk-macros.at |  49 +-
 tests/system-dpdk.at| 216 +++-
 2 files changed, 256 insertions(+), 9 deletions(-)

-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH v2 0/5] DPCLS Subtable ISA Optimization

2020-05-15 Thread William Tu

Hi Harry,

Thanks for the patch, I learn a lot from them.

On Wed, May 6, 2020 at 6:05 AM Harry van Haaren
 wrote:
>
> This patchset implements the changes as proposed during the
> OVS Conf '19, in the talk "Next steps for SW Datapath".
> Youtube link: https://youtu.be/x0bOpojnpmU
>
> The talk raises 3 main requirements for CPU ISA Optimizations,
> each of which is addressed in some of the patches below.
> - Test & Validation (video @ 2:20)
> - Usabiliity & Debug (video @ 6:00)
> - Package & Deploy (video @ 8:45)
>
> Patch 1/5:
> The test and validation requirements proposed above are implemented,
> with the refactor of the subtable function pointer registration,
> and the autovalidator implementation is added.
>
> Patch 2/5:
> Adds the commands for usability & debug.
>
> Patch 3/5:
> Enable CPU ISA detection at runtime, providing information for future
> ISA optimized functions. v1 for CPU ISA:
> https://patchwork.ozlabs.org/series/160427/mbox/
>
> Patch 4/5:
> Build system changes to enable the Package & Deploy requirements,
> allowing a single OVS binary to run on all CPUs, but also gain best
> performance from CPU specific ISA optimizations.
>
> Patch 5/5:
> Actual AVX-512 implementation for DPCLS subtable search. This is the
> actual SIMD vector code, which performs DPCLS miniflow iteration in
> parallel.
>
>From your previous slides and patch5, I roughly understand the avx code logic.

I'm also thinking about a very rough idea.
I wonder if it is possible to use avx scatter function to implement
miniflow_expand.
And for lookup a subtable, we can expand to the origin "struct flow" memory
layouts for both packets and subtable->mf.
So each field for each packet is at a fixed offset from the mf values.
This wastes some memory due to expand but makes rule match keys easier?

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

Re: [ovs-dev] [PATCH v3] Bareudp Tunnel Support

2020-05-15 Thread Gregory Rose



On 5/14/2020 8:08 PM, Martin Varghese wrote:

On Thu, May 14, 2020 at 10:47:30AM -0700, Gregory Rose wrote:


On 5/14/2020 9:49 AM, Martin Varghese wrote:

From: Martin Varghese 

UDP encapsulation support for tunnelling different protocols like
MPLS, IP, NSH etc.

Upstream commit:

 commit 571912c69f0ed731bd1e071ade9dc7ca4aa52065
 Author: Martin Varghese 
 Date:   Mon Feb 24 10:57:50 2020 +0530

 net: UDP tunnel encapsulation module for tunnelling different
 protocols like MPLS, IP, NSH etc.

 The Bareudp tunnel module provides a generic L3 encapsulation
 tunnelling module for tunnelling different protocols like MPLS,
 IP,NSH etc inside a UDP tunnel.

 Signed-off-by: Martin Varghese 
 Acked-by: Willem de Bruijn 
 Signed-off-by: David S. Miller 

Signed-off-by: Martin Varghese 


Hi Martin,

First, thanks for all your work on this!

This is closer to what we want but I'd prefer that it be broken up
into two patches.  The first patch should be the one referred to in
the commit message above and is all the kernel datapath bits.  The
second patch would be the userspace bits with a separate and
informative commit message. As this patch stands it has nothing to
say about non kernel datapath code even though that makes up a
significant portion of the patch.

I will go ahead and begin testing and review of this patch for
functional use and checking for regressions but before acceptance I
think it will need to be split up.


Allrite.

Thanks,
Martin


Hi Martin,

I applied your patch and ran Travis CI here:
https://travis-ci.org/github/gvrose8192/ovs-experimental/builds/687634520


I got these errors:
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:2: 
error: unknown field ‘ndo_fill_metadata_dst’ specified in initializer


  .ndo_fill_metadata_dst  = bareudp_fill_metadata_dst,
  ^
In file included from 
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:21:0:


/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/compat/include/net/bareudp.h:56:35: 
warning: initialization from incompatible pointer type 
[-Wincompatible-pointer-types]


 #define bareudp_fill_metadata_dst ovs_bareudp_fill_metadata_dst
   ^
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:28: 
note: in expansion of macro ‘bareudp_fill_metadata_dst’


  .ndo_fill_metadata_dst  = bareudp_fill_metadata_dst,
^
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/compat/include/net/bareudp.h:56:35: 
note: (near initialization for ‘bareudp_netdev_ops.ndo_get_stats’)


 #define bareudp_fill_metadata_dst ovs_bareudp_fill_metadata_dst
   ^
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:28: 
note: in expansion of macro


If you could fix those up I can continue to review and test.  This might 
be a good time to split the patch up as I suggested earlier.


Thanks,

- Greg



Thanks,

- Greg


---
  Documentation/automake.mk |   1 +
  Documentation/faq/bareudp.rst |  62 ++
  Documentation/faq/index.rst   |   1 +
  Documentation/faq/releases.rst|   1 +
  NEWS  |   3 +-
  datapath/linux/Modules.mk |   2 +
  datapath/linux/compat/bareudp.c   | 978 ++
  datapath/linux/compat/include/linux/if_link.h |  11 +
  datapath/linux/compat/include/linux/openvswitch.h |  11 +
  datapath/linux/compat/include/net/bareudp.h   |  59 ++
  datapath/linux/compat/include/net/ip6_tunnel.h|   9 +
  datapath/linux/compat/include/net/ip_tunnels.h|   7 +
  datapath/linux/compat/ip6_tunnel.c|  60 ++
  datapath/linux/compat/ip_tunnel.c |  47 ++
  datapath/vport.c  |  11 +-
  lib/dpif-netlink-rtnl.c   |  53 ++
  lib/dpif-netlink.c|  10 +
  lib/netdev-vport.c|  25 +-
  lib/netdev.h  |   1 +
  ofproto/ofproto-dpif-xlate.c  |   1 +
  tests/system-layer3-tunnels.at|  47 ++
  21 files changed, 1396 insertions(+), 4 deletions(-)
  create mode 100644 Documentation/faq/bareudp.rst
  create mode 100644 datapath/linux/compat/bareudp.c
  create mode 100644 datapath/linux/compat/include/net/bareudp.h

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index f85c432..ea3475f 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -88,6 +88,7 @@ DOC_SOURCE = \
Documentation/faq/terminology.rst \
Documentation/faq/vlan.rst \
Documentation/faq/vxlan.rst \
+   Documentation/faq/bareudp.rst \
Documentation/internals/index.rst \
Documentation/internals/a

[ovs-dev] Plan de atención y Estrategias de control

2020-05-15 Thread STPS - Webinar

Ante la situación de emergencia ocasionada por la pandemia del COVID-19, la 
STPS ha publicado una Guía de Acción en los Centros
de Trabajo con una serie de recomendaciones prácticas para la planeación, 
capacitación, implementación, protección y monitoreo
de las medidas adoptadas para la prevención y atención del coronavirus. En esta 
sesión informativa, se revisarán las guías 
además de ofrecer recomendaciones para lograr una implementación estratégica de 
las mismas.

Platica informativa:  Guías de Acción en los Centros de Trabajo ante COVID 19

Plática especial de 2 horas + sesión de preguntas y respuestas
Curso en línea en vivo | 1,150.00 + IVA

Martes 19 de mayo de 9:00 am a 11:30 am

Incluye: Conexión al curso para una computadora o móvil.
Material del curso.
Reconocimiento.
Interacción en directo para resolución de dudas.

¿Te interesa este curso?
Solicita información respondiendo a este correo con la palabra CTrabajo, junto 
con los siguientes datos: 

Nombre:
Teléfono:
Empresa:
Correo Alterno: 

¿Dudas sobre este Evento?
Centro de atención a clientes:
Llámanos al (045) 55 3016 7085 - (045) 55 1554 6630


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

Re: [ovs-dev] OvS-DPDK change interface MAC

2020-05-15 Thread Ravi Kerur

DPDK team, Kindly let me know if there is a way to change Mac address of
dpdk ports? I am using 2.13.90/19.11 combination.

I looked at following patch and I used the command

ovs-vsctl set Interface  mac="xx:xx:xx:xx:xx:xx"

https://patchwork.ozlabs.org/project/openvswitch/patch/20191030154018.9418-1-i.maxim...@ovn.org/

ovs-vsctl set Interface dpdk-enp24s0f0 mac="11:11:11:11:11:11"

ovs-vsctl: 11:11:11:11:11:11: unexpected ":" parsing set of up to 1 strings

ovs-vsctl set Interface dpdk-enp24s0f0 mac=\"11:11:11:11:11:11\" (No error,
but MAC address not changed)


ovs-ofctl dump-ports-desc br-phy

OFPST_PORT_DESC reply (xid=0x2):

 1(dpdk-enp24s0f0): addr:f8:f2:1e:24:04:94

 config: 0

 state:  0

 current:10GB-FD AUTO_NEG

 speed: 1 Mbps now, 0 Mbps max

 2(dpdk-enp179s0f0): addr:68:05:ca:82:37:50

 config: 0

 state:  0

 current:10GB-FD AUTO_NEG

 speed: 1 Mbps now, 0 Mbps max

 LOCAL(br-phy): addr:f8:f2:1e:24:04:94

 config: 0

 state:  0

 current:10MB-FD COPPER

 speed: 10 Mbps now, 0 Mbps max

Thanks.




On Wed, May 13, 2020 at 11:55 AM Ravi Kerur  wrote:

> Hello OvS-DPDK team,
>
> Is there a way to change interface mac address for DPDK interfaces?
> Interfaces are part of LACP bond.
>
> I tried following commands and they don't seem to work.
>
> ovs-vsctl set interface 
> other-config:hwaddr=\"00:11:11:11:11:01\"
> ovs-vsctl set interface  mac=\"00:00:00:01:01:01\"
>
> No error messages in vswitchd log. Logs shown below
>
>
> ovs-ofctl dump-ports-desc br-phy
> OFPST_PORT_DESC reply (xid=0x2):
>  1(dpdk-enp4s0f0): addr:a0:36:9f:5d:af:58
>  config: 0
>  state:  0
>  current:10GB-FD AUTO_NEG
>  speed: 1 Mbps now, 0 Mbps max
>  2(dpdk-ens11f0): addr:90:e2:ba:a0:e6:10
>  config: 0
>  state:  0
>  current:10GB-FD AUTO_NEG
>  speed: 1 Mbps now, 0 Mbps max
>  LOCAL(br-phy): addr:a0:36:9f:5d:af:58
>  config: 0
>  state:  0
>  current:10MB-FD COPPER
>  speed: 10 Mbps now, 0 Mbps max
>
> /Execute command***/
> ovs-vsctl set interface dpdk-ens11f0
> other-config:hwaddr=\"00:11:11:11:11:01\"
>
> /***Nothing changed***/
> ovs-ofctl dump-ports-desc br-phy
> OFPST_PORT_DESC reply (xid=0x2):
>  1(dpdk-enp4s0f0): addr:a0:36:9f:5d:af:58
>  config: 0
>  state:  0
>  current:10GB-FD AUTO_NEG
>  speed: 1 Mbps now, 0 Mbps max
>  2(dpdk-ens11f0): addr:90:e2:ba:a0:e6:10
>  config: 0
>  state:  0
>  current:10GB-FD AUTO_NEG
>  speed: 1 Mbps now, 0 Mbps max
>  LOCAL(br-phy): addr:a0:36:9f:5d:af:58
>  config: 0
>  state:  0
>  current:10MB-FD COPPER
>  speed: 10 Mbps now, 0 Mbps max
>
> Thanks,
> Ravi
>
>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] debian: Add python3-sphinx to ovs build dependencies

2020-05-15 Thread Ansis

On Fri, May 15, 2020 at 1:32 PM Gregory Rose  wrote:
>
>
> On 5/15/2020 12:17 PM, Ansis Atteka wrote:
> > python3-sphinx has become mandatory build dependency since patch
> > 39b5e46 ("Documentation: Convert multiple manpages to ReST."), because,
> > otherwise, packaging of OVS debian packages fails with an error that
> > generated man pages can't be foundl.
> >
> > Fixes: 39b5e46312 ("Documentation: Convert multiple manpages to ReST.")
> > CC: Ben Pfaff 
> > Signed-off-by: Ansis Atteka 
> > Reported-by: Artem Teleshev 
> > ---
> >   debian/control | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/debian/control b/debian/control
> > index e47767d75..0646b22a1 100644
> > --- a/debian/control
> > +++ b/debian/control
> > @@ -14,6 +14,7 @@ Build-Depends: graphviz,
> >  openssl,
> >  procps,
> >  python3-all,
> > +   python3-sphinx,
> >  python3-twisted,
> >  python3-zope.interface,
> >  libunbound-dev,
> >
>
> Acked-by: Greg Rose 
Thanks pushed to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 2/2] netdev-offload-tc: re-fetch block ID after probing

2020-05-15 Thread Aaron Conole

It's possible that block_id could changes after the probe for block
support.  Therefore, fetch the block_id again after the probe.

Fixes: edc2055a2bf7 ("netdev-offload-tc: Flush rules on ingress block when init 
tc flow api")
Cc: Dmytro Linkin 
Co-authored-by: Marcelo Leitner 
Acked-by: Roi Dayan 
Signed-off-by: Marcelo Leitner 
Signed-off-by: Aaron Conole 
---
 lib/netdev-offload-tc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 875ebef719..e188e63e56 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1931,6 +1931,8 @@ netdev_tc_init_flow_api(struct netdev *netdev)
 
 if (ovsthread_once_start(&block_once)) {
 probe_tc_block_support(ifindex);
+/* Need to re-fetch block id as it depends on feature availability. */
+block_id = get_block_id_from_netdev(netdev);
 ovsthread_once_done(&block_once);
 }
 
-- 
2.25.1

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

[ovs-dev] [PATCH v2 0/2] TC fixes

2020-05-15 Thread Aaron Conole

The following fixes are for TC offload support:

1. Race w/ipv6 code
2. Block id support issue with ordering

v1->v2:
- Incorporate a suggestion from Ilya

Aaron Conole (2):
  netdev-linux: update LAG in all cases
  netdev-offload-tc: re-fetch block ID after probing

 lib/netdev-linux.c  | 11 +--
 lib/netdev-offload-tc.c |  2 ++
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
2.25.1

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

[ovs-dev] [PATCH v2 1/2] netdev-linux: update LAG in all cases

2020-05-15 Thread Aaron Conole

In some cases, when processing a netlink change event, it's possible for
an alternate part of OvS (like the IPv6 endpoint processing) to hold an
active netdev interface.  This creates a race-condition, where sometimes
the OvS change processing will take the normal path.  This doesn't work
because the netdev device object won't actually be enslaved to the
ovs-system (for instance, a linux bond) and ingress qdisc entries will
be missing.

To address this, we update the LAG information in ALL cases where
LAG infromation could come in.

Fixes: d22f8927c3c9 ("netdev-linux: monitor and offload LAG slaves to TC")
Cc: Marcelo Leitner 
Cc: John Hurley 
Acked-by: Roi Dayan 
Signed-off-by: Aaron Conole 
---
 lib/netdev-linux.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index b52071e92e..4006746bb3 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -655,10 +655,6 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
 {
 struct linux_lag_slave *lag;
 
-if (!rtnetlink_type_is_rtnlgrp_link(change->nlmsg_type)) {
-return;
-}
-
 if (change->slave && netdev_linux_kind_is_lag(change->slave)) {
 lag = shash_find_data(&lag_shash, change->ifname);
 
@@ -756,8 +752,11 @@ netdev_linux_run(const struct netdev_class *netdev_class 
OVS_UNUSED)
 netdev_linux_update(netdev, nsid, &change);
 ovs_mutex_unlock(&netdev->mutex);
 }
-else if (!netdev_ && change.ifname) {
-/* Netdev is not present in OvS but its master could be. */
+
+if (change.ifname &&
+rtnetlink_type_is_rtnlgrp_link(change.nlmsg_type)) {
+
+/* Need to try updating the LAG information */
 ovs_mutex_lock(&lag_mutex);
 netdev_linux_update_lag(&change);
 ovs_mutex_unlock(&lag_mutex);
-- 
2.25.1

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

Re: [ovs-dev] [PATCH] debian: Add python3-sphinx to ovs build dependencies

2020-05-15 Thread Gregory Rose




On 5/15/2020 12:17 PM, Ansis Atteka wrote:

python3-sphinx has become mandatory build dependency since patch
39b5e46 ("Documentation: Convert multiple manpages to ReST."), because,
otherwise, packaging of OVS debian packages fails with an error that
generated man pages can't be foundl.

Fixes: 39b5e46312 ("Documentation: Convert multiple manpages to ReST.")
CC: Ben Pfaff 
Signed-off-by: Ansis Atteka 
Reported-by: Artem Teleshev 
---
  debian/control | 1 +
  1 file changed, 1 insertion(+)

diff --git a/debian/control b/debian/control
index e47767d75..0646b22a1 100644
--- a/debian/control
+++ b/debian/control
@@ -14,6 +14,7 @@ Build-Depends: graphviz,
 openssl,
 procps,
 python3-all,
+   python3-sphinx,
 python3-twisted,
 python3-zope.interface,
 libunbound-dev,



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

Re: [ovs-dev] [PATCH 2/2] netdev-offload-tc: re-fetch block ID after probing

2020-05-15 Thread Marcelo Leitner

On Fri, May 15, 2020 at 09:49:15PM +0200, Ilya Maximets wrote:
> On 5/7/20 5:54 PM, Aaron Conole wrote:
> > It's possible that port ordering could cause the block ID to change
> > after enabling / detecting TC Flower support.  Therefore, fetch the
> > block_id again after probing.
> > 
> > Fixes: edc2055a2bf7 ("netdev-offload-tc: Flush rules on ingress block when 
> > init tc flow api")
> > Cc: Dmytro Linkin 
> > Co-authored-by: Marcelo Leitner 
> > Signed-off-by: Marcelo Leitner 
> > Signed-off-by: Aaron Conole 
> > ---
> >  lib/netdev-offload-tc.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 875ebef719..8864555d01 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -1939,6 +1939,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
> >  ovsthread_once_done(&multi_mask_once);
> >  }
> >  
> > +block_id = get_block_id_from_netdev(netdev);
> 
> This is not obvious from the code why we're fetching block id twice.
> Maybe we could move this right after probing and add the comment?
> Something like:
> 
> if (ovsthread_once_start(&block_once)) {
> probe_tc_block_support(ifindex);
> /* Need to re-fetch block id since it depends on feature 
> availability. */
> block_id = get_block_id_from_netdev(netdev);
> ovsthread_once_done(&block_once);
> }
> 
> What do you think?

Yes, LGTM. Good point, it doesn't need to be re-fetched if the first
one was good already (i.e., if the block support was already checked).

Thanks,
Marcelo

> 
> Best regards, Ilya Maximets.
> 

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

Re: [ovs-dev] [PATCH 2/2] netdev-offload-tc: re-fetch block ID after probing

On 5/7/20 5:54 PM, Aaron Conole wrote:
> It's possible that port ordering could cause the block ID to change
> after enabling / detecting TC Flower support.  Therefore, fetch the
> block_id again after probing.
> 
> Fixes: edc2055a2bf7 ("netdev-offload-tc: Flush rules on ingress block when 
> init tc flow api")
> Cc: Dmytro Linkin 
> Co-authored-by: Marcelo Leitner 
> Signed-off-by: Marcelo Leitner 
> Signed-off-by: Aaron Conole 
> ---
>  lib/netdev-offload-tc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 875ebef719..8864555d01 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1939,6 +1939,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>  ovsthread_once_done(&multi_mask_once);
>  }
>  
> +block_id = get_block_id_from_netdev(netdev);

This is not obvious from the code why we're fetching block id twice.
Maybe we could move this right after probing and add the comment?
Something like:

if (ovsthread_once_start(&block_once)) {
probe_tc_block_support(ifindex);
/* Need to re-fetch block id since it depends on feature availability. 
*/
block_id = get_block_id_from_netdev(netdev);
ovsthread_once_done(&block_once);
}

What do you think?

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

[ovs-dev] [PATCH] debian: Add python3-sphinx to ovs build dependencies

2020-05-15 Thread Ansis Atteka

python3-sphinx has become mandatory build dependency since patch
39b5e46 ("Documentation: Convert multiple manpages to ReST."), because,
otherwise, packaging of OVS debian packages fails with an error that
generated man pages can't be foundl.

Fixes: 39b5e46312 ("Documentation: Convert multiple manpages to ReST.")
CC: Ben Pfaff 
Signed-off-by: Ansis Atteka 
Reported-by: Artem Teleshev 
---
 debian/control | 1 +
 1 file changed, 1 insertion(+)

diff --git a/debian/control b/debian/control
index e47767d75..0646b22a1 100644
--- a/debian/control
+++ b/debian/control
@@ -14,6 +14,7 @@ Build-Depends: graphviz,
openssl,
procps,
python3-all,
+   python3-sphinx,
python3-twisted,
python3-zope.interface,
libunbound-dev,
-- 
2.20.1

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

Re: [ovs-dev] [PATCH] odp-util.c: Fix dp_hash execution with slowpath actions.

On Fri, May 15, 2020 at 12:18 AM Han Zhou  wrote:
>
> When dp_hash is executed with slowpath actions, it results in endless
> recirc loop in kernel datapath, and finally drops the packet, with
> kernel logs:
>
> openvswitch: ovs-system: deferred action limit reached, drop recirc action
>
> The root cause is that the dp_hash value calculated by slowpath is not
> passed to datapath when executing the recirc action, thus when the
recirced
> packet miss upcall comes to userspace again, it generates the dp_hash
> and recirc action again, with same recirc_id, which in turn generates
> a megaflow with recirc action with the recird_id same as the recirc_id in
> its match condition, which causes a loop in datapath.
>
> For example, this can be reproduced with below setup of OVN environment:
>
>  LS1LS2
>   |  |
>   |--R1--|
> VIF--LS0---R0-|  |--R3
>   |--R2--|
>
> Assume there is a route from the VIF to R3: R0 -> R1 -> R3, and there are
two
> routes (ECMP) from R3 to the VIF:
> R3 -> R1 -> R0
> R3 -> R2 -> R0
>
> Now if we ping from the VIF to R3, the OVS flow execution on the HV of
the VIF
> will hit the R3's datapath which has flows that responds to the ICMP
packet
> by setting ICMP fields, which requires slowpath actions, and in later flow
> tables it will hit the "group" action that selects between the ECMP
routes.
>
> By default OVN uses "dp_hash" method for the "group" action.
>
> For the first miss upcall packet, dp_hash value is empty, so the group
action
> will be translated to "dp_hash" and "recirc".
>
> During action execution, because of the previous actions that sets ICMP
fields,
> the whole execution requires slowpath, so it tries to execute all actions
in
> userspace in odp_execute_actions(), including dp_hash action, except the
> recirc action, which can only be executed in datapath. So the dp_hash
value
> is calculated in userspace, and then the packet is injected to datapath
for
> recirc action execution.
>
> However, the dp_hash calculated by the userspace is not passed to
datapath.
>
> Because of this, the packet recirc in datapath doesn't have dp_hash value,
> and the miss upcall for the recirced packet hits the same flow tables and
> triggers same "dp_hash" and "recirc" action again, with exactly same
recirc_id!
>
> This time, the new upcall doesn't require any slowpath execution, so both
> the dp_hash and recirc actions are executed in datapath, after creating a
> datapath megaflow like:
>
> recirc_id(XYZ),..., actions:hash(l4(0)),recirc(XYZ)
>
> with match recirc_id equals the recirc id in the action, thus creating a
loop.
>
> This patch fixes the problem by passing the calculated dp_hash value to
> datapath in odp_key_from_dp_packet().
>
> Signed-off-by: Han Zhou 
> ---
>  lib/odp-util.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index b66d266..ac532fe 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -6392,6 +6392,10 @@ odp_key_from_dp_packet(struct ofpbuf *buf, const
struct dp_packet *packet)
>
>  nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, md->skb_priority);
>
> +if (md->dp_hash) {
> +nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH, md->dp_hash);
> +}
> +
>  if (flow_tnl_dst_is_set(&md->tunnel)) {
>  tun_key_to_attr(buf, &md->tunnel, &md->tunnel, NULL, NULL);
>  }
> --
> 2.1.0
>

Ben and Ilya, this is the fix to the dp_hash problem we discussed in
yesterday's meeting. The actual fix is simpler that I thought it would be.
I didn't take the approach of executing dp_hash in datapath because in this
case since the flow is required to be slowpathed, all the following packets
for this flow will anyway get upcalled. If all the dp_hash for the flow is
executed in slowpath then there is no consistency problem. So I think it is
ok to keep the calculation in userspace and the fix is simple. Let me know
if you think differently.

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

Re: [ovs-dev] [PATCH ovn] ovn-northd: Increase ECMP route priority with 400.

On Fri, May 15, 2020 at 9:01 AM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:
>
> > > On Thu, May 14, 2020 at 1:12 AM Dumitru Ceara 
wrote:
> > > >
> > > > On 5/13/20 11:51 PM, Han Zhou wrote:
> > > > > In commit c0bf32d the priorities of the regular routes were
increased by
> > > > > 400, but ECMP routes were not updated. This patch fixes it. Since
> > > > > for ECMP routes the outport is unknown (there can be many
different
> > > > > outports) the condition check of whether the outport is
distributed
> > > > > gateway port is skipped.
> > > > >
> > > > > Fixes: c0bf32d ("Manage ARP process locally in a DVR scenario")
> > > > > Cc: Lorenzo Bianconi 
> > > > > Signed-off-by: Han Zhou 
> >
> > [...]
> >
> > > So I wonder if we should change the solution of the commit c0bf32d,
instead
> > > of just increasing the ECMP routes priority only. I don't completely
> > > understand the problem that commit was trying to solve. Is there an
example
> > > that describes the scenario in more detail and the reason why the
route
> > > priorities have to be different?
> >
> > Hi Han,
> >
> > this morning Dumitru and me worked trying to find a solution for this
issue.
> > We though to restore the original configuration in table 9
> > and add a new table used only to take care of FIP/DVR traffic.
> > Does it work for you?
> > I sent a RFC with the basic implementation. I tested it and it works
fine
> > regarding to FIP traffic. I will work on unitest waiting for feedbacks.
> >
> > Regards,
> > Lorenzo
>
> ops I forgot, this is the patch:
>
https://patchwork.ozlabs.org/project/openvswitch/patch/a99d46b275a10a6d8278434f7cefa0466bb78af5.1589557561.git.lorenzo.bianc...@redhat.com/
>
> Regards,
> Lorenzo
>

That's great. Thanks Lorenzo. I will abandon this patch.

> >
> > >
> > > >
> > > > > ---
> > > > >  northd/ovn-northd.8.xml |  3 ++-
> > > > >  northd/ovn-northd.c | 22 --
> > > > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > > > > index 8f224b0..b0475d0 100644
> > > > > --- a/northd/ovn-northd.8.xml
> > > > > +++ b/northd/ovn-northd.8.xml
> > > > > @@ -2601,7 +2601,8 @@ next;
> > > > >ids MID1, MID2, ..., a logical
flow
> > > with match
> > > > >in CIDR notation ip4.dst ==
> > > N/M,
> > > > >or ip6.dst == N/M,
whose
> > > priority
> > > > > -  is the integer value of M, has the following
> > > actions:
> > > > > +  is 400 + the integer value of
M, has
> > > the
> > > > > +  following actions:
> > > > >  
> > > > >
> > > > >  
> > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > > > index b25152d..c02e305 100644
> > > > > --- a/northd/ovn-northd.c
> > > > > +++ b/northd/ovn-northd.c
> > > > > @@ -7337,7 +7337,8 @@ build_route_prefix_s(const struct v46_ip
*prefix,
> > > unsigned int plen)
> > > > >  }
> > > > >
> > > > >  static void
> > > > > -build_route_match(const struct ovn_port *op_inport, const char
> > > *network_s,
> > > > > +build_route_match(const struct ovn_port *op_inport,
> > > > > +  const struct ovn_port *op_outport, const char
> > > *network_s,
> > > > >int plen, bool is_src_route, bool is_ipv4,
struct ds
> > > *match,
> > > > >uint16_t *priority)
> > > > >  {
> > > > > @@ -7351,6 +7352,13 @@ build_route_match(const struct ovn_port
> > > *op_inport, const char *network_s,
> > > > >  dir = "dst";
> > > > >  *priority = (plen * 2) + 1;
> > > > >  }
> > > > > +/* traffic for internal IPs of logical switch ports must be
sent to
> > > > > + * the gw controller through the overlay tunnels
> > > > > + */
> > > > > +if (!op_outport ||
> > > > > +(op_outport->nbrp &&
!op_outport->nbrp->n_gateway_chassis)) {
> > > > > +priority += DROUTE_PRIO;
> > > > > +}
> > > > >
> > > > >  if (op_inport) {
> > > > >  ds_put_format(match, "inport == %s && ",
op_inport->json_key);
> > > > > @@ -7434,8 +7442,8 @@ build_ecmp_route_flow(struct hmap *lflows,
struct
> > > ovn_datapath *od,
> > > > >  uint16_t priority;
> > > > >
> > > > >  char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
> > > > > -build_route_match(NULL, prefix_s, eg->plen, eg->is_src_route,
> > > is_ipv4,
> > > > > -  &match, &priority);
> > > > > +build_route_match(NULL, NULL, prefix_s, eg->plen,
eg->is_src_route,
> > > > > +  is_ipv4, &match, &priority);
> > > > >  free(prefix_s);
> > > > >
> > > > >  struct ds actions = DS_EMPTY_INITIALIZER;
> > > > > @@ -7548,14 +7556,8 @@ add_route(struct hmap *lflows, const struct
> > > ovn_port *op,
> > > > >  op_inport = op;
> > > > >  }
> > > > >  }
> > > > > -build_route_match(op_inport, network_s, plen, is_src_route,
> > > is_ipv4,
> > > > > +build_route_match(op_inport, op, netw

Re: [ovs-dev] [PATCH] ofproto: Fix statistics of removed flow.

2020-05-15 Thread Compras, Comercio Exterior, Logística y Operaciones

On 5/14/20 9:43 PM, Roi Dayan wrote:
> 
> 
> On 2020-05-14 9:33 PM, Ilya Maximets wrote:
>> 'fr' is a new variable on the stack.  '+=' here adds the real statistics
>> to a random stack memory.
>>
>> Fixes: 164413156cf9 ("Add offload packets statistics")
>> Signed-off-by: Ilya Maximets 
>> ---
>>  ofproto/ofproto.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index 0fbd6c380..59f06aa94 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -6085,8 +6085,8 @@ ofproto_rule_send_removed(struct rule *rule)
>>  fr.hard_timeout = rule->hard_timeout;
>>  ovs_mutex_unlock(&rule->mutex);
>>  rule->ofproto->ofproto_class->rule_get_stats(rule, &stats, &used);
>> -fr.packet_count += stats.n_packets;
>> -fr.byte_count += stats.n_bytes;
>> +fr.packet_count = stats.n_packets;
>> +fr.byte_count = stats.n_bytes;
>>  connmgr_send_flow_removed(connmgr, &fr);
>>  ovs_mutex_unlock(&ofproto_mutex);
>>  }
>>
> 
> Acked-by: Roi Dayan 
> 

Thanks!

Applied to master and  branch-2.13.

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

[ovs-dev] Cursos privados en línea para tu empresa

Cursos privados en línea para tu empresa.

Capacita a tu área de Logística, Operaciones y Compras con cursos privados 
personalizados, en línea, en vivo y de la mano de expertos con amplia
experiencia profesional en empresas de renombre.

Solicita nuestro catálogo para conocer nuestra oferta completa para las áreas 
de Compras, Comercio Exterior, Logística y Operaciones. 

Algunos de nuestros cursos disponibles en modalidad privada:

Curso: Gestión aduanera y de transporte
Duración total: 6
Objetivo: Desarrollar capacidades para realizar operaciones de importación y 
exportación, en el marco de las regulaciones aduaneras  mexicana e
internacional, así como de la logística multimodal del transporte - Desde 
12,600 + IVA

Curso: Clasificación arancelaria
Duración total: 4
Objetivo: Aportar al participante los conceptos y bases del Sistema Armonizado 
de clasificación arancelaria, partiendo de lo cual para aportarle 
conocimientos y herramientas para aplicar correctamente la clasificación de la 
Ley de los Impuestos Generales de Importación y Exportación Desde 9,600 + IVA


Otros temas disponibles:

Análisis de indicadores clave para Compras - Gestión de proveedores: selección, 
desarrollo y evaluación de desempeño-  Administración de compras por 
categorías de gasto (Category Management) - Negociación para compradores - 
Contratos de compras - Técnicas avanzadas de compras - Experto en compras
internacionales - Planeación de recursos de manufactura (MRP) -  Logística y 
distribución -  Estadística para los negocios y la producción -  Administración 
efectiva de inventarios.

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

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

Pregunte por nuestros cursos grabados y sus costos!

Números de Atención: 

(045) 55 3016 7085 - (045) 55 1554 6630

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder
con la palabra baja o enviar un correo a bajas@ innovalearn.net


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

Re: [ovs-dev] [RFC ovn] ovn: introduce IP_SRC_POLICY stage in ingress router pipeline

2020-05-15 Thread Dumitru Ceara

On 5/15/20 5:49 PM, Lorenzo Bianconi wrote:
> In order to fix the issues introduced by commit
> c0bf32d72f8b ("Manage ARP process locally in a DVR scenario "), restore
> previous configuration of table 9 in ingress router pipeline and
> introduce a new stage called 'ip_src_policy' used to set the src address
> info in order to not distribute FIP traffic if DVR is enabled
> 
> Fixes: c0bf32d72f8b ("Manage ARP process locally in a DVR scenario ")
> Signed-off-by: Lorenzo Bianconi 

Hi Lorenzo,

Thanks for working on fixing this issue!

As discussed offline, I'm a bit concerned about adding a new stage to
the OVN pipeline and I'd prefer if we could find a different way to
address this.

This being said, if there's no other easy way of fixing this, I think
your change looks ok and will handle all the bugs reported, including [0].

Thanks,
Dumitru

[0] https://bugzilla.redhat.com/show_bug.cgi?id=1834433

> ---
>  northd/ovn-northd.8.xml | 65 -
>  northd/ovn-northd.c | 38 ++--
>  tests/ovn.at| 28 +-
>  3 files changed, 54 insertions(+), 77 deletions(-)
> 
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 8f224b07f..09dbb52b4 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2484,37 +2484,6 @@ output;
>  
>
>  
> -  
> -
> -  For distributed logical routers where one of the logical router 
> ports
> -  specifies a redirect-chassis, a priority-400 logical
> -  flow for each dnat_and_snat NAT rules configured.
> -  These flows will allow to properly forward traffic to the external
> -  connections if available and avoid sending it through the tunnel.
> -  Assuming the following NAT rule has been configured:
> -
> -
> -
> -external_ip = A;
> -external_mac = B;
> -logical_ip = C;
> -
> -
> -
> -  the following action will be applied:
> -
> -
> -
> -ip.ttl--;
> -reg0 = ip.dst;
> -reg1 = A;
> -eth.src = B;
> -outport = router-port;
> -next;
> -
> -
> -  
> -
>
>  
>IPv4 routing table.  For each route to IPv4 network N 
> with
> @@ -2660,7 +2629,35 @@ outport = P;
>
>  
>  
> -Ingress Table 12: ARP/ND Resolution
> +Ingress Table 12: IP Source Policy
> +
> +
> +  This table contains for distributed logical routers where one of
> +  the logical router ports specifies a redirect-chassis,
> +  a priority-100 logical flow for each dnat_and_snat
> +  NAT rules configured.
> +  These flows will allow to properly forward traffic to the external
> +  connections if available and avoid sending it through the tunnel.
> +  Assuming the following NAT rule has been configured:
> +
> +
> +
> +external_ip = A;
> +external_mac = B;
> +logical_ip = C;
> +
> +
> +
> +  the following action will be applied:
> +
> +
> +
> +reg1 = A;
> +eth.src = B;
> +next;
> +
> +
> +Ingress Table 13: ARP/ND Resolution
>  
>  
>Any packet that reaches this table is an IP packet whose next-hop
> @@ -2819,7 +2816,7 @@ outport = P;
>  
>  
>  
> -Ingress Table 13: Check packet length
> +Ingress Table 14: Check packet length
>  
>  
>For distributed logical routers with distributed gateway port 
> configured
> @@ -2849,7 +2846,7 @@ REGBIT_PKT_LARGER = check_pkt_larger(L); 
> next;
>and advances to the next table.
>  
>  
> -Ingress Table 14: Handle larger packets
> +Ingress Table 15: Handle larger packets
>  
>  
>For distributed logical routers with distributed gateway port 
> configured
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3c0070ea7..d5f3997a9 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -175,11 +175,12 @@ enum ovn_stage {
>  PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  9, "lr_in_ip_routing")   \
>  PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_ECMP, 10, 
> "lr_in_ip_routing_ecmp") \
>  PIPELINE_STAGE(ROUTER, IN,  POLICY,  11, "lr_in_policy")   \
> -PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 12, "lr_in_arp_resolve")  \
> -PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   ,  13, "lr_in_chk_pkt_len")   \
> -PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 14,"lr_in_larger_pkts")   \
> -PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 15, "lr_in_gw_redirect")  \
> -PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 16, "lr_in_arp_request")  \
> +PIPELINE_STAGE(ROUTER, IN,  IP_SRC_POLICY,   12, "lr_in_ip_src_policy") \
> +PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 13, "lr_in_arp_resolve")  \
> +PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   ,  14, "lr_in_chk_pkt_len")   \
> +PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 15,"lr_in_larger_pkts")   \
> +PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 16, "lr_in_gw_redirect")

Re: [ovs-dev] OVN 20.06 Soft Freeze Today

2020-05-15 Thread Mark Michelson

On 5/15/20 1:12 PM, Ihar Hrachyshka wrote:

On 5/15/20 12:24 PM, Numan Siddique wrote:
On Fri, May 15, 2020 at 8:23 PM Ihar Hrachyshka
wrote:

On 5/11/20 10:15 AM, Ihar Hrachyshka wrote:

On 5/8/20 2:17 PM, Numan Siddique wrote:

On Fri, May 8, 2020 at 9:58 PM Ihar Hrachyshka mailto:ihrac...@redhat.com>> wrote:

Hi all,

I would like the series to support multiple localnet ports per
switch
("routed provider networks for OpenStack") considered for
inclusion.

The series is here:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=173328

There's an updated (v3) version of 5/6 patch from the series
here:

https://patchwork.ozlabs.org/project/openvswitch/patch/20200508154425.112544-1-ihrac...@redhat.com/

(I'm not sure I posted it correctly - since other patches in the
series
didn't need any update from v2, I pushed only 5/6 but now it's
displayed
separately in patchwork; let me know if that's an issue and/or I
should
repost the whole series.)

I think it's better to repost as a whole series.

Thanks. Here is the full series (I also attached a cover letter):
https://patchwork.ozlabs.org/project/openvswitch/list/?series=176134

Note there's one issue in the new test cases that I am working on
right now and hope to fix a fix today. There will probably be v5 of
the series but I hope we can pick any other issues in the
implementation, reviews are welcome.

Hi all,

Bear with me since I am new to OVN release freeze process. Now that we
approach(ed?) hard freeze, I wonder if there's still a chance to include
the series mentioned above (there's v7 of the same up for review with
one ack), or it will have to slip into a next release. I understand that
not everything raised can - nor should - be included during soft freeze.
On the other hand, I haven't seen much of discussion as to merit of
inclusion of particular patches mentioned in this soft freeze thread.
Did the discussion happen anywhere else? In general, how does the
project decide on which exception requests to grant and which to shelve?

Hi Ihar,

Since your patches were submitted before the soft freeze deadline I think
they are definitely candidates to be included for 20.06.

Even if the branch-20.06 is created, we can backport your patches.
I see that you have received Ack from Dumitru for your series.

I applied the first 4 patches of your series to master just a while back.
I didn't get the chance to review your other 2 patches and I'm also
waiting

for Han if he has any comments for the other 2 patches.

Thanks!!!

BTW to clarify, I don't *complain* about the series not getting merged
(though I am glad if it does!), just trying to understand if there are
any other actions I should (have) take(n) to improve chances of timely
merge before branch cut-off. What I gather from your reply is 1)
inclusion or non-inclusion is case by case and up to individual core
contributors, there's no formal group decision; and 2) even after hard
freeze, feature patches *may* be backported.

Though re 2), on the other hand,
Documentation/internals/release-process.rst says about the branch
created on hard freeze: "Release branches are intended for testing and
stabilization. At this stage and in later stages, they should receive
only bug fixes, not new features."

Yes, ideally it would be very rare that we have to backport a feature
after the branch is created.

In your case, your patch was up for review for a while, and you made it
clear that you wanted it included for 20.06. It's on the rest of us to
ensure your patch gets reviewed in time for inclusion. It's not your
fault that we've been slow. You shouldn't get penalized for that.

All that being said, despite the fact that technically today is hard
freeze, I'm not creating the branch for 20.06 until Monday. So if we can
get it in before the end of the day Monday, it shouldn't even require a
backport to the branch. And if there's a compelling reason to delay the
branch creation, then that's fine too. These deadlines are of our own
making, and there's some flexibility built in just in case delays are
required.

Ihar

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

Re: [ovs-dev] OVN 20.06 Soft Freeze Today

On Fri, May 15, 2020 at 10:42 PM Ihar Hrachyshka 
wrote:

> On 5/15/20 12:24 PM, Numan Siddique wrote:
> > On Fri, May 15, 2020 at 8:23 PM Ihar Hrachyshka 
> wrote:
> >
> >> On 5/11/20 10:15 AM, Ihar Hrachyshka wrote:
> >>> On 5/8/20 2:17 PM, Numan Siddique wrote:
> 
>  On Fri, May 8, 2020 at 9:58 PM Ihar Hrachyshka   > wrote:
> 
>   Hi all,
> 
>   I would like the series to support multiple localnet ports per
>  switch
>   ("routed provider networks for OpenStack") considered for
> inclusion.
> 
>   The series is here:
>  https://patchwork.ozlabs.org/project/openvswitch/list/?series=173328
> 
>   There's an updated (v3) version of 5/6 patch from the series
> here:
> 
> >>
> https://patchwork.ozlabs.org/project/openvswitch/patch/20200508154425.112544-1-ihrac...@redhat.com/
>   (I'm not sure I posted it correctly - since other patches in the
>   series
>   didn't need any update from v2, I pushed only 5/6 but now it's
>   displayed
>   separately in patchwork; let me know if that's an issue and/or I
>   should
>   repost the whole series.)
> 
> 
>  I think it's better to repost as a whole series.
> 
> >>> Thanks. Here is the full series (I also attached a cover letter):
> >>> https://patchwork.ozlabs.org/project/openvswitch/list/?series=176134
> >>>
> >>>
> >>> Note there's one issue in the new test cases that I am working on
> >>> right now and hope to fix a fix today. There will probably be v5 of
> >>> the series but I hope we can pick any other issues in the
> >>> implementation, reviews are welcome.
> >>>
> >> Hi all,
> >>
> >> Bear with me since I am new to OVN release freeze process. Now that we
> >> approach(ed?) hard freeze, I wonder if there's still a chance to include
> >> the series mentioned above (there's v7 of the same up for review with
> >> one ack), or it will have to slip into a next release. I understand that
> >> not everything raised can - nor should - be included during soft freeze.
> >> On the other hand, I haven't seen much of discussion as to merit of
> >> inclusion of particular patches mentioned in this soft freeze thread.
> >> Did the discussion happen anywhere else? In general, how does the
> >> project decide on which exception requests to grant and which to shelve?
> >>
> >>
> > Hi Ihar,
> >
> > Since your patches were submitted before the soft freeze deadline I think
> > they are definitely candidates to be included for 20.06.
> >
> > Even if the branch-20.06 is created, we can backport your patches.
> > I see that you have received Ack from Dumitru for your series.
> >
> > I applied the first 4 patches of your series to master just a while back.
> > I didn't get the chance to review your other 2 patches and I'm also
> waiting
> > for Han if he has any comments for the other 2 patches.
>
> Thanks!!!
>
> BTW to clarify, I don't *complain* about the series not getting merged
> (though I am glad if it does!), just trying to understand if there are
> any other actions I should (have) take(n) to improve chances of timely
> merge before branch cut-off. What I gather from your reply is 1)
> inclusion or non-inclusion is case by case and up to individual core
> contributors, there's no formal group decision; and 2) even after hard
> freeze, feature patches *may* be backported.
>
>
I may be wrong. But this is what I've seen a few times.
But If I'm wrong, I'm happy to correct myself :).

Numan



> Though re 2), on the other hand,
> Documentation/internals/release-process.rst says about the branch
> created on hard freeze: "Release branches are intended for testing and
> stabilization.  At this stage and in later stages, they should receive
> only bug fixes, not new features."
>
> Ihar
>
> ___
> 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 ovs v2 0/4] expand the meter table and fix bug

On 5/13/20 3:31 PM, xiangxia.m@gmail.com wrote:
> From: Tonghao Zhang 
> 
> The patch set expand or shrink the meter table when necessary.
> and other patches fix bug or improve codes.
> 
> Tonghao Zhang (4):
>   dpif-netdev: Expand the meters supported number
>   dpif-netdev: Add burst size to buckets
>   dpif-netdev: Use the u64 instead of u32 for buckets
>   Revert "dpif-netdev: includes microsecond delta in meter bucket
> calculation"
> 
>  include/openvswitch/ofp-meter.h |   2 +-
>  lib/dpif-netdev.c   | 336 
>  lib/ofp-meter.c |   4 +-
>  3 files changed, 257 insertions(+), 85 deletions(-)
> 

Thanks for working on this!

General note about sending patches:
Please, don't send patches in reply to the previous version.  This messes up
mailboxes.  Send new versions separately.
And it's better to have a dot at the end of a patch subject.

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

Re: [ovs-dev] OVN 20.06 Soft Freeze Today

2020-05-15 Thread Ihar Hrachyshka

On 5/15/20 12:24 PM, Numan Siddique wrote:

On Fri, May 15, 2020 at 8:23 PM Ihar Hrachyshka wrote:

On 5/11/20 10:15 AM, Ihar Hrachyshka wrote:

On 5/8/20 2:17 PM, Numan Siddique wrote:

On Fri, May 8, 2020 at 9:58 PM Ihar Hrachyshka mailto:ihrac...@redhat.com>> wrote:

Hi all,

I would like the series to support multiple localnet ports per
switch
("routed provider networks for OpenStack") considered for inclusion.

The series is here:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=173328

There's an updated (v3) version of 5/6 patch from the series here:

https://patchwork.ozlabs.org/project/openvswitch/patch/20200508154425.112544-1-ihrac...@redhat.com/

I think it's better to repost as a whole series.

Thanks. Here is the full series (I also attached a cover letter):
https://patchwork.ozlabs.org/project/openvswitch/list/?series=176134

Hi all,

Hi Ihar,

Since your patches were submitted before the soft freeze deadline I think
they are definitely candidates to be included for 20.06.

Even if the branch-20.06 is created, we can backport your patches.
I see that you have received Ack from Dumitru for your series.

I applied the first 4 patches of your series to master just a while back.
I didn't get the chance to review your other 2 patches and I'm also waiting
for Han if he has any comments for the other 2 patches.

Thanks!!!

Ihar

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

Re: [ovs-dev] [PATCH ovs v2 1/4] dpif-netdev: Expand the meters supported number

On 5/13/20 3:31 PM, xiangxia.m@gmail.com wrote:
> From: Tonghao Zhang 
> 
> For now, ovs-vswitchd use the array of the dp_meter struct
> to store meter's data, and at most, there are only 65536
> (defined by MAX_METERS) meters that can be used. But in some
> case, for example, in the edge gateway, we should use 200,000,
> at least, meters for IP address bandwidth limitation.
> Every one IP address will use two meters for its rx and tx
> path[1]. In other way, ovs-vswitchd should support meter-offload
> (rte_mtr_xxx api introduced by dpdk.), but there are more than
> 65536 meters in the hardware, such as Mellanox ConnectX-6.
> 
> This patch use array to manage the meter, but it can ben expanded.
> 
> [1].
> $ in_port=p0,ip,ip_dst=1.1.1.x action=meter:n,output:p1
> $ in_port=p1,ip,ip_src=1.1.1.x action=meter:m,output:p0
> 
> Cc: Ilya Maximets 
> Cc: William Tu 
> Cc: Jarno Rajahalme 
> Cc: Ben Pfaff 
> Cc: Andy Zhou 
> Cc: Pravin Shelar 
> Signed-off-by: Tonghao Zhang 
> ---
> v2:
> * add comments for dp_meter_instance
> * change the log
> * remove extra newline
> * I don't move the dp_netdev_meter_init/destroy up. because
>   them depends other meters function and put all meter function
>   together may make the codes clean.
> ---

Hi.  Thanks for working on this!

This is not a full review, just a few things that I spotted on a quick glance.
I didn't review any thread safety/rcu aspects yet.

Best regards, Ilya Maximets.


>  lib/dpif-netdev.c | 319 --
>  1 file changed, 250 insertions(+), 69 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index ef14e83b5f06..b5deaab31eb0 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -98,9 +98,12 @@ DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
>  
>  /* Configuration parameters. */
>  enum { MAX_FLOWS = 65536 }; /* Maximum number of flows in flow table. */
> -enum { MAX_METERS = 65536 };/* Maximum number of meters. */
> -enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
> -enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
> +
> +/* Maximum number of meters in the table. */
> +#define METER_ENTRY_MAX (20ULL)
> +/* Maximum number of bands / meter. */
> +#define METER_BAND_MAX  (8)
> +#define DP_METER_ARRAY_SIZE_MIN (1ULL << 10)

Why we need to change enums to defines and also rename them?

>  
>  COVERAGE_DEFINE(datapath_drop_meter);
>  COVERAGE_DEFINE(datapath_drop_upcall_error);
> @@ -283,12 +286,26 @@ struct dp_meter {
>  uint16_t flags;
>  uint16_t n_bands;
>  uint32_t max_delta_t;
> +uint32_t id;
> +struct ovs_mutex lock;
>  uint64_t used;
>  uint64_t packet_count;
>  uint64_t byte_count;
>  struct dp_meter_band bands[];
>  };
>  
> +struct dp_meter_instance {
> +uint32_t n_meters;

This should be called 'n_allocated' or smething like this.
'n_meters' makes me think that it's the number of actually used meters.

> +/* Followed by struct dp_meter[n]; where n is the n_meters. */
> +OVSRCU_TYPE(struct dp_meter *) dp_meters[];
> +};
> +
> +struct dp_meter_table {
> +OVSRCU_TYPE(struct dp_meter_instance *) ti;

What does 'ti' mean?  I looked throught the code it always stands for meter 
instance,
but how 'meter instance' relates to 'ti'?  That is confusing.

> +uint32_t count;

Why count is part of 'dp_meter_table'?  I think it should be part of 
'dp_meter_instance'
and named something like 'n_used', or actually 'n_meters'.

> +struct ovs_mutex lock;
> +};

Why we need this structure at all?  Can it be just 3 fields inside struct 
dp_netdev?
Why it is table?  It's not a table. 'instance' is a table.  Confusing.

> +
>  struct pmd_auto_lb {
>  bool auto_lb_requested; /* Auto load balancing requested by user. */
>  bool is_enabled;/* Current status of Auto load balancing. */
> @@ -329,8 +346,7 @@ struct dp_netdev {
>  atomic_uint32_t tx_flush_interval;
>  
>  /* Meters. */
> -struct ovs_mutex meter_locks[N_METER_LOCKS];
> -struct dp_meter *meters[MAX_METERS]; /* Meter bands. */
> +struct dp_meter_table meter_tbl;
>  
>  /* Probability of EMC insertions is a factor of 'emc_insert_min'.*/
>  OVS_ALIGNED_VAR(CACHE_LINE_SIZE) atomic_uint32_t emc_insert_min;
> @@ -378,19 +394,6 @@ struct dp_netdev {
>  struct pmd_auto_lb pmd_alb;
>  };
>  
> -static void meter_lock(const struct dp_netdev *dp, uint32_t meter_id)
> -OVS_ACQUIRES(dp->meter_locks[meter_id % N_METER_LOCKS])
> -{
> -ovs_mutex_lock(&dp->meter_locks[meter_id % N_METER_LOCKS]);
> -}
> -
> -static void meter_unlock(const struct dp_netdev *dp, uint32_t meter_id)
> -OVS_RELEASES(dp->meter_locks[meter_id % N_METER_LOCKS])
> -{
> -ovs_mutex_unlock(&dp->meter_locks[meter_id % N_METER_LOCKS]);
> -}
> -
> -
>  static struct dp_netdev_port *dp_netdev_lookup_port(const struct dp_netdev 
> *dp,
>  odp_port_t)

[ovs-dev] [PATCH] ovsdb-server: Fix schema leak while reading db.

parse_txn() function doesn't always take ownership of the 'schema'
passed.  So, if the schema of the clustered db has same version as the
one that already in use, parse_txn() will not use it, resulting with a
memory leak:

 7,827 (56 direct, 7,771 indirect) bytes in 1 blocks are definitely lost
at 0x483BB1A: calloc (vg_replace_malloc.c:762)
by 0x44AD02: xcalloc (util.c:121)
by 0x40E70E: ovsdb_schema_create (ovsdb.c:41)
by 0x40EA6D: ovsdb_schema_from_json (ovsdb.c:217)
by 0x415EDD: ovsdb_storage_read (storage.c:280)
by 0x408968: read_db (ovsdb-server.c:607)
by 0x40733D: main_loop (ovsdb-server.c:227)
by 0x40733D: main (ovsdb-server.c:469)

While we could put ovsdb_schema_destroy() in a few places inside
'parse_txn()', from the users' point of view it seems better to have a
constant argument and just clone the 'schema' if needed.  The caller
will be responsible for destroying the 'schema' it owns.

CC: Ben Pfaff 
Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered 
databases.")
Signed-off-by: Ilya Maximets 
---
 ovsdb/ovsdb-server.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index d416f1b60..ef4e996df 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -540,7 +540,7 @@ close_db(struct server_config *config, struct db *db, char 
*comment)
 
 static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
 parse_txn(struct server_config *config, struct db *db,
-  struct ovsdb_schema *schema, const struct json *txn_json,
+  const struct ovsdb_schema *schema, const struct json *txn_json,
   const struct uuid *txnid)
 {
 if (schema && (!db->db->schema || strcmp(schema->version,
@@ -565,7 +565,7 @@ parse_txn(struct server_config *config, struct db *db,
  ? xasprintf("database %s schema changed", db->db->name)
  : xasprintf("database %s connected to storage", db->db->name)));
 
-ovsdb_replace(db->db, ovsdb_create(schema, NULL));
+ovsdb_replace(db->db, ovsdb_create(ovsdb_schema_clone(schema), NULL));
 
 /* Force update to schema in _Server database. */
 db->row_uuid = UUID_ZERO;
@@ -614,6 +614,7 @@ read_db(struct server_config *config, struct db *db)
 } else {
 error = parse_txn(config, db, schema, txn_json, &txnid);
 json_destroy(txn_json);
+ovsdb_schema_destroy(schema);
 if (error) {
 break;
 }
-- 
2.25.4

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

Re: [ovs-dev] OVN 20.06 Soft Freeze Today

On Fri, May 15, 2020 at 8:23 PM Ihar Hrachyshka  wrote:

> On 5/11/20 10:15 AM, Ihar Hrachyshka wrote:
> > On 5/8/20 2:17 PM, Numan Siddique wrote:
> >>
> >>
> >> On Fri, May 8, 2020 at 9:58 PM Ihar Hrachyshka  >> > wrote:
> >>
> >> Hi all,
> >>
> >> I would like the series to support multiple localnet ports per
> >> switch
> >> ("routed provider networks for OpenStack") considered for inclusion.
> >>
> >> The series is here:
> >> https://patchwork.ozlabs.org/project/openvswitch/list/?series=173328
> >>
> >> There's an updated (v3) version of 5/6 patch from the series here:
> >>
> https://patchwork.ozlabs.org/project/openvswitch/patch/20200508154425.112544-1-ihrac...@redhat.com/
> >>
> >> (I'm not sure I posted it correctly - since other patches in the
> >> series
> >> didn't need any update from v2, I pushed only 5/6 but now it's
> >> displayed
> >> separately in patchwork; let me know if that's an issue and/or I
> >> should
> >> repost the whole series.)
> >>
> >>
> >> I think it's better to repost as a whole series.
> >>
> >
> > Thanks. Here is the full series (I also attached a cover letter):
> > https://patchwork.ozlabs.org/project/openvswitch/list/?series=176134
> >
> >
> > Note there's one issue in the new test cases that I am working on
> > right now and hope to fix a fix today. There will probably be v5 of
> > the series but I hope we can pick any other issues in the
> > implementation, reviews are welcome.
> >
>
> Hi all,
>
> Bear with me since I am new to OVN release freeze process. Now that we
> approach(ed?) hard freeze, I wonder if there's still a chance to include
> the series mentioned above (there's v7 of the same up for review with
> one ack), or it will have to slip into a next release. I understand that
> not everything raised can - nor should - be included during soft freeze.
> On the other hand, I haven't seen much of discussion as to merit of
> inclusion of particular patches mentioned in this soft freeze thread.
> Did the discussion happen anywhere else? In general, how does the
> project decide on which exception requests to grant and which to shelve?
>
>
Hi Ihar,

Since your patches were submitted before the soft freeze deadline I think
they are definitely candidates to be included for 20.06.

Even if the branch-20.06 is created, we can backport your patches.
I see that you have received Ack from Dumitru for your series.

I applied the first 4 patches of your series to master just a while back.
I didn't get the chance to review your other 2 patches and I'm also waiting
for Han if he has any comments for the other 2 patches.

Thanks
Numan


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

Re: [ovs-dev] [PATCH ovn 0/6 v7] Support logical switches with multiple localnet ports

On Thu, May 14, 2020 at 4:00 PM Dumitru Ceara  wrote:

> On 5/13/20 3:38 PM, Ihar Hrachyshka wrote:
> > Hi all,
> >
> > This series is to allow for multiple localnet ports to be present in a
> > logical switch. Even before the series, it was allowed to create
> > multiple ports of this kind but they were handled inconsistently.
> >
> > This series uses logical switches with multiple localnet ports
> > (LSMLP) to implement what is called "routed provider networks" in
> > OpenStack. To elaborate, this allows to natively model a network that
> > has multiple segments with implied L3 routing between the segments
> > realized by the fabric. A user can operate such a network as a single
> > entity instead of deciding on which segment to choose for their
> > port bindings.
> >
> > The assumption in this implementation is that while a logical switch can
> > have multiple localnet ports, each chassis has only one of corresponding
> > physical networks mapped. Meaning, if the same LS has localnet ports for
> > networks A, B, and C, then each chassis can either be mapped to A, B, or
> > C, but not several of the networks. (Note this doesn't mean that a
> > chassis can't be mapped to network D, as long as it doesn't have a
> > corresponding localnet port in this same logical switch.)
> >
> > Ihar Hrachyshka (6):
> >   Spin out flow generation into build_dhcpv4_options_flows
> >   Spin out flow generation into build_dhcpv6_options_flows
> >   Spin out flow generation into build_pre_acl_flows
> >   Spin out flow generation into
> > build_drop_arp_nd_flows_for_unbound_router_ports
> >   Support logical switches with multiple localnet ports
> >   Log missing bridge per localnet port just once
>
> For the series:
>
> Acked-by: Dumitru Ceara 
>

Hi Ihar,

Thanks for the patches and thanks Dumitru for the reviews.

I applied the first 4 patches of this series to master.
I didn't get the chance to look into the other two. Also I'm waiting
to see if Han has any further comments.

Thanks
Numan


>
> Thanks,
> Dumitru
>
> >
> > ---
> > v2: - rebase on top of series that refactors code dealing with
> >   localnet ports.
> > - tests: send packets both ways, more test scenarios covered.
> > - use x2nrealloc to allocate ->localnet_ports.
> > - use n_localnet_ports counter instead of localnet_ports pointer
> >   to detect switches with localnet ports.
> > v3: - adjusted documentation to be more explicit about how multiple
> >   localnet ports scenario should be used in practice.
> > - more tests (broadcast, multiple co-hosted switches with multiple
> >   localnet ports)
> > v4: - sent as a series, fixed test description to reflect we test
> >   broadcast only.
> > v5: - fixed a test case failure on slower machines due to service
> >   broadcast traffic captured.
> > - rearranged parameters in new functions to keep output parameters
> >   at the end.
> > v6: - fixed several memory leaks due to struct ds not destroyed /
> >   char* not freed.
> > - explained why we don't rate limit messages about unbound
> >   localnet ports.
> > - docs: fixed a missing space between sentences.
> > - nit: rearranged code inside controller/patch.c to avoid an `if`.
> > v7: - simplified new build_* functions by removing stage_hint
> >   calculation.
> > - simplified signature of build_pre_acl_flows_for_nbsp.
> > - renamed build_pre_acl_flows_for_nbsp -> build_pre_acl_flows.
> > - nit: removed redundant newlines between new build_* functions.
> > ---
> >
> >  controller/binding.c   |  16 ++
> >  controller/patch.c |  31 ++-
> >  northd/ovn-northd.c| 474 --
> >  ovn-architecture.7.xml |  50 ++--
> >  ovn-nb.xml |  23 +-
> >  ovn-sb.xml |  21 +-
> >  tests/ovn.at   | 504 +
> >  7 files changed, 862 insertions(+), 257 deletions(-)
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH OVN v3] ovn-nbctl.c: Fix lr-policy-del command

On Fri, May 15, 2020 at 1:45 PM Tao YunXiang <
taoyunxi...@cmss.chinamobile.com> wrote:

> This change will check the existence of lr-policy uuid.
> If not, it will print "uuid is not found".
>
> Fixes: 1b030874c32("ovn-nbctl.c: Add an optional way to delete router
> policy by uuid")
>
>
> Author: Tao YunXiang 
> Signed-off-by: Tao YunXiang 
> CC: Dumitru Ceara 
>
> ---
>  utilities/ovn-nbctl.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index f4da7c8ed..185146ac9 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -3647,6 +3647,10 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
>  new_policies[n_policies++] = lr->policies[i];
>  }
>  }
> +if (n_policies == lr->n_policies) {
> +ctl_error(ctx, "uuid is not found");
> +}
>

Thanks for the patch.

I applied this patch to master with the below 2 changes

--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -3648,7 +3648,7 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
 }
 }
 if (n_policies == lr->n_policies) {
-ctl_error(ctx, "uuid is not found");
+ctl_error(ctx, "Logical router policy uuid is not found.");
+return;
 }


Thanks
Numan


> +
>  /* If match is not specified, delete all routing policies with the
>   * specified priority. */
>  } else {
> --
> 2.17.1
>
>
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovn-northd: Increase ECMP route priority with 400.

2020-05-15 Thread Lorenzo Bianconi

> > On Thu, May 14, 2020 at 1:12 AM Dumitru Ceara  wrote:
> > >
> > > On 5/13/20 11:51 PM, Han Zhou wrote:
> > > > In commit c0bf32d the priorities of the regular routes were increased by
> > > > 400, but ECMP routes were not updated. This patch fixes it. Since
> > > > for ECMP routes the outport is unknown (there can be many different
> > > > outports) the condition check of whether the outport is distributed
> > > > gateway port is skipped.
> > > >
> > > > Fixes: c0bf32d ("Manage ARP process locally in a DVR scenario")
> > > > Cc: Lorenzo Bianconi 
> > > > Signed-off-by: Han Zhou 
> 
> [...]
> 
> > So I wonder if we should change the solution of the commit c0bf32d, instead
> > of just increasing the ECMP routes priority only. I don't completely
> > understand the problem that commit was trying to solve. Is there an example
> > that describes the scenario in more detail and the reason why the route
> > priorities have to be different?
> 
> Hi Han,
> 
> this morning Dumitru and me worked trying to find a solution for this issue.
> We though to restore the original configuration in table 9
> and add a new table used only to take care of FIP/DVR traffic.
> Does it work for you?
> I sent a RFC with the basic implementation. I tested it and it works fine
> regarding to FIP traffic. I will work on unitest waiting for feedbacks.
> 
> Regards,
> Lorenzo

ops I forgot, this is the patch:
https://patchwork.ozlabs.org/project/openvswitch/patch/a99d46b275a10a6d8278434f7cefa0466bb78af5.1589557561.git.lorenzo.bianc...@redhat.com/

Regards,
Lorenzo

> 
> > 
> > >
> > > > ---
> > > >  northd/ovn-northd.8.xml |  3 ++-
> > > >  northd/ovn-northd.c | 22 --
> > > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > > > index 8f224b0..b0475d0 100644
> > > > --- a/northd/ovn-northd.8.xml
> > > > +++ b/northd/ovn-northd.8.xml
> > > > @@ -2601,7 +2601,8 @@ next;
> > > >ids MID1, MID2, ..., a logical flow
> > with match
> > > >in CIDR notation ip4.dst ==
> > N/M,
> > > >or ip6.dst == N/M, whose
> > priority
> > > > -  is the integer value of M, has the following
> > actions:
> > > > +  is 400 + the integer value of M, has
> > the
> > > > +  following actions:
> > > >  
> > > >
> > > >  
> > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > > index b25152d..c02e305 100644
> > > > --- a/northd/ovn-northd.c
> > > > +++ b/northd/ovn-northd.c
> > > > @@ -7337,7 +7337,8 @@ build_route_prefix_s(const struct v46_ip *prefix,
> > unsigned int plen)
> > > >  }
> > > >
> > > >  static void
> > > > -build_route_match(const struct ovn_port *op_inport, const char
> > *network_s,
> > > > +build_route_match(const struct ovn_port *op_inport,
> > > > +  const struct ovn_port *op_outport, const char
> > *network_s,
> > > >int plen, bool is_src_route, bool is_ipv4, struct ds
> > *match,
> > > >uint16_t *priority)
> > > >  {
> > > > @@ -7351,6 +7352,13 @@ build_route_match(const struct ovn_port
> > *op_inport, const char *network_s,
> > > >  dir = "dst";
> > > >  *priority = (plen * 2) + 1;
> > > >  }
> > > > +/* traffic for internal IPs of logical switch ports must be sent to
> > > > + * the gw controller through the overlay tunnels
> > > > + */
> > > > +if (!op_outport ||
> > > > +(op_outport->nbrp && !op_outport->nbrp->n_gateway_chassis)) {
> > > > +priority += DROUTE_PRIO;
> > > > +}
> > > >
> > > >  if (op_inport) {
> > > >  ds_put_format(match, "inport == %s && ", op_inport->json_key);
> > > > @@ -7434,8 +7442,8 @@ build_ecmp_route_flow(struct hmap *lflows, struct
> > ovn_datapath *od,
> > > >  uint16_t priority;
> > > >
> > > >  char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
> > > > -build_route_match(NULL, prefix_s, eg->plen, eg->is_src_route,
> > is_ipv4,
> > > > -  &match, &priority);
> > > > +build_route_match(NULL, NULL, prefix_s, eg->plen, eg->is_src_route,
> > > > +  is_ipv4, &match, &priority);
> > > >  free(prefix_s);
> > > >
> > > >  struct ds actions = DS_EMPTY_INITIALIZER;
> > > > @@ -7548,14 +7556,8 @@ add_route(struct hmap *lflows, const struct
> > ovn_port *op,
> > > >  op_inport = op;
> > > >  }
> > > >  }
> > > > -build_route_match(op_inport, network_s, plen, is_src_route,
> > is_ipv4,
> > > > +build_route_match(op_inport, op, network_s, plen, is_src_route,
> > is_ipv4,
> > > >&match, &priority);
> > > > -/* traffic for internal IPs of logical switch ports must be sent to
> > > > - * the gw controller through the overlay tunnels
> > > > - */
> > > > -if (op->nbrp && !op->nbrp->n_gateway_chassis) {
> > > > -priority += DROUTE_PRIO;
> > > > -}
> > > >
>

Re: [ovs-dev] [PATCH ovn] ovn-northd: Increase ECMP route priority with 400.

2020-05-15 Thread Lorenzo Bianconi

> On Thu, May 14, 2020 at 1:12 AM Dumitru Ceara  wrote:
> >
> > On 5/13/20 11:51 PM, Han Zhou wrote:
> > > In commit c0bf32d the priorities of the regular routes were increased by
> > > 400, but ECMP routes were not updated. This patch fixes it. Since
> > > for ECMP routes the outport is unknown (there can be many different
> > > outports) the condition check of whether the outport is distributed
> > > gateway port is skipped.
> > >
> > > Fixes: c0bf32d ("Manage ARP process locally in a DVR scenario")
> > > Cc: Lorenzo Bianconi 
> > > Signed-off-by: Han Zhou 

[...]

> So I wonder if we should change the solution of the commit c0bf32d, instead
> of just increasing the ECMP routes priority only. I don't completely
> understand the problem that commit was trying to solve. Is there an example
> that describes the scenario in more detail and the reason why the route
> priorities have to be different?

Hi Han,

this morning Dumitru and me worked trying to find a solution for this issue.
We though to restore the original configuration in table 9
and add a new table used only to take care of FIP/DVR traffic.
Does it work for you?
I sent a RFC with the basic implementation. I tested it and it works fine
regarding to FIP traffic. I will work on unitest waiting for feedbacks.

Regards,
Lorenzo

> 
> >
> > > ---
> > >  northd/ovn-northd.8.xml |  3 ++-
> > >  northd/ovn-northd.c | 22 --
> > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > > index 8f224b0..b0475d0 100644
> > > --- a/northd/ovn-northd.8.xml
> > > +++ b/northd/ovn-northd.8.xml
> > > @@ -2601,7 +2601,8 @@ next;
> > >ids MID1, MID2, ..., a logical flow
> with match
> > >in CIDR notation ip4.dst ==
> N/M,
> > >or ip6.dst == N/M, whose
> priority
> > > -  is the integer value of M, has the following
> actions:
> > > +  is 400 + the integer value of M, has
> the
> > > +  following actions:
> > >  
> > >
> > >  
> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > index b25152d..c02e305 100644
> > > --- a/northd/ovn-northd.c
> > > +++ b/northd/ovn-northd.c
> > > @@ -7337,7 +7337,8 @@ build_route_prefix_s(const struct v46_ip *prefix,
> unsigned int plen)
> > >  }
> > >
> > >  static void
> > > -build_route_match(const struct ovn_port *op_inport, const char
> *network_s,
> > > +build_route_match(const struct ovn_port *op_inport,
> > > +  const struct ovn_port *op_outport, const char
> *network_s,
> > >int plen, bool is_src_route, bool is_ipv4, struct ds
> *match,
> > >uint16_t *priority)
> > >  {
> > > @@ -7351,6 +7352,13 @@ build_route_match(const struct ovn_port
> *op_inport, const char *network_s,
> > >  dir = "dst";
> > >  *priority = (plen * 2) + 1;
> > >  }
> > > +/* traffic for internal IPs of logical switch ports must be sent to
> > > + * the gw controller through the overlay tunnels
> > > + */
> > > +if (!op_outport ||
> > > +(op_outport->nbrp && !op_outport->nbrp->n_gateway_chassis)) {
> > > +priority += DROUTE_PRIO;
> > > +}
> > >
> > >  if (op_inport) {
> > >  ds_put_format(match, "inport == %s && ", op_inport->json_key);
> > > @@ -7434,8 +7442,8 @@ build_ecmp_route_flow(struct hmap *lflows, struct
> ovn_datapath *od,
> > >  uint16_t priority;
> > >
> > >  char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
> > > -build_route_match(NULL, prefix_s, eg->plen, eg->is_src_route,
> is_ipv4,
> > > -  &match, &priority);
> > > +build_route_match(NULL, NULL, prefix_s, eg->plen, eg->is_src_route,
> > > +  is_ipv4, &match, &priority);
> > >  free(prefix_s);
> > >
> > >  struct ds actions = DS_EMPTY_INITIALIZER;
> > > @@ -7548,14 +7556,8 @@ add_route(struct hmap *lflows, const struct
> ovn_port *op,
> > >  op_inport = op;
> > >  }
> > >  }
> > > -build_route_match(op_inport, network_s, plen, is_src_route,
> is_ipv4,
> > > +build_route_match(op_inport, op, network_s, plen, is_src_route,
> is_ipv4,
> > >&match, &priority);
> > > -/* traffic for internal IPs of logical switch ports must be sent to
> > > - * the gw controller through the overlay tunnels
> > > - */
> > > -if (op->nbrp && !op->nbrp->n_gateway_chassis) {
> > > -priority += DROUTE_PRIO;
> > > -}
> > >
> > >  struct ds actions = DS_EMPTY_INITIALIZER;
> > >  ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %sreg0
> = ",
> > >
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] controller: Free the qos hmap built in binding_run().

On Fri, May 15, 2020 at 5:22 PM Dumitru Ceara  wrote:

> On 5/15/20 12:17 PM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > Fixes the memory leak because of this.
> >
> > Signed-off-by: Numan Siddique 
>
> Looks good to me.
>
> Acked-by: Dumitru Ceara 
>

Thanks Dumitru.

I applied this patch to master and branch-20.03.

Thanks
Numan


>
> Thanks,
> Dumitru
>
> > ---
> >  controller/binding.c | 13 -
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 9d37a23cc..a5525a310 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -390,6 +390,17 @@ setup_qos(const char *egress_iface, struct hmap
> *queue_map)
> >  netdev_close(netdev_phy);
> >  }
> >
> > +static void
> > +destroy_qos_map(struct hmap *qos_map)
> > +{
> > +struct qos_queue *qos_queue;
> > +HMAP_FOR_EACH_POP (qos_queue, node, qos_map) {
> > +free(qos_queue);
> > +}
> > +
> > +hmap_destroy(qos_map);
> > +}
> > +
> >  static void
> >  update_local_lport_ids(struct sset *local_lport_ids,
> > const struct sbrec_port_binding *binding_rec)
> > @@ -792,7 +803,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
> binding_ctx_out *b_ctx_out)
> >
> >  shash_destroy(&lport_to_iface);
> >  sset_destroy(&egress_ifaces);
> > -hmap_destroy(&qos_map);
> > +destroy_qos_map(&qos_map);
> >  }
> >
> >  /* Returns true if port-binding changes potentially require flow
> changes on
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC ovn] ovn: introduce IP_SRC_POLICY stage in ingress router pipeline

2020-05-15 Thread Lorenzo Bianconi

In order to fix the issues introduced by commit
c0bf32d72f8b ("Manage ARP process locally in a DVR scenario "), restore
previous configuration of table 9 in ingress router pipeline and
introduce a new stage called 'ip_src_policy' used to set the src address
info in order to not distribute FIP traffic if DVR is enabled

Fixes: c0bf32d72f8b ("Manage ARP process locally in a DVR scenario ")
Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.8.xml | 65 -
 northd/ovn-northd.c | 38 ++--
 tests/ovn.at| 28 +-
 3 files changed, 54 insertions(+), 77 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 8f224b07f..09dbb52b4 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2484,37 +2484,6 @@ output;
 
   
 
-  
-
-  For distributed logical routers where one of the logical router ports
-  specifies a redirect-chassis, a priority-400 logical
-  flow for each dnat_and_snat NAT rules configured.
-  These flows will allow to properly forward traffic to the external
-  connections if available and avoid sending it through the tunnel.
-  Assuming the following NAT rule has been configured:
-
-
-
-external_ip = A;
-external_mac = B;
-logical_ip = C;
-
-
-
-  the following action will be applied:
-
-
-
-ip.ttl--;
-reg0 = ip.dst;
-reg1 = A;
-eth.src = B;
-outport = router-port;
-next;
-
-
-  
-
   
 
   IPv4 routing table.  For each route to IPv4 network N with
@@ -2660,7 +2629,35 @@ outport = P;
   
 
 
-Ingress Table 12: ARP/ND Resolution
+Ingress Table 12: IP Source Policy
+
+
+  This table contains for distributed logical routers where one of
+  the logical router ports specifies a redirect-chassis,
+  a priority-100 logical flow for each dnat_and_snat
+  NAT rules configured.
+  These flows will allow to properly forward traffic to the external
+  connections if available and avoid sending it through the tunnel.
+  Assuming the following NAT rule has been configured:
+
+
+
+external_ip = A;
+external_mac = B;
+logical_ip = C;
+
+
+
+  the following action will be applied:
+
+
+
+reg1 = A;
+eth.src = B;
+next;
+
+
+Ingress Table 13: ARP/ND Resolution
 
 
   Any packet that reaches this table is an IP packet whose next-hop
@@ -2819,7 +2816,7 @@ outport = P;
 
 
 
-Ingress Table 13: Check packet length
+Ingress Table 14: Check packet length
 
 
   For distributed logical routers with distributed gateway port configured
@@ -2849,7 +2846,7 @@ REGBIT_PKT_LARGER = check_pkt_larger(L); next;
   and advances to the next table.
 
 
-Ingress Table 14: Handle larger packets
+Ingress Table 15: Handle larger packets
 
 
   For distributed logical routers with distributed gateway port configured
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3c0070ea7..d5f3997a9 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -175,11 +175,12 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  9, "lr_in_ip_routing")   \
 PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_ECMP, 10, "lr_in_ip_routing_ecmp") \
 PIPELINE_STAGE(ROUTER, IN,  POLICY,  11, "lr_in_policy")   \
-PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 12, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   ,  13, "lr_in_chk_pkt_len")   \
-PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 14,"lr_in_larger_pkts")   \
-PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 15, "lr_in_gw_redirect")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 16, "lr_in_arp_request")  \
+PIPELINE_STAGE(ROUTER, IN,  IP_SRC_POLICY,   12, "lr_in_ip_src_policy") \
+PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 13, "lr_in_arp_resolve")  \
+PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   ,  14, "lr_in_chk_pkt_len")   \
+PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 15,"lr_in_larger_pkts")   \
+PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 16, "lr_in_gw_redirect")  \
+PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 17, "lr_in_arp_request")  \
   \
 /* Logical router egress stages. */   \
 PIPELINE_STAGE(ROUTER, OUT, UNDNAT,0, "lr_out_undnat")\
@@ -7103,8 +7104,6 @@ build_routing_policy_flow(struct hmap *lflows, struct 
ovn_datapath *od,
 ds_destroy(&actions);
 }
 
-/* default logical flow prioriry for distributed routes */
-#define DROUTE_PRIO 400
 struct parsed_route {
 struct ovs_list list_node;
 struct v46_ip prefix;
@@ -7493,7 +7492,7 @@ build_ecmp_route_flow(struct hmap *lflows, struct 
ovn_datapath *od,
 }
 
 static void
-add_distributed_routes(struct hmap *lflows,

Re: [ovs-dev] Windows OVS issue

2020-05-15 Thread Alin Serdean

Hello Wenying,

Trimming the email a bit.

Please see my comments inlined.

> Hi,
> 
> I am using OVS on Windows Server 2019 to provide container networking.
> OpenFlow is used to support both container and host networking, and OVS
> conntrack feature is also used on my setup. I find a lot of error or warning 
> in
> ovs-vswitchd log, are there any thoughts about the errors?
> 
> e.g.,
> 2020-04-27T19:54:27.142Z|34004|odp_util(handler7)|WARN|Dropped 56
> log messages in last 59 seconds (most recently, 1 seconds ago) due to 
> excessive
> rate 2020-04-27T19:54:27.142Z|34005|odp_util(handler7)|WARN|the flow
> key in error is:
> recirc_id(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),ct_tuple4(src=0.0.0.
> 0,dst=0.0.0.0,proto=0,tp_src=0,tp_dst=0),eth(src=68:4f:64:15:61:9a,dst=01:0
> 0:0c:cc:cc:cd),in_port(1),eth_type(0x05ff)
> 2020-04-27T19:55:27.535Z|34006|odp_util(handler7)|WARN|Dropped 56
> log messages in last 59 seconds (most recently, 2 seconds ago) due to 
> excessive
> rate 2020-04-27T19:55:27.535Z|34007|odp_util(handler7)|WARN|invalid
> Ethertype 1535 in flow key 2020-04-
> 27T19:55:27.535Z|34008|odp_util(handler7)|WARN|Dropped 56 log
> messages in last 59 seconds (most recently, 2 seconds ago) due to excessive 
> rate
> 2020-04-27T19:55:27.535Z|34009|odp_util(handler7)|WARN|the flow key in
> error is:
> recirc_id(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),ct_tuple4(src=0.0.0.
> 0,dst=0.0.0.0,proto=0,tp_src=0,tp_dst=0),eth(src=68:4f:64:15:61:9a,dst=01:0
> 0:0c:cc:cc:cd),in_port(1),eth_type(0x05ff)

[Alin Serdean] From what I can see those are packets which are missing a valid
Ethernet type. 

> 
> Except for above error logs, there are also some igmp warnings printed:
> 
...
> 2020-04-27T19:45:53.412Z|33960|dpif(handler7)|WARN|system@ovs-
> system: execute ct(zone=65520,nat),recirc(0x59b7) failed (Unknown error) on
> packet
> igmp,vlan_tci=0x,dl_src=00:15:5d:49:62:bd,dl_dst=01:00:5e:00:00:16,nw
> _src=172.16.6.17,nw_dst=224.0.0.22,nw_tos=0,nw_ecn=0,nw_ttl=1,igmp_ty
> pe=34,igmp_code=0
> with metadata skb_priority(0),skb_mark(0),in_port(5) mtu 0

[Alin Serdean] The windows datapath does not support IGMP, only ICMP, TCP,
UDP and minimal support for FTP.

> 
> Thanks,
> Wenying
> ___
> 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] OVN 20.06 Soft Freeze Today

2020-05-15 Thread Ihar Hrachyshka


On 5/11/20 10:15 AM, Ihar Hrachyshka wrote:

On 5/8/20 2:17 PM, Numan Siddique wrote:



On Fri, May 8, 2020 at 9:58 PM Ihar Hrachyshka > wrote:


    Hi all,

    I would like the series to support multiple localnet ports per 
switch

    ("routed provider networks for OpenStack") considered for inclusion.

    The series is here:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=173328

    There's an updated (v3) version of 5/6 patch from the series here:
https://patchwork.ozlabs.org/project/openvswitch/patch/20200508154425.112544-1-ihrac...@redhat.com/

    (I'm not sure I posted it correctly - since other patches in the
    series
    didn't need any update from v2, I pushed only 5/6 but now it's
    displayed
    separately in patchwork; let me know if that's an issue and/or I
    should
    repost the whole series.)


I think it's better to repost as a whole series.



Thanks. Here is the full series (I also attached a cover letter): 
https://patchwork.ozlabs.org/project/openvswitch/list/?series=176134



Note there's one issue in the new test cases that I am working on 
right now and hope to fix a fix today. There will probably be v5 of 
the series but I hope we can pick any other issues in the 
implementation, reviews are welcome.




Hi all,

Bear with me since I am new to OVN release freeze process. Now that we 
approach(ed?) hard freeze, I wonder if there's still a chance to include 
the series mentioned above (there's v7 of the same up for review with 
one ack), or it will have to slip into a next release. I understand that 
not everything raised can - nor should - be included during soft freeze. 
On the other hand, I haven't seen much of discussion as to merit of 
inclusion of particular patches mentioned in this soft freeze thread. 
Did the discussion happen anywhere else? In general, how does the 
project decide on which exception requests to grant and which to shelve?


Thanks,

Ihar

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

[ovs-dev] [PATCHv2] ovsdb-idl: Fix NULL deref reported by Coverity.

2020-05-15 Thread William Tu

When 'datum.values' or 'datum.keys' is NULL, some code path calling
into ovsdb_idl_txn_write__ triggers NULL deref.

An example:
ovsrec_open_vswitch_set_cur_cfg(const struct ovsrec_open_vswitch
{
struct ovsdb_datum datum;
union ovsdb_atom key;

datum.n = 1;
datum.keys = &key;

key.integer = cur_cfg;
//  1. assign_zero: Assigning: datum.values = NULL.
datum.values = NULL;
//  CID 1421356 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
//  2. var_deref_model: Passing &datum to ovsdb_idl_txn_write_clone,\
// which dereferences null datum.values.
ovsdb_idl_txn_write_clone(&row->header_, &ovsrec_open_vswitch_col
}

And with the following calls:
ovsdb_idl_txn_write_clone
  ovsdb_idl_txn_write__
6. deref_parm_in_call: Function ovsdb_datum_destroy dereferences
   datum->values
ovsdb_datum_destroy

Signed-off-by: William Tu 
---
v2:
   - I applied previous patch e398275024e815b52with yifeng's comments,
 but accidently remove this chunk.  With this fix, the Coverity
 shows around 133 defects. (now it's around 300)
---
 lib/ovsdb-idl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 1535ad7b5197..6614ea1617ef 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -4449,7 +4449,8 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_,
  * transaction only does writes of existing values, without making any real
  * changes, we will drop the whole transaction later in
  * ovsdb_idl_txn_commit().) */
-if (write_only && ovsdb_datum_equals(ovsdb_idl_read(row, column),
+if (datum->keys && datum->values &&
+write_only && ovsdb_datum_equals(ovsdb_idl_read(row, column),
  datum, &column->type)) {
 goto discard_datum;
 }
-- 
2.7.4

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

Re: [ovs-dev] [PATCH ovn v5 4/9] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

2020-05-15 Thread Dumitru Ceara

On 5/15/20 2:23 PM, Numan Siddique wrote:
> 
> 
> On Wed, May 13, 2020 at 7:24 PM Dumitru Ceara  > wrote:
> 
> On 5/11/20 11:46 AM, num...@ovn.org  wrote:
> > From: Numan Siddique mailto:num...@ovn.org>>
> >
> > If ofctrl_check_and_add_flow(F') is called where flow F' has
> match-actions (M, A2)
> > and if there already exists a flow F with match-actions (M, A1) in
> the desired flow
> > table, then this function  doesn't update the existing flow F with
> new actions
> > actions A2.
> >
> > This patch is required for the upcoming patch in this series which
> > adds incremental processing for OVS interface in the flow output
> stage.
> > Since we will be not be clearing the flow output data if these changes
> > are handled incrementally, some of the existing flows will be updated
> > with new actions. One such example would be flows in physical
> > table OFTABLE_LOCAL_OUTPUT (table 33). And this patch is required to
> > update such flows. Otherwise we will have incomplete actions
> installed.
> >
> > Signed-off-by: Numan Siddique mailto:num...@ovn.org>>
> 
> Hi Numan,
> 
> I think the title of the commit should be "ofctrl: Replace the actions
> of an existing flow if actions have changed."
> 
> > ---
> >  controller/ofctrl.c | 23 ---
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index 4b51cd86e..8f2f13767 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct
> ovn_desired_flow_table *flow_table,
> > 
> >      ovn_flow_log(f, "ofctrl_add_flow");
> > 
> > -    if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) {
> > -        if (log_duplicate_flow) {
> > -            static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 5);
> > -            if (!VLOG_DROP_DBG(&rl)) {
> > -                char *s = ovn_flow_to_string(f);
> > -                VLOG_DBG("dropping duplicate flow: %s", s);
> > -                free(s);
> > +    struct ovn_flow *existing_f =
> > +        ovn_flow_lookup(&flow_table->match_flow_table, f, true);
> > +    if (existing_f) {
> > +        if (ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > +                          existing_f->ofpacts,
> existing_f->ofpacts_len)) {
> > +            if (log_duplicate_flow) {
> > +                static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 5);
> > +                if (!VLOG_DROP_DBG(&rl)) {
> > +                    char *s = ovn_flow_to_string(f);
> > +                    VLOG_DBG("dropping duplicate flow: %s", s);
> > +                    free(s);
> > +                }
> >              }
> > +        } else {
> > +            free(existing_f->ofpacts);
> > +            existing_f->ofpacts = xmemdup(f->ofpacts,
> f->ofpacts_len);
> > +            existing_f->ofpacts_len = f->ofpacts_len;
> 
> We could avoid the free(existing_f->ofpacts) followed by xmemdup by
> swapping f->ofpacts with existing_f->ofpacts (same for ofpacts_len).
> We'd probably need a function for that and we'd have to call it in
> ofctrl_add_or_append_flow() too.
> 
> 
> Good idea. I'll do as suggested. But I don't think we can do the same for
> ofctrl_add_or_append_flow() as it appends the actions.
> 

You're right, we can't do it in ofctrl_add_or_append_flow(), thanks for
double checking.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn v5 4/9] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

On Wed, May 13, 2020 at 7:24 PM Dumitru Ceara  wrote:

> On 5/11/20 11:46 AM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > If ofctrl_check_and_add_flow(F') is called where flow F' has
> match-actions (M, A2)
> > and if there already exists a flow F with match-actions (M, A1) in the
> desired flow
> > table, then this function  doesn't update the existing flow F with new
> actions
> > actions A2.
> >
> > This patch is required for the upcoming patch in this series which
> > adds incremental processing for OVS interface in the flow output stage.
> > Since we will be not be clearing the flow output data if these changes
> > are handled incrementally, some of the existing flows will be updated
> > with new actions. One such example would be flows in physical
> > table OFTABLE_LOCAL_OUTPUT (table 33). And this patch is required to
> > update such flows. Otherwise we will have incomplete actions installed.
> >
> > Signed-off-by: Numan Siddique 
>
> Hi Numan,
>
> I think the title of the commit should be "ofctrl: Replace the actions
> of an existing flow if actions have changed."
>
> > ---
> >  controller/ofctrl.c | 23 ---
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index 4b51cd86e..8f2f13767 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct
> ovn_desired_flow_table *flow_table,
> >
> >  ovn_flow_log(f, "ofctrl_add_flow");
> >
> > -if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) {
> > -if (log_duplicate_flow) {
> > -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 5);
> > -if (!VLOG_DROP_DBG(&rl)) {
> > -char *s = ovn_flow_to_string(f);
> > -VLOG_DBG("dropping duplicate flow: %s", s);
> > -free(s);
> > +struct ovn_flow *existing_f =
> > +ovn_flow_lookup(&flow_table->match_flow_table, f, true);
> > +if (existing_f) {
> > +if (ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > +  existing_f->ofpacts,
> existing_f->ofpacts_len)) {
> > +if (log_duplicate_flow) {
> > +static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 5);
> > +if (!VLOG_DROP_DBG(&rl)) {
> > +char *s = ovn_flow_to_string(f);
> > +VLOG_DBG("dropping duplicate flow: %s", s);
> > +free(s);
> > +}
> >  }
> > +} else {
> > +free(existing_f->ofpacts);
> > +existing_f->ofpacts = xmemdup(f->ofpacts, f->ofpacts_len);
> > +existing_f->ofpacts_len = f->ofpacts_len;
>
> We could avoid the free(existing_f->ofpacts) followed by xmemdup by
> swapping f->ofpacts with existing_f->ofpacts (same for ofpacts_len).
> We'd probably need a function for that and we'd have to call it in
> ofctrl_add_or_append_flow() too.
>
>
Good idea. I'll do as suggested. But I don't think we can do the same for
ofctrl_add_or_append_flow() as it appends the actions.

Thanks
Numan



> I'm not sure if that makes the code harder to read though. What do you
> think?
>
> Thanks,
> Dumitru
>
> >  }
> >  ovn_flow_destroy(f);
> >  return;
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v5 4/9] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

On Fri, May 15, 2020 at 6:50 AM Mark Michelson  wrote:

> On 5/11/20 5:46 AM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > If ofctrl_check_and_add_flow(F') is called where flow F' has
> match-actions (M, A2)
> > and if there already exists a flow F with match-actions (M, A1) in the
> desired flow
> > table, then this function  doesn't update the existing flow F with new
> actions
> > actions A2.
> >
> > This patch is required for the upcoming patch in this series which
> > adds incremental processing for OVS interface in the flow output stage.
> > Since we will be not be clearing the flow output data if these changes
> > are handled incrementally, some of the existing flows will be updated
> > with new actions. One such example would be flows in physical
> > table OFTABLE_LOCAL_OUTPUT (table 33). And this patch is required to
> > update such flows. Otherwise we will have incomplete actions installed.
>
> I understand the explanation for this patch, but I'm wondering if this
> now makes it possible to do silly things like define ACLs with duplicate
> matches but different verdicts. Previously, if you did this, the first
> ACL would get installed, and you'd see a debug message about dropping
> the duplicate flow. With this change, whichever ACL is processed last
> wins and there's no debug message.
>
> Maybe we could store a value in the ovn_flow that indicates when the
> flow was added to the desired flow table. Perhaps each time the
> incremental engine runs, an int is incremented. This way, you could have
> logic like:
>
> existing_f = ovn_flow_lookup();
> if (existing_f) {
>  if (existing_f->age == f->age || ofpacts_equal(existing_f, f)) {
>  // This is a duplicate flow
>  } else {
>  // replace existing_f's actions with f's actions
>  }
> }
>
>
I don't think we really need to maintain an age param for this.


I did some testing with the present master..

I added the below ACLs to a logical switch

ovn-nbctl acl-add sw0 from-lport 1002 "ip4.src == 10.0.0.4" "drop"
ovn-nbctl acl-add sw0 from-lport 1003 "ip4.src == 10.0.0.4" "allow"

And the corresponding OF flows are:

**
cookie=0xd23a2018, duration=6.057s, table=14, n_packets=0, n_bytes=0,
priority=2002,ip,metadata=0x1,nw_src=10.0.0.4 actions=drop
cookie=0x9bb3ead1, duration=3.218s, table=14, n_packets=0, n_bytes=0,
priority=2003,ip,metadata=0x1,nw_src=10.0.0.4 actions=resubmit(,15)
***

And then changed the second ACL with priority 1003 to 1002
(ovn-nbctl actl-add doesn't allow to add duplicate ACLs)

ovn-nbctl set acl $acl_id  priority=1002

After this I see the below flows:

 cookie=0x8515a20e, duration=15.873s, table=14, n_packets=0, n_bytes=0,
priority=2002,ip,metadata=0x1,nw_src=10.0.0.4 actions=resubmit(,15)

I was expecting to see a debug warning message in ovn-controller. But It
doesn't show up.

This happens because right now ovn_flow_lookup() is called here [1]
with cmp_sb_uuid set to true.

Since both the above ACLs will have different sb_uuid, ovn_flow_lookup()
fails.

I think we should pass 'cmp_sb_uuid' to false.

I think we should consider a flow as duplicate (with the same match) and
ignore it if the sb_uuid is different (as in the case of above ACLs)
And if sb_uuid matches, then we should replace the existing actions and
this is what this patch does.

Examples of such flows are flows with MC_Flood, MC_Unknown.

Thanks
Numan





> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   controller/ofctrl.c | 23 ---
> >   1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index 4b51cd86e..8f2f13767 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct
> ovn_desired_flow_table *flow_table,
> >
> >   ovn_flow_log(f, "ofctrl_add_flow");
> >
> > -if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) {
> > -if (log_duplicate_flow) {
> > -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 5);
> > -if (!VLOG_DROP_DBG(&rl)) {
> > -char *s = ovn_flow_to_string(f);
> > -VLOG_DBG("dropping duplicate flow: %s", s);
> > -free(s);
> > +struct ovn_flow *existing_f =
> > +ovn_flow_lookup(&flow_table->match_flow_table, f, true);
> > +if (existing_f) {
> > +if (ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > +  existing_f->ofpacts,
> existing_f->ofpacts_len)) {
> > +if (log_duplicate_flow) {
> > +static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 5);
> > +if (!VLOG_DROP_DBG(&rl)) {
> > +char *s = ovn_flow_to_string(f);
> > +VLOG_DBG("dropping duplicate flow: %s", s);
> > +free(s);
> > +}
> >   }
> > +} else {
> > +free(existing_f->ofpacts);
> > +exis

Re: [ovs-dev] [PATCH] ofproto: report coverage on hitting datapath flow limit

On Thu, May 14, 2020 at 8:06 PM William Tu  wrote:

> On Mon, Apr 20, 2020 at 07:13:42PM +0530, Gowrishankar Muthukrishnan wrote:
> > Whenever the number of flows in the datapath crosses above
> > the flow limit set/autoconfigured, it is helpful to report
> > this event through coverage counter for an operator/devops
> > engineer to know and take proactive corrections in the
> > switch configuration.
> >
> > Today, these events are reported in ovs vswitch log when
> > a new flow can not be inserted in upcall processing in which
> > case ovs writes a warning, otherwise an auto correction
> > made by ovs to flush old flows without any intimation at all.
> >
> > Signed-off-by: Gowrishankar Muthukrishnan 
> > ---
>
> Thanks, the patch looks good to me.
>
> I thought logging to ovs-vswitchd.log is good enough, because
> that's usually the first file we look, then if necessary we check
> the coverage log. Just curious, do you have some case where you
> keep seeing the flow_limit_hit frequently?
>
> I have not had a chance to see this event in deployments, but with a
coverage counter we can check *quickly* going forward.
Thanks for accepting the patch.

Regards,
Gowrishankar

> Acked-by: William Tu 
>
> >  ofproto/ofproto-dpif-upcall.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/ofproto/ofproto-dpif-upcall.c
> b/ofproto/ofproto-dpif-upcall.c
> > index 5e08ef10d..a76532ec7 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -56,6 +56,7 @@ COVERAGE_DEFINE(handler_duplicate_upcall);
> >  COVERAGE_DEFINE(upcall_ukey_contention);
> >  COVERAGE_DEFINE(upcall_ukey_replace);
> >  COVERAGE_DEFINE(revalidate_missed_dp_flow);
> > +COVERAGE_DEFINE(upcall_flow_limit_hit);
> >
> >  /* A thread that reads upcalls from dpif, forwards each upcall's packet,
> >   * and possibly sets up a kernel flow as a cache. */
> > @@ -1281,6 +1282,7 @@ should_install_flow(struct udpif *udpif, struct
> upcall *upcall)
> >
> >  atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
> >  if (udpif_get_n_flows(udpif) >= flow_limit) {
> > +COVERAGE_INC(upcall_flow_limit_hit);
> >  VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached");
> >  return false;
> >  }
> > @@ -2624,6 +2626,10 @@ revalidate(struct revalidator *revalidator)
> >   *   datapath flows, so we will recover before all the
> flows are
> >   *   gone.) */
> >  n_dp_flows = udpif_get_n_flows(udpif);
> > +if (n_dp_flows >= flow_limit) {
> > +COVERAGE_INC(upcall_flow_limit_hit);
> > +}
> > +
> >  kill_them_all = n_dp_flows > flow_limit * 2;
> >  max_idle = n_dp_flows > flow_limit ? 100 : ofproto_max_idle;
> >
> > --
> > 2.21.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>

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

Re: [ovs-dev] [PATCH ovn] controller: Free the qos hmap built in binding_run().

2020-05-15 Thread Dumitru Ceara

On 5/15/20 12:17 PM, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Fixes the memory leak because of this.
> 
> Signed-off-by: Numan Siddique 

Looks good to me.

Acked-by: Dumitru Ceara 

Thanks,
Dumitru

> ---
>  controller/binding.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/controller/binding.c b/controller/binding.c
> index 9d37a23cc..a5525a310 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -390,6 +390,17 @@ setup_qos(const char *egress_iface, struct hmap 
> *queue_map)
>  netdev_close(netdev_phy);
>  }
>  
> +static void
> +destroy_qos_map(struct hmap *qos_map)
> +{
> +struct qos_queue *qos_queue;
> +HMAP_FOR_EACH_POP (qos_queue, node, qos_map) {
> +free(qos_queue);
> +}
> +
> +hmap_destroy(qos_map);
> +}
> +
>  static void
>  update_local_lport_ids(struct sset *local_lport_ids,
> const struct sbrec_port_binding *binding_rec)
> @@ -792,7 +803,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct 
> binding_ctx_out *b_ctx_out)
>  
>  shash_destroy(&lport_to_iface);
>  sset_destroy(&egress_ifaces);
> -hmap_destroy(&qos_map);
> +destroy_qos_map(&qos_map);
>  }
>  
>  /* Returns true if port-binding changes potentially require flow changes on
> 

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

[ovs-dev] [PATCH] ovsdb: Remove duplicated include.

2020-05-15 Thread wangyunjian

From: Yunjian Wang 

Signed-off-by: Yunjian Wang 
---
 ovsdb/monitor.c | 1 -
 ovsdb/rbac.c| 1 -
 2 files changed, 2 deletions(-)

diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 1c66b42..532dedc 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -31,7 +31,6 @@
 #include "simap.h"
 #include "hash.h"
 #include "table.h"
-#include "hash.h"
 #include "timeval.h"
 #include "transaction.h"
 #include "jsonrpc-server.h"
diff --git a/ovsdb/rbac.c b/ovsdb/rbac.c
index b85ca9a..2986027 100644
--- a/ovsdb/rbac.c
+++ b/ovsdb/rbac.c
@@ -21,7 +21,6 @@
 
 #include "column.h"
 #include "condition.h"
-#include "condition.h"
 #include "file.h"
 #include "mutation.h"
 #include "openvswitch/vlog.h"
-- 
1.8.3.1


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

[ovs-dev] [PATCH] ofproto: Remove duplicated include.

2020-05-15 Thread wangyunjian

From: pandongyang 

Signed-off-by: pandongyang 
---
 ofproto/ofproto-dpif.h | 1 -
 ofproto/tunnel.c   | 2 --
 2 files changed, 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index aee61d6..2de8060 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -54,7 +54,6 @@
 #include "ovs-thread.h"
 #include "ofproto-provider.h"
 #include "util.h"
-#include "ovs-thread.h"
 
 struct dpif_flow_stats;
 struct ofproto_async_msg;
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 03f0ab7..3455ed2 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -13,8 +13,6 @@
  * limitations under the License. */
 
 #include 
-#include "tunnel.h"
-
 #include 
 
 #include "byte-order.h"
-- 
1.8.3.1


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

[ovs-dev] [PATCH ovn] controller: Free the qos hmap built in binding_run().

2020-05-15 Thread numans

From: Numan Siddique 

Fixes the memory leak because of this.

Signed-off-by: Numan Siddique 
---
 controller/binding.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/controller/binding.c b/controller/binding.c
index 9d37a23cc..a5525a310 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -390,6 +390,17 @@ setup_qos(const char *egress_iface, struct hmap *queue_map)
 netdev_close(netdev_phy);
 }
 
+static void
+destroy_qos_map(struct hmap *qos_map)
+{
+struct qos_queue *qos_queue;
+HMAP_FOR_EACH_POP (qos_queue, node, qos_map) {
+free(qos_queue);
+}
+
+hmap_destroy(qos_map);
+}
+
 static void
 update_local_lport_ids(struct sset *local_lport_ids,
const struct sbrec_port_binding *binding_rec)
@@ -792,7 +803,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct 
binding_ctx_out *b_ctx_out)
 
 shash_destroy(&lport_to_iface);
 sset_destroy(&egress_ifaces);
-hmap_destroy(&qos_map);
+destroy_qos_map(&qos_map);
 }
 
 /* Returns true if port-binding changes potentially require flow changes on
-- 
2.26.2

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

[ovs-dev] [PATCH OVN v3] ovn-nbctl.c: Fix lr-policy-del command

2020-05-15 Thread Tao YunXiang

This change will check the existence of lr-policy uuid.
If not, it will print "uuid is not found".

Fixes: 1b030874c32("ovn-nbctl.c: Add an optional way to delete router policy by 
uuid")


Author: Tao YunXiang 
Signed-off-by: Tao YunXiang 
CC: Dumitru Ceara 

---
 utilities/ovn-nbctl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index f4da7c8ed..185146ac9 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -3647,6 +3647,10 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
 new_policies[n_policies++] = lr->policies[i];
 }
 }
+if (n_policies == lr->n_policies) {
+ctl_error(ctx, "uuid is not found");
+}
+
 /* If match is not specified, delete all routing policies with the
  * specified priority. */
 } else {
-- 
2.17.1



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

[ovs-dev] [PATCH] odp-util.c: Fix dp_hash execution with slowpath actions.