[ovs-dev] [PATCH 2/3] ovsdb-dot: Fix flake8 issues.

[Ilya Maximets]

Missing and extra spaces, missing empty lines, unused imports and
variables, long lines.

Decided to just comment out the unused 'tail' and 'head' as they
seem useful in documenting the meaning of the words.

Files added to flake8-check to avoid future issues.

Signed-off-by: Ilya Maximets 
---
 ovsdb/automake.mk  |  1 +
 ovsdb/dot2pic  |  6 +++---
 ovsdb/ovsdb-dot.in | 41 ++---
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index e8149224b..d484fe9de 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -120,6 +120,7 @@ ovsdb/ovsdb-doc: python/ovs/dirs.py
 
 # ovsdb-dot
 EXTRA_DIST += ovsdb/ovsdb-dot.in ovsdb/dot2pic
+FLAKE8_PYFILES += ovsdb/ovsdb-dot.in ovsdb/dot2pic
 noinst_SCRIPTS += ovsdb/ovsdb-dot
 CLEANFILES += ovsdb/ovsdb-dot
 OVSDB_DOT = $(run_python) $(srcdir)/ovsdb/ovsdb-dot.in
diff --git a/ovsdb/dot2pic b/ovsdb/dot2pic
index 2f858e19d..3db6444de 100755
--- a/ovsdb/dot2pic
+++ b/ovsdb/dot2pic
@@ -17,6 +17,7 @@
 import getopt
 import sys
 
+
 def dot2pic(src, dst):
 scale = 1.0
 while True:
@@ -49,8 +50,8 @@ def dot2pic(src, dst):
 dst.write("box at %f,%f wid %f height %f\n"
   % (x, y, width, height))
 elif command == 'edge':
-tail = words[1]
-head = words[2]
+# tail = words[1]
+# head = words[2]
 n = int(words[3])
 
 # Extract x,y coordinates.
@@ -114,4 +115,3 @@ else:
 if font_scale:
 print(".ps %+d" % font_scale)
 print(".PE")
-
diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 41b986c0a..f1eefd49c 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -1,15 +1,13 @@
 #! @PYTHON3@
 
-from datetime import date
 import ovs.db.error
 import ovs.db.schema
 import getopt
-import os
-import re
 import sys
 
 argv0 = sys.argv[0]
 
+
 def printEdge(tableName, type, baseType, label):
 if baseType.ref_table_name:
 if type.n_min == 0:
@@ -31,38 +29,42 @@ def printEdge(tableName, type, baseType, label):
 options['label'] = '"%s%s"' % (label, arity)
 if baseType.ref_type == 'weak':
 options['style'] = 'dotted'
-print ("\t%s -> %s [%s];" % (
+print("\t%s -> %s [%s];" % (
 tableName,
 baseType.ref_table_name,
-', '.join(['%s=%s' % (k,v) for k,v in options.items()])))
+', '.join(['%s=%s' % (k, v) for k, v in options.items()])))
+
 
 def schemaToDot(schemaFile, arrows):
 schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schemaFile))
 
-print ("digraph %s {" % schema.name)
-print ('\trankdir=LR;')
-print ('\tsize="6.5,4";')
-print ('\tmargin="0";')
-print ("\tnode [shape=box];")
+print("digraph %s {" % schema.name)
+print('\trankdir=LR;')
+print('\tsize="6.5,4";')
+print('\tmargin="0";')
+print("\tnode [shape=box];")
 if not arrows:
-print ("\tedge [dir=none, arrowhead=none, arrowtail=none];")
+print("\tedge [dir=none, arrowhead=none, arrowtail=none];")
 for tableName, table in schema.tables.items():
 options = {}
 if table.is_root:
 options['style'] = 'bold'
-print ("\t%s [%s];" % (
+print("\t%s [%s];" % (
 tableName,
-', '.join(['%s=%s' % (k,v) for k,v in options.items()])))
+', '.join(['%s=%s' % (k, v) for k, v in options.items()])))
 for columnName, column in table.columns.items():
 if column.type.value:
