Re: [ovs-dev] [PATCH] hash, jhash: Fix unaligned access to the hash remainder.

On Thu, May 2, 2024 at 1:22 PM Ilya Maximets wrote: > On 5/2/24 12:22, Ales Musil wrote: > > The has was passed to memcpy as uin32_t *, however the hash bytes > > 'The has was passed' ? :) > Oops :) > > > might be unaligned at that point. Case it to uint8_t * instead > > which has only single

[ovs-dev] [PATCH ovn v4 0/2] Add support to disable vxlan mode.

v4: - Addressed Dumitru's and Ihar's review comments; - single patch was split into two: 1. function call replaced with a global variable `vxlan_mode`; 2. introduced `disable_vxlan_mode` configuration knob; - rebased onto latest main branch. v3: - Removed accidental ovs submodule

[ovs-dev] [PATCH ovn v4 2/2] northd: Add support for disabling vxlan mode.

Commit [1] introduced a "vxlan mode" concept. It brought a limitation for available tunnel IDs because of lack of space in VXLAN VNI. In vxlan mode OVN is limited by 4095 datapaths (LRs or non-transit LSs) and 2047 logical switch ports per datapath. Prior to this patch vxlan mode was enabled

[ovs-dev] [PATCH ovn v4 1/2] northd: Make `vxlan_mode` a global variable.

This simplifies code and subsequent commit to explicitely disable vxlan mode is based on these changes. Also `vxlan mode` term is introduced in ovn-architecture man page. Signed-off-by: Vladislav Odintsov --- northd/en-global-config.c | 4 +- northd/northd.c | 94

[ovs-dev] [PATCH] sparse: Add additional define for sparse on GCC >= 14.

GCC 14 renamed one of the AVX512 defines to have only single underscore instead of two [0]. Add the single underscore define to keep compatibility with multiple GCC versions. [0] https://github.com/gcc-mirror/gcc/commit/aea8e4105553cd16799f2134d15420ccf182d732 Tested-by: Dumitru Ceara

[ovs-dev] [PATCH] hash, jhash: Fix unaligned access to the hash remainder.

The has was passed to memcpy as uin32_t *, however the hash bytes might be unaligned at that point. Case it to uint8_t * instead which has only single byte alignment requirement. lib/hash.c:46:22: runtime error: load of misaligned address 0x50700065 for type 'const uint32_t *' (aka 'const

Re: [ovs-dev] [PATCH] hash, jhash: Fix unaligned access to the hash remainder.

On 5/2/24 12:22, Ales Musil wrote: > The has was passed to memcpy as uin32_t *, however the hash bytes 'The has was passed' ? :) > might be unaligned at that point. Case it to uint8_t * instead > which has only single byte alignment requirement. > > lib/hash.c:46:22: runtime error: load of

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

On 1 May 2024, at 15:36, Finn, Emma wrote: >> -Original Message- >> From: Eelco Chaudron >> Sent: Wednesday, May 1, 2024 1:52 PM >> To: Simon Horman >> Cc: Finn, Emma ; Stokes, Ian ; >> sunil.pa...@intel.com; Van Haaren, Harry ; >> d...@openvswitch.org; Flavio Leitner ; Ilya Maximets

[ovs-dev] [PATCH] utilities: Correct deletion reason in flow_reval_monitor.py.

The flow_reval_monitor.py script incorrectly reported the reasons for FDR_PURGE and FDR_TOO_EXPENSIVE, as their descriptions were swapped. This patch rectifies the order. Fixes: 86b9e653ef22 ("revalidator: Add a USDT probe during flow deletion with purge reason.") Signed-off-by: Eelco Chaudron

Re: [ovs-dev] [PATCH ovn v3] controller: Track individual address set constants.

On Wed, May 1, 2024 at 6:38 PM Han Zhou wrote: > > > On Tue, Apr 30, 2024 at 9:56 AM Ales Musil wrote: > > > > Instead of tracking address set per struct expr_constant_set track it > > per individual struct expr_constant. This allows more fine grained > > control for I-P processing of address

[ovs-dev] [PATCH v2] hash, jhash: Fix unaligned access to the hash remainder.

The pointer was passed to memcpy as uin32_t *, however the hash bytes might be unaligned at that point. Case it to uint8_t * instead which has only single byte alignment requirement. This seems to be a false positive reported by clang [0]. lib/hash.c:46:22: runtime error: load of misaligned

[ovs-dev] [PATCH ovn v4] controller: Track individual address set constants.

Instead of tracking address set per struct expr_constant_set track it per individual struct expr_constant. This allows more fine grained control for I-P processing of address sets in controller. It helps with scenarios like matching on two address sets in one expression e.g. "ip4.src == {$as1,

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

> -Original Message- > From: Eelco Chaudron > Sent: Thursday, May 2, 2024 8:14 AM > To: Finn, Emma > Cc: Simon Horman ; Stokes, Ian ; > Van Haaren, Harry ; d...@openvswitch.org; > Flavio Leitner ; Ilya Maximets > Subject: Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header >

Re: [ovs-dev] [PATCH ovn v4 1/2] northd: Make `vxlan_mode` a global variable.

