Re: [ovs-dev] [ovs-dev v5] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread 0-day Robot
Bleep bloop. Greetings Peng He, 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 Peng He needs to sign off. WARNING: Unexpected sign-offs from developers who are not a

[ovs-dev] [ovs-dev v5] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread Peng He
From hepeng: https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473 also from guohongzhi : http://patchwork.ozlabs.org/project/openvswitch/patch/20200306130555.19884-1-guohongz...@huawei.com/ also from a discussion about the mixing use of

Re: [ovs-dev] [ovs-dev v4] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread Peng He
Hi, Gaëtan Thanks for the detailed reviewing! Gaëtan Rivet 于2022年2月5日周六 00:02写道: > Thank you Peng for doing this. > > On Tue, Jan 18, 2022, at 16:01, Peng He wrote: > > From hepeng: > > > https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#248

Re: [ovs-dev] [PATCH ovn] ovn-controller: Avoid reprocessing same lflows in the same I-P run.

2022-02-04 Thread Han Zhou
On Fri, Feb 4, 2022 at 7:58 AM Mark Michelson wrote: > > As far as I can tell, this looks correct. > > Acked-by: Mark Michelson Thanks Mark. I applied it to the main branch. > > On 1/20/22 13:06, Han Zhou wrote: > > For I-P node lflow_output, different change handlers can trigger > > reprocessin

Re: [ovs-dev] [ovs-dev v4] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread Peng He
Hi, Adrian, Adrian Moreno 于2022年2月4日周五 21:37写道: > > > On 1/18/22 16:01, Peng He wrote: > > From hepeng: > > > https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473 > > > > also from guohongzhi : > > > http://patchwork.ozlabs.org/proje

Re: [ovs-dev] [PATCH v3 2/2] ofproto-dpif: avoid unneccesary backer revalidation

2022-02-04 Thread 0-day Robot
Bleep bloop. Greetings lic121, 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: Co-author Eelco Chaudron needs to sign off. Lines checked: 126, Warnings: 0, Errors: 1 Pleas

[ovs-dev] [PATCH v3 2/2] ofproto-dpif: avoid unneccesary backer revalidation

2022-02-04 Thread lic121
If lldp didn't change, we are not supposed to trigger backer revalidation. Without this patch, bridge_reconfigure() always trigger udpif revalidator because of lldp. Signed-off-by: lic121 Co-authored-by: Eelco Chaudron --- lib/ovs-lldp.c | 8 lib/ovs-lldp.h | 1 + of

[ovs-dev] [PATCH v3 1/2] ofproto-dpif: trigger revalidation when ipfix config set

2022-02-04 Thread lic121
Currently, ipfix conf creation/deletion don't trigger dpif backer revalidation. This is not expected, as we need the revalidation to commit ipfix into xlate. So that xlate can generate ipfix actions. This patch covers only new creation/deletion of ipfix config. Will upload one more patch to cover

Re: [ovs-dev] [PATCH v2 07/18] python: introduce OpenFlow Flow parsing

2022-02-04 Thread Mark Michelson
On 1/31/22 01:20, Adrian Moreno wrote: On 1/28/22 22:18, Ilya Maximets wrote: On 1/28/22 17:04, Adrian Moreno wrote: Introduce OFPFlow class and all its decoders. Most of the decoders are generic (from decoders.py). Some have special syntax and need a specific implementation. Decoders for n

Re: [ovs-dev] [PATCH v2 03/18] python: add list parser

2022-02-04 Thread Mark Michelson
On 1/28/22 11:04, Adrian Moreno wrote: Some openflow or dpif flows encode their arguments in lists, eg: "some_action(arg1,arg2,arg3)". In order to decode this in a way that can be then stored and queried, add ListParser and ListDecoders classes that parse lists into KeyValue instances. The ListP

Re: [ovs-dev] [PATCH v2 01/18] python: add generic Key-Value parser

2022-02-04 Thread Mark Michelson
On 1/28/22 11:04, Adrian Moreno wrote: Most of ofproto and dpif flows are based on key-value pairs. These key-value pairs can be represented in several ways, eg: key:value, key=value, key(value). Add the following classes that allow parsing of key-value strings: * KeyValue: holds a key-value pai

[ovs-dev] [PATCH v3 0/2] fix dpif backer revalidation

2022-02-04 Thread lic121
v3: - fix lldp test case failure v2: - add TODO comments to clear ipfix patch coverage - test lldp enabling more than pointer - add test cases for lldp ovsdb change or netlink notification trigger bridge_reconfigure. In bridge_reconfigure, backer->need_revalidate flag is set if backer revalidatio