-printEdge(tableName, column.type, column.type.key, "%s key" % 
columnName)
-printEdge(tableName, column.type, column.type.value, "%s 
value" % columnName)
+printEdge(tableName, column.type, column.type.key,
+  "%s key" % columnName)
+printEdge(tableName, column.type, column.type.value,
+  "%s value" % columnName)
 else:
 printEdge(tableName, column.type, column.type.key, columnName)
-print ("}");
+print("}")
+
 
 def usage():
-print ("""\
+print("""\
 %(argv0)s: compiles ovsdb schemas to graphviz format
 Prints a .dot file that "dot" can render to an entity-relationship diagram
 usage: %(argv0)s [OPTIONS] SCHEMA
@@ -75,12 +77,13 @@ The following options are also available:
 """ % {'argv0': argv0})
 sys.exit(0)
 
+
 if __name__ == "__main__":
 try:
 try:
 options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
   ['no-arrows',
-   'help', 'version',])
+   'help', 'version'])
 except getopt.GetoptError as geo:
 sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
 sys.exit(1)
@@ -92,7 +95,7 @@ if __name__ == "__main__":
 elif key in ['-h', 

[ovs-dev] [PATCH 3/3] github: Update python to 3.12.

[Ilya Maximets]

We pinned the python version to 3.9 because we had issues building
older meson 0.47.1 with python 3.10.  Since then meson was updated to
0.53.2 in our CI, but we didn't reconsider the python version.

Newer versions of python uncover more issues with our python files.
And newer major distributions are using newer versions of python.  But
we do not really want to use bleeding edge of python releases either to
avoid unexpected CI failures that need immediate fixes.

Pin python version to 3.12 as it is the latest released version and we
should not have any issues with this version.

While at it, updating meson to a newer version that plays nicely with
python 3.12.  We do not really care much about the version we use here
as long as it is able to build the version of DPDK we're using.  Meson
has no LTS releases, as far as I can tell, so just choosing the latest
stable 1.4.x series.  It should be fine to use for a next few years.
Major distributions are using 1.0+ versions.  Upcoming F40 and Ubuntu
24.03 have meson 1.3.

It would also be nice to test the minimal supported version of python,
but 3.6 is not available in setup-python for 22.04.  The oldest is 3.7.
And 3.7 is EoL, so pip fails to install some of our dependencies.  The
oldest version we can use today is 3.8.  But, in the end, this becomes
a race against older python versions reaching end of their life and
packages dropping support of these versions.  This may cause unexpected
CI failures.  So, not doing that for now.

Signed-off-by: Ilya Maximets 
---

CC: David Marchand 
CC: Kevin Traynor 

 .ci/dpdk-prepare.sh  |  2 +-
 .github/workflows/build-and-test.yml | 11 +++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/.ci/dpdk-prepare.sh b/.ci/dpdk-prepare.sh
index f7e6215dd..4424f9eb9 100755
--- a/.ci/dpdk-prepare.sh
+++ b/.ci/dpdk-prepare.sh
@@ -8,4 +8,4 @@ set -ev
 # https://github.com/pypa/pip/issues/10655
 pip3 install --disable-pip-version-check --user wheel
 pip3 install --disable-pip-version-check --user pyelftools
-pip3 install --user  'meson==0.53.2'
+pip3 install --user  'meson>=1.4,<1.5'
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 6f5139304..44491db3e 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -2,6 +2,9 @@ name: Build and Test
 
 on: [push, pull_request]
 
+env:
+  python_default: 3.12
+
 jobs:
   build-dpdk:
 env:
@@ -54,7 +57,7 @@ jobs:
   if: steps.dpdk_cache.outputs.cache-hit != 'true'
   uses: actions/setup-python@v5
   with:
-python-version: '3.9'
+python-version: ${{ env.python_default }}
 
 - name: update APT cache
   if: steps.dpdk_cache.outputs.cache-hit != 'true'
@@ -217,7 +220,7 @@ jobs:
 - name: set up python
   uses: actions/setup-python@v5
   with:
-python-version: '3.9'
+python-version: ${{ env.python_default }}
 
 - name: cache
   if:   matrix.dpdk != '' || matrix.dpdk_shared != ''
@@ -354,7 +357,7 @@ jobs:
 - name: set up python
   uses: actions/setup-python@v5
   with:
-python-version: '3.9'
+python-version: ${{ env.python_default }}
 
 - name: get cached dpdk-dir
   uses: actions/cache/restore@v4
@@ -407,7 +410,7 @@ jobs:
 - name: set up python
   uses: actions/setup-python@v5
   with:
-python-version: '3.9'
+python-version: ${{ env.python_default }}
 - name: install dependencies
   run:  brew install automake libtool
 - name: prepare
-- 
2.44.0

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

[ovs-dev] [PATCH 1/3] ovsdb-doc: Fix syntax warning with Python 3.12 and flake8 issues.

[Ilya Maximets]

ovsdb-doc script generates the following syntax warning while running
with Python 3.12:

  /ovsdb/ovsdb-doc:240: SyntaxWarning: invalid escape sequence '\{'
  s += """

This doesn't cause a build failure because so far it's only a warning,
but it will become a syntax error in the future.

Fix that by converting to a raw string and removing unnecessary
escape sequences.

Adding ovsdb-doc to flake8-check to avoid re-introducing issues in
the future.  This means also fixing all the other issues with the
script like unused imports and variables, long lines, missing empty
lines, wildcarded imports.  Also cleaning up one place that handles
compatibility with Python 2 types, since we do not support Python 2
for a long time now.

Signed-off-by: Ilya Maximets 
---
 ovsdb/automake.mk |  1 +
 ovsdb/ovsdb-doc   | 50 +++
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index eba713bb6..e8149224b 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -114,6 +114,7 @@ $(OVSIDL_BUILT): ovsdb/ovsdb-idlc.in python/ovs/dirs.py
 
 # ovsdb-doc
 EXTRA_DIST += ovsdb/ovsdb-doc
+FLAKE8_PYFILES += ovsdb/ovsdb-doc
 OVSDB_DOC = $(run_python) $(srcdir)/ovsdb/ovsdb-doc
 ovsdb/ovsdb-doc: python/ovs/dirs.py
 
diff --git a/ovsdb/ovsdb-doc b/ovsdb/ovsdb-doc
index 099770d25..2edf487a2 100755
--- a/ovsdb/ovsdb-doc
+++ b/ovsdb/ovsdb-doc
@@ -14,9 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from datetime import date
 import getopt
-import os
 import sys
 import xml.dom.minidom
 
@@ -24,10 +22,13 @@ import ovs.json
 from ovs.db import error
 import ovs.db.schema
 
-from ovs_build_helpers.nroff import *
+from ovs_build_helpers.nroff import block_xml_to_nroff
+from ovs_build_helpers.nroff import escape_nroff_literal
+from ovs_build_helpers.nroff import text_to_nroff
 
 argv0 = sys.argv[0]
 
+
 def typeAndConstraintsToNroff(column):
 type = column.type.toEnglish(escape_nroff_literal)
 constraints = column.type.constraintsToEnglish(escape_nroff_literal,
@@ -38,6 +39,7 @@ def typeAndConstraintsToNroff(column):
 type += " (must be unique within table)"
 return type
 
+
 def columnGroupToNroff(table, groupXml, documented_columns):
 introNodes = []
 columnNodes = []
@@ -49,7 +51,10 @@ def columnGroupToNroff(table, groupXml, documented_columns):
 if (columnNodes
 and not (node.nodeType == node.TEXT_NODE
  and node.data.isspace())):
-raise error.Error("text follows  or  inside 
: %s" % node)
+raise error.Error(
+"text follows  or  inside : %s"
+% node
+)
 introNodes += [node]
 
 summary = []
@@ -65,15 +70,9 @@ def columnGroupToNroff(table, groupXml, documented_columns):
 if node.hasAttribute('type'):
 type_string = node.attributes['type'].nodeValue
 type_json = ovs.json.from_string(str(type_string))
-# py2 -> py3 means str -> bytes and unicode -> str
-try:
-if type(type_json) in (str, unicode):
-raise error.Error("%s %s:%s has invalid 'type': 
%s" 
-  % (table.name, name, key, 
type_json))
-except:
-if type(type_json) in (bytes, str):
-raise error.Error("%s %s:%s has invalid 'type': 
%s" 
-  % (table.name, name, key, 
type_json))
+if type(type_json) in (bytes, str):
+raise error.Error("%s %s:%s has invalid 'type': %s"
+  % (table.name, name, key, type_json))
 type_ = ovs.db.types.BaseType.from_json(type_json)
 else:
 type_ = column.type.value
@@ -91,10 +90,11 @@ def columnGroupToNroff(table, groupXml, documented_columns):
 else:
 if type_.type != column.type.value.type:
 type_english = type_.toEnglish()
+typeNroff += ", containing "
 if type_english[0] in 'aeiou':
-typeNroff += ", containing an %s" % 
type_english
+typeNroff += "an %s" % type_english
 else:
-typeNroff += ", containing a %s" % type_english
+typeNroff += "a %s" % type_english
 constraints = (
 type_.constraintsToEnglish(escape_nroff_literal,
text_to_nroff))
@@ -121,6 +121,7 @@ def columnGroupToNroff(table, 

[ovs-dev] [PATCH 0/3] Fix more issues with Python and update to 3.12.

[Ilya Maximets]

Fixing a few more issues with Python 3.12 and Python files in general.
Switch CI testing to Python 3.12, more explanation in commit messages.

Plan s to also backport these changes to fix issues on older branches
and also to have uniform CI on all branches if possible.

Ilya Maximets (3):
  ovsdb-doc: Fix syntax warning with Python 3.12 and flake8 issues.
  ovsdb-dot: Fix flake8 issues.
  github: Update python to 3.12.

 .ci/dpdk-prepare.sh  |  2 +-
 .github/workflows/build-and-test.yml | 11 +++---
 ovsdb/automake.mk|  2 ++
 ovsdb/dot2pic|  6 ++--
 ovsdb/ovsdb-doc  | 50 ++--
 ovsdb/ovsdb-dot.in   | 41 ---
 6 files changed, 60 insertions(+), 52 deletions(-)

-- 
2.44.0

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

Re: [ovs-dev] [PATCH] rhel/systemd: Set ovsdb-server timeout to 5 minutes

[Ilya Maximets]

On 4/10/24 17:48, Chris Riches wrote:
> If the database is particularly large (multi-GB), ovsdb-server can take

Hi, Chris.  May I ask how did you end up with multi-GB database?
I would understand if it was an OVN Southbound DB, for example,
but why the local database that only stores ports/bridges and
some other not that large things ends up with so much data?

Sounds a little strange.

Best regards, Ilya Maximets.

> several minutes to come up. This tends to fall afoul of the default
> systemd start timeout, which is typically 90s, putting the service into
> an infinite restart loop.
> 
> To avoid this, set the timeout to a more generous 5 minutes.
> 
> This change brings ovsdb-server's timeout in line with ovs-vswitchd,
> which got the same treatment in commit c1c69e8a45 ("rhel/systemd: Set
> ovs-vswitchd timeout to 5 minutes").
> 
> Signed-off-by: Chris Riches 
> ---
>  rhel/usr_lib_systemd_system_ovsdb-server.service | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
> b/rhel/usr_lib_systemd_system_ovsdb-server.service
> index 49dc06e38..558632320 100644
> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -29,3 +29,4 @@ ExecStop=/usr/share/openvswitch/scripts/ovs-ctl 
> --no-ovs-vswitchd stop
>  ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
> ${OVS_USER_OPT} \
> --no-monitor restart $OPTIONS
> +TimeoutSec=300

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

[ovs-dev] [PATCH] rhel/systemd: Set ovsdb-server timeout to 5 minutes

[Chris Riches]

If the database is particularly large (multi-GB), ovsdb-server can take
several minutes to come up. This tends to fall afoul of the default
systemd start timeout, which is typically 90s, putting the service into
an infinite restart loop.

To avoid this, set the timeout to a more generous 5 minutes.

This change brings ovsdb-server's timeout in line with ovs-vswitchd,
which got the same treatment in commit c1c69e8a45 ("rhel/systemd: Set
ovs-vswitchd timeout to 5 minutes").

Signed-off-by: Chris Riches 
---
 rhel/usr_lib_systemd_system_ovsdb-server.service | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 49dc06e38..558632320 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -29,3 +29,4 @@ ExecStop=/usr/share/openvswitch/scripts/ovs-ctl 
--no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
${OVS_USER_OPT} \
--no-monitor restart $OPTIONS
+TimeoutSec=300
-- 
2.36.6

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

[ovs-dev] [PATCH 3/3] python: ovsdb-idl: Convert new_uuid insert() arg to UUID.

[Terry Wilson]

The argument to insert() should be a uuid.UUID object. If it isn't
then a Row is created with a string uuid attribute and that row is
added to table.rows with a string key instead of a UUID key.

Fixes: 55b9507e6824 ("ovsdb-idl: Add the support to specify the uuid for row 
insert.")
Signed-off-by: Terry Wilson 
---
 python/ovs/db/idl.py | 2 +-
 tests/test-ovsdb.py  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index a80da84e7..0e201366b 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -1854,7 +1854,7 @@ class Transaction(object):
 if row._data is None:
 op["op"] = "insert"
 if row._persist_uuid:
-op["uuid"] = row.uuid
+op["uuid"] = str(row.uuid)
 else:
 op["uuid-name"] = _uuid_name_from_uuid(row.uuid)
 
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 48f8ee2d7..6307aa2bd 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -434,7 +434,7 @@ def idl_set(idl, commands, step):
 sys.stderr.write('"set" command requires 2 argument\n')
 sys.exit(1)
 
-s = txn.insert(idl.tables["simple"], new_uuid=args[0],
+s = txn.insert(idl.tables["simple"], new_uuid=uuid.UUID(args[0]),
persist_uuid=True)
 s.i = int(args[1])
 elif name == "delete":
-- 
2.34.3

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

[ovs-dev] [PATCH 2/3] python: ovsdb-idl: Make IndexedRows mirror hmap.

[Terry Wilson]

The Python IDL code very closely mirrors the C IDL code, which uses
an hmap to store table rows. hmap code allows duplicate keys, while
IndexedRows, which is derived from DictBase does not.

The persistent UUID code can attempt to temporarily add a Row with
a duplicate UUID to table.rows, so IndexedRows is modified to
behave similarly to the C IDL's hmap implementation.

Fixes: 55b9507e6824 ("ovsdb-idl: Add the support to specify the uuid for row 
insert.")
Signed-off-by: Terry Wilson 
---
 python/ovs/db/custom_index.py | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/python/ovs/db/custom_index.py b/python/ovs/db/custom_index.py
index 587caf5e3..3fa03d3c9 100644
--- a/python/ovs/db/custom_index.py
+++ b/python/ovs/db/custom_index.py
@@ -90,14 +90,21 @@ class IndexedRows(DictBase, object):
 index = self.indexes[name] = MultiColumnIndex(name)
 return index
 
+def __getitem__(self, key):
+return self.data[key][-1]
+
 def __setitem__(self, key, item):
-self.data[key] = item
+try:
+self.data[key].append(item)
+except KeyError:
+self.data[key] = [item]
 for index in self.indexes.values():
 index.add(item)
 
 def __delitem__(self, key):
-val = self.data[key]
-del self.data[key]
+val = self.data[key].pop()
+if len(self.data[key]) == 0:
+del self.data[key]
 for index in self.indexes.values():
 index.remove(val)
 
-- 
2.34.3

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

[ovs-dev] [PATCH 1/3] ovsdb-idl: Add python keyword to persistent UUID test.

[Terry Wilson]

The Python persistent UUID tests should have the keyword "python"
added so that TESTSUITEFLAGS="-k python" will not miss testing
them.

Fixes: 55b9507e6824 ("ovsdb-idl: Add the support to specify the uuid for row 
insert.")
Signed-off-by: Terry Wilson 
---
 tests/ovsdb-idl.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index fb568dd82..c9e36d678 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2713,7 +2713,7 @@ m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT_C],
 
 m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT_PY],
   [AT_SETUP([$1 - Python3])
-   AT_KEYWORDS([idl persistent uuid insert])
+   AT_KEYWORDS([idl python persistent uuid insert])
OVSDB_START_IDLTEST([], ["$abs_srcdir/idltest.ovsschema"])
AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py  -t10 idl 
$srcdir/idltest.ovsschema unix:socket $2],
 [0], [stdout], [stderr])
-- 
2.34.3

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

Re: [ovs-dev] [PATCH] python: Remove hacking dependency and use recent flake8.

[Ilya Maximets]

On 4/10/24 18:24, Simon Horman wrote:
> On Wed, Apr 10, 2024 at 03:56:24PM +0200, Dumitru Ceara wrote:
>> The previously enabled 'hacking' checks were only applicable to Python 2
>> code.  OVS doesn't support Python 2 for a while now so it's fine to
>> remove the dependency on hacking.
>>
>> A similar change landed in OVN a while ago:
>> https://github.com/ovn-org/ovn/commit/271186fa7d76
>>
>> Signed-off-by: Dumitru Ceara 
> 
> Acked-by: Simon Horman 

Thanks, Dumitru and Simon!

Applied to all branches down to 2.17.

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

Re: [ovs-dev] [PATCH] vlog: Log stack trace on vlog_abort.

[Ilya Maximets]

On 4/10/24 18:24, Simon Horman wrote:
> On Wed, Apr 10, 2024 at 02:10:20PM +0200, Ilya Maximets wrote:
>> On 4/6/24 00:08, Ilya Maximets wrote:
>>> Currently, calls like ovs_assert() just print out a condition that
>>> caused assertion to fail.  But it may be not enough to understand what
>>> exactly has happened, especially if assertion failed in some generic
>>> function like dp_packet_resize() or similar.
>>>
>>> Print the stack trace along with the abort message to give more context
>>> for the later debugging.
>>>
>>> This should be especially useful in case the issue happens in an
>>> environment with core dumps disabled.
>>>
>>> Adding the log to vlog_abort() to cover a little more cases where
>>> VLOG_ABORT is called and not only assertion failures.
>>>
>>> It would be nice to also have stack traces in case of reaching the
>>> OVS_NOT_REACHED().  But this macro is used in some places as a last
>>> resort and should not actually do more than just stopping the process
>>> immediately.  And it also can be used in contexts without logging
>>> initialized.  Such a change will need to be done more carefully.
>>> Better solution might be to use VLOG_ABORT() where appropriate instead.
>>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>  lib/vlog.c   | 10 --
>>>  tests/library.at |  4 +++-
>>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> If this change is accepted, I'd also suggest backporting it to 3.3.
>> It is a debug-only change that touches only the code executed under
>> fatal failure conditions, so should be safe enough.  Backporting
>> will allow us easier debugging in to-be-LTS release.  And also OVN
>> 24.03 LTS can make use of it this way as well.
> 
> Acked-by: Simon Horman 
> 

Thanks, Simon and Kevin!

Applied to main and branch-3.3.

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

Re: [ovs-dev] [PATCH branch-2.17] xenserver: Fix tests with Python 3.12.

[Ilya Maximets]

On 4/10/24 18:17, Simon Horman wrote:
> On Tue, Apr 09, 2024 at 10:24:09PM +0200, Ilya Maximets wrote:
>> Without this change many unit tests are failing on systems with
>> Python 3.12:
>>
>>   +++ /tests/testsuite.dir/at-groups/2352/stdout
>>   @@ -1,3 +1,5 @@
>>   +/./interface-reconfigure:98: SyntaxWarning: invalid escape sequence '\s'
>>   +  p = re.compile(".*\s%(MAC)s\s.*" % pifrec, re.IGNORECASE)
>>
>> Converting to a raw string to avoid the issue.
>>
>> This only affects OVS 2.17, support for XenServer was removed
>> in OVS 3.0.
>>
>> Signed-off-by: Ilya Maximets 
> 
> Thanks Ilya,
> 
> I confirmed that on Fedora 41, which has Python 3.12.2,
> the following tests fail without this patch and succeed with it.
> Or more to the point, all tests pass with this patch applied.
> 
> 2353: VLAN, non-bond  ok
> 2354: Bond, non-VLAN  ok
> 2355: VLAN on bondok
> 2352: non-VLAN, non-bond  ok
> 
> Acked-by: Simon Horman 
> Tested-by: Simon Horman 

Thanks, Simon!  Applied to branch-2.17.

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Ilya Maximets]

On 4/10/24 12:04, Kevin Traynor wrote:
> On 10/04/2024 10:51, Eelco Chaudron wrote:
>>
>>
>> On 10 Apr 2024, at 11:41, Ilya Maximets wrote:
>>
>>> 13.3 was released on March 5 and 13.2 will reach EoL in June.
>>> Update now.
>>
>> Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough GitHub 
>> actions;
>>
> 
> I ran it here:
> https://cirrus-ci.com/build/5951252381564928
> 
> Acked-by: Kevin Traynor 
> 
>> Acked-by: Eelco Chaudron 

Thanks, Eelco and Kevin!

Applied to all branches down to 2.17.

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

Re: [ovs-dev] [PATCH] checkpatch: Don't spellcheck names in tags.

[Ilya Maximets]

On 4/10/24 11:37, Eelco Chaudron wrote:
> 
> 
> On 9 Apr 2024, at 21:55, Ilya Maximets wrote:
> 
>> Current code checks spelling of names in commit message tags and that
>> makes no sense.
>>
>> Most of the tags are explicitly handled, but tags like 'Tested-by' or
>> other lesser used ones are falling through to the spellchecker and
>> need to be excluded.
>>
>> Signed-off-by: Ilya Maximets 
> 
> Thanks for the enhancement.
> 
> Acked-by: Eelco Chaudron 
> 

Applied.  Thanks!

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

Re: [ovs-dev] [ANN] Primary OVS branch renamed as main development branch as main.

[Han Zhou]

On Wed, Apr 10, 2024 at 6:52 AM Simon Horman  wrote:
>
> Hi,
>
> I would like to announce that the primary development branch for OvS
> has been renamed main.
>
> The rename occurred a little earlier today.
>
> OVS is currently hosted on GitHub. We can expect the following behaviour
> after the rename:
>
> * GitHub pull requests against master should have been automatically
>   re-homed on main.
> * GitHub Issues should not to be affected - the test issue I
>   created had no association with a branch
> * URLs accessed via the GitHub web UI are automatically renamed
> * Clones may also rename their primary branch - you may
>   get a notification about this in the Web UI
>
> As a result of this change it may be necessary to update your local git
> configuration for checked out branches.
>
> For example:
> # Fetch origin: new remote main branch; remote master branch is deleted
> git fetch -tp origin
> # Rename local branch
> git branch -m master main
> # Update local main branch to use remote main branch as it's upstream
> git branch --set-upstream-to=origin/main main
>
> If you have an automation that fetches the master branch then please
> update the automation to fetch main. If your automation is fetching
> main and falling back to master, then it should now be safe to
> remove the fallback.
>
> This change is in keeping with OVS's recently OVS adopted a policy of
using
> the inclusive naming word list v1 [1, 2].
>
> [1] df5e5cf4318a ("Documentation: Add section on inclusive language.")
> [2] https://inclusivenaming.org/word-lists/
>
> Kind regards,
> Simon
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Simon. Shall this be announced to ovs-announce as well?

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

Re: [ovs-dev] [PATCH 2/2] github: Remove reference to master branch.

[Ilya Maximets]

On 4/10/24 16:09, Simon Horman wrote:
> The OvS primary development branch has been renamed main
> so there is no longer any need for this CI configuration
> to refer to master.
> 
> Signed-off-by: Simon Horman 
> ---
>  .github/workflows/build-and-test.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.github/workflows/build-and-test.yml 
> b/.github/workflows/build-and-test.yml
> index 6f5139304ae2..2d64937e41b2 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -307,7 +307,7 @@ jobs:
>  MIN_DISTANCE=1000
>  git remote add upstream https://github.com/openvswitch/ovs.git
>  git fetch upstream
> -for upstream_head in $(git ls-remote --heads upstream main 
> master dpdk-latest branch-2.17 branch-[3456789]* | cut -f 1); do
> +for upstream_head in $(git ls-remote --heads upstream main 
> dpdk-latest branch-2.17 branch-[3456789]* | cut -f 1); do
>CURR_BASE=$(git merge-base ${upstream_head} HEAD 2>/dev/null)
>if [ ${CURR_BASE} ]; then
>  DISTANCE=$(git log --oneline ${CURR_BASE}..HEAD | wc -l);
> 

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

Re: [ovs-dev] [PATCH 1/2] appveyor: Remove reference to master branch.

[Ilya Maximets]

On 4/10/24 16:09, Simon Horman wrote:
> The OvS primary development branch has been renamed main
> so there is no longer any need for this CI configuration
> to refer to master.
> 
> Signed-off-by: Simon Horman 
> ---
>  appveyor.yml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/appveyor.yml b/appveyor.yml
> index 050c7dead786..baa844753962 100644
> --- a/appveyor.yml
> +++ b/appveyor.yml
> @@ -3,7 +3,6 @@ image: Visual Studio 2019
>  branches:
>only:
>- main
> -  - master
>  configuration:
>- Debug
>- Release
> 

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

Re: [ovs-dev] [PATCH] python: Remove hacking dependency and use recent flake8.

[Simon Horman]

On Wed, Apr 10, 2024 at 03:56:24PM +0200, Dumitru Ceara wrote:
> The previously enabled 'hacking' checks were only applicable to Python 2
> code.  OVS doesn't support Python 2 for a while now so it's fine to
> remove the dependency on hacking.
> 
> A similar change landed in OVN a while ago:
> https://github.com/ovn-org/ovn/commit/271186fa7d76
> 
> Signed-off-by: Dumitru Ceara 

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH] vlog: Log stack trace on vlog_abort.

[Simon Horman]

On Wed, Apr 10, 2024 at 02:10:20PM +0200, Ilya Maximets wrote:
> On 4/6/24 00:08, Ilya Maximets wrote:
> > Currently, calls like ovs_assert() just print out a condition that
> > caused assertion to fail.  But it may be not enough to understand what
> > exactly has happened, especially if assertion failed in some generic
> > function like dp_packet_resize() or similar.
> > 
> > Print the stack trace along with the abort message to give more context
> > for the later debugging.
> > 
> > This should be especially useful in case the issue happens in an
> > environment with core dumps disabled.
> > 
> > Adding the log to vlog_abort() to cover a little more cases where
> > VLOG_ABORT is called and not only assertion failures.
> > 
> > It would be nice to also have stack traces in case of reaching the
> > OVS_NOT_REACHED().  But this macro is used in some places as a last
> > resort and should not actually do more than just stopping the process
> > immediately.  And it also can be used in contexts without logging
> > initialized.  Such a change will need to be done more carefully.
> > Better solution might be to use VLOG_ABORT() where appropriate instead.
> > 
> > Signed-off-by: Ilya Maximets 
> > ---
> >  lib/vlog.c   | 10 --
> >  tests/library.at |  4 +++-
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> If this change is accepted, I'd also suggest backporting it to 3.3.
> It is a debug-only change that touches only the code executed under
> fatal failure conditions, so should be safe enough.  Backporting
> will allow us easier debugging in to-be-LTS release.  And also OVN
> 24.03 LTS can make use of it this way as well.

Acked-by: Simon Horman 

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

Re: [ovs-dev] [PATCH branch-2.17] xenserver: Fix tests with Python 3.12.

[Simon Horman]

On Tue, Apr 09, 2024 at 10:24:09PM +0200, Ilya Maximets wrote:
> Without this change many unit tests are failing on systems with
> Python 3.12:
> 
>   +++ /tests/testsuite.dir/at-groups/2352/stdout
>   @@ -1,3 +1,5 @@
>   +/./interface-reconfigure:98: SyntaxWarning: invalid escape sequence '\s'
>   +  p = re.compile(".*\s%(MAC)s\s.*" % pifrec, re.IGNORECASE)
> 
> Converting to a raw string to avoid the issue.
> 
> This only affects OVS 2.17, support for XenServer was removed
> in OVS 3.0.
> 
> Signed-off-by: Ilya Maximets 

Thanks Ilya,

I confirmed that on Fedora 41, which has Python 3.12.2,
the following tests fail without this patch and succeed with it.
Or more to the point, all tests pass with this patch applied.

2353: VLAN, non-bond  ok
2354: Bond, non-VLAN  ok
2355: VLAN on bondok
2352: non-VLAN, non-bond  ok

Acked-by: Simon Horman 
Tested-by: Simon Horman 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] treewide: Rename references from OvS master to main.

[Dumitru Ceara]

On 4/10/24 17:24, Simon Horman wrote:
> On Wed, Apr 10, 2024 at 03:45:53PM +0200, Ales Musil wrote:
>> There was recent switch in OvS from master to main branch.
>> Use main in all the references across OVN code base.
>>
>> Signed-off-by: Ales Musil 
>> ---
>> v2: Add the missing two references.
> 
> Acked-by: Simon Horman 
> 

Thanks, Ales and Simon!  Applied to main.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn v2] treewide: Rename references from OvS master to main.

[Simon Horman]

On Wed, Apr 10, 2024 at 03:45:53PM +0200, Ales Musil wrote:
> There was recent switch in OvS from master to main branch.
> Use main in all the references across OVN code base.
> 
> Signed-off-by: Ales Musil 
> ---
> v2: Add the missing two references.

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

Re: [ovs-dev] [PATCH] vlog: Log stack trace on vlog_abort.

[Kevin Traynor]

On 10/04/2024 16:16, Ilya Maximets wrote:
> On 4/10/24 17:01, Kevin Traynor wrote:
>> On 05/04/2024 23:08, Ilya Maximets wrote:
>>> Currently, calls like ovs_assert() just print out a condition that
>>> caused assertion to fail.  But it may be not enough to understand what
>>> exactly has happened, especially if assertion failed in some generic
>>> function like dp_packet_resize() or similar.
>>>
>>> Print the stack trace along with the abort message to give more context
>>> for the later debugging.
>>>
>>> This should be especially useful in case the issue happens in an
>>> environment with core dumps disabled.
>>>
>>> Adding the log to vlog_abort() to cover a little more cases where
>>> VLOG_ABORT is called and not only assertion failures.
>>>
>>> It would be nice to also have stack traces in case of reaching the
>>> OVS_NOT_REACHED().  But this macro is used in some places as a last
>>> resort and should not actually do more than just stopping the process
>>> immediately.  And it also can be used in contexts without logging
>>> initialized.  Such a change will need to be done more carefully.
>>> Better solution might be to use VLOG_ABORT() where appropriate instead.
>>>
>> Thanks Ilya. Tried it and it's working. One comment below.
>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>  lib/vlog.c   | 10 --
>>>  tests/library.at |  4 +++-
>>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/vlog.c b/lib/vlog.c
>>> index b2653142f..e78c785f7 100644
>>> --- a/lib/vlog.c
>>> +++ b/lib/vlog.c
>>> @@ -29,6 +29,7 @@
>>>  #include 
>>>  #include 
>>>  #include "async-append.h"
>>> +#include "backtrace.h"
>>>  #include "coverage.h"
>>>  #include "dirs.h"
>>>  #include "openvswitch/dynamic-string.h"
>>> @@ -1274,8 +1275,9 @@ vlog_fatal(const struct vlog_module *module, const 
>>> char *message, ...)
>>>  va_end(args);
>>>  }
>>>  
>>> -/* Logs 'message' to 'module' at maximum verbosity, then calls abort().  
>>> Always
>>> - * writes the message to stderr, even if the console destination is 
>>> disabled.
>>> +/* Attempts to log a stack trace, logs 'message' to 'module' at maximum
>>> + * verbosity, then calls abort().  Always writes the message to stderr, 
>>> even
>>> + * if the console destination is disabled.
>>>   *
>>>   * Choose this function instead of vlog_fatal_valist() if the daemon 
>>> monitoring
>>>   * facility should automatically restart the current daemon.  */
>>> @@ -1289,6 +1291,10 @@ vlog_abort_valist(const struct vlog_module *module_,
>>>   * message written by the later ovs_abort_valist(). */
>>>  module->levels[VLF_CONSOLE] = VLL_OFF;
>>>  
>>> +/* Printing the stack trace before the 'message', because the 'message'
>>> + * will flush the async log queue (VLL_EMER).  With a different order 
>>> we
>>> + * would need to flush the queue manually again. */
>>> +log_backtrace();
>>>  vlog_valist(module, VLL_EMER, message, args);
>>>  ovs_abort_valist(0, message, args);
>>>  }
>>
>> Is it worth adding to vlog_fatal_valist() as well for VLOG_FATAL()?
>>
>> If there's some reason not to, then LGTM as is.
> 
> VLOG_FATAL is used more widely for user-errors or environment issues.
> It's not appropriate to print stack traces in such scenarios.
> For example, I don't think we want to dump the trace when users provide
> incorrect command line arguments in tools (VLOG_FATAL is used for that).
> 
> Abort, OTOH, usually signifies a programming error and should not actually
> be invoked under normal circumstances, so it should be fine to dump the
> stack there.
> 

ok, that sounds reasonable. Thanks.

>>
>> Acked-by: Kevin Traynor 
>>
>>> diff --git a/tests/library.at b/tests/library.at
>>> index 7b4acebb8..d962e1b3f 100644
>>> --- a/tests/library.at
>>> +++ b/tests/library.at
>>> @@ -230,7 +230,9 @@ AT_CHECK([ovstest test-util -voff -vfile:info 
>>> '-vPATTERN:file:%c|%p|%m' --log-fi
>>>[$exit_status], [], [stderr])
>>>  
>>>  AT_CHECK([sed 's/\(opened log file\) .*/\1/
>>> -s/|[[^|]]*: /|/' test-util.log], [0], [dnl
>>> +s/|[[^|]]*: /|/
>>> +/backtrace/d
>>> +/|.*|/!d' test-util.log], [0], [dnl
>>>  vlog|INFO|opened log file
>>>  util|EMER|assertion false failed in test_assert()
>>>  ])
>>
> 

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

Re: [ovs-dev] [PATCH] vlog: Log stack trace on vlog_abort.

[Ilya Maximets]

On 4/10/24 17:01, Kevin Traynor wrote:
> On 05/04/2024 23:08, Ilya Maximets wrote:
>> Currently, calls like ovs_assert() just print out a condition that
>> caused assertion to fail.  But it may be not enough to understand what
>> exactly has happened, especially if assertion failed in some generic
>> function like dp_packet_resize() or similar.
>>
>> Print the stack trace along with the abort message to give more context
>> for the later debugging.
>>
>> This should be especially useful in case the issue happens in an
>> environment with core dumps disabled.
>>
>> Adding the log to vlog_abort() to cover a little more cases where
>> VLOG_ABORT is called and not only assertion failures.
>>
>> It would be nice to also have stack traces in case of reaching the
>> OVS_NOT_REACHED().  But this macro is used in some places as a last
>> resort and should not actually do more than just stopping the process
>> immediately.  And it also can be used in contexts without logging
>> initialized.  Such a change will need to be done more carefully.
>> Better solution might be to use VLOG_ABORT() where appropriate instead.
>>
> Thanks Ilya. Tried it and it's working. One comment below.
> 
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/vlog.c   | 10 --
>>  tests/library.at |  4 +++-
>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/vlog.c b/lib/vlog.c
>> index b2653142f..e78c785f7 100644
>> --- a/lib/vlog.c
>> +++ b/lib/vlog.c
>> @@ -29,6 +29,7 @@
>>  #include 
>>  #include 
>>  #include "async-append.h"
>> +#include "backtrace.h"
>>  #include "coverage.h"
>>  #include "dirs.h"
>>  #include "openvswitch/dynamic-string.h"
>> @@ -1274,8 +1275,9 @@ vlog_fatal(const struct vlog_module *module, const 
>> char *message, ...)
>>  va_end(args);
>>  }
>>  
>> -/* Logs 'message' to 'module' at maximum verbosity, then calls abort().  
>> Always
>> - * writes the message to stderr, even if the console destination is 
>> disabled.
>> +/* Attempts to log a stack trace, logs 'message' to 'module' at maximum
>> + * verbosity, then calls abort().  Always writes the message to stderr, even
>> + * if the console destination is disabled.
>>   *
>>   * Choose this function instead of vlog_fatal_valist() if the daemon 
>> monitoring
>>   * facility should automatically restart the current daemon.  */
>> @@ -1289,6 +1291,10 @@ vlog_abort_valist(const struct vlog_module *module_,
>>   * message written by the later ovs_abort_valist(). */
>>  module->levels[VLF_CONSOLE] = VLL_OFF;
>>  
>> +/* Printing the stack trace before the 'message', because the 'message'
>> + * will flush the async log queue (VLL_EMER).  With a different order we
>> + * would need to flush the queue manually again. */
>> +log_backtrace();
>>  vlog_valist(module, VLL_EMER, message, args);
>>  ovs_abort_valist(0, message, args);
>>  }
> 
> Is it worth adding to vlog_fatal_valist() as well for VLOG_FATAL()?
> 
> If there's some reason not to, then LGTM as is.

VLOG_FATAL is used more widely for user-errors or environment issues.
It's not appropriate to print stack traces in such scenarios.
For example, I don't think we want to dump the trace when users provide
incorrect command line arguments in tools (VLOG_FATAL is used for that).

Abort, OTOH, usually signifies a programming error and should not actually
be invoked under normal circumstances, so it should be fine to dump the
stack there.

> 
> Acked-by: Kevin Traynor 
> 
>> diff --git a/tests/library.at b/tests/library.at
>> index 7b4acebb8..d962e1b3f 100644
>> --- a/tests/library.at
>> +++ b/tests/library.at
>> @@ -230,7 +230,9 @@ AT_CHECK([ovstest test-util -voff -vfile:info 
>> '-vPATTERN:file:%c|%p|%m' --log-fi
>>[$exit_status], [], [stderr])
>>  
>>  AT_CHECK([sed 's/\(opened log file\) .*/\1/
>> -s/|[[^|]]*: /|/' test-util.log], [0], [dnl
>> +s/|[[^|]]*: /|/
>> +/backtrace/d
>> +/|.*|/!d' test-util.log], [0], [dnl
>>  vlog|INFO|opened log file
>>  util|EMER|assertion false failed in test_assert()
>>  ])
> 

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

Re: [ovs-dev] [PATCH] vlog: Log stack trace on vlog_abort.

[Kevin Traynor]

On 05/04/2024 23:08, Ilya Maximets wrote:
> Currently, calls like ovs_assert() just print out a condition that
> caused assertion to fail.  But it may be not enough to understand what
> exactly has happened, especially if assertion failed in some generic
> function like dp_packet_resize() or similar.
> 
> Print the stack trace along with the abort message to give more context
> for the later debugging.
> 
> This should be especially useful in case the issue happens in an
> environment with core dumps disabled.
> 
> Adding the log to vlog_abort() to cover a little more cases where
> VLOG_ABORT is called and not only assertion failures.
> 
> It would be nice to also have stack traces in case of reaching the
> OVS_NOT_REACHED().  But this macro is used in some places as a last
> resort and should not actually do more than just stopping the process
> immediately.  And it also can be used in contexts without logging
> initialized.  Such a change will need to be done more carefully.
> Better solution might be to use VLOG_ABORT() where appropriate instead.
> 
Thanks Ilya. Tried it and it's working. One comment below.

> Signed-off-by: Ilya Maximets 
> ---
>  lib/vlog.c   | 10 --
>  tests/library.at |  4 +++-
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/vlog.c b/lib/vlog.c
> index b2653142f..e78c785f7 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include "async-append.h"
> +#include "backtrace.h"
>  #include "coverage.h"
>  #include "dirs.h"
>  #include "openvswitch/dynamic-string.h"
> @@ -1274,8 +1275,9 @@ vlog_fatal(const struct vlog_module *module, const char 
> *message, ...)
>  va_end(args);
>  }
>  
> -/* Logs 'message' to 'module' at maximum verbosity, then calls abort().  
> Always
> - * writes the message to stderr, even if the console destination is disabled.
> +/* Attempts to log a stack trace, logs 'message' to 'module' at maximum
> + * verbosity, then calls abort().  Always writes the message to stderr, even
> + * if the console destination is disabled.
>   *
>   * Choose this function instead of vlog_fatal_valist() if the daemon 
> monitoring
>   * facility should automatically restart the current daemon.  */
> @@ -1289,6 +1291,10 @@ vlog_abort_valist(const struct vlog_module *module_,
>   * message written by the later ovs_abort_valist(). */
>  module->levels[VLF_CONSOLE] = VLL_OFF;
>  
> +/* Printing the stack trace before the 'message', because the 'message'
> + * will flush the async log queue (VLL_EMER).  With a different order we
> + * would need to flush the queue manually again. */
> +log_backtrace();
>  vlog_valist(module, VLL_EMER, message, args);
>  ovs_abort_valist(0, message, args);
>  }

Is it worth adding to vlog_fatal_valist() as well for VLOG_FATAL()?

If there's some reason not to, then LGTM as is.

Acked-by: Kevin Traynor 

> diff --git a/tests/library.at b/tests/library.at
> index 7b4acebb8..d962e1b3f 100644
> --- a/tests/library.at
> +++ b/tests/library.at
> @@ -230,7 +230,9 @@ AT_CHECK([ovstest test-util -voff -vfile:info 
> '-vPATTERN:file:%c|%p|%m' --log-fi
>[$exit_status], [], [stderr])
>  
>  AT_CHECK([sed 's/\(opened log file\) .*/\1/
> -s/|[[^|]]*: /|/' test-util.log], [0], [dnl
> +s/|[[^|]]*: /|/
> +/backtrace/d
> +/|.*|/!d' test-util.log], [0], [dnl
>  vlog|INFO|opened log file
>  util|EMER|assertion false failed in test_assert()
>  ])

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

Re: [ovs-dev] [PATCH 2/2] github: Remove reference to master branch.

[Eelco Chaudron]

On 10 Apr 2024, at 16:09, Simon Horman wrote:

> The OvS primary development branch has been renamed main
> so there is no longer any need for this CI configuration
> to refer to master.
>
> Signed-off-by: Simon Horman 

Thanks for following through with the main branch change.

Acked-by: Eelco Chaudron 

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

Re: [ovs-dev] [PATCH 1/2] appveyor: Remove reference to master branch.

[Eelco Chaudron]

On 10 Apr 2024, at 16:09, Simon Horman wrote:

> The OvS primary development branch has been renamed main
> so there is no longer any need for this CI configuration
> to refer to master.
>
> Signed-off-by: Simon Horman 

Thanks for following through with the main branch change.

Acked-by: Eelco Chaudron 

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

Re: [ovs-dev] [PATCH 1/2] appveyor: Remove reference to master branch.

[Alin Serdean]

Acked-by: Alin Gabriel Serdean 


> 
> On 10 Apr 2024, at 16:09, Simon Horman  wrote:
> 
> The OvS primary development branch has been renamed main
> so there is no longer any need for this CI configuration
> to refer to master.
> 
> Signed-off-by: Simon Horman 
> ---
> appveyor.yml | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/appveyor.yml b/appveyor.yml
> index 050c7dead786..baa844753962 100644
> --- a/appveyor.yml
> +++ b/appveyor.yml
> @@ -3,7 +3,6 @@ image: Visual Studio 2019
> branches:
>   only:
>   - main
> -  - master
> configuration:
>   - Debug
>   - Release
> 
> --
> 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 1/2] appveyor: Remove reference to master branch.

[Simon Horman]

The OvS primary development branch has been renamed main
so there is no longer any need for this CI configuration
to refer to master.

Signed-off-by: Simon Horman 
---
 appveyor.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/appveyor.yml b/appveyor.yml
index 050c7dead786..baa844753962 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -3,7 +3,6 @@ image: Visual Studio 2019
 branches:
   only:
   - main
-  - master
 configuration:
   - Debug
   - Release

-- 
2.43.0

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

[ovs-dev] [PATCH 2/2] github: Remove reference to master branch.

[Simon Horman]

The OvS primary development branch has been renamed main
so there is no longer any need for this CI configuration
to refer to master.

Signed-off-by: Simon Horman 
---
 .github/workflows/build-and-test.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 6f5139304ae2..2d64937e41b2 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -307,7 +307,7 @@ jobs:
 MIN_DISTANCE=1000
 git remote add upstream https://github.com/openvswitch/ovs.git
 git fetch upstream
-for upstream_head in $(git ls-remote --heads upstream main master 
dpdk-latest branch-2.17 branch-[3456789]* | cut -f 1); do
+for upstream_head in $(git ls-remote --heads upstream main 
dpdk-latest branch-2.17 branch-[3456789]* | cut -f 1); do
   CURR_BASE=$(git merge-base ${upstream_head} HEAD 2>/dev/null)
   if [ ${CURR_BASE} ]; then
 DISTANCE=$(git log --oneline ${CURR_BASE}..HEAD | wc -l);

-- 
2.43.0

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

[ovs-dev] [PATCH 0/2] ci: Remove reference to master branch.

[Simon Horman]

The OvS primary development branch has been renamed main
so there is no longer any need for CI configuration to
refer to master.

Signed-off-by: Simon Horman 

---
Simon Horman (2):
  appveyor: Remove reference to master branch.
  github: Remove reference to master branch.

 .github/workflows/build-and-test.yml | 2 +-
 appveyor.yml | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

base-commit: 74cf01436fd2d05eea755b0a43b5f306aa06bc1f

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

[ovs-dev] [PATCH] python: Remove hacking dependency and use recent flake8.

[Dumitru Ceara]

The previously enabled 'hacking' checks were only applicable to Python 2
code.  OVS doesn't support Python 2 for a while now so it's fine to
remove the dependency on hacking.

A similar change landed in OVN a while ago:
https://github.com/ovn-org/ovn/commit/271186fa7d76

Signed-off-by: Dumitru Ceara 
---
 .ci/linux-prepare.sh| 2 +-
 Documentation/intro/install/general.rst | 5 +
 Makefile.am | 6 --
 3 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 5028bdc442dc..2a191b57fb8f 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -23,7 +23,7 @@ cd ..
 # https://github.com/pypa/pip/issues/10655
 pip3 install --disable-pip-version-check --user wheel
 pip3 install --disable-pip-version-check --user \
-flake8 'hacking>=3.0' netaddr pyparsing sarif-tools sphinx setuptools
+flake8 netaddr pyparsing sarif-tools sphinx setuptools
 
 # Install python test dependencies
 pip3 install -r python/test_requirements.txt
diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index 17c154268054..23585117647d 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -176,10 +176,7 @@ following to obtain better warnings:
 
 - clang, version 3.4 or later
 
-- flake8 along with the hacking flake8 plugin (for Python code). The automatic
-  flake8 check that runs against Python code has some warnings enabled that
-  come from the "hacking" flake8 plugin. If it's not installed, the warnings
-  just won't occur until it's run on a system with "hacking" installed.
+- flake8 (for Python code)
 
 - the python packages listed in "python/test_requirements.txt" (compatible
   with pip). If they are installed, the pytest-based Python unit tests will
diff --git a/Makefile.am b/Makefile.am
index 45fce1243a72..e6c90a911aa8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -402,16 +402,10 @@ ALL_LOCAL += flake8-check
 #   F811 redefinition of unused  from line  (only from flake8 v2.0)
 # D*** -- warnings from flake8-docstrings plugin
 # H*** -- warnings from flake8 hacking plugin (custom style checks beyond PEP8)
-#   H231 Python 3.x incompatible 'except x,y:' construct
-#   H232 Python 3.x incompatible octal 077 should be written as 0o77
-#   H233 Python 3.x incompatible use of print operator
-#   H238 old style class declaration, use new style (inherit from `object`)
-FLAKE8_SELECT = H231,H232,H233,H238
 FLAKE8_IGNORE = 
E121,E123,E125,E126,E127,E128,E129,E131,E203,E722,W503,W504,F811,D,H,I
 flake8-check: $(FLAKE8_PYFILES)
$(FLAKE8_WERROR)$(AM_V_GEN) \
  src='$^' && \
- flake8 $$src --select=$(FLAKE8_SELECT) $(FLAKE8_FLAGS) && \
  flake8 $$src --ignore=$(FLAKE8_IGNORE) $(FLAKE8_FLAGS) && \
  touch $@
 endif
-- 
2.44.0

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

[ovs-dev] [ANN] Primary OVS branch renamed as main development branch as main.

[Simon Horman]

Hi,

I would like to announce that the primary development branch for OvS
has been renamed main.

The rename occurred a little earlier today.

OVS is currently hosted on GitHub. We can expect the following behaviour
after the rename:

* GitHub pull requests against master should have been automatically
  re-homed on main.
* GitHub Issues should not to be affected - the test issue I
  created had no association with a branch
* URLs accessed via the GitHub web UI are automatically renamed
* Clones may also rename their primary branch - you may
  get a notification about this in the Web UI

As a result of this change it may be necessary to update your local git
configuration for checked out branches.

For example:
# Fetch origin: new remote main branch; remote master branch is deleted
git fetch -tp origin
# Rename local branch
git branch -m master main
# Update local main branch to use remote main branch as it's upstream
git branch --set-upstream-to=origin/main main

If you have an automation that fetches the master branch then please
update the automation to fetch main. If your automation is fetching
main and falling back to master, then it should now be safe to
remove the fallback.

This change is in keeping with OVS's recently OVS adopted a policy of using
the inclusive naming word list v1 [1, 2].

[1] df5e5cf4318a ("Documentation: Add section on inclusive language.")
[2] https://inclusivenaming.org/word-lists/

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

Re: [ovs-dev] [PATCH ovn] ci: Rename OvS master branch to main.

[Ales Musil]

On Wed, Apr 10, 2024 at 3:42 PM Dumitru Ceara  wrote:

> Hi Ales, Simon,
>
> On 4/10/24 15:39, Simon Horman wrote:
> > On Wed, Apr 10, 2024 at 03:26:25PM +0200, Ales Musil wrote:
> >> There was recent switch in OvS from master to main branch.
> >> Use main instead in ovn-fake-mutlinode tests.
>
> Nit: typo :)
>
> >>
> >> Signed-off-by: Ales Musil 
> >
> > Acked-by: Simon Horman 
> >
>
> There are two more references to OVS "master" branch, in ovn-sandbox.rst
> and in README.rst.
>
> Regards,
> Dumitru
>
>
You are right, posted v2.

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

[ovs-dev] [PATCH ovn v2] treewide: Rename references from OvS master to main.

[Ales Musil]

There was recent switch in OvS from master to main branch.
Use main in all the references across OVN code base.

Signed-off-by: Ales Musil 
---
v2: Add the missing two references.
---
 .github/workflows/ovn-fake-multinode-tests.yml | 4 ++--
 Documentation/tutorials/ovn-sandbox.rst| 2 +-
 README.rst | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ovn-fake-multinode-tests.yml 
b/.github/workflows/ovn-fake-multinode-tests.yml
index 79b6c4253..ed4d6c4ef 100644
--- a/.github/workflows/ovn-fake-multinode-tests.yml
+++ b/.github/workflows/ovn-fake-multinode-tests.yml
@@ -35,12 +35,12 @@ jobs:
 # Check out ovn and ovs separately inside ovn-fake-multinode/ovn and 
ovn-fake-multinode/ovs
 # 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
+- name: Check out ovs main
   uses: actions/checkout@v4
   with:
 path: 'ovn-fake-multinode/ovs'
 repository: 'openvswitch/ovs'
-ref: 'master'
+ref: 'main'
 
 - name: Check out ovn ${{ matrix.cfg.branch }}
   uses: actions/checkout@v4
diff --git a/Documentation/tutorials/ovn-sandbox.rst 
b/Documentation/tutorials/ovn-sandbox.rst
index 4e40a96eb..4acc22cdd 100644
--- a/Documentation/tutorials/ovn-sandbox.rst
+++ b/Documentation/tutorials/ovn-sandbox.rst
@@ -174,5 +174,5 @@ man page for more detail.
 .. _ovn-nbctl(8): http://www.ovn.org/support/dist-docs/ovn-nbctl.8.html
 .. _ovn-sbctl(8): http://www.ovn.org/support/dist-docs/ovn-sbctl.8.html
 .. _ovn-trace(8): http://www.ovn.org/support/dist-docs/ovn-trace.8.html
-.. _ovs-advanced: 
https://github.com/openvswitch/ovs/blob/master/Documentation/tutorials/ovs-advanced.rst
+.. _ovs-advanced: 
https://github.com/openvswitch/ovs/blob/main/Documentation/tutorials/ovs-advanced.rst
 
diff --git a/README.rst b/README.rst
index 428cd8ee8..97e443b1f 100644
--- a/README.rst
+++ b/README.rst
@@ -75,7 +75,7 @@ For answers to common questions, refer to the `FAQ 
`__.
 To learn about some advanced features of the Open vSwitch software switch, read
 the tutorial_.
 
-.. _tutorial: 
https://github.com/openvswitch/ovs/blob/master/Documentation/tutorials/ovs-advanced.rst
+.. _tutorial: 
https://github.com/openvswitch/ovs/blob/main/Documentation/tutorials/ovs-advanced.rst
 
 Each OVN program is accompanied by a manpage.  Many of the manpages are 
customized
 to your configuration as part of the build process, so we recommend building 
OVN
-- 
2.44.0

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

Re: [ovs-dev] [PATCH] checkpatch: Allow rST manpages to be added.

[Eelco Chaudron]

On 9 Apr 2024, at 9:19, Adrian Moreno wrote:

> The current __check_doc_is_listed() verifies that the new .rst file is
> listed in Documentation/automake.mk with the full path (i.e:
> "{directory}/{filename}").
>
> While this holds true for generic documentation files, which are added
> to DOC_SOURCE with the full path, it's not true for rST manpages which
> are added only by filename to RST_MANPAGES target (see
> Documentation/automake.mk).
>
> This makes the current implementation of the check to incorrectly raise
> a warning as follows even though the patch does add the file to
> RST_MANPAGES:
>
> """
> WARNING: New doc ovs-flowviz.8.rst not listed in
> Documentation/automake.mk
> """
>
> Fix it by making the {dir}/ part of the docre regexp optional.
>
> Signed-off-by: Adrian Moreno 

Thanks for fixing this! The change looks good to me.

Acked-by: Eelco Chaudron 

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

Re: [ovs-dev] [PATCH ovn] ci: Rename OvS master branch to main.

[Dumitru Ceara]

Hi Ales, Simon,

On 4/10/24 15:39, Simon Horman wrote:
> On Wed, Apr 10, 2024 at 03:26:25PM +0200, Ales Musil wrote:
>> There was recent switch in OvS from master to main branch.
>> Use main instead in ovn-fake-mutlinode tests.

Nit: typo :)

>>
>> Signed-off-by: Ales Musil 
> 
> Acked-by: Simon Horman 
> 

There are two more references to OVS "master" branch, in ovn-sandbox.rst
and in README.rst.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn] ci: Rename OvS master branch to main.