On Thu, May 2, 2024 at 5:51 AM Vladislav Odintsov wrote: > This simplifies code and subsequent commit to explicitely disable vxlan > I personally find it debatable that moving from explicit dependency through a function argument to implicit dependency through a global variable is a

Re: [ovs-dev] [PATCH ovn] northd: Fix the comment about route priorities.

On Mon, Apr 22, 2024 at 2:41 AM Han Zhou wrote: > > The current comments are obviously conflicting. Fixing it according the > current implementation - static route overrides src-ip route. > > Signed-off-by: Han Zhou Acked-by: Numan Siddique Numan > --- > northd/northd.c | 2 +- > 1 file

Re: [ovs-dev] [PATCH ovn] northd: Fix logical router load-balancer nat rules when using DGP.

On Fri, Mar 22, 2024 at 7:51 AM Roberto Bartzen Acosta wrote: > > Hi Mark, > > Thanks for your feedback. > > Em seg., 18 de mar. de 2024 às 11:53, Mark Michelson > escreveu: > > > Hi Roberto, > > > > I have some concerns about this patch. Let's use the test case you added > > as an example

Re: [ovs-dev] [PATCH ovn v4 1/2] northd: Make `vxlan_mode` a global variable.

Hi Ihar, thanks for your review! > On 2 May 2024, at 18:11, Ihar Hrachyshka wrote: > > On Thu, May 2, 2024 at 5:51 AM Vladislav Odintsov > wrote: > >> This simplifies code and subsequent commit to explicitely disable vxlan >> > > I personally find it debatable

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

On 2 May 2024, at 16:48, Finn, Emma wrote: >> -Original Message- >> From: Eelco Chaudron >> Sent: Thursday, May 2, 2024 8:14 AM >> To: Finn, Emma >> Cc: Simon Horman ; Stokes, Ian ; >> Van Haaren, Harry ; d...@openvswitch.org; >> Flavio Leitner ; Ilya Maximets >> Subject: Re:

Re: [ovs-dev] [PATCH ovn v4] controller: Track individual address set constants.

On Thu, May 2, 2024 at 6:29 AM Ales Musil wrote: > > Instead of tracking address set per struct expr_constant_set track it > per individual struct expr_constant. This allows more fine grained > control for I-P processing of address sets in controller. It helps with > scenarios like matching on

Re: [ovs-dev] [PATCH] netdev-tc-offloads: Don't offload header modification on ip fragments.

On 4/29/24 16:48, Eelco Chaudron wrote: > While offloading header modifications to TC, OVS is using {TCA_PEDIT} + > {TCA_CSUM} combination as that it the only way to represent header > rewrite. However, {TCA_CSUM} is unable to calculate L4 checksums for > IP fragments. > > Since TC already

[ovs-dev] [PATCH ovn] northd: Add bfd and bfd_consumer nodes to I-P engine.

Introduce bfd and bfd_consumer nodes to northd I-P engine to track bfd connections and northd static_route/policy_route changes. Reported-at: https://issues.redhat.com/browse/FDP-600 Signed-off-by: Lorenzo Bianconi --- northd/en-lflow.c| 19 +-- northd/en-northd.c | 92

Re: [ovs-dev] [PATCH ovn v4 2/2] northd: Add support for disabling vxlan mode.

