Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Eelco Chaudron]

On 2 Feb 2024, at 11:31, Adrian Moreno wrote:

> On 2/1/24 10:02, Eelco Chaudron wrote:
>>
>>
>> On 31 Jan 2024, at 18:03, Aaron Conole wrote:
>>
>>> Eelco Chaudron  writes:
>>>
 On 25 Jan 2024, at 21:55, Aaron Conole wrote:

> From: Kevin Sprague 
>
> During normal operations, it is useful to understand when a particular 
> flow
> gets removed from the system. This can be useful when debugging 
> performance
> issues tied to ofproto flow changes, trying to determine deployed traffic
> patterns, or while debugging dynamic systems where ports come and go.
>
> Prior to this change, there was a lack of visibility around flow 
> expiration.
> The existing debugging infrastructure could tell us when a flow was added 
> to
> the datapath, but not when it was removed or why.
>
> This change introduces a USDT probe at the point where the revalidator
> determines that the flow should be removed.  Additionally, we track the
> reason for the flow eviction and provide that information as well.  With
> this change, we can track the complete flow lifecycle for the netlink
> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, 
> and
> the revaldiator USDT, letting us watch as flows are added and removed from
> the kernel datapath.
>
> This change only enables this information via USDT probe, so it won't be
> possible to access this information any other way (see:
> Documentation/topics/usdt-probes.rst).
>
> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
> which serves as a demonstration of how the new USDT probe might be used
> going forward.
>
> Signed-off-by: Kevin Sprague 
> Co-authored-by: Aaron Conole 
> Signed-off-by: Aaron Conole 

 Thanks for following this up Aaron! See comments on this patch below. I 
 have no additional comments on patch 2.

 Cheers,

 Eelco


> ---
>   Documentation/topics/usdt-probes.rst |   1 +
>   ofproto/ofproto-dpif-upcall.c|  42 +-
>   utilities/automake.mk|   3 +
>   utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>   4 files changed, 693 insertions(+), 6 deletions(-)
>   create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>
> diff --git a/Documentation/topics/usdt-probes.rst 
> b/Documentation/topics/usdt-probes.rst
> index e527f43bab..a8da9bb1f7 100644
> --- a/Documentation/topics/usdt-probes.rst
> +++ b/Documentation/topics/usdt-probes.rst
> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>   - dpif_recv:recv_upcall
>   - main:poll_block
>   - main:run_start
> +- revalidate:flow_result
>   - revalidate_ukey\_\_:entry
>   - revalidate_ukey\_\_:exit
>   - udpif_revalidator:start_dump

 You are missing the specific flow_result result section. This is from the 
 previous patch:
>>>
>>> D'oh!  Thanks for catching it.  I'll re-add it.
>>>
 @@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
   - ``utilities/usdt-scripts/bridge_loop.bt``


 +probe revalidate:flow_result
 +~
 +
 +**Description**:
 +This probe is triggered when the revalidator decides whether or not to
 +revalidate a flow. ``reason`` is an enum that denotes that either the flow
 +is being kept, or the reason why the flow is being deleted. The
 +``flow_reval_monitor.py`` script uses this probe to notify users when 
 flows
 +matching user-provided criteria are deleted.
 +
 +**Arguments**:
 +
 +- *arg0*: ``(enum flow_del_reason) reason``
 +- *arg1*: ``(struct udpif *) udpif``
 +- *arg2*: ``(struct udpif_key *) ukey``
 +
 +**Script references**:
 +
 +- ``utilities/usdt-scripts/flow_reval_monitor.py``
 +
 +
   Adding your own probes
   --

> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index b5cbeed878..97d75833f7 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -269,6 +269,18 @@ enum ukey_state {
>   };
>   #define N_UKEY_STATES (UKEY_DELETED + 1)
>
> +enum flow_del_reason {
> +FDR_REVALIDATE = 0, /* The flow was revalidated. */

 It was called FDR_FLOW_LIVE before, which might make more sense. As the 
 flow is just NOT deleted. It might or might not have been revalidated. 
 Thoughts?
>>>
>>> I think it had to have been revalidated if we emit the reason, because
>>> we only emit the reason code after revalidation.  IE: there are many
>>> places where we skip revalidation but the flow stays live - and we don't
>>> emit reasons in those cases.
>>>
>>> So at least for this patch, it MUST have been revalidated. 

Re: [ovs-dev] [PATCH net-next] selftests: openvswitch: Test ICMP related matches work with SNAT

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller :

On Wed, 31 Jan 2024 17:08:22 +1300 you wrote:
> Add a test case for regression in openvswitch nat that was fixed by
> commit e6345d2824a3 ("netfilter: nf_nat: fix action not being set for
> all ct states").
> 
> Link: https://lore.kernel.org/netdev/20231221224311.130319-1-b...@faucet.nz/
> Link: https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410476.html
> Suggested-by: Aaron Conole 
> Signed-off-by: Brad Cowie 
> 
> [...]

Here is the summary with links:
  - [net-next] selftests: openvswitch: Test ICMP related matches work with SNAT
https://git.kernel.org/netdev/net-next/c/094bdd48afb8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 03/10] python: ovs: flowviz: Add console formatting.

[Adrian Moreno]

On 1/30/24 16:49, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


Add a flow formatting framework and one implementation for console
printing using rich.

The flow formatting framework is a simple set of classes that can be
used to write different flow formatting implementations. It supports
styles to be described by any class, highlighting and config-file based
style definition.

The first flow formatting implementation is also introduced: the
ConsoleFormatter. It uses the an advanced rich-text printing library
[1].

The console printing supports:
- Heatmap: printing the packet/byte statistics of each flow in a color
   that represents its relative size: blue (low) -> red (high).
- Printing a banner with the file name and alias.
- Extensive style definition via config file.

This console format is added to both OpenFlow and Datapath flows.

Examples:
- Highlight drops in datapath flows:
$ ovs-flowviz -i flows.txt --highlight "drop" datapath console
- Quickly detect where most packets are going using heatmap and
   paginated output:
$ ovs-ofctl dump-flows br-int | ovs-flowviz -p openflow console -h


-p is not supported.



Good catch!
The original tool had a "--page|-p" option that paged the output, but I decided 
to remove it because it added very little value and it required users to edit 
the pager to visualize colors (e.g: adding "-R" to "less") anyway.




[1] https://rich.readthedocs.io/en/stable/introduction.html

Signed-off-by: Adrian Moreno 
---


When trying to install this using the python setup, I get the following error:

   running install_data
   warning: install_data: setup script did not provide a directory for 
'ovs/flowviz/ovs-flowviz.conf' -- installing right in 
'/home/echaudron/Documents/review/ovs_adrian_tools/python-venv'

   error: can't copy 'ovs/flowviz/ovs-flowviz.conf': doesn't exist or not a 
regular file
   [end of output]

Guess you need to move this file from patch 4 to 3.



Right! Thanks.


Also, note that rich is not installed as a dependency.



It should if you run "pip install .[flowviz]".


Below are some small nits, but the code looks good.

Cheers,

Eelco


  python/automake.mk|   2 +
  python/ovs/flowviz/console.py | 174 
  python/ovs/flowviz/format.py  | 372 ++
  python/ovs/flowviz/main.py|  57 +-
  python/ovs/flowviz/odp/cli.py |  25 +++
  python/ovs/flowviz/ofp/cli.py |  26 +++
  python/ovs/flowviz/process.py |  87 +++-
  python/setup.py   |   5 +-
  8 files changed, 738 insertions(+), 10 deletions(-)
  create mode 100644 python/ovs/flowviz/console.py
  create mode 100644 python/ovs/flowviz/format.py

diff --git a/python/automake.mk b/python/automake.mk
index 4845565b8..8d82cb4a8 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -65,6 +65,8 @@ ovs_pytests = \

  ovs_flowviz = \
python/ovs/flowviz/__init__.py \
+   python/ovs/flowviz/console.py \
+   python/ovs/flowviz/format.py \
python/ovs/flowviz/main.py \
python/ovs/flowviz/odp/__init__.py \
python/ovs/flowviz/odp/cli.py \
diff --git a/python/ovs/flowviz/console.py b/python/ovs/flowviz/console.py
new file mode 100644
index 0..5b4b047c2
--- /dev/null
+++ b/python/ovs/flowviz/console.py
@@ -0,0 +1,174 @@
+# Copyright (c) 2023 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import colorsys


In other files you have a cr/lf between import and from.



Yep, thanks!


+from rich.console import Console
+from rich.text import Text
+from rich.style import Style
+from rich.color import Color
+from rich.panel import Panel
+from rich.emoji import Emoji
+
+from ovs.flowviz.format import FlowFormatter, FlowBuffer, FlowStyle
+
+
+def file_header(name):
+return Panel(
+Text(
+Emoji.replace(":scroll:")
++ " "
++ name
++ " "
++ Emoji.replace(":scroll:"),
+style="bold",
+justify="center",
+)
+)
+
+
+class ConsoleBuffer(FlowBuffer):
+"""ConsoleBuffer implements FlowBuffer to provide console-based text
+formatting based on rich.Text.
+
+Append functions accept a rich.Style.
+
+Args:
+rtext(rich.Text): Optional; text instance to reuse
+"""
+
+def __init__(self, rtext):
+self._text = rtext or Text()
+
+@property
+def text(self):
+return self._text
+
+def 

Re: [ovs-dev] [RFC PATCH 04/10] python: ovs: flowviz: Add default config file.

[Adrian Moreno]

On 1/30/24 16:50, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


It has two basic styles defined: "dark" and "light" intended for
dark and light terminals.


This patch looks fine to me. Should we maybe add an example in the commit 
message on how to use the --style option?



Sure!



Cheers,

Eelco


Signed-off-by: Adrian Moreno 


Acked-by: Eelco Chaudron 



--
Adrián Moreno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 00/10] Add flow visualization utility

[Adrian Moreno]

On 1/30/24 16:45, Eelco Chaudron wrote:



On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


This series introduces a python utility called ovs-flowviz.

The goal of this utility is to read both datapath and Openflow flows
(using the flow library already available) and print them in different
formats and styles to make it easier to understand them and troubleshoot
issues.

The formats are quite opinionated and so are the colors chosen so I'm
eager to hear what is the impression caused to the community.

Here is a summary of the formats and features supported:

- Openflow
- Console: Prints flows back to the console allowing filtering and
  extensive formatting.
- Cookie: Arranges flows based on cookie (instead of table) to ease
  visualization of OVN-generated flows.
- HTML: Prints an HTML file that shows the flows arranged by tables.
  resubmit actions have a hyperlinke to the target table to ease
  navegation.
- Logic: Many times OVN generates many "logically-equivalent" flows.
  These are flows that have the same structure: match on the same
  values and have the same actions. The only thing that changes is
  the actual value they match against and maybe the arguments of the
  actions. This format collapses these flows so you can visualize the
  logical pipeline easier.
- JSON: JSON format.

More OpenFlow features:
- OVN metadata: Both the "cookie" and the "logic" format allow to
  connect to a running OVN NB/SB databases and enrich the flows with
  OVN context based on the flow's cookie.

- Datapath:
- Console: Prints flows back to the console allowing filtering and
  extensive formatting.
- Tree: Datapath flows use recirculation to further execute
  additional actions. This format builds a tree of flows following
  the recirculation identifiers and prints it in the console.
- HTML: Prints the datapath flow tree in HTML. It includes some
  minimal JS to support expanding and collapsing of entire branches.
- Graph: Following the "tree" format, this one prints the tree in
  graphviz format.
- JSON: JSON format.

Additional datapath features:
- Many datapath formats are based on the tree flow hierarchy. An
  interesting feature of this structure is that filtering is done at
  the branch level. This means that when a flow satisfies the filter,
  the entire sub-tree leading to that flow is shown.

Additional common features:
- Styling: Styles for both console and HTML formats can be defined
  using a configuration file.
- Heat maps: Some formats support heat-maps. A color pallete ranging
  from blue (cold) to red (hot) is used to print the number of
  packets and bytes of the flows. That way, the flows that are
  handling more traffic are much easier to spot

I know this is a long series, so it's OK if it takes time to review. I'm
specially interested to know what is found useful and what is worth
removing (if any).


Thanks Adrian, for submitting the series and I think the quality of the code is 
good enough to submit it as a non-RFC. Most of the comments I have are style 
nits.

The only main thing missing is some documentation/man pages for the tool.



Thanks Eelco for the review.


I’ll reply to the individual patches with comments.

Cheers,

Eelco



--
Adrián Moreno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovn-sb.xml: Remove IPv4-only restriction from Service Monitors.

[Ales Musil]

On Fri, Feb 2, 2024 at 4:21 AM Mark Michelson  wrote:

> The documentation for Service_Monitors in the southbound database state
> that only IPv4 is supported. However, IPv6 service monitors have been
> supported since OVN 23.03 was released.
>
> This patch addresses the problem by removing the incorrect
> documentation.
>
> Fixes: 40a686e8e70f ("Add IPv6 support for lb health-check")
> Signed-off-by: Mark Michelson 
> ---
>  ovn-sb.xml | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index e393f92b3..95733f99b 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -4774,8 +4774,7 @@ tcp.flags = RST;
>Each row in this table configures monitoring a service for its
> liveness.
>The service can be an IPv4 TCP or UDP
>service. ovn-controller periodically sends out service
> -  monitor packets and updates the status of the service. Service
> monitoring
> -  for IPv6 services is not supported.
> +  monitor packets and updates the status of the service.
>  
>
>  
> @@ -4812,7 +4811,7 @@ tcp.flags = RST;
>
>
>
> -Source IPv4 address to use in the service monitor packet.
> +Source IPv4 or IPv6 address to use in the service monitor packet.
>
>
>
> --
> 2.43.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>

Looks good to me, thanks.

Acked-by: Ales Musil 


-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.com

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).

[Ales Musil]

On Tue, Jan 30, 2024 at 1:07 PM Ilya Maximets  wrote:

> checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using
> outdated Node.js 16 which is now deprecated in GHA [1], so these
> actions may stop working soon.
>
> Updating to most recent major versions with Node.js 20.  This stops
> GHA from throwing warnings in every build.
>
> [1]
> https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
>
> While at it also updating upload-artifact and download-artifact to
> the latest versions.
>
> Removing versions from the upload-artifact comment, since the
> behavior doesn't seem to change much between versions.
>
> New setup-go@v5 attempts to cache dependencies by default.  However,
> the default path it uses is go.sum in the root directory.  This
> triggers a warning, since the file doesn't exist:
>
>   Restore cache failed: Dependencies file is not found in
>   /home/runner/work/ovn-kubernetes/ovn-kubernetes.
>   Supported file pattern: go.sum
>
> Specify a path to all .sum files we have in the repository to make
> the setup-go happy.  This should in theory make the builds a touch
> faster.  This change is in line with recent changes in ovn-kubernetes
> itself.
>
> Signed-off-by: Ilya Maximets 
> ---
>  .github/workflows/containers.yml  |  2 +-
>  .../workflows/ovn-fake-multinode-tests.yml| 26 +++---
>  .github/workflows/ovn-kubernetes.yml  | 19 +-
>  .github/workflows/test.yml| 36 +--
>  4 files changed, 42 insertions(+), 41 deletions(-)
>
> diff --git a/.github/workflows/containers.yml
> b/.github/workflows/containers.yml
> index bdd118087..87e28d645 100644
> --- a/.github/workflows/containers.yml
> +++ b/.github/workflows/containers.yml
> @@ -20,7 +20,7 @@ jobs:
>matrix:
>  distro: [ fedora, ubuntu ]
>  steps:
> -  - uses: actions/checkout@v3
> +  - uses: actions/checkout@v4
>
>- name: Update APT cache
>  run: sudo apt update
> diff --git a/.github/workflows/ovn-fake-multinode-tests.yml
> b/.github/workflows/ovn-fake-multinode-tests.yml
> index b3ba82a30..179c1d662 100644
> --- a/.github/workflows/ovn-fake-multinode-tests.yml
> +++ b/.github/workflows/ovn-fake-multinode-tests.yml
> @@ -26,7 +26,7 @@ jobs:
>XDG_RUNTIME_DIR: ''
>  steps:
>  - name: Check out ovn-fake-multi-node
> -  uses: actions/checkout@v3
> +  uses: actions/checkout@v4
>with:
>  repository: 'ovn-org/ovn-fake-multinode'
>  path: 'ovn-fake-multinode'
> @@ -36,14 +36,14 @@ jobs:
>  # ovn-fake-multinode builds and installs ovs from
> ovn-fake-multinode/ovs
>  # and it builds and installs ovn from ovn-fake-multinode/ovn. It uses
> the ovs submodule for ovn compilation.
>  - name: Check out ovs master
> -  uses: actions/checkout@v3
> +  uses: actions/checkout@v4
>with:
>  path: 'ovn-fake-multinode/ovs'
>  repository: 'openvswitch/ovs'
>  ref: 'master'
>
>  - name: Check out ovn ${{ matrix.cfg.branch }}
> -  uses: actions/checkout@v3
> +  uses: actions/checkout@v4
>with:
>  path: 'ovn-fake-multinode/ovn'
>  submodules: recursive
> @@ -63,7 +63,7 @@ jobs:
>  sudo podman save ovn/ovn-multi-node:${{ matrix.cfg.branch }} >
> /tmp/_output/ovn_${{ matrix.cfg.branch }}_image.tar
>working-directory: ovn-fake-multinode
>
> -- uses: actions/upload-artifact@v3
> +- uses: actions/upload-artifact@v4
>with:
>  name: test-${{ matrix.cfg.branch }}-image
>  path: /tmp/_output/ovn_${{ matrix.cfg.branch }}_image.tar
> @@ -100,7 +100,7 @@ jobs:
>
>  steps:
>  - name: Check out ovn
> -  uses: actions/checkout@v3
> +  uses: actions/checkout@v4
>
>  - name: install required dependencies
>run:  |
> @@ -112,11 +112,11 @@ jobs:
>  . .ci/linux-util.sh
>  free_up_disk_space_ubuntu
>
> -- uses: actions/download-artifact@v3
> +- uses: actions/download-artifact@v4
>with:
>  name: test-main-image
>
> -- uses: actions/download-artifact@v3
> +- uses: actions/download-artifact@v4
>with:
>  name: test-branch-22.03-image
>
> @@ -126,7 +126,7 @@ jobs:
>  sudo podman load --input ovn_branch-22.03_image.tar
>
>  - name: Check out ovn-fake-multi-node
> -  uses: actions/checkout@v3
> +  uses: actions/checkout@v4
>with:
>  repository: 'ovn-org/ovn-fake-multinode'
>  path: 'ovn-fake-multinode'
> @@ -156,12 +156,12 @@ jobs:
>  echo "$HOME/.local/bin" >> $GITHUB_PATH
>
>  - name: set up python
> -  uses: actions/setup-python@v4
> +  uses: actions/setup-python@v5
>with:
>  python-version: '3.12'
>
>  - name: Check out ovn
> -  uses: actions/checkout@v3
> +  uses: actions/checkout@v4
>with:
>  path: 'ovn'
>  submodules: recursive
> @@ -190,9 

Re: [ovs-dev] [RFC PATCH 01/10] python: ovs: Add flowviz scheleton.

[Adrian Moreno]

On 1/30/24 16:46, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


Add a new python package (just the scheleton for now) to hold a flow
visualization tool based on the flow parsing library.


Thanks for this series, and sorry for the late review :(



No worries! Thanks for the review, it's a big series!


Maybe I'm doing something wrong, but doing 'the pip install .' in the python 
directory does not complain, or install the click dependency.

   (python-venv) [ebuild:~/..._adrian_tools/python]$ ovs-flowviz
   Traceback (most recent call last):
 File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/bin/ovs-flowviz", line 
17, in 
   from ovs.flowviz import main
 File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/main.py",
 line 15, in 
   import click
   ModuleNotFoundError: No module named 'click'

The same for netaddr/pyparsing needed by the flow library.



The requirements for both flowviz and flow parsing library are specified in 
extras_dependencies (see setup.py). To install them you need to explicitly 
request them:


pip install .[flowviz]



In addition, two small style comments below.


Signed-off-by: Adrian Moreno 
---
  python/automake.mk | 12 +++--
  python/ovs/flowviz/__init__.py |  0
  python/ovs/flowviz/main.py | 41 ++
  python/ovs/flowviz/odp/__init__.py |  0
  python/ovs/flowviz/ofp/__init__.py |  0
  python/ovs/flowviz/ovs-flowviz | 20 +++
  python/setup.py| 11 +---
  7 files changed, 79 insertions(+), 5 deletions(-)
  create mode 100644 python/ovs/flowviz/__init__.py
  create mode 100644 python/ovs/flowviz/main.py
  create mode 100644 python/ovs/flowviz/odp/__init__.py
  create mode 100644 python/ovs/flowviz/ofp/__init__.py
  create mode 100755 python/ovs/flowviz/ovs-flowviz

diff --git a/python/automake.mk b/python/automake.mk
index 84cf2eab5..4302f0136 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -63,6 +63,14 @@ ovs_pytests = \
python/ovs/tests/test_odp.py \
python/ovs/tests/test_ofp.py

+ovs_flowviz = \
+   python/ovs/flowviz/__init__.py \
+   python/ovs/flowviz/main.py \
+   python/ovs/flowviz/odp/__init__.py \
+   python/ovs/flowviz/ofp/__init__.py \
+   python/ovs/flowviz/ovs-flowviz
+
+
  # These python files are used at build time but not runtime,
  # so they are not installed.
  EXTRA_DIST += \
@@ -81,7 +89,7 @@ EXTRA_DIST += \
  # C extension support.
  EXTRA_DIST += python/ovs/_json.c

-PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests)
+PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests) 
$(ovs_flowviz)

  EXTRA_DIST += $(PYFILES)
  PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
@@ -95,7 +103,7 @@ FLAKE8_PYFILES += \
python/ovs/dirs.py.template \
python/setup.py

-nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles)
+nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles) $(ovs_flowviz)
  ovs-install-data-local:
$(MKDIR_P) python/ovs
sed \
diff --git a/python/ovs/flowviz/__init__.py b/python/ovs/flowviz/__init__.py
new file mode 100644
index 0..e69de29bb
diff --git a/python/ovs/flowviz/main.py b/python/ovs/flowviz/main.py
new file mode 100644
index 0..a2d5ca1fa
--- /dev/null
+++ b/python/ovs/flowviz/main.py
@@ -0,0 +1,41 @@
+# Copyright (c) 2023 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import click
+
+
+class Options(dict):
+"""Options dictionary"""
+
+
+@click.group(
+subcommand_metavar="TYPE",
+context_settings=dict(help_option_names=["-h", "--help"]),
+)
+@click.pass_context
+def maincli(ctx):
+"""
+OpenvSwitch flow visualization utility.
+
+It reads openflow and datapath flows
+(such as the output of ovs-ofctl dump-flows or ovs-appctl dpctl/dump-flows)
+and prints them in different formats.
+"""
+
+
+def main():
+"""
+Main Function
+"""
+maincli()
diff --git a/python/ovs/flowviz/odp/__init__.py 
b/python/ovs/flowviz/odp/__init__.py
new file mode 100644
index 0..e69de29bb
diff --git a/python/ovs/flowviz/ofp/__init__.py 
b/python/ovs/flowviz/ofp/__init__.py
new file mode 100644
index 0..e69de29bb
diff --git a/python/ovs/flowviz/ovs-flowviz b/python/ovs/flowviz/ovs-flowviz
new file mode 100755
index 