[Simon Horman]

On Wed, Apr 10, 2024 at 03:26:25PM +0200, Ales Musil wrote:
> There was recent switch in OvS from master to main branch.
> Use main instead in ovn-fake-mutlinode tests.
> 
> Signed-off-by: Ales Musil 

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

Re: [ovs-dev] [PATCH v3] Documentation: Updates for rename of primary development branch as main.

[Simon Horman]

On Wed, Apr 10, 2024 at 12:07:12PM +0100, Simon Horman wrote:
> On Tue, Apr 09, 2024 at 07:28:08PM +0200, Ilya Maximets wrote:
> > On 4/5/24 15:57, Simon Horman wrote:
> > > Recently OVS adopted a policy of using the inclusive naming word list v1
> > > [1, 2].
> > > 
> > > In keeping with this policy rename the primary development branch from
> > > 'master' to 'main'. This patch does not actually make that change, but
> > > rather updates references to the branch in documentation in the source
> > > tree.  It is intended to be applied at (approximately) the same time
> > > that the change is made.
> > > 
> > > OVS is currently hosted on GitHub. We can expect the following behaviour
> > > after the rename:
> > > 
> > > 1. GitHub pull requests against are renamed branch are automatically
> > >re-homed on new branch
> > > 2. GitHub Issues do not seem to be affected - at least the test issue I
> > >created had no association with a branch
> > > 3. URLs accessed via the GitHub web UI are automatically renamed
> > >(so long as a new branch called master is not created).
> > > 4. Using the git cli command, fetch will fetch the new branch (main),
> > >and fetch -p will remove (prune) the old branch (master)
> > > 
> > > [1] df5e5cf4318a ("Documentation: Add section on inclusive language.")
> > > [2] https://inclusivenaming.org/word-lists/
> > > 
> > > Signed-off-by: Simon Horman 
> > > ---
> > > Changes in v3:
> > > - This patch only updates documentation. Update the patch prefix and
> > >   description accordingly.
> > > - Drop documentation of 'tested on "master" branch' rather than updating
> > >   it, as the updated text seems somewhat nonsensical.
> > > - Correct indentation of NEWS entry.
> > 
> > Thanks, Simon!
> > 
> > Acked-by: Ilya Maximets 
> > 
> > > 
> > > Changes in v2:
> > > - Keep two blank lines between versions.
> > > - Drop bogus update to OpenSSL hashes URL in appveyor.yml.
> > > - Drop other appveyor.yml changes, they are now present upstream.
> > >   + appveyor: Prepare for rename of primary development branch.
> > > https://github.com/openvswitch/ovs/commit/95ff912edef8
> > > - Add note about updates to git configuration.
> > > ---
> > > Notes:
> > > 
> > > * Now is the time to raise any concerns regarding this patch.
> > >   It is planned to implement this change next week.
> > 
> > Do you have particular date in mind?
> 
> Hi Ilya,
> 
> I think that today or tomorrow would be good, to give a bit
> of space before the weekend. But whenever suits you: I believe
> it is you who needs to make the change.