Re: [ovs-dev] [PATCH v2 1/2] ofproto-dpif: trigger revalidation when ipfix config set

2022-02-04 Thread lic121
>On 4 Feb 2022, at 15:25, lic121 wrote: > >>> On 28 Jan 2022, at 7:40, lic121 wrote: >>> Currently, ipfix conf creation/deletion don't trigger dpif backer revalidation. This is not expected, as we need the revalidation to commit ipfix into xlate. So that xlate can generate ipfix

Re: [ovs-dev] [PATCH v2] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Numan Siddique
On Fri, Feb 4, 2022 at 4:51 PM Ilya Maximets wrote: > > On 2/4/22 20:39, Dumitru Ceara wrote: > > On 2/4/22 17:32, Ilya Maximets wrote: > >> GHA is broken due to update to pip>=22.0. This happens because now it > >> stops backtracking packages on build failure making it impossible to > >> find wo

[ovs-dev] [PATCH v2 ovn] introduce rdnss, dnssl and route_info opt in put_nd_ra_opts action

2022-02-04 Thread Lorenzo Bianconi
Send non-periodic Router Advertisement with rdnss, dnssl and route_info options if configured by the user. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1851788 Signed-off-by: Lorenzo Bianconi --- Changes since v1: - copy input string before running strtok_r() in encode_ra_dnssl_opt()

Re: [ovs-dev] [PATCH ovn] introduce rdnss, dnssl and route_info opt in put_nd_ra_opts action

2022-02-04 Thread Lorenzo Bianconi
> Hi Lorenzo, the change is mostly good but I have a few findings down below: Hi Mark, thx for the review. > > On 2/1/22 12:59, Lorenzo Bianconi wrote: > > Send non-periodic Router Advertisement with rdnss, dnssl and route_info > > options if configured by the user. > > > > Reported-at: https:

Re: [ovs-dev] [PATCH] ovsdb-idl: Only process successful txn in ovsdb_idl_loop_run.

2022-02-04 Thread Ilya Maximets
On 1/28/22 10:08, Dumitru Ceara wrote: > Otherwise we hide the transaction result from the user. This may cause > problems as the user will not detect error cases. For example, if the > server refuses a transaction with "constraint violation", the user > should be notified because the transaction

Re: [ovs-dev] [PATCH 1/1] odp-util: Fix tunnel key attr for GTP-U.

2022-02-04 Thread Ilya Maximets
On 2/1/22 02:05, Nobuhiro MIKI wrote: > CC: William Tu > Fixes: 3c6d05a02e0f ("userspace: Add GTP-U support.") > Signed-off-by: Nobuhiro MIKI > --- > lib/odp-util.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 503675e83..9a705c

Re: [ovs-dev] [PATCH branch-2.14] ovsdb-idl: Clear last_id on reconnect if condition changes in-flight.

2022-02-04 Thread Ilya Maximets
On 2/1/22 11:15, Dumitru Ceara wrote: > When reconnecting, if there are condition changes already sent to the > server but not yet acked, reset the db's 'last-id', esentially clearing > the local cache after reconnect. > > This is needed because the client cannot easily differentiate between > the

Re: [ovs-dev] [PATCH v2] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Ilya Maximets
On 2/4/22 20:39, Dumitru Ceara wrote: > On 2/4/22 17:32, Ilya Maximets wrote: >> GHA is broken due to update to pip>=22.0. This happens because now it >> stops backtracking packages on build failure making it impossible to >> find working combination of versions. >> >> We're not able to build 'hac

Re: [ovs-dev] [PATCH v2] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Mark Michelson
On 2/4/22 14:39, Dumitru Ceara wrote: On 2/4/22 17:32, Ilya Maximets wrote: GHA is broken due to update to pip>=22.0. This happens because now it stops backtracking packages on build failure making it impossible to find working combination of versions. We're not able to build 'hacking', becaus

Re: [ovs-dev] [PATCH] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Ilya Maximets
On 2/4/22 20:51, Aaron Conole wrote: > Ilya Maximets writes: > >> GHA is broken due to update to pip>=22.0. This happens because now it >> stops backtracking packages on build failure making it impossible to >> find working combination of versions. >> >> We're not able to build 'hacking', becaus

Re: [ovs-dev] [PATCH] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Aaron Conole
Ilya Maximets writes: > GHA is broken due to update to pip>=22.0. This happens because now it > stops backtracking packages on build failure making it impossible to > find working combination of versions. > > We're not able to build 'hacking', because 'wheel' is not installed > at that point in