Re: [ovs-dev] [PATCH ovn v2] OVN-SB: Exposes igmp group protocol version through IGMP table.

[Ales Musil]

On Wed, Jan 31, 2024 at 1:14 PM Mohammad Heib  wrote:

> Expose the igmp/mld group protocol version through the
> IGMP_GROUP table in SBDB.
>
> This patch can be used by ovn consumer for debuggability purposes, user
> now can  match between the protocol version used in the OVN logical
> switches and the uplink ports.
>
> Rreported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2160476
> Signed-off-by
> :
> Mohammad Heib 
> ---
>

Hi Mohammad.
thank you for the patch, I have two small comments down below.


>  NEWS  |  2 ++
>  controller/ip-mcast.c | 10 --
>  controller/ip-mcast.h |  5 +++--
>  controller/pinctrl.c  | 28 
>  northd/ovn-northd.c   |  2 +-
>  ovn-sb.ovsschema  |  5 +++--
>  ovn-sb.xml|  4 
>  tests/ovn.at  |  3 +++
>  8 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 6553bd078..6505ef22b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -18,6 +18,8 @@ Post v23.09.0
>- Support selecting encapsulation IP based on the source/destination
> VIF's
>  settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
>  details.
> +  - IGMP_Group has new "protocol" column that displays the the group
> +protocol version.
>
>  OVN v23.09.0 - 15 Sep 2023
>  --
> diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
> index a870fb29e..18a3e1fc2 100644
> --- a/controller/ip-mcast.c
> +++ b/controller/ip-mcast.c
> @@ -107,11 +107,12 @@ igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn,
>  }
>
>  void
> -igmp_group_update_ports(const struct sbrec_igmp_group *g,
> +igmp_group_update(const struct sbrec_igmp_group *g,
>  struct ovsdb_idl_index *datapaths,
>  struct ovsdb_idl_index *port_bindings,
>  const struct mcast_snooping *ms OVS_UNUSED,
> -const struct mcast_group *mc_group)
> +const struct mcast_group *mc_group,
> +const char *protocol)
>  OVS_REQ_RDLOCK(ms->rwlock)
>  {
>  struct igmp_group_port *old_ports_storage =
> @@ -151,6 +152,11 @@ igmp_group_update_ports(const struct sbrec_igmp_group
> *g,
>  sbrec_igmp_group_update_ports_delvalue(g, igmp_port->port);
>  }
>
> +/* set Group protocol */
> +if (protocol) {
> + sbrec_igmp_group_set_protocol(g, protocol);
> +}
> +
>  free(old_ports_storage);
>  hmap_destroy(_ports);
>  }
> diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h
> index 326f39db1..2a8921976 100644
> --- a/controller/ip-mcast.h
> +++ b/controller/ip-mcast.h
> @@ -45,11 +45,12 @@ struct sbrec_igmp_group *igmp_mrouter_create(
>  const struct sbrec_datapath_binding *datapath,
>  const struct sbrec_chassis *chassis);
>
> -void igmp_group_update_ports(const struct sbrec_igmp_group *g,
> +void igmp_group_update(const struct sbrec_igmp_group *g,
>   struct ovsdb_idl_index *datapaths,
>   struct ovsdb_idl_index *port_bindings,
>   const struct mcast_snooping *ms,
> - const struct mcast_group *mc_group)
> + const struct mcast_group *mc_group,
> + const char *protocol)
>  OVS_REQ_RDLOCK(ms->rwlock);
>  void
>  igmp_mrouter_update_ports(const struct sbrec_igmp_group *g,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index bd3bd3d81..f3597b489 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -180,6 +180,7 @@ struct pinctrl {
>  bool mac_binding_can_timestamp;
>  bool fdb_can_timestamp;
>  bool dns_supports_ovn_owned;
> +bool igmp_support_protocol;
>  };
>
>  static struct pinctrl pinctrl;
> @@ -3586,11 +3587,21 @@ pinctrl_update(const struct ovsdb_idl *idl, const
> char *br_int_name)
>  if (dns_supports_ovn_owned != pinctrl.dns_supports_ovn_owned) {
>  pinctrl.dns_supports_ovn_owned = dns_supports_ovn_owned;
>
> -/* Notify pinctrl_handler that fdb timestamp column
> +/* Notify pinctrl_handler that dns ovn_owned column
> * availability has changed. */
>

This change is nice, but not really related to the commit itself. I should
be separate.


>  notify_pinctrl_handler();
>  }
>
> +bool igmp_support_proto =
> +sbrec_server_has_igmp_group_table_col_protocol(idl);
> +if (igmp_support_proto != pinctrl.igmp_support_protocol) {
> +pinctrl.igmp_support_protocol = igmp_support_proto;
> +
> +/* Notify pinctrl_handler that igmp protocol column
> + * availability has changed. */
> +notify_pinctrl_handler();
> +}
> +
>  ovs_mutex_unlock(_mutex);
>  }
>
> @@ -5400,9 +5411,18 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn,
>

Re: [ovs-dev] [RFC PATCH 09/10] python: ovs: flowviz: Add datapath html format.

[Adrian Moreno]

On 1/30/24 16:56, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


Using the existing FlowTree and HTMLFormatter, create an HTML tree
visualization that also supports collapsing and expanding entire flow
trees and subtrees.


This looks good to me, maybe add an example in the commit message.



Sure, thanks.


Acked-by: Eelco Chaudron 


Signed-off-by: Adrian Moreno 
---




--
Adrián Moreno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Adrian Moreno]

On 2/1/24 10:02, Eelco Chaudron wrote:



On 31 Jan 2024, at 18:03, Aaron Conole wrote:


Eelco Chaudron  writes:


On 25 Jan 2024, at 21:55, Aaron Conole wrote:


From: Kevin Sprague 

During normal operations, it is useful to understand when a particular flow
gets removed from the system. This can be useful when debugging performance
issues tied to ofproto flow changes, trying to determine deployed traffic
patterns, or while debugging dynamic systems where ports come and go.

Prior to this change, there was a lack of visibility around flow expiration.
The existing debugging infrastructure could tell us when a flow was added to
the datapath, but not when it was removed or why.

This change introduces a USDT probe at the point where the revalidator
determines that the flow should be removed.  Additionally, we track the
reason for the flow eviction and provide that information as well.  With
this change, we can track the complete flow lifecycle for the netlink
datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
the revaldiator USDT, letting us watch as flows are added and removed from
the kernel datapath.

This change only enables this information via USDT probe, so it won't be
possible to access this information any other way (see:
Documentation/topics/usdt-probes.rst).

Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
which serves as a demonstration of how the new USDT probe might be used
going forward.

Signed-off-by: Kevin Sprague 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 


Thanks for following this up Aaron! See comments on this patch below. I have no 
additional comments on patch 2.

Cheers,

Eelco



---
  Documentation/topics/usdt-probes.rst |   1 +
  ofproto/ofproto-dpif-upcall.c|  42 +-
  utilities/automake.mk|   3 +
  utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
  4 files changed, 693 insertions(+), 6 deletions(-)
  create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py

diff --git a/Documentation/topics/usdt-probes.rst 
b/Documentation/topics/usdt-probes.rst
index e527f43bab..a8da9bb1f7 100644
--- a/Documentation/topics/usdt-probes.rst
+++ b/Documentation/topics/usdt-probes.rst
@@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
  - dpif_recv:recv_upcall
  - main:poll_block
  - main:run_start
+- revalidate:flow_result
  - revalidate_ukey\_\_:entry
  - revalidate_ukey\_\_:exit
  - udpif_revalidator:start_dump


You are missing the specific flow_result result section. This is from the 
previous patch:


D'oh!  Thanks for catching it.  I'll re-add it.


@@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
  - ``utilities/usdt-scripts/bridge_loop.bt``


+probe revalidate:flow_result
+~
+
+**Description**:
+This probe is triggered when the revalidator decides whether or not to
+revalidate a flow. ``reason`` is an enum that denotes that either the flow
+is being kept, or the reason why the flow is being deleted. The
+``flow_reval_monitor.py`` script uses this probe to notify users when flows
+matching user-provided criteria are deleted.
+
+**Arguments**:
+
+- *arg0*: ``(enum flow_del_reason) reason``
+- *arg1*: ``(struct udpif *) udpif``
+- *arg2*: ``(struct udpif_key *) ukey``
+
+**Script references**:
+
+- ``utilities/usdt-scripts/flow_reval_monitor.py``
+
+
  Adding your own probes
  --


diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index b5cbeed878..97d75833f7 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -269,6 +269,18 @@ enum ukey_state {
  };
  #define N_UKEY_STATES (UKEY_DELETED + 1)

+enum flow_del_reason {
+FDR_REVALIDATE = 0, /* The flow was revalidated. */


It was called FDR_FLOW_LIVE before, which might make more sense. As the flow is 
just NOT deleted. It might or might not have been revalidated. Thoughts?


I think it had to have been revalidated if we emit the reason, because
we only emit the reason code after revalidation.  IE: there are many
places where we skip revalidation but the flow stays live - and we don't
emit reasons in those cases.

So at least for this patch, it MUST have been revalidated.  But maybe in
the future, we would want to catch cases where the flow hasn't been.  In
that case, it makes sense to add the FDR_FLOW_LIVE at that time - I
think.

Maybe you disagree?


Well, it depends on how you define revalidation, it might only have updated the 
counters. i.e. it all depends on ‘bool need_revalidate = ukey->reval_seq != 
reval_seq;’ in revalidate_ukey(). That was why I opted for a more general name.


+FDR_FLOW_IDLE,  /* The flow went unused and was deleted. */
+FDR_TOO_EXPENSIVE,  /* The flow was too expensive to revalidate. */
+FDR_FLOW_WILDCARDED,/* The flow needed a narrower wildcard mask. */
+FDR_BAD_ODP_FIT,/* The flow had a bad ODP 

Re: [ovs-dev] [PATCH v2] netlink-conntrack: Optimize flushing ct zone.

[Ilya Maximets]

On 2/2/24 10:32, Felix Huettner via dev wrote:
> Previously the kernel did not provide a netlink interface to flush/list
> only conntrack entries matching a specific zone. With [1] it is now
> possible to flush and list conntrack entries filtered by zone. Older
> kernels not yet supporting this feature will ignore the filter.
> For the list request that means just returning all entries (which we can
> then filter in userspace as before).
> For the flush request that means deleting all conntrack entries.
> 
> These significantly improves the performance of flushing conntrack zones
> when the conntrack table is large. Since flushing a conntrack zone is
> normally triggered via an openflow command it blocks the main ovs thread
> and thereby also blocks new flows from being applied. Using this new
> feature we can reduce the flushing time for zones by around 93%.
> 
> In combination with OVN the creation of a Logical_Router (which causes
> the flushing of a ct zone) could block other operations, e.g. the
> failover of Logical_Routers (as they cause new flows to be created).
> This is visible from a user perspective as a ovn-controller that is idle
> (as it waits for vswitchd) and vswitchd reporting:
> "blocked 1000 ms waiting for main to quiesce" (potentially with ever
> increasing times).
> 
> The following performance tests where run in a qemu vm with 500.000
> conntrack entries distributed evenly over 500 ct zones using `ovstest
> test-netlink-conntrack flush zone=`.
> 
> With this patch and kernel v6.8-rc2:
> 



> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index f363a778c..869728a1d 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -3214,6 +3214,14 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | 
> FORMAT_CT(10.1.1.4)], [0], [dnl
>  
> tcp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=,dport=),reply=(src=10.1.1.4,dst=10.1.1.3,sport=,dport=),zone=2,protoinfo=(state=)
>  ])
>  
> +dnl flushing one zone should leave the others intact

Hi, Felix.  Not a full review, but I'm a little concerned if that part
actually works for zone 0, i.e. that zone 0 remains intact when we flush
other zones.

I sent a question to the netdev list:
  https://lore.kernel.org/netdev/2032238f-31ac-4106-8f22-522e76df5...@ovn.org/

Please, reply there.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v6 02/13] northd: Add a new engine 'lr_stateful' to manage lr's stateful data.

[Dumitru Ceara]

On 1/30/24 22:21, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> This new engine now maintains the load balancer and NAT data of a
> logical router which was earlier part of northd engine node data.
> The main inputs to this engine are:
>- northd node
>- lr_nat node
>- lb_data node
> 
> A record for each logical router is maintained in the 'lr_stateful_table'
> hmap table and this record
>- stores the lb related data
>- embeds the 'lr_nat' record.
> 
> This engine node becomes an input to 'lflow' node.
> 
> Signed-off-by: Numan Siddique 
> ---

I have two small comments, style related, below.  With those addressed:

Acked-by: Dumitru Ceara 

[...]

> +static void lr_stateful_table_build(struct lr_stateful_table *,
> +   const struct lr_nat_table *,
> +   const struct ovn_datapaths *lr_datapaths,
> +   const struct hmap *lb_datapaths_map,
> +   const struct hmap *lbgrp_datapaths_map);

Nit: indentation.

[...]

> +
> +static inline bool
> +lr_stateful_has_tracked_data(struct lr_stateful_tracked_data *trk_data) {

Nit: curly brace on next line.

Thanks,
Dumitru

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Eelco Chaudron]

On 1 Feb 2024, at 18:28, Aaron Conole wrote:

> Eelco Chaudron  writes:
>
>> On 31 Jan 2024, at 18:03, Aaron Conole wrote:
>>
>>> Eelco Chaudron  writes:
>>>
 On 25 Jan 2024, at 21:55, Aaron Conole wrote:

> From: Kevin Sprague 
>
> During normal operations, it is useful to understand when a particular 
> flow
> gets removed from the system. This can be useful when debugging 
> performance
> issues tied to ofproto flow changes, trying to determine deployed traffic
> patterns, or while debugging dynamic systems where ports come and go.
>
> Prior to this change, there was a lack of visibility around flow 
> expiration.
> The existing debugging infrastructure could tell us when a flow was added 
> to
> the datapath, but not when it was removed or why.
>
> This change introduces a USDT probe at the point where the revalidator
> determines that the flow should be removed.  Additionally, we track the
> reason for the flow eviction and provide that information as well.  With
> this change, we can track the complete flow lifecycle for the netlink
> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, 
> and
> the revaldiator USDT, letting us watch as flows are added and removed from
> the kernel datapath.
>
> This change only enables this information via USDT probe, so it won't be
> possible to access this information any other way (see:
> Documentation/topics/usdt-probes.rst).
>
> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
> which serves as a demonstration of how the new USDT probe might be used
> going forward.
>
> Signed-off-by: Kevin Sprague 
> Co-authored-by: Aaron Conole 
> Signed-off-by: Aaron Conole 

 Thanks for following this up Aaron! See comments on this patch below. I 
 have no additional comments on patch 2.

 Cheers,

 Eelco


> ---
>  Documentation/topics/usdt-probes.rst |   1 +
>  ofproto/ofproto-dpif-upcall.c|  42 +-
>  utilities/automake.mk|   3 +
>  utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>  4 files changed, 693 insertions(+), 6 deletions(-)
>  create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>
> diff --git a/Documentation/topics/usdt-probes.rst 
> b/Documentation/topics/usdt-probes.rst
> index e527f43bab..a8da9bb1f7 100644
> --- a/Documentation/topics/usdt-probes.rst
> +++ b/Documentation/topics/usdt-probes.rst
> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>  - dpif_recv:recv_upcall
>  - main:poll_block
>  - main:run_start
> +- revalidate:flow_result
>  - revalidate_ukey\_\_:entry
>  - revalidate_ukey\_\_:exit
>  - udpif_revalidator:start_dump

 You are missing the specific flow_result result section. This is from the 
 previous patch:
>>>
>>> D'oh!  Thanks for catching it.  I'll re-add it.
>>>
 @@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
  - ``utilities/usdt-scripts/bridge_loop.bt``


 +probe revalidate:flow_result
 +~
 +
 +**Description**:
 +This probe is triggered when the revalidator decides whether or not to
 +revalidate a flow. ``reason`` is an enum that denotes that either the flow
 +is being kept, or the reason why the flow is being deleted. The
 +``flow_reval_monitor.py`` script uses this probe to notify users when 
 flows
 +matching user-provided criteria are deleted.
 +
 +**Arguments**:
 +
 +- *arg0*: ``(enum flow_del_reason) reason``
 +- *arg1*: ``(struct udpif *) udpif``
 +- *arg2*: ``(struct udpif_key *) ukey``
 +
 +**Script references**:
 +
 +- ``utilities/usdt-scripts/flow_reval_monitor.py``
 +
 +
  Adding your own probes
  --

> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index b5cbeed878..97d75833f7 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -269,6 +269,18 @@ enum ukey_state {
>  };
>  #define N_UKEY_STATES (UKEY_DELETED + 1)
>
> +enum flow_del_reason {
> +FDR_REVALIDATE = 0, /* The flow was revalidated. */

 It was called FDR_FLOW_LIVE before, which might make more sense. As the 
 flow is just NOT deleted. It might or might not have been revalidated. 
 Thoughts?
>>>
>>> I think it had to have been revalidated if we emit the reason, because
>>> we only emit the reason code after revalidation.  IE: there are many
>>> places where we skip revalidation but the flow stays live - and we don't
>>> emit reasons in those cases.
>>>
>>> So at least for this patch, it MUST have been revalidated.  But maybe in
>>> the future, we 

Re: [ovs-dev] [RFC PATCH 06/10] python: ovs: flowviz: Add datapath tree format.

[Adrian Moreno]

On 1/30/24 16:53, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


Datapath flows can be arranged into a "tree"-like structure based on
recirculation ids, e.g:

  recirc(0),eth(...),ipv4(...) actions=ct,recirc(0x42)
\-> recirc(42),ct_state(0/0),eth(...),ipv4(...) actions=1
\-> recirc(42),ct_state(1/0),eth(...),ipv4(...) actions=userspace(...)

This patch adds support for building such logical datapath trees in a
format-agnostic way and adds support for console-based formatting
supporting:
- head-maps formatting of statistics
- hash-based pallete of recirculation ids: each recirculation id is
   assigned a unique color to easily follow the sequence of related
   actions.
- full-tree filtering: if a user specifies a filter, an entire subtree
   is filtered out if none of its branches satisfy it.

Signed-off-by: Adrian Moreno 


This patch looks good to me. One small nit on a comment not ending with a dot.

Acked-by: Eelco Chaudron 


---
  python/automake.mk |   1 +
  python/ovs/flowviz/console.py  |  22 +++
  python/ovs/flowviz/odp/cli.py  |  21 ++-
  python/ovs/flowviz/odp/tree.py | 290 +
  4 files changed, 332 insertions(+), 2 deletions(-)
  create mode 100644 python/ovs/flowviz/odp/tree.py

diff --git a/python/automake.mk b/python/automake.mk
index b4c1f84be..5050089e9 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -71,6 +71,7 @@ ovs_flowviz = \
python/ovs/flowviz/main.py \
python/ovs/flowviz/odp/__init__.py \
python/ovs/flowviz/odp/cli.py \
+   python/ovs/flowviz/odp/tree.py \
python/ovs/flowviz/ofp/__init__.py \
python/ovs/flowviz/ofp/cli.py \
python/ovs/flowviz/ofp/html.py \
diff --git a/python/ovs/flowviz/console.py b/python/ovs/flowviz/console.py
index 5b4b047c2..2d65f9bb6 100644
--- a/python/ovs/flowviz/console.py
+++ b/python/ovs/flowviz/console.py
@@ -13,6 +13,9 @@
  # limitations under the License.

  import colorsys
+import itertools
+import zlib
+
  from rich.console import Console
  from rich.text import Text
  from rich.style import Style
@@ -169,6 +172,25 @@ def heat_pallete(min_value, max_value):
  return heat


+def hash_pallete(hue, saturation, value):
+"""Generates a color pallete with the cartesian product
+of the hsv values provided and returns a callable that assigns a color for
+each value hash
+"""
+HSV_tuples = itertools.product(hue, saturation, value)
+RGB_tuples = map(lambda x: colorsys.hsv_to_rgb(*x), HSV_tuples)
+styles = [
+Style(color=Color.from_rgb(r * 255, g * 255, b * 255))
+for r, g, b in RGB_tuples
+]
+
+def get_style(string):
+hash_val = zlib.crc32(bytes(str(string), "utf-8"))
+return styles[hash_val % len(styles)]
+
+return get_style
+
+
  def default_highlight():
  """Generates a default style for highlights."""
  return Style(underline=True)
diff --git a/python/ovs/flowviz/odp/cli.py b/python/ovs/flowviz/odp/cli.py
index 78f5cfff4..4740e753e 100644
--- a/python/ovs/flowviz/odp/cli.py
+++ b/python/ovs/flowviz/odp/cli.py
@@ -13,12 +13,12 @@
  # limitations under the License.

  import click
-
  from ovs.flowviz.main import maincli
+from ovs.flowviz.odp.tree import ConsoleTreeProcessor
  from ovs.flowviz.process import (
  DatapathFactory,
-JSONProcessor,
  ConsoleProcessor,
+JSONProcessor,
  )


@@ -65,3 +65,20 @@ def console(opts, heat_map):
  )
  proc.process()
  proc.print()