As discussed off-list, the rename has now occurred, thanks!

And I have now applied this patch to main.
- Documentation: Updates for rename of primary development branch as main.
  https://github.com/openvswitch/ovs/commit/74cf01436fd2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] northd, controller: Use paused controller action for packet buffering.

[Ales Musil]

On Tue, Apr 9, 2024 at 5:54 PM Numan Siddique  wrote:

>
>
> On Tue, Apr 9, 2024, 7:46 AM Dumitru Ceara  wrote:
>
>> On 4/8/24 09:19, Ales Musil wrote:
>> > On Fri, Apr 5, 2024 at 10:35 PM Numan Siddique  wrote:
>> >
>> >>
>> >>
>> >> On Tue, Apr 2, 2024 at 2:28 AM Ales Musil  wrote:
>> >>
>> >>> The current packet injection loses ct_state in the process. When
>> >>> the ct_state is lost we might commit to DNAT zone and perform
>> >>> zero SNAT after the packet injection. This causes the first session
>> >>> to be wrong as the reply packets are not unDNATted.
>> >>>
>> >>> Instead of re-injecting the packet back into the pipeline when
>> >>> we get the MAC binding, use paused controller action. The paused
>> >>> controller action stores ct_state, which is important for the behavior
>> >>> of the resumed packet.
>> >>>
>> >>> At the same time bump the OvS submodule latest branch-3.3. This is
>> >>> mainly for [0], which fixes metering for paused controller actions.
>> >>>
>> >>> [0] c560f6ca3257 ("ofproto-dpif-xlate: Fix continuations with
>> associated
>> >>> metering.")
>> >>>
>> >>> Reported-at: https://issues.redhat.com/browse/FDP-439
>> >>> Signed-off-by: Ales Musil 
>> >>> Acked-by: Mark Michelson 
>> >>> ---
>> >>> v2: Fix the Jira link and add ack from Mark.
>> >>>
>> >>
>> >> Hi Ales,
>> >>
>> >> I see one problem during upgrades.  The packet buffering functionality
>> >> would be broken when ovn-controllers are updated to the version
>> >> which  has your patch  but ovn-northd is still yet to.
>> >>
>> >> In this case, the below logical flow will be present  (without the
>> outer
>> >> "output" action)
>> >>
>> >> ---
>> >>   table=21(lr_in_arp_request  ), priority=100  , match=(eth.dst ==
>> >> 00:00:00:00:00:00 && ip4), action=(arp { eth.dst = ff:ff:ff:ff:ff:ff;
>> >> arp.spa = reg1; arp.tpa = reg0; arp.op = 1; output; };)
>> >> ---
>> >>
>> >> The updated ovn-controller will now resume the packet after learning
>> the
>> >> nexthop.  But this resumed packet will be dropped because there is no
>> >> "next" or "output" action following arp {}.
>> >>
>> >> Is this behavior acceptable until ovn-northd is also updated/upgraded ?
>> >>
>> >
>> >> Thanks
>> >> Numan
>> >>
>> >>
>> > Hi Numan,
>> >
>> > this is a very good point, thank you for raising this. It probably
>> depends
>> > how long the upgrade takes, for ovn-kubernetes with ic it is all done
>> per
>> > node so that should be fine. However it might be more concerning for
>>
>> I'm afraid with OpenStack upgrade roll outs can take significantly
>> longer.  That means running with an older version of ovn-northd while
>> some of the chassis run newer ovn-controllers.
>>
>> > OpenStack. In the end we would be losing only a small amount of packets
>> (in
>> > most cases only 1) for connections without MAC binding, on the other
>> hand
>> > the scenario it tries to fix is causing the first connection (whole
>> > connection) to be completely broken.
>> >
>> > I've tried to think about a possible solution to avoid this problem and
>> I
>> > see only one way out of this. That would involve keeping the old arp
>> > unchanged, adding new action that would result in the pause instead. I'm
>> > not sure if that's worth the effort, it's hard to estimate how big of an
>> > impact it might have during the upgrade. One unfortunate thing is that
>> we
>> > cannot modify the actions of the continuation, if we could there
>> wouldn't
>> > be any change required in northd.
>> >
>> > Let me know if you have other ideas on how to avoid this issue, maybe
>> the
>> > solution might be easier.
>> >
>>
>> I don't have a great alternative.  The only thing that seems safe is:
>> - add a new action + a feature flag
>> - deprecate and keep the old action for a release then remove it
>>
>
>
> My suggestion would be
>  - In ovn-controller, check it's  version and the northd version.
>  - if northd version is lesser than it's version,  then don't encode
> the arp/ns with pause flag.  Otherwise do.
>  - I think you can add new action types (only for pinctrl.c) for
> arp/ns with pause flag set.
>
>
> This way ovn-controller can still have 2 versions of arp/ns with just one
> version of OVN actions - arp/ ns which northd uses.
>
>
>
>
> Thanks
> Numan
>

After some discussion and testing I went with adding an option in northd
that will indicate if it's supported, if not the controller will emit
implicit output action. To ensure that this works I have added a check into
ovn-fake-mutlinode test.
Posted in v3.



>
>
> Regards,
>> Dumitru
>>
>> >
>> >> ---
>> >>
>> >>>  controller/mac-learn.c | 30 +
>> >>>  controller/mac-learn.h |  9 ++--
>> >>>  controller/pinctrl.c   | 64 ++--
>> >>>  lib/actions.c  | 41 +-
>> >>>  northd/northd.c|  6 +--
>> >>>  ovs|  2 +-
>> >>>  tests/ovn-northd.at|  3 ++
>> >>>  tests/ovn.at   |  8 ++--
>> >>>  tests/system-ovn.at| 95
>> 

[ovs-dev] [PATCH ovn v3 2/2] northd, controller: Use paused controller action for packet buffering.

[Ales Musil]

The current packet injection loses ct_state in the process. When
the ct_state is lost we might commit to DNAT zone and perform
zero SNAT after the packet injection. This causes the first session
to be wrong as the reply packets are not unDNATted.

Instead of re-injecting the packet back into the pipeline when
we get the MAC binding, use paused controller action. The paused
controller action stores ct_state, which is important for the behavior
of the resumed packet.

At the same time bump the OvS submodule latest branch-3.3. This is
mainly for [0], which fixes metering for paused controller actions.

In order to make sure that the paused action works during upgrade add
the output implicitly. Once the upgrade is done northd will create option
to inform controllers that the implicit action is no longer needed.

[0] c560f6ca3257 ("ofproto-dpif-xlate: Fix continuations with associated 
metering.")

Reported-at: https://issues.redhat.com/browse/FDP-439
Signed-off-by: Ales Musil 
---
v4: Add flag to ensure that the paused action works during upgrade.
v3: Rebase on top of main.
v2: Fix the Jira link and add ack from Mark.
---
 controller/lflow.c  |  1 +
 controller/lflow.h  |  1 +
 controller/mac-learn.c  | 30 
 controller/mac-learn.h  |  9 ++--
 controller/ovn-controller.c | 21 
 controller/pinctrl.c| 64 +
 include/ovn/actions.h   |  3 ++
 lib/actions.c   | 47 +-
 northd/en-global-config.c   |  4 ++
 northd/northd.c |  6 +--
 ovs |  2 +-
 tests/multinode.at  |  8 
 tests/ovn-northd.at |  3 ++
 tests/ovn.at|  8 ++--
 tests/system-ovn.at | 95 +
 tests/test-ovn.c|  1 +
 16 files changed, 240 insertions(+), 63 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 895d17d19..760ec0b41 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -874,6 +874,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow 
*lflow,
 .collector_ids = l_ctx_in->collector_ids,
 .lflow_uuid = lflow->header_.uuid,
 .dp_key = ldp->datapath->tunnel_key,
+.explicit_arp_ns_output = l_ctx_in->explicit_arp_ns_output,
 
 .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
 .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
diff --git a/controller/lflow.h b/controller/lflow.h
index 9b7ffa19c..295d004f4 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -130,6 +130,7 @@ struct lflow_ctx_in {
 bool lb_hairpin_use_ct_mark;
 bool localnet_learn_fdb;
 bool localnet_learn_fdb_changed;
+bool explicit_arp_ns_output;
 };
 
 struct lflow_ctx_out {
diff --git a/controller/mac-learn.c b/controller/mac-learn.c
index 071f01b4f..0c3b60c23 100644
--- a/controller/mac-learn.c
+++ b/controller/mac-learn.c
@@ -199,15 +199,24 @@ ovn_fdb_add(struct hmap *fdbs, uint32_t dp_key, struct 
eth_addr mac,
 /* packet buffering functions */
 
 struct packet_data *
-ovn_packet_data_create(struct ofpbuf ofpacts,
-   const struct dp_packet *original_packet)
+ovn_packet_data_create(const struct ofputil_packet_in *pin,
+   const struct ofpbuf *continuation)
 {
 struct packet_data *pd = xmalloc(sizeof *pd);
 
-pd->ofpacts = ofpacts;
-/* clone the packet to send it later with correct L2 address */
-pd->p = dp_packet_clone_data(dp_packet_data(original_packet),
- dp_packet_size(original_packet));
+pd->pin = (struct ofputil_packet_in) {
+.packet = xmemdup(pin->packet, pin->packet_len),
+.packet_len = pin->packet_len,
+.flow_metadata = pin->flow_metadata,
+.reason = pin->reason,
+.table_id = pin->table_id,
+.cookie = pin->cookie,
+/* Userdata are empty on purpose,
+ * it is not needed for the continuation. */
+.userdata = NULL,
+.userdata_len = 0,
+};
+pd->continuation = ofpbuf_clone(continuation);
 
 return pd;
 }
@@ -216,8 +225,8 @@ ovn_packet_data_create(struct ofpbuf ofpacts,
 void
 ovn_packet_data_destroy(struct packet_data *pd)
 {
-dp_packet_delete(pd->p);
-ofpbuf_uninit(>ofpacts);
+free(pd->pin.packet);
+ofpbuf_delete(pd->continuation);
 free(pd);
 }
 
@@ -307,7 +316,10 @@ ovn_buffered_packets_ctx_run(struct buffered_packets_ctx 
*ctx,
 
 struct packet_data *pd;
 LIST_FOR_EACH_POP (pd, node, >queue) {
-struct eth_header *eth = dp_packet_data(pd->p);
+struct dp_packet packet;
+dp_packet_use_const(, pd->pin.packet, pd->pin.packet_len);
+
+struct eth_header *eth = dp_packet_data();
 eth->eth_dst = mac;
 
 ovs_list_push_back(>ready_packets_data, >node);
diff --git a/controller/mac-learn.h b/controller/mac-learn.h
index e0fd6a8d1..20a015e1a 100644
--- 

[ovs-dev] [PATCH ovn v3 1/2] ci: Make sure that multinode test runs on correct branch.

[Ales Musil]

The ovn-fake-multinode workflow can be triggered manually,
however the definition didn't respect the branch for the manual
run and always used main branch. Make sure that the correct
branch is used for the ovn-fake-multinode workflow.

Fixes: 033f5bebf94d ("CI: Add a couple of periodic jobs using 
ovn-fake-multinode.")
Signed-off-by: Ales Musil 
---
 .github/workflows/ovn-fake-multinode-tests.yml | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/.github/workflows/ovn-fake-multinode-tests.yml 
b/.github/workflows/ovn-fake-multinode-tests.yml
index 79b6c4253..f097ee9a1 100644
--- a/.github/workflows/ovn-fake-multinode-tests.yml
+++ b/.github/workflows/ovn-fake-multinode-tests.yml
@@ -17,7 +17,7 @@ jobs:
 strategy:
   matrix:
 cfg:
-- { branch: "main" }
+- { branch: "${{ github.ref_name }}" }
 - { branch: "branch-22.03" }
 env:
   RUNC_CMD: podman
@@ -47,7 +47,7 @@ jobs:
   with:
 path: 'ovn-fake-multinode/ovn'
 submodules: recursive
-ref: ${{ matrix.cfg.branch }}
+ref: "${{ matrix.cfg.branch }}"
 
 - name: Install dependencies
   run: |
@@ -76,16 +76,15 @@ jobs:
   fail-fast: false
   matrix:
 cfg:
-- { branch: "main", testsuiteflags: ""}
 - { branch: "branch-22.03", testsuiteflags: "-k 'basic test'" }
 name: multinode tests ${{ join(matrix.cfg.*, ' ') }}
 env:
   RUNC_CMD: podman
   OS_IMAGE: "fedora:37"
   CENTRAL_IMAGE: "ovn/ovn-multi-node:${{ matrix.cfg.branch }}"
-  CHASSIS_IMAGE: "ovn/ovn-multi-node:main"
-  RELAY_IMAGE: "ovn/ovn-multi-node:main"
-  GW_IMAGE: "ovn/ovn-multi-node:main"
+  CHASSIS_IMAGE: "ovn/ovn-multi-node:${{ github.ref_name }}"
+  RELAY_IMAGE: "ovn/ovn-multi-node:${{ github.ref_name }}"
+  GW_IMAGE: "ovn/ovn-multi-node:${{ github.ref_name }}"
   # Disable SSL for now. Revisit this if required.
   ENABLE_SSL: no
   CC: gcc
@@ -114,7 +113,7 @@ jobs:
 
 - uses: actions/download-artifact@v4
   with:
-name: test-main-image
+name: test-${{ github.ref_name }}-image
 
 - uses: actions/download-artifact@v4
   with:
@@ -122,7 +121,7 @@ jobs:
 
 - name: Load podman image
   run: |
-sudo podman load --input ovn_main_image.tar
+sudo podman load --input ovn_${{ github.ref_name }}_image.tar
 sudo podman load --input ovn_branch-22.03_image.tar
 
 - name: Check out ovn-fake-multi-node
-- 
2.44.0

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

[ovs-dev] [PATCH ovn] ci: Rename OvS master branch to main.

[Ales Musil]

There was recent switch in OvS from master to main branch.
Use main instead in ovn-fake-mutlinode tests.

Signed-off-by: Ales Musil 
---
 .github/workflows/ovn-fake-multinode-tests.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/ovn-fake-multinode-tests.yml 
b/.github/workflows/ovn-fake-multinode-tests.yml
index 79b6c4253..ed4d6c4ef 100644
--- a/.github/workflows/ovn-fake-multinode-tests.yml
+++ b/.github/workflows/ovn-fake-multinode-tests.yml
@@ -35,12 +35,12 @@ jobs:
 # Check out ovn and ovs separately inside ovn-fake-multinode/ovn and 
ovn-fake-multinode/ovs
 # 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
+- name: Check out ovs main
   uses: actions/checkout@v4
   with:
 path: 'ovn-fake-multinode/ovs'
 repository: 'openvswitch/ovs'
-ref: 'master'
+ref: 'main'
 
 - name: Check out ovn ${{ matrix.cfg.branch }}
   uses: actions/checkout@v4
-- 
2.44.0

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

Re: [ovs-dev] [PATCH] checkpatch: Allow rST manpages to be added.

[Simon Horman]

On Tue, Apr 09, 2024 at 09:19:02AM +0200, Adrian Moreno wrote:
> The current __check_doc_is_listed() verifies that the new .rst file is
> listed in Documentation/automake.mk with the full path (i.e:
> "{directory}/{filename}").
> 
> While this holds true for generic documentation files, which are added
> to DOC_SOURCE with the full path, it's not true for rST manpages which
> are added only by filename to RST_MANPAGES target (see
> Documentation/automake.mk).
> 
> This makes the current implementation of the check to incorrectly raise
> a warning as follows even though the patch does add the file to
> RST_MANPAGES:
> 
> """
> WARNING: New doc ovs-flowviz.8.rst not listed in
> Documentation/automake.mk
> """
> 
> Fix it by making the {dir}/ part of the docre regexp optional.
> 
> Signed-off-by: Adrian Moreno 

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

Re: [ovs-dev] [PATCH] vlog: Log stack trace on vlog_abort.

[Ilya Maximets]

On 4/6/24 00:08, Ilya Maximets wrote:
> Currently, calls like ovs_assert() just print out a condition that
> caused assertion to fail.  But it may be not enough to understand what
> exactly has happened, especially if assertion failed in some generic
> function like dp_packet_resize() or similar.
> 
> Print the stack trace along with the abort message to give more context
> for the later debugging.
> 
> This should be especially useful in case the issue happens in an
> environment with core dumps disabled.
> 
> Adding the log to vlog_abort() to cover a little more cases where
> VLOG_ABORT is called and not only assertion failures.
> 
> It would be nice to also have stack traces in case of reaching the
> OVS_NOT_REACHED().  But this macro is used in some places as a last
> resort and should not actually do more than just stopping the process
> immediately.  And it also can be used in contexts without logging
> initialized.  Such a change will need to be done more carefully.
> Better solution might be to use VLOG_ABORT() where appropriate instead.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/vlog.c   | 10 --
>  tests/library.at |  4 +++-
>  2 files changed, 11 insertions(+), 3 deletions(-)

If this change is accepted, I'd also suggest backporting it to 3.3.
It is a debug-only change that touches only the code executed under
fatal failure conditions, so should be safe enough.  Backporting
will allow us easier debugging in to-be-LTS release.  And also OVN
24.03 LTS can make use of it this way as well.

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

Re: [ovs-dev] [PATCH v7] rhel: make the version, displayed to the user, customizable

[Ilya Maximets]

On 4/3/24 17:15, Timothy Redaelli wrote:
> Since on CentOS/RHEL the builds are based on stable branches and not on
> tags for debugging purpose it's better to have the downstream version as
> version so it's easier to know which commits are included in a build.
> 
> This commit adds --with-version-suffix as ./configure option in
> order to set an OVS version suffix that should be shown to the user via
> ovs-vsctl -V and, so, also on database, on ovs-vsctl show and the other
> utilities.
> 
> --with-version-suffix is used in Fedora/CentOS/RHEL spec file in order to have
> the version be aligned with the downstream one.
> 
> Signed-off-by: Timothy Redaelli 
> ---
> v1 -> v2: Use --with-version-suffix= and add version to other utilies
>   (as requested by Ilya).
> 
> v2 -> v3: Add versioning to python utilities and python library itself
>   (as suggested by Aaron).
> 
> v3 -> v4: Remove versioning to python library itself to avoid breaking
>   PEP440 (as requested by Ilya). Versioning is still used in
>   python utilities.
> 
> v4 -> v5: Re-add versioning to python library itself, but don't use it on
>   setup.py (to avoid breaking PEP440). This will permit to have the
>   custom version as ovs.version.VERSION (in case somebody uses it) 
> and,
>   so, also in python/ovs/unixctl/server.py (as suggested by Ilya).
> 
> v5 -> v6: Fix some setup.py leftovers and change the test as a loop
>   (as suggested by Ilya).
> 
> v6 -> v7: Rebase with last master (it should pass CI now)
> ---
>  Makefile.am|  1 +
>  acinclude.m4   | 13 +
>  configure.ac   |  1 +
>  include/openvswitch/version.h.in   |  2 +-
>  lib/ovsdb-error.c  |  2 +-
>  lib/util.c |  5 +++--
>  ovsdb/ovsdb-server.c   |  3 ++-
>  python/.gitignore  |  1 +
>  python/automake.mk | 11 ++-
>  python/{setup.py => setup.py.template} |  9 -
>  rhel/openvswitch-fedora.spec.in|  1 +
>  utilities/ovs-dpctl-top.in |  2 +-
>  utilities/ovs-lib.in   |  2 +-
>  utilities/ovs-parse-backtrace.in   |  2 +-
>  utilities/ovs-pcap.in  |  2 +-
>  utilities/ovs-pki.in   |  2 +-
>  utilities/ovs-tcpdump.in   |  4 ++--
>  utilities/ovs-tcpundump.in |  2 +-
>  utilities/ovs-vlan-test.in |  2 +-
>  vswitchd/bridge.c  |  3 ++-
>  20 files changed, 49 insertions(+), 21 deletions(-)
>  rename python/{setup.py => setup.py.template} (95%)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 45fce1243..987bc89ca 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -163,6 +163,7 @@ SUFFIXES += .in
>   -e 's,[@]PYTHON3[@],$(PYTHON3),g' \
>   -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>   -e 's,[@]VERSION[@],$(VERSION),g' \
> + -e 's,[@]VERSION_SUFFIX[@],$(VERSION_SUFFIX),g' \
>   -e 's,[@]localstatedir[@],$(localstatedir),g' \
>   -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>   -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> diff --git a/acinclude.m4 b/acinclude.m4
> index f1ba046c2..f7a81a734 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -497,6 +497,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>  ])
>  
> +dnl Append a version suffix
> +
> +AC_DEFUN([OVS_CHECK_VERSION_SUFFIX], [
> +  AC_ARG_WITH([version-suffix],
> +  [AS_HELP_STRING([--with-version-suffix=ver_suffix],
> +  [Specify a version suffix that will be appended
> +   to OVS version])])
> +  AC_DEFINE_UNQUOTED([VERSION_SUFFIX], ["$with_version_suffix"],
> + [Package version suffix])
> +  AC_SUBST([VERSION_SUFFIX], [$with_version_suffix])
> +  ])
> +])
> +
>  dnl Checks for net/if_dl.h.
>  dnl
>  dnl (We use this as a proxy for checking whether we're building on FreeBSD
> diff --git a/configure.ac b/configure.ac
> index dd6553fea..8323e481d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -202,6 +202,7 @@ OVS_CHECK_LINUX_SCTP_CT
>  OVS_CHECK_LINUX_VIRTIO_TYPES
>  OVS_CHECK_DPDK
>  OVS_CHECK_PRAGMA_MESSAGE
> +OVS_CHECK_VERSION_SUFFIX
>  AC_SUBST([CFLAGS])
>  AC_SUBST([OVS_CFLAGS])
>  AC_SUBST([OVS_LDFLAGS])
> diff --git a/include/openvswitch/version.h.in 
> b/include/openvswitch/version.h.in
> index 23d8fde4f..231f61e30 100644
> --- a/include/openvswitch/version.h.in
> +++ b/include/openvswitch/version.h.in
> @@ -19,7 +19,7 @@
>  #define OPENVSWITCH_VERSION_H 1
>  
>  #define OVS_PACKAGE_STRING  "@PACKAGE_STRING@"
> -#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@"
> +#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@@VERSION_SUFFIX@"
>  
>  #define OVS_LIB_VERSION @LT_CURRENT@
>  #define OVS_LIB_REVISION@LT_REVISION@
> diff --git 

Re: [ovs-dev] [PATCH v3] Documentation: Updates for rename of primary development branch as main.

[Simon Horman]

On Tue, Apr 09, 2024 at 07:28:08PM +0200, Ilya Maximets wrote:
> On 4/5/24 15:57, Simon Horman wrote:
> > Recently OVS adopted a policy of using the inclusive naming word list v1
> > [1, 2].
> > 
> > In keeping with this policy rename the primary development branch from
> > 'master' to 'main'. This patch does not actually make that change, but
> > rather updates references to the branch in documentation in the source
> > tree.  It is intended to be applied at (approximately) the same time
> > that the change is made.
> > 
> > OVS is currently hosted on GitHub. We can expect the following behaviour
> > after the rename:
> > 
> > 1. GitHub pull requests against are renamed branch are automatically
> >re-homed on new branch
> > 2. GitHub Issues do not seem to be affected - at least the test issue I
> >created had no association with a branch
> > 3. URLs accessed via the GitHub web UI are automatically renamed
> >(so long as a new branch called master is not created).
> > 4. Using the git cli command, fetch will fetch the new branch (main),
> >and fetch -p will remove (prune) the old branch (master)
> > 
> > [1] df5e5cf4318a ("Documentation: Add section on inclusive language.")
> > [2] https://inclusivenaming.org/word-lists/
> > 
> > Signed-off-by: Simon Horman 
> > ---
> > Changes in v3:
> > - This patch only updates documentation. Update the patch prefix and
> >   description accordingly.
> > - Drop documentation of 'tested on "master" branch' rather than updating
> >   it, as the updated text seems somewhat nonsensical.
> > - Correct indentation of NEWS entry.
> 
> Thanks, Simon!
> 
> Acked-by: Ilya Maximets 
> 
> > 
> > Changes in v2:
> > - Keep two blank lines between versions.
> > - Drop bogus update to OpenSSL hashes URL in appveyor.yml.
> > - Drop other appveyor.yml changes, they are now present upstream.
> >   + appveyor: Prepare for rename of primary development branch.
> > https://github.com/openvswitch/ovs/commit/95ff912edef8
> > - Add note about updates to git configuration.
> > ---
> > Notes:
> > 
> > * Now is the time to raise any concerns regarding this patch.
> >   It is planned to implement this change next week.
> 
> Do you have particular date in mind?

Hi Ilya,

I think that today or tomorrow would be good, to give a bit
of space before the weekend. But whenever suits you: I believe
it is you who needs to make the change.

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Eelco Chaudron]

On 10 Apr 2024, at 12:49, Ilya Maximets wrote:

> On 4/10/24 12:44, Eelco Chaudron wrote:
>>
>>
>> On 10 Apr 2024, at 12:09, Kevin Traynor wrote:
>>
>>> On 10/04/2024 11:04, Kevin Traynor wrote:
 On 10/04/2024 10:51, Eelco Chaudron wrote:
>
>
> On 10 Apr 2024, at 11:41, Ilya Maximets wrote:
>
>> 13.3 was released on March 5 and 13.2 will reach EoL in June.
>> Update now.
>
> Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough 
> GitHub actions;
>

 I ran it here:
 https://cirrus-ci.com/build/5951252381564928

>>>
>>> I was assuming you meant cirrus, but just in case, github actions is
>>> here: https://github.com/kevintraynor/ovs/actions/runs/8629238065
>>
>> Well I meant Cirrus invocation trough the GitHub actions framework ;)
>
> They are separate.  GHA doesn't invoke Cirrus or AppVeyor.
> They invoke themselves by being subscribed to repo changes.

Good to know!

>>
>> Thanks,
>>
>>
>> Eelco
>>
 Acked-by: Kevin Traynor 

> Acked-by: Eelco Chaudron 
>
> //Eelco
>
>
>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  .cirrus.yml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/.cirrus.yml b/.cirrus.yml
>> index d8a972280..8db385f00 100644
>> --- a/.cirrus.yml
>> +++ b/.cirrus.yml
>> @@ -2,7 +2,7 @@ freebsd_build_task:
>>
>>freebsd_instance:
>>  matrix:
>> -  image_family: freebsd-13-2-snap
>> +  image_family: freebsd-13-3-snap
>>image_family: freebsd-14-0-snap
>>  cpu: 4
>>  memory: 4G
>> -- 
>> 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
>

>>

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Ilya Maximets]

On 4/10/24 12:44, Eelco Chaudron wrote:
> 
> 
> On 10 Apr 2024, at 12:09, Kevin Traynor wrote:
> 
>> On 10/04/2024 11:04, Kevin Traynor wrote:
>>> On 10/04/2024 10:51, Eelco Chaudron wrote:


 On 10 Apr 2024, at 11:41, Ilya Maximets wrote:

> 13.3 was released on March 5 and 13.2 will reach EoL in June.
> Update now.

 Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough 
 GitHub actions;

>>>
>>> I ran it here:
>>> https://cirrus-ci.com/build/5951252381564928
>>>
>>
>> I was assuming you meant cirrus, but just in case, github actions is
>> here: https://github.com/kevintraynor/ovs/actions/runs/8629238065
> 
> Well I meant Cirrus invocation trough the GitHub actions framework ;)

They are separate.  GHA doesn't invoke Cirrus or AppVeyor.
They invoke themselves by being subscribed to repo changes.

> 
> Thanks,
> 
> 
> Eelco
> 
>>> Acked-by: Kevin Traynor 
>>>
 Acked-by: Eelco Chaudron 

 //Eelco



> Signed-off-by: Ilya Maximets 
> ---
>  .cirrus.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index d8a972280..8db385f00 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -2,7 +2,7 @@ freebsd_build_task:
>
>freebsd_instance:
>  matrix:
> -  image_family: freebsd-13-2-snap
> +  image_family: freebsd-13-3-snap
>image_family: freebsd-14-0-snap
>  cpu: 4
>  memory: 4G
> -- 
> 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

>>>
> 

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Eelco Chaudron]