Re: [ovs-dev] [PATCH v2] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Dumitru Ceara
On 2/4/22 17:32, Ilya Maximets wrote: > GHA is broken due to update to pip>=22.0. This happens because now it > stops backtracking packages on build failure making it impossible to > find working combination of versions. > > We're not able to build 'hacking', because 'wheel' is not installed > at

Re: [ovs-dev] [PATCH ovn] ovs-sandbox: Convert initial databases if needed.

2022-02-04 Thread Mark Michelson
Acked-by: Mark Michelson On 1/26/22 09:17, Dumitru Ceara wrote: The --nbdb-source and --sbdb-source options allow users to point the sandbox daemons to already existing database files. It's useful if these are first converted to use the currently supported schema. Signed-off-by: Dumitru Ceara

Re: [ovs-dev] [PATCH ovn v2] Copy external_ids from Logical_Router_Port to SB database This patch makes ovn-northd copy all string-string pairs in external_ids column of the Logical_Router_Port table

2022-02-04 Thread Mark Michelson
Hello, I had a look, and while the code does what it claims, it's not clear why you want to do this. Can you provide some background? Also, it appears in this version of the patch, you put the entire description in the subject line :) Thanks On 1/27/22 07:50, Selvaraj Palaniyappan wrote:

Re: [ovs-dev] [PATCH ovn] controller: Use precomputed is_switch instead of querying external IDs.

2022-02-04 Thread Dumitru Ceara
On 2/4/22 16:10, Mark Michelson wrote: > The idea looks good to me, but I have an additional question. Is it > still useful to have the datapath_is_switch() function exposed via > ovn-util.h? Aside from when the local_datapath has its is_switch field > assigned, is there an occasion where this func

[ovs-dev] [PATCH v2] ovsdb-idl: Fix use-after-free when destroying an IDL loop.

2022-02-04 Thread Dumitru Ceara
Transactions that are still incomplete (waiting for a reply from the server) are kept in the IDL's 'outstanding_txns' map. When a transaction is destroyed, ovsdb_idl_txn_destroy() will take care of removing the transaction from the 'outstanding_txns' map if the transaction was incomplete but also

Re: [ovs-dev] [PATCH] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Ilya Maximets
CC: ovs-dev Not sure how the mail list fell off. Adding back. On 2/4/22 16:49, Ilya Maximets wrote: > On 2/4/22 16:25, Gaëtan Rivet wrote: >> On Fri, Feb 4, 2022, at 16:04, Ilya Maximets wrote: >>> GHA is broken due to update to pip>=22.0. This happens because now it >>> stops backtracking packa

[ovs-dev] [PATCH v2] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Ilya Maximets
GHA is broken due to update to pip>=22.0. This happens because now it stops backtracking packages on build failure making it impossible to find working combination of versions. We're not able to build 'hacking', because 'wheel' is not installed at that point in time. Installing it separately to

Re: [ovs-dev] [PATCH v1 2/3] dpif-netdev: Fix a race condition in deletion of offloaded flows

2022-02-04 Thread 0-day Robot
Bleep bloop. Greetings Gaetan 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: Unexpected sign-offs from developers who are not authors or co-authors or committers:

Re: [ovs-dev] [PATCH v1 1/3] dpif-netdev: Move port flush after datapath reconfiguration

2022-02-04 Thread 0-day Robot
Bleep bloop. Greetings Gaetan 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: Unexpected sign-offs from developers who are not authors or co-authors or committers:

[ovs-dev] [PATCH v1 1/3] dpif-netdev: Move port flush after datapath reconfiguration

2022-02-04 Thread Gaetan Rivet
Port flush and offload uninit should be moved after the datapath has been reconfigured. That way, no other thread such as PMDs will find this port to poll and enqueue further offload requests. After a flush, almost no further offload request for this port should be found in the queue. There will

[ovs-dev] [PATCH v1 3/3] dpif-netdev: Use dp_netdev reference in offload threads

2022-02-04 Thread Gaetan Rivet
The PMD reference taken is not actually used, it is only needed to get the dp_netdev linked. Additionally, the taking of the PMD reference does not protect against the disappearance of the dp_netdev, so it is misleading. The dp reference is protected by the way the ports are being deleted during d

[ovs-dev] [PATCH v1 2/3] dpif-netdev: Fix a race condition in deletion of offloaded flows

2022-02-04 Thread Gaetan Rivet
From: Sriharsha Basavapatna In dp_netdev_pmd_remove_flow() we schedule the deletion of an offloaded flow, if a mark has been assigned to the flow. But if this occurs in the window in which the offload thread completes offloading the flow and assigns a mark to the flow, then we miss deleting the f