+
+
+@datapath.command()
+@click.option(
+"-h",
+"--heat-map",
+is_flag=True,
+default=False,
+show_default=True,
+help="Create heat-map with packet and byte counters",
+)
+@click.pass_obj
+def tree(opts, heat_map):
+"""Print the flows in a tree based on the 'recirc_id'."""
+processor = ConsoleTreeProcessor(opts)
+processor.process()
+processor.print(heat_map)
diff --git a/python/ovs/flowviz/odp/tree.py b/python/ovs/flowviz/odp/tree.py
new file mode 100644
index 0..cfddb162e
--- /dev/null
+++ b/python/ovs/flowviz/odp/tree.py
@@ -0,0 +1,290 @@
+# Copyright (c) 2023 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from rich.style import Style
+from rich.text import Text
+from rich.tree import Tree
+
+from ovs.flowviz.console import (
+ConsoleFormatter,
+ConsoleBuffer,
+hash_pallete,
+heat_pallete,
+file_header,
+)
+from ovs.flowviz.process import (
+

Re: [ovs-dev] [RFC PATCH 05/10] python: ovs: flowviz: Add html formatting.

[Adrian Moreno]

On 1/30/24 16:51, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


Add a HTML Formatter and use it to print OpenFlow flows in an HTML list
with table links.

Signed-off-by: Adrian Moreno 


No real comments from my side, this looks good! Maybe add an example to the 
commit message.



Sure!


One small potential addition request below?

Acked-by: Eelco Chaudron 


---
  python/automake.mk  |   3 +-
  python/ovs/flowviz/html_format.py   | 136 
  python/ovs/flowviz/ofp/cli.py   |  10 ++
  python/ovs/flowviz/ofp/html.py  |  80 
  python/ovs/flowviz/ovs-flowviz.conf |  16 +++-
  5 files changed, 243 insertions(+), 2 deletions(-)
  create mode 100644 python/ovs/flowviz/html_format.py
  create mode 100644 python/ovs/flowviz/ofp/html.py

diff --git a/python/automake.mk b/python/automake.mk
index cf8b71659..b4c1f84be 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -67,15 +67,16 @@ ovs_flowviz = \
python/ovs/flowviz/__init__.py \
python/ovs/flowviz/console.py \
python/ovs/flowviz/format.py \
+   python/ovs/flowviz/html_format.py \
python/ovs/flowviz/main.py \
python/ovs/flowviz/odp/__init__.py \
python/ovs/flowviz/odp/cli.py \
python/ovs/flowviz/ofp/__init__.py \
python/ovs/flowviz/ofp/cli.py \
+   python/ovs/flowviz/ofp/html.py \
python/ovs/flowviz/ovs-flowviz \
python/ovs/flowviz/process.py

-
  # These python files are used at build time but not runtime,
  # so they are not installed.
  EXTRA_DIST += \
diff --git a/python/ovs/flowviz/html_format.py 
b/python/ovs/flowviz/html_format.py
new file mode 100644
index 0..ebfa65c34
--- /dev/null
+++ b/python/ovs/flowviz/html_format.py
@@ -0,0 +1,136 @@
+# Copyright (c) 2023 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from ovs.flowviz.format import FlowFormatter, FlowBuffer, FlowStyle
+
+
+class HTMLStyle:
+"""HTMLStyle defines a style for html-formatted flows.
+
+Args:
+color(str): Optional; a string representing the CSS color to use
+anchor_gen(callable): Optional; a callable to be used to generate the
+href
+"""
+
+def __init__(self, color=None, anchor_gen=None):
+self.color = color
+self.anchor_gen = anchor_gen
+
+
+class HTMLBuffer(FlowBuffer):
+"""HTMLBuffer implementes FlowBuffer to provide html-based flow formatting.
+
+Each flow gets formatted as:
+...
+"""
+
+def __init__(self):
+self._text = ""
+
+@property
+def text(self):
+return self._text
+
+def _append(self, string, color, href):
+"""Append a key a string"""
+style = ' style="color:{}"'.format(color) if color else ""
+self._text += "".format(style)
+if href:
+self._text += "".format(href)
+self._text += string
+if href:
+self._text += ""
+self._text += ""
+
+def append_key(self, kv, style):
+"""Append a key.
+Args:
+kv (KeyValue): the KeyValue instance to append
+style (HTMLStyle): the style to use
+"""
+href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
+return self._append(
+kv.meta.kstring, style.color if style else "", href
+)
+
+def append_delim(self, kv, style):
+"""Append a delimiter.
+Args:
+kv (KeyValue): the KeyValue instance to append
+style (HTMLStyle): the style to use
+"""
+href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
+return self._append(kv.meta.delim, style.color if style else "", href)
+
+def append_end_delim(self, kv, style):
+"""Append an end delimiter.
+Args:
+kv (KeyValue): the KeyValue instance to append
+style (HTMLStyle): the style to use
+"""
+href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
+return self._append(
+kv.meta.end_delim, style.color if style else "", href
+)
+
+def append_value(self, kv, style):
+"""Append a value.
+Args:
+kv (KeyValue): the KeyValue instance to append
+style (HTMLStyle): the style to use
+"""
+href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
+return 

Re: [ovs-dev] [PATCH ovn] ovn-controller: Stop dropping bind_vport requests immediately after handling.

[Ales Musil]

On Tue, Jan 30, 2024 at 2:59 PM Mohammad Heib  wrote:

> ovn-controller immediately removes the vport_bindings requests that were
> generated by VIFs after handling them locally, this approach is intended
> to avoid binding the vport to one VIF only and allocate the vport
> between the different VIFs that exist in the vport:virtual-parents.
>
> Although the behavior mentioned above is correct, in some cases when the
> SB Database is busy the transaction that binds this vport to the desired
> VIF/chassis can fail and the controller will not re-try to bind the
> vport again because we deleted the bind_vport request in the previous
> loop/TXN.
>
> This patch aims to change the above behavior by storing the bind_vport
> requests for a bit longer time and this is done by the following:
> 1. add relevancy_time for each new bind_vport request and
>mark this request as new.
>
> 2. loop0: ovn-controller will try to handle this bind_vport request
>for the first time as usual (no change).
>
>3. loop0: ovn-controller will try to delete the already handled
> bind_vport
>   request as usual but first, it will check if this request is marked
> as new and
>   if the relevancy_time is still valid if so the controller will mark
> this
>   request as an old request and keep it, otherwise remove it.
>
>4.loop1: ovn-controller will try to commit the same change again for
>  the old request, if the previous commit in loop0 succeeded the
>  change will not have any effect on SB, otherwise we will try to
>  commit the same vport_bind request again.
>
>   5. loop1: delete the old bind_vport request.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1954659
> Signed-off-by: Mohammad Heib 
> ---
>

Hi  Mohammad,

overall the change makes sense, I have a couple of comments see down below.

 controller/pinctrl.c | 58 +++-
>  1 file changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index bd3bd3d81..152962448 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6519,10 +6519,52 @@ struct put_vport_binding {
>  uint32_t vport_key;
>
>  uint32_t vport_parent_key;
> +
> +/* This vport record Only relevant if "relevancy_time"
> + * is earlier than the current_time, "new_record" is true.
> + */
> +long long int relevancy_time;
>

The intention of the variable should be probably clearer e.g.
relevant_until_ms.

Also reading through the rest of the code it doesn't seem possible that the
binding wouldn't be deleted, hence I think there isn't any need for the
relevancy time, it should be enough to have a flag that will be flipped. In
any case we will try to commit twice. I would leave out the whole relevancy
and keep the flag flipping it on first commit WDYT?


> +bool new_record;
>  };
>
>  /* Contains "struct put_vport_binding"s. */
>  static struct hmap put_vport_bindings;
> +/* the relevance time in ms of vport record before deleteing. */
> +#define VPORT_RELEVANCE_TIME 1500
> +
> +/*
> + * Validate if the vport_binding record that was added
> + * by the pinctrl thread is still relevant and needs
> + * to be updated in the SBDB or not.
> + *
> + * vport_binding record is only relevant and needs to be updated in SB if:
> + *   1. The put_vport_binding:relevancy_time still valid.
> + *   2. The put_vport_binding:new_record is true:
> + *   The new_record will be set to "true" when this vport record is
> created
> + *   by function "pinctrl_handle_bind_vport".
> + *
> + *   After the first attempt to bind this vport to the chassis and
> + *   virtual_parent by function "run_put_vport_bindings" we will set
> the
> + *   value of vpb:new_record to "false" and keep it in
> "put_vport_bindings"
> + *
> + *   After the second attempt of binding the vpb it will be removed by
> + *   this function.
> + *
> + *   The above guarantees that we will try to bind the vport twice in
> + *   a certain amount of time.
> + *
> +*/
> +static bool
> +is_vport_binding_relevant(struct put_vport_binding *vpb)
> +{
> +long long int cur_time = time_msec();
> +
> +if (vpb->new_record && vpb->relevancy_time > cur_time) {
> +vpb->new_record = false;
>

This is a nasty side effect that I wouldn't expect from starting with is_.


> +return true;
> +}
> +return false;
> +}
>
>  static void
>  init_put_vport_bindings(void)
> @@ -6531,18 +6573,21 @@ init_put_vport_bindings(void)
>  }
>
>  static void
> -flush_put_vport_bindings(void)
> +flush_put_vport_bindings(bool force_flush)
>  {
>  struct put_vport_binding *vport_b;
> -HMAP_FOR_EACH_POP (vport_b, hmap_node, _vport_bindings) {
> -free(vport_b);
> +HMAP_FOR_EACH_SAFE (vport_b, hmap_node, _vport_bindings) {
> +if (!is_vport_binding_relevant(vport_b) || force_flush) {
> +hmap_remove(_vport_bindings, 

Re: [ovs-dev] [PATCH ovn] ovn-controller: Stop dropping bind_vport requests immediately after handling.

[Ales Musil]

On Fri, Feb 2, 2024 at 12:19 PM Ales Musil  wrote:

>
>
> On Tue, Jan 30, 2024 at 2:59 PM Mohammad Heib  wrote:
>
>> ovn-controller immediately removes the vport_bindings requests that were
>> generated by VIFs after handling them locally, this approach is intended
>> to avoid binding the vport to one VIF only and allocate the vport
>> between the different VIFs that exist in the vport:virtual-parents.
>>
>> Although the behavior mentioned above is correct, in some cases when the
>> SB Database is busy the transaction that binds this vport to the desired
>> VIF/chassis can fail and the controller will not re-try to bind the
>> vport again because we deleted the bind_vport request in the previous
>> loop/TXN.
>>
>> This patch aims to change the above behavior by storing the bind_vport
>> requests for a bit longer time and this is done by the following:
>> 1. add relevancy_time for each new bind_vport request and
>>mark this request as new.
>>
>> 2. loop0: ovn-controller will try to handle this bind_vport request
>>for the first time as usual (no change).
>>
>>3. loop0: ovn-controller will try to delete the already handled
>> bind_vport
>>   request as usual but first, it will check if this request is marked
>> as new and
>>   if the relevancy_time is still valid if so the controller will mark
>> this
>>   request as an old request and keep it, otherwise remove it.
>>
>>4.loop1: ovn-controller will try to commit the same change again for
>>  the old request, if the previous commit in loop0 succeeded the
>>  change will not have any effect on SB, otherwise we will try to
>>  commit the same vport_bind request again.
>>
>>   5. loop1: delete the old bind_vport request.
>>
>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1954659
>> Signed-off-by: Mohammad Heib 
>> ---
>>
>
> Hi  Mohammad,
>
> overall the change makes sense, I have a couple of comments see down below.
>
>  controller/pinctrl.c | 58 +++-
>>  1 file changed, 52 insertions(+), 6 deletions(-)
>>
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index bd3bd3d81..152962448 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -6519,10 +6519,52 @@ struct put_vport_binding {
>>  uint32_t vport_key;
>>
>>  uint32_t vport_parent_key;
>> +
>> +/* This vport record Only relevant if "relevancy_time"
>> + * is earlier than the current_time, "new_record" is true.
>> + */
>> +long long int relevancy_time;
>>
>
> The intention of the variable should be probably clearer e.g.
> relevant_until_ms.
>
> Also reading through the rest of the code it doesn't seem possible that
> the binding wouldn't be deleted, hence I think there isn't any need for the
> relevancy time, it should be enough to have a flag that will be flipped. In
> any case we will try to commit twice. I would leave out the whole relevancy
> and keep the flag flipping it on first commit WDYT?
>
>
>> +bool new_record;
>>  };
>>
>>  /* Contains "struct put_vport_binding"s. */
>>  static struct hmap put_vport_bindings;
>> +/* the relevance time in ms of vport record before deleteing. */
>> +#define VPORT_RELEVANCE_TIME 1500
>> +
>> +/*
>> + * Validate if the vport_binding record that was added
>> + * by the pinctrl thread is still relevant and needs
>> + * to be updated in the SBDB or not.
>> + *
>> + * vport_binding record is only relevant and needs to be updated in SB
>> if:
>> + *   1. The put_vport_binding:relevancy_time still valid.
>> + *   2. The put_vport_binding:new_record is true:
>> + *   The new_record will be set to "true" when this vport record is
>> created
>> + *   by function "pinctrl_handle_bind_vport".
>> + *
>> + *   After the first attempt to bind this vport to the chassis and
>> + *   virtual_parent by function "run_put_vport_bindings" we will set
>> the
>> + *   value of vpb:new_record to "false" and keep it in
>> "put_vport_bindings"
>> + *
>> + *   After the second attempt of binding the vpb it will be removed
>> by
>> + *   this function.
>> + *
>> + *   The above guarantees that we will try to bind the vport twice in
>> + *   a certain amount of time.
>> + *
>> +*/
>> +static bool
>> +is_vport_binding_relevant(struct put_vport_binding *vpb)
>> +{
>> +long long int cur_time = time_msec();
>> +
>> +if (vpb->new_record && vpb->relevancy_time > cur_time) {
>> +vpb->new_record = false;
>>
>
> This is a nasty side effect that I wouldn't expect from starting with is_.
>
>
>> +return true;
>> +}
>> +return false;
>> +}
>>
>>  static void
>>  init_put_vport_bindings(void)
>> @@ -6531,18 +6573,21 @@ init_put_vport_bindings(void)
>>  }
>>
>>  static void
>> -flush_put_vport_bindings(void)
>> +flush_put_vport_bindings(bool force_flush)
>>  {
>>  struct put_vport_binding *vport_b;
>> -HMAP_FOR_EACH_POP (vport_b, hmap_node, _vport_bindings) {
>> -

[ovs-dev] [PATCH v2] netlink-conntrack: Optimize flushing ct zone.

[Felix Huettner via dev]

Previously the kernel did not provide a netlink interface to flush/list
only conntrack entries matching a specific zone. With [1] it is now
possible to flush and list conntrack entries filtered by zone. Older
kernels not yet supporting this feature will ignore the filter.
For the list request that means just returning all entries (which we can
then filter in userspace as before).
For the flush request that means deleting all conntrack entries.

These significantly improves the performance of flushing conntrack zones
when the conntrack table is large. Since flushing a conntrack zone is
normally triggered via an openflow command it blocks the main ovs thread
and thereby also blocks new flows from being applied. Using this new
feature we can reduce the flushing time for zones by around 93%.

In combination with OVN the creation of a Logical_Router (which causes
the flushing of a ct zone) could block other operations, e.g. the
failover of Logical_Routers (as they cause new flows to be created).
This is visible from a user perspective as a ovn-controller that is idle
(as it waits for vswitchd) and vswitchd reporting:
"blocked 1000 ms waiting for main to quiesce" (potentially with ever
increasing times).

The following performance tests where run in a qemu vm with 500.000
conntrack entries distributed evenly over 500 ct zones using `ovstest
test-netlink-conntrack flush zone=`.

With this patch and kernel v6.8-rc2:

-
   Min (s) Median (s)  90%ile (s)  
99%ile (s)  Max (s) Mean (s)Total (s)   Count
-
flush zone with 1000 entries   0.260   0.319   0.335   
0.348   0.362   0.320   80.02   250
flush zone with no entry   0.228   0.298   0.325   
0.340   0.348   0.296   73.93   250
-

With this patch and kernel v6.7.1:

-
   Min (s) Median (s)  90%ile (s)  
99%ile (s)  Max (s) Mean (s)Total (s)   Count
-
flush zone with 1000 entries   3.946   4.237   4.367   
4.495   4.543   4.236   1058.992250
flush zone with no entry   3.462   4.460   4.662   
4.931   5.390   4.430   1107.479250
-

Without this patch and kernel v6.8-rc2:

-
   Min (s) Median (s)  90%ile (s)  
99%ile (s)  Max (s) Mean (s)Total (s)   Count
-
flush zone with 1000 entries   3.497   4.349   4.522   
4.773   5.054   4.331   1082.802250
flush zone with no entry   3.212   4.010   4.572   
6.003   6.396   4.071   1017.838250
-

[1]: 
https://github.com/torvalds/linux/commit/eff3c558bb7e61c41b53e4c8130e514a5a4df9ba

Co-Authored-By: Luca Czesla 
Signed-off-by: Luca Czesla 
Co-Authored-By: Max Lamprecht 
Signed-off-by: Max Lamprecht 
Signed-off-by: Felix Huettner 
---

v1->v2:
- fixed wrong signed-off-by

 lib/netlink-conntrack.c | 57 +++--
 tests/system-traffic.at |  8 ++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
index 492bfcffb..1b050894d 100644
--- a/lib/netlink-conntrack.c
+++ b/lib/netlink-conntrack.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "byte-order.h"
 #include "compiler.h"
@@ -141,6 +142,9 @@ nl_ct_dump_start(struct nl_ct_dump_state **statep, const 
uint16_t 

Re: [ovs-dev] [PATCH ovn v3] ovn-ic: Fix global blacklist filter for IPv6 addresses.

[Ales Musil]

On Tue, Jan 30, 2024 at 3:10 PM Roberto Bartzen Acosta via dev <
ovs-dev@openvswitch.org> wrote:

> This commit fixes the prefix filter function as the return condition for
> IPv6 addresses is disabling the advertisement of all learned prefixes
> regardless of the match with the blacklist or not.
>
> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
> Signed-off-by: Roberto Bartzen Acosta 
> ---
>

Hi Roberto,

thank you for the patch.


>  ic/ovn-ic.c |  22 ---
>  tests/ovn-ic.at | 100 
>  2 files changed, 116 insertions(+), 6 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 6f8f5734d..1c9c9ae2c 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -1024,6 +1024,20 @@ prefix_is_link_local(struct in6_addr *prefix,
> unsigned int plen)
>  ((prefix->s6_addr[1] & 0xc0) == 0x80));
>  }
>
> +static bool
> +compare_ipv6_prefixes(const struct in6_addr *s_prefix,
> +  const struct in6_addr *d_prefix2, int plen)
> +{
> +struct in6_addr mask = ipv6_create_mask(plen);
> +for (int i = 0; i <= (plen / 8); i++) {
> +if ((s_prefix->s6_addr[i] & mask.s6_addr[i]) ^
> +(d_prefix2->s6_addr[i] & mask.s6_addr[i])) {
> +return false;
> +}
> +}
> +return true;
> +}
> +
>
 static bool
>  prefix_is_black_listed(const struct smap *nb_options,
> struct in6_addr *prefix,
> @@ -1064,12 +1078,8 @@ prefix_is_black_listed(const struct smap
> *nb_options,
>  continue;
>  }
>  } else {
> -struct in6_addr mask = ipv6_create_mask(bl_plen);
> -for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
> -if ((prefix->s6_addr[i] & mask.s6_addr[i])
> -!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
> -continue;
> -}
> +if (!compare_ipv6_prefixes(prefix, _prefix, bl_plen)) {
> +continue;
>  }
>


There is no need for hand implementation, OvS provides plenty of helpers.
This can be replaced with something like the snippet below:

struct in6_addr mask = ipv6_create_mask(bl_plen);
struct in6_addr m_prefix = ipv6_addr_bitand(prefix, );
struct in6_addr m_bl_prefix = ipv6_addr_bitand(_prefix, );

if (!ipv6_addr_equals(_prefix, _bl_prefix)) {
continue;
}


>  }
>  matched = true;
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index d4c436f84..1f9df71e9 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2])
>
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
> +AT_KEYWORDS([IPv6-route-sync-blacklist])
> +
> +ovn_init_ic_db
> +ovn-ic-nbctl ts-add ts1
> +
> +for i in 1 2; do
> +ovn_start az$i
> +ovn_as az$i
> +
> +# Enable route learning at AZ level
> +ovn-nbctl set nb_global . options:ic-route-learn=true
> +# Enable route advertising at AZ level
> +ovn-nbctl set nb_global . options:ic-route-adv=true
> +# Enable blacklist single filter for IPv6
> +ovn-nbctl set nb_global .
> options:ic-route-blacklist="2003:db8:1::/64,\
> +2004:::/32,2005:1234::/21"
> +
> +OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
> +
> +# Create LRP and connect to TS
> +ovn-nbctl lr-add lr$i
> +ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i
> 2001:db8:1::$i/64
> +ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
> +-- lsp-set-addresses lsp-ts1-lr$i router \
> +-- lsp-set-type lsp-ts1-lr$i router \
> +-- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
> +
> +ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i
> 2002:db8:1::$i/64
> +
> +# Create blacklisted LRPs and connect to TS
> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
> +11:11:11:11:11:1$i 2003:db8:1::$i/64
> +
> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
> +22:22:22:22:22:2$i 2004::bbb::$i/48
> +
> +# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
> +33:33:33:33:33:3$i 2005:1734:5678::$i/50
> +
> +# additional not filtered prefix -> different subnet bits
> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \
> +33:33:33:33:33:3$i 2005:1834:5678::$i/50
> +
> +done
> +
> +for i in 1 2; do
> +OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep
> learned])
> +done
> +
> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 |
> +awk '/learned/{print $1, $2}' ], [0], [dnl
> +2002:db8:1::/64 2001:db8:1::2
> +2005:1834:5678::/50 2001:db8:1::2
> +])
> +
> +for i in 1 2; do
> +ovn_as az$i
> +
> +# Drop blacklist
> +ovn-nbctl remove nb_global . options ic-route-blacklist
> +
> +done
> +
> 

Re: [ovs-dev] [PATCH ovn v2] tests: Fix flaky test 'SB Port binding incremental processing'.

[Ales Musil]

On Thu, Feb 1, 2024 at 1:10 AM  wrote:

> From: Numan Siddique 
>
> Fixes: 121661678317 ("northd: Move router ports SB PB options sync to
> sync_to_sb_pb node.")
>
> Signed-off-by: Numan Siddique 
> ---
>
> v1 -> v2
> -
>* Fix the checkpatch errors.
>
>  tests/ovn-northd.at | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 67e81ddba3..fbd0d9ef18 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -10404,6 +10404,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  # Make lrp a gateway port
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb lrp-set-gateway-chassis lrp hv1
> +wait_column "hosting-chassis=hv1" nb:Logical_Router_Port status name=lrp
> +
>  # There will be 3 recomputes of northd engine node
>  #   1. missing handler for input NB_logical_router
>  #   2. missing handler for input SB_ha_chassis_group
> --
> 2.43.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.com

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 02/10] python: ovs: flowviz: Add file processing infra.

[Adrian Moreno]

On 1/30/24 16:47, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


process.py contains a useful base class that processes files
odp.py and ofp.py: contain datapath and openflow subcommand definitions
as well as the first formatting option: json.

Also, this patch adds basic filtering support.

Examples:
$ ovs-ofctl dump-flows br-int | ovs-flowviz openflow json
$ ovs-ofctl dump-flows br-int > flows.txt && ovs-flowviz -i flows.txt openflow 
json
$ ovs-ofctl appctl dpctl/dump-flows | ovs-flowviz -f 'ct' datapath json
$ ovs-ofctl appctl dpctl/dump-flows > flows.txt && ovs-flowviz -i flows.txt -f 
'drop' datapath json


