Re: [ovs-dev] [PATCH] tests: Skip netlink policy test on non-Linux platforms.

2021-08-27 Thread Frode Nordahl
lør. 28. aug. 2021, 03:57 skrev Ilya Maximets : > FreeBSD tests in Cirrus CI are broken and, I guess, windows tests too: > > 89. library.at:258: testing netlink policy ... > ./library.at:259: ovstest test-netlink-policy ll_addr > --- /dev/null 2021-08-20 19:02:41.907547000 + > +++

[ovs-dev] [PATCH] tests: Skip netlink policy test on non-Linux platforms.

2021-08-27 Thread Ilya Maximets
FreeBSD tests in Cirrus CI are broken and, I guess, windows tests too: 89. library.at:258: testing netlink policy ... ./library.at:259: ovstest test-netlink-policy ll_addr --- /dev/null 2021-08-20 19:02:41.907547000 + +++ /tmp/cirrus-ci-build/tests/testsuite.dir/at-groups/89/stderr

Re: [ovs-dev] [PATCH v6 1/1] ovsdb-idl : Add APIs to query if a table and a column is present or not.

2021-08-27 Thread Ilya Maximets
On 8/27/21 12:56 AM, num...@ovn.org wrote: > From: Numan Siddique > > This patch adds 2 new APIs in the ovsdb-idl client library > - ovsdb_idl_server_has_table() and ovsdb_idl_server_has_column() to > query if a table and a column is present in the IDL or not. This > patch also adds IDL helper

Re: [ovs-dev] [PATCH ovn v2] Revert features detection and zero-snat patches.

2021-08-27 Thread Numan Siddique
On Fri, Aug 27, 2021 at 8:06 PM Mark Michelson wrote: > > Thank you both for the quick reviews. I pushed the change to master. Would this require a backport to branch-21.06 ? Numan > > On 8/27/21 5:28 PM, Flavio Fernandes wrote: > > > > > >> On Aug 27, 2021, at 4:11 PM, Mark Michelson >>

Re: [ovs-dev] [PATCH ovn v3 1/7] ovn-sb: Add plugged_by column to Port_Binding.

2021-08-27 Thread Han Zhou
On Wed, Aug 25, 2021 at 11:00 PM Frode Nordahl wrote: > > > + > > > + > > > +If set, OVN will attempt to to perform plugging of this VIF. In > > > +order to get this port plugged by the OVN controller, OVN must be > > > +built with support for VIF

Re: [ovs-dev] [PATCH ovn v2] Revert features detection and zero-snat patches.