On 10 Apr 2024, at 12:09, Kevin Traynor wrote:

> On 10/04/2024 11:04, Kevin Traynor wrote:
>> On 10/04/2024 10:51, Eelco Chaudron wrote:
>>>
>>>
>>> On 10 Apr 2024, at 11:41, Ilya Maximets wrote:
>>>
 13.3 was released on March 5 and 13.2 will reach EoL in June.
 Update now.
>>>
>>> Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough 
>>> GitHub actions;
>>>
>>
>> I ran it here:
>> https://cirrus-ci.com/build/5951252381564928
>>
>
> I was assuming you meant cirrus, but just in case, github actions is
> here: https://github.com/kevintraynor/ovs/actions/runs/8629238065

Well I meant Cirrus invocation trough the GitHub actions framework ;)

Thanks,


Eelco

>> Acked-by: Kevin Traynor 
>>
>>> Acked-by: Eelco Chaudron 
>>>
>>> //Eelco
>>>
>>>
>>>
 Signed-off-by: Ilya Maximets 
 ---
  .cirrus.yml | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/.cirrus.yml b/.cirrus.yml
 index d8a972280..8db385f00 100644
 --- a/.cirrus.yml
 +++ b/.cirrus.yml
 @@ -2,7 +2,7 @@ freebsd_build_task:

freebsd_instance:
  matrix:
 -  image_family: freebsd-13-2-snap
 +  image_family: freebsd-13-3-snap
image_family: freebsd-14-0-snap
  cpu: 4
  memory: 4G
 -- 
 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
>>>
>>

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Kevin Traynor]

On 10/04/2024 11:04, Kevin Traynor wrote:
> On 10/04/2024 10:51, Eelco Chaudron wrote:
>>
>>
>> On 10 Apr 2024, at 11:41, Ilya Maximets wrote:
>>
>>> 13.3 was released on March 5 and 13.2 will reach EoL in June.
>>> Update now.
>>
>> Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough GitHub 
>> actions;
>>
> 
> I ran it here:
> https://cirrus-ci.com/build/5951252381564928
> 

I was assuming you meant cirrus, but just in case, github actions is
here: https://github.com/kevintraynor/ovs/actions/runs/8629238065

> Acked-by: Kevin Traynor 
> 
>> Acked-by: Eelco Chaudron 
>>
>> //Eelco
>>
>>
>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>  .cirrus.yml | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/.cirrus.yml b/.cirrus.yml
>>> index d8a972280..8db385f00 100644
>>> --- a/.cirrus.yml
>>> +++ b/.cirrus.yml
>>> @@ -2,7 +2,7 @@ freebsd_build_task:
>>>
>>>freebsd_instance:
>>>  matrix:
>>> -  image_family: freebsd-13-2-snap
>>> +  image_family: freebsd-13-3-snap
>>>image_family: freebsd-14-0-snap
>>>  cpu: 4
>>>  memory: 4G
>>> -- 
>>> 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
>>
> 

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Kevin Traynor]