Some small comments below!


Signed-off-by: Adrian Moreno 
---
  python/automake.mk |   5 +-
  python/ovs/flowviz/__init__.py |   2 +
  python/ovs/flowviz/main.py | 103 +-
  python/ovs/flowviz/odp/cli.py  |  42 
  python/ovs/flowviz/ofp/cli.py  |  42 
  python/ovs/flowviz/process.py  | 192 +
  6 files changed, 384 insertions(+), 2 deletions(-)
  create mode 100644 python/ovs/flowviz/odp/cli.py
  create mode 100644 python/ovs/flowviz/ofp/cli.py
  create mode 100644 python/ovs/flowviz/process.py

diff --git a/python/automake.mk b/python/automake.mk
index 4302f0136..4845565b8 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -67,8 +67,11 @@ ovs_flowviz = \
python/ovs/flowviz/__init__.py \
python/ovs/flowviz/main.py \
python/ovs/flowviz/odp/__init__.py \
+   python/ovs/flowviz/odp/cli.py \
python/ovs/flowviz/ofp/__init__.py \
-   python/ovs/flowviz/ovs-flowviz
+   python/ovs/flowviz/ofp/cli.py \
+   python/ovs/flowviz/ovs-flowviz \
+   python/ovs/flowviz/process.py


  # These python files are used at build time but not runtime,
diff --git a/python/ovs/flowviz/__init__.py b/python/ovs/flowviz/__init__.py
index e69de29bb..898dba522 100644
--- a/python/ovs/flowviz/__init__.py
+++ b/python/ovs/flowviz/__init__.py
@@ -0,0 +1,2 @@
+import ovs.flowviz.ofp.cli  # noqa: F401
+import ovs.flowviz.odp.cli  # noqa: F401
diff --git a/python/ovs/flowviz/main.py b/python/ovs/flowviz/main.py
index a2d5ca1fa..a45c06e48 100644
--- a/python/ovs/flowviz/main.py
+++ b/python/ovs/flowviz/main.py
@@ -12,19 +12,67 @@
  # See the License for the specific language governing permissions and
  # limitations under the License.

+import os
+
  import click


Do we maybe want all imports together and sorted, i.e.:

import click
import os



Sure. Will do.


+from ovs.flow.filter import OFFilter
+

  class Options(dict):
  """Options dictionary"""


+def validate_input(ctx, param, value):
+"""Validate the "-i" option"""
+result = list()
+for input_str in value:
+parts = input_str.strip().split(",")
+if len(parts) == 2:
+file_parts = tuple(parts)
+elif len(parts) == 1:
+file_parts = tuple(["Filename: " + parts[0], parts[0]])
+else:
+raise click.BadParameter(
+"input filename should have the following format: "
+"[alias,]FILENAME"
+)
+
+if not os.path.isfile(file_parts[1]):
+raise click.BadParameter(
+"input filename %s does not exist" % file_parts[1]
+)
+result.append(file_parts)
+return result
+
+
  @click.group(
  subcommand_metavar="TYPE",
  context_settings=dict(help_option_names=["-h", "--help"]),
  )
+@click.option(
+"-i",
+"--input",
+"filename",
+help="Read flows from specified filepath. If not provided, flows will be"
+" read from stdin. This option can be specified multiple times."
+" Format [alias,]FILENAME. Where alias is a name that shall be used to"
+" refer to this FILENAME",
+multiple=True,
+type=click.Path(),
+callback=validate_input,
+)
+@click.option(
+"-f",
+"--filter",
+help="Filter flows that match the filter expression."
+"Run 'ovs-flowviz filter' for a detailed description of the filtering "
+"syntax",
+type=str,
+show_default=False,
+)
  @click.pass_context
-def maincli(ctx):
+def maincli(ctx, filename, filter):
  """
  OpenvSwitch flow visualization utility.

@@ -32,6 +80,59 @@ def maincli(ctx):
  (such as the output of ovs-ofctl dump-flows or ovs-appctl 
dpctl/dump-flows)
  and prints them in different formats.
  """