This patch is the meat of the feature, and I think it could be as well implemented without patch 1/2 in the series. I agree with this particular patch, (assuming we adjust documentation to talk about `VXLAN mode`, capitalized). But I also have some reservations about patch 1/2 in the series (or at

Re: [ovs-dev] [PATCH v9 1/6] Add global option for JSON output to ovs-appctl.

On 4/12/24 09:26, jm...@redhat.com wrote: > From: Jakob Meng > > For monitoring systems such as Prometheus it would be beneficial if > OVS would expose statistics in a machine-readable format. > > This patch introduces support for different output formats to ovs-xxx > tools, in particular

Re: [ovs-dev] [PATCH v9 4/6] python: Add option '--pretty' for pretty-printing JSON output.

On 4/12/24 09:26, jm...@redhat.com wrote: > From: Jakob Meng > > With the '--pretty' option, appctl.py will now print JSON output in a > more readable fashion, i.e. with additional line breaks, spaces and > sorted dictionary keys. The pretty-printed output from appctl.py is not > strictly the

Re: [ovs-dev] [PATCH ovn] northd: Fix the comment about route priorities.

On Thu, May 2, 2024 at 9:01 AM Numan Siddique wrote: > > On Mon, Apr 22, 2024 at 2:41 AM Han Zhou wrote: > > > > The current comments are obviously conflicting. Fixing it according the > > current implementation - static route overrides src-ip route. > > > > Signed-off-by: Han Zhou > >

Re: [ovs-dev] [PATCH v9 3/6] appctl: Add option '--pretty' for pretty-printing JSON output.

On 4/12/24 09:26, jm...@redhat.com wrote: > From: Jakob Meng > > With the '--pretty' option, ovs-appctl will now print JSON output in a > more readable fashion, i.e. with additional line breaks, spaces and > sorted dictionary keys. > > Signed-off-by: Jakob Meng > --- > NEWS

[ovs-dev] [PATCH ovn v5] controller: Allow br-int connection via other methods.

The br-int connection is hardcoded to use unix socket, which requires for the socket to be visible for ovn-controller. This is achievable in container by mounting the socket, but in turn the container requires additional privileges. Add option to vswitchd external-ids that allows to specify

Re: [ovs-dev] [PATCH v9 5/6] vswitchd: Add JSON output for 'list-commands' command.

On 4/12/24 09:26, jm...@redhat.com wrote: > From: Jakob Meng > > The 'list-commands' command now supports machine-readable JSON output > in addition to the plain-text output for humans. > > Reported-at: https://bugzilla.redhat.com/1824861 > Signed-off-by: Jakob Meng > --- > NEWS

Re: [ovs-dev] [PATCH v2] hash, jhash: Fix unaligned access to the hash remainder.

On 5/2/24 15:28, Ales Musil wrote: > The pointer was passed to memcpy as uin32_t *, however the hash bytes > might be unaligned at that point. Case it to uint8_t * instead 'Case' ? > which has only single byte alignment requirement. This seems to be > a false positive reported by clang [0].

Re: [ovs-dev] [PATCH v9 2/6] python: Add global option for JSON output to Python tools.

On 4/12/24 09:26, jm...@redhat.com wrote: > From: Jakob Meng > > This patch introduces support for different output formats to the > Python code, as did the previous commit for ovs-xxx tools like > 'ovs-appctl --format json dpif/show'. > In particular, tests/appctl.py gains a global option

Re: [ovs-dev] [PATCH v3] docs: Document manual cluster recovery procedure.

On 4/26/24 18:54, Ihar Hrachyshka wrote: > Remove the notion of cluster/leave --force since it was never > implemented. Instead of these instructions, document how a broken > cluster can be re-initialized with the old database contents. > > Signed-off-by: Ihar Hrachyshka > > --- > > v1:

Re: [ovs-dev] [PATCH v3] docs: Document manual cluster recovery procedure.

On Thu, May 2, 2024 at 5:52 PM Ilya Maximets wrote: > On 4/26/24 18:54, Ihar Hrachyshka wrote: > > Remove the notion of cluster/leave --force since it was never > > implemented. Instead of these instructions, document how a broken > > cluster can be re-initialized with the old database contents.

[ovs-dev] [PATCH] ofproto-dpif-trace: Fix access to an out-of-scope stack memory.

While tracing NAT actions, pointer to the action may be stored in the recirculation node for future reference. However, while translating actions for the group bucket in xlate_group_bucket, the action list is allocated temporarily on stack. So, in case the group translation leads to NAT, the

Re: [ovs-dev] [PATCH] socket: Don't fail when AF_UNIX connect() returns EAGAIN.

On 4/27/24 00:42, Ihar Hrachyshka wrote: > POSIX defines EINPROGRESS as the return value for non-blocking connect() > [1]. But in Linux, AF_UNIX connect() returns EAGAIN instead of > EINPROGRESS. (but only for AF_UNIX sockets!) [2] > > Both cases should be handled the same way - by returning the

[ovs-dev] [PATCH v3] hash, jhash: Fix unaligned access to the hash remainder.

Partially revert db5a101931c5, this was to avoid warning, however we shouldn't use pointer to "uint32_t" when the data are potentially unaligned [0]. Use pointer to "uint8_t" right from the start, this requires us to use ALIGNED_CAST for the get_unaligned_u32, which is fine in that case, because

Re: [ovs-dev] [PATCH ovn] docs: Explain nature of ovs dependency.

On Fri, Apr 26, 2024 at 10:35 PM Ihar Hrachyshka wrote: > The dependency is during build time, not runtime. > > Signed-off-by: Ihar Hrachyshka > --- > Documentation/intro/install/ovn-upgrades.rst | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git

Re: [ovs-dev] [PATCH ovn] docs: List supported rolling upgrade paths.

On Fri, Apr 26, 2024 at 10:49 PM Ihar Hrachyshka wrote: > The wording above is not completely clear without these scenarios > listed. A confused reader may incorrectly read it as: > > ``` > Only LTS-to-LTS is supported for rolling upgrades. > ``` > > which is wrong. > > Signed-off-by: Ihar

Re: [ovs-dev] [PATCH ovn v4] controller: Track individual address set constants.

On Thu, May 2, 2024 at 6:23 PM Han Zhou wrote: > > > > On Thu, May 2, 2024 at 6:29 AM Ales Musil wrote: > > > > Instead of tracking address set per struct expr_constant_set track it > > per individual struct expr_constant. This allows more fine grained > > control for I-P processing of address

Re: [ovs-dev] [PATCH v2] hash, jhash: Fix unaligned access to the hash remainder.

On Thu, May 2, 2024 at 8:03 PM Ilya Maximets wrote: > On 5/2/24 15:28, Ales Musil wrote: > > The pointer was passed to memcpy as uin32_t *, however the hash bytes > > might be unaligned at that point. Case it to uint8_t * instead > > 'Case' ? > > > which has only single byte alignment