On 10/04/2024 10:51, Eelco Chaudron wrote:
> 
> 
> On 10 Apr 2024, at 11:41, Ilya Maximets wrote:
> 
>> 13.3 was released on March 5 and 13.2 will reach EoL in June.
>> Update now.
> 
> Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough GitHub 
> actions;
> 

I ran it here:
https://cirrus-ci.com/build/5951252381564928

Acked-by: Kevin Traynor 

> Acked-by: Eelco Chaudron 
> 
> //Eelco
> 
> 
> 
>> Signed-off-by: Ilya Maximets 
>> ---
>>  .cirrus.yml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/.cirrus.yml b/.cirrus.yml
>> index d8a972280..8db385f00 100644
>> --- a/.cirrus.yml
>> +++ b/.cirrus.yml
>> @@ -2,7 +2,7 @@ freebsd_build_task:
>>
>>freebsd_instance:
>>  matrix:
>> -  image_family: freebsd-13-2-snap
>> +  image_family: freebsd-13-3-snap
>>image_family: freebsd-14-0-snap
>>  cpu: 4
>>  memory: 4G
>> -- 
>> 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
> 

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

Re: [ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Eelco Chaudron]

On 10 Apr 2024, at 11:41, Ilya Maximets wrote:

> 13.3 was released on March 5 and 13.2 will reach EoL in June.
> Update now.

