[ovs-dev] [PATCH ovn] lflow.c: Use warn level log for a lflow that has neither dp nor dpg.

2021-04-21 Thread Han Zhou
This should not happen in normal cases, so at least should be a WARN. Signed-off-by: Han Zhou --- controller/lflow.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 680b8cca1..b8424e1fb 100644 --- a/controller/lflow.c +++

[ovs-dev] [PATCH ovn] ovn-controller.c: Remove extra local_lports_changed setting.

2021-04-21 Thread Han Zhou
Signed-off-by: Han Zhou --- controller/ovn-controller.c | 1 - 1 file changed, 1 deletion(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6f7c9ea61..86b92debb 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1400,7 +1400,6 @@

Re: [ovs-dev] [PATCH ovn v2] ovn-northd: bypass ct for allow ACLs

2021-04-21 Thread Ihar Hrachyshka
On Tue, Apr 13, 2021 at 4:02 AM Dumitru Ceara wrote: > > On 4/13/21 2:26 AM, Ihar Hrachyshka wrote: > > On Mon, Apr 12, 2021 at 5:00 AM Dumitru Ceara wrote: > >> > >> On 4/6/21 6:35 PM, Ihar Hrachyshka wrote: > >>> On Fri, Apr 2, 2021 at 7:23 AM Dumitru Ceara wrote: > > On 3/24/21

Re: [ovs-dev] [PATCH v2 02/11] mpsc-queue: Module for lock-free message passing

2021-04-21 Thread 0-day Robot
Bleep bloop. Greetings Gaëtan Rivet, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 97 characters long (recommended limit is 79) #201 FILE: lib/mpsc-queue.c:107:

Re: [ovs-dev] [PATCH v1 0/9] conntrack: improve multithread scalability

2021-04-21 Thread Gaëtan Rivet
On Wed, Feb 17, 2021, at 17:34, Gaetan Rivet wrote: > Conntracks are executed within the datapath. Locks along this path are crucial > and their critical section should be minimal. The global 'ct_lock' is > necessary > before any action taken on connection states. This lock is needed for many >

[ovs-dev] [PATCH v2 10/11] conntrack: Do not log empty ct-sweep

2021-04-21 Thread Gaetan Rivet
Do not add noise to the DBG log for empty sweeps. Only log time taken when some connections were cleaned. Signed-off-by: Gaetan Rivet Reviewed-by: Eli Britstein --- lib/conntrack.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index

[ovs-dev] [PATCH v2 09/11] conntrack: Do not rate limit ct-sweep

2021-04-21 Thread Gaetan Rivet
The current rate limit is set to allow other threads to update the connections when applicable. This was valid when taking the 'ct_lock' was needed with a global critical section. Now that the size of the critical section for 'ct_lock' is reduced, it is not necessary to rate limit calls to

[ovs-dev] [PATCH v2 11/11] conntrack: Use an atomic conn expiration value

2021-04-21 Thread Gaetan Rivet
A lock is taken during conn_lookup() to check whether a connection is expired before returning it. This lock can have some contention. Even though this lock ensures a consistent sequence of writes, it does not imply a specific order. A ct_clean thread taking the lock first could read a value that

[ovs-dev] [PATCH v2 07/11] conntrack: Inverse conn and ct lock precedence

2021-04-21 Thread Gaetan Rivet
The lock priority order is for the global 'ct_lock' to be taken first and then 'conn->lock'. This is an issue, as multiple operations on connections are thus blocked between threads contending on the global 'ct_lock'. This was previously necessary due to how the expiration lists, timeout policies

[ovs-dev] [PATCH v2 03/11] conntrack: Use mpsc-queue to store conn expirations

2021-04-21 Thread Gaetan Rivet
Change the connection expiration lists from ovs_list to mpsc-queue. This is a pre-step towards reducing the granularity of 'ct_lock'. It simplifies the responsibilities toward updating the expiration queue. The dataplane now appends the new conn for expiration once during creation. Any further

[ovs-dev] [PATCH v2 08/11] conntrack: Do not schedule zero ms timers

2021-04-21 Thread Gaetan Rivet
When ct_sweep() is far behind on its work, the 'next_wake' returned can be before the moment it started. When it happens, the thread schedules a zero ms timer that is logged as an error. Instead, mark the thread for immediate wake in the next poll_block(). Signed-off-by: Gaetan Rivet

[ovs-dev] [PATCH v2 06/11] conntrack-tp: Use a cmap to store timeout policies

2021-04-21 Thread Gaetan Rivet
Multiple lookups are done to stored timeout policies, each time blocking the global 'ct_lock'. This is usually not necessary and it should be acceptable to get policy updates slightly delayed (by one RCU sync at most). Using a CMAP reduces multiple lock taking and releasing in the connection

[ovs-dev] [PATCH v2 04/11] conntrack: Use a cmap to store zone limits

2021-04-21 Thread Gaetan Rivet
Change the data structure from hmap to cmap for zone limits. As they are shared amongst multiple conntrack users, multiple readers want to check the current zone limit state before progressing in their processing. Using a CMAP allows doing lookups without taking the global 'ct_lock', thus reducing

[ovs-dev] [PATCH v2 02/11] mpsc-queue: Module for lock-free message passing

2021-04-21 Thread Gaetan Rivet
Add a lockless multi-producer/single-consumer (MPSC), linked-list based, intrusive, unbounded queue that does not require deferred memory management. The queue is an implementation of the structure described by Dmitri Vyukov[1]. It adds a slightly more explicit API explaining the proper use of

[ovs-dev] [PATCH v2 05/11] conntrack: Init hash basis first at creation

2021-04-21 Thread Gaetan Rivet
The 'hash_basis' field is used sometimes during sub-systems init routine. It will be 0 by default before randomization. Sub-systems would then init some nodes with incorrect hash values. The timeout policies module is affected, making the default policy being referenced using an incorrect hash

[ovs-dev] [PATCH v2 01/11] ovs-atomic: Expose atomic exchange operation

2021-04-21 Thread Gaetan Rivet
The atomic exchange operation is a useful primitive that should be available as well. Most compiler already expose or offer a way to use it, but a single symbol needs to be defined. Signed-off-by: Gaetan Rivet Reviewed-by: Eli Britstein --- lib/ovs-atomic-c++.h | 3 +++

[ovs-dev] [PATCH v2 00/11] conntrack: improve multithread scalability

2021-04-21 Thread Gaetan Rivet
Conntracks are executed within the datapath. Locks along this path are crucial and their critical section should be minimal. The global 'ct_lock' is necessary before any action taken on connection states. This lock is needed for many operations on the conntrack, slowing down the datapath. The

Re: [ovs-dev] [PATCH v2 14/28] dpif-netdev: Quiesce offload thread periodically

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:20 PM, Gaetan Rivet wrote: > Similar to what was done for the PMD threads [1], reduce the performance > impact of quiescing too often in the offload thread. > > After each processed offload, the offload thread currently quiesce and > will sync with RCU. This synchronization can be

Re: [ovs-dev] [PATCH v2 08/28] dpif-netdev: Implement hardware offloads stats query

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > In the netdev datapath, keep track of the enqueued offloads between > the PMDs and the offload thread. Additionally, query each netdev > for their hardware offload counters. > > Signed-off-by: Gaetan Rivet > Reviewed-by: Eli Britstein > --- >

Re: [ovs-dev] [PATCH v2 07/28] mov-avg: Add a moving average helper structure

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > Add a new module offering a helper to compute the Cumulative > Moving Average (CMA) and the Exponential Moving Average (EMA) > of a series of values. > > Use the new helpers to add latency metrics in dpif-netdev. > > Signed-off-by: Gaetan Rivet >

Re: [ovs-dev] [PATCH v2 06/28] dpif-netdev: Rename offload thread structure

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > The offload management in userspace is done through a separate thread. > The naming of the structure holding the objects used for synchronization > with the dataplane is generic and nondescript. > > Clarify the object function by renaming it. > >

[ovs-dev] [PATCH v3 ovn] ovn-sbctl: Prevent core dump from ovn-sbctl lflow-list [datpath] 0xflow

2021-04-21 Thread Alexey Roytman
From: Alexey Roytman When ovn-sbctl lflow-list gets lflow argument with 0x prefix, e.g. 0x8131c8a8, it prints correct output, but fails with coredump. For example: ovn-sbctl --uuid lflow-list sw1 0x8131c8a8 Datapath: "sw1"

Re: [ovs-dev] [PATCH ovn v5 5/5] northd: Flood ARPs to routers for "unreachable" addresses.

2021-04-21 Thread Numan Siddique
On Tue, Apr 20, 2021 at 11:57 AM Mark Michelson wrote: > > Previously, ARP TPAs were filtered down only to "reachable" addresses. > Reachable addresses are all router interface addresses, as well as NAT > external addresses and load balancer VIPs that are within the subnet > handled by a router's

Re: [ovs-dev] [PATCH ovn v5 4/5] pinctrl: Add Chassis MAC_Bindings for all router addresses.

2021-04-21 Thread Numan Siddique
On Tue, Apr 20, 2021 at 11:57 AM Mark Michelson wrote: > > Previous behavior of ovn-controller was to only create MAC bindings for > the same addresses for which we would send gARPs. That is: > > * A logical switch has its "nat_addresses" configured. > * That logical switch has a localnet port. >

Re: [ovs-dev] [PATCH v2 05/28] dpif-netdev: Rename flow offload thread

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > ovs_strlcpy silently fails to copy the thread name if it is too long. > Rename the flow offload thread to differentiate it from the main thread. > > Fixes: 02bb2824e51d ("dpif-netdev: do hw flow offload in a thread") > Signed-off-by: Gaetan Rivet >

Re: [ovs-dev] [PATCH ovn v5 3/5] northd: Save all router addresses in Port_Bindings

2021-04-21 Thread Numan Siddique
On Tue, Apr 20, 2021 at 11:57 AM Mark Michelson wrote: > > northd currently stores logical switch port's configured "nat-addresses" > in the southbound Port_Binding "nat_addresses" column. This allows for > explicit configuration of which NAT addresses should be broadcast in > gARP messages by

Re: [ovs-dev] [PATCH v2 04/28] dpctl: Add function to read hardware offload statistics

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > Expose a function to query datapath offload statistics. > This function is separate from the current one in netdev-offload > as it exposes more detailed statistics from the datapath, instead of > only from the netdev-offload provider. > > Each datapath

Re: [ovs-dev] [PATCH v2 ovn] ovn-sbctl: Prevent core dump from ovn-sbctl lflow-list [datpath] 0xflow

2021-04-21 Thread Mark Michelson
Hi Alexey, nice find! I have a comment down below: On 4/20/21 6:22 PM, Alexey Roytman wrote: From: Alexey Roytman When ovn-sbctl lflow-list gets lflow argument with 0x prefix, e.g. 0x8131c8a8, it prints correct output, but fails with coredump. For example: ovn-sbctl --uuid lflow-list sw1

Re: [ovs-dev] [PATCH v2] conntrack: document NULL SNAT behavior and add a test case

2021-04-21 Thread Ilya Maximets
On 3/30/21 2:25 PM, Eelco Chaudron wrote: > Currently, conntrack in the kernel has an undocumented feature referred > to as NULL SNAT. Basically, when a source port collision is detected > during the commit, the source port will be translated to an ephemeral > port. If there is no collision, no

[ovs-dev] [PATCH net] openvswitch: meter: remove rate from the bucket size calculation

2021-04-21 Thread Ilya Maximets
Implementation of meters supposed to be a classic token bucket with 2 typical parameters: rate and burst size. Burst size in this schema is the maximum number of bytes/packets that could pass without being rate limited. Recent changes to userspace datapath made meter implementation to be in line

[ovs-dev] [PATCH] dpif-netdev: Remove meter rate from the bucket size calculation.

2021-04-21 Thread Ilya Maximets
Implementation of meters supposed to be a classic token bucket with 2 typical parameters: rate and burst size. Burst size in this schema is the maximum number of bytes/packets that could pass without being rate limited. Recent changes to userspace datapath made meter implementation to be in line

Re: [ovs-dev] [PATCH ovn] ovn-controller-vtep: Set chassis_name for newly created Encap.

2021-04-21 Thread Numan Siddique
On Wed, Apr 21, 2021 at 3:43 AM Dumitru Ceara wrote: > > On 4/20/21 8:55 PM, Odintsov Vladislav wrote: > > Hi, > > > > Thanks for the quick reaction! > > I've tested your patch against 20.06.3 and it works well for me. > > May I ask you to backport your patch down to supported branches? > > > >

Re: [ovs-dev] [PATCH v4] conntrack: handle SNAT with all-zero IP address

2021-04-21 Thread Aaron Conole
Paolo Valerio writes: > this patch introduces for the userspace datapath the handling > of rules like the following: > > ct(commit,nat(src=0.0.0.0),...) > > Kernel datapath already handle this case that is particularly > handy in scenarios like the following: > > Given A: 10.1.1.1, B:

Re: [ovs-dev] [PATCH v2] conntrack: document NULL SNAT behavior and add a test case

2021-04-21 Thread Aaron Conole
Eelco Chaudron writes: > Currently, conntrack in the kernel has an undocumented feature referred > to as NULL SNAT. Basically, when a source port collision is detected > during the commit, the source port will be translated to an ephemeral > port. If there is no collision, no SNAT is performed.

Re: [ovs-dev] [PATCH] ofproto/ofproto-dpif-sflow: Check sflow agent in case of race

2021-04-21 Thread Simon Horman
On Mon, Apr 19, 2021 at 10:26:04AM +0300, Gavin Li wrote: > sFlow agent may not exist while calling dpif_sflow_received. The sFlow > may be deleted in another thread, eg, by user, which will cause crash. > > Issue: 2576607 > Change-Id: Iad8202be1d06cda7d096109a2719ef934d245248 > Signed-off-by:

Re: [ovs-dev] [PATCH v2 03/28] netdev-offload-dpdk: Implement hw-offload statistics read

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > In the DPDK offload provider, keep track of inserted rte_flow and report > it when queried. > > Signed-off-by: Gaetan Rivet > Reviewed-by: Eli Britstein > --- > lib/netdev-offload-dpdk.c | 31 +++ > 1 file changed, 31

Re: [ovs-dev] [PATCH v2 02/28] netdev-offload-dpdk: Use per-netdev offload metadata

2021-04-21 Thread Maxime Coquelin
On 4/12/21 5:19 PM, Gaetan Rivet wrote: > Add a per-netdev offload data field as part of netdev hw_info structure. > Use this field in netdev-offload-dpdk to map offload metadata (ufid to > rte_flow). Use flow API deinit ops to destroy the per-netdev metadata > when deallocating a netdev. Use

Re: [ovs-dev] [PATCH ovn] tests: Run tests with Datapath Groups enabled/disabled.

2021-04-21 Thread Numan Siddique
On Tue, Apr 20, 2021 at 2:47 PM Mark Michelson wrote: > > Acked-by: Mark Michelson > Thanks Dumitru (and Mark). I applied the patch to the main branch. Numan > On 4/14/21 6:09 AM, Dumitru Ceara wrote: > > This doubles the size of the test matrix but should help protecting > > against

Re: [ovs-dev] [PATCH ovn v2] ovn-nbctl: fails to delete and add lr-nat in trx

2021-04-21 Thread Aidan Shribman
yes. will add a test On Wed, Apr 21, 2021, 10:51 Numan Siddique wrote: > On Thu, Apr 8, 2021 at 3:13 AM Aidan Shribman > wrote: > > > > The delete operation did not update the internal state cuasing the add > > operation to wrongly believe that the lr-nat entry is still present and > > thus

[ovs-dev] [PATCH] dpdk: Use DPDK 20.11.1 release

2021-04-21 Thread Hariprasad Govindharajan
Modify ci linux build script to use the latest DPDK stable release. Modify Documentation to use the latest DPDK stable release 20.11.1 Update NEWS file to reflect the latest DPDK stable releases. FAQ is updated to reflect the latest DPDK for each branch. Signed-off-by: Hariprasad Govindharajan

[ovs-dev] [PATCH branch-2.15] dpdk: Use DPDK 20.11.1 release

2021-04-21 Thread Hariprasad Govindharajan
Modify ci linux build script to use the latest DPDK stable release. Modify Documentation to use the latest DPDK stable release 20.11.1 Update NEWS file to reflect the latest DPDK stable releases. FAQ is updated to reflect the latest DPDK for each branch. Signed-off-by: Hariprasad Govindharajan

Re: [ovs-dev] [PATCH ovn v2] controller: Monitor all logical flows that refer to datapath groups.

2021-04-21 Thread Ilya Maximets
On 4/21/21 9:40 AM, Dumitru Ceara wrote: > On 4/20/21 8:54 PM, Han Zhou wrote: >> On Tue, Apr 20, 2021 at 12:43 AM Dumitru Ceara wrote: >>> >>> On 4/19/21 7:24 PM, Han Zhou wrote: On Mon, Apr 19, 2021 at 12:27 AM Dumitru Ceara >> wrote: > > On 4/16/21 7:16 PM, Mark Michelson wrote:

Re: [ovs-dev] [PATCH ovn 0/2] ovn-ctl: stop databases with stop_ovn_daemon()

2021-04-21 Thread Numan Siddique
On Tue, Apr 20, 2021 at 1:06 PM Mark Michelson wrote: > > Hi Dan, > > Acked-by: Mark Michelson > > for both patches in the series. Thanks Dan (and Mark). I applied the patches to the main branch. Numan > > On 4/12/21 10:58 AM, Dan Williams wrote: > > Wait for the databases to actually exit

Re: [ovs-dev] [PATCH ovn v2] ovn-nbctl: fails to delete and add lr-nat in trx

2021-04-21 Thread Numan Siddique
On Thu, Apr 8, 2021 at 3:13 AM Aidan Shribman wrote: > > The delete operation did not update the internal state cuasing the add > operation to wrongly believe that the lr-nat entry is still present and > thus can't be added. > > Below is the sequance is failed before and now passes: > > *** >

[ovs-dev] [PATCH 1/3] dpif-netdev: Do not flush PMD offloads on reload

2021-04-21 Thread Eli Britstein
Before flushing offloads of a removed port was supported by [1], it was necessary to flush the 'marks'. In doing so, all offloads of the PMD are removed, include the ones that are not related to the removed port and that are not modified following this removal. As a result such flows are evicted

[ovs-dev] [PATCH 2/3] dpif-netdev: Fix offloads of modified flows

2021-04-21 Thread Eli Britstein
Association of a mark to a flow is done as part of its offload handling, in the offloading thread. However, the PMD thread specifies whether an offload request is an "add" or "modify" by the association of a mark to the flow. This is exposed to a race condition. A flow might be created with

[ovs-dev] [PATCH 3/3] dpif-netdev: Log flow modification in debug level

2021-04-21 Thread Eli Britstein
Log flow modifications to help debugging. Signed-off-by: Eli Britstein Reviewed-by: Gaetan Rivet --- lib/dpif-netdev.c | 101 +- 1 file changed, 55 insertions(+), 46 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index

[ovs-dev] [PATCH 0/3] dpif-netdev offload transitions

2021-04-21 Thread Eli Britstein
This patch-set improves offloads transitions behavior. Patch #1 avoids flushing PMD offloads unnecessarily. Patch #2 fixes a race condition with flow modifications. Patch #3 improves debuggability of flow modifications. Travis: v1: https://travis-ci.org/github/elibritstein/OVS/builds/767839987

Re: [ovs-dev] [PATCH V6 00/13] Netdev vxlan-decap offload

2021-04-21 Thread Kovacevic, Marko
> On 4/7/2021 12:10 PM, Sriharsha Basavapatna wrote: > > > > On Sun, Apr 4, 2021 at 3:25 PM Eli Britstein > > wrote: > > > > VXLAN decap in OVS-DPDK configuration consists of two flows: > > F1: in_port(ens1f0),eth(),ipv4(),udp(), > >

Re: [ovs-dev] [PATCH ovn] ovn-controller-vtep: Set chassis_name for newly created Encap.

2021-04-21 Thread Dumitru Ceara
On 4/20/21 8:55 PM, Odintsov Vladislav wrote: > Hi, > > Thanks for the quick reaction! > I've tested your patch against 20.06.3 and it works well for me. > May I ask you to backport your patch down to supported branches? > > Tested-by: Vladislav Odintsov Thanks for trying it out, Vladislav,

Re: [ovs-dev] [PATCH ovn v2] controller: Monitor all logical flows that refer to datapath groups.

2021-04-21 Thread Dumitru Ceara
On 4/20/21 8:54 PM, Han Zhou wrote: > On Tue, Apr 20, 2021 at 12:43 AM Dumitru Ceara wrote: >> >> On 4/19/21 7:24 PM, Han Zhou wrote: >>> On Mon, Apr 19, 2021 at 12:27 AM Dumitru Ceara > wrote: On 4/16/21 7:16 PM, Mark Michelson wrote: > Thanks for the explanation Dumitru. It made