+ctx.obj = Options()
+ctx.obj["filename"] = filename or None
+if filter:
+try:
+ctx.obj["filter"] = OFFilter(filter)
+except Exception as e:
+raise click.BadParameter("Wrong filter syntax: {}".format(e))
+
+
+@maincli.command(hidden=True)
+@click.pass_context
+def filter(ctx):
+"""
+\b
+Filter Syntax
+*
+
+ [! | not ] {key}[[.subkey]...] [OPERATOR] {value})] [LOGICAL OPERATOR] ...
+
+\b
+Comparison operators are:
+=   equality
+<   less than
+ 

Re: [ovs-dev] [RFC PATCH 07/10] python: ovs: flowviz: Add OpenFlow logical view.

[Adrian Moreno]

On 1/30/24 16:54, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


This view is interesting for debugging the logical pipeline. It arranges
the flows in "logical" groups (not to be confused with OVN's
Logical_Flows). A logical group of flows is a set of flows that:
- Have the same table number and priority
- Match on the same fields (regardless of the value they match against)
- Have the same actions, regardless of the arguments for tose actions,
   except for output and recirc, for which arguments do care.

Optionally, the cookie can also be force to be unique for the logical
group. By doing so, we can extend the information we show by querying an
external OVN database and running "ovn-detrace" on each cookie. The
result is a compact list of flow groups with interlieved OVN
information.

Furthermore, if connected to an OVN database, we can apply an OVN
regexp filter.

Examples:
$ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic
$ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic -s -h
$ export OVN_NB_DB=...
$ export OVN_SB_DB=...
$ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic -d
$ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic -d
--ovn-filter="acl.*icmp4"


When trying some random trace I got the following:

$ ovs-flowviz -i ~/ofctl.txt -l "drop" --style=dark openflow logic -h

╭─╮
│   
    Filename: 
/home/echaudron/ofctl.txt  
 │
╰─╯
Traceback (most recent call last):
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/bin/ovs-flowviz", line 
20, in 
 main.main()
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/main.py",
 line 196, in main
 maincli()
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py",
 line 1157, in __call__
 return self.main(*args, **kwargs)
^^
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py",
 line 1078, in main
 rv = self.invoke(ctx)
  
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py",
 line 1688, in invoke
 return _process_result(sub_ctx.command.invoke(sub_ctx))
^^^
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py",
 line 1688, in invoke
 return _process_result(sub_ctx.command.invoke(sub_ctx))
^^^
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py",
 line 1434, in invoke
 return ctx.invoke(self.callback, **ctx.params)
^^^
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py",
 line 783, in invoke
 return __callback(*args, **kwargs)
^^^
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/decorators.py",
 line 45, in new_func
 return f(get_current_context().obj, *args, **kwargs)
^
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/ofp/cli.py",
 line 182, in logic
 processor.print(show_flows)
   File 
"/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/ofp/logic.py",
 line 203, in print
 self.console.style.set_value_style(
 
AttributeError: 'LogicFlowProcessor' object has no attribute 'console'

This gets solved if I apply the next patch, see the comment in the next patch.



Thanks for doing per-patch testing. I missed that and, since the patch 
organization is artificial (meaning this was developed over many patches in my 
private repo and then artificially squashed) I 

Re: [ovs-dev] [RFC PATCH 08/10] python: ovs: flowviz: Add Openflow cookie format.

[Adrian Moreno]

On 1/30/24 16:55, Eelco Chaudron wrote:

On 1 Dec 2023, at 20:14, Adrian Moreno wrote:


When anaylizing OVN issues, it might be useful to see what OpenFlow
flows were generated from each logical flow. In order to make it simpler
to visualize this, add a cookie format that simply sorts the flows first
by cookie, then by table.


The code looks good to me, however, did not try with ovs-detrace. One comment 
on code that needs to move to the previous patch.
 > Maybe add an example in the commit message.


Sure.



Acked-by: Eelco Chaudron 


Signed-off-by: Adrian Moreno 
---
  python/ovs/flowviz/ofp/cli.py   | 57 -
  python/ovs/flowviz/ofp/logic.py | 63 -
  2 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/python/ovs/flowviz/ofp/cli.py b/python/ovs/flowviz/ofp/cli.py
index 6b1435ea1..9658d00d3 100644
--- a/python/ovs/flowviz/ofp/cli.py
+++ b/python/ovs/flowviz/ofp/cli.py
@@ -18,7 +18,7 @@ import click

  from ovs.flowviz.main import maincli
  from ovs.flowviz.ofp.html import HTMLProcessor
-from ovs.flowviz.ofp.logic import LogicFlowProcessor
+from ovs.flowviz.ofp.logic import CookieProcessor, LogicFlowProcessor
  from ovs.flowviz.process import (
  OpenFlowFactory,
  JSONProcessor,
@@ -182,6 +182,61 @@ def logic(
  processor.print(show_flows)


+@openflow.command()
+@click.option(
+"-d",
+"--ovn-detrace",
+"ovn_detrace_flag",
+is_flag=True,
+show_default=True,
+help="Use ovn-detrace to extract cookie information",
+)
+@click.option(
+"--ovn-detrace-path",
+default="/usr/bin",
+type=click.Path(),
+help="Use an alternative path to where ovn_detrace.py is located. "
+"Instead of using this option you can just set PYTHONPATH accordingly",
+show_default=True,
+callback=ovn_detrace_callback,
+)
+@click.option(
+"--ovnnb-db",
+default=os.getenv("OVN_NB_DB") or "unix:/var/run/ovn/ovnnb_db.sock",
+help="Specify the OVN NB database string (implies -d). "
+"If the OVN_NB_DB environment variable is set, it's used as default. "
+"Otherwise, the default is unix:/var/run/ovn/ovnnb_db.sock",
+callback=ovn_detrace_callback,
+)
+@click.option(
+"--ovnsb-db",
+default=os.getenv("OVN_SB_DB") or "unix:/var/run/ovn/ovnsb_db.sock",
+help="Specify the OVN NB database string (implies -d). "
+"If the OVN_NB_DB environment variable is set, it's used as default. "
+"Otherwise, the default is unix:/var/run/ovn/ovnnb_db.sock",
+callback=ovn_detrace_callback,
+)
+@click.option(
+"-o",
+"--ovn-filter",
+help="Specify a filter to be run on ovn-detrace information (implied -d). "
+"Format: python regular expression "
+"(see https://docs.python.org/3/library/re.html)",
+callback=ovn_detrace_callback,
+)
+@click.pass_obj
+def cookie(
+opts, ovn_detrace_flag, ovn_detrace_path, ovnnb_db, ovnsb_db, ovn_filter
+):
+"""Print the flow tables sorted by cookie."""
+if ovn_detrace_flag:
+opts["ovn_detrace_flag"] = True
+
+processor = CookieProcessor(opts)
+processor.process()
+processor.print()
+
+
  @openflow.command()
  @click.pass_obj
  def html(opts):
diff --git a/python/ovs/flowviz/ofp/logic.py b/python/ovs/flowviz/ofp/logic.py
index cb4568cf1..9d244d137 100644
--- a/python/ovs/flowviz/ofp/logic.py
+++ b/python/ovs/flowviz/ofp/logic.py
@@ -200,7 +200,7 @@ class LogicFlowProcessor(OpenFlowFactory, FileProcessor):
  if len(self.heat_map) > 0 and len(table.values()) > 0:
  for i, field in enumerate(self.heat_map):
  (min_val, max_val) = self.min_max[name][i]
-self.console.style.set_value_style(
+formatter.style.set_value_style(


This probably needs to move to patch 7.



Yep!


  field, heat_pallete(min_val, max_val)
  )

@@ -301,3 +301,64 @@ cookie_style_gen = hash_pallete(
  saturation=[0.5],
  value=[0.5 + x / 10 * (0.85 - 0.5) for x in range(0, 10)],
  )
+
+
+class CookieProcessor(OpenFlowFactory, FileProcessor):
+"""Processor that sorts flows into cookies and tables."""
+
+def __init__(self, opts):
+super().__init__(opts)
+self.data = dict()
+self.ovn_detrace = (
+OVNDetrace(opts) if opts.get("ovn_detrace_flag") else None
+)
+
+def start_file(self, name, filename):
+self.cookies = dict()
+
+def stop_file(self, name, filename):
+self.data[name] = self.cookies
+
+def process_flow(self, flow, name):
+"""Sort the flows by table and logical flow."""
+cookie = flow.info.get("cookie") or 0
+if not self.cookies.get(cookie):
+self.cookies[cookie] = dict()
+
+table = flow.info.get("table") or 0
+if not self.cookies[cookie].get(table):
+self.cookies[cookie][table] = list()
+

Re: [ovs-dev] [PATCH ovn v6 00/13] northd lflow incremental processing

[Dumitru Ceara]

On 1/30/24 22:20, num...@ovn.org wrote:
> From: Numan Siddique 
> 

Hi Numan,

> This patch series adds incremental processing in the lflow engine
> node to handle changes to northd and other engine nodes.
> Changed related to load balancers and NAT are mainly handled in
> this patch series.
> 
> This patch series can also be found here - 
> https://github.com/numansiddique/ovn/tree/northd_lbnatacl_lflow/v5
> 
> Prior to this patch series, most of the changes to northd engine
> resulted in full recomputation of logical flows.  This series
> aims to improve the performance of ovn-northd by adding the I-P
> support.  In order to add this support, some of the northd engine
> node data (from struct ovn_datapath) is split and moved over to
> new engine nodes - mainly related to load balancers, NAT and ACLs.
> 
> Below are the scale testing results done with these patches applied
> using ovn-heater.  The test ran the scenario  -
> ocp-500-density-heavy.yml [1].
> 
> With all the lflow I-P patches applied, the resuts are:
> 
> ---
> Min (s) Median (s)  90%ile (s)  
> 99%ile (s)  Max (s) Mean (s)Total (s)   Count   Failed
> ---
> Iteration Total 0.1368831.1290161.192001
> 1.2041671.2127280.66501783.127099   125 0
> Namespace.add_ports 0.0052160.0057360.007034
> 0.0154860.0189780.0062110.776373125 0
> WorkerNode.bind_port0.0350300.0460820.052469
> 0.0582930.0603110.04597311.493259   250 0
> WorkerNode.ping_port0.0050570.0067271.047692
> 1.0692531.0713360.26689666.724094   250 0
> ---
> 
> The results with the present main are:
> 
> ---
> Min (s) Median (s)  90%ile (s)  
> 99%ile (s)  Max (s) Mean (s)Total (s)   Count   Failed
> ---
> Iteration Total 0.1354912.2238053.311270
> 3.3390783.3453461.729172216.146495  125 0
> Namespace.add_ports 0.0053800.0057440.006819
> 0.0187730.0208000.0062920.786532125 0
> WorkerNode.bind_port0.0341790.0460550.053488
> 0.0588010.0710430.04611711.529311   250 0
> WorkerNode.ping_port0.0049560.0069523.086952
> 3.1917433.1928070.791544197.886026  250 0
> ---
> 
> Please see the link [2] which has a high level description of the
> changes done in this patch series.
> 
> 
> [1] - 
> https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
> [2] - https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410053.html
> 
> v5 -> v6
> --
>* Applied the first 3 patches of v5 after addressing all the review
>  comments (and with the Acks)
>  
>* Rebased to latest main and resolved the conflicts.
> 
>* Addressed almost all of the review comments received for v5 from
>  Han and Dumitru.
> - Added detailed documentation on 'struct lflow_ref' and life
>   cycle of 'struct lflow_ref_node'.
> - Added documentation on the thread safety limitations when
>   using 'struct lflow_ref'.
> 
> v4 -> v5
> ---
>* Rebased to latest main and resolved the conflicts.
> 
>* Addressed the review comments from Han in patch 15 (and in p8).  Removed 
> the
>  assert if SB dp group is missing and handled it by returning false
>  so that lflow engine recomputes.  Added test cases to cover this
>  scenario for both lflows (p8) and SB load balancers (p15) .
> 
> v3 -> v4
> ---
>* Addressed most of the review comments from Dumitru and Han.
> 
>* Found a couple of bugs in v3 patch 9 -
>  "northd: Refactor lflow management into a separate module."
>  and addressed them in v4.
>  To brief  the issue, if a 

[ovs-dev] [PATCH RFC 1/2] vswitch.xml: Use member wording for bonds.

[Simon Horman]

Since the patch-set that included [1] there has been a policy of using
the term member for bonds, LACP, and bundle contexts.  This is
consistent with the more recently adopted policy of using the inclusive
naming word list v1 [2, 3].

This patch addresses two instances where the term member should be used
in vswitch.xml. It does not address instances of alternative wording
that require code updates, which can addressed as follow-up activity.

[1] 91fc374a9c5a ("Eliminate use of term "slave" in bond, LACP, and bundle 
contexts.")
[2] df5e5cf4318a ("Documentation: Add section on inclusive language.")
[3] https://inclusivenaming.org/word-lists/

Signed-off-by: Simon Horman 
---
 vswitchd/vswitch.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 612ba41e3b24..8a1b607d71b9 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2159,7 +2159,7 @@
 
   
-If a slave interface with this name exists in the bond and
+If a member interface with this name exists in the bond and
 is up, it will be made active.  Relevant only when  is
 active-backup or if balance-tcp falls back
@@ -6291,7 +6291,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch 
options:peer=p1 \
 and avoids recirculation of packet in datapath. It is supported
 only for balance-tcp bond mode in netdev datapath. The new action
 gives higher performance by using bond buckets instead of post
-recirculation flows for selection of slave port from bond. By default
+recirculation flows for selection of member port from bond. By default
 this new action is disabled, however it can be enabled by setting
  in
  table.

-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH RFC 0/2] vswitch.xml: Use member wording for bonds.

[Simon Horman]

Since the patch-set that included [1] there has been a policy of using
the term member for bonds, LACP, and bundle contexts.  This is
consistent with the more recently adopted policy of using the inclusive
naming word list v1 [2, 3].

This patch-set addresses several instances where the term member should be used
in vswitch.xml. It does not address all instances of alternative wording
that require code updates, which can addressed as follow-up activity.

[1] 91fc374a9c5a ("Eliminate use of term "slave" in bond, LACP, and bundle 
contexts.")
[2] df5e5cf4318a ("Documentation: Add section on inclusive language.")
[3] https://inclusivenaming.org/word-lists/

---
Simon Horman (2):
  vswitch.xml: Use member wording for bonds.
  vswitch.xml: Rename bond_active_member.

 vswitchd/bridge.c  | 6 +++---
 vswitchd/vswitch.ovsschema | 6 +++---
 vswitchd/vswitch.xml   | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

base-commit: 6bdca15791ce53e15101c436d8524f2944336031

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH RFC 2/2] vswitch.xml: Rename bond_active_member.

[Simon Horman]

Since the patch-set that included [1] there has been a policy of using
the term member for bonds, LACP, and bundle contexts.  This is
consistent with the more recently adopted policy of using the inclusive
naming word list v1 [2, 3].

This patch addresses the name of the bond_active_member ovsdb column.
This is used internally to allow the active member of a bond across OVS
restart. And the effect of this patch is that when restarting OVS, if
the instance before restart did not have this patch and the instance
after does, or vice-versa, then active bond member selection will not be
sticky across that restart. Otherwise, the existing behaviour is
maintained.

[1] 91fc374a9c5a ("Eliminate use of term "slave" in bond, LACP, and bundle 
contexts.")
[2] df5e5cf4318a ("Documentation: Add section on inclusive language.")
[3] https://inclusivenaming.org/word-lists/
[4] 3e5aeeb581fa ("bridge: Keep bond active slave selection across OVS restart")

Signed-off-by: Simon Horman 
---
 vswitchd/bridge.c  | 6 +++---
 vswitchd/vswitch.ovsschema | 6 +++---
 vswitchd/vswitch.xml   | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 95a65fcdcd5e..01ec5e183091 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -463,7 +463,7 @@ bridge_init(const char *remote)
 ovsdb_idl_omit_alert(idl, _port_col_rstp_status);
 ovsdb_idl_omit_alert(idl, _port_col_rstp_statistics);
 ovsdb_idl_omit_alert(idl, _port_col_statistics);
-ovsdb_idl_omit_alert(idl, _port_col_bond_active_slave);
+ovsdb_idl_omit_alert(idl, _port_col_bond_active_member);
 ovsdb_idl_omit(idl, _port_col_external_ids);
 ovsdb_idl_omit_alert(idl, _port_col_trunks);
 ovsdb_idl_omit_alert(idl, _port_col_vlan_mode);
@@ -3003,7 +3003,7 @@ port_refresh_bond_status(struct port *port, bool 
force_update)
 
 ds_init(_s);
 ds_put_format(_s, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
-ovsrec_port_set_bond_active_slave(port->cfg, ds_cstr(_s));
+ovsrec_port_set_bond_active_member(port->cfg, ds_cstr(_s));
 ds_destroy(_s);
 }
 }
@@ -4640,7 +4640,7 @@ port_configure_bond(struct port *port, struct 
bond_settings *s)
 netdev_set_miimon_interval(iface->netdev, miimon_interval);
 }
 
-mac_s = port->cfg->bond_active_slave;
+mac_s = port->cfg->bond_active_member;
 if (!mac_s || !ovs_scan(mac_s, ETH_ADDR_SCAN_FMT,
 ETH_ADDR_SCAN_ARGS(s->active_member_mac))) {
 /* OVSDB did not store the last active interface */
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index e2d5e2e85e60..bfa9f39afb49 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "8.5.0",
- "cksum": "4040946650 27557",
+ "version": "9.0.0",
+ "cksum": "3156488466 27558",
  "tables": {
"Open_vSwitch": {
  "columns": {
@@ -194,7 +194,7 @@
  "type": "integer"},
"bond_downdelay": {
  "type": "integer"},
-   "bond_active_slave": {
+   "bond_active_member": {
  "type": {"key": {"type": "string"},
   "min": 0, "max": 1}},
"bond_fake_iface": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 8a1b607d71b9..5bf5a4107b5b 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2557,7 +2557,7 @@
   
 
 
-
+
   For a bonded port, record the MAC address of the current active
   member.
 

-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v6 10/13] northd: Add ls_stateful handler for lflow engine node.

[Dumitru Ceara]

On 1/30/24 22:23, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Acked-by: Han Zhou 
> Signed-off-by: Numan Siddique 
> ---

Acked-by: Dumitru Ceara 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Aaron Conole]

Eelco Chaudron  writes:

> On 1 Feb 2024, at 18:28, Aaron Conole wrote:
>
>> Eelco Chaudron  writes:
>>
>>> On 31 Jan 2024, at 18:03, Aaron Conole wrote:
>>>
 Eelco Chaudron  writes:

> On 25 Jan 2024, at 21:55, Aaron Conole wrote:
>
>> From: Kevin Sprague 
>>
>> During normal operations, it is useful to understand when a particular 
>> flow
>> gets removed from the system. This can be useful when debugging 
>> performance
>> issues tied to ofproto flow changes, trying to determine deployed traffic
>> patterns, or while debugging dynamic systems where ports come and go.
>>
>> Prior to this change, there was a lack of visibility around flow 
>> expiration.
>> The existing debugging infrastructure could tell us when a flow was 
>> added to
>> the datapath, but not when it was removed or why.
>>
>> This change introduces a USDT probe at the point where the revalidator
>> determines that the flow should be removed.  Additionally, we track the
>> reason for the flow eviction and provide that information as well.  With
>> this change, we can track the complete flow lifecycle for the netlink
>> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, 
>> and
>> the revaldiator USDT, letting us watch as flows are added and removed 
>> from
>> the kernel datapath.
>>
>> This change only enables this information via USDT probe, so it won't be
>> possible to access this information any other way (see:
>> Documentation/topics/usdt-probes.rst).
>>
>> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
>> which serves as a demonstration of how the new USDT probe might be used
>> going forward.
>>
>> Signed-off-by: Kevin Sprague 
>> Co-authored-by: Aaron Conole 
>> Signed-off-by: Aaron Conole 
>
> Thanks for following this up Aaron! See comments on this patch below. I 
> have no additional comments on patch 2.
>
> Cheers,
>
> Eelco
>
>
>> ---
>>  Documentation/topics/usdt-probes.rst |   1 +
>>  ofproto/ofproto-dpif-upcall.c|  42 +-
>>  utilities/automake.mk|   3 +
>>  utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>>  4 files changed, 693 insertions(+), 6 deletions(-)
>>  create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>>
>> diff --git a/Documentation/topics/usdt-probes.rst 
>> b/Documentation/topics/usdt-probes.rst
>> index e527f43bab..a8da9bb1f7 100644
>> --- a/Documentation/topics/usdt-probes.rst
>> +++ b/Documentation/topics/usdt-probes.rst
>> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>>  - dpif_recv:recv_upcall
>>  - main:poll_block
>>  - main:run_start
>> +- revalidate:flow_result
>>  - revalidate_ukey\_\_:entry
>>  - revalidate_ukey\_\_:exit
>>  - udpif_revalidator:start_dump
>
> You are missing the specific flow_result result section. This is from the 
> previous patch:

 D'oh!  Thanks for catching it.  I'll re-add it.

> @@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
>  - ``utilities/usdt-scripts/bridge_loop.bt``
>
>
> +probe revalidate:flow_result
> +~
> +
> +**Description**:
> +This probe is triggered when the revalidator decides whether or not to
> +revalidate a flow. ``reason`` is an enum that denotes that either the 
> flow
> +is being kept, or the reason why the flow is being deleted. The
> +``flow_reval_monitor.py`` script uses this probe to notify users when 
> flows
> +matching user-provided criteria are deleted.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(enum flow_del_reason) reason``
> +- *arg1*: ``(struct udpif *) udpif``
> +- *arg2*: ``(struct udpif_key *) ukey``
> +
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
>  Adding your own probes
>  --
>
>> diff --git a/ofproto/ofproto-dpif-upcall.c 
>> b/ofproto/ofproto-dpif-upcall.c
>> index b5cbeed878..97d75833f7 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -269,6 +269,18 @@ enum ukey_state {
>>  };
>>  #define N_UKEY_STATES (UKEY_DELETED + 1)
>>
>> +enum flow_del_reason {
>> +FDR_REVALIDATE = 0, /* The flow was revalidated. */
>
> It was called FDR_FLOW_LIVE before, which might make more sense. As the 
> flow is just NOT deleted. It might or might not have been revalidated. 
> Thoughts?

 I think it had to have been revalidated if we emit the reason, because
 we only emit the reason code after revalidation.  IE: there are many
 places where we skip revalidation 

[ovs-dev] [PATCH branch-3.2] dpdk: Use DPDK 22.11.4 release for OVS 3.2.

[Kevin Traynor]

Update the CI and docs to use DPDK 22.11.4.

Signed-off-by: Kevin Traynor 
---
 .github/workflows/build-and-test.yml | 2 +-
 Documentation/faq/releases.rst   | 8 
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 725d82d4d..b50c42de6 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -9,5 +9,5 @@ jobs:
   CC: gcc
   DPDK_GIT: https://dpdk.org/git/dpdk-stable
-  DPDK_VER: 22.11.3
+  DPDK_VER: 22.11.4
 name: dpdk gcc
 outputs:
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 362bf4ec7..f47d40836 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -216,8 +216,8 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.5
-3.0.x21.11.5
-3.1.x22.11.3
-3.2.x22.11.3
+2.17.x   21.11.6
+3.0.x21.11.6
+3.1.x22.11.4
+3.2.x22.11.4
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 02eaf8b10..27df48493 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 22.11.3
+- DPDK 22.11.4
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-22.11.3.tar.xz
-   $ tar xf dpdk-22.11.3.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.3
+   $ wget https://fast.dpdk.org/rel/dpdk-22.11.4.tar.xz
+   $ tar xf dpdk-22.11.4.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.4
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index 0c5a20c2c..0387b66d5 100644
--- a/NEWS
+++ b/NEWS
@@ -2,5 +2,5 @@ v3.2.2 - xx xxx 
 
- DPDK:
- * OVS validated with DPDK 22.11.3.
+ * OVS validated with DPDK 22.11.4.
 
 v3.2.1 - 17 Oct 2023
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH master/branch-3.3] faq: Update DPDK releases for older branches.

[Kevin Traynor]

Branches 2.17/3.0/3.1/3.2 are using newer DPDK LTS releases.

Update the faq.

Signed-off-by: Kevin Traynor 
---
 Documentation/faq/releases.rst | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 3a8387f84..49b987b61 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -217,8 +217,8 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.5
-3.0.x21.11.5
-3.1.x22.11.3
-3.2.x22.11.3
+2.17.x   21.11.6
+3.0.x21.11.6
+3.1.x22.11.4
+3.2.x22.11.4
 3.3.x23.11
  
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-3.0] dpdk: Use DPDK 21.11.6 release for OVS 3.0.

[Kevin Traynor]

Update the CI and docs to use DPDK 21.11.6.

Signed-off-by: Kevin Traynor 
---
 .ci/linux-build.sh   | 2 +-
 Documentation/faq/releases.rst   | 4 ++--
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 2df466b0c..7cb183dde 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -229,5 +229,5 @@ fi
 if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="21.11.5"
+DPDK_VER="21.11.6"
 fi
 install_dpdk $DPDK_VER
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 3825e2695..0dd2ec865 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -214,6 +214,6 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.5
-3.0.x21.11.5
+2.17.x   21.11.6
+3.0.x21.11.6
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 559e8eb1f..fff49007e 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 21.11.5
+- DPDK 21.11.6
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz
-   $ tar xf dpdk-21.11.5.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5
+   $ wget https://fast.dpdk.org/rel/dpdk-21.11.6.tar.xz
+   $ tar xf dpdk-21.11.6.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.6
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index 631239bde..2a9a99a8f 100644
--- a/NEWS
+++ b/NEWS
@@ -2,5 +2,5 @@ v3.0.6 - xx xxx 
 
- DPDK:
- * OVS validated with DPDK 21.11.5.
+ * OVS validated with DPDK 21.11.6.
 
 v3.0.5 - 17 Oct 2023
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-3.1] dpdk: Use DPDK 22.11.4 release for OVS 3.1.

[Kevin Traynor]

Update the CI and docs to use DPDK 22.11.4.

Signed-off-by: Kevin Traynor 
---
 .github/workflows/build-and-test.yml | 2 +-
 Documentation/faq/releases.rst   | 6 +++---
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index e19e8b402..ade24abc8 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -9,5 +9,5 @@ jobs:
   CC: gcc
   DPDK_GIT: https://dpdk.org/git/dpdk-stable
-  DPDK_VER: 22.11.3
+  DPDK_VER: 22.11.4
 name: dpdk gcc
 outputs:
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index f956c7e10..a393f78a8 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -215,7 +215,7 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.5
-3.0.x21.11.5
-3.1.x22.11.3
+2.17.x   21.11.6
+3.0.x21.11.6
+3.1.x22.11.4
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 02eaf8b10..27df48493 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 22.11.3
+- DPDK 22.11.4
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-22.11.3.tar.xz
-   $ tar xf dpdk-22.11.3.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.3
+   $ wget https://fast.dpdk.org/rel/dpdk-22.11.4.tar.xz
+   $ tar xf dpdk-22.11.4.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.4
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index b88a37b93..77cf4bbac 100644
--- a/NEWS
+++ b/NEWS
@@ -2,5 +2,5 @@ v3.1.4 - xx xxx 
 
- DPDK:
- * OVS validated with DPDK 22.11.3.
+ * OVS validated with DPDK 22.11.4.
 
 v3.1.3 - 17 Oct 2023
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2 1/3] rbac: Only allow relevant chassis to update service monitors.

[Ales Musil]

On Tue, Jan 30, 2024 at 10:08 PM Mark Michelson  wrote:

> Service monitors already had the restriction that chassis could not
> insert or delete records. However, there was nothing restricting chassis
> from updating records for service monitors that are relevant to other
> chassis.
>
> This change adds a new "chassis_name" column to the Service_Monitor
> table. ovn-northd will set this column to the chassis on which the
> relevant logical port is bound. This way, only that particular chassis
> can update the status of the service monitor.
>
> Signed-off-by: Mark Michelson 
> ---
> v1 -> v2:
> * Rebased on top of currrent main
> ---
>  northd/northd.c | 19 +--
>  northd/ovn-northd.c |  2 +-
>  ovn-sb.ovsschema|  5 +++--
>  ovn-sb.xml  |  4 
>  4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index d2091d4bc..2a2fab231 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3799,13 +3799,19 @@ static struct service_monitor_info *
>  create_or_get_service_mon(struct ovsdb_idl_txn *ovnsb_txn,
>struct hmap *monitor_map,
>const char *ip, const char *logical_port,
> -  uint16_t service_port, const char *protocol)
> +  uint16_t service_port, const char *protocol,
> +  const char *chassis_name)
>  {
>  struct service_monitor_info *mon_info =
>  get_service_mon(monitor_map, ip, logical_port, service_port,
>  protocol);
>
>  if (mon_info) {
> +if (chassis_name && strcmp(mon_info->sbrec_mon->chassis_name,
> +   chassis_name)) {
> +sbrec_service_monitor_set_chassis_name(mon_info->sbrec_mon,
> +   chassis_name);
> +}
>  return mon_info;
>  }
>
> @@ -3820,6 +3826,9 @@ create_or_get_service_mon(struct ovsdb_idl_txn
> *ovnsb_txn,
>  sbrec_service_monitor_set_port(sbrec_mon, service_port);
>  sbrec_service_monitor_set_logical_port(sbrec_mon, logical_port);
>  sbrec_service_monitor_set_protocol(sbrec_mon, protocol);
> +if (chassis_name) {
> +sbrec_service_monitor_set_chassis_name(sbrec_mon, chassis_name);
> +}
>  mon_info = xzalloc(sizeof *mon_info);
>  mon_info->sbrec_mon = sbrec_mon;
>  hmap_insert(monitor_map, _info->hmap_node, hash);
> @@ -3862,12 +3871,18 @@ ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn,
>  protocol = "tcp";
>  }
>
> +const char *chassis_name = NULL;
> +if (op->sb && op->sb->chassis) {
> +chassis_name = op->sb->chassis->name;
> +}
> +
>  struct service_monitor_info *mon_info =
>  create_or_get_service_mon(ovnsb_txn, monitor_map,
>backend->ip_str,
>backend_nb->logical_port,
>backend->port,
> -  protocol);
> +  protocol,
> +  chassis_name);
>  ovs_assert(mon_info);
>  sbrec_service_monitor_set_options(
>  mon_info->sbrec_mon,
> _vip_nb->lb_health_check->options);
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index dadc1af38..c32a11cbd 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -114,7 +114,7 @@ static const char *rbac_mac_binding_update[] =
>  {"logical_port", "ip", "mac", "datapath", "timestamp"};
>
>  static const char *rbac_svc_monitor_auth[] =
> -{""};
> +{"chassis_name"};
>  static const char *rbac_svc_monitor_auth_update[] =
>  {"status"};
>  static const char *rbac_igmp_group_auth[] =
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 72e230b75..1d2b3028d 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Southbound",
> -"version": "20.30.0",
> -"cksum": "2972392849 31172",
> +"version": "20.31.0",
> +"cksum": "2473562445 31224",
>  "tables": {
>  "SB_Global": {
>  "columns": {
> @@ -509,6 +509,7 @@
>  "logical_port": {"type": "string"},
>  "src_mac": {"type": "string"},
>  "src_ip": {"type": "string"},
> +"chassis_name": {"type": "string"},
>  "status": {
>  "type": {"key": {"type": "string",
>   "enum": ["set", ["online", "offline",
> "error"]]},
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index e393f92b3..1f3b318e0 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -4815,6 +4815,10 @@ tcp.flags = RST;
>  Source IPv4 address to use in the service monitor packet.
>
>
> +  
> +The 

Re: [ovs-dev] [PATCH ovn v2] tests: Fix flaky test 'SB Port binding incremental processing'.

[Numan Siddique]

On Fri, Feb 2, 2024 at 5:37 AM Ales Musil  wrote:
>
> On Thu, Feb 1, 2024 at 1:10 AM  wrote:
>
> > From: Numan Siddique 
> >
> > Fixes: 121661678317 ("northd: Move router ports SB PB options sync to
> > sync_to_sb_pb node.")
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >
> > v1 -> v2
> > -
> >* Fix the checkpatch errors.
> >
> >  tests/ovn-northd.at | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 67e81ddba3..fbd0d9ef18 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -10404,6 +10404,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >  # Make lrp a gateway port
> >  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> >  check ovn-nbctl --wait=sb lrp-set-gateway-chassis lrp hv1
> > +wait_column "hosting-chassis=hv1" nb:Logical_Router_Port status name=lrp
> > +
> >  # There will be 3 recomputes of northd engine node
> >  #   1. missing handler for input NB_logical_router
> >  #   2. missing handler for input SB_ha_chassis_group
> > --
> > 2.43.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
> Looks good to me, thanks.
>
> Acked-by: Ales Musil 

Thanks.  I applied to the main.

Numan

>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA 
>
> amu...@redhat.com
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.

[Lorenzo Bianconi]

Add the capbility to mark (through pkt.mark) incoming/outgoing packets
in logical_switch datapath according to user configured QoS rule.

Co-developed-by: Dumitru Ceara 
Reported-at: https://issues.redhat.com/browse/FDP-42
Signed-off-by: Lorenzo Bianconi 
---
Changes since v2:
- set max dscp/mark value in ovn-nb.ovsschema
- add unit-test for new dscp/mark boundaries
Changes since v1:
- move qos packet mark action in QOS_MARK ls {ingress/egress} stage
---
 NEWS  |  2 +
 northd/northd.c   | 33 +---
 northd/ovn-northd.8.xml   |  6 +++
 ovn-nb.ovsschema  |  8 ++--
 ovn-nb.xml| 12 +-
 tests/ovn-nbctl.at|  8 +++-
 tests/ovn-northd.at   | 81 ++
 tests/ovn.at  | 83 +++
 utilities/ovn-nbctl.8.xml |  5 ++-
 utilities/ovn-nbctl.c | 27 ++---
 10 files changed, 245 insertions(+), 20 deletions(-)

diff --git a/NEWS b/NEWS
index 6553bd078..a8beb09fb 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,8 @@ Post v23.09.0
   - Support selecting encapsulation IP based on the source/destination VIF's
 settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
 details.
+  - Add the capability to mark (through pkt.mark) incoming/outgoing packets
+in the logical switch datapath according to user configured QoS rule.
 
 OVN v23.09.0 - 15 Sep 2023
 --
diff --git a/northd/northd.c b/northd/northd.c
index d2091d4bc..a77919af3 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8363,6 +8363,8 @@ build_acls(struct ovn_datapath *od, const struct 
chassis_features *features,
 ds_destroy();
 }
 
+#define QOS_MAX_DSCP 63
+
 static void
 build_qos(struct ovn_datapath *od, struct hmap *lflows) {
 struct ds action = DS_EMPTY_INITIALIZER;
@@ -8376,21 +8378,40 @@ build_qos(struct ovn_datapath *od, struct hmap *lflows) 
{
 struct nbrec_qos *qos = od->nbs->qos_rules[i];
 bool ingress = !strcmp(qos->direction, "from-lport") ? true :false;
 enum ovn_stage stage = ingress ? S_SWITCH_IN_QOS_MARK : 
S_SWITCH_OUT_QOS_MARK;
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
 int64_t rate = 0;
 int64_t burst = 0;
 
+ds_clear();
 for (size_t j = 0; j < qos->n_action; j++) {
+if (strcmp(qos->key_action[j], "dscp") &&
+strcmp(qos->key_action[j], "mark")) {
+continue;
+}
+
 if (!strcmp(qos->key_action[j], "dscp")) {
-ds_clear();
-ds_put_format(, "ip.dscp = %"PRId64"; next;",
+if (qos->value_action[j] > QOS_MAX_DSCP) {
+VLOG_WARN_RL(, "Bad 'dscp' value %"PRId64" in qos "
+  UUID_FMT, qos->value_action[j],
+  UUID_ARGS(>header_.uuid));
+continue;
+}
+
+ds_put_format(, "ip.dscp = %"PRId64"; ",
+  qos->value_action[j]);
+} else if (!strcmp(qos->key_action[j], "mark")) {
+ds_put_format(, "pkt.mark = %"PRId64"; ",
   qos->value_action[j]);
-ovn_lflow_add_with_hint(lflows, od, stage,
-qos->priority,
-qos->match, ds_cstr(),
->header_);
 }
 }
 
+if (action.length) {
+ds_put_cstr(, "next;");
+ovn_lflow_add_with_hint(lflows, od, stage, qos->priority,
+qos->match, ds_cstr(),
+>header_);
+}
+
 for (size_t n = 0; n < qos->n_bandwidth; n++) {
 if (!strcmp(qos->key_bandwidth[n], "rate")) {
 rate = qos->value_bandwidth[n];
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 068d47e1a..9583abeff 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -915,6 +915,12 @@
 QoS table.
   
 
+  
+For every qos_rules entry in a logical switch with packet marking
+enabled, a flow will be added at the priority mentioned in the
+QoS table.
+  
+
   
 One priority-0 fallback flow that matches all packets and advances to
 the next table.
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index b2e0993e0..12ddce3f6 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
-"version": "7.2.0",
-"cksum": "1069338687 34162",
+"version": "7.3.0",
+"cksum": "4237284067 34178",
 "tables": {
 "NB_Global": {
 "columns": {
@@ -293,10 +293,10 @@
 "enum": ["set", ["from-lport", 
"to-lport"]]}}},
 "match": {"type": "string"},
 

Re: [ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.

[Dumitru Ceara]

On 2/2/24 16:41, Lorenzo Bianconi wrote:
> Add the capbility to mark (through pkt.mark) incoming/outgoing packets
> in logical_switch datapath according to user configured QoS rule.
> 
> Co-developed-by: Dumitru Ceara 
> Reported-at: https://issues.redhat.com/browse/FDP-42
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v2:
> - set max dscp/mark value in ovn-nb.ovsschema
> - add unit-test for new dscp/mark boundaries
> Changes since v1:
> - move qos packet mark action in QOS_MARK ls {ingress/egress} stage
> ---
>  NEWS  |  2 +
>  northd/northd.c   | 33 +---
>  northd/ovn-northd.8.xml   |  6 +++
>  ovn-nb.ovsschema  |  8 ++--
>  ovn-nb.xml| 12 +-
>  tests/ovn-nbctl.at|  8 +++-
>  tests/ovn-northd.at   | 81 ++
>  tests/ovn.at  | 83 +++
>  utilities/ovn-nbctl.8.xml |  5 ++-
>  utilities/ovn-nbctl.c | 27 ++---
>  10 files changed, 245 insertions(+), 20 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 6553bd078..a8beb09fb 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -18,6 +18,8 @@ Post v23.09.0
>- Support selecting encapsulation IP based on the source/destination VIF's
>  settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
>  details.
> +  - Add the capability to mark (through pkt.mark) incoming/outgoing packets
> +in the logical switch datapath according to user configured QoS rule.
>  
>  OVN v23.09.0 - 15 Sep 2023
>  --
> diff --git a/northd/northd.c b/northd/northd.c
> index d2091d4bc..a77919af3 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -8363,6 +8363,8 @@ build_acls(struct ovn_datapath *od, const struct 
> chassis_features *features,
>  ds_destroy();
>  }
>  
> +#define QOS_MAX_DSCP 63
> +
>  static void
>  build_qos(struct ovn_datapath *od, struct hmap *lflows) {
>  struct ds action = DS_EMPTY_INITIALIZER;
> @@ -8376,21 +8378,40 @@ build_qos(struct ovn_datapath *od, struct hmap 
> *lflows) {
>  struct nbrec_qos *qos = od->nbs->qos_rules[i];
>  bool ingress = !strcmp(qos->direction, "from-lport") ? true :false;
>  enum ovn_stage stage = ingress ? S_SWITCH_IN_QOS_MARK : 
> S_SWITCH_OUT_QOS_MARK;
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>  int64_t rate = 0;
>  int64_t burst = 0;
>  
> +ds_clear();
>  for (size_t j = 0; j < qos->n_action; j++) {
> +if (strcmp(qos->key_action[j], "dscp") &&
> +strcmp(qos->key_action[j], "mark")) {
> +continue;
> +}
> +

This check seems redundant we recheck the qos->key_action[j] just below.
 Would it be possible to remove it when applying the patch to the main
branch (if the patch is accepted)?

Thanks,
Dumitru

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v6 05/13] northd: Refactor lflow management into a separate module.

[Dumitru Ceara]

On 1/30/24 22:21, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> ovn_lflow_add() and other related functions/macros are now moved
> into a separate module - lflow-mgr.c.  This module maintains a
> table 'struct lflow_table' for the logical flows.  lflow table
> maintains a hmap to store the logical flows.
> 
> It also maintains the logical switch and router dp groups.
> 
> Previous commits which added lflow incremental processing for
> the VIF logical ports, stored the references to
> the logical ports' lflows using 'struct lflow_ref_list'.  This
> struct is renamed to 'struct lflow_ref' and is part of lflow-mgr.c.
> It is  modified a bit to store the resource to lflow references.
> 
> Example usage of 'struct lflow_ref'.
> 
> 'struct ovn_port' maintains 2 instances of lflow_ref.  i,e
> 
> struct ovn_port {
>...
>...
>struct lflow_ref *lflow_ref;
>struct lflow_ref *stateful_lflow_ref;
> };
> 
> All the logical flows generated by
> build_lswitch_and_lrouter_iterate_by_lsp() uses the ovn_port->lflow_ref.
> 
> All the logical flows generated by build_lsp_lflows_for_lbnats()
> uses the ovn_port->stateful_lflow_ref.
> 
> When handling the ovn_port changes incrementally, the lflows referenced
> in 'struct ovn_port' are cleared and regenerated and synced to the
> SB logical flows.
> 
> eg.
> 
> lflow_ref_clear_lflows(op->lflow_ref);
> build_lswitch_and_lrouter_iterate_by_lsp(op, ...);
> lflow_ref_sync_lflows_to_sb(op->lflow_ref, ...);
> 
> This patch does few more changes:
>   -  Logical flows are now hashed without the logical
>  datapaths.  If a logical flow is referenced by just one
>  datapath, we don't rehash it.
> 
>   -  The synthetic 'hash' column of sbrec_logical_flow now
>  doesn't use the logical datapath.  This means that
>  when ovn-northd is updated/upgraded and has this commit,
>  all the logical flows with 'logical_datapath' column
>  set will get deleted and re-added causing some disruptions.
> 
>   -  With the commit [1] which added I-P support for logical
>  port changes, multiple logical flows with same match 'M'
>  and actions 'A' are generated and stored without the
>  dp groups, which was not the case prior to
>  that patch.
>  One example to generate these lflows is:
>  ovn-nbctl lsp-set-addresses sw0p1 "MAC1 IP1"
>  ovn-nbctl lsp-set-addresses sw1p1 "MAC1 IP1"
>ovn-nbctl lsp-set-addresses sw2p1 "MAC1 IP1"
> 
>  Now with this patch we go back to the earlier way.  i.e
>  one logical flow with logical_dp_groups set.
> 
>   -  With this patch any updates to a logical port which
>  doesn't result in new logical flows will not result in
>  deletion and addition of same logical flows.
>  Eg.
>  ovn-nbctl set logical_switch_port sw0p1 external_ids:foo=bar
>  will be a no-op to the SB logical flow table.
> 
> [1] - 8bbd678("northd: Incremental processing of VIF additions in 'lflow' 
> node.")
> 
> Signed-off-by: Numan Siddique 
> ---

[...]

> +
> +/* Logical flow sync using 'struct lflow_ref'
> + * ==
> + * The 'struct lflow_ref' represents a collection of (or references to)
> + * logical flows (struct ovn_lflow) which belong to a logical entity 'E'.
> + * This entity 'E' is external to lflow manager (see northd.h and northd.c)
> + * Eg. logical datapath (struct ovn_datapath), logical switch and router 
> ports
> + * (struct ovn_port), load balancer (struct lb_datapath) etc.
> + *
> + * General guidelines on using 'struct lflow_ref'.
> + *   - For an entity 'E', create an instance of lflow_ref
> + *   E->lflow_ref = lflow_ref_create();
> + *
> + *   - For each logical flow L(M, A) generated for the entity 'E'
> + * pass E->lflow_ref when adding L(M, A) to the lflow table.
> + * Eg. lflow_table_add_lflow(lflow_table, od_of_E, M, A, .., 
> E->lflow_ref);
> + *
> + * If lflows L1, L2 and L3 are generated for 'E', then
> + * E->lflow_ref stores these in its hmap.
> + * i.e E->lflow_ref->lflow_ref_nodes = hmap[LRN(L1, E1), LRN(L2, E1),
> + *  LRN(L3, E1)]
> + *
> + * LRN is an instance of 'struct lflow_ref_node'.
> + * 'struct lflow_ref_node' is used to store a logical lflow L(M, A) as a
> + * reference in the lflow_ref.  It is possible that an lflow L(M,A) can be
> + * referenced by one or more lflow_ref's.  For each reference, an instance of
> + * this struct 'lflow_ref_node' is created.
> + *
> + * For example, if entity E1 generates lflows L1, L2 and L3
> + * and entity E2 generates lflows L1, L3, and L4 then
> + * an instance of this struct is created for each entity.
> + * For example LRN(L1, E1).
> + *
> + * Each logical flow's L also maintains a list of its references in the
> + * ovn_lflow->referenced_by list.
> + *
> + *
> + *
> + *L1L2 L3 L4
> + *| |  (list)  |  |
> + *   

Re: [ovs-dev] [PATCH ovn v6 08/13] northd: Handle lb changes in lflow engine.

[Dumitru Ceara]

On 1/30/24 22:22, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Since northd tracked data has the changed lb information,
> the lflow change handler for northd inputs can now handle
> lb updates incrementally.  All the lflows generated for
> each lb is stored in the ovn_lb_datapaths->lflow_ref and
> this is used similar to how we handle ovn_port changes.
> 
> Acked-by: Han Zhou 
> Signed-off-by: Numan Siddique 
> ---

Acked-by: Dumitru Ceara 

Thanks!

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v6 13/13] northd: Add I-P for NB_Global and SB_Global.

[Dumitru Ceara]

On 1/30/24 22:23, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> A new engine node "global_config" is added which handles the changes
> to NB_Global an SB_Global tables.  It also creates these rows if
> not present.
> 
> Without the I-P, any changes to the options column of these tables
> result in recompute of 'northd' and 'lflow' engine nodes.
> 
> Acked-by: Dumitru Ceara 
> Acked-by: Han Zhou 
> Signed-off-by: Numan Siddique 
> ---

[...]

> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 929bb45aed..d8ec3eead5 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -8838,7 +8838,7 @@ AT_CHECK([grep "ls_in_lb " S1flows | sed 
> 's/table=../table=??/' | sort], [0], [d
>table=??(ls_in_lb   ), priority=0, match=(1), action=(next;)
>  ])
>  
> -ovn-nbctl --wait=sb set NB_Global . options:install_ls_lb_from_router=true
> +check ovn-nbctl --wait=sb set NB_Global . 
> options:install_ls_lb_from_router=true
>  
>  ovn-sbctl dump-flows S0 > S0flows
>  ovn-sbctl dump-flows S1 > S1flows
> @@ -8857,6 +8857,7 @@ AT_CHECK([grep "ls_in_lb " S1flows | sed 
> 's/table=../table=??/' | sort], [0], [d
>table=??(ls_in_lb   ), priority=120  , match=(ct.new && ip4.dst == 
> 172.16.0.11 && tcp.dst == 8080), action=(reg0[[1]] = 0; 
> ct_lb_mark(backends=10.0.0.2:8080);)
>  ])
>  
> +

Nit: unrelated.

With that addressed, my ack from v5 stands.

Regards,
Dumitru

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v3] ovn-ic: Fix global blacklist filter for IPv6 addresses.

[Roberto Bartzen Acosta via dev]

Thanks for your review Ales.

On Fri, 2 Feb 2024 at 07:34 Ales Musil  wrote:

>
>
> On Tue, Jan 30, 2024 at 3:10 PM Roberto Bartzen Acosta via dev <
> ovs-dev@openvswitch.org> wrote:
>
>> This commit fixes the prefix filter function as the return condition for
>> IPv6 addresses is disabling the advertisement of all learned prefixes
>> regardless of the match with the blacklist or not.
>>
>> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
>> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
>> Signed-off-by: Roberto Bartzen Acosta 
>> ---
>>
>
> Hi Roberto,
>
> thank you for the patch.
>
>
>>  ic/ovn-ic.c |  22 ---
>>  tests/ovn-ic.at | 100 
>>  2 files changed, 116 insertions(+), 6 deletions(-)
>>
>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
>> index 6f8f5734d..1c9c9ae2c 100644
>> --- a/ic/ovn-ic.c
>> +++ b/ic/ovn-ic.c
>> @@ -1024,6 +1024,20 @@ prefix_is_link_local(struct in6_addr *prefix,
>> unsigned int plen)
>>  ((prefix->s6_addr[1] & 0xc0) == 0x80));
>>  }
>>
>> +static bool
>> +compare_ipv6_prefixes(const struct in6_addr *s_prefix,
>> +  const struct in6_addr *d_prefix2, int plen)
>> +{
>> +struct in6_addr mask = ipv6_create_mask(plen);
>> +for (int i = 0; i <= (plen / 8); i++) {
>> +if ((s_prefix->s6_addr[i] & mask.s6_addr[i]) ^
>> +(d_prefix2->s6_addr[i] & mask.s6_addr[i])) {
>> +return false;
>> +}
>> +}
>> +return true;
>> +}
>> +
>>
>  static bool
>>  prefix_is_black_listed(const struct smap *nb_options,
>> struct in6_addr *prefix,
>> @@ -1064,12 +1078,8 @@ prefix_is_black_listed(const struct smap
>> *nb_options,
>>  continue;
>>  }
>>  } else {
>> -struct in6_addr mask = ipv6_create_mask(bl_plen);
>> -for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
>> -if ((prefix->s6_addr[i] & mask.s6_addr[i])
>> -!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
>> -continue;
>> -}
>> +if (!compare_ipv6_prefixes(prefix, _prefix, bl_plen)) {
>> +continue;
>>  }
>>
>
>
> There is no need for hand implementation, OvS provides plenty of helpers.
> This can be replaced with something like the snippet below:
>
> struct in6_addr mask = ipv6_create_mask(bl_plen);
> struct in6_addr m_prefix = ipv6_addr_bitand(prefix, );
> struct in6_addr m_bl_prefix = ipv6_addr_bitand(_prefix, );
>
> if (!ipv6_addr_equals(_prefix, _bl_prefix)) {
> continue;
> }
>

Sure, this makes sense. I will change it and send a new patch version.
Thanks


>
>>  }
>>  matched = true;
>> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
>> index d4c436f84..1f9df71e9 100644
>> --- a/tests/ovn-ic.at
>> +++ b/tests/ovn-ic.at
>> @@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2])
>>
>>  AT_CLEANUP
>>  ])
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
>> +AT_KEYWORDS([IPv6-route-sync-blacklist])
>> +
>> +ovn_init_ic_db
>> +ovn-ic-nbctl ts-add ts1
>> +
>> +for i in 1 2; do
>> +ovn_start az$i
>> +ovn_as az$i
>> +
>> +# Enable route learning at AZ level
>> +ovn-nbctl set nb_global . options:ic-route-learn=true
>> +# Enable route advertising at AZ level
>> +ovn-nbctl set nb_global . options:ic-route-adv=true
>> +# Enable blacklist single filter for IPv6
>> +ovn-nbctl set nb_global .
>> options:ic-route-blacklist="2003:db8:1::/64,\
>> +2004:::/32,2005:1234::/21"
>> +
>> +OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
>> +
>> +# Create LRP and connect to TS
>> +ovn-nbctl lr-add lr$i
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i
>> 2001:db8:1::$i/64
>> +ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
>> +-- lsp-set-addresses lsp-ts1-lr$i router \
>> +-- lsp-set-type lsp-ts1-lr$i router \
>> +-- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
>> +
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i
>> 2002:db8:1::$i/64
>> +
>> +# Create blacklisted LRPs and connect to TS
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
>> +11:11:11:11:11:1$i 2003:db8:1::$i/64
>> +
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
>> +22:22:22:22:22:2$i 2004::bbb::$i/48
>> +
>> +# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
>> +33:33:33:33:33:3$i 2005:1734:5678::$i/50
>> +
>> +# additional not filtered prefix -> different subnet bits
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \
>> +33:33:33:33:33:3$i 2005:1834:5678::$i/50
>> +
>> +done
>> +
>> +for i in 1 2; do
>> +OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep
>> learned])
>> +done
>> +
>> +AT_CHECK([ovn_as az1 ovn-nbctl 

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Eelco Chaudron]

On 2 Feb 2024, at 15:16, Aaron Conole wrote:



>>> +OVS_USDT_PROBE(revalidate, flow_result, reason, udpif, 
>>> ukey);
>>>
>>> I have been experimenting with several upcall tracking techniques
>> that would make it easier to correlate upcalls with their subsequent
>> related events.
>>> To achieve that, we need (among other things) some easy-to-compare
>> unique value in the events. For revalidation events, I think a good
>> candidate would be "ukey->ufid" and so does the script in this patch.
>>>
>>> However, requiring all external tools to know the layout of "struct
>> udpif_key" in order to get that value makes things quite complicated
>> for CORE tools (e.g: retis).
>>>
>>> With all this, would you consider adding the ufid to probe payload directly?
>>
>> Dont see why we can not, but if we need anything else it would quickly
>> explode in too much arguments. I guess CORE needs a good solution for
>> userspace.
>
> I think having the ufid and udpif makes sense for now.  Actually, maybe
> for the long term we can do something as an appctl command which will
> dump the relevant datastructures.  That way a tool like this, which
> needs the pid anyway, can generate an appctl command against the daemon
> and get the exact right header layout.
>
> WDYT?

I think we discussed this in the past, but I can’t remember what the arguments 
against this approach were. Adrian, can you remember?

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v6 09/13] northd: Add lr_stateful handler for lflow engine node.

[Dumitru Ceara]

On 1/30/24 22:22, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Acked-by: Han Zhou 
> Signed-off-by: Numan Siddique 
> ---

Acked-by: Dumitru Ceara 

Thanks!

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.17] dpdk: Use DPDK 21.11.6 release for OVS 2.17.

[Kevin Traynor]

Update the CI and docs to use DPDK 21.11.6.

Signed-off-by: Kevin Traynor 
---
 .ci/linux-build.sh   | 2 +-
 Documentation/faq/releases.rst   | 2 +-
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 9464ea49c..2e1bf2588 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -221,5 +221,5 @@ fi
 if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="21.11.5"
+DPDK_VER="21.11.6"
 fi
 install_dpdk $DPDK_VER
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 0e0c589a3..a9045a2d5 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -211,5 +211,5 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.5
+2.17.x   21.11.6
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 559e8eb1f..fff49007e 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 21.11.5
+- DPDK 21.11.6
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz
-   $ tar xf dpdk-21.11.5.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5
+   $ wget https://fast.dpdk.org/rel/dpdk-21.11.6.tar.xz
+   $ tar xf dpdk-21.11.6.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.6
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index 3e3e5a115..7ef97cfa8 100644
--- a/NEWS
+++ b/NEWS
@@ -2,5 +2,5 @@ v2.17.9 - xx xxx 
 -
- DPDK:
- * OVS validated with DPDK 21.11.5.
+ * OVS validated with DPDK 21.11.6.
 
 v2.17.8 - 17 Oct 2023
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2 2/3] rbac: Restrict IGMP_Group updates to relevant chassis.

[Ales Musil]

On Tue, Jan 30, 2024 at 10:44 PM Mark Michelson  wrote:

> On 1/30/24 16:08, Mark Michelson wrote:
> > RBAC did not restrict which chassis could update IGMP_Groups. With this
> > change, we add a new "chassis_name" column to IGMP_Group.
> >
> > This may seem odd since there is already a "chassis" column in
> > IGMP_Group. But RBAC specifically works by string matching based on the
> > certificate common name. Therefore, we need to have a chassis_name
> > string column instead of a chassis UUID column.
> >
> > Getting RBAC to function properly required me to fix an existing bug as
> > well. igmp_group_cleanup() did not ensure that only local IGMP group
> > records were deleted. This presumably meant that when one ovn-controller
> > in a cluster was shut down, it would delete ALL IGMP_Group records in
> > the southbound DB, not just the local ones.
> >
> > Signed-off-by: Mark Michelson 
> > ---
> > v1 -> v2:
> >  * Rebased on top of current main
> >  * Fixed igmp_group_cleanup() to only delete local records.
> > ---
> >   controller/ip-mcast.c   | 26 +++---
> >   controller/ip-mcast.h   |  9 ++---
> >   controller/ovn-controller.c |  3 ++-
> >   controller/pinctrl.c| 16 +---
> >   northd/ovn-northd.c |  2 +-
> >   ovn-sb.ovsschema|  7 ---
> >   ovn-sb.xml  |  5 +
> >   tests/ovn.at|  2 +-
> >   8 files changed, 51 insertions(+), 19 deletions(-)
> >
> > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
> > index a870fb29e..b457c7e69 100644
> > --- a/controller/ip-mcast.c
> > +++ b/controller/ip-mcast.c
> > @@ -38,7 +38,8 @@ static struct sbrec_igmp_group *
> >   igmp_group_create_(struct ovsdb_idl_txn *idl_txn,
> >  const char *addr_str,
> >  const struct sbrec_datapath_binding *datapath,
> > -   const struct sbrec_chassis *chassis);
> > +   const struct sbrec_chassis *chassis,
> > +   bool igmp_group_has_chassis_name);
> >
> >   struct ovsdb_idl_index *
> >   igmp_group_index_create(struct ovsdb_idl *idl)
> > @@ -86,7 +87,8 @@ struct sbrec_igmp_group *
> >   igmp_group_create(struct ovsdb_idl_txn *idl_txn,
> > const struct in6_addr *address,
> > const struct sbrec_datapath_binding *datapath,
> > -  const struct sbrec_chassis *chassis)
> > +  const struct sbrec_chassis *chassis,
> > +  bool igmp_group_has_chassis_name)
> >   {
> >   char addr_str[INET6_ADDRSTRLEN];
> >
> > @@ -94,16 +96,18 @@ igmp_group_create(struct ovsdb_idl_txn *idl_txn,
> >   return NULL;
> >   }
> >
> > -return igmp_group_create_(idl_txn, addr_str, datapath, chassis);
> > +return igmp_group_create_(idl_txn, addr_str, datapath, chassis,
> > +  igmp_group_has_chassis_name);
> >   }
> >
> >   struct sbrec_igmp_group *
> >   igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn,
> >   const struct sbrec_datapath_binding *datapath,
> > -const struct sbrec_chassis *chassis)
> > +const struct sbrec_chassis *chassis,
> > +bool igmp_group_has_chassis_name)
> >   {
> >   return igmp_group_create_(idl_txn, OVN_IGMP_GROUP_MROUTERS,
> datapath,
> > -  chassis);
> > +  chassis, igmp_group_has_chassis_name);
> >   }
> >
> >   void
> > @@ -211,7 +215,8 @@ igmp_group_delete(const struct sbrec_igmp_group *g)
> >
> >   bool
> >   igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > -   struct ovsdb_idl_index *igmp_groups)
> > +   struct ovsdb_idl_index *igmp_groups,
> > +   const struct sbrec_chassis *chassis)
> >   {
> >   const struct sbrec_igmp_group *g;
> >
> > @@ -220,6 +225,9 @@ igmp_group_cleanup(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> >   }
> >
> >   SBREC_IGMP_GROUP_FOR_EACH_BYINDEX (g, igmp_groups) {
> > +if (chassis != g->chassis) {
> > +continue;
> > +}
> >   igmp_group_delete(g);
> >   }
> >
> > @@ -249,13 +257,17 @@ static struct sbrec_igmp_group *
> >   igmp_group_create_(struct ovsdb_idl_txn *idl_txn,
> >  const char *addr_str,
> >  const struct sbrec_datapath_binding *datapath,
> > -   const struct sbrec_chassis *chassis)
> > +   const struct sbrec_chassis *chassis,
> > +   bool igmp_group_has_chassis_name)
> >   {
> >   struct sbrec_igmp_group *g = sbrec_igmp_group_insert(idl_txn);
> >
> >   sbrec_igmp_group_set_address(g, addr_str);
> >   sbrec_igmp_group_set_datapath(g, datapath);
> >   sbrec_igmp_group_set_chassis(g, chassis);
> > +if (igmp_group_has_chassis_name) {
> > +sbrec_igmp_group_set_chassis_name(g, chassis->name);
> > +

Re: [ovs-dev] [PATCH ovn v2 3/3] rbac: Only allow relevant chassis to update BFD.

[Ales Musil]

On Tue, Jan 30, 2024 at 10:08 PM Mark Michelson  wrote:

> This adds a new "chassis_name" column to the BFD table. ovn-northd sets
> this to the logical port's chassis name when creating the BFD record.
> RBAC has been updated so that chassis may only update their own records.
>
> Signed-off-by: Mark Michelson 
> ---
> v1 -> v2:
> * Rebased on current main
> ---
>  northd/northd.c | 9 -
>  northd/ovn-northd.c | 2 +-
>  ovn-sb.ovsschema| 7 ---
>  ovn-sb.xml  | 4 
>  4 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 2a2fab231..51622c302 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -10912,6 +10912,7 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
>  nbrec_bfd_set_status(nb_bt, "admin_down");
>  }
>
> +struct ovn_port *op = ovn_port_find(lr_ports,
> nb_bt->logical_port);
>  bfd_e = bfd_port_lookup(_only, nb_bt->logical_port,
> nb_bt->dst_ip);
>  if (!bfd_e) {
>  int udp_src = bfd_get_unused_port(bfd_src_ports);
> @@ -10925,6 +10926,9 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
>  sbrec_bfd_set_disc(sb_bt, 1 + random_uint32());
>  sbrec_bfd_set_src_port(sb_bt, udp_src);
>  sbrec_bfd_set_status(sb_bt, nb_bt->status);
> +if (op && op->sb && op->sb->chassis) {
> +sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name);
> +}
>
>  int min_tx = nb_bt->n_min_tx ? nb_bt->min_tx[0] :
> BFD_DEF_MINTX;
>  sbrec_bfd_set_min_tx(sb_bt, min_tx);
> @@ -10943,6 +10947,10 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
>  }
>  }
>  build_bfd_update_sb_conf(nb_bt, bfd_e->sb_bt);
> +if (op && op->sb && op->sb->chassis &&
> +strcmp(op->sb->chassis->name, sb_bt->chassis_name)) {
> +sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name);
> +}
>
>  hmap_remove(_only, _e->hmap_node);
>  bfd_e->ref = false;
> @@ -10951,7 +10959,6 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
>  hmap_insert(bfd_connections, _e->hmap_node, hash);
>  }
>
> -struct ovn_port *op = ovn_port_find(lr_ports,
> nb_bt->logical_port);
>  if (op) {
>  op->has_bfd = true;
>  }
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 90a6d62b1..fdd5939e5 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -122,7 +122,7 @@ static const char *rbac_igmp_group_auth[] =
>  static const char *rbac_igmp_group_update[] =
>  {"address", "chassis", "datapath", "ports"};
>  static const char *rbac_bfd_auth[] =
> -{""};
> +{"chassis_name"};
>  static const char *rbac_bfd_update[] =
>  {"status"};
>
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index b42f18b04..84ae09515 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Southbound",
> -"version": "20.32.0",
> -"cksum": "1262133774 31276",
> +"version": "20.33.0",
> +"cksum": "4076371179 31328",
>  "tables": {
>  "SB_Global": {
>  "columns": {
> @@ -578,7 +578,8 @@
>   "min": 0, "max": "unlimited"}},
>  "options": {
>  "type": {"key": "string", "value": "string",
> - "min": 0, "max": "unlimited"}}},
> + "min": 0, "max": "unlimited"}},
> +"chassis_name": {"type": "string"}},
>  "indexes": [["logical_port", "dst_ip", "src_port", "disc"]],
>  "isRoot": true},
>  "FDB": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 2de7228e7..1b18a27a0 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -4989,6 +4989,10 @@ tcp.flags = RST;
>  receiving system in Asynchronous mode.
>
>
> +  
> +The name of the chassis where the logical port is bound.
> +  
> +
>
>  Reserved for future use.
>
> --
> 2.43.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.com

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 ovn 3/3] northd: Add BFD support for ECMP route policy.

[Lorenzo Bianconi]

Similar to OVN static routes, introduce the capability to link a BFD
session for OVN reroute policies.

Reported-at: https://issues.redhat.com/browse/FDP-234
Signed-off-by: Lorenzo Bianconi 
---
 NEWS  |  1 +
 northd/northd.c   | 86 ++
 ovn-nb.ovsschema  |  9 +++-
 ovn-nb.xml|  7 +++
 tests/ovn-nbctl.at|  6 +++
 tests/ovn-northd.at   | 20 +++-
 tests/system-ovn.at   | 51 ++--
 utilities/ovn-nbctl.8.xml |  8 
 utilities/ovn-nbctl.c | 97 ++-
 9 files changed, 267 insertions(+), 18 deletions(-)

diff --git a/NEWS b/NEWS
index 6553bd078..d4227ac99 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,7 @@ Post v23.09.0
   - Support selecting encapsulation IP based on the source/destination VIF's
 settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
 details.
+  - Introduce next-hop BFD availability check for OVN reroute policies.
 
 OVN v23.09.0 - 15 Sep 2023
 --
diff --git a/northd/northd.c b/northd/northd.c
index d2091d4bc..ae50e4ff9 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10991,10 +10991,63 @@ get_outport_for_routing_policy_nexthop(struct 
ovn_datapath *od,
 return NULL;
 }
 
+static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER;
+
+static bool check_bfd_state(
+const struct nbrec_logical_router_policy *rule,
+const struct hmap *bfd_connections,
+struct ovn_port *out_port,
+const char *nexthop)
+{
+struct in6_addr nexthop_v6;
+bool is_nexthop_v6 = ipv6_parse(nexthop, _v6);
+bool ret = true;
+
+for (size_t i = 0; i < rule->n_bfd_sessions; i++) {
+/* Check if there is a BFD session associated to the reroute
+ * policy. */
+const struct nbrec_bfd *nb_bt = rule->bfd_sessions[i];
+struct in6_addr dst_ipv6;
+bool is_dst_v6 = ipv6_parse(nb_bt->dst_ip, _ipv6);
+
+if (is_nexthop_v6 ^ is_dst_v6) {
+continue;
+}
+
+if ((is_nexthop_v6 && !ipv6_addr_equals(_v6, _ipv6)) ||
+strcmp(nb_bt->dst_ip, nexthop)) {
+continue;
+}
+
+if (strcmp(nb_bt->logical_port, out_port->key)) {
+continue;
+}
+
+struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections,
+  nb_bt->logical_port,
+  nb_bt->dst_ip);
+ovs_mutex_lock(_lock);
+if (bfd_e) {
+bfd_e->ref = true;
+}
+
+if (!strcmp(nb_bt->status, "admin_down")) {
+nbrec_bfd_set_status(nb_bt, "down");
+}
+
+ret = strcmp(nb_bt->status, "down");
+ovs_mutex_unlock(_lock);
+break;
+}
+
+return ret;
+}
+
 static void
 build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od,
   const struct hmap *lr_ports,
   const struct nbrec_logical_router_policy *rule,
+  const struct hmap *bfd_connections,
   const struct ovsdb_idl_row *stage_hint)
 {
 struct ds match = DS_EMPTY_INITIALIZER;
@@ -11019,6 +11072,11 @@ build_routing_policy_flow(struct hmap *lflows, struct 
ovn_datapath *od,
  rule->priority, nexthop);
 return;
 }
+
+if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) {
+return;
+}
+
 uint32_t pkt_mark = smap_get_uint(>options, "pkt_mark", 0);
 if (pkt_mark) {
 ds_put_format(, "pkt.mark = %u; ", pkt_mark);
@@ -11060,6 +8,7 @@ static void
 build_ecmp_routing_policy_flows(struct hmap *lflows, struct ovn_datapath *od,
 const struct hmap *lr_ports,
 const struct nbrec_logical_router_policy *rule,
+const struct hmap *bfd_connections,
 uint16_t ecmp_group_id)
 {
 ovs_assert(rule->n_nexthops > 1);
@@ -11088,6 +11147,9 @@ build_ecmp_routing_policy_flows(struct hmap *lflows, 
struct ovn_datapath *od,
 struct ds match = DS_EMPTY_INITIALIZER;
 struct ds actions = DS_EMPTY_INITIALIZER;
 
+size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops);
+size_t n_valid_nexthops = 0;
+
 for (size_t i = 0; i < rule->n_nexthops; i++) {
 struct ovn_port *out_port = get_outport_for_routing_policy_nexthop(
  od, lr_ports, rule->priority, rule->nexthops[i]);
@@ -11105,6 +11167,13 @@ build_ecmp_routing_policy_flows(struct hmap *lflows, 
struct ovn_datapath *od,
 goto cleanup;
 }
 
+if (!check_bfd_state(rule, bfd_connections, out_port,
+ rule->nexthops[i])) {
+continue;
+}
+
+valid_nexthops[n_valid_nexthops++] = i + 1;
+
 

[ovs-dev] [PATCH v2 ovn 2/3] ovn-nbctl: Fix nbctl_pre_lr_route_add for BFD.

[Lorenzo Bianconi]

In order to properly check if we already have a running BFD session for
the next hop used by the brand new route, load logical port column of
the nb BFD table in nbctl_pre_lr_route_add routine.

Acked-by: Mark Michelson 
Fixes: db6d30783bec ("ovn-nbctl: Don't replicate entire database 
unnecessarily.")
Signed-off-by: Lorenzo Bianconi 
---
 utilities/ovn-nbctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 526369b68..0586eccdb 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4394,6 +4394,7 @@ nbctl_pre_lr_route_add(struct ctl_context *ctx)
 ovsdb_idl_add_column(ctx->idl, _logical_router_port_col_name);
 
 ovsdb_idl_add_column(ctx->idl, _bfd_col_dst_ip);
+ovsdb_idl_add_column(ctx->idl, _bfd_col_logical_port);
 
 ovsdb_idl_add_column(ctx->idl,
  _logical_router_static_route_col_ip_prefix);
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 ovn 0/3] Introduce BFD support for ECMP route policy

[Lorenzo Bianconi]

Changes since v1:
- improve ipv6 address lookup

Lorenzo Bianconi (3):
  test: Fix false positive in BFD system test.
  ovn-nbctl: Fix nbctl_pre_lr_route_add for BFD.
  northd: Add BFD support for ECMP route policy.

 NEWS  |  1 +
 northd/northd.c   | 86 ++
 ovn-nb.ovsschema  |  9 +++-
 ovn-nb.xml|  7 +++
 tests/ovn-nbctl.at|  6 +++
 tests/ovn-northd.at   | 20 +++-
 tests/system-ovn.at   | 53 ++---
 utilities/ovn-nbctl.8.xml |  8 
 utilities/ovn-nbctl.c | 98 ++-
 9 files changed, 269 insertions(+), 19 deletions(-)

-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 ovn 1/3] test: Fix false positive in BFD system test.

[Lorenzo Bianconi]

Fix regression introduce in BFD system tests

Acked-by: Mark Michelson 
Fixes: 1655a6c146ca ("northd, utils: support for RouteTables in LRs")
Signed-off-by: Lorenzo Bianconi 
---
 tests/system-ovn.at | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index cb4124b70..9ecb29cdb 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6831,7 +6831,7 @@ route_uuid=$(fetch_column nb:logical_router_static_route 
_uuid ip_prefix="100.0.
 check ovn-nbctl --wait=hv sync
 
 wait_column "up" nb:bfd status logical_port=rp-public
-OVS_WAIT_UNTIL([ovn-sbctl dump-flows R1 | grep 'match=(ip4.dst == 
100.0.0.0/8)' | grep -q 172.16.1.50])
+OVS_WAIT_UNTIL([ovn-sbctl dump-flows R1 |grep lr_in_ip_routing |grep 'ip4.dst 
== 100.0.0.0/8' |grep -q 172.16.1.50])
 
 # un-associate the bfd connection and the static route
 check ovn-nbctl clear logical_router_static_route $route_uuid bfd
@@ -6861,7 +6861,7 @@ stopping
 ])
 
 wait_column "down" nb:bfd status logical_port=rp-public
-OVS_WAIT_UNTIL([test "$(ovn-sbctl dump-flows R1 | grep 'match=(ip4.dst == 
100.0.0.0/8)' | grep 172.16.1.50)" = ""])
+OVS_WAIT_UNTIL([test "$(ovn-sbctl dump-flows R1 |grep lr_in_ip_routing |grep 
'ip4.dst == 100.0.0.0/8' |grep 172.16.1.50)" = ""])
 
 # remove bfd entry
 ovn-nbctl destroy bfd $uuid
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Aaron Conole]

Eelco Chaudron  writes:

> On 2 Feb 2024, at 11:31, Adrian Moreno wrote:
>
>> On 2/1/24 10:02, Eelco Chaudron wrote:
>>>
>>>
>>> On 31 Jan 2024, at 18:03, Aaron Conole wrote:
>>>
 Eelco Chaudron  writes:

> On 25 Jan 2024, at 21:55, Aaron Conole wrote:
>
>> From: Kevin Sprague 
>>
>> During normal operations, it is useful to understand when a particular 
>> flow
>> gets removed from the system. This can be useful when debugging 
>> performance
>> issues tied to ofproto flow changes, trying to determine deployed traffic
>> patterns, or while debugging dynamic systems where ports come and go.
>>
>> Prior to this change, there was a lack of visibility around flow 
>> expiration.
>> The existing debugging infrastructure could tell us when a flow was 
>> added to
>> the datapath, but not when it was removed or why.
>>
>> This change introduces a USDT probe at the point where the revalidator
>> determines that the flow should be removed.  Additionally, we track the
>> reason for the flow eviction and provide that information as well.  With
>> this change, we can track the complete flow lifecycle for the netlink
>> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, 
>> and
>> the revaldiator USDT, letting us watch as flows are added and removed 
>> from
>> the kernel datapath.
>>
>> This change only enables this information via USDT probe, so it won't be
>> possible to access this information any other way (see:
>> Documentation/topics/usdt-probes.rst).
>>
>> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
>> which serves as a demonstration of how the new USDT probe might be used
>> going forward.
>>
>> Signed-off-by: Kevin Sprague 
>> Co-authored-by: Aaron Conole 
>> Signed-off-by: Aaron Conole 
>
> Thanks for following this up Aaron! See comments on this patch
> below. I have no additional comments on patch 2.
>
> Cheers,
>
> Eelco
>
>
>> ---
>>   Documentation/topics/usdt-probes.rst |   1 +
>>   ofproto/ofproto-dpif-upcall.c|  42 +-
>>   utilities/automake.mk|   3 +
>>   utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>>   4 files changed, 693 insertions(+), 6 deletions(-)
>>   create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>>
>> diff --git a/Documentation/topics/usdt-probes.rst 
>> b/Documentation/topics/usdt-probes.rst
>> index e527f43bab..a8da9bb1f7 100644
>> --- a/Documentation/topics/usdt-probes.rst
>> +++ b/Documentation/topics/usdt-probes.rst
>> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>>   - dpif_recv:recv_upcall
>>   - main:poll_block
>>   - main:run_start
>> +- revalidate:flow_result
>>   - revalidate_ukey\_\_:entry
>>   - revalidate_ukey\_\_:exit
>>   - udpif_revalidator:start_dump
>
> You are missing the specific flow_result result section. This is from the 
> previous patch:

 D'oh!  Thanks for catching it.  I'll re-add it.

> @@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
>   - ``utilities/usdt-scripts/bridge_loop.bt``
>
>
> +probe revalidate:flow_result
> +~
> +
> +**Description**:
> +This probe is triggered when the revalidator decides whether or not to
> +revalidate a flow. ``reason`` is an enum that denotes that either the 
> flow
> +is being kept, or the reason why the flow is being deleted. The
> +``flow_reval_monitor.py`` script uses this probe to notify users when 
> flows
> +matching user-provided criteria are deleted.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(enum flow_del_reason) reason``
> +- *arg1*: ``(struct udpif *) udpif``
> +- *arg2*: ``(struct udpif_key *) ukey``
> +
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
>   Adding your own probes
>   --
>
>> diff --git a/ofproto/ofproto-dpif-upcall.c 
>> b/ofproto/ofproto-dpif-upcall.c
>> index b5cbeed878..97d75833f7 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -269,6 +269,18 @@ enum ukey_state {
>>   };
>>   #define N_UKEY_STATES (UKEY_DELETED + 1)
>>
>> +enum flow_del_reason {
>> +FDR_REVALIDATE = 0, /* The flow was revalidated. */
>
> It was called FDR_FLOW_LIVE before, which might make more
> sense. As the flow is just NOT deleted. It might or might not have
> been revalidated. Thoughts?

 I think it had to have been revalidated if we emit the reason, because
 we only emit the reason code after revalidation.  IE: there are many
 places 

Re: [ovs-dev] [PATCH ovn v3 1/2] actions: Adjust the ct_commit_nat action.

[Dumitru Ceara]

On 1/29/24 07:20, Ales Musil wrote:
> The ct_commit nat was hardcoded to use DNAT zone
> in router pipeline. Extend it that it accepts
> two new arguments (snat/dnat) which will determine
> the zone for router pipeline. The switch pipeline
> has only one, so it resolves to the same for both arguments.
> 
> In order to keep backward compatibility the ct_commit_nat
> without any arguments is the same as ct_commit_nat(dnat).
> 
> Signed-off-by: Ales Musil 
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.

[0-day Robot]

Bleep bloop.  Greetings Lorenzo Bianconi, 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 286 characters long (recommended limit is 79)
#402 FILE: utilities/ovn-nbctl.8.xml:466:
[--may-exist] qos-add switch 
direction priority match 
[mark=mark] [dscp=dscp] 
[rate=rate [burst=burst]]

WARNING: Line is 94 characters long (recommended limit is 79)
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#425 FILE: utilities/ovn-nbctl.c:286:
  qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP] 
[mark=MARK]\n\

Lines checked: 505, Warnings: 5, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH 00/10] Add flow visualization utility

[Eelco Chaudron]

On 2 Feb 2024, at 11:55, Adrian Moreno wrote:

> On 1/30/24 16:45, Eelco Chaudron wrote:
>>
>>
>> On 1 Dec 2023, at 20:14, Adrian Moreno wrote:
>>
>>> This series introduces a python utility called ovs-flowviz.
>>>
>>> The goal of this utility is to read both datapath and Openflow flows
>>> (using the flow library already available) and print them in different
>>> formats and styles to make it easier to understand them and troubleshoot
>>> issues.
>>>
>>> The formats are quite opinionated and so are the colors chosen so I'm
>>> eager to hear what is the impression caused to the community.
>>>
>>> Here is a summary of the formats and features supported:
>>>
>>> - Openflow
>>> - Console: Prints flows back to the console allowing filtering and
>>>   extensive formatting.
>>> - Cookie: Arranges flows based on cookie (instead of table) to ease
>>>   visualization of OVN-generated flows.
>>> - HTML: Prints an HTML file that shows the flows arranged by tables.
>>>   resubmit actions have a hyperlinke to the target table to ease
>>>   navegation.
>>> - Logic: Many times OVN generates many "logically-equivalent" flows.
>>>   These are flows that have the same structure: match on the same
>>>   values and have the same actions. The only thing that changes is
>>>   the actual value they match against and maybe the arguments of the
>>>   actions. This format collapses these flows so you can visualize the
>>>   logical pipeline easier.
>>> - JSON: JSON format.
>>>
>>> More OpenFlow features:
>>> - OVN metadata: Both the "cookie" and the "logic" format allow to
>>>   connect to a running OVN NB/SB databases and enrich the flows with
>>>   OVN context based on the flow's cookie.
>>>
>>> - Datapath:
>>> - Console: Prints flows back to the console allowing filtering and
>>>   extensive formatting.
>>> - Tree: Datapath flows use recirculation to further execute
>>>   additional actions. This format builds a tree of flows following
>>>   the recirculation identifiers and prints it in the console.
>>> - HTML: Prints the datapath flow tree in HTML. It includes some
>>>   minimal JS to support expanding and collapsing of entire branches.
>>> - Graph: Following the "tree" format, this one prints the tree in
>>>   graphviz format.
>>> - JSON: JSON format.
>>>
>>> Additional datapath features:
>>> - Many datapath formats are based on the tree flow hierarchy. An
>>>   interesting feature of this structure is that filtering is done at
>>>   the branch level. This means that when a flow satisfies the filter,
>>>   the entire sub-tree leading to that flow is shown.
>>>
>>> Additional common features:
>>> - Styling: Styles for both console and HTML formats can be defined
>>>   using a configuration file.
>>> - Heat maps: Some formats support heat-maps. A color pallete ranging
>>>   from blue (cold) to red (hot) is used to print the number of
>>>   packets and bytes of the flows. That way, the flows that are
>>>   handling more traffic are much easier to spot
>>>
>>> I know this is a long series, so it's OK if it takes time to review. I'm
>>> specially interested to know what is found useful and what is worth
>>> removing (if any).
>>
>> Thanks Adrian, for submitting the series and I think the quality of the code 
>> is good enough to submit it as a non-RFC. Most of the comments I have are 
>> style nits.
>>
>> The only main thing missing is some documentation/man pages for the tool.
>>
>
> Thanks Eelco for the review.

My pleasure. I looked at all your comments on my comments, and I have nothing 
more to add. Looking forward to your none RFC submission ;)

//Eelco

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

[Aaron Conole]

Adrian Moreno  writes:

> On 2/1/24 10:02, Eelco Chaudron wrote:
>> On 31 Jan 2024, at 18:03, Aaron Conole wrote:
>> 
>>> Eelco Chaudron  writes:
>>>
 On 25 Jan 2024, at 21:55, Aaron Conole wrote:

> From: Kevin Sprague 
>
> During normal operations, it is useful to understand when a particular 
> flow
> gets removed from the system. This can be useful when debugging 
> performance
> issues tied to ofproto flow changes, trying to determine deployed traffic
> patterns, or while debugging dynamic systems where ports come and go.
>
> Prior to this change, there was a lack of visibility around flow 
> expiration.
> The existing debugging infrastructure could tell us when a flow was added 
> to
> the datapath, but not when it was removed or why.
>
> This change introduces a USDT probe at the point where the revalidator
> determines that the flow should be removed.  Additionally, we track the
> reason for the flow eviction and provide that information as well.  With
> this change, we can track the complete flow lifecycle for the netlink
> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, 
> and
> the revaldiator USDT, letting us watch as flows are added and removed from
> the kernel datapath.
>
> This change only enables this information via USDT probe, so it won't be
> possible to access this information any other way (see:
> Documentation/topics/usdt-probes.rst).
>
> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
> which serves as a demonstration of how the new USDT probe might be used
> going forward.
>
> Signed-off-by: Kevin Sprague 
> Co-authored-by: Aaron Conole 
> Signed-off-by: Aaron Conole 

 Thanks for following this up Aaron! See comments on this patch
> below. I have no additional comments on patch 2.

 Cheers,

 Eelco


> ---
>   Documentation/topics/usdt-probes.rst |   1 +
>   ofproto/ofproto-dpif-upcall.c|  42 +-
>   utilities/automake.mk|   3 +
>   utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>   4 files changed, 693 insertions(+), 6 deletions(-)
>   create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>
> diff --git a/Documentation/topics/usdt-probes.rst 
> b/Documentation/topics/usdt-probes.rst
> index e527f43bab..a8da9bb1f7 100644
> --- a/Documentation/topics/usdt-probes.rst
> +++ b/Documentation/topics/usdt-probes.rst
> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>   - dpif_recv:recv_upcall
>   - main:poll_block
>   - main:run_start
> +- revalidate:flow_result
>   - revalidate_ukey\_\_:entry
>   - revalidate_ukey\_\_:exit
>   - udpif_revalidator:start_dump

 You are missing the specific flow_result result section. This is from the 
 previous patch:
>>>
>>> D'oh!  Thanks for catching it.  I'll re-add it.
>>>
 @@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
   - ``utilities/usdt-scripts/bridge_loop.bt``


 +probe revalidate:flow_result
 +~
 +
 +**Description**:
 +This probe is triggered when the revalidator decides whether or not to
 +revalidate a flow. ``reason`` is an enum that denotes that either the flow
 +is being kept, or the reason why the flow is being deleted. The
 +``flow_reval_monitor.py`` script uses this probe to notify users when 
 flows
 +matching user-provided criteria are deleted.
 +
 +**Arguments**:
 +
 +- *arg0*: ``(enum flow_del_reason) reason``
 +- *arg1*: ``(struct udpif *) udpif``
 +- *arg2*: ``(struct udpif_key *) ukey``
 +
 +**Script references**:
 +
 +- ``utilities/usdt-scripts/flow_reval_monitor.py``
 +
 +
   Adding your own probes
   --

> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index b5cbeed878..97d75833f7 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -269,6 +269,18 @@ enum ukey_state {
>   };
>   #define N_UKEY_STATES (UKEY_DELETED + 1)
>
> +enum flow_del_reason {
> +FDR_REVALIDATE = 0, /* The flow was revalidated. */

 It was called FDR_FLOW_LIVE before, which might make more
> sense. As the flow is just NOT deleted. It might or might not have
> been revalidated. Thoughts?
>>>
>>> I think it had to have been revalidated if we emit the reason, because
>>> we only emit the reason code after revalidation.  IE: there are many
>>> places where we skip revalidation but the flow stays live - and we don't
>>> emit reasons in those cases.
>>>
>>> So at least for this patch, it MUST have been revalidated.  But maybe in
>>> the future, we would 

Re: [ovs-dev] [PATCH ovn 2/2] Prepare for post-24.03.0.

[0-day Robot]

Bleep bloop.  Greetings Mark Michelson, 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 Mark Michelson  needs to sign off.
Lines checked: 53, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 ovn 3/3] northd: Add BFD support for ECMP route policy.

[Mark Michelson]

Thanks Lorenzo,

Acked-by: Mark Michelson 

On 2/2/24 08:49, Lorenzo Bianconi wrote:

Similar to OVN static routes, introduce the capability to link a BFD
session for OVN reroute policies.

Reported-at: https://issues.redhat.com/browse/FDP-234
Signed-off-by: Lorenzo Bianconi 
---
  NEWS  |  1 +
  northd/northd.c   | 86 ++
  ovn-nb.ovsschema  |  9 +++-
  ovn-nb.xml|  7 +++
  tests/ovn-nbctl.at|  6 +++
  tests/ovn-northd.at   | 20 +++-
  tests/system-ovn.at   | 51 ++--
  utilities/ovn-nbctl.8.xml |  8 
  utilities/ovn-nbctl.c | 97 ++-
  9 files changed, 267 insertions(+), 18 deletions(-)

diff --git a/NEWS b/NEWS
index 6553bd078..d4227ac99 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,7 @@ Post v23.09.0
- Support selecting encapsulation IP based on the source/destination VIF's
  settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
  details.
+  - Introduce next-hop BFD availability check for OVN reroute policies.
  
  OVN v23.09.0 - 15 Sep 2023

  --
diff --git a/northd/northd.c b/northd/northd.c
index d2091d4bc..ae50e4ff9 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10991,10 +10991,63 @@ get_outport_for_routing_policy_nexthop(struct 
ovn_datapath *od,
  return NULL;
  }
  
+static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER;

+
+static bool check_bfd_state(
+const struct nbrec_logical_router_policy *rule,
+const struct hmap *bfd_connections,
+struct ovn_port *out_port,
+const char *nexthop)
+{
+struct in6_addr nexthop_v6;
+bool is_nexthop_v6 = ipv6_parse(nexthop, _v6);
+bool ret = true;
+
+for (size_t i = 0; i < rule->n_bfd_sessions; i++) {
+/* Check if there is a BFD session associated to the reroute
+ * policy. */
+const struct nbrec_bfd *nb_bt = rule->bfd_sessions[i];
+struct in6_addr dst_ipv6;
+bool is_dst_v6 = ipv6_parse(nb_bt->dst_ip, _ipv6);
+
+if (is_nexthop_v6 ^ is_dst_v6) {
+continue;
+}
+
+if ((is_nexthop_v6 && !ipv6_addr_equals(_v6, _ipv6)) ||
+strcmp(nb_bt->dst_ip, nexthop)) {
+continue;
+}
+
+if (strcmp(nb_bt->logical_port, out_port->key)) {
+continue;
+}
+
+struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections,
+  nb_bt->logical_port,
+  nb_bt->dst_ip);
+ovs_mutex_lock(_lock);
+if (bfd_e) {
+bfd_e->ref = true;
+}
+
+if (!strcmp(nb_bt->status, "admin_down")) {
+nbrec_bfd_set_status(nb_bt, "down");
+}
+
+ret = strcmp(nb_bt->status, "down");
+ovs_mutex_unlock(_lock);
+break;
+}
+
+return ret;
+}
+
  static void
  build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od,
const struct hmap *lr_ports,
const struct nbrec_logical_router_policy *rule,
+  const struct hmap *bfd_connections,
const struct ovsdb_idl_row *stage_hint)
  {
  struct ds match = DS_EMPTY_INITIALIZER;
@@ -11019,6 +11072,11 @@ build_routing_policy_flow(struct hmap *lflows, struct 
ovn_datapath *od,
   rule->priority, nexthop);
  return;
  }
+
+if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) {
+return;
+}
+
  uint32_t pkt_mark = smap_get_uint(>options, "pkt_mark", 0);
  if (pkt_mark) {
  ds_put_format(, "pkt.mark = %u; ", pkt_mark);
@@ -11060,6 +8,7 @@ static void
  build_ecmp_routing_policy_flows(struct hmap *lflows, struct ovn_datapath *od,
  const struct hmap *lr_ports,
  const struct nbrec_logical_router_policy 
*rule,
+const struct hmap *bfd_connections,
  uint16_t ecmp_group_id)
  {
  ovs_assert(rule->n_nexthops > 1);
@@ -11088,6 +11147,9 @@ build_ecmp_routing_policy_flows(struct hmap *lflows, 
struct ovn_datapath *od,
  struct ds match = DS_EMPTY_INITIALIZER;
  struct ds actions = DS_EMPTY_INITIALIZER;
  
+size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops);

+size_t n_valid_nexthops = 0;
+
  for (size_t i = 0; i < rule->n_nexthops; i++) {
  struct ovn_port *out_port = get_outport_for_routing_policy_nexthop(
   od, lr_ports, rule->priority, rule->nexthops[i]);
@@ -11105,6 +11167,13 @@ build_ecmp_routing_policy_flows(struct hmap *lflows, 
struct ovn_datapath *od,
  goto cleanup;
  }
  
+if (!check_bfd_state(rule, bfd_connections, out_port,

+  

[ovs-dev] [PATCH ovn 1/2] Prepare for 24.03.0.

[Mark Michelson]

---
 NEWS | 4 ++--
 configure.ac | 2 +-
 debian/changelog | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 6553bd078..ff5e3c9ce 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-Post v23.09.0
--
+OVN v24.03.0 - xx xxx 
+--
   - DNS now have an "options" column for configuration of extra options.
   - A new DNS option "ovn-owned" has been added to allow defining domains
 that are owned only by ovn, queries for that domain will not be processed
diff --git a/configure.ac b/configure.ac
index 8284800e7..5faf1b7e1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 23.09.90, b...@openvswitch.org)
+AC_INIT(ovn, 24.03.0, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index cb26e645b..ad21b4eda 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-ovn (23.09.90-1) unstable; urgency=low
+ovn (24.03.0-1) unstable; urgency=low
 
* New upstream version
 
- -- OVN team   Fri, 01 Sep 2023 12:14:56 -0400
+ -- OVN team   Fri, 02 Feb 2024 11:16:48 -0500
 
 ovn (23.09.0-1) unstable; urgency=low
 
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn 2/2] Prepare for post-24.03.0.

[Mark Michelson]

---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index ff5e3c9ce..fadfcacfb 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+Post v24.03.0
+-
+
 OVN v24.03.0 - xx xxx 
 --
   - DNS now have an "options" column for configuration of extra options.
diff --git a/configure.ac b/configure.ac
index 5faf1b7e1..6a6b0db6a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 24.03.0, b...@openvswitch.org)
+AC_INIT(ovn, 24.03.90, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index ad21b4eda..950af8b64 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+ovn (24.03.90-1) unstable; urgency=low
+
+   * New upstream version
+
+ -- OVN team   Fri, 02 Feb 2024 11:16:48 -0500
+
 ovn (24.03.0-1) unstable; urgency=low
 
* New upstream version
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 1/2] Prepare for 24.03.0.

[0-day Robot]

Bleep bloop.  Greetings Mark Michelson, 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 Mark Michelson  needs to sign off.
Lines checked: 55, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 3/3] northd: add BFD support for ECMP route policy

[Lorenzo Bianconi]

> Hi Lorenzo,
> 
> I have a few comments below.
> 
> On 1/17/24 06:11, Lorenzo Bianconi wrote:
> > Similar to OVN static routes, introduce the capability to link a BFD
> > session for OVN reroute policies.
> > 
> > Reported-at: https://issues.redhat.com/browse/FDP-234
> > Signed-off-by: Lorenzo Bianconi 
> > ---
> >   NEWS  |  1 +
> >   northd/northd.c   | 71 +
> >   ovn-nb.ovsschema  |  9 -
> >   ovn-nb.xml|  7 
> >   tests/ovn-nbctl.at|  6 +++
> >   tests/ovn-northd.at   | 20 +-
> >   tests/system-ovn.at   | 51 +---
> >   utilities/ovn-nbctl.8.xml |  8 
> >   utilities/ovn-nbctl.c | 84 ++-
> >   9 files changed, 239 insertions(+), 18 deletions(-)
> > 
> > diff --git a/NEWS b/NEWS
> > index 5f267b4c6..5c0ea7e80 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -14,6 +14,7 @@ Post v23.09.0
> > - ovn-northd-ddlog has been removed.
> > - A new LSP option "enable_router_port_acl" has been added to enable
> >   conntrack for the router port whose peer is l3dgw_port if set it true.
> > +  - Introduce next-hop BFD availability check for OVN reroute policies.
> >   OVN v23.09.0 - 15 Sep 2023
> >   --
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 952f8200d..b517e5a7b 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -10887,10 +10887,48 @@ get_outport_for_routing_policy_nexthop(struct 
> > ovn_datapath *od,
> >   return NULL;
> >   }
> > +static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER;
> > +
> > +static bool check_bfd_state(
> > +const struct nbrec_logical_router_policy *rule,
> > +const struct hmap *bfd_connections,
> > +struct ovn_port *out_port,
> > +const char *nexthop)
> > +{
> > +for (size_t i = 0; i < rule->n_bfd_sessions; i++) {
> > +/* Check if there is a BFD session associated to the reroute
> > + * policy. */
> > +const struct nbrec_bfd *nb_bt = rule->bfd_sessions[i];
> > +if (!strcmp(nb_bt->dst_ip, nexthop) &&
> 
> If nb_bt->dst_ip and nexthop are IPv6 addresses, then strcmp is a not a good
> way to compare addresses, since there are many ways to represent the same
> IPv6 address as a string.

ack, I will fix it.

> 
> > +!strcmp(nb_bt->logical_port, out_port->key)) {
> > +struct bfd_entry *bfd_e = bfd_port_lookup(
> > +bfd_connections, nb_bt->logical_port,
> > +nb_bt->dst_ip);
> > +
> > +ovs_mutex_lock(_lock);
> 
> Correct me if I'm wrong, but I think that there is a 1-to-1 relationship
> between northbound BFD rows and internal northd bfd_entry structs. If that's
> correct, then could we add a mutex to struct bfd_entry and lock that here
> instead of using a global bfd_lock? That way, contention is limited to
> reroute policies that reroute to the same destination IP address.

yep, I agree but I do not this is strictly related to this series since we
already have bfd_lock for static route as well. What about address your
comments in a subsequent patch?

> 
> > +if (bfd_e) {
> > +bfd_e->ref = true;
> > +}
> > +
> > +if (!strcmp(nb_bt->status, "admin_down")) {
> > +nbrec_bfd_set_status(nb_bt, "down");
> 
> You could unlock and return false here.

we will need to duplicate the code with the one below in this way, don't we?

> 
> > +}
> > +
> > +if (!strcmp(nb_bt->status, "down")) {
> > +ovs_mutex_unlock(_lock);
> > +return false;
> > +}
> > +ovs_mutex_unlock(_lock);
> 
> Would it be possible to return true here? Or can there be multiple BFD
> sessions associated with the nexthop IP address that we need to check the
> state of? And must all of them be "up" for us to return true?

ack, we can just return here, I will fix it.

> 
> > +}
> > +}
> > +return true;
> > +}
> > +
> >   static void
> >   build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od,
> > const struct hmap *lr_ports,
> > const struct nbrec_logical_router_policy *rule,
> > +  const struct hmap *bfd_connections,
> > const struct ovsdb_idl_row *stage_hint)
> >   {
> >   struct ds match = DS_EMPTY_INITIALIZER;
> > @@ -10915,6 +10953,11 @@ build_routing_policy_flow(struct hmap *lflows, 
> > struct ovn_datapath *od,
> >rule->priority, nexthop);
> >   return;
> >   }
> > +
> > +if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) {
> > +return;
> > +}
> > +
> >   uint32_t pkt_mark = ovn_smap_get_uint(>options, "pkt_mark", 
> > 0);
> >   if (pkt_mark) {
> >

Re: [ovs-dev] [PATCH RFC 2/2] vswitch.xml: Rename bond_active_member.

[Ilya Maximets]

On 2/2/24 16:38, Simon Horman wrote:
> Since the patch-set that included [1] there has been a policy of using
> the term member for bonds, LACP, and bundle contexts.  This is
> consistent with the more recently adopted policy of using the inclusive
> naming word list v1 [2, 3].
> 
> This patch addresses the name of the bond_active_member ovsdb column.
> This is used internally to allow the active member of a bond across OVS
> restart. And the effect of this patch is that when restarting OVS, if
> the instance before restart did not have this patch and the instance
> after does, or vice-versa, then active bond member selection will not be
> sticky across that restart. Otherwise, the existing behaviour is
> maintained.

Hi, Simon.  Unfortunately it is not possible to remove existing columns
from the database.  The new schema is not compatible and the existing
clients will fail to connect to the updated database.  You may see that
if you only update the schema, but not re-compile ovs-vswitchd:

2024-02-02T16:43:00.228Z|00027|ovsdb_cs|INFO|unix:sandbox/db.sock:
  received unexpected error response in DATA_MONITOR_REQUESTED state:
  {"error":{"details":"bond_active_slave is not a valid column name",
   "error":"syntax error",
   "syntax":"[\"bond_active_slave\", \"bond_downdelay\",\"bond_fake_iface\",
  \"bond_mode\",\"bond_updelay\",\"cvlans\",\"fake_bridge\",
  \"interfaces\",\"lacp\",\"mac\",\"name\",\"other_config\",
  \"protected\",\"qos\",\"rstp_statistics\",\"rstp_status\",
  \"statistics\",\"status\",\"tag\",\"trunks\",\"vlan_mode\"]"
  },"id":52,"result":null}


The column have to stay, similarly to other public interfaces.

What we can probably do is to add a new column and use/store values
from/into both, while removing the old one from the documentation.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.

[Mark Michelson]

Thanks a bunch Lorenzo.

Acked-by: Mark Michelson 

I plan to merge this in a few hours, giving a bit of time in case others 
want to review this. When I merge, I'll fold in Dumitru's suggestion.


On 2/2/24 11:03, Dumitru Ceara wrote:

On 2/2/24 16:41, Lorenzo Bianconi wrote:

Add the capbility to mark (through pkt.mark) incoming/outgoing packets
in logical_switch datapath according to user configured QoS rule.

Co-developed-by: Dumitru Ceara 
Reported-at: https://issues.redhat.com/browse/FDP-42
Signed-off-by: Lorenzo Bianconi 
---
Changes since v2:
- set max dscp/mark value in ovn-nb.ovsschema
- add unit-test for new dscp/mark boundaries
Changes since v1:
- move qos packet mark action in QOS_MARK ls {ingress/egress} stage
---
  NEWS  |  2 +
  northd/northd.c   | 33 +---
  northd/ovn-northd.8.xml   |  6 +++
  ovn-nb.ovsschema  |  8 ++--
  ovn-nb.xml| 12 +-
  tests/ovn-nbctl.at|  8 +++-
  tests/ovn-northd.at   | 81 ++
  tests/ovn.at  | 83 +++
  utilities/ovn-nbctl.8.xml |  5 ++-
  utilities/ovn-nbctl.c | 27 ++---
  10 files changed, 245 insertions(+), 20 deletions(-)

diff --git a/NEWS b/NEWS
index 6553bd078..a8beb09fb 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,8 @@ Post v23.09.0
- Support selecting encapsulation IP based on the source/destination VIF's
  settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
  details.
+  - Add the capability to mark (through pkt.mark) incoming/outgoing packets
+in the logical switch datapath according to user configured QoS rule.
  
  OVN v23.09.0 - 15 Sep 2023

  --
diff --git a/northd/northd.c b/northd/northd.c
index d2091d4bc..a77919af3 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8363,6 +8363,8 @@ build_acls(struct ovn_datapath *od, const struct 
chassis_features *features,
  ds_destroy();
  }
  
+#define QOS_MAX_DSCP 63

+
  static void
  build_qos(struct ovn_datapath *od, struct hmap *lflows) {
  struct ds action = DS_EMPTY_INITIALIZER;
@@ -8376,21 +8378,40 @@ build_qos(struct ovn_datapath *od, struct hmap *lflows) 
{
  struct nbrec_qos *qos = od->nbs->qos_rules[i];
  bool ingress = !strcmp(qos->direction, "from-lport") ? true :false;
  enum ovn_stage stage = ingress ? S_SWITCH_IN_QOS_MARK : 
S_SWITCH_OUT_QOS_MARK;
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
  int64_t rate = 0;
  int64_t burst = 0;
  
+ds_clear();

  for (size_t j = 0; j < qos->n_action; j++) {
+if (strcmp(qos->key_action[j], "dscp") &&
+strcmp(qos->key_action[j], "mark")) {
+continue;
+}
+


This check seems redundant we recheck the qos->key_action[j] just below.
  Would it be possible to remove it when applying the patch to the main
branch (if the patch is accepted)?

Thanks,
Dumitru



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 1/2] Prepare for 24.03.0.

[Numan Siddique]

On Fri, Feb 2, 2024 at 11:50 AM Mark Michelson  wrote:
>
> ---
>  NEWS | 4 ++--
>  configure.ac | 2 +-
>  debian/changelog | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)

With your signed-off-by tag added

Acked-by: Numan Siddique 

Numan

>
> diff --git a/NEWS b/NEWS
> index 6553bd078..ff5e3c9ce 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,5 @@
> -Post v23.09.0
> --
> +OVN v24.03.0 - xx xxx 
> +--
>- DNS now have an "options" column for configuration of extra options.
>- A new DNS option "ovn-owned" has been added to allow defining domains
>  that are owned only by ovn, queries for that domain will not be processed
> diff --git a/configure.ac b/configure.ac
> index 8284800e7..5faf1b7e1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>
>  AC_PREREQ(2.63)
> -AC_INIT(ovn, 23.09.90, b...@openvswitch.org)
> +AC_INIT(ovn, 24.03.0, b...@openvswitch.org)
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> diff --git a/debian/changelog b/debian/changelog
> index cb26e645b..ad21b4eda 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,8 +1,8 @@
> -ovn (23.09.90-1) unstable; urgency=low
> +ovn (24.03.0-1) unstable; urgency=low
>
> * New upstream version
>
> - -- OVN team   Fri, 01 Sep 2023 12:14:56 -0400
> + -- OVN team   Fri, 02 Feb 2024 11:16:48 -0500
>
>  ovn (23.09.0-1) unstable; urgency=low
>
> --
> 2.43.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] tests: Fix incorrect 'check_engine_stats' helper function.

[numans]

From: Numan Siddique 

The patch which added the helper function 'check_engine_stats'
didn't add it properly because of which we are seeing the below
warnings.

test-source: line 2187: check_engine_stats: command not found

This patch fixes it by using m4_divert_push/m4_divert_pop macros.

Fixes: c69119ca3b59 ("tests: Add a couple of tests in ovn-northd for I-P.")

Signed-off-by: Numan Siddique 
---
 tests/ovn-northd.at | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 5c597eb1ce..817defdd67 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -36,6 +36,7 @@ m4_define([CHECK_NO_CHANGE_AFTER_RECOMPUTE], [
 ])
 ])
 
+m4_divert_push([PREPARE_TESTS])
 # Checks if the provided engine node recomputed or not
 # and if it computed or not.
 # 1st argument is the engine node.
@@ -79,6 +80,7 @@ $recompute : compute - $compute"
 check test "$node_compute_ct" -ne "0"
   fi
 }
+m4_divert_pop([PREPARE_TESTS])
 
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([check from NBDB to SBDB])
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] tests: Fix incorrect 'check_engine_stats' helper function.

[Numan Siddique]

On Fri, Feb 2, 2024 at 5:23 PM Mark Michelson  wrote:
>
> Acked-by: Mark Michelson 

Thanks.  Applied to main.

Numan

>
> On 2/2/24 16:57, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > The patch which added the helper function 'check_engine_stats'
> > didn't add it properly because of which we are seeing the below
> > warnings.
> >
> > test-source: line 2187: check_engine_stats: command not found
> >
> > This patch fixes it by using m4_divert_push/m4_divert_pop macros.
> >
> > Fixes: c69119ca3b59 ("tests: Add a couple of tests in ovn-northd for I-P.")
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   tests/ovn-northd.at | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 5c597eb1ce..817defdd67 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -36,6 +36,7 @@ m4_define([CHECK_NO_CHANGE_AFTER_RECOMPUTE], [
> >   ])
> >   ])
> >
> > +m4_divert_push([PREPARE_TESTS])
> >   # Checks if the provided engine node recomputed or not
> >   # and if it computed or not.
> >   # 1st argument is the engine node.
> > @@ -79,6 +80,7 @@ $recompute : compute - $compute"
> >   check test "$node_compute_ct" -ne "0"
> > fi
> >   }
> > +m4_divert_pop([PREPARE_TESTS])
> >
> >   OVN_FOR_EACH_NORTHD_NO_HV([
> >   AT_SETUP([check from NBDB to SBDB])
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 2/2] Prepare for post-24.03.0.

[Numan Siddique]

On Fri, Feb 2, 2024 at 11:50 AM Mark Michelson  wrote:
>
> ---
>  NEWS | 3 +++
>  configure.ac | 2 +-
>  debian/changelog | 6 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)

With your signed-off-by tag added

Acked-by: Numan Siddique 

Numan


>
> diff --git a/NEWS b/NEWS
> index ff5e3c9ce..fadfcacfb 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,6 @@
> +Post v24.03.0
> +-
> +
>  OVN v24.03.0 - xx xxx 
>  --
>- DNS now have an "options" column for configuration of extra options.
> diff --git a/configure.ac b/configure.ac
> index 5faf1b7e1..6a6b0db6a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>
>  AC_PREREQ(2.63)
> -AC_INIT(ovn, 24.03.0, b...@openvswitch.org)
> +AC_INIT(ovn, 24.03.90, b...@openvswitch.org)
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> diff --git a/debian/changelog b/debian/changelog
> index ad21b4eda..950af8b64 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +ovn (24.03.90-1) unstable; urgency=low
> +
> +   * New upstream version
> +
> + -- OVN team   Fri, 02 Feb 2024 11:16:48 -0500
> +
>  ovn (24.03.0-1) unstable; urgency=low
>
> * New upstream version
> --
> 2.43.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] tests: Fix incorrect 'check_engine_stats' helper function.

[Mark Michelson]

Acked-by: Mark Michelson 

On 2/2/24 16:57, num...@ovn.org wrote:

From: Numan Siddique 

The patch which added the helper function 'check_engine_stats'
didn't add it properly because of which we are seeing the below
warnings.

test-source: line 2187: check_engine_stats: command not found

This patch fixes it by using m4_divert_push/m4_divert_pop macros.

Fixes: c69119ca3b59 ("tests: Add a couple of tests in ovn-northd for I-P.")

Signed-off-by: Numan Siddique 
---
  tests/ovn-northd.at | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 5c597eb1ce..817defdd67 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -36,6 +36,7 @@ m4_define([CHECK_NO_CHANGE_AFTER_RECOMPUTE], [
  ])
  ])
  
+m4_divert_push([PREPARE_TESTS])

  # Checks if the provided engine node recomputed or not
  # and if it computed or not.
  # 1st argument is the engine node.
@@ -79,6 +80,7 @@ $recompute : compute - $compute"
  check test "$node_compute_ct" -ne "0"
fi
  }
+m4_divert_pop([PREPARE_TESTS])
  
  OVN_FOR_EACH_NORTHD_NO_HV([

  AT_SETUP([check from NBDB to SBDB])


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev