Re: [ovs-dev] [PATCH ovn v2] northd: Support routing over other address families.

2024-05-07 Thread Felix Huettner via dev
> 
> Let's say we add this static route -  "prefix = 172.168.0.0/24,
> nexthop = aef0::4"
> and lets assume OVN doesn't know the next hop "aef0::4" mac address.
> So with your patch, the original packet will be dropped because
> pinctrl.c is not handling it properly.
> 
> I see 2 approaches here :
> 1.  Fix it in ovn-northd/pinctrl so that for IPv6 next hop, neigh
> solicit packet is generated and for IPv4 next hop, ARP request packet
> is generated.
> 2.  CMS should add a static mac binding entry in the OVN Northbound db
> for the next hop for this scenario to work properly.
>  Eg.  ovn-nbctl static-mac-binding-add lr0-public aef0::5 
> 10:14:00:00:00:05
>  In this case, ovn-controller(s) will add an openflow  like below
> 
> ---
>  ovs-ofctl dump-flows br-int table=66
>  cookie=0xa4249f73, duration=470.024s, table=66, n_packets=0,
> n_bytes=0, 
> priority=50,reg0=0xaef0,reg1=0,reg2=0,reg3=0x5,reg15=0x3,metadata=0x3
> actions=mod_dl_dst:10:14:00:00:00:05,load:0x1->NXM_NX_REG10[6]
> 
> 
> ovn-northd adds below logical flows in the router ingress pipeline
> stage - "lr_in_arp_resolve".  These logical flows are hit if OVN can't
> resolve the next hop mac address
> 
>   table=20(lr_in_arp_resolve  ), priority=1, match=(ip4),
> action=(get_arp(outport, reg0); next;)
>   table=20(lr_in_arp_resolve  ), priority=1, match=(ip6),
> action=(get_nd(outport, xxreg0); next;)
> 
> The OVN actions get_arp and get_nd gets translated to OVS actions -
> resubmit(,66).
> The problem is that for IPv4 packets, reg0 is used and for IPv6
> packetsm xxreg0 is used.  But with your patch to support mixed static
> routes,  it will not work.
> 
> For both the approaches (whichever we chose to take),  we need to
> solve this problem.  Maybe we can use a register bit to indicate if
> nexhop is IPv4 addr or IPv6 addr
> in the "lr_in_ip_routing" stage and depending on that we can have
> below logical flows,
> 
>  table=20(lr_in_arp_resolve  ), priority=1, match=(reg10[1] == 0),
> action=(get_arp(outport, reg0); next;)
>   table=20(lr_in_arp_resolve  ), priority=1, match=(eg10[1] == 1),
> action=(get_nd(outport, xxreg0); next;)
> 
> I think your next version should address this, otherwise the feature
> will be incomplete.
> 
> If we go with approach (1),  we can indicate pinctrl to generate ARP
> or NS depending on a register bit.
> 
> Thanks
> Numan
> 

Sounds good, will do that in the coming weeks.

Thank you
Felix

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


Re: [ovs-dev] [PATCH v2] selftests/net: fix uninitialized variables

2024-05-07 Thread patchwork-bot+netdevbpf
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski :

On Mon,  6 May 2024 12:02:04 -0700 you wrote:
> When building with clang, via:
> 
> make LLVM=1 -C tools/testing/selftest
> 
> ...clang warns about three variables that are not initialized in all
> cases:
> 
> [...]

Here is the summary with links:
  - [v2] selftests/net: fix uninitialized variables
https://git.kernel.org/netdev/net-next/c/eb709b5f6536

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


[ovs-dev] [PATCH ovn] northd: Add documentation for is_cr_port() and is_l3dgw_port().

2024-05-07 Thread numans
From: Numan Siddique 

Although these util functions are correct in their implementation,
they are confusing to the reader.  Add proper documentation
and make the is_l3dgw_port() more clearer by checking if the
gateway chassis or ha chassis group is set or not.

Signed-off-by: Numan Siddique 
---
 northd/northd.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/northd/northd.c b/northd/northd.c
index 0cabda7ea0..d1afc52c4a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1077,12 +1077,32 @@ struct ovn_port_routable_addresses {
 
 static bool lsp_can_be_inc_processed(const struct nbrec_logical_switch_port *);
 
+/* This function returns true if 'op' is a gateway router port.
+ * False otherwise.
+ * For 'op' to be a gateway router port.
+ *  1. op->nbrp->gateway_chassis or op->nbrp->ha_chassis_group should
+ * be configured.
+ *  2. op->cr_port should not be NULL.  If op->nbrp->gateway_chassis or
+ * op->nbrp->ha_chassis_group is set by the user, northd WILL create
+ * a chassis resident port in the SB port binding.
+ * See join_logical_ports().
+ */
 static bool
 is_l3dgw_port(const struct ovn_port *op)
 {
-return op->cr_port;
+return op->cr_port && op->nbrp &&
+   (op->nbrp->n_gateway_chassis || op->nbrp->ha_chassis_group);
 }
 
+/* This function returns true if 'op' is a chassis resident
+ * derived port. False otherwise.
+ * There are 2 ways to check if 'op' is chassis resident port.
+ *  1. op->sb->type is "chassisresident"
+ *  2. op->l3dgw_port is not NULL.  If op->l3dgw_port is set,
+ * it means 'op' is derived from the gateway port (op->l3dgw_port).
+ *
+ * This function uses the (2) method as it doesn't involve strcmp().
+ */
 static bool
 is_cr_port(const struct ovn_port *op)
 {
-- 
2.44.0

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


Re: [ovs-dev] [PATCH ovn v2 0/4] Mac cache handling refactor

2024-05-07 Thread Numan Siddique
On Tue, May 7, 2024 at 12:51 PM Ales Musil  wrote:
>
> On Tue, May 7, 2024 at 6:49 PM Numan Siddique  wrote:
>
> > On Tue, May 7, 2024 at 2:25 AM Ales Musil  wrote:
> > >
> > > There were two modules in controller mac_cache and mac-learn, both of
> > > them did very similar thing with pretty big overlap. The goal of the
> > > series is to consolidate and merge both of those modules into single
> > > one. That will reduce the duplication and should make it easier for
> > > future updates to MAC binding, FDB or packet buffering functionality.
> > >
> > > There is also fix to properly handle tunnel_key change for LSP, LRP,
> > > LR and LS. This was inconsistent and could lead to wrong flows being
> > > still present even after the tunnel key change. This is not a huge
> > > issue because the tunnel_key is rarelyt changed during runtime.
> > >
> > > Ales Musil (4):
> > >   northd, controller: Handle tunnel_key change consistently.
> > >   controller: Rename mac_cache to to mac-cache.
> > >   controller: Merge the mac-cache and mac-learn.
> > >   controller: Use datapath key for the mac cache thresholds.
> >
> > Thanks.  I applied the series to the main branch.
> > Do we need a backport ?  If so,  I'm inclined to backport the first
> > patch only as it fixes the issue.
> > Let me know your thoughts.
> >
>
> That's correct, only the first patch should be backported.
The backport doesn't apply cleanly to branch-24.03.  Can you please
submit the backport patch for branch-24.03.

Thanks
Numan

>
>
> >
> > Numan
> >
> >
> > >
> > >  controller/automake.mk  |   6 +-
> > >  controller/binding.c|  13 +-
> > >  controller/mac-cache.c  | 745 
> > >  controller/mac-cache.h  | 210 ++
> > >  controller/mac-learn.c  | 482 ---
> > >  controller/mac-learn.h  | 145 ---
> > >  controller/mac_cache.c  | 547 --
> > >  controller/mac_cache.h  | 124 --
> > >  controller/ovn-controller.c | 214 +++
> > >  controller/pinctrl.c| 165 
> > >  controller/statctrl.c   |   7 +-
> > >  controller/statctrl.h   |   2 +-
> > >  northd/northd.c |   7 +
> > >  tests/ovn.at|  56 ++-
> > >  14 files changed, 1253 insertions(+), 1470 deletions(-)
> > >  create mode 100644 controller/mac-cache.c
> > >  create mode 100644 controller/mac-cache.h
> > >  delete mode 100644 controller/mac-learn.c
> > >  delete mode 100644 controller/mac-learn.h
> > >  delete mode 100644 controller/mac_cache.c
> > >  delete mode 100644 controller/mac_cache.h
> > >
> > > --
> > > 2.44.0
> > >
> > > ___
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
> >
> Thanks,
> Ales
>
> --
>
> 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


Re: [ovs-dev] [PATCH ovn 2/4] Inclusive language substitutions: "master".

2024-05-07 Thread 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:
WARNING: Line is 80 characters long (recommended limit is 79)
#36 FILE: Documentation/topics/high-availability.rst:299:
to elect a single leader. The simplest example is two gateways which stop seeing

Lines checked: 212, Warnings: 1, 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


[ovs-dev] [PATCH ovn 1/4] Inclusive language substitutions: "abort".

2024-05-07 Thread Mark Michelson
This commit replaces the word "abort" with alternatives when
possible.

Places where "abort" is not replaced:
* The "abort()" system call, as well as the "ovs_abort()" wrapper.
* Any references to SCTP abort. This is the name of the method in SCTP,
  so referring to it as a different name would be confusing.
* Any printed messages about OVSDB transactions being aborted. Like with
  SCTP, if we use a nonstandard name, it would just be confusing. If the
  OVSDB state machine eventually updates its state names, then we
  can also update our printed messages to use the new terminology.

Signed-off-by: Mark Michelson 
---
 .ci/dpdk-prepare.sh |  2 +-
 controller/chassis.c|  2 +-
 controller/ovn-controller.8.xml |  4 +--
 controller/ovn-controller.c |  6 ++--
 lib/inc-proc-eng.c  | 38 +++---
 lib/inc-proc-eng.h  | 10 +++---
 northd/inc-proc-northd.c|  4 +--
 tests/ovn-northd.at | 56 -
 tests/ovs-macros.at |  4 +--
 utilities/ovn-appctl.8.xml  |  4 +--
 10 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/.ci/dpdk-prepare.sh b/.ci/dpdk-prepare.sh
index f7e6215dd..5543da90a 100755
--- a/.ci/dpdk-prepare.sh
+++ b/.ci/dpdk-prepare.sh
@@ -4,7 +4,7 @@ set -ev
 
 # Installing wheel separately because it may be needed to build some
 # of the packages during dependency backtracking and pip >= 22.0 will
-# abort backtracking on build failures:
+# stop backtracking on build failures:
 # https://github.com/pypa/pip/issues/10655
 pip3 install --disable-pip-version-check --user wheel
 pip3 install --disable-pip-version-check --user pyelftools
diff --git a/controller/chassis.c b/controller/chassis.c
index 9bb2eba95..4942ba281 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -1003,7 +1003,7 @@ store_chassis_index_if_needed(
 goto out;
 }
 }
-/* All indices consumed: it's safer to just abort. */
+/* All indices consumed: it's safer to just exit. */
 VLOG_ERR("All unique controller indices consumed. Exiting.");
 exit(EXIT_FAILURE);
 }
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index 85e7966d7..4953ec93e 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -762,7 +762,7 @@
 compute
   
   
-abort
+cancel
   
 
   
@@ -773,7 +773,7 @@
 Display the ovn-controller engine counter(s) for the
 specified engine_node_name.  counter_name is
 optional and can be one of recompute,
-compute or abort.
+compute or cancel.
   
   
 
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 453dc62fd..89aa75bc9 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -5856,7 +5856,7 @@ main(int argc, char *argv[])
 /* Even if there's no SB DB transaction available,
  * try to run the engine so that we can handle any
  * incremental changes that don't require a recompute.
- * If a recompute is required, the engine will abort,
+ * If a recompute is required, the engine will cancel,
  * triggerring a full run in the next iteration.
  */
 engine_run(false);
@@ -6053,8 +6053,8 @@ main(int argc, char *argv[])
  " either: br_int %p, chassis %p",
  br_int, chassis);
 }
