Re: [ovs-dev] [PATCH] connmgr: Check nullptr inside ofmonitor_report()

2021-02-16 Thread William Tu
On Tue, Feb 16, 2021 at 1:40 PM Yi-Hung Wei wrote: > > On Tue, Feb 16, 2021 at 1:06 PM Yifeng Sun wrote: > > > > ovs-vswitchd could crash under these circumstances: > > 1. When one bridge is being destroyed, ofproto_destroy() is called and > > connmgr pointer of its ofproto struct is nullified.

[ovs-dev] [PATCH] Documentation: Fix DPDK qos example.

2021-02-16 Thread William Tu
RFC4115 says "The CIR and EIR are both measured in bits/s." Fix the example use case based on the decription. 64-Byte packet * 8 * 1000 pps = 512000 Fixes: e61bdffc2a98 ("netdev-dpdk: Add new DPDK RFC 4115 egress policer") Signed-off-by: William Tu --- Documentation/topics/dpdk/qos.rst | 12

Re: [ovs-dev] [PATCH ovn] ovn-northd: Don't add arp responder flows for lports with 'unknown' address.

2021-02-16 Thread Numan Siddique
On Tue, Feb 16, 2021 at 6:56 PM Lucas Alvares Gomes wrote: > > Hi Numan, > > Can we backport this change to 20.03 please ? I did locally, and > should be a clean merge. > > The OVN packages from Ubuntu LTS 20.04 (focal) which is used upstream > to test OpenStack is based on 20.03 and this patch