[ovs-dev] [PATCH v1 0/3] Fix offload rule flush race condition

2022-02-04 Thread Gaetan Rivet
A race condition has been identified during datapath destruction, along with the port offload flushes issued. This series addresses these race conditions, cleaning up the port deletion process. The last patch also cleans up the offload structure. It is not strictly necessary like the first two fi

[ovs-dev] [PATCH ovn v2] ovn.at: Fix flaky tests "VLAN transparency, passthru=true, multiple hosts"

2022-02-04 Thread Xavier Simonart
Tests were waiting for ports to be reported up before sending packets. However, waiting for both ports to be up is not enough to guarantee that all flows are installed for both ports. We now wait for flows outputting to patch port to be installed Following tests were are fixed: - VLAN transparency

Re: [ovs-dev] [ovs-dev v4] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread Gaëtan Rivet
Thank you Peng for doing this. On Tue, Jan 18, 2022, at 16:01, Peng He wrote: > From hepeng: > https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473 > > also from guohongzhi : > http://patchwork.ozlabs.org/project/openvswitch/patch/2020030

Re: [ovs-dev] [PATCH ovn] ovn-controller: Avoid reprocessing same lflows in the same I-P run.

2022-02-04 Thread Mark Michelson
As far as I can tell, this looks correct. Acked-by: Mark Michelson On 1/20/22 13:06, Han Zhou wrote: For I-P node lflow_output, different change handlers can trigger reprocessing lflows. However, it is a waste to reprocess the same lflow multiple times in the same run, because each processing

Re: [ovs-dev] [PATCH] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Gaëtan Rivet
On Fri, Feb 4, 2022, at 16:04, Ilya Maximets wrote: > GHA is broken due to update to pip>=22.0. This happens because now it > stops backtracking packages on build failure making it impossible to > find working combination of versions. > > We're not able to build 'hacking', because 'wheel' is not i

Re: [ovs-dev] [PATCH ovn] controller: Use precomputed is_switch instead of querying external IDs.

2022-02-04 Thread Mark Michelson
The idea looks good to me, but I have an additional question. Is it still useful to have the datapath_is_switch() function exposed via ovn-util.h? Aside from when the local_datapath has its is_switch field assigned, is there an occasion where this function would be useful? Could we make it a lo

[ovs-dev] [PATCH] ci: Install wheel before installing any other python packages.

2022-02-04 Thread Ilya Maximets
GHA is broken due to update to pip>=22.0. This happens because now it stops backtracking packages on build failure making it impossible to find working combination of versions. We're not able to build 'hacking', because 'wheel' is not installed at that point in time. Installing it separately to

Re: [ovs-dev] [PATCH v2 1/2] ofproto-dpif: trigger revalidation when ipfix config set

2022-02-04 Thread Eelco Chaudron
On 4 Feb 2022, at 15:25, lic121 wrote: >> On 28 Jan 2022, at 7:40, lic121 wrote: >> >>> Currently, ipfix conf creation/deletion don't trigger dpif backer >>> revalidation. This is not expected, as we need the revalidation >>> to commit ipfix into xlate. So that xlate can generate ipfix >>> acti

Re: [ovs-dev] [PATCH ovn] introduce rdnss, dnssl and route_info opt in put_nd_ra_opts action

2022-02-04 Thread Mark Michelson
Hi Lorenzo, the change is mostly good but I have a few findings down below: On 2/1/22 12:59, Lorenzo Bianconi wrote: Send non-periodic Router Advertisement with rdnss, dnssl and route_info options if configured by the user. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1851788 Signed

Re: [ovs-dev] [PATCH v2 1/2] ofproto-dpif: trigger revalidation when ipfix config set

2022-02-04 Thread lic121
>On 28 Jan 2022, at 7:40, lic121 wrote: > >> Currently, ipfix conf creation/deletion don't trigger dpif backer >> revalidation. This is not expected, as we need the revalidation >> to commit ipfix into xlate. So that xlate can generate ipfix >> actions. >> >> This patch covers only new creation/del

Re: [ovs-dev] [PATCH v2 2/2] ofproto-dpif: avoid unneccesary backer revalidation

2022-02-04 Thread lic121
>On 28 Jan 2022, at 7:41, lic121 wrote: > >> If lldp didn't change, we are not supposed to trigger backer >> revalidation. >> >> Without this patch, bridge_reconfigure() always trigger udpif >> revalidator because of lldp. >> >> Signed-off-by: lic121 >> --- >> lib/ovs-lldp.c | 8