-} else if (engine_aborted()) {
-VLOG_DBG("engine was aborted, force recompute next time: "
+} else if (engine_canceled()) {
+VLOG_DBG("engine was canceled, force recompute next time: "
  "br_int %p, chassis %p", br_int, chassis);
 engine_set_force_recompute(true);
 poll_immediate_wake();
diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
index 21afcf92b..4b379229e 100644
--- a/lib/inc-proc-eng.c
+++ b/lib/inc-proc-eng.c
@@ -34,7 +34,7 @@
 VLOG_DEFINE_THIS_MODULE(inc_proc_eng);
 
 static bool engine_force_recompute = false;
-static bool engine_run_aborted = false;
+static bool engine_run_canceled = false;
 static const struct engine_context *engine_context;
 
 static struct engine_node **engine_nodes;
@@ -44,7 +44,7 @@ static const char *engine_node_state_name[EN_STATE_MAX] = {
 [EN_STALE] = "Stale",
 [EN_UPDATED]   = "Updated",
 [EN_UNCHANGED] = "Unchanged",
-[EN_ABORTED]   = "Aborted",
+[EN_CANCELED]   = "Canceled",
 };
 
 static long long engine_compute_log_timeout_msec = 500;
@@ -144,16 +144,16 @@ engine_dump_stats(struct unixctl_conn *conn, int argc,
  "Node: %s\n"
  "- recompute: %1

[ovs-dev] [PATCH ovn 0/4] Inclusive language changes for OVN code.

2024-05-07 Thread Mark Michelson
There is a growing movement to be more inclusive in the language used in
code and its documentation.

For this task, the following site was used to find a list of words that
should be avoided: https://inclusivenaming.org/word-lists/ .

Each of their tier 1, tier 2, and tier 3 words were searched in the code
and replaced as necessary.

Each commit is focused around the replacement of a single word from the
word lists. In each case, the commit message will explain if there are
any exceptions where the word was not replaced.

There are some words you will find in the linked word list that are not
addressed at all in these commits:

* "segregate": This words appears only in an old NEWS entry. Changing 
  this would be more odd than leaving it alone. Considering this is a
  tier 3 word on the linked word list, I opted to leave it as-is.
* "man-in-the-middle": This appears in all ovn-*ctl utility manpages.
  However, this is generated from an included file in the ovs submodule.
  Therefore, changing this in OVN would require a change in OVS instead.

The rest of the words in the list that are not addressed in this series
do not appear anywhere in the OVN repository, as far as I could find.

Mark Michelson (4):
  Inclusive language substitutions: "abort".
  Inclusive language substitutions: "master".
  Inclusive language substitutions: "blacklist/whitelist".
  Inclusive language substitutions: "sanity-check".

 .ci/dpdk-prepare.sh|  2 +-
 Documentation/topics/high-availability.rst |  2 +-
 NEWS   |  4 ++
 controller/chassis.c   |  2 +-
 controller/ha-chassis.c|  2 +-
 controller/ovn-controller.8.xml|  4 +-
 controller/ovn-controller.c|  6 +--
 ic/ovn-ic.c| 23 +
 lib/inc-proc-eng.c | 38 +++---
 lib/inc-proc-eng.h | 12 ++---
 northd/inc-proc-northd.c   |  4 +-
 ovn-architecture.7.xml |  2 +-
 ovn-nb.xml | 12 ++---
 ovn-sb.xml |  8 +--
 tests/ofproto-macros.at|  4 +-
 tests/ovn-controller-vtep.at   |  2 +-
 tests/ovn-ic.at| 32 ++--
 tests/ovn-northd.at| 60 +++---
 tests/ovn-performance.at   |  4 +-
 tests/ovn.at   | 14 ++---
 tests/ovs-macros.at|  4 +-
 tests/system-kmod-macros.at|  4 +-
 tests/system-userspace-macros.at   |  4 +-
 utilities/ovn-appctl.8.xml |  4 +-
 utilities/ovn-nbctl.c  |  2 +-
 25 files changed, 131 insertions(+), 124 deletions(-)

-- 
2.44.0

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


[ovs-dev] [PATCH ovn 3/4] Inclusive language substitutions: "blacklist/whitelist".

2024-05-07 Thread Mark Michelson
This commit changes the word "blacklist" to "denylist" throughout the
code. It also changes the word "whitelist" to "allowlist" throughout the
code.

The option "ic-route-blacklist" in the northbound global options has
been renamed to "ic-route-denylist", but the old option name is still
accepted in order to maintain backwards compatibility. The old option
name is no longer documented, however.

Signed-off-by: Mark Michelson 
---
 NEWS |  4 
 ic/ovn-ic.c  | 23 +--
 ovn-nb.xml   |  2 +-
 tests/ofproto-macros.at  |  4 ++--
 tests/ovn-controller-vtep.at |  2 +-
 tests/ovn-ic.at  | 32 
 tests/system-kmod-macros.at  |  4 ++--
 tests/system-userspace-macros.at |  4 ++--
 8 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/NEWS b/NEWS
index 3b5e93dc9..b2df43b3a 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,10 @@ Post v24.03.0
 external-ids, the option is no longer needed as it became effectively
 "true" for all scenarios.
   - Added DHCPv4 relay support.
+  - The "options:ic-route-blacklist" option in the Northbound NB_Global table
+has been renamed to "options:ic-route-denylist" in order to comply with
+inclusive language guidelines. The previous name is still recognized to
+aid with backwards compatibility.
 
 OVN v24.03.0 - 01 Mar 2024
 --
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index e947323bf..3fd74ecba 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1029,24 +1029,27 @@ prefix_is_link_local(struct in6_addr *prefix, unsigned 
int plen)
 }
 
 static bool
-prefix_is_black_listed(const struct smap *nb_options,
-   struct in6_addr *prefix,
-   unsigned int plen)
+prefix_is_deny_listed(const struct smap *nb_options,
+  struct in6_addr *prefix,
+  unsigned int plen)
 {
-const char *blacklist = smap_get(nb_options, "ic-route-blacklist");
-if (!blacklist || !blacklist[0]) {
-return false;
+const char *denylist = smap_get(nb_options, "ic-route-denylist");
+if (!denylist || !denylist[0]) {
+denylist = smap_get(nb_options, "ic-route-blacklist");
+if (!denylist || !denylist[0]) {
+return false;
+}
 }
 struct in6_addr bl_prefix;
 unsigned int bl_plen;
 char *cur, *next, *start;
-next = start = xstrdup(blacklist);
+next = start = xstrdup(denylist);
 bool matched = false;
 while ((cur = strsep(&next, ",")) && *cur) {
 if (!ip46_parse_cidr(cur, &bl_prefix, &bl_plen)) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
 VLOG_WARN_RL(&rl, "Bad format in nb_global options:"
- "ic-route-blacklist: %s. CIDR expected.", cur);
+ "ic-route-denylist: %s. CIDR expected.", cur);
 continue;
 }
 
@@ -1109,7 +1112,7 @@ route_need_advertise(const char *policy,
 return false;
 }
 
-if (prefix_is_black_listed(nb_options, prefix, plen)) {
+if (prefix_is_deny_listed(nb_options, prefix, plen)) {
 return false;
 }
 return true;
@@ -1281,7 +1284,7 @@ route_need_learn(const struct nbrec_logical_router *lr,
 return false;
 }
 
-if (prefix_is_black_listed(nb_options, prefix, plen)) {
+if (prefix_is_deny_listed(nb_options, prefix, plen)) {
 return false;
 }
 
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 3382d4db6..15976b95a 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -442,7 +442,7 @@
   ic-route-learn is true.
 
 
-
+
   A string value contains a list of CIDRs delimited by ",".  A route
   will not be advertised or learned if the route's prefix belongs to
   any of the CIDRs listed.
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 31a067c1e..ab73e50d9 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -354,11 +354,11 @@ add_pmd_of_ports () {
 
 m4_divert_pop([PREPARE_TESTS])
 
-# OVS_VSWITCHD_STOP([WHITELIST])
+# OVS_VSWITCHD_STOP([ALLOWLIST])
 #
 # Gracefully stops ovs-vswitchd and ovsdb-server, checking their log files
 # for messages with severity WARN or higher and signaling an error if any
-# is present.  The optional WHITELIST may contain shell-quoted "sed"
+# is present.  The optional ALLOWLIST may contain shell-quoted "sed"
 # commands to delete any warnings that are actually expected, e.g.:
 #
 #   OVS_VSWITCHD_STOP(["/expected error/d"])
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index d35dbbd05..ea67c2a5c 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -57,7 +57,7 @@ m4_define([OVN_CONTROLLER_VTEP_START], [
 
--ovnsb-db=unix:$ovs_base/ovn-sb/ovn-sb.sock
 ])
 
-# OVN_CONTROLLER_VTEP_STOP(WHITELIST, SIM_

[ovs-dev] [PATCH ovn 2/4] Inclusive language substitutions: "master".

2024-05-07 Thread Mark Michelson
This commit replaces the word "master" with alternative words.

Places where "master" is not replaced:
* References to third-party URLs, option names, and code repositories.
* The "integration" documentation is unchanged. It references pacemaker
  options which use the word "master". The text also refers to both
  "masters" and "slaves". In this case, changing the text could cause
  confusion if pacemaker uses "master" and "slave" in its user-facing
  options/documentation.

Signed-off-by: Mark Michelson 
---
 Documentation/topics/high-availability.rst |  2 +-
 controller/ha-chassis.c|  2 +-
 ovn-architecture.7.xml |  2 +-
 ovn-nb.xml | 10 +-
 ovn-sb.xml |  8 
 tests/ovn-performance.at   |  4 ++--
 tests/ovn.at   | 14 +++---
 7 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/topics/high-availability.rst 
b/Documentation/topics/high-availability.rst
index c3c962c1d..e85a1cd04 100644
--- a/Documentation/topics/high-availability.rst
+++ b/Documentation/topics/high-availability.rst
@@ -296,7 +296,7 @@ all traffic.
 
 We should note that this method works well under the assumption that there
 are no inter-gateway connectivity failures, in such case this method would fail
-to elect a single master. The simplest example is two gateways which stop 
seeing
+to elect a single leader. The simplest example is two gateways which stop 
seeing
 each other but can still reach the hypervisors. Protocols like VRRP or CARP
 have the same issue. A mitigation for this type of failure mode could be
 achieved by having all network elements (hypervisors and gateways) periodically
diff --git a/controller/ha-chassis.c b/controller/ha-chassis.c
index d6ec7b658..945c306b6 100644
--- a/controller/ha-chassis.c
+++ b/controller/ha-chassis.c
@@ -164,7 +164,7 @@ is_local_chassis_only_candidate(const struct 
sbrec_ha_chassis_group *ha_ch_grp,
 return (local_chassis_present && n_active_ha_chassis == 1);
 }
 
-/* Returns true if the local_chassis is the master of
+/* Returns true if the local_chassis is the active chassis of
  * the HA chassis group, false otherwise. */
 bool
 ha_chassis_group_is_active(
diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
index bfd8680ce..86874f106 100644
--- a/ovn-architecture.7.xml
+++ b/ovn-architecture.7.xml
@@ -1813,7 +1813,7 @@
   
 When multiple chassis have been specified for a gateway, all chassis that
 may send packets to that gateway will enable BFD on tunnels to all
-configured gateway chassis.  The current master chassis for the gateway
+configured gateway chassis.  The current active chassis for the gateway
 is the highest priority gateway chassis that is currently viewed as
 active based on BFD status.
   
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 5cb6ba640..3382d4db6 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -943,7 +943,7 @@
   send any traffic to this port. OVN can support
   native services like DHCPv4/DHCPv6/DNS for this port.
   If  is defined,
-  ovn-controller running in the master chassis of
+  ovn-controller running in the active chassis of
   the HA chassis group will bind this port to provide these native
   services. It is expected that this port belong to a bridged
   logical switch (with a localnet port).
@@ -4794,10 +4794,10 @@ or
   Table representing a group of chassis which can provide high availability
   services. Each chassis in the group is represented by the table
   . The HA chassis with highest priority will
-  be the master of this group. If the master chassis failover is detected,
-  the HA chassis with the next higher priority takes over the
+  be the active chassis of this group. If the active chassis failover is
+  detected, the HA chassis with the next higher priority takes over the
   responsibility of providing the HA. If a distributed gateway router port
-  references a row in this table, then the master HA chassis in this group
+  references a row in this table, then the active HA chassis in this group
   provides the gateway functionality.
 
 
@@ -4830,7 +4830,7 @@ or
 
   
 Priority of the chassis. Chassis with highest priority will be
-the master.
+the active chassis.
   
 
 
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 507a0b571..73a1be5ed 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4682,7 +4682,7 @@ tcp.flags = RST;
 
   
 Priority of the HA chassis. Chassis with highest priority will be
-the master in the HA chassis group.
+the active chassis in the HA chassis group.
   
 
 
@@ -4698,13 +4698,13 @@ tcp.flags = RST;
   Table representing a group of chassis which can provide High availability
   

[ovs-dev] [PATCH ovn 4/4] Inclusive language substitutions: "sanity-check".

2024-05-07 Thread Mark Michelson
This change introduces alternatives for "sanity-check" (and its
variants) throughout the code.

Places where "sanity-check" is not replaced:
* Many libtool-related files are auto-generated and contain the phrase.
  Since these files are auto-generated, I did not update these files.

Signed-off-by: Mark Michelson 
---
 lib/inc-proc-eng.h| 2 +-
 tests/ovn-northd.at   | 4 ++--
 utilities/ovn-nbctl.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 68bbd8266..5bb3b8f3e 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -342,7 +342,7 @@ bool engine_canceled(void);
  */
 void *engine_get_data(struct engine_node *node);
 
-/* Return a pointer to node data *without* performing any sanity checks on
+/* Return a pointer to node data *without* performing any coherance checks on
  * the state of the node. This may be used only in specific cases when data
  * is guaranteed to be valid, e.g., immediately after initialization and
  * before the first engine_run().
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index ec18d2dd4..6a7eb839a 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -7340,7 +7340,7 @@ AT_CHECK([cat log_flows], [0], [dnl
 
 rm log_flows
 
-# And just for sanity's sake, let's remove the allow-related ACL and make sure
+# And just to verify, let's remove the allow-related ACL and make sure
 # all the special log messages are gone.
 check ovn-nbctl acl-del pg2
 check ovn-nbctl --wait=sb sync
@@ -7445,7 +7445,7 @@ AT_CHECK([cat log_flows], [0], [dnl
 
 rm log_flows
 
-# And just for sanity's sake, let's remove the allow-related ACL and make sure
+# And just to verify, let's remove the allow-related ACL and make sure
 # all the special log messages are gone.
 check ovn-nbctl acl-del pg2
 check ovn-nbctl --wait=sb sync
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 618f3a18b..99d2cba8c 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -5992,7 +5992,7 @@ nbctl_lrp_add(struct ctl_context *ctx)
 return;
 }
 
-/* Special-case sanity-check of peer ports. */
+/* Special-case coherance-check of peer ports. */
 const char *peer = NULL;
 for (int i = 0; i < n_settings; i++) {
 if (!strncmp(settings[i], "peer=", 5)) {
-- 
2.44.0

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


Re: [ovs-dev] [PATCH ovn v2 0/4] Mac cache handling refactor

2024-05-07 Thread Ales Musil
On Tue, May 7, 2024 at 6:49 PM Numan Siddique  wrote:

> On Tue, May 7, 2024 at 2:25 AM Ales Musil  wrote:
> >
> > There were two modules in controller mac_cache and mac-learn, both of
> > them did very similar thing with pretty big overlap. The goal of the
> > series is to consolidate and merge both of those modules into single
> > one. That will reduce the duplication and should make it easier for
> > future updates to MAC binding, FDB or packet buffering functionality.
> >
> > There is also fix to properly handle tunnel_key change for LSP, LRP,
> > LR and LS. This was inconsistent and could lead to wrong flows being
> > still present even after the tunnel key change. This is not a huge
> > issue because the tunnel_key is rarelyt changed during runtime.
> >
> > Ales Musil (4):
> >   northd, controller: Handle tunnel_key change consistently.
> >   controller: Rename mac_cache to to mac-cache.
> >   controller: Merge the mac-cache and mac-learn.
> >   controller: Use datapath key for the mac cache thresholds.
>
> Thanks.  I applied the series to the main branch.
> Do we need a backport ?  If so,  I'm inclined to backport the first
> patch only as it fixes the issue.
> Let me know your thoughts.
>

That's correct, only the first patch should be backported.


>
> Numan
>
>
> >
> >  controller/automake.mk  |   6 +-
> >  controller/binding.c|  13 +-
> >  controller/mac-cache.c  | 745 
> >  controller/mac-cache.h  | 210 ++
> >  controller/mac-learn.c  | 482 ---
> >  controller/mac-learn.h  | 145 ---
> >  controller/mac_cache.c  | 547 --
> >  controller/mac_cache.h  | 124 --
> >  controller/ovn-controller.c | 214 +++
> >  controller/pinctrl.c| 165 
> >  controller/statctrl.c   |   7 +-
> >  controller/statctrl.h   |   2 +-
> >  northd/northd.c |   7 +
> >  tests/ovn.at|  56 ++-
> >  14 files changed, 1253 insertions(+), 1470 deletions(-)
> >  create mode 100644 controller/mac-cache.c
> >  create mode 100644 controller/mac-cache.h
> >  delete mode 100644 controller/mac-learn.c
> >  delete mode 100644 controller/mac-learn.h
> >  delete mode 100644 controller/mac_cache.c
> >  delete mode 100644 controller/mac_cache.h
> >
> > --
> > 2.44.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
Thanks,
Ales

-- 

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 v2 0/4] Mac cache handling refactor

2024-05-07 Thread Numan Siddique
On Tue, May 7, 2024 at 2:25 AM Ales Musil  wrote:
>
> There were two modules in controller mac_cache and mac-learn, both of
> them did very similar thing with pretty big overlap. The goal of the
> series is to consolidate and merge both of those modules into single
> one. That will reduce the duplication and should make it easier for
> future updates to MAC binding, FDB or packet buffering functionality.
>
> There is also fix to properly handle tunnel_key change for LSP, LRP,
> LR and LS. This was inconsistent and could lead to wrong flows being
> still present even after the tunnel key change. This is not a huge
> issue because the tunnel_key is rarelyt changed during runtime.
>
> Ales Musil (4):
>   northd, controller: Handle tunnel_key change consistently.
>   controller: Rename mac_cache to to mac-cache.
>   controller: Merge the mac-cache and mac-learn.
>   controller: Use datapath key for the mac cache thresholds.

Thanks.  I applied the series to the main branch.
Do we need a backport ?  If so,  I'm inclined to backport the first
patch only as it fixes the issue.
Let me know your thoughts.

Numan


>
>  controller/automake.mk  |   6 +-
>  controller/binding.c|  13 +-
>  controller/mac-cache.c  | 745 
>  controller/mac-cache.h  | 210 ++
>  controller/mac-learn.c  | 482 ---
>  controller/mac-learn.h  | 145 ---
>  controller/mac_cache.c  | 547 --
>  controller/mac_cache.h  | 124 --
>  controller/ovn-controller.c | 214 +++
>  controller/pinctrl.c| 165 
>  controller/statctrl.c   |   7 +-
>  controller/statctrl.h   |   2 +-
>  northd/northd.c |   7 +
>  tests/ovn.at|  56 ++-
>  14 files changed, 1253 insertions(+), 1470 deletions(-)
>  create mode 100644 controller/mac-cache.c
>  create mode 100644 controller/mac-cache.h
>  delete mode 100644 controller/mac-learn.c
>  delete mode 100644 controller/mac-learn.h
>  delete mode 100644 controller/mac_cache.c
>  delete mode 100644 controller/mac_cache.h
>
> --
> 2.44.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 v2] northd: Support routing over other address families.

2024-05-07 Thread Numan Siddique
On Tue, May 7, 2024 at 11:10 AM Felix Huettner via dev
 wrote:
>
> On Tue, May 07, 2024 at 10:06:07AM -0400, Numan Siddique wrote:
> > On Tue, May 7, 2024 at 5:24 AM Felix Huettner via dev
> >  wrote:
> > >
> > > On Mon, May 06, 2024 at 11:03:42PM -0400, Numan Siddique wrote:
> > > > On Mon, Apr 22, 2024 at 9:02 AM Felix Huettner via dev
> > > >  wrote:
> > > > >
> > > > > In most cases IPv4 packets are routed only over other IPv4 networks 
> > > > > and
> > > > > IPv6 packets are routed only over IPv6 networks. However there is no
> > > > > inherent reason for this limitation. Routing IPv4 packets over IPv6
> > > > > networks just requires the router to contain a route for an IPv4 
> > > > > network
> > > > > with an IPv6 nexthop.
> > > > >
> > > > > This was previously prevented in OVN in ovn-nbctl and northd. By
> > > > > removing these filters the forwarding will work if the mac addresses 
> > > > > are
> > > > > prepopulated.
> > > > >
> > > > > If the mac addresses are not prepopulated we will attempt to resolve 
> > > > > them using
> > > > > the original address family of the packet and not the address family 
> > > > > of the
> > > > > nexthop. This will fail and we will not forward the packet.
> > > > >
> > > >
> > > > Thanks for adding this feature.
> > > >
> > > > If I understand correctly from the above commit message about
> > > > prepopulation,  is it expected that
> > > > CMS/admin needs to add a static mac binding entry in the OVN
> > > > Northbound 'Static_MAC_Binding' table
> > > > for this feature to work ?  Please correct me if I'm wrong.
> > > > If this is the case, then it needs to be documented properly (perhaps
> > > > in ovn-nb.xml)
> > >
> > > no the limitation is only that the CMS must not set
> > > dynamic_neigh_routers to true on the Logical_Routers.
> > >
> > > If there are other reasons that cause OVN to send ARP request to lookup
> > > the nexthop of a route then these will have that issue as well. I am
> > > just no aware of any such reason.
> >
> > Thanks.  I'm still not clear what happens if OVN doesn't know the
> > nexthop mac address.
> > What happens in that case ? Normally, OVN would generate ARP request
> > for the next hop IP and
> > learn the mac.  What happens in the mix route case ?
> >
> > Numan
>
> So lets assume you run an ipv4 prefix and route that over an ipv6 network.
> If you then reach the Logical_Router you will reach the arp action in
> S_ROUTER_IN_ARP_REQUEST (build_arp_request_flows_for_lrouter).
>
> This will be handled by the node executing that action as a
> packet-in to pinctrl_handle_arp which is here actually inappropriate.
> We would need to reach pinctrl_handle_nd_na as we need to send a
> neighbor discovery packet.
>
> It could be changed in northd.c when using an additional register bit
> to store the nexthop address family and then using that bit to figure
> out what address family we need to lookup against, instead of relying on
> the packet address family.
>
> However if we end up in pinctrl_handle_nd_na we seem to reuse the
> existing ipv4 packet and patch the Neighbor Discovery packet over it.
> I thought this causes the lookup then to fail, as the address families
> missmatch.
> However i am no longer sure of that explanation after writing this :)

Let's say we add this static route -  "prefix = 172.168.0.0/24,
nexthop = aef0::4"
and lets assume OVN doesn't know the next hop "aef0::4" mac address.
So with your patch, the original packet will be dropped because
pinctrl.c is not handling it properly.

I see 2 approaches here :
1.  Fix it in ovn-northd/pinctrl so that for IPv6 next hop, neigh
solicit packet is generated and for IPv4 next hop, ARP request packet
is generated.
2.  CMS should add a static mac binding entry in the OVN Northbound db
for the next hop for this scenario to work properly.
 Eg.  ovn-nbctl static-mac-binding-add lr0-public aef0::5 10:14:00:00:00:05
 In this case, ovn-controller(s) will add an openflow  like below

---
 ovs-ofctl dump-flows br-int table=66
 cookie=0xa4249f73, duration=470.024s, table=66, n_packets=0,
n_bytes=0, 
priority=50,reg0=0xaef0,reg1=0,reg2=0,reg3=0x5,reg15=0x3,metadata=0x3
actions=mod_dl_dst:10:14:00:00:00:05,load:0x1->NXM_NX_REG10[6]


ovn-northd adds below logical flows in the router ingress pipeline
stage - "lr_in_arp_resolve".  These logical flows are hit if OVN can't
resolve the next hop mac address

  table=20(lr_in_arp_resolve  ), priority=1, match=(ip4),
action=(get_arp(outport, reg0); next;)
  table=20(lr_in_arp_resolve  ), priority=1, match=(ip6),
action=(get_nd(outport, xxreg0); next;)

The OVN actions get_arp and get_nd gets translated to OVS actions -
resubmit(,66).
The problem is that for IPv4 packets, reg0 is used and for IPv6
packetsm xxreg0 is used.  But with your patch to support mixed static
routes,  it will not work.

For both the approaches (whichever we chose to take),  we need to
solve this problem.  Maybe we can use a register bit to indicate i

Re: [ovs-dev] [PATCH] ci: Set platform parameter when building DPDK.

2024-05-07 Thread Simon Horman
On Tue, May 07, 2024 at 05:04:34PM +0200, David Marchand wrote:
> This change has no impact, since -Dmachine=default gets converted by
> DPDK into -Dplatform=generic (since v21.08, see the link to DPDK commit
> below). Yet, switch to explicitly setting -Dplatform and avoid the
> following warning:
> 
> 2024-04-18T14:50:16.8001092Z config/meson.build:113: WARNING: The
>   "machine" option is deprecated. Please use "cpu_instruction_set"
>   instead.
> 
> While at it, solve another warning and call explicitly meson setup.
> 
> 2024-04-18T14:50:17.0770596Z WARNING: Running the setup command as
>   `meson [options]` instead of `meson setup [options]` is ambiguous
>   and deprecated.
> 
> Link: https://git.dpdk.org/dpdk/commit/?id=bf66003b51ec
> Signed-off-by: David Marchand 

Acked-by: Simon Horman 

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


Re: [ovs-dev] [Patch] ovsdb-client: Add missing arg to help for 'dump'.

2024-05-07 Thread Simon Horman
On Tue, May 07, 2024 at 03:16:25PM +0100, Simon Horman wrote:
> On Fri, May 03, 2024 at 02:22:27PM +0200, martin.kal...@canonical.com wrote:

...

> Thanks,
> 
> Sorry for the confusion on my part.
> Now that I can see it again I have applied it to main.
> 
> - ovsdb-client: Add missing arg to help for 'dump'.
>   https://github.com/openvswitch/ovs/commit/0940a51b1f5a
> 
> As a follow-up I'll look at backporting this.

Hi Martin,

In preparing the backports I noticed that dump also supports optional
TABLES arguments. Could you consider a follow-up patch to add
that to the help text too?

In any case, I have prepared and pushed the following backports.

branch-3.3:
- ovsdb-client: Add missing arg to help for 'dump'.
  https://github.com/openvswitch/ovs/commit/20ed5491c53f

branch-3.2:
- ovsdb-client: Add missing arg to help for 'dump'.
  https://github.com/openvswitch/ovs/commit/a3f9c5ae1b1e

branch-3.1:
- ovsdb-client: Add missing arg to help for 'dump'.
  https://github.com/openvswitch/ovs/commit/8b029bd258d5

branch-3.0:
- ovsdb-client: Add missing arg to help for 'dump'.
  https://github.com/openvswitch/ovs/commit/65dcd5f27da0

branch-2.17:
- ovsdb-client: Add missing arg to help for 'dump'.
  https://github.com/openvswitch/ovs/commit/d1a2af7c33fa

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


[ovs-dev] [PATCH] odp-execute: Fix AVX checksum calculation.

2024-05-07 Thread Emma Finn
The AVX implementation for calcualting checksum was not
handling carry-over addition correctly in some cases.
This patch adds an additional shuffle to add 16-bit padding
to the final part of the calculation to handle such cases.

Fixes: 92eb03f7b03a ("odp-execute: Add ISA implementation of set_masked IPv4 
action")
Signed-off-by: Emma Finn 
Reported-by: Eelco Chaudron 
---
 lib/odp-execute-avx512.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
index 50c48bfd4..911fdd3ea 100644
--- a/lib/odp-execute-avx512.c
+++ b/lib/odp-execute-avx512.c
@@ -366,6 +366,8 @@ avx512_get_delta(__m256i old_header, __m256i new_header)
   0xF, 0xF, 0xF, 0xF);
 v_delta = _mm256_permutexvar_epi32(v_swap32a, v_delta);
 
+v_delta = _mm256_hadd_epi32(v_delta, v_zeros);
+v_delta = _mm256_shuffle_epi8(v_delta, v_swap16a);
 v_delta = _mm256_hadd_epi32(v_delta, v_zeros);
 v_delta = _mm256_hadd_epi16(v_delta, v_zeros);
 
-- 
2.25.1

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


Re: [ovs-dev] [PATCH ovn v2] northd: Support routing over other address families.

2024-05-07 Thread Felix Huettner via dev
On Tue, May 07, 2024 at 10:06:07AM -0400, Numan Siddique wrote:
> On Tue, May 7, 2024 at 5:24 AM Felix Huettner via dev
>  wrote:
> >
> > On Mon, May 06, 2024 at 11:03:42PM -0400, Numan Siddique wrote:
> > > On Mon, Apr 22, 2024 at 9:02 AM Felix Huettner via dev
> > >  wrote:
> > > >
> > > > In most cases IPv4 packets are routed only over other IPv4 networks and
> > > > IPv6 packets are routed only over IPv6 networks. However there is no
> > > > inherent reason for this limitation. Routing IPv4 packets over IPv6
> > > > networks just requires the router to contain a route for an IPv4 network
> > > > with an IPv6 nexthop.
> > > >
> > > > This was previously prevented in OVN in ovn-nbctl and northd. By
> > > > removing these filters the forwarding will work if the mac addresses are
> > > > prepopulated.
> > > >
> > > > If the mac addresses are not prepopulated we will attempt to resolve 
> > > > them using
> > > > the original address family of the packet and not the address family of 
> > > > the
> > > > nexthop. This will fail and we will not forward the packet.
> > > >
> > >
> > > Thanks for adding this feature.
> > >
> > > If I understand correctly from the above commit message about
> > > prepopulation,  is it expected that
> > > CMS/admin needs to add a static mac binding entry in the OVN
> > > Northbound 'Static_MAC_Binding' table
> > > for this feature to work ?  Please correct me if I'm wrong.
> > > If this is the case, then it needs to be documented properly (perhaps
> > > in ovn-nb.xml)
> >
> > no the limitation is only that the CMS must not set
> > dynamic_neigh_routers to true on the Logical_Routers.
> >
> > If there are other reasons that cause OVN to send ARP request to lookup
> > the nexthop of a route then these will have that issue as well. I am
> > just no aware of any such reason.
> 
> Thanks.  I'm still not clear what happens if OVN doesn't know the
> nexthop mac address.
> What happens in that case ? Normally, OVN would generate ARP request
> for the next hop IP and
> learn the mac.  What happens in the mix route case ?
> 
> Numan

So lets assume you run an ipv4 prefix and route that over an ipv6 network.
If you then reach the Logical_Router you will reach the arp action in
S_ROUTER_IN_ARP_REQUEST (build_arp_request_flows_for_lrouter).

This will be handled by the node executing that action as a
packet-in to pinctrl_handle_arp which is here actually inappropriate.
We would need to reach pinctrl_handle_nd_na as we need to send a
neighbor discovery packet.

It could be changed in northd.c when using an additional register bit
to store the nexthop address family and then using that bit to figure
out what address family we need to lookup against, instead of relying on
the packet address family.

However if we end up in pinctrl_handle_nd_na we seem to reuse the
existing ipv4 packet and patch the Neighbor Discovery packet over it.
I thought this causes the lookup then to fail, as the address families
missmatch.
However i am no longer sure of that explanation after writing this :)

Thanks
Felix

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


Re: [ovs-dev] [PATCH v4 12/12] documentation: Document ovs-flowviz.

2024-05-07 Thread 0-day Robot
Bleep bloop.  Greetings Adrian Moreno, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 81 characters long (recommended limit is 79)
#546 FILE: Documentation/ref/ovs-flowviz.8.rst:464:
   [! | not ] {key}[[.subkey]...] [OPERATOR] {value})] [LOGICAL OPERATOR] 
...

WARNING: Line is 80 characters long (recommended limit is 79)
#560 FILE: Documentation/ref/ovs-flowviz.8.rst:478:
  To compare against a match or info field, use the field directly, e.g:

WARNING: Line is 80 characters long (recommended limit is 79)
#567 FILE: Documentation/ref/ovs-flowviz.8.rst:485:
  Actions values might be dictionaries, use subkeys to access individual

WARNING: Line is 110 characters long (recommended limit is 79)
#603 FILE: Documentation/ref/ovs-flowviz.8.rst:521:
$ ovs-flowviz -i flows.txt --style "light" --highlight "n_packets > 0 and 
drop" openflow html > flows.html

WARNING: Line is 141 characters long (recommended limit is 79)
#699 FILE: Documentation/topics/flow-visualization.rst:80:
  cookie=0xf76b4b20, duration=765.107s, table=0, n_packets=0, n_bytes=0, 
priority=180,vlan_tci=0x/0x1000 actions=conjunction(100,2/2)

WARNING: Line is 328 characters long (recommended limit is 79)
#700 FILE: Documentation/topics/flow-visualization.rst:81:
  cookie=0xf76b4b20, duration=765.107s, table=0, n_packets=0, n_bytes=0, 
priority=180,conj_id=100,in_port="patch-br-int-to",vlan_tci=0x/0x1000 
actions=load:0xa->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],load:0xb->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],mod_dl_src:02:42:ac:12:00:03,resubmit(,8)

WARNING: Line is 286 characters long (recommended limit is 79)
#701 FILE: Documentation/topics/flow-visualization.rst:82:
  cookie=0x0, duration=765.388s, table=0, n_packets=0, n_bytes=0, 
priority=100,in_port="ovn-6bb3b3-0" 
actions=move:NXM_NX_TUN_ID[0..23]->OXM_OF_METADATA[0..23],move:NXM_NX_TUN_METADATA0[16..30]->NXM_NX_REG14[0..14],move:NXM_NX_TUN_METADATA0[0..15]->NXM_NX_REG15[0..15],resubmit(,40)

WARNING: Line is 286 characters long (recommended limit is 79)
#702 FILE: Documentation/topics/flow-visualization.rst:83:
  cookie=0x0, duration=765.388s, table=0, n_packets=0, n_bytes=0, 
priority=100,in_port="ovn-a6ff98-0" 
actions=move:NXM_NX_TUN_ID[0..23]->OXM_OF_METADATA[0..23],move:NXM_NX_TUN_METADATA0[16..30]->NXM_NX_REG14[0..14],move:NXM_NX_TUN_METADATA0[0..15]->NXM_NX_REG15[0..15],resubmit(,40)

WARNING: Line is 262 characters long (recommended limit is 79)
#703 FILE: Documentation/topics/flow-visualization.rst:84:
  cookie=0xf2ca6195, duration=765.107s, table=0, n_packets=6, n_bytes=636, 
priority=100,in_port="ovn-k8s-mp0" 
actions=load:0x1->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x2->NXM_NX_REG14[],resubmit(,8)

WARNING: Line is 266 characters long (recommended limit is 79)
#704 FILE: Documentation/topics/flow-visualization.rst:85:
  cookie=0x236e941d, duration=408.874s, table=0, n_packets=11, n_bytes=846, 
priority=100,in_port=aceac9829941d11 
actions=load:0x11->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x3->NXM_NX_REG14[],resubmit(,8)

WARNING: Line is 268 characters long (recommended limit is 79)
#705 FILE: Documentation/topics/flow-visualization.rst:86:
  cookie=0x3facf689, duration=405.581s, table=0, n_packets=11, n_bytes=846, 
priority=100,in_port="363ba22029cd92b" 
actions=load:0x12->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x4->NXM_NX_REG14[],resubmit(,8)

WARNING: Line is 268 characters long (recommended limit is 79)
#706 FILE: Documentation/topics/flow-visualization.rst:87:
  cookie=0xe7c8c4bb, duration=405.570s, table=0, n_packets=11, n_bytes=846, 
priority=100,in_port="6a62cde0d50ef44" 
actions=load:0x13->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x5->NXM_NX_REG14[],resubmit(,8)

WARNING: Line is 266 characters long (recommended limit is 79)
#707 FILE: Documentation/topics/flow-visualization.rst:88:
  cookie=0x99a0ffc1, duration=59.391s, table=0, n_packets=8, n_bytes=636, 
priority=100,in_port="5ff3bfaaa4eb622" 
actions=load:0x14->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x6->NXM_NX_REG14[],resubmit(,8)

WARNING: Line is 266 characters long (recommended limit is 79)
#708 FILE: Documentation/topics/flow-visualization.rst:89:
  cookie=0xe1b5c263, duration=59.365s, table=0, n_packets=8, n_bytes=636, 
priority=100,in_port="8d9e0bc76347e59" 
actions=load:0x15->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x7->NXM_NX_REG14[],resubmit(,8)

WARNING: Line is 123 characters long (recommended limit is 79)
#719 FILE: Docume

[ovs-dev] [PATCH] ci: Set platform parameter when building DPDK.

2024-05-07 Thread David Marchand
This change has no impact, since -Dmachine=default gets converted by
DPDK into -Dplatform=generic (since v21.08, see the link to DPDK commit
below). Yet, switch to explicitly setting -Dplatform and avoid the
following warning:

2024-04-18T14:50:16.8001092Z config/meson.build:113: WARNING: The
"machine" option is deprecated. Please use "cpu_instruction_set"
instead.

While at it, solve another warning and call explicitly meson setup.

2024-04-18T14:50:17.0770596Z WARNING: Running the setup command as
`meson [options]` instead of `meson setup [options]` is ambiguous
and deprecated.

Link: https://git.dpdk.org/dpdk/commit/?id=bf66003b51ec
Signed-off-by: David Marchand 
---
 .ci/dpdk-build.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
index 23f3166a54..e1b8e3ccbb 100755
--- a/.ci/dpdk-build.sh
+++ b/.ci/dpdk-build.sh
@@ -25,9 +25,9 @@ function build_dpdk()
 pushd dpdk-src
 fi
 
-# Switching to 'default' machine to make the dpdk cache usable on
+# Switching to 'generic' platform to make the dpdk cache usable on
 # different CPUs. We can't be sure that all CI machines are exactly same.
-DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
+DPDK_OPTS="$DPDK_OPTS -Dplatform=generic"
 
 # Disable building DPDK unit tests. Not needed for OVS build or tests.
 DPDK_OPTS="$DPDK_OPTS -Dtests=false"
@@ -49,7 +49,7 @@ function build_dpdk()
 # Install DPDK using prefix.
 DPDK_OPTS="$DPDK_OPTS --prefix=$DPDK_INSTALL_DIR"
 
-meson $DPDK_OPTS build
+meson setup $DPDK_OPTS build
 ninja -C build
 ninja -C build install
 popd
-- 
2.44.0

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


[ovs-dev] [PATCH v4 12/12] documentation: Document ovs-flowviz.

2024-05-07 Thread Adrian Moreno
Add a man page for ovs-flowviz as well as a topic page with some more
detailed examples.

Signed-off-by: Adrian Moreno 
---
 Documentation/automake.mk   |   4 +-
 Documentation/conf.py   |   2 +
 Documentation/ref/index.rst |   1 +
 Documentation/ref/ovs-flowviz.8.rst | 531 
 Documentation/topics/flow-visualization.rst | 271 ++
 Documentation/topics/index.rst  |   1 +
 rhel/openvswitch-fedora.spec.in |   1 +
 rhel/openvswitch.spec.in|   1 +
 8 files changed, 811 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ref/ovs-flowviz.8.rst
 create mode 100644 Documentation/topics/flow-visualization.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 47d2e336a..539870aa2 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -45,7 +45,7 @@ DOC_SOURCE = \
Documentation/topics/fuzzing/ovs-fuzzing-infrastructure.rst \
Documentation/topics/fuzzing/ovs-fuzzers.rst \
Documentation/topics/fuzzing/security-analysis-of-ovs-fuzzers.rst \
-   Documentation/topics/testing.rst \
+   Documentation/topics/flow-visualization.rst \
Documentation/topics/integration.rst \
Documentation/topics/language-bindings.rst \
Documentation/topics/networking-namespaces.rst \
@@ -55,6 +55,7 @@ DOC_SOURCE = \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
Documentation/topics/record-replay.rst \
+   Documentation/topics/testing.rst \
Documentation/topics/tracing.rst \
Documentation/topics/usdt-probes.rst \
Documentation/topics/userspace-checksum-offloading.rst \
@@ -162,6 +163,7 @@ RST_MANPAGES = \
ovs-actions.7.rst \
ovs-appctl.8.rst \
ovs-ctl.8.rst \
+   ovs-flowviz.8.rst \
ovs-l3ping.8.rst \
ovs-parse-backtrace.8.rst \
ovs-pki.8.rst \
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 15785605a..3a82f23a7 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -120,6 +120,8 @@ _man_pages = [
  u'utility for configuring running Open vSwitch daemons'),
 ('ovs-ctl.8',
  u'OVS startup helper script'),
+('ovs-flowviz.8',
+ u'utility for visualizing OpenFlow and datapath flows'),
 ('ovs-l3ping.8',
  u'check network deployment for L3 tunneling problems'),
 ('ovs-parse-backtrace.8',
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 03ada932f..7f2fe6177 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -42,6 +42,7 @@ time:
ovs-actions.7
ovs-appctl.8
ovs-ctl.8
+   ovs-flowviz.8
ovs-l3ping.8
ovs-pki.8
ovs-sim.1
diff --git a/Documentation/ref/ovs-flowviz.8.rst 
b/Documentation/ref/ovs-flowviz.8.rst
new file mode 100644
index 0..77b89fc65
--- /dev/null
+++ b/Documentation/ref/ovs-flowviz.8.rst
@@ -0,0 +1,531 @@
+..
+  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.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+===
+ovs-flowviz
+===
+
+Synopsis
+
+
+``ovs-flowviz``
+[``[-i | --input] <[alias,]file>``]
+[``[-c | --config] ``]
+[``[-f | --filter] ``]
+[``[-h | --highlight] ``]
+[``--style 

[ovs-dev] [PATCH v4 11/12] python: ovs: flowviz: Support html dark style.

2024-05-07 Thread Adrian Moreno
In order to support dark style in html outputs, allow the config file to
express the background color and set it in a top style object.

Acked-by: Eelco Chaudron 
Signed-off-by: Adrian Moreno 
---
 python/ovs/flowviz/html_format.py   |  4 +++-
 python/ovs/flowviz/odp/html.py  | 30 -
 python/ovs/flowviz/ofp/html.py  | 28 ++-
 python/ovs/flowviz/ovs-flowviz.conf | 20 +++
 4 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/python/ovs/flowviz/html_format.py 
b/python/ovs/flowviz/html_format.py
index ebfa65c34..3f3550da5 100644
--- a/python/ovs/flowviz/html_format.py
+++ b/python/ovs/flowviz/html_format.py
@@ -48,7 +48,9 @@ class HTMLBuffer(FlowBuffer):
 style = ' style="color:{}"'.format(color) if color else ""
 self._text += "".format(style)
 if href:
-self._text += "".format(href)
+self._text += " ".format(
+href, 'style="color:{}"'.format(color) if color else ""
+)
 self._text += string
 if href:
 self._text += ""
diff --git a/python/ovs/flowviz/odp/html.py b/python/ovs/flowviz/odp/html.py
index 4aa08dc70..b2855bf40 100644
--- a/python/ovs/flowviz/odp/html.py
+++ b/python/ovs/flowviz/odp/html.py
@@ -55,10 +55,18 @@ class HTMLTree(FlowTree):
 flows(dict[int, list[DPFlow]): Optional; initial flows
 """
 
+html_body_style = """
+
+body {{
+background-color: {bg};
+color: {fg};
+}}
+"""
+
 html_header = """
 
 .flow{
-background-color:white;
+background-color:inherit;
 display: inline-block;
 text-align: left;
 font-family: monospace;
@@ -177,9 +185,9 @@ class HTMLTree(FlowTree):
 append()
 """
 
-def __init__(self, parent_name, flow=None, opts=None):
+def __init__(self, parent_name, flow=None, fmt=None, opts=None):
 self._parent_name = parent_name
-self._formatter = HTMLFormatter(opts)
+self._formatter = fmt
 self._opts = opts
 super(HTMLTree.HTMLTreeElem, self).__init__(flow)
 
@@ -232,13 +240,14 @@ class HTMLTree(FlowTree):
 def __init__(self, name, opts, flows=None):
 self.opts = opts
 self.name = name
+self._formatter = HTMLFormatter(opts)
 super(HTMLTree, self).__init__(
 flows, self.HTMLTreeElem("", flow=None, opts=self.opts)
 )
 
 def _new_elem(self, flow, _):
 """Override _new_elem to provide HTMLTreeElems."""
-return self.HTMLTreeElem(self.name, flow, self.opts)
+return self.HTMLTreeElem(self.name, flow, self._formatter, self.opts)
 
 def render(self):
 """Render the Tree in HTML.
@@ -247,10 +256,21 @@ class HTMLTree(FlowTree):
 an html string representing the element
 """
 name = self.name.replace(" ", "_")
+bg = (
+self._formatter.style.get("background").color
+if self._formatter.style.get("background")
+else "white"
+)
+fg = (
+self._formatter.style.get("default").color
+if self._formatter.style.get("default")
+else "black"
+)
 
 html_text = """
 """  # noqa: E501
-html_obj = self.html_header + html_text.format(name=name)
+html_obj = self.html_body_style.format(bg=bg, fg=fg)
+html_obj += self.html_header + html_text.format(name=name)
 
 html_obj += "
".format(name=name) (html_elem, _) = self.root.render() diff --git a/python/ovs/flowviz/ofp/html.py b/python/ovs/flowviz/ofp/html.py index a66f5fe8e..b00ca58f0 100644 --- a/python/ovs/flowviz/ofp/html.py +++ b/python/ovs/flowviz/ofp/html.py @@ -24,6 +24,7 @@ class HTMLProcessor(OpenFlowFactory, FileProcessor): def __init__(self, opts): super().__init__(opts) +self.formatter = HTMLFormatter(self.opts) self.data = dict() def start_file(self, name, filename): @@ -39,21 +40,38 @@ class HTMLProcessor(OpenFlowFactory, FileProcessor): self.tables[table].append(flow) def html(self): -html_obj = "" +bg = ( +self.formatter.style.get("background").color +if self.formatter.style.get("background") +else "white" +) +fg = ( +self.formatter.style.get("default").color +if self.formatter.style.get("default") +else "black" +) +html_obj = """ +