Changing from 13.2 to 13.3 looks fine to me. Assume you ran it trough GitHub 
actions;

Acked-by: Eelco Chaudron 

//Eelco



> Signed-off-by: Ilya Maximets 
> ---
>  .cirrus.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index d8a972280..8db385f00 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -2,7 +2,7 @@ freebsd_build_task:
>
>freebsd_instance:
>  matrix:
> -  image_family: freebsd-13-2-snap
> +  image_family: freebsd-13-3-snap
>image_family: freebsd-14-0-snap
>  cpu: 4
>  memory: 4G
> -- 
> 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

[ovs-dev] [PATCH] cirrus: Update to FreeBSD 13.3.

[Ilya Maximets]

13.3 was released on March 5 and 13.2 will reach EoL in June.
Update now.

Signed-off-by: Ilya Maximets 
---
 .cirrus.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index d8a972280..8db385f00 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -2,7 +2,7 @@ freebsd_build_task:
 
   freebsd_instance:
 matrix:
-  image_family: freebsd-13-2-snap
+  image_family: freebsd-13-3-snap
   image_family: freebsd-14-0-snap
 cpu: 4
 memory: 4G
-- 
2.44.0

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

Re: [ovs-dev] [PATCH] checkpatch: Don't spellcheck names in tags.

[Eelco Chaudron]

On 9 Apr 2024, at 21:55, Ilya Maximets wrote:

> Current code checks spelling of names in commit message tags and that
> makes no sense.
>
> Most of the tags are explicitly handled, but tags like 'Tested-by' or
> other lesser used ones are falling through to the spellchecker and
> need to be excluded.
>
> Signed-off-by: Ilya Maximets 

Thanks for the enhancement.

Acked-by: Eelco Chaudron 

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

Re: [ovs-dev] Hardware Offload Scaling

[Eelco Chaudron]

On 9 Apr 2024, at 16:58, Shahaji Bhosle wrote:

> Thanks Eelco, Sorry for the late reply.
> We are not using NVDIA NIC, and are using OVS+DPDK.
> It appears the revalidator threads are complaining because it is not able
> to get all the stats for million flows within its time window.
> Just wondering if there are knobs to relax the revalidator to not complain.
> We have used the below settings
> other_config: {dpdk-extra="--log-level=pmd.*,debug",
> dpdk-init="true", dpdk-lcore-mask="0x4", dpdk-mem-channels="4",
> dpdk-socket-mem="4096,0", flow-limit="200", hw-offload="true",
> max-idle="600", max-revalidator="4000", n-handler-threads="4",
> n-offload-threads="3", n-revalidator-threads="4", per-port-memory="true",
> pmd-cpu-mask="0x15554"}

It would be interesting to profile, and see where statistics gathering can be 
further optimized. What I remember from the past, is that 1M none hw-offloaded 
OVS-DPDK flows were not a problem.

The following blog explains the revalidator process; 
https://developers.redhat.com/articles/2022/10/19/open-vswitch-revalidator-process-explained#error=login_required=e0f29d5d-e75c-4e6a-89de-cfd7cdccff7e

There is a tool (also explained in the blog) you can use to see how much time 
the revalidator needs.
You can also see the reason for not having the desired number of flows, i.e. is 
it because the dynamic flow_limit is adjusted lower, and/or ‘duration < 
(other_config:max-revalidator / 2’ see ‘The revalidate_ukey function’.

For the later, you can try to increase the max-revalidator value to a second, 
‘ovs-vsctl set Open_vSwitch . other_config:max-revalidator=1000’. I would 
personally not go over 1seconds, as it affects the statists shown in dump-flows.

Good luck,

Eelco

> Thanks, Shahaji
>
>
> On Thu, Mar 28, 2024 at 1:54 PM Eelco Chaudron  wrote:
>
>>
>>
>> On 28 Mar 2024, at 16:31, Shahaji Bhosle via dev wrote:
>>
>>> Hi,
>>> I am looking for tuning params for scaling OVS-DPDK/TC to million flows.
>> It
>>> seems like flow dump(revalidator) is not able to collect the stats after
>>> about 150K flows per second, thus it is trying to delete the flows.
>>> Any guidance where million flows are offloaded to hardware would be
>> great.
>>> Thanks, Shahaji
>>
>> This is probably because it takes OVS a while to get all the stats from
>> the kernel/driver. What kind of NIC are you using? If it’s Nvidia, which
>> mode, dmfs or smfs. I’ve seen dmfs being rather slow causing a similar
>> effect as you describe. Try switching to smfs and see if the problem goes
>> away.
>>
>> I also assume you use a recent version of OVS, if not make sure your
>> version includes the following:
>>
>>   e1e5eac5b016 ("tc: Add TCA_KIND flower to delete and get operation to
>> avoid rtnl_lock().")
>>
>>
>> Cheers,
>>
>>
>> Eelco
>>
>>
>
> -- 
> This electronic communication and the information and any files transmitted
> with it, or attached to it, are confidential and are intended solely for
> the use of the individual or entity to whom it is addressed and may contain
> information that is confidential, legally privileged, protected by privacy
> laws, or otherwise restricted from disclosure to anyone else. If you are
> not the intended recipient or the person responsible for delivering the
> e-mail to the intended recipient, you are hereby notified that any use,
> copying, distributing, dissemination, forwarding, printing, or copying of
> this e-mail is strictly prohibited. If you received this e-mail in error,
> please return the e-mail to the sender, delete it from your computer, and
> destroy any printed copy of it.

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