Re: [ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Numan Siddique
On Wed, Feb 17, 2021 at 2:26 AM Ilya Maximets wrote: > > On 2/16/21 8:48 PM, num...@ovn.org wrote: > > From: Numan Siddique > > > > In one of the scaled deployments, ovn-controller is asserting with the > > below stack trace > > > > *** > > (gdb) bt > >0 raise () from /lib64/libc.so.6 > >

Re: [ovs-dev] [PATCH ovn v9 7/7] ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.

2021-02-16 Thread Ben Pfaff
On Fri, Dec 11, 2020 at 03:43:22PM +0100, Dumitru Ceara wrote: > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 6e396895826f..2e4eaf7f12f7 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -16982,6 +16982,10 @@ AT_CLEANUP > > > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([ovn -- IGMP

Re: [ovs-dev] [PATCH] ofp-actions: Fix use-after-free while decoding RAW_ENCAP.

2021-02-16 Thread William Tu
On Tue, Feb 16, 2021 at 2:27 PM Ilya Maximets wrote: > > While decoding RAW_ENCAP action, decode_ed_prop() might re-allocate > ofpbuf if there is no enough space left. However, function > 'decode_NXAST_RAW_ENCAP' continues to use old pointer to 'encap' > structure leading to write-after-free and

Re: [ovs-dev] [PATCH] conntrack: Add a test for IPv4 UDP zero checksum

2021-02-16 Thread William Tu
Hi Aaron, Should we also consider the case where udp checksum is 0x? I saw in netdev_tnl_calc_udp_csum, we set to 0x when a packet has udp checksum = 0. Thanks William On Wed, Feb 10, 2021 at 12:15 PM Flavio Leitner wrote: > > On Wed, Feb 10, 2021 at 02:45:06PM -0500, Aaron Conole

[ovs-dev] [PATCH] netdev-linux: Fix indentation.

2021-02-16 Thread William Tu
Remove one extra space. No actual code logic changed. Fixes: 2109841b79845 ("Use batch process recv for tap and raw socket in netdev datapath") Signed-off-by: William Tu --- lib/netdev-linux.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git

[ovs-dev] [PATCH] ofp-actions: Fix use-after-free while decoding RAW_ENCAP.

2021-02-16 Thread Ilya Maximets
While decoding RAW_ENCAP action, decode_ed_prop() might re-allocate ofpbuf if there is no enough space left. However, function 'decode_NXAST_RAW_ENCAP' continues to use old pointer to 'encap' structure leading to write-after-free and incorrect decoding. ==3549105==ERROR: AddressSanitizer:

Re: [ovs-dev] [PATCH] pcap-file: Fix calculation of TCP payload length in tcp_reader_run().

2021-02-16 Thread William Tu
I see, thanks. On Tue, Feb 16, 2021 at 12:52 PM Ben Pfaff wrote: > > On Fri, Feb 12, 2021 at 11:15:46AM -0800, William Tu wrote: > > I'm confused with what l2_pad_size is. > > I thought it's between L2 and L3 header, there is a 2-byte padding to > > make it 16-byte alignment. But it doesn't look

Re: [ovs-dev] [PATCH] connmgr: Check nullptr inside ofmonitor_report()

2021-02-16 Thread Yi-Hung Wei
On Tue, Feb 16, 2021 at 1:06 PM Yifeng Sun wrote: > > ovs-vswitchd could crash under these circumstances: > 1. When one bridge is being destroyed, ofproto_destroy() is called and > connmgr pointer of its ofproto struct is nullified. This ofproto struct is > deallocated through

Re: [ovs-dev] [PATCH ovn v9 7/7] ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.

2021-02-16 Thread Ben Pfaff
On Fri, Dec 11, 2020 at 03:43:22PM +0100, Dumitru Ceara wrote: > There are a few tests that fail (also northd-C tests) and I suspect it's > because the OVS IDL split patches [0] this commit depends on don't > include some of the recent fixes that went into the OVS IDL code, at > least the

[ovs-dev] [PATCH] connmgr: Check nullptr inside ofmonitor_report()

2021-02-16 Thread Yifeng Sun
ovs-vswitchd could crash under these circumstances: 1. When one bridge is being destroyed, ofproto_destroy() is called and connmgr pointer of its ofproto struct is nullified. This ofproto struct is deallocated through 'ovsrcu_postpone(ofproto_destroy_defer__, p);'. 2. Before RCU enters quiesce

Re: [ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Ilya Maximets
On 2/16/21 8:48 PM, num...@ovn.org wrote: > From: Numan Siddique > > In one of the scaled deployments, ovn-controller is asserting with the > below stack trace > > *** > (gdb) bt >0 raise () from /lib64/libc.so.6 >1 abort () from /lib64/libc.so.6 >2 ovs_abort_valist ("%s:

Re: [ovs-dev] [PATCH] pcap-file: Fix calculation of TCP payload length in tcp_reader_run().

2021-02-16 Thread Ben Pfaff
On Fri, Feb 12, 2021 at 11:15:46AM -0800, William Tu wrote: > I'm confused with what l2_pad_size is. > I thought it's between L2 and L3 header, there is a 2-byte padding to > make it 16-byte alignment. But it doesn't look like that. Ethernet has a 64-byte minimum packet length. l2_pad_size is

Re: [ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Numan Siddique
On Tue, Feb 16, 2021 at 4:41 PM Ilya Maximets wrote: > > On 2/16/21 8:02 AM, num...@ovn.org wrote: > > From: Numan Siddique > > Hi, Numan. > Thanks for the fix! The logic seems correct, but I'd suggest to > implement the fix a bit differently just to avoid unnecessary > complications. This

Re: [ovs-dev] [PATCH ovn v2] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Numan Siddique
On Wed, Feb 17, 2021 at 1:18 AM wrote: > > From: Numan Siddique Please ignore this patch. I made a mistake in Suggested-by tag. I have the resent the patch fixing this. Numan > > In one of the scaled deployments, ovn-controller is asserting with the > below stack trace > > *** > (gdb) bt >

[ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread numans
From: Numan Siddique In one of the scaled deployments, ovn-controller is asserting with the below stack trace *** (gdb) bt 0 raise () from /lib64/libc.so.6 1 abort () from /lib64/libc.so.6 2 ovs_abort_valist ("%s: assertion %s failed in %s()") at lib/util.c:419 3

[ovs-dev] [PATCH ovn v2] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread numans
From: Numan Siddique In one of the scaled deployments, ovn-controller is asserting with the below stack trace *** (gdb) bt 0 raise () from /lib64/libc.so.6 1 abort () from /lib64/libc.so.6 2 ovs_abort_valist ("%s: assertion %s failed in %s()") at lib/util.c:419 3

Re: [ovs-dev] [PATCH] docs: Add instruction to set local_ip to ipsec tutorial

2021-02-16 Thread Mark Gray
On 16/02/2021 15:06, Balazs Nemeth wrote: Thanks Balazs. Good catch > Signed-off-by: Balazs Nemeth > --- > Documentation/tutorials/ipsec.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/tutorials/ipsec.rst > b/Documentation/tutorials/ipsec.rst > index

[ovs-dev] [PATCH] ovs-ctl: Allow recording hostname separately

2021-02-16 Thread Frode Nordahl
ovs-ctl determines the system FQDN or hostname and records it in the `external-ids:hostname` field of the `Open-vSwitch` table on system startup. This value may be consumed by downstream software and having it unset or set to a incorrect value could lead to erratic behavior of a system. When a

[ovs-dev] [PATCH ovn] ofctrl: Do not link a desired flow twice.

2021-02-16 Thread Dumitru Ceara
Depending on the logical flow matches, multiple SB flows can point to the same desired flow. If it happens that the desired flow conflicts with a more restrictive (already installed) flow, then we shouldn't try to add the desired flow multiple times to the list maintained for the installed flow.

[ovs-dev] [PATCH] docs: Add instruction to set local_ip to ipsec tutorial

2021-02-16 Thread Balazs Nemeth
Signed-off-by: Balazs Nemeth --- Documentation/tutorials/ipsec.rst | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/tutorials/ipsec.rst b/Documentation/tutorials/ipsec.rst index 3b3e42c59..8256b5aa6 100644 --- a/Documentation/tutorials/ipsec.rst +++

Re: [ovs-dev] [PATCH ovn] controller: introduce coverage_counters for ovn-controller incremental processing

2021-02-16 Thread Mark Gray
Thanks Lorenzo, looks really useful. In the subject line, should there be an underscore between coverage and counters? On 12/02/2021 17:49, Lorenzo Bianconi wrote: > In order to help understanding system behaviour for debugging purpose, > introduce coverage counters for ovn-controller I-P

Re: [ovs-dev] [PATCH ovn] ovn-northd: Don't add arp responder flows for lports with 'unknown' address.

2021-02-16 Thread Lucas Alvares Gomes
Hi Numan, Can we backport this change to 20.03 please ? I did locally, and should be a clean merge. The OVN packages from Ubuntu LTS 20.04 (focal) which is used upstream to test OpenStack is based on 20.03 and this patch fixes an issue we have with one tempest test [0] [0]

[ovs-dev] [PATCH ovn v3] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case

2021-02-16 Thread Mark Gray
If a chassis has multiple interfaces, 'ovn-encap-ip' can be used to specify the IP address of the interface that is used for tunnel traffic. OVN uses that IP address to configure the 'remote_ip' of a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, according to the OVS

Re: [ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Ilya Maximets
On 2/16/21 12:11 PM, Ilya Maximets wrote: > On 2/16/21 8:02 AM, num...@ovn.org wrote: >> From: Numan Siddique > > Hi, Numan. > Thanks for the fix! The logic seems correct, but I'd suggest to > implement the fix a bit differently just to avoid unnecessary > complications. This code is already

Re: [ovs-dev] Bug#974588: Bug#974588: openvswitch: DPDK 20.11 support and transition for bullseye

2021-02-16 Thread Luca Boccassi
On Sat, 2021-01-09 at 15:50 +, Luca Boccassi wrote: > On Thu, 7 Jan 2021 at 20:14, Luca Boccassi wrote: > > On Thu, 7 Jan 2021 at 19:53, Thomas Goirand wrote: > > > On 1/7/21 4:54 PM, Christian Ehrhardt wrote: > > > > Hi, > > > > as an FYI Ubuntu moved to recent commit def6eb1ea and for us

Re: [ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Ilya Maximets
On 2/16/21 8:02 AM, num...@ovn.org wrote: > From: Numan Siddique Hi, Numan. Thanks for the fix! The logic seems correct, but I'd suggest to implement the fix a bit differently just to avoid unnecessary complications. This code is already very hard to understand. Test would be nice, but it

Re: [ovs-dev] [PATCH ovn v2] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case

2021-02-16 Thread 0-day Robot
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. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch' to see the failed patch

Re: [ovs-dev] [ovn] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case

2021-02-16 Thread 0-day Robot
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. 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 v2] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case

2021-02-16 Thread Mark Gray
If a chassis has multiple interfaces, 'ovn-encap-ip' can be used to specify the IP address of the interface that is used for tunnel traffic. OVN uses that IP address to configure the 'remote_ip' of a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, according to the OVS

[ovs-dev] [ovn] ovn-controller: Add 'local_ip' option to tunnel ports for IPsec case

2021-02-16 Thread Mark Gray
If a chassis has multiple interfaces, 'ovn-encap-ip' can be used to specify the IP address of the interface that is used for tunnel traffic. OVN uses that IP address to configure the 'remote_ip' of a tunnel port. OVS tunnel ports also accept 'options:local_ip', which, according to the OVS

Re: [ovs-dev] [PATCH ovn] ofctrl: Fix the assert seen when flood removing flows.

2021-02-16 Thread Numan Siddique
On Tue, Feb 16, 2021, 12:33 PM wrote: > From: Numan Siddique > > In one of the scaled deployments, ovn-controller is asserting with the > below stack trace > > > (gdb) bt >#0 0x7fa6aed4970f in raise () from /lib64/libc.so.6 >#1 0x7fa6aed33b25 in abort () from