Looks good to me.
Reviewed-by: Harold Huang
于2022年1月25日周二 13:25写道：
>
> From: Tonghao Zhang
>
> The old version of openvswitch doesn't remove the padding from
> packet before L3+ conntrack processing and then packets are dropped
> in linux kernel stack. The patch [1] fixes the issue. We fix
From: Tonghao Zhang
The old version of openvswitch doesn't remove the padding from
packet before L3+ conntrack processing and then packets are dropped
in linux kernel stack. The patch [1] fixes the issue. We fix this
issue on gateway which running ovs-dpdk as a quick workaround. Padding
should
On Sun, Dec 19, 2021 at 6:09 AM Ilya Maximets wrote:
>
> If a single transaction exceeds the size of the whole database (e.g.,
> a lot of rows got removed and new ones added), transaction history will
> be drained. This leads to sending UUID_ZERO to the clients as the last
> transaction id in
On Sun, Dec 19, 2021 at 6:09 AM Ilya Maximets wrote:
>
> Current code doesn't use the last id received in the monitor reply.
> That may result in re-downloading the database content if the
> re-connection happened after receiving the initial monitor reply,
> but before receiving any other
Currently tc offload flow packet counters will roll over every ~4
billion packets. This is because the packet counter in struct
tc_stats provided by TCA_STATS_BASIC is a 32bit integer.
Now we check for the optional TCA_STATS_PKT64 attribute which provides
the full 64bit packet counter if the
Bleep bloop. Greetings Mike Pattrick, 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.
build:
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include
-I ./lib -I ./lib
Currently tc offload flow packet counters will roll over every ~4
billion packets. This is because the packet counter in struct
tc_stats provided by TCA_STATS_BASIC is a 32bit integer.
Now we check for the optional TCA_STATS_PKT64 attribute which provides
the full 64bit packet counter if the
On 1/24/22 19:40, Adrian Moreno wrote:
>
>
> On 1/24/22 18:49, Jeffrey Walton wrote:
>> On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara wrote:
>>>
>>> As privately reported by Aaron Conole, and by Jeffrey Walton [0]
>>> there's currently a number of undefined behavior instances in
>>> the OVS
On 1/24/22 17:57, Jeffrey Walton wrote:
> On Mon, Jan 24, 2022 at 9:21 AM Dumitru Ceara wrote:
>>
>> Note: There still is an UB instance when using SSE4.2 as reported here:
>> https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390904.html
>>
>> Acked-by: Aaron Conole
>> Signed-off-by:
On 1/24/22 17:42, Jeffrey Walton wrote:
> Hi Dumitru,
Hi Jeff,
>
> One small comment. I held it back a few days ago, but I should
> probably point it out...
>
Thanks for looking into this.
> On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara wrote:
>>
>> As privately reported by Aaron Conole,
On Tue, Dec 7, 2021 at 11:54 AM Flavio Leitner wrote:
>
> The netdev receiving packets is supposed to provide the flags
> indicating if the IP csum was verified and it is OK or BAD,
> otherwise the stack will check when appropriate by software.
>
> If the packet comes with good checksum, then
On 1/24/22 18:49, Jeffrey Walton wrote:
On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara wrote:
As privately reported by Aaron Conole, and by Jeffrey Walton [0]
there's currently a number of undefined behavior instances in
the OVS code base. Running the OVS (and OVN) tests with UBSan [1]
On Tue, Dec 7, 2021 at 11:53 AM Flavio Leitner wrote:
>
> Currently 'p' and 'b' and used for packets, so use
> a convention that struct dp_packet is 'p' and
> struct dp_packet_batch is 'b'.
>
> Some comments needed new formatting to not pass the
> 80 column.
>
> Some variables were using 'p' or
On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara wrote:
>
> As privately reported by Aaron Conole, and by Jeffrey Walton [0]
> there's currently a number of undefined behavior instances in
> the OVS code base. Running the OVS (and OVN) tests with UBSan [1]
> enabled uncovers these.
> ...
> Note:
This patch fixes the thread mode part, as the static
thread-to-txq mapping selection depends on whether the
number of queues is strictly greater than the number of
PMD threads, and anot greater or equal.
The section is also reworded as per Ilya's suggestion.
Fixes: c18e707b2f25 ("dpif-netdev:
This patch documents PMD's other_config:tx-steering option.
Acked-by: Kevin Traynor
Signed-off-by: Maxime Coquelin
---
vswitchd/vswitch.xml | 23 +++
1 file changed, 23 insertions(+)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 064e0facf..0c6632617 100644
This series is a follow-up of "Add missing tx-steering PMD
option." patch. The second patch fixes incorrect statement on
static/dynamic thread-to-txq mapping in thread mode.
Changes in v2:
==
- Add Documentation patch to fix incorrect statement (Kevin)
- Reword thread mode
On Mon, Jan 24, 2022 at 9:21 AM Dumitru Ceara wrote:
>
> Note: There still is an UB instance when using SSE4.2 as reported here:
> https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390904.html
>
> Acked-by: Aaron Conole
> Signed-off-by: Dumitru Ceara
> ---
> v3:
> - Fix typo in
Hi Dumitru,
One small comment. I held it back a few days ago, but I should
probably point it out...
On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara wrote:
>
> As privately reported by Aaron Conole, and by Jeffrey Walton [0]
> there's currently a number of undefined behavior instances in
> the OVS
Making the log extack messages as ERR rather than DBG will help debugging
when we dont know for sure where is the issue. If it is DBG these useful
log extack messages might get lost due to excessive logs. Also these
error messages will be printed only when we enable it.
On 1/21/22 12:25, Lorenzo Bianconi wrote:
> Introduce the capability to specify CoPP UUID or CoPP name in order to
> reuse the same CoPP reference on multiple datapaths.
> Introduce logical_router and logical_switches columns in CoPP table in
> order to specify datapaths where CoPP is installed.
>
On 24/01/2022 16:00, Maxime Coquelin wrote:
On 1/20/22 14:20, Kevin Traynor wrote:
On 20/01/2022 12:16, Ilya Maximets wrote:
On 1/18/22 11:51, Kevin Traynor wrote:
On 17/01/2022 22:25, Maxime Coquelin wrote:
This patch documents PMD's other_config:tx-steering option.
Signed-off-by: Maxime
On 1/20/22 14:20, Kevin Traynor wrote:
On 20/01/2022 12:16, Ilya Maximets wrote:
On 1/18/22 11:51, Kevin Traynor wrote:
On 17/01/2022 22:25, Maxime Coquelin wrote:
This patch documents PMD's other_config:tx-steering option.
Signed-off-by: Maxime Coquelin
---
vswitchd/vswitch.xml | 22
On 1/20/22 18:08, Lorenzo Bianconi wrote:
> Use VLOG_INFO_RL in engine_recompute and engine_compute routines
> to log compute time over a given threshold (i.e. 500ms).
> Make timeout threshold configurable through ovs-appctl command:
>
> $ovn-appctl -t ovn-controller
> -Original Message-
> From: Eelco Chaudron
> Sent: Friday, January 14, 2022 10:03 AM
> To: Van Haaren, Harry
> Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [PATCH] dpif-netdev: fix handling of vlan and ipv4
> parsing
> in avx512
> > -#define
On 24/01/2022 11:14, Wan Junjie wrote:
The ```rte_mempool_avail_count``` and ```rte_mempool_in_use_count```
can tell us the usage of the mempool. It could be helpful for debug
on any memleak in the mempool.
Add a line in the cmd's output.
- Count: avail (118988), in use (12084)
Signed-off-by:
On Mon, Jan 17, 2022 at 10:28 PM Peng He wrote:
>
> considering a multi-thread PMD setting, when the frags are reassembled
> in one PMD, another thread might call *ipf_execute_reass_pkts* and 'steal'
> the reassembled packets into its ipf ctx, then this reassembled
> packet will enter into
On Mon, Jan 17, 2022 at 10:44 PM Peng He wrote:
>
> In the commit 1df7f7aac8c976690167261fec9a50d832ef9e7e, the packet
> metadata's in_port has been changed from ofp port into odp port,
> however, the odp->flow->in_port is still ofp_port. This patch changes
> the odp->flow->in_port into odp_port.
On 1/24/22 13:36, Dumitru Ceara wrote:
> On 1/21/22 15:34, Jeffrey Walton wrote:
>> On Fri, Jan 21, 2022 at 3:19 AM Dumitru Ceara wrote:
>>>
>>> On 1/21/22 02:12, Jeffrey Walton wrote:
>>>
I am testing Open vSwitch 2.16.2. Undefined Behavior sanitizer is
producing some findings.
Note: There still is an UB instance when using SSE4.2 as reported here:
https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390904.html
Acked-by: Aaron Conole
Signed-off-by: Dumitru Ceara
---
v3:
- Fix typo in variable name.
- Added SSE4.2 UB note to commit log.
---
.ci/linux-build.sh
Reported-by: David Marchand
Signed-off-by: Dumitru Ceara
---
.ci/linux-build.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 8d111b6d7f6b..6cd38ff3efb5 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -246,8
UB Sanitizer was flagging OBJECT_CONTAINING() as undefined behavior
due to the way it's used when iterating through collections, e.g.,
HMAP_FOR_EACH_INIT(). However, in such cases we don't actually
dereference the pointer, we just use its value. Avoid the undefined
behavior by casting to 'void
Some openflow actions can be misaligned, e.g., actions whithin OF 1.0
replies to statistics reply messages which have a header of 12 bytes
and no additional padding.
Also, buggy controllers might incorrectly encode actions.
When decoding multiple actions in ofpacts_decode(), make sure that
when
Trim the ofpbuf to ensure proper alignment.
UB Sanitizer report:
lib/ofp-print.c:1218:24: runtime error: member access within misaligned
address 0x019229d2 for type 'const struct ofp_header', which requires 4
byte alignment
0x019229d2: note: pointer points here
00 00 5a 5a 05 22
For example is parsing the OVN "eth.dst[40] = 1;" action, which
triggered the following warning from UndefinedBehaviorSanitizer:
lib/meta-flow.c:3210:9: runtime error: member access within misaligned
address 0x00de4e36 for type 'const union mf_value', which requires 8 byte
alignment
This is undefined behavior and was reported by UB Sanitizer:
lib/meta-flow.c:3445:16: runtime error: member access within null pointer of
type 'struct vl_mf_field'
#0 0x6aad0f in mf_get_vl_mff lib/meta-flow.c:3445
#1 0x6d96d7 in mf_from_oxm_header lib/nx-match.c:260
#2
UB Sanitizer reports:
tests/test-hash.c:59:40: runtime error: shift exponent 64 is too large for
64-bit type 'long unsigned int'
#0 0x44c3c9 in get_range128 tests/test-hash.c:59
#1 0x44cb2e in check_hash_bytes128 tests/test-hash.c:178
#2 0x44d14d in test_hash_main
UB Sanitizer report:
lib/dp-packet.h:297:39: runtime error: applying zero offset to null pointer
#0 0x7946f5 in dp_packet_tail /root/ovs/./lib/dp-packet.h:297:39
#1 0x794331 in dp_packet_tailroom /root/ovs/./lib/dp-packet.h:325:49
#2 0x7942a0 in dp_packet_prealloc_tailroom
UB Sanitizer reports:
lib/bfd.c:748:16: runtime error: member access within misaligned address
0x01f0d6ea for type 'struct msg', which requires 4 byte alignment
0x01f0d6ea: note: pointer points here
00 20 00 00 20 40 03 18 93 f9 0a 6e 00 00 00 00 00 0f 42 40 00 0f 42 40
00 00
Reported by UndefinedBehaviorSanitizer:
tests/idltest.c:3602:12: runtime error: member access within null pointer of
type 'const struct idltest_simple'
#0 0x4295af in idltest_simple_cursor_first_ge tests/idltest.c:3602
#1 0x41c81b in test_idl_compound_index_single_column
UB Sanitizer report:
lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of
bounds for type 'long long unsigned int [50]'
#0 0x698358 in calc_percentile lib/stopwatch.c:119
#1 0x69ada1 in add_sample lib/stopwatch.c:231
#2 0x69c086 in
Remove the forced cache-line size alignment markers from
struct dp_netdev_pmd_thread and struct dp_netdev as discussed
at [0]. They don't seem to add any benefit and cause 64 byte
alignment requirements.
UB Sanitizer report:
lib/dpif-netdev.c:6758:13: runtime error: member access within
It's actually undefined behavior to pass NULL to standard library
functions that manipulate arrays (e.g., qsort, memcpy, memcmp), even if
the passed number of items is 0.
UB Sanitizer reports:
ovsdb/monitor.c:408:9: runtime error: null pointer passed as argument 1,
which is declared to never
As privately reported by Aaron Conole, and by Jeffrey Walton [0]
there's currently a number of undefined behavior instances in
the OVS code base. Running the OVS (and OVN) tests with UBSan [1]
enabled uncovers these.
This series fixes the issues reported by UBSan and, throught the last
patch,
Wan Junjie writes:
> The ```rte_mempool_avail_count``` and ```rte_mempool_in_use_count```
> can tell us the usage of the mempool. It could be helpful for debug
> on any memleak in the mempool.
> Add a line in the cmd's output.
> - Count: avail (118988), in use (12084)
>
> Signed-off-by: Wan
On Thu, Jan 6, 2022 at 5:51 AM David Marchand wrote:
>
> The experimantal typo got copy/paste a few times.
>
> Fixes: be56e063d028 ("netdev-offload-dpdk: Support tunnel pop action.")
> Fixes: e098c2f966cb ("netdev-dpdk-offload: Add vxlan pattern matching
> function.")
> Fixes: 7617d0583c73
On 1/21/22 15:34, Jeffrey Walton wrote:
> On Fri, Jan 21, 2022 at 3:19 AM Dumitru Ceara wrote:
>>
>> On 1/21/22 02:12, Jeffrey Walton wrote:
>>
>>> I am testing Open vSwitch 2.16.2. Undefined Behavior sanitizer is
>>> producing some findings.
>>>
>>> My question is, is the undefined behavior
We are currently requiring in_port to be a valid port number for ipfix
sampling even if the sampling is done on the output port (egress).
This restriction is not really needed and can affect pipelines that
intentionally set the in_port to OFPP_NONE during flow processing. For
instance, OVN does
After commit [0] the mandatory vtep chassis's option 'is-vtep' has appeared.
The upgrade scenario for ovn-controller-vtep was not supported:
'is-vtep' option was set only on vtep chassis creation. If chassis was
created prior to a new codebase, it was left intact and HW VTEP connectivity
was
The ```rte_mempool_avail_count``` and ```rte_mempool_in_use_count```
can tell us the usage of the mempool. It could be helpful for debug
on any memleak in the mempool.
Add a line in the cmd's output.
- Count: avail (118988), in use (12084)
Signed-off-by: Wan Junjie
---
lib/netdev-dpdk.c | 3 +++
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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: sha1 information is lacking or useless (lib/netdev-dpdk.c).
error: could not build fake
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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: Improper whitespace around control block
#397 FILE: lib/ovs-numa.h:71:
Bleep bloop. Greetings Adrian Moreno, 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.
Patch skipped due to previous failure.
Please check this out. If you feel there has been an error,
Bleep bloop. Greetings Adrian Moreno, 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: Use ovs_assert() in place of assert()
#127 FILE: tests/test-list.c:64:
assert(e ==
On 1/20/22 11:31, Eelco Chaudron wrote:
On 20 Jan 2022, at 11:06, Adrian Moreno wrote:
On 1/20/22 10:13, Eelco Chaudron wrote:
On 17 Jan 2022, at 10:27, Adrian Moreno wrote:
We are currently requiring in_port to be a valid port number for ipfix
sampling even if the sampling is done on
Apart from being safer from the point of view of potential undefined
behavior, the multi-variable version of the safe iterators have the
benefit of not requiring the declaration of an external variable to hold
the next position.
This patch removes the NEXT variable from HMAP_FOR_ALL_SAFE macro
Apart from being safer from the point of view of potential undefined
behavior, the multi-variable version of the safe iterators have the
benefit of not requiring the declaration of an external variable to hold
the next position.
This patch removes the NEXT variable from all HINDEX_*_SAFE macros
Use multi-variable iteration helpers to rewrite rculist loops macros.
There is an important behavior change compared with the previous
implementation: When the loop ends normally (i.e: not via "break;"),
the object pointer provided by the user is NULL. This is safer
because it's not guaranteed
The _SAFE version of the iterator allows the user to free the iterator
structure safely.
Signed-off-by: Adrian Moreno
---
vtep/vtep-ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 99c4adcd5..685b0007a 100644
--- a/vtep/vtep-ctl.c
Re-write hindex's loops using multi-variable helpers.
For safe loops, keep the (now unused) NEXT variable to maintain
backwards compatibility.
Signed-off-by: Adrian Moreno
---
lib/hindex.h | 42 +++---
1 file changed, 23 insertions(+), 19 deletions(-)
diff
Re-write cmap's loops using multi-variable helpers.
For iterators based on cmap_cursor, we just need to make sure the NODE
variable is not used after the loop, so we set it to NULL.
Signed-off-by: Adrian Moreno
---
lib/cmap.h | 24 ++--
1 file changed, 14 insertions(+), 10
HMAP_FOR_EACH_POP iterator has an additional difficulty, which is the
use of two iterator variables of different types.
In order to re-write this loop in a UB-safe manner, create a iterator
struct to be used as loop variable.
Signed-off-by: Adrian Moreno
---
include/openvswitch/hmap.h | 31
Apart from being safer from the point of view of potential undefined
behavior, the multi-variable version of the safe iterators have the
benefit of not requiring the declaration of an external variable
to hold the next position.
This patch removes the extra variable from the LIST_FOR_EACH_SAFE
Rewrite hmap's loops using multi-variable helpers.
For safe loops, we keep the (now unused) NEXT variable to keep backwards
compatibility.
Signed-off-by: Adrian Moreno
---
include/openvswitch/hmap.h | 61 +++---
1 file changed, 31 insertions(+), 30 deletions(-)
After the loop ends, the iterator is not guaranteed to point to a valid
object and should not be used. Make it NULL to avoid undefined behavior.
Signed-off-by: Adrian Moreno
---
include/openvswitch/list.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git
Use multi-variable iteration helpers to rewrite non-safe loops.
There is an important behavior change compared with the previous
implementation: When the loop ends normally (i.e: not via "break;"), the
object pointer provided by the user is NULL. This is safer because it's
not guaranteed that it
Safe version of multi-variable iterator helpers declare an internal
variable to store the next value of the iterator temporarily. This
eliminates the need to declare an external variable to store it, making
loops more readable and less error prone.
Signed-off-by: Adrian Moreno
---
Multi-variable loop iterators avoid potential undefined behavior by
using an internal iterator variable to perform the iteration and only
referencing the containing object (via OBJECT_CONTAINING) if the
iterator has been validated via the second expression of the for
statement.
That way, the user
When running builds with UBSan, some undefined behavior was detected in the
iteration of common data data structures in OVS.
Coincidentally, a bug was reported [1] whose root cause whas another, this time
undetected, undefined behavior in the iteration macros.
>From both cases, we conclude that
75 matches
Mail list logo