2021-08-27 Thread Mark Michelson
Thank you both for the quick reviews. I pushed the change to master. On 8/27/21 5:28 PM, Flavio Fernandes wrote: On Aug 27, 2021, at 4:11 PM, Mark Michelson > wrote: This commit reverts the following two commits: 56e2cd3a (ovn-controller: Detect OVS datapath

Re: [ovs-dev] [PATCH ovn 1/8] ovn-northd-ddlog: Intern all strings in OVSDB tables.

2021-08-27 Thread 0-day Robot
Bleep bloop. Greetings Ben Pfaff, 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. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch' to see the failed patch

[ovs-dev] [PATCH ovn 8/8] ovn-northd-ddlog: Intern strings before joining when possible.

2021-08-27 Thread Ben Pfaff
By interning earlier, we do less copying across the joins with Router and Switch, because DDlog always copies by value and copying an interned string is cheap. Signed-off-by: Ben Pfaff --- northd/ovn_northd.dl | 57 +++- 1 file changed, 30 insertions(+),

[ovs-dev] [PATCH ovn 6/8] ovn-northd-ddlog: Simplify LBVIPWithStatus to include up_backends string.

2021-08-27 Thread Ben Pfaff
There was only one use for the 'backends' map, which was to be converted to a string that listed all the backends that were up, so we might as well do that at its point of origination. At the same time, everything else in LBVIPWithStatus was just a copy of the underlying LBVIP, so we might as

[ovs-dev] [PATCH ovn 7/8] ovn-northd-ddlog: Avoid storing unused 'lbs' field in Router.

2021-08-27 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- northd/lrouter.dl | 9 - 1 file changed, 9 deletions(-) diff --git a/northd/lrouter.dl b/northd/lrouter.dl index c0ec6be47..582e07654 100644 --- a/northd/lrouter.dl +++ b/northd/lrouter.dl @@ -488,7 +488,6 @@ typedef Router = Router { is_gateway:

[ovs-dev] [PATCH ovn 5/8] ovn-northd-ddlog: Avoid re-parsing LB IP addresses and ports.

2021-08-27 Thread Ben Pfaff
The LBVIPs already contain parsed versions of the load balancer keys, but the code was parsing it redundantly. Slight performance improvement but much cleaner. Signed-off-by: Ben Pfaff --- northd/ovn_northd.dl | 37 - 1 file changed, 12 insertions(+), 25

[ovs-dev] [PATCH ovn 4/8] ovn-northd-ddlog: Reverse order of joins for connection tracking flows.

2021-08-27 Thread Ben Pfaff
DDlog evaluates rules in the order given by syntax. The rules for load balancers all first evaluated a Router or Switch, then joined that against the load balancers. However, the expensive logic was all on the load balancers. This meant that the load balancer logic was happening many times,

[ovs-dev] [PATCH ovn 2/8] ovn-northd-ddlog: Make joins for ARP/ND flows slightly more efficient.

2021-08-27 Thread Ben Pfaff
DDlog can index equality joins within a expression like this, but not conditional expression that follow such an expression. This doesn't actually matter much in this case because ordinarily the expression will be true (most router ports are enabled). Signed-off-by: Ben Pfaff ---

[ovs-dev] [PATCH ovn 3/8] ovn-northd-ddlog: Derive load balancer IP addresses in new LoadBalancer.

2021-08-27 Thread Ben Pfaff
This makes the get_router_load_balancer_ips() function much faster. This function is a hot path for the use of load balancers, so it improves performance overall when they are in use. Signed-off-by: Ben Pfaff --- northd/lrouter.dl| 68 +--

[ovs-dev] [PATCH ovn 0/8] 3x performance improvement for ddlog with load balancer benchmark

2021-08-27 Thread Ben Pfaff
With and without these patches, I see the following performance when I run the load-balancer heavy benchmark. The measurements include cold start with all the load balancers, then deleting the sctp load balancer and waiting for it to finish, then the same with the tcp load balancer, then the same

Re: [ovs-dev] [PATCH ovn v3 5/7] lib: Add infrastructure for plugging providers.

2021-08-27 Thread Frode Nordahl
fre. 27. aug. 2021, 22:01 skrev Han Zhou : > > > On Fri, Aug 27, 2021 at 1:47 AM Frode Nordahl > wrote: > > > > On Fri, Aug 27, 2021 at 9:29 AM Han Zhou wrote: > > > > > > > > > > > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl < > frode.nord...@canonical.com> wrote: > > > > > > > > New

Re: [ovs-dev] [PATCH ovn v3 7/7] binding: Consider plugging of ports on CMS request.

2021-08-27 Thread Frode Nordahl
fre. 27. aug. 2021, 21:32 skrev Han Zhou : > > > On Fri, Aug 27, 2021 at 2:17 AM Frode Nordahl > wrote: > > > > On Fri, Aug 27, 2021 at 9:29 AM Han Zhou wrote: > > > > > > > > > > > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl < > frode.nord...@canonical.com> wrote: > > > > > > > > When OVN

Re: [ovs-dev] [PATCH ovn v2] Revert features detection and zero-snat patches.

2021-08-27 Thread Flavio Fernandes
> On Aug 27, 2021, at 4:11 PM, Mark Michelson wrote: > > This commit reverts the following two commits: > 56e2cd3a (ovn-controller: Detect OVS datapath capabilities.) > 58683a42 (ovn-controller: Handle DNAT/no-NAT conntrack tuple > collisions.) > > The former commit causes an incompatibility

Re: [ovs-dev] [PATCH ovn] northd: Enable logical dp groups by default.

2021-08-27 Thread Numan Siddique
On Fri, Aug 27, 2021 at 1:00 PM Han Zhou wrote: > > Acked-by: Han Zhou \ Thanks. I applied this patch to the main branch. Numan > > On Fri, Aug 27, 2021 at 9:20 AM Mark Michelson wrote: > > > I agree this should be the default. > > > > Acked-by: Mark Michelson > > > > On 8/20/21 5:08 PM,

Re: [ovs-dev] [PATCH ovn] ovn-northd: Hash pipeline as a digit.

2021-08-27 Thread Numan Siddique
On Fri, Aug 27, 2021 at 1:08 PM Mark Michelson wrote: > > Simple enough, works for me. > > Acked-by: Mark Michelson Thanks. I applied this patch to the main branch. Numan > > On 8/24/21 5:15 PM, Ilya Maximets wrote: > > Currently pipeline is hashed as a string ("egress" or "ingress"), > >

Re: [ovs-dev] [PATCH ovn 0/2] northd: Rework and optimize reconciliation of datapath groups.

2021-08-27 Thread Mark Michelson
Thanks, Ilya. For the series: Acked-by: Mark Michelson On 8/27/21 3:17 PM, Ilya Maximets wrote: This patch set fixes a small issue and optimizes reconciliation process for flows with datapath groups. As such, avoids unnecessary comparisons and creation of identical datapath groups. Allows

Re: [ovs-dev] Update OVN Soft Freeze for 21.09 Approaching

2021-08-27 Thread Numan Siddique
On Fri, Aug 27, 2021 at 4:55 PM Mark Michelson wrote: > > Hi all, > > As a previous email of mine stated, we pushed the OVN 21.09 soft freeze > to 3 September. That is one week away. If you have patches you are > working on for 21.09, please be sure they are posted for review by then. > Once you

Re: [ovs-dev] [PATCH ovn v2] Revert features detection and zero-snat patches.

2021-08-27 Thread Numan Siddique
On Fri, Aug 27, 2021 at 4:12 PM Mark Michelson wrote: > > This commit reverts the following two commits: > 56e2cd3a (ovn-controller: Detect OVS datapath capabilities.) > 58683a42 (ovn-controller: Handle DNAT/no-NAT conntrack tuple > collisions.) > > The former commit causes an incompatibility

[ovs-dev] Update OVN Soft Freeze for 21.09 Approaching

2021-08-27 Thread Mark Michelson
Hi all, As a previous email of mine stated, we pushed the OVN 21.09 soft freeze to 3 September. That is one week away. If you have patches you are working on for 21.09, please be sure they are posted for review by then. Once you have patches posted, if you would like to ensure they are

[ovs-dev] [PATCH ovn v2] Revert features detection and zero-snat patches.

2021-08-27 Thread Mark Michelson
This commit reverts the following two commits: 56e2cd3a (ovn-controller: Detect OVS datapath capabilities.) 58683a42 (ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions.) The former commit causes an incompatibility with OVS databases that do not have the datapath table present. This

Re: [ovs-dev] [PATCH ovn v3 5/7] lib: Add infrastructure for plugging providers.

2021-08-27 Thread Han Zhou
On Fri, Aug 27, 2021 at 1:47 AM Frode Nordahl wrote: > > On Fri, Aug 27, 2021 at 9:29 AM Han Zhou wrote: > > > > > > > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl < frode.nord...@canonical.com> wrote: > > > > > > New module contains the infrastructure for registering and > > > instantiating

Re: [ovs-dev] [PATCH ovn] Revert "ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions."

2021-08-27 Thread Mark Michelson
On 8/27/21 2:41 PM, Flavio Fernandes wrote: This revert does not include a 1 liner change needed done later in https://github.com/ovn-org/ovn/commit/4e6c498068dc4fa9546d3661f78f0a42e99c74bb That is not an issue,

Re: [ovs-dev] [PATCH ovn] Revert "ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions."

2021-08-27 Thread Numan Siddique
On Fri, Aug 27, 2021 at 2:42 PM Flavio Fernandes wrote: > > > This revert does not include a 1 liner change needed done later in > https://github.com/ovn-org/ovn/commit/4e6c498068dc4fa9546d3661f78f0a42e99c74bb > >

Re: [ovs-dev] [PATCH ovn v1] Add a northbound interface to program MAC_Binding table

2021-08-27 Thread Mark Michelson
Hi Karthik, One thing that's not clear to me is why logical routers need to have references to the northbound MAC_Bindings. The MAC_Bindings contain a logical port. This means that the only valid configuration would be if the router that contains that logical port references the MAC_Binding.

Re: [ovs-dev] [PATCH ovn v3 7/7] binding: Consider plugging of ports on CMS request.

2021-08-27 Thread Han Zhou
On Fri, Aug 27, 2021 at 2:17 AM Frode Nordahl wrote: > > On Fri, Aug 27, 2021 at 9:29 AM Han Zhou wrote: > > > > > > > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl < frode.nord...@canonical.com> wrote: > > > > > > When OVN is linked with an appropriate plugging implementation, > > > CMS can

Re: [ovs-dev] [PATCH ovn] ovn-northd: Don't generate identical flows for same LBs with different protocol.

2021-08-27 Thread Ilya Maximets
On 8/27/21 8:03 PM, Ilya Maximets wrote: > On 8/27/21 7:05 PM, Mark Gray wrote: >> On 27/08/2021 16:24, Ilya Maximets wrote: >>> It's common for CMS (e.g., ovn-kubernetes) to create 3 identical load >>> balancers, one per protocol (tcp, udp, sctp). However, if VIPs of >>> these load balancers has

[ovs-dev] [PATCH ovn 2/2] ovn-northd: Don't check datapath groups in full if not needed.

2021-08-27 Thread Ilya Maximets
The '/* Push changes to the Logical_Flow table to database. */' loop reads all the datapaths from the datapath group and checks them even if it's not required. To check that flow has at least one valid datapath, we need to find one valid datapath. To find Sb logical flow we need a datapath type

[ovs-dev] [PATCH ovn 1/2] ovn-northd: Avoid creation of identical datapath groups.

2021-08-27 Thread Ilya Maximets
Currently, in a case where datapath groups doesn't need to be changed, but a new flow should be added, northd will create a new identical datapath group for a new flow. It happens because we're not looking for suitable group in Southbound database, but only tracking ones created on current

[ovs-dev] [PATCH ovn 0/2] northd: Rework and optimize reconciliation of datapath groups.

2021-08-27 Thread Ilya Maximets
This patch set fixes a small issue and optimizes reconciliation process for flows with datapath groups. As such, avoids unnecessary comparisons and creation of identical datapath groups. Allows to save second or more in tests with big databases. Ilya Maximets (2): ovn-northd: Avoid creation

Re: [ovs-dev] [PATCH ovn] Revert "ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions."

2021-08-27 Thread Flavio Fernandes
This revert does not include a 1 liner change needed done later in https://github.com/ovn-org/ovn/commit/4e6c498068dc4fa9546d3661f78f0a42e99c74bb That is not an issue, since regex change there is backwards

Re: [ovs-dev] [PATCH ovn] Fix the test case "ACL label usage" for northd-ddlog.

2021-08-27 Thread Mark Michelson
Acked-by: Mark Michelson I went ahead and merged the change to master since it is trivial. On 8/20/21 2:38 PM, num...@ovn.org wrote: From: Numan Siddique Commit in the fixes tag added an extra space in the action causing the test case to fail. Fixes: b5b434b4d49("Intern all the matches and

Re: [ovs-dev] [PATCH v3 ovn 0/3] improve code efficiency for ovn-northd lb-unreachable ips

2021-08-27 Thread Mark Michelson
Thanks Lorenzo and Mark, I pushed this series to master. On 8/26/21 12:50 PM, Lorenzo Bianconi wrote: ovn-master: --- Statistics for 'ovnnb_db_run' Total samples: 37 Maximum: 12656 msec Minimum: 12276 msec 95th percentile: 12557.00 msec Short term average:

Re: [ovs-dev] [OVN Patch v4 4/4] northd: Restore parallel build with dp_groups

2021-08-27 Thread 0-day Robot
Bleep bloop. Greetings Anton Ivanov, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 91 characters long (recommended limit is 79) WARNING: Line has trailing

Re: [ovs-dev] [PATCH ovn] ovn-northd: Don't generate identical flows for same LBs with different protocol.

2021-08-27 Thread Ilya Maximets
On 8/27/21 7:05 PM, Mark Gray wrote: > On 27/08/2021 16:24, Ilya Maximets wrote: >> It's common for CMS (e.g., ovn-kubernetes) to create 3 identical load >> balancers, one per protocol (tcp, udp, sctp). However, if VIPs of >> these load balancers has no ports specified (!vip_port), northd will >>

Re: [ovs-dev] [OVN Patch v4 3/4] northd: Optimize dp groups operations

2021-08-27 Thread 0-day Robot
Bleep bloop. Greetings Anton Ivanov, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 83 characters long (recommended limit is 79) #40 FILE:

Re: [ovs-dev] [PATCH v2 ovn] northd: reduce number of nd_na lb logical flows

2021-08-27 Thread Lorenzo Bianconi
> Hi Lorenzo, > > This mostly looks good, but I have one question below: > > On 8/17/21 1:51 PM, Lorenzo Bianconi wrote: > > As it has been already done for IPv4, collapse IPv6 Neighbour > > Advertisment flows for load balancer using address list and > > reduce the number of logical flows from

Re: [ovs-dev] [PATCH ovn 0/2] northd: Split northd

2021-08-27 Thread Mark Michelson
Hi Mark, I had a look at this series, but I'm not 100% sure what the intent is. In patch 2, you mentioned modularity and the ability to include northd as a library. But I'm not sure where that allows us to go. Can you elaborate a bit? Thanks, Mark On 8/26/21 3:04 PM, Mark Gray wrote:

[ovs-dev] [OVN Patch v4 4/4] northd: Restore parallel build with dp_groups

2021-08-27 Thread anton . ivanov
From: Anton Ivanov Restore parallel build with dp groups using rwlock instead of per row locking as an underlying mechanism. This provides improvement ~ 10% end-to-end on ovn-heater under virutalization despite awakening some qemu gremlin which makes qemu climb to silly CPU usage. The gain on

[ovs-dev] [OVN Patch v4 3/4] northd: Optimize dp groups operations

2021-08-27 Thread anton . ivanov
From: Anton Ivanov Remove full hash walks to form lflow dp_groups and add them to the overall parallelizeable lflow build. Make processing of "with dp groups" and "without" in build_lflows independent to allow these to run in parallel after the updates to the parallel API have been merged.

[ovs-dev] [OVN Patch v4 2/4] northd: Resize the hash to correct parameters after build

2021-08-27 Thread anton . ivanov
From: Anton Ivanov Parallel builds may result in suboptimal hash bucket sizing. In the absense of dp-groups this does not matter as the hash is purely storage and not used for lookups during the build. Such a hash needs to be resized to a correct size at the end of the build to ensure that any

[ovs-dev] [OVN Patch v4 1/4] northd: Disable parallel processing for logical_dp_groups

2021-08-27 Thread anton . ivanov
From: Anton Ivanov Work on improving processing with dp_groups enabled has discovered that the locking mechanism presently in use is not reliable. Disabling parallel processing if dp_groups are enabled until the root cause is determined and fixed. Signed-off-by: Anton Ivanov ---

Re: [ovs-dev] [PATCH v2 ovn] northd: reduce number of nd_na lb logical flows

2021-08-27 Thread Mark Michelson
Hi Lorenzo, This mostly looks good, but I have one question below: On 8/17/21 1:51 PM, Lorenzo Bianconi wrote: As it has been already done for IPv4, collapse IPv6 Neighbour Advertisment flows for load balancer using address list and reduce the number of logical flows from N*M to N where N is

Re: [ovs-dev] [PATCH v3 ovn 3/3] northd: refactor unreachable IPs lb flows

2021-08-27 Thread Mark Gray
On 26/08/2021 17:50, Lorenzo Bianconi wrote: > Refactor unreachable IPs for vip load-balancers inverting the logic used > during the lb flow creation in order to visit lb first and then related > datapath/ovn_ports. Rely on ovn_lflow_add_at_with_hash and > ovn_dp_group_add_with_reference in

Re: [ovs-dev] [PATCH v3 ovn 2/3] norhd: optimize build_lrouter_defrag_flows_for_lb

2021-08-27 Thread Mark Gray
On 26/08/2021 17:50, Lorenzo Bianconi wrote: > Similar to build_lb_rules(), precompute hash and lflow pointer in > build_lrouter_defrag_flows_for_lb routine since they do not depend on > datapath updating datapath group. Using this approach we can reduce > ovn-northd loop time of ~1s running an

Re: [ovs-dev] [PATCH v3 ovn 1/3] northd: optimize build_lb_rules routine

2021-08-27 Thread Mark Gray
On 27/08/2021 17:20, Lorenzo Bianconi wrote: > [...] >>> +struct ovn_lflow *lflow_ref = NULL; >>> +uint32_t hash = ovn_logical_flow_hash( >>> +ovn_stage_get_table(S_SWITCH_IN_STATEFUL), >>> +ovn_stage_get_pipeline_name(S_SWITCH_IN_STATEFUL), >>>

Re: [ovs-dev] [PATCH ovn 1/2] ovn-northd: Rename ovn-northd.c to northd.c

2021-08-27 Thread Aaron Conole
Mark Gray writes: > On 26/08/2021 21:51, 0-day Robot wrote: >> Bleep bloop. Greetings Mark Gray, 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. >> > > These line length errors

Re: [ovs-dev] [PATCH ovn] ovn-northd: Hash pipeline as a digit.

2021-08-27 Thread Mark Michelson
Simple enough, works for me. Acked-by: Mark Michelson On 8/24/21 5:15 PM, Ilya Maximets wrote: Currently pipeline is hashed as a string ("egress" or "ingress"), and this seems wasteful. Hashing it as a digit (P_IN or P_OUT) to eliminate one call to hash_string() for every generated logical

Re: [ovs-dev] [PATCH ovn] ovn-northd: Don't generate identical flows for same LBs with different protocol.

2021-08-27 Thread Mark Gray
On 27/08/2021 16:24, Ilya Maximets wrote: > It's common for CMS (e.g., ovn-kubernetes) to create 3 identical load > balancers, one per protocol (tcp, udp, sctp). However, if VIPs of > these load balancers has no ports specified (!vip_port), northd will > generate identical logical flows for them.

Re: [ovs-dev] [PATCH ovn] northd: Enable logical dp groups by default.

2021-08-27 Thread Han Zhou
Acked-by: Han Zhou On Fri, Aug 27, 2021 at 9:20 AM Mark Michelson wrote: > I agree this should be the default. > > Acked-by: Mark Michelson > > On 8/20/21 5:08 PM, num...@ovn.org wrote: > > From: Numan Siddique > > > > Support for logical dp group is present since more than > > 2 releases

Re: [ovs-dev] [PATCH v3 ovn 1/3] northd: optimize build_lb_rules routine

2021-08-27 Thread Lorenzo Bianconi
[...] > > +struct ovn_lflow *lflow_ref = NULL; > > +uint32_t hash = ovn_logical_flow_hash( > > +ovn_stage_get_table(S_SWITCH_IN_STATEFUL), > > +ovn_stage_get_pipeline_name(S_SWITCH_IN_STATEFUL), > > priority, > > +ds_cstr(match),

Re: [ovs-dev] [PATCH ovn] northd: Enable logical dp groups by default.

2021-08-27 Thread Mark Michelson
I agree this should be the default. Acked-by: Mark Michelson On 8/20/21 5:08 PM, num...@ovn.org wrote: From: Numan Siddique Support for logical dp group is present since more than 2 releases now and its time to enable it as a default. Signed-off-by: Numan Siddique --- NEWS

Re: [ovs-dev] [PATCH ovn v2] checkpatch: Avoid a case of catastrophic backtracking

2021-08-27 Thread Mark Michelson
Hi Frode, This looks good to me, so Acked-by: Mark Michelson I have one small nit below, but I don't think you need to submit a v3 of the patch. I think whoever merges this can fix this in the process: On 8/20/21 10:27 AM, Frode Nordahl wrote: The checkpatch script will hang forever on

Re: [ovs-dev] [PATCH ovn v2] northd: Add additional stopwatches for debugging long poll intervals

2021-08-27 Thread Mark Michelson
Hi Mark, looks good to me. Acked-by: Mark Michelson On 8/20/21 4:14 AM, Mark Gray wrote: In order to debug northd, add the following stopwatches to help determine what causes long poll intervals cause by lflow generation. Signed-off-by: Mark Gray --- Notes: v2: Move northd-loop

Re: [ovs-dev] [PATCH v3 ovn 1/3] northd: optimize build_lb_rules routine

2021-08-27 Thread Mark Gray
On 26/08/2021 17:50, Lorenzo Bianconi wrote: > Introduce ovn_lflow_add_at_with_hash to use a precomputed hash if not > dependent on the logical flow. > Introduce ovn_dp_group_add_with_reference routine to update the > dapath_group with the datapath pointer without run a lflow lookup. > Optimize

Re: [ovs-dev] [PATCH v3 ovn 2/3] norhd: optimize build_lrouter_defrag_flows_for_lb

2021-08-27 Thread Mark Gray
On 27/08/2021 09:18, Lorenzo Bianconi wrote: >> On 26/08/2021 17:50, Lorenzo Bianconi wrote: >>> Similar to build_lb_rules(), precompute hash and lflow pointer in >>> build_lrouter_defrag_flows_for_lb routine since they do not depend on >>> datapath updating datapath group. Using this approach we

[ovs-dev] [PATCH] dpif-netdev: Mention SMC in 2 pmd_thread comments

2021-08-27 Thread Cian Ferriter
These comments are relevant to SMC too. Signed-off-by: Cian Ferriter --- lib/dpif-netdev-private-dfc.h| 3 ++- lib/dpif-netdev-private-thread.h | 8 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/dpif-netdev-private-dfc.h b/lib/dpif-netdev-private-dfc.h index

[ovs-dev] [PATCH] docs/userspace-tunneling: Fix IP addresses for host2

2021-08-27 Thread Cian Ferriter
The IP addresses being recommended for the VM interface and the "remote_ip" on the tunnel port are wrong. The host1 values were being used before. Update to use the host2 values. Signed-off-by: Cian Ferriter --- I hope I'm reading the guide correctly here. I'm fairly sure that the VM interface

[ovs-dev] [PATCH ovn] Revert "ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions."

2021-08-27 Thread Mark Michelson
This commit resulted in significant decreased dataplane performance when testing a dense OpenShift cluster. This was pinpointed to be due to an extra ct(nat(src)) that this commit added. For now, revert this commit. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012 Signed-off-by:

Re: [ovs-dev] [[PATCH] ovn] Revert "ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions."

2021-08-27 Thread Mark Michelson
I messed up the format of the subject line, so I re-sent the patch with it corrected. You can ignore this. On 8/27/21 11:18 AM, Mark Michelson wrote: This commit resulted in significant decreased dataplane performance when testing a dense OpenShift cluster. This was pinpointed to be due to an

[ovs-dev] [PATCH ovn] ovn-northd: Don't generate identical flows for same LBs with different protocol.

2021-08-27 Thread Ilya Maximets
It's common for CMS (e.g., ovn-kubernetes) to create 3 identical load balancers, one per protocol (tcp, udp, sctp). However, if VIPs of these load balancers has no ports specified (!vip_port), northd will generate identical logical flows for them. 2 of 3 such flows will be just discarded, so

Re: [ovs-dev] [PATCH 2/2] docs: Recommend the use of dpdkvhostuserclient ports

2021-08-27 Thread 0-day Robot
Bleep bloop. Greetings Cian Ferriter, 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 82 characters long (recommended limit is 79) #90 FILE:

[ovs-dev] [[PATCH] ovn] Revert "ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions."

2021-08-27 Thread Mark Michelson
This commit resulted in significant decreased dataplane performance when testing a dense OpenShift cluster. This was pinpointed to be due to an extra ct(nat(src)) that this commit added. For now, revert this commit. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012 Signed-off-by:

Re: [ovs-dev] [PATCH 1/2] docs/install/afxdp: Fix wrapping in QEMU CMDs

2021-08-27 Thread 0-day Robot
Bleep bloop. Greetings Cian Ferriter, 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 111 characters long (recommended limit is 79) #24 FILE:

[ovs-dev] [PATCH 2/2] docs: Recommend the use of dpdkvhostuserclient ports

2021-08-27 Thread Cian Ferriter
dpdkvhostuser ports are deprecated, but are still being recommended to users through the documentation. Fix this. Signed-off-by: Cian Ferriter --- Documentation/howto/dpdk.rst| 30 +++-- Documentation/howto/userspace-tunneling.rst | 3 ++-

[ovs-dev] [PATCH 1/2] docs/install/afxdp: Fix wrapping in QEMU CMDs

2021-08-27 Thread Cian Ferriter
Directly copying the CMD from either the .rst file or the docs on the web caused errors in the QEMU command because of how it was wrapped. Signed-off-by: Cian Ferriter --- The CMD could be wrapped, but without the indentation to look like this: qemu-system-x86_64 -hda ubuntu1810.qcow \ -m

Re: [ovs-dev] [PATCH v4 15/27] dpif-netdev: Quiesce offload thread periodically

2021-08-27 Thread Maxime Coquelin
On 6/9/21 3:09 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 v4 13/27] id-fpool: Module for fast ID generation

2021-08-27 Thread Maxime Coquelin
On 6/9/21 3:09 PM, Gaetan Rivet wrote: > The current id-pool module is slow to allocate the > next valid ID, and can be optimized when restricting > some properties of the pool. > > Those restrictions are: > > * No ability to add a random ID to the pool. > > * A new ID is no more the

Re: [ovs-dev] [PATCH v2] pmd.at: Add test-cases for set and get commands for DPCLS and DPIF

2021-08-27 Thread Amber, Kumar
Hi Kevin I have updated the Scenarios you wanted to check with 'set' and 'get' combinations in V3  even if the dummy-pmd will not changes but still commands can be checked for DPCLS as priority change function will be called no matter what eg: This command set the max priority and then use

Re: [ovs-dev] [PATCH v4 12/27] mpsc-queue: Module for lock-free message passing

2021-08-27 Thread Maxime Coquelin
On 6/9/21 3:09 PM, Gaetan Rivet wrote: > Add a lockless multi-producer/single-consumer (MPSC), linked-list based, > intrusive, unbounded queue that does not require deferred memory > management. > > The queue is designed to improve the specific MPSC setup. A benchmark > accompanies the unit

Re: [ovs-dev] [PATCH] conntrack : dpif parameter is used, need to remove OVS_UNUSED.

2021-08-27 Thread 0-day Robot
Bleep bloop. Greetings lin huang, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author lin huang needs to sign off. WARNING: Unexpected sign-offs from developers who are

[ovs-dev] [PATCH] conntrack : dpif parameter is used, need to remove OVS_UNUSED.

2021-08-27 Thread lin huang
The dpif parameter of zone limits function is used. We need to remove OVS_UNUSED flag. Signed-off-by: linhuang --- lib/dpif-netdev.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index b3e57bb95..1708a5f9e 100644 ---

Re: [ovs-dev] [PATCH V2 4/4] netdev-offload-dpdk: Don't ignore frags as they are handled

2021-08-27 Thread Maxime Coquelin
On 8/27/21 12:12 PM, Eli Britstein wrote: > > On 8/27/2021 12:30 PM, Maxime Coquelin wrote: >> External email: Use caution opening links or attachments >> >> >> On 8/16/21 3:53 PM, Eli Britstein via dev wrote: >>> Signed-off-by: Eli Britstein >>> --- >>>   NEWS  | 2 ++ >>>  

Re: [ovs-dev] [PATCH V2 4/4] netdev-offload-dpdk: Don't ignore frags as they are handled

2021-08-27 Thread Eli Britstein via dev
On 8/27/2021 12:30 PM, Maxime Coquelin wrote: External email: Use caution opening links or attachments On 8/16/21 3:53 PM, Eli Britstein via dev wrote: Signed-off-by: Eli Britstein --- NEWS | 2 ++ lib/netdev-offload-dpdk.c | 5 - 2 files changed, 2

Re: [ovs-dev] [PATCH v4 11/27] ovs-atomic: Expose atomic exchange operation

2021-08-27 Thread Maxime Coquelin
On 6/9/21 3:09 PM, Gaetan Rivet wrote: > The atomic exchange operation is a useful primitive that should be > available as well. Most compiler already expose or offer a way s/compiler/compilers/ > to use it, but a single symbol needs to be defined. > > Signed-off-by: Gaetan Rivet >

Re: [ovs-dev] [PATCH v2] pmd.at: Add test-cases for set and get commands for DPCLS and DPIF

2021-08-27 Thread Kevin Traynor
Hi Amber, On 26/08/2021 18:05, Amber, Kumar wrote: > Hi Kevin, > > Thanks a lot for the Reviews. > Responses are inlined. > >> -Original Message- >> From: Kevin Traynor >> Sent: Thursday, August 26, 2021 8:50 PM >> To: Amber, Kumar ; ovs-dev@openvswitch.org >> Cc: i.maxim...@ovn.org >>

Re: [ovs-dev] [PATCH V2 4/4] netdev-offload-dpdk: Don't ignore frags as they are handled

2021-08-27 Thread Maxime Coquelin
On 8/16/21 3:53 PM, Eli Britstein via dev wrote: > Signed-off-by: Eli Britstein > --- > NEWS | 2 ++ > lib/netdev-offload-dpdk.c | 5 - > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/NEWS b/NEWS > index 26920e215..80466c014 100644 > --- a/NEWS >

Re: [ovs-dev] [PATCH V2 3/4] netdev-offload-dpdk: Support IPv6 fragmentation types

2021-08-27 Thread Maxime Coquelin
On 8/16/21 3:53 PM, Eli Britstein via dev wrote: > Support IPv6 fragmentation matching. > > Signed-off-by: Eli Britstein > --- > lib/netdev-offload-dpdk.c | 82 ++- > 1 file changed, 81 insertions(+), 1 deletion(-) > Reviewed-by: Maxime Coquelin

Re: [ovs-dev] [PATCH V2 2/4] netdev-offload-dpdk: Support IPv4 fragmentation types

2021-08-27 Thread Maxime Coquelin
On 8/16/21 3:53 PM, Eli Britstein via dev wrote: > Support IPv4 fragmentation matching. > > Signed-off-by: Eli Britstein > --- > lib/netdev-offload-dpdk.c | 47 +-- > 1 file changed, 45 insertions(+), 2 deletions(-) > Looks good to me: Reviewed-by:

Re: [ovs-dev] [PATCH ovn v3 7/7] binding: Consider plugging of ports on CMS request.

2021-08-27 Thread Frode Nordahl
On Fri, Aug 27, 2021 at 9:29 AM Han Zhou wrote: > > > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl > wrote: > > > > When OVN is linked with an appropriate plugging implementation, > > CMS can request OVN to plug individual lports into the local > > Open vSwitch instance. > > > > The port and

Re: [ovs-dev] [PATCH V2 1/4] netdev-offload-dpdk: Add last attribute to patterns

2021-08-27 Thread Maxime Coquelin
Hi Eli, On 8/16/21 3:53 PM, Eli Britstein via dev wrote: > Matching on frag types requires range. Add 'last' attribute to patterns. > > Signed-off-by: Eli Britstein > --- > lib/netdev-offload-dpdk.c | 151 -- > 1 file changed, 81 insertions(+), 70

Re: [ovs-dev] [PATCH ovn v3 5/7] lib: Add infrastructure for plugging providers.

2021-08-27 Thread Frode Nordahl
On Fri, Aug 27, 2021 at 9:29 AM Han Zhou wrote: > > > > On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl > wrote: > > > > New module contains the infrastructure for registering and > > instantiating plugging classes which may be hosted inside or > > outside the core OVN repository. The data

Re: [ovs-dev] [PATCH v3 ovn 2/3] norhd: optimize build_lrouter_defrag_flows_for_lb

2021-08-27 Thread Lorenzo Bianconi
> On 26/08/2021 17:50, Lorenzo Bianconi wrote: > > Similar to build_lb_rules(), precompute hash and lflow pointer in > > build_lrouter_defrag_flows_for_lb routine since they do not depend on > > datapath updating datapath group. Using this approach we can reduce > > ovn-northd loop time of ~1s

Re: [ovs-dev] [PATCH ovn v3 7/7] binding: Consider plugging of ports on CMS request.

2021-08-27 Thread Han Zhou
On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl wrote: > > When OVN is linked with an appropriate plugging implementation, > CMS can request OVN to plug individual lports into the local > Open vSwitch instance. > > The port and instance record will be maintained during the lifetime > of the lport

Re: [ovs-dev] [PATCH ovn v3 5/7] lib: Add infrastructure for plugging providers.

2021-08-27 Thread Han Zhou
On Thu, Aug 19, 2021 at 4:09 AM Frode Nordahl wrote: > > New module contains the infrastructure for registering and > instantiating plugging classes which may be hosted inside or > outside the core OVN repository. The data structures and functions > for interacting with these plugging classes

Re: [ovs-dev] [PATCH v3 ovn 2/3] norhd: optimize build_lrouter_defrag_flows_for_lb

2021-08-27 Thread Mark Gray
On 26/08/2021 17:50, Lorenzo Bianconi wrote: > Similar to build_lb_rules(), precompute hash and lflow pointer in > build_lrouter_defrag_flows_for_lb routine since they do not depend on > datapath updating datapath group. Using this approach we can reduce > ovn-northd loop time of ~1s running an

Re: [ovs-dev] [PATCH v3 ovn 0/3] improve code efficiency for ovn-northd lb-unreachable ips

2021-08-27 Thread Mark Gray
On 26/08/2021 17:50, Lorenzo Bianconi wrote: > ovn-master: > --- > Statistics for 'ovnnb_db_run' > Total samples: 37 > Maximum: 12656 msec > Minimum: 12276 msec > 95th percentile: 12557.00 msec > Short term average: 12475.213654 msec > Long term average: 12336.498446 msec >

[ovs-dev] [PATCH v2] conntrack: fix src port selection for DNAT case

2021-08-27 Thread wenxu
From: wenxu For DNAT case the src port should never modified. Fixes: 61e48c2d1db2 ("conntrack: Handle SNAT with all-zero IP address") Signed-off-by: wenxu --- lib/conntrack.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index

Re: [ovs-dev] [PATCH ovn 1/2] ovn-northd: Rename ovn-northd.c to northd.c

2021-08-27 Thread Mark Gray
On 26/08/2021 21:51, 0-day Robot wrote: > Bleep bloop. Greetings Mark Gray, 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. > These line length errors were already present in the