Re: [ovs-dev] [PATCH] ovsdb-idl: Fix use-after-free when destroying an IDL loop.

2022-02-04 Thread Dumitru Ceara
On 2/4/22 12:14, Ilya Maximets wrote: > On 2/2/22 15:51, Dumitru Ceara wrote: >> Transactions that are still incomplete (waiting for a reply from the >> server) are kept in the IDL's 'outstanding_txns' map. When a transaction >> is destroyed, ovsdb_idl_txn_destroy() will take care of removing the

Re: [ovs-dev] [PATCH net-next v3 1/1] net/sched: Enable tc skb ext allocation on chain miss only when needed

2022-02-04 Thread Jamal Hadi Salim
On 2022-02-03 03:44, Paul Blakey wrote: Currently tc skb extension is used to send miss info from tc to ovs datapath module, and driver to tc. For the tc to ovs miss it is currently always allocated even if it will not be used by ovs datapath (as it depends on a requested feature). Export the st

Re: [ovs-dev] [ovs-dev v4] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread Mike Pattrick
On Fri, Feb 4, 2022 at 8:37 AM Adrian Moreno wrote: > > > > On 1/18/22 16:01, Peng He wrote: > > From hepeng: > > https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473 > > > > also from guohongzhi : > > http://patchwork.ozlabs.org/project

Re: [ovs-dev] [ovs-dev v4] ofproto: add refcount to ofproto to fix ofproto use-after-free

2022-02-04 Thread Adrian Moreno
On 1/18/22 16:01, Peng He wrote: From hepeng: https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473 also from guohongzhi : http://patchwork.ozlabs.org/project/openvswitch/patch/20200306130555.19884-1-guohongz...@huawei.com/ also from

Re: [ovs-dev] [PATCH v2] tc: Fix incorrect TC rule for decap+encap datapath flow

2022-02-04 Thread Eelco Chaudron
On 2 Dec 2021, at 13:38, Roi Dayan via dev wrote: > A datapath flow generated for traffic from vxlan port to another vxlan port > looks like this: > > tunnel(tun_id=0x65,src=10.10.11.3,dst=10.10.11.2,ttl=0/0,tp_dst=4789,flags(+key)),...,in_port(vxlan_sys_4789),..., > > actions:set(tunnel(tun_i

Re: [ovs-dev] [PATCH v2 1/2] ofproto-dpif: trigger revalidation when ipfix config set

2022-02-04 Thread Eelco Chaudron
On 28 Jan 2022, at 7:40, lic121 wrote: > Currently, ipfix conf creation/deletion don't trigger dpif backer > revalidation. This is not expected, as we need the revalidation > to commit ipfix into xlate. So that xlate can generate ipfix > actions. > > This patch covers only new creation/deletion

Re: [ovs-dev] [PATCH v2 2/2] ofproto-dpif: avoid unneccesary backer revalidation

2022-02-04 Thread Eelco Chaudron
On 28 Jan 2022, at 7:41, lic121 wrote: > If lldp didn't change, we are not supposed to trigger backer > revalidation. > > Without this patch, bridge_reconfigure() always trigger udpif > revalidator because of lldp. > > Signed-off-by: lic121 > --- > lib/ovs-lldp.c | 8 > lib/

Re: [ovs-dev] [PATCH] dpif-netdev-dpcls: Make subtable reprobe thread-safe.

2022-02-04 Thread Ilya Maximets
On 2/2/22 15:51, Cian Ferriter wrote: > The subtable search function can be used at any time by a PMD thread. > Setting the subtable search function should be done atomically to > prevent garbage data from being read. > > A dpcls_subtable_lookup_reprobe() call can happen at the same time that > DP

Re: [ovs-dev] [PATCH] ovsdb-idl: Fix use-after-free when destroying an IDL loop.

2022-02-04 Thread Ilya Maximets
On 2/2/22 15:51, Dumitru Ceara wrote: > Transactions that are still incomplete (waiting for a reply from the > server) are kept in the IDL's 'outstanding_txns' map. When a transaction > is destroyed, ovsdb_idl_txn_destroy() will take care of removing the > transaction from the 'outstanding_txns' m

Re: [ovs-dev] [PATCH v3] dpif-netdev: fix vlan and ipv4 parsing in avx512

2022-02-04 Thread Eelco Chaudron
On 31 Jan 2022, at 14:54, Harry van Haaren wrote: > This commit fixes the minimum packet size for the vlan/ipv4/tcp > traffic profile, which was previously incorrectly set. > > This commit also disallows any fragmented IPv4 packets from being > matched in the optimized miniflow-extract, avoiding