Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go

2024-06-16 Thread Robin Jarry
Hi Guilherme,

Thanks for reaching out. I have been lagging behind with this. Yes I could use 
some help. There are some dependencies missing.



Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go

2024-06-16 Thread Robin Jarry
Hi Guilherme,

Thanks for reaching out. I have been lagging behind with this. Yes I could use 
some help. There are some dependencies missing.



Re: [ovs-dev] [PATCH v2] netdev-dpdk: Use LSC interrupt mode.

2024-06-14 Thread Robin Jarry

David Marchand, Jun 14, 2024 at 08:48:

diff --git a/NEWS b/NEWS
index 5ae0108d55..1e19beb793 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,9 @@ Post-v3.3.0
  https://github.com/openvswitch/ovs.git
- DPDK:
  * OVS validated with DPDK 23.11.1.
+ * Link status changes are now handled via interrupt mode if the DPDK
+   driver supports it.  It is possible to revert to polling mode by setting
+   per interface 'dpdk-lsc-interrupt' other_config to 'false'.


Nit pick: this setting is options:dpdk-lsc-interrupt not in 
other_config.


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


Re: [ovs-dev] [PATCH v2] netdev-dpdk: Use LSC interrupt mode.

2024-06-14 Thread Robin Jarry

David Marchand, Jun 14, 2024 at 08:48:

Querying link status may get delayed for an undeterministic (long) time
with mlx5 ports. This is a consequence of the mlx5 driver calling ethtool
kernel API and getting stuck on the kernel RTNL lock while some other
operation is in progress under this lock.

One impact for long link status query is that it is called under the bond
lock taken in write mode periodically in bond_run().
In parallel, datapath threads may block requesting to read bonding related
info (like for example in bond_check_admissibility()).

The LSC interrupt mode is available with many DPDK drivers and is used by
default with testpmd.

It seems safe enough to switch on this feature by default in OVS.
We keep the per interface option to disable this feature in case of an
unforeseen bug.

Signed-off-by: David Marchand 
---
Changes since v1:
- (early) fail when interrupt lsc is requested by user but not supported
  by the driver,
- otherwise, log a debug message if user did not request interrupt mode,


This looks good to me. Is there a chance that this could be backported 
to the 3.1 branch?


Reviewed-by: Robin Jarry 

Thanks David!

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


Bug#1064881: ITP: golang-github-soniakeys-quant -- An interface for image color quantizers.

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-soniakeys-quant
  Version : 1.0.0-1
  Upstream Author : Sonia Keys
* URL : https://github.com/soniakeys/quant
* License : Expat
  Programming Lang: Go
  Description : An interface for image color quantizers.

 Quant
 .
 Experiments with color quantizers
 .
 Implemented here are two rather simple quantizers and an (also simple)
 ditherer. The quantizers satisfy the draw.Quantizer interface of the
 standard library. The ditherer satisfies the draw.Drawer interface of
 the standard library.

We need this as an indirect dependency for golang-sourcehut-rockorager-vaxis.



Bug#1064881: ITP: golang-github-soniakeys-quant -- An interface for image color quantizers.

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-soniakeys-quant
  Version : 1.0.0-1
  Upstream Author : Sonia Keys
* URL : https://github.com/soniakeys/quant
* License : Expat
  Programming Lang: Go
  Description : An interface for image color quantizers.

 Quant
 .
 Experiments with color quantizers
 .
 Implemented here are two rather simple quantizers and an (also simple)
 ditherer. The quantizers satisfy the draw.Quantizer interface of the
 standard library. The ditherer satisfies the draw.Drawer interface of
 the standard library.

We need this as an indirect dependency for golang-sourcehut-rockorager-vaxis.



Bug#1064881: ITP: golang-github-soniakeys-quant -- An interface for image color quantizers.

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-soniakeys-quant
  Version : 1.0.0-1
  Upstream Author : Sonia Keys
* URL : https://github.com/soniakeys/quant
* License : Expat
  Programming Lang: Go
  Description : An interface for image color quantizers.

 Quant
 .
 Experiments with color quantizers
 .
 Implemented here are two rather simple quantizers and an (also simple)
 ditherer. The quantizers satisfy the draw.Quantizer interface of the
 standard library. The ditherer satisfies the draw.Drawer interface of
 the standard library.

We need this as an indirect dependency for golang-sourcehut-rockorager-vaxis.



Bug#1064881: ITP: golang-github-soniakeys-quant -- An interface for image color quantizers.

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-soniakeys-quant
  Version : 1.0.0-1
  Upstream Author : Sonia Keys
* URL : https://github.com/soniakeys/quant
* License : Expat
  Programming Lang: Go
  Description : An interface for image color quantizers.

 Quant
 .
 Experiments with color quantizers
 .
 Implemented here are two rather simple quantizers and an (also simple)
 ditherer. The quantizers satisfy the draw.Quantizer interface of the
 standard library. The ditherer satisfies the draw.Drawer interface of
 the standard library.

We need this as an indirect dependency for golang-sourcehut-rockorager-vaxis.



Bug#1064880: ITP: golang-github-mattn-go-sixel -- DRCS/Sixel Encoder/Decoder

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-mattn-go-sixel
  Version : 0.0.5-1
  Upstream Author : mattn
* URL : https://github.com/mattn/go-sixel
* License : Expat
  Programming Lang: Go
  Description : DRCS/Sixel Encoder/Decoder

 go-sixel
 .
 DRCS Sixel Encoder/Decoder
 .
 (http://go-gyazo.appspot.com/75ec3ce96dfc573e.png)
 .
 Installation
 .
   $ go get github.com/mattn/go-sixel
 .
 You can install gosr (go sixel renderer), gosd (go sixel decoder) with
 following installation instruction.
 .
   $ go get github.com/mattn/go-sixel/cmd/gosr
   $ go get github.com/mattn/go-sixel/cmd/gosd
 .
   COMMAND | DESCRIPTION
 --+---
   gosr| Image renderer
   gosd| Decoder to png
   goscat  | Render cats
   gosgif  | Render animation GIF
   gosl| Run SL
 .
 Usage
 .
 Encode
 .
   $ cat foo.png | gosr -
 .
 Decode
 .
   $ cat foo.drcs | gosd > foo.png
 .
 Use as library
 .
   img, _, _ := image.Decode(filename)
   sixel.NewEncoder(os.Stdout).Encode(img)
 .
 License
 .
 MIT
 .
 Author
 .
 Yasuhiro Matsumoto (a.k.a mattn)

We need this as a dependency for vaxis.



Bug#1064880: ITP: golang-github-mattn-go-sixel -- DRCS/Sixel Encoder/Decoder

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-mattn-go-sixel
  Version : 0.0.5-1
  Upstream Author : mattn
* URL : https://github.com/mattn/go-sixel
* License : Expat
  Programming Lang: Go
  Description : DRCS/Sixel Encoder/Decoder

 go-sixel
 .
 DRCS Sixel Encoder/Decoder
 .
 (http://go-gyazo.appspot.com/75ec3ce96dfc573e.png)
 .
 Installation
 .
   $ go get github.com/mattn/go-sixel
 .
 You can install gosr (go sixel renderer), gosd (go sixel decoder) with
 following installation instruction.
 .
   $ go get github.com/mattn/go-sixel/cmd/gosr
   $ go get github.com/mattn/go-sixel/cmd/gosd
 .
   COMMAND | DESCRIPTION
 --+---
   gosr| Image renderer
   gosd| Decoder to png
   goscat  | Render cats
   gosgif  | Render animation GIF
   gosl| Run SL
 .
 Usage
 .
 Encode
 .
   $ cat foo.png | gosr -
 .
 Decode
 .
   $ cat foo.drcs | gosd > foo.png
 .
 Use as library
 .
   img, _, _ := image.Decode(filename)
   sixel.NewEncoder(os.Stdout).Encode(img)
 .
 License
 .
 MIT
 .
 Author
 .
 Yasuhiro Matsumoto (a.k.a mattn)

We need this as a dependency for vaxis.



Bug#1064880: ITP: golang-github-mattn-go-sixel -- DRCS/Sixel Encoder/Decoder

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-mattn-go-sixel
  Version : 0.0.5-1
  Upstream Author : mattn
* URL : https://github.com/mattn/go-sixel
* License : Expat
  Programming Lang: Go
  Description : DRCS/Sixel Encoder/Decoder

 go-sixel
 .
 DRCS Sixel Encoder/Decoder
 .
 (http://go-gyazo.appspot.com/75ec3ce96dfc573e.png)
 .
 Installation
 .
   $ go get github.com/mattn/go-sixel
 .
 You can install gosr (go sixel renderer), gosd (go sixel decoder) with
 following installation instruction.
 .
   $ go get github.com/mattn/go-sixel/cmd/gosr
   $ go get github.com/mattn/go-sixel/cmd/gosd
 .
   COMMAND | DESCRIPTION
 --+---
   gosr| Image renderer
   gosd| Decoder to png
   goscat  | Render cats
   gosgif  | Render animation GIF
   gosl| Run SL
 .
 Usage
 .
 Encode
 .
   $ cat foo.png | gosr -
 .
 Decode
 .
   $ cat foo.drcs | gosd > foo.png
 .
 Use as library
 .
   img, _, _ := image.Decode(filename)
   sixel.NewEncoder(os.Stdout).Encode(img)
 .
 License
 .
 MIT
 .
 Author
 .
 Yasuhiro Matsumoto (a.k.a mattn)

We need this as a dependency for vaxis.



Bug#1064880: ITP: golang-github-mattn-go-sixel -- DRCS/Sixel Encoder/Decoder

2024-02-27 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-github-mattn-go-sixel
  Version : 0.0.5-1
  Upstream Author : mattn
* URL : https://github.com/mattn/go-sixel
* License : Expat
  Programming Lang: Go
  Description : DRCS/Sixel Encoder/Decoder

 go-sixel
 .
 DRCS Sixel Encoder/Decoder
 .
 (http://go-gyazo.appspot.com/75ec3ce96dfc573e.png)
 .
 Installation
 .
   $ go get github.com/mattn/go-sixel
 .
 You can install gosr (go sixel renderer), gosd (go sixel decoder) with
 following installation instruction.
 .
   $ go get github.com/mattn/go-sixel/cmd/gosr
   $ go get github.com/mattn/go-sixel/cmd/gosd
 .
   COMMAND | DESCRIPTION
 --+---
   gosr| Image renderer
   gosd| Decoder to png
   goscat  | Render cats
   gosgif  | Render animation GIF
   gosl| Run SL
 .
 Usage
 .
 Encode
 .
   $ cat foo.png | gosr -
 .
 Decode
 .
   $ cat foo.drcs | gosd > foo.png
 .
 Use as library
 .
   img, _, _ := image.Decode(filename)
   sixel.NewEncoder(os.Stdout).Encode(img)
 .
 License
 .
 MIT
 .
 Author
 .
 Yasuhiro Matsumoto (a.k.a mattn)

We need this as a dependency for vaxis.



Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go

2024-02-26 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-vaxis
  Version : 0.8.2-1
  Upstream Author : Tim Culverhouse <>
* URL : https://git.sr.ht/~rockorager/vaxis
* License : Apache-2.0
  Programming Lang: Go
  Description : A modern TUI library for go

Vaxis is a Terminal User Interface (TUI) library for go. Vaxis supports modern
terminal features, such as styled underlines and graphics. A widgets package is
provided with some useful widgets.

Vaxis is blazingly fast at rendering. It might not be as fast or efficient as
notcurses, but significant profiling has been done to reduce all render
bottlenecks while still maintaining the feature-set.

All input parsing is done using a real terminal parser, based on the excellent
state machine by Paul Flo Williams. Some modifications have been made to allow
for proper SGR parsing (':' separated sub-parameters)

Vaxis does not use terminfo. Support for features is detected through terminal
queries. Vaxis assumes xterm-style escape codes everywhere else.

This will be a new dependency for aerc 0.18 and onwards.



Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go

2024-02-26 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-vaxis
  Version : 0.8.2-1
  Upstream Author : Tim Culverhouse <>
* URL : https://git.sr.ht/~rockorager/vaxis
* License : Apache-2.0
  Programming Lang: Go
  Description : A modern TUI library for go

Vaxis is a Terminal User Interface (TUI) library for go. Vaxis supports modern
terminal features, such as styled underlines and graphics. A widgets package is
provided with some useful widgets.

Vaxis is blazingly fast at rendering. It might not be as fast or efficient as
notcurses, but significant profiling has been done to reduce all render
bottlenecks while still maintaining the feature-set.

All input parsing is done using a real terminal parser, based on the excellent
state machine by Paul Flo Williams. Some modifications have been made to allow
for proper SGR parsing (':' separated sub-parameters)

Vaxis does not use terminfo. Support for features is detected through terminal
queries. Vaxis assumes xterm-style escape codes everywhere else.

This will be a new dependency for aerc 0.18 and onwards.



Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go

2024-02-26 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-vaxis
  Version : 0.8.2-1
  Upstream Author : Tim Culverhouse <>
* URL : https://git.sr.ht/~rockorager/vaxis
* License : Apache-2.0
  Programming Lang: Go
  Description : A modern TUI library for go

Vaxis is a Terminal User Interface (TUI) library for go. Vaxis supports modern
terminal features, such as styled underlines and graphics. A widgets package is
provided with some useful widgets.

Vaxis is blazingly fast at rendering. It might not be as fast or efficient as
notcurses, but significant profiling has been done to reduce all render
bottlenecks while still maintaining the feature-set.

All input parsing is done using a real terminal parser, based on the excellent
state machine by Paul Flo Williams. Some modifications have been made to allow
for proper SGR parsing (':' separated sub-parameters)

Vaxis does not use terminfo. Support for features is detected through terminal
queries. Vaxis assumes xterm-style escape codes everywhere else.

This will be a new dependency for aerc 0.18 and onwards.



Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go

2024-02-26 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-vaxis
  Version : 0.8.2-1
  Upstream Author : Tim Culverhouse <>
* URL : https://git.sr.ht/~rockorager/vaxis
* License : Apache-2.0
  Programming Lang: Go
  Description : A modern TUI library for go

Vaxis is a Terminal User Interface (TUI) library for go. Vaxis supports modern
terminal features, such as styled underlines and graphics. A widgets package is
provided with some useful widgets.

Vaxis is blazingly fast at rendering. It might not be as fast or efficient as
notcurses, but significant profiling has been done to reduce all render
bottlenecks while still maintaining the feature-set.

All input parsing is done using a real terminal parser, based on the excellent
state machine by Paul Flo Williams. Some modifications have been made to allow
for proper SGR parsing (':' separated sub-parameters)

Vaxis does not use terminfo. Support for features is detected through terminal
queries. Vaxis assumes xterm-style escape codes everywhere else.

This will be a new dependency for aerc 0.18 and onwards.



Bug#1061857: Recommends: is too strong for dante-client

2024-02-01 Thread Robin Jarry

Nilesh Patra, Feb 01, 2024 at 10:07:

From what I gather, filters/html still uses socksify from dante package.
This looks OK as recommends - given that in 2024, people do get a bunch of HTML 
mails.

I want to close this bug if you agree.


We *could* drop it and change the default html filter to only use w3m 
without socksify.


Actually, aerc already ships an html-unsafe filter that does exactly 
that:


https://git.sr.ht/~rjarry/aerc/tree/master/item/filters/html-unsafe

The name may be frightening, I guess we could remove the hard dependency 
to socksify. People with high concerns for privacy can always tweak it 
to their needs.


Thoughts?



Bug#1057929: marked as pending in buildbot

2023-12-30 Thread Robin Jarry
Control: tag -1 pending

Hello,

Bug #1057929 in buildbot reported by you has been fixed in the
Git repository and is awaiting an upload. You can see the commit
message below and you can check the diff of the fix at:

https://salsa.debian.org/python-team/packages/buildbot/-/commit/c340fc5a17db85a06a15b345cbf6b9dfe14bb63e


d/patches: force C.UTF-8 locale when building docs

Closes: #1057929
Signed-off-by: Robin Jarry 


(this message was generated automatically)
-- 
Greetings

https://bugs.debian.org/1057929



Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-12-07 Thread Robin Jarry

Nilesh Patra, Dec 06, 2023 at 16:44:

My understanding is that we can maybe ignore the CI here.


Hey Nilesh,

thanks for looking at this. Unless you see something wrong I think it is 
good for another attempt at NEW.


Cheers!



Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-12-05 Thread Robin Jarry

Nilesh Patra, Dec 05, 2023 at 20:44:
You should have the permissions now for this repo. LMK if there's 
still an issue.


Thanks I managed to push everything needed. However, the CI fails for 
some obscure reason now:


https://salsa.debian.org/go-team/packages/golang-sourcehut-rockorager-go-jmap/-/jobs/5005483

 $ GBP_CONF_FILES=:debian/gbp.conf gbp buildpackage 
--git-no-pristine-tar --git-ignore-branch --git-ignore-new 
--git-export-dir=/tmp/export --git-no-overlay 
--git-tarball-dir=/nonexistent --git-cleaner=/bin/true 
--git-builder='dpkg-buildpackage -S -d --no-sign'
 gbp:info: Tarballs 
 'golang-sourcehut-rockorager-go-jmap_0.4.4.orig.tar.gz' not found at 
 '/nonexistent'

 gbp:error: upstream is not a valid branch

Do you have a clue? It seems related to my change in debian/gbp.conf

https://salsa.debian.org/go-team/packages/golang-sourcehut-rockorager-go-jmap/-/commit/b5e8586dbced8af76352917f9f2d1d4da0641b28

But I am not sure at all. I wanted to avoid importing the upstream 
source as a git archive of the tag and have gbp import-orig 
automatically pull the upstream source and rebase the upstream branch 
accordingly.


Maybe I don't understand how to deal with this...



Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-12-05 Thread Robin Jarry

Hi Nilesh,

I finally get time to work on this. I tried updating the repo but 
somehow I messed up the upstream branch (updated it with gbp import-orig 
--uscan) last time.


https://salsa.debian.org/go-team/packages/golang-sourcehut-rockorager-go-jmap/

I'd like to follow the standard go team workflow which is to preserve 
all history in the upstream branch.


https://go-team.pages.debian.net/workflow-changes.html

I tried renaming the branch to old/upstream and pushing a new one but 
I don't have the permissions to do so:


remote: GitLab: You are not allowed to delete protected branches from 
this project. Only a project maintainer or owner can delete 
a protected branch.


Can you help?

Thanks!



Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-11-22 Thread Robin Jarry

Hi Nilesh,

Nilesh Patra, Nov 22, 2023 at 21:03:
I am pinging here to ask if you have a clue as to what could be done 
about it.


As this is blocking aerc update, do you find it possible to use 
another golang library in place of rockorager-go-jmap that may offer 
similar functionality?


Sorry, I got sidetracked on the matter. I have worked to remove the RFC 
text quotes from all the project:


https://git.sr.ht/~rockorager/go-jmap/commit/a79dd619191ffcd97dab0d5be21c98ae70ff8a25
https://git.sr.ht/~rockorager/go-jmap/commit/bc07e105979181f8ec1b744982919e0e4d9d606c

There was a new tagged release but I haven't taken the time to integrate 
it into the debian package.


I'll try to do this before the end of the week.

Thanks for caring :)



Re: [ovs-dev] [PATCH v7 3/3] netdev-dpdk: Sync and clean {get, set}_config() callbacks.

2023-10-30 Thread Robin Jarry

, Oct 30, 2023 at 10:50:

From: Jakob Meng 

For better usability, the function pairs get_config() and
set_config() for netdevs should be symmetric: Options which are
accepted by set_config() should be returned by get_config() and the
latter should output valid options for set_config() only.

This patch moves key-value pairs which are no valid options from
get_config() to the get_status() callback. For example, get_config()
in lib/netdev-dpdk.c returned {configured,requested}_{rx,tx}_queues
previously. For requested rx queues the proper option name is n_rxq,
so requested_rx_queues has been renamed respectively. Tx queues
cannot be changed by the user, hence requested_tx_queues has been
dropped. Both configured_{rx,tx}_queues will be returned as
n_{r,t}xq in the get_status() callback.

The netdev dpdk classes no longer share a common get_config() callback,
instead both the dpdk_class and the dpdk_vhost_client_class define
their own callbacks. The get_config() callback for dpdk_vhost_class has
been dropped because it does not have a set_config() callback.

The documentation in vswitchd/vswitch.xml for status columns as well
as tests have been updated accordingly.

Reported-at: https://bugzilla.redhat.com/1949855
Signed-off-by: Jakob Meng 


Hi Jakob, this looks good. Thanks!

Reviewed-by: Robin Jarry 

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


Re: [ovs-dev] [PATCH v3] editorconfig: Remove [*] section and trim_trailing_whitespace.

2023-10-27 Thread Robin Jarry

, Oct 27, 2023 at 14:10:

From: Jakob Meng 

Wildcard sections [*] and [**] are unsafe because properties cannot be
applied safely to any filetype in general. For example, IDEs like
Visual Studio Code and KDevelop store configuration files in subfolders
like .vscode or .kdev4. Properties from wildcard sections also apply to
those files which is not safe in general.
Another example are patches created with 'git format-patch' which can
contain trailing whitespaces. When editing a patch, e.g. to fix a typo
in the title, trailing whitespaces should not be removed.

Property trim_trailing_whitespace should not be defined at all because
it is interpreted differently by editors. Some wipe whitespaces from
the whole file, others remove them from edited lines only and a few
change their behavior between releases [0]. Limiting the property to a
subset of files like *.c/*.h will not mitigate the issue:

Multiple definitions of a whitespace exist. Unicode considers a form
feed (0x0C) to be a whitespace [1]. QChar::isSpace() [2] from Qt
follows this definition, causing the Kate editor identify a form feed
as a trailing whitespace and removing it from sources [3]. This breaks
patches when editors remove form feeds and thus causing broken patches
which cannot be applied cleanly.

Removing trim_trailing_whitespace will be a minor inconvienence, in
particular because utilities/checkpatch.py and thus 0-day Robot will
prevent trailing whitespaces for our definition of a whitespace.

[0] 
https://github.com/KDE/ktexteditor/commit/94b328fc64e543d91930845d2a96ce08d3043295
[1] https://en.wikipedia.org/wiki/Whitespace_character
[2] 
https://github.com/qt/qtbase/blob/5628600a07295db6ed6683e97fafb0c45ddea505/src/corelib/text/qchar.h#L554
[3] 
https://github.com/KDE/ktexteditor/blob/10210ec1dd06afa1e7b19a4fff722a8a23719161/src/document/katedocument.cpp#L5643

Fixes: 07f6d6a0cb51 ("Add editorconfig file.")

Signed-off-by: Jakob Meng 
---
 .editorconfig | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/.editorconfig b/.editorconfig
index 685c72750..41ba51bf3 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -2,15 +2,18 @@
 
 root = true
 
-[*]

-end_of_line = lf
-insert_final_newline = true
-trim_trailing_whitespace = true
-charset = utf-8
+# No wildcard sections [*] and [**] because properties cannot be
+# applied safely to any filetype in general.
+
+# Property trim_trailing_whitespace should not be defined at all
+# because it is interpreted differently by editors.
 
 [*.{c,h}]

+charset = utf-8
+end_of_line = lf
 indent_style = space
 indent_size = 4
+insert_final_newline = true
 max_line_length = 79
 
 [include/linux/**.h]

--
2.39.2


Perfect, thanks Jakob!

Reviewed-by: Robin-Jarry 

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


Re: [ovs-dev] [PATCH v2] editorconfig: Remove [*] section and trim_trailing_whitespace.

2023-10-26 Thread Robin Jarry

Jakob Meng, Oct 26, 2023 at 14:17:

On 26.10.23 13:52, Robin Jarry wrote:
> , Oct 26, 2023 at 13:07:
>> From: Jakob Meng 
>>
>> Wildcard sections [*] and [**] are unsafe because properties cannot be
>> applied safely to any filetype in general. For example, IDEs like
>> Visual Studio Code and KDevelop store configuration files in subfolders
>> like .vscode or .kdev4. Properties from wildcard sections also apply to
>> those files which is not safe in general.
>> Another example are patches created with 'git format-patch' which can
>> contain trailing whitespaces. When editing a patch, e.g. to fix a typo
>> in the title, trailing whitespaces should not be removed.
>>
>> Property trim_trailing_whitespace should not be defined at all because
>> it is interpreted differently by editors. Some wipe whitespaces from
>> the whole file, others remove them from edited lines only and a few
>> change their behavior between releases [0]. Limiting the property to a
>> subset of files like *.c/*.h will not mitigate the issue:
>>
>> Multiple definitions of a whitespace exist. Unicode considers a form
>> feed (0x0C) to be a whitespace [1]. QChar::isSpace() [2] from Qt
>> follows this definition, causing the Kate editor identify a form feed
>> as a trailing whitespace and removing it from sources [3]. This breaks
>> patches when editors remove form feeds and thus causing broken patches
>> which cannot be applied cleanly.
>>
>> Removing trim_trailing_whitespace will be a minor inconvienence, in
>> particular because utilities/checkpatch.py and thus 0-day Robot will
>> prevent trailing whitespaces for our definition of a whitespace.
>>
>> [0] 
https://github.com/KDE/ktexteditor/commit/94b328fc64e543d91930845d2a96ce08d3043295
>> [1] https://en.wikipedia.org/wiki/Whitespace_character
>> [2] 
https://github.com/qt/qtbase/blob/5628600a07295db6ed6683e97fafb0c45ddea505/src/corelib/text/qchar.h#L554
>> [3] 
https://github.com/KDE/ktexteditor/blob/10210ec1dd06afa1e7b19a4fff722a8a23719161/src/document/katedocument.cpp#L5643
>>
>> Fixes: 07f6d6a0cb51 ("Add editorconfig file.")
>>
>> Signed-off-by: Jakob Meng 
>> ---
>>  .editorconfig | 34 +-
>>  1 file changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/.editorconfig b/.editorconfig
>> index 685c72750..aebcf3a72 100644
>> --- a/.editorconfig
>> +++ b/.editorconfig
>> @@ -2,47 +2,71 @@
>>  
>>  root = true
>>  
>> -[*]
>> -end_of_line = lf
>> -insert_final_newline = true
>> -trim_trailing_whitespace = true
>> -charset = utf-8
>
> Hi Jakob,
>
> I think you could keep these two options:
>
> end_of_line = lf
> charset = utf-8
>

You cannot decide this in general for all possible filetypes across 
all possible dev platforms. Again, those properties in [*] would also 
apply to non-ovs-owned files inside the source tree, e.g. created by 
IDEs. Unsafe. Please don't.


Ok fair enough.

> And probably adding insert_final_newline = true is not necessary. 
> checkpatch should complain if the final newline is missing.


I left it in .editorconfig because it is not causing trouble. But 
I can remove it, if you want.


I don't think it is important you can leave it or remove it.

However I just realized that you could simply copy the settings in the 
[*.{c,h}] section as other sections will inherit from it unless they 
override something.


> Your patch should only remove insert_final_newline and 
> trim_trailing_whitespace from the default section.



>
>> +# No wildcard sections [*] and [**] because properties cannot be
>> +# applied safely to any filetype in general.
>> +
>> +# Property trim_trailing_whitespace should not be defined at all
>> +# because it is interpreted differently by editors.
>>  
>>  [*.{c,h}]
>> +charset = utf-8
>> +end_of_line = lf
>>  indent_style = space
>>  indent_size = 4
>> +insert_final_newline = true
>>  max_line_length = 79
>>  
>>  [include/linux/**.h]
>> +charset = utf-8
>> +end_of_line = lf
>>  indent_style = tab
>>  indent_size = tab
>> +insert_final_newline = true
>>  tab_width = 8
>>  
>>  [include/sparse/rte_*.h]
>> +charset = utf-8
>> +end_of_line = lf
>>  indent_style = tab
>> +insert_final_newline = true
>>  tab_width = 8
>>  
>>  [include/windows/getopt.h]
>> +charset = utf-8
>> +end_of_line = lf
>>  indent_style = tab
>>  indent_size = tab
>> +insert_final_newline = true
>>  tab_width = 8
>>  
>>  [include/windows

Re: [ovs-dev] [PATCH v2] editorconfig: Remove [*] section and trim_trailing_whitespace.

2023-10-26 Thread Robin Jarry

, Oct 26, 2023 at 13:07:

From: Jakob Meng 

Wildcard sections [*] and [**] are unsafe because properties cannot be
applied safely to any filetype in general. For example, IDEs like
Visual Studio Code and KDevelop store configuration files in subfolders
like .vscode or .kdev4. Properties from wildcard sections also apply to
those files which is not safe in general.
Another example are patches created with 'git format-patch' which can
contain trailing whitespaces. When editing a patch, e.g. to fix a typo
in the title, trailing whitespaces should not be removed.

Property trim_trailing_whitespace should not be defined at all because
it is interpreted differently by editors. Some wipe whitespaces from
the whole file, others remove them from edited lines only and a few
change their behavior between releases [0]. Limiting the property to a
subset of files like *.c/*.h will not mitigate the issue:

Multiple definitions of a whitespace exist. Unicode considers a form
feed (0x0C) to be a whitespace [1]. QChar::isSpace() [2] from Qt
follows this definition, causing the Kate editor identify a form feed
as a trailing whitespace and removing it from sources [3]. This breaks
patches when editors remove form feeds and thus causing broken patches
which cannot be applied cleanly.

Removing trim_trailing_whitespace will be a minor inconvienence, in
particular because utilities/checkpatch.py and thus 0-day Robot will
prevent trailing whitespaces for our definition of a whitespace.

[0] 
https://github.com/KDE/ktexteditor/commit/94b328fc64e543d91930845d2a96ce08d3043295
[1] https://en.wikipedia.org/wiki/Whitespace_character
[2] 
https://github.com/qt/qtbase/blob/5628600a07295db6ed6683e97fafb0c45ddea505/src/corelib/text/qchar.h#L554
[3] 
https://github.com/KDE/ktexteditor/blob/10210ec1dd06afa1e7b19a4fff722a8a23719161/src/document/katedocument.cpp#L5643

Fixes: 07f6d6a0cb51 ("Add editorconfig file.")

Signed-off-by: Jakob Meng 
---
 .editorconfig | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/.editorconfig b/.editorconfig
index 685c72750..aebcf3a72 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -2,47 +2,71 @@
 
 root = true
 
-[*]

-end_of_line = lf
-insert_final_newline = true
-trim_trailing_whitespace = true
-charset = utf-8


Hi Jakob,

I think you could keep these two options:

end_of_line = lf
charset = utf-8

And probably adding insert_final_newline = true is not necessary. 
checkpatch should complain if the final newline is missing.


Your patch should only remove insert_final_newline and 
trim_trailing_whitespace from the default section.


What do you think?


+# No wildcard sections [*] and [**] because properties cannot be
+# applied safely to any filetype in general.
+
+# Property trim_trailing_whitespace should not be defined at all
+# because it is interpreted differently by editors.
 
 [*.{c,h}]

+charset = utf-8
+end_of_line = lf
 indent_style = space
 indent_size = 4
+insert_final_newline = true
 max_line_length = 79
 
 [include/linux/**.h]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
 indent_size = tab
+insert_final_newline = true
 tab_width = 8
 
 [include/sparse/rte_*.h]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
+insert_final_newline = true
 tab_width = 8
 
 [include/windows/getopt.h]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
 indent_size = tab
+insert_final_newline = true
 tab_width = 8
 
 [include/windows/netinet/{icmp6,ip6}.h]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
 indent_size = tab
+insert_final_newline = true
 tab_width = 8
 
 [lib/getopt_long.c]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
 indent_size = tab
+insert_final_newline = true
 tab_width = 8
 
 [lib/sflow*.{c,h}]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
 indent_size = tab
+insert_final_newline = true
 tab_width = 8
 
 [lib/strsep.c]

+charset = utf-8
+end_of_line = lf
 indent_style = tab
 indent_size = tab
+insert_final_newline = true
 tab_width = 8
--
2.39.2




--
Robin Jarry
Principal Software Engineer
Red Hat, Telco/NFV

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


Re: [ovs-dev] [PATCH] editorconfig: Leave *.patch and *.diff files untouched.

2023-10-25 Thread Robin Jarry

Eelco Chaudron, Oct 25, 2023 at 14:56:

On 25 Oct 2023, at 13:52, jm...@redhat.com wrote:

> From: Jakob Meng 
>
> A patch created with 'git format-patch' can contain trailing spaces.
> When editing a patch, e.g. to fix a typo in the title, the trailing
> spaces should not be removed. This becomes tricky when editors like
> Kate identify a space followed by form feed as a trailing space and
> remove both. This will result in a broken patch which cannot be applied
> cleanly. Although this case should likely be considered a editor bug,
> keeping trailing spaces in a patch is the right thing to do and also
> helps mitigating these kinds of editor bugs.
>
> Fixes: 07f6d6a0cb51 ("Add editorconfig file.")

This looks good to me, however as you mention this is more of an 
editor configuration issue. It looks like leaving out any default 
value will cause the editor to use its configured value.


Acked-by: Eelco Chaudron 


It seems OK as well. But another safer option would have been to move 
the trim_trailing_whitespace = true option in specific sections. Or 
completely remove it since it will be caught by checkpatch.


What do you think?

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


Re: [ovs-dev] [PATCH v6 1/3] netdev-dpdk: Sync and clean {get, set}_config() callbacks.

2023-10-13 Thread Robin Jarry

, Oct 13, 2023 at 11:07:

From: Jakob Meng 

For better usability, the function pairs get_config() and
set_config() for netdevs should be symmetric: Options which are
accepted by set_config() should be returned by get_config() and the
latter should output valid options for set_config() only.

This patch moves key-value pairs which are no valid options from
get_config() to the get_status() callback. For example, get_config()
in lib/netdev-dpdk.c returned {configured,requested}_{rx,tx}_queues
previously. For requested rx queues the proper option name is n_rxq,
so requested_rx_queues has been renamed respectively. Tx queues
cannot be changed by the user, hence requested_tx_queues has been
dropped. Both configured_{rx,tx}_queues will be returned as
n_{r,t}xq in the get_status() callback.

The documentation in vswitchd/vswitch.xml for status columns as well
as tests have been updated accordingly.

Reported-at: https://bugzilla.redhat.com/1949855
Signed-off-by: Jakob Meng 


Hi Jakob, this looks good, thanks!

Reviewed-by: Robin Jarry 

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


Re: [ovs-dev] [PATCH v5] netdev: Sync'ed and cleaned {get, set}_config().

2023-10-12 Thread Robin Jarry

Kevin Traynor, Oct 12, 2023 at 17:34:
ok, 'rss' is documented as default, so maybe we don't need to display if 
it is in use by default, selected by user or as fallback.


That would make things a bit easier as 'rx-steering:' is free to use to 
indicate the unsupported case.


So maybe status could just have:
'rx-steering:rss+lacp' for user enabled rss+lacp and NIC supports
'rx-steering:unsupported' for user enabled rss+lacp and NIC does not 
support.


If rx-steering is not reported, then it is using the default 'rss'.

Robin, what do you think ?


It may be surprising to users not to see any mention in get_config() 
when explicitly setting rx-steering=rss but I don't see that as 
a common/standard use case. So I think it should be OK.


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


Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-10-06 Thread Robin Jarry

Nilesh Patra, Oct 05, 2023 at 23:52:

Uploaded to NEW -- I hope the copyright checks would be green!


Thanks \o/



Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-10-05 Thread Robin Jarry

Nilesh Patra, Oct 02, 2023 at 00:08:

When a file has multiple licenses, you can mention the license as
"License: Expat and RFC-Reference" and add a
comment section stating the doctrings are licensed under RFC license.

I took a hint for how it's done in gnupg2 package[1]. You could do something
similar here for the description of the RFC, hopefully this should work.

I personally don't care too much about this being mentioned or not, but FTP
master would likely reject the package if copyright is not perfect so here
we are.

[1]: 
https://salsa.debian.org/debian/gnupg2/-/blob/debian/unstable/debian/copyright#L123


Hi Nilesh,

I have updated d/copyright and bumped the upstream version as well. It 
should be good.


Let me know if I need to change anything.

Thanks!



Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - issues

2023-10-04 Thread Robin Jarry via discuss

Hi Mark,

Mark Michelson, Oct 03, 2023 at 23:09:

Hi Robin,

Thanks a bunch for putting these two emails together. I've read through 
them and the replies.


I think there's one major issue: a lack of data.


That's my concern as well... The problem is, it is very hard to get 
reliable and actionable data when it comes to that level of scale. 
I have been trying to collect such data and put together realistic 
scenarios but failed until now.


I think the four bullet points you listed below are admirable goals. The 
problem is that I think we're putting the cart before the horse with 
both the issues and proposals.


In other words, before being able to properly evaluate these emails, we 
need to see a scenario that

1) Has clear goals for what scalability metrics are desired.
2) Shows evidence that these scalability goals are not being met.
3) Shows evidence that one or more of the issues listed in this email 
are the cause for the scalability issues in the scenario.
4) Shows evidence that the proposed changes would fix the scalability 
issues in the scenario.


I hope that the ongoing work on ovn-heater will help in that regard.

I listed them in this order because without a failing scenario, we can't 
claim the scalability is poor. Then if we have a failing scenario, it's 
possible that the problem and solution is much simpler than any of the 
issues or proposals that have been brought up here. Then, it's also 
possible that maybe only a subset of the issues listed in this email are 
contributing to the failure. Even if the issues identified here are 
directly causing the scenario to fail, there may still be simpler 
solutions than what has been proposed. And finally, it's possible that 
the proposed solutions don't actually result in the expected scale increase.


I want to make sure my tone is coming across clearly here. I don't think 
the current OVN architecture is perfect, and I don't want to be 
dismissive of the issues you've raised. If there are changes we can make 
to simplify OVN and scale better at the same time, I'm all for it. The 
problem is that, as you pointed out in your proposal email, most of 
these proposals result in difficulties for upgrades/downgrades, as well 
as code maintenance. Therefore, if we are going to do any of these, we 
need to first be certain that we aren't scaling as well as we would 
like, and that there are not simpler paths to reach our scalability targets.


I get your point and this is specifically why I did split the 
conversation in two. I did not want my proposals to be mixed up with 
the issues.


I will see if I can get hard data that can demonstrate what I claim.

Thanks!

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-10-04 Thread Robin Jarry via discuss
Hi Felix,

Felix Huettner, Oct 04, 2023 at 09:24:
> Hi Robin,
>
> i'll try to answer what i can.
>
> On Tue, Oct 03, 2023 at 09:22:53AM +0200, Robin Jarry via discuss wrote:
> > Hi all,
> >
> > Felix Huettner, Oct 02, 2023 at 09:35:
> > > Hi everyone,
> > >
> > > just want to add my experience below
> > > On Sun, Oct 01, 2023 at 03:49:31PM -0700, Han Zhou wrote:
> > > > On Sun, Oct 1, 2023 at 12:34 PM Robin Jarry  wrote:
> > > > >
> > > > > Hi Han,
> > > > >
> > > > > Please see my comments/questions inline.
> > > > >
> > > > > Han Zhou, Sep 30, 2023 at 21:59:
> > > > > > > Distributed mac learning
> > > > > > > 
> > > > > > >
> > > > > > > Use one OVS bridge per logical switch with mac learning
> > > > > > > enabled. Only create the bridge if the logical switch has
> > > > > > > a port bound to the local chassis.
> > > > > > >
> > > > > > > Pros:
> > > > > > >
> > > > > > > - Minimal openflow rules required in each bridge (ACLs and NAT
> > > > > > >   mostly).
> > > > > > > - No central mac binding table required.
> > > > > >
> > > > > > Firstly to clarify the terminology of "mac binding" to avoid
> > > > > > confusion, the mac_binding table currently in SB DB has nothing
> > > > > > to do with L2 MAC learning. It is actually the ARP/Neighbor
> > > > > > table of distributed logical routers. We should probably call it
> > > > > > IP_MAC_binding table, or just Neighbor table.
> > > > >
> > > > > Yes sorry about the confusion. I actually meant the FDB table.
> > > > >
> > > > > > Here what you mean is actually L2 MAC learning, which today is
> > > > > > implemented by the FDB table in SB DB, and it is only for
> > > > > > uncommon use cases when the NB doesn't have the knowledge of
> > > > > > a MAC address of a VIF.
> > > > >
> > > > > This is not that uncommon in telco use cases where VNFs can send
> > > > > packets from mac addresses unknown to OVN.
> > > > >
> > > > Understand, but VNFs contributes a very small portion of the
> > > > workloads, right? Maybe I should rephrase that: it is uncommon to
> > > > have "unknown" addresses for the majority of ports in a large scale
> > > > cloud. Is this understanding correct?
> > >
> > > I can only share numbers for our usecase with ~650 chassis we have the
> > > following distribution of "unknown" in the `addresses` field of
> > > Logical_Switch_Port:
> > > * 23000 with a mac address + ip and without "unknown"
> > > * 250 with a mac address + ip and with "unknown"
> > > * 30 with just "unknown"
> > >
> > > The usecase is a generic public cloud and we do not have any telco
> > > related things.
> >
> > I don't have any numbers from telco deployments at hand but I will poke
> > around.
> >
> > > > > > The purpose of this proposal is clear - to avoid using a central
> > > > > > table in DB for L2 information but instead using L2 MAC learning
> > > > > > to populate such information on chassis, which is a reasonable
> > > > > > alternative with pros and cons.
> > > > > > However, I don't think it is necessary to use separate OVS
> > > > > > bridges for this purpose. L2 MAC learning can be easily
> > > > > > implemented in the br-int bridge with OVS flows, which is much
> > > > > > simpler than managing dynamic number of OVS bridges just for the
> > > > > > purpose of using the builtin OVS mac-learning.
> > > > >
> > > > > I agree that this could also be implemented with VLAN tags on the
> > > > > appropriate ports. But since OVS does not support trunk ports, it
> > > > > may require complicated OF pipelines. My intent with this idea was
> > > > > two fold:
> > > > >
> > > > > 1) Avoid a central point of failure for mac learning/aging.
> > > > > 2) Simplify the OF pipeline by making all FDB operations dynamic.
> > > >
> > > > IM

Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-10-03 Thread Robin Jarry via discuss
Frode Nordahl, Oct 02, 2023 at 10:05:
> > I must admit I don't have enough field experience operating large
> > network fabrics to state what issues multicast can cause with these.
> > This is why I raised this in the cons list :)
> >
> > What specific issues did you have in mind?
>
> It has been a while since I was overseeing the operations of large
> metro networks, but I have vivid memories of multicast routing being a
> recurring issue. A fabric supporting 10k computes would most likely
> not be one large L2, there would be L3 routing involved and as a
> consequence your proposal imposes configuration and scale requirements
> on the fabric.
>
> Datapoints that suggest other people see this as an issue too can be
> found in the fact that popular top of rack vendors have chosen control
> plane based MAC learning for their EVPN implementations (RFC 7432).
> There are also multiple papers discussing the scaling issues of
> Multicast.

Thanks, I will try to educate myself better about this :)

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-10-03 Thread Robin Jarry via discuss
Hi all,

Felix Huettner, Oct 02, 2023 at 09:35:
> Hi everyone,
>
> just want to add my experience below
> On Sun, Oct 01, 2023 at 03:49:31PM -0700, Han Zhou wrote:
> > On Sun, Oct 1, 2023 at 12:34 PM Robin Jarry  wrote:
> > >
> > > Hi Han,
> > >
> > > Please see my comments/questions inline.
> > >
> > > Han Zhou, Sep 30, 2023 at 21:59:
> > > > > Distributed mac learning
> > > > > 
> > > > >
> > > > > Use one OVS bridge per logical switch with mac learning
> > > > > enabled. Only create the bridge if the logical switch has
> > > > > a port bound to the local chassis.
> > > > >
> > > > > Pros:
> > > > >
> > > > > - Minimal openflow rules required in each bridge (ACLs and NAT
> > > > >   mostly).
> > > > > - No central mac binding table required.
> > > >
> > > > Firstly to clarify the terminology of "mac binding" to avoid
> > > > confusion, the mac_binding table currently in SB DB has nothing
> > > > to do with L2 MAC learning. It is actually the ARP/Neighbor
> > > > table of distributed logical routers. We should probably call it
> > > > IP_MAC_binding table, or just Neighbor table.
> > >
> > > Yes sorry about the confusion. I actually meant the FDB table.
> > >
> > > > Here what you mean is actually L2 MAC learning, which today is
> > > > implemented by the FDB table in SB DB, and it is only for
> > > > uncommon use cases when the NB doesn't have the knowledge of
> > > > a MAC address of a VIF.
> > >
> > > This is not that uncommon in telco use cases where VNFs can send
> > > packets from mac addresses unknown to OVN.
> > >
> > Understand, but VNFs contributes a very small portion of the
> > workloads, right? Maybe I should rephrase that: it is uncommon to
> > have "unknown" addresses for the majority of ports in a large scale
> > cloud. Is this understanding correct?
>
> I can only share numbers for our usecase with ~650 chassis we have the
> following distribution of "unknown" in the `addresses` field of
> Logical_Switch_Port:
> * 23000 with a mac address + ip and without "unknown"
> * 250 with a mac address + ip and with "unknown"
> * 30 with just "unknown"
>
> The usecase is a generic public cloud and we do not have any telco
> related things.

I don't have any numbers from telco deployments at hand but I will poke
around.

> > > > The purpose of this proposal is clear - to avoid using a central
> > > > table in DB for L2 information but instead using L2 MAC learning
> > > > to populate such information on chassis, which is a reasonable
> > > > alternative with pros and cons.
> > > > However, I don't think it is necessary to use separate OVS
> > > > bridges for this purpose. L2 MAC learning can be easily
> > > > implemented in the br-int bridge with OVS flows, which is much
> > > > simpler than managing dynamic number of OVS bridges just for the
> > > > purpose of using the builtin OVS mac-learning.
> > >
> > > I agree that this could also be implemented with VLAN tags on the
> > > appropriate ports. But since OVS does not support trunk ports, it
> > > may require complicated OF pipelines. My intent with this idea was
> > > two fold:
> > >
> > > 1) Avoid a central point of failure for mac learning/aging.
> > > 2) Simplify the OF pipeline by making all FDB operations dynamic.
> >
> > IMHO, the L2 pipeline is not really complex. It is probably the
> > simplest part (compared with other features for L3, NAT, ACL, LB,
> > etc.). Adding dynamic learning to this part probably makes it *a
> > little* more complex, but should still be straightforward. We don't
> > need any VLAN tag because the incoming packet has geneve VNI in the
> > metadata. We just need a flow that resubmits to lookup
> > a MAC-tunnelSrc mapping table, and inject a new flow (with related
> > tunnel endpont information) if the src MAC is not found, with the
> > help of the "learn" action. The entries are per-logical_switch
> > (VNI). This would serve your purpose of avoiding a central DB for
> > L2. At least this looks much simpler to me than managing dynamic
> > number of OVS bridges and the patch pairs between them.

Would that work for non GENEVE networks (localnet) when there is no VNI?
Does that apply as well?

Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - issues

2023-10-02 Thread Robin Jarry via discuss
Han Zhou, Oct 01, 2023 at 21:30:
> Please note that tunnels are needed not only between nodes related to same
> logical switches, but also when they are related to different logical
> switches connected by logical routers (even multiple LR+LS hops away).

Yep.

> To clarify a little more, openstack deployment can have different logical
> topologies. So to evaluate the impact of monitor_all settings there should
> be different test cases to capture different types of deployment, e.g.
> full-mesh topology (monitor_all=true is better) v.s. "small islands"
> toplogy (monitor_all=false is reasonable).

This is one thing to note for the recent ovn-heater work that adds
openstack test cases.

> FDB and MAC_binding tables are used by ovn-controllers. They are
> essentially the central storage for MAC tables of the distributed logical
> switches (FDB) and ARP/Neighbour tables for distributed logical routers
> (MAC_binding). A record can be populate by one chassis and consumed by many
> other chassis.
>
> monitor_all should work the same way for these tables: if monitor_all =
> false, only rows related to "local datapaths" should be downloaded to the
> chassis. However, for FDB table, the condition is not set for now (which
> may have been a miss in the initial implementation). Perhaps this is not
> noticed because MAC learning is not a very widely used feature and no scale
> impact noticed, but I just proposed a patch to enable the conditional
> monitoring:
> https://patchwork.ozlabs.org/project/ovn/patch/20231001192658.1012806-1-hz...@ovn.org/

Ok thanks!

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


Re: Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-10-01 Thread Robin Jarry
Nilesh Patra, Oct 01, 2023 at 12:28:
> The jmap.go file header says:
>
> """
> // Package jmap implements JMAP Core protocol as defined in
> // draft-ietf-jmap-core-17 (published March 2019).
> //
> // Documentation strings for most of the protocol objects are taken from (or
> // based on) contents of draft-ietf-jmap-core-17 and is subject to the IETF
> // Trust Provisions.
> // See https://trustee.ietf.org/trust-legal-provisions.html for details.
> // See included draft-ietf-jmap-core-17.txt for related copyright notices.
> """
>
> Seems like this has copyright of one more license. (There's no
> jmap-core-17 txt file in the package btw). Can you update d/copyright
> please?
>
> Best,
> Nilesh

This comment was outdated. I sent a patch to fix it upstream. However,
I am not sure how to declare the copyright.

Only the documentation (comments) are inspired or copied from the stated
RFCs.

This page states that any IETF document is distributed under these
terms (very long text):

https://trustee.ietf.org/documents/trust-legal-provisions/tlp-5/

It says that the "code components" (examples) are published under the
BSD revised (3-Clauses) license. I don't think any code excerpt was
taken from these RFCs, though.

I can add this line in d/copyright:

  2019 IETF Trust and the persons identified as authors of the code.

But since the upstream project is licensed with MIT, I'm not sure if
this is correct.

Thoughts?



Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-10-01 Thread Robin Jarry via discuss
Hi Han,

Please see my comments/questions inline.

Han Zhou, Sep 30, 2023 at 21:59:
> > Distributed mac learning
> > 
> >
> > Use one OVS bridge per logical switch with mac learning enabled. Only
> > create the bridge if the logical switch has a port bound to the local
> > chassis.
> >
> > Pros:
> >
> > - Minimal openflow rules required in each bridge (ACLs and NAT mostly).
> > - No central mac binding table required.
>
> Firstly to clarify the terminology of "mac binding" to avoid confusion, the
> mac_binding table currently in SB DB has nothing to do with L2 MAC
> learning. It is actually the ARP/Neighbor table of distributed logical
> routers. We should probably call it IP_MAC_binding table, or just Neighbor
> table.

Yes sorry about the confusion. I actually meant the FDB table.

> Here what you mean is actually L2 MAC learning, which today is implemented
> by the FDB table in SB DB, and it is only for uncommon use cases when the
> NB doesn't have the knowledge of a MAC address of a VIF.

This is not that uncommon in telco use cases where VNFs can send packets
from mac addresses unknown to OVN.

> The purpose of this proposal is clear - to avoid using a central table in
> DB for L2 information but instead using L2 MAC learning to populate such
> information on chassis, which is a reasonable alternative with pros and
> cons.
> However, I don't think it is necessary to use separate OVS bridges for this
> purpose. L2 MAC learning can be easily implemented in the br-int bridge
> with OVS flows, which is much simpler than managing dynamic number of OVS
> bridges just for the purpose of using the builtin OVS mac-learning.

I agree that this could also be implemented with VLAN tags on the
appropriate ports. But since OVS does not support trunk ports, it may
require complicated OF pipelines. My intent with this idea was two fold:

1) Avoid a central point of failure for mac learning/aging.
2) Simplify the OF pipeline by making all FDB operations dynamic.

> Now back to the distributed MAC learning idea itself. Essentially for two
> VMs/pods to communicate on L2, say, VM1@Chassis1 needs to send a packet to
> VM2@chassis2, assuming VM1 already has VM2's MAC address (we will discuss
> this later), Chassis1 needs to know that VM2's MAC is located on Chassis2.
>
> In OVN today this information is conveyed through:
>
> - MAC and LSP mapping (NB -> northd -> SB -> Chassis)
> - LSP and Chassis mapping (Chassis -> SB -> Chassis)
>
> In your proposal:
>
> - MAC and Chassis mapping (can be learned through initial L2
>   broadcast/flood)
>
> This indeed would avoid the control plane cost through the centralized
> components (for this L2 binding part). Given that today's SB OVSDB is a
> bottleneck, this idea may sound attractive. But please also take into
> consideration the below improvement that could mitigate the OVN central
> scale issue:
>
> - For MAC and LSP mapping, northd is now capable of incrementally
>   processing VIF related L2/L3 changes, so the cost of NB -> northd ->
>   SB is very small. For SB -> Chassis, a more scalable DB deployment,
>   such as the OVSDB relays, may largely help.

But using relays will only help with read-only operations (SB ->
chassis). Write operations (from dynamically learned mac addresses) will
be equivalent.

> - For LSP and Chassis mapping, the round trip through a central DB
>   obviously costs higher than a direct L2 broadcast (the targets are
>   the same). But this can be optimized if the MAC and Chassis is known
>   by the CMS system (which is true for most openstack/k8s env
>   I believe). Instead of updating the binding from each Chassis, CMS
>   can tell this information through the same NB -> northd -> SB ->
>   Chassis path, and the Chassis can just read the SB without updating
>   it.

This is only applicable for known L2 addresses.  Maybe telco use cases
are very specific, but being able to have ports that send packets from
unknown addresses is a strong requirement.

> On the other hand, the dynamic MAC learning approach has its own drawbacks.
>
> - It is simple to consider L2 only, but if considering more SDB
>   features, a central DB is more flexible to extend and implement new
>   features than a network protocol based approach.
> - It is more predictable and easier to debug with pre-populated
>   information through CMS than states learned dynamically in
>   data-plane.
> - With the DB approach we can suppress most of L2 broadcast/flood,
>   while with the distributed MAC learning broadcast/flood can't be
>   avoided. Although it may happen mostly when a new workload is
>   launched, it can also happen when aging. The cost of broadcast in
>   large L2 is also a potential threat to scale.

I may lack the field experience of operating large datacenter networks
but I was not aware of any scaling issues because of ARP and/or other L2
broadcasts.  Is this an actual problem that was reported by cloud/telco
operators and which influenced the 

Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - issues

2023-10-01 Thread Robin Jarry via discuss
Hi Han,

thanks a lot for your detailed answer.

Han Zhou, Sep 30, 2023 at 01:03:
> > I think ovn-controller only consumes the logical flows. The chassis and
> > port bindings tables are used by northd to updated these logical flows.
>
> Felix was right. For example, port-binding is firstly a configuration from
> north-bound, but the states such as its physical location (the chassis
> column) are populated by ovn-controller of the owning chassis and consumed
> by other ovn-controllers that are interested in that port-binding.

I was not aware of this. Thanks.

> > Exactly, but was the signaling between the nodes ever an issue?
>
> I am not an expert of BGP, but at least for what I am aware of, there are
> scaling issues in things like BGP full mesh signaling, and there are
> solutions such as route reflector (which is again centralized) to solve
> such issues.

I am not familiar with BGP full mesh signaling. But from what can tell,
it looks like the same concept than the full mesh GENEVE tunnels. Except
that the tunnels are only used when the same logical switch is
implemented between two nodes.

> > So you have enabled monitor_all=true as well? Or did you test at scale
> > with monitor_all=false.
> >
> We do use monitor_all=false, primarily to reduce memory footprint (and also
> CPU cost of IDL processing) on each chassis. There are trade-offs to the SB
> DB server performance:
>
> - On one hand it increases the cost of conditional monitoring, which
>   is expensive for sure
> - On the other hand, it reduces the total amount of data for the
>   server to propagate to clients
>
> It really depends on your topology for making the choice. If most of the
> nodes would anyway monitor most of the DB data (something similar to a
> full-mesh), it is more reasonable to use monitor_all=true. Otherwise, in
> topology like ovn-kubernetes where each node has its dedicated part of the
> data, or in topologies where you have lots of small "island" such as a
> cloud with many small tenants that never talks to each other, using
> monitor_all=false could make sense (but still need to be carefully
> evaluated and tested for your own use cases).

I didn't see recent scale testing for openstack, but in past testing we
had to set monitor_all=true because the CPU usage of the SB ovsdb was
a bottleneck.

> > The memory usage would be reduced but I don't know to which point. One
> > of the main consumers is the logical flows table which is required
> > everywhere. Unless there is a way to only sync a portion of this table
> > depending on the chassis, disabling monitor_all would save syncing the
> > unneeded tables for ovn-controller: chassis, port bindings, etc.
>
> Probably it wasn't what you meant, but I'd like to clarify that it is not
> about unneeded tables, but unneeded rows in those tables (mainly
> logical_flow and port_binding).
> It indeed syncs only a portion of the tables. It is not depending directly
> on chassis, but depending on what port-bindings are on the chassis and what
> logical connectivity those port-bindings have. So, again, the choice really
> depends on your use cases.

What about the FDB (mac-port) and MAC binding (ip-mac) tables? I thought
ovn-controller does not need them. If that is the case, I thought that
by default, the whole tables (not only some of their rows) were excluded
from the synchronized data.

Thanks!

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-09-30 Thread Robin Jarry via discuss
Hi Vladislav, Frode,

Thanks for your replies.

Frode Nordahl, Sep 30, 2023 at 10:55:
> On Sat, Sep 30, 2023 at 9:43 AM Vladislav Odintsov via discuss
>  wrote:
> > > On 29 Sep 2023, at 18:14, Robin Jarry via discuss 
> > >  wrote:
> > >
> > > Felix Huettner, Sep 29, 2023 at 15:23:
> > >>> Distributed mac learning
> > >>> 
> > > [snip]
> > >>>
> > >>> Cons:
> > >>>
> > >>> - How to manage seamless upgrades?
> > >>> - Requires ovn-controller to move/plug ports in the correct bridge.
> > >>> - Multiple openflow connections (one per managed bridge).
> > >>> - Requires ovn-trace to be reimplemented differently (maybe other tools
> > >>>  as well).
> > >>
> > >> - No central information anymore on mac bindings. All nodes need to
> > >>  update their data individually
> > >> - Each bridge generates also a linux network interface. I do not know if
> > >>  there is some kind of limit to the linux interfaces or the ovs bridges
> > >>  somewhere.
> > >
> > > That's a good point. However, only the bridges related to one
> > > implemented logical network would need to be created on a single
> > > chassis. Even with the largest OVN deployments, I doubt this would be
> > > a limitation.
> > >
> > >> Would you still preprovision static mac addresses on the bridge for all
> > >> port_bindings we know the mac address from, or would you rather leave
> > >> that up for learning as well?
> > >
> > > I would leave everything dynamic.
> > >
> > >> I do not know if there is some kind of performance/optimization penality
> > >> for moving packets between different bridges.
> > >
> > > As far as I know, once the openflow pipeline has been resolved into
> > > a datapath flow, there is no penalty.
> > >
> > >> You can also not only use the logical switch that have a local port
> > >> bound. Assume the following topology:
> > >> +---+ +---+ +---+ +---+ +---+ +---+ +---+
> > >> |vm1+-+ls1+-+lr1+-+ls2+-+lr2+-+ls3+-+vm2|
> > >> +---+ +---+ +---+ +---+ +---+ +---+ +---+
> > >> vm1 and vm2 are both running on the same hypervisor. Creating only local
> > >> logical switches would mean only ls1 and ls3 are available on that
> > >> hypervisor. This would break the connection between the two vms which
> > >> would in the current implementation just traverse the two logical
> > >> routers.
> > >> I guess we would need to create bridges for each locally reachable
> > >> logical switch. I am concerned about the potentially significant
> > >> increase in bridges and openflow connections this brings.
> > >
> > > That is one of the concerns I raised in the last point. In my opinion
> > > this is a trade off. You remove centralization and require more local
> > > processing. But overall, the processing cost should remain equivalent.
> >
> > Just want to clarify.
> >
> > For topology described by Felix above, you propose to create 2 OVS
> > bridges, right? How will the packet traverse from vm1 to vm2?

In this particular case, there would be 3 OVS bridges, one for each
logical switch.

> > Currently when the packet enters OVS all the logical switching and
> > routing openflow calculation is done with no packet re-entering OVS,
> > and this results in one DP flow match to deliver this packet from
> > vm1 to vm2 (if no conntrack used, which could introduce
> > recirculations).
> >
> > Do I understand correctly, that in this proposal OVS needs to
> > receive packet from “ls1” bridge, next run through lrouter “lr1”
> > OpenFlow pipelines, then output packet to “ls2” OVS bridge for mac
> > learning between logical routers (should we have here OF flow with
> > learn action?), then send packet again to OVS, calculate “lr2”
> > OpenFlow pipeline and finally reach destination OVS bridge “ls3” to
> > send packet to a vm2?

What I am proposing is to implement the northbound L2 network intent
with actual OVS bridges and builtin OVS mac learning. The L3/L4 network
constructs and ACLs would require patch ports and specific OF pipelines.

We could even think of adding more advanced L3 capabilities (RIB) into
OVS to simplify the OF pipelines.

> > Also, will such behavior be compatible with HW-offload-capable to
> > smartnics/DPUs?
>
> I am also a bit concerned about this, what woul

Sponsor upload request: golang-sourcehut-rockorager-go-jmap

2023-09-29 Thread Robin Jarry
Hi all,

I am preparing for packaging the latest release of aerc:

https://git.sr.ht/~rjarry/aerc/refs/0.16.0

It requires a new dependency which is not available in Debian yet:

git.sr.ht/~rockorager/go-jmaphttps://bugs.debian.org/1053244

Using dh-make-golang, I have prepared the package on salsa:

https://salsa.debian.org/go-team/packages/golang-sourcehut-rockorager-go-jmap

Could a kind soul please upload them to NEW on my behalf?

Thanks a bunch.

-- 
Robin



Bug#1053244: ITP: golang-sourcehut-rockorager-go-jmap -- A JMAP client library

2023-09-29 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-go-jmap
  Version : 0.3.0-1
  Upstream Author : Tim Culverhouse
* URL : https://git.sr.ht/~rockorager/go-jmap
* License : Expat
  Programming Lang: Go
  Description : A JMAP client library

A JMAP client library. Includes support for all core functionality (including
PushSubscription and EventSource event streams), mail, smime-verify, and MDN
specifications.

This is a new build dependency of aerc.



Bug#1053244: ITP: golang-sourcehut-rockorager-go-jmap -- A JMAP client library

2023-09-29 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-go-jmap
  Version : 0.3.0-1
  Upstream Author : Tim Culverhouse
* URL : https://git.sr.ht/~rockorager/go-jmap
* License : Expat
  Programming Lang: Go
  Description : A JMAP client library

A JMAP client library. Includes support for all core functionality (including
PushSubscription and EventSource event streams), mail, smime-verify, and MDN
specifications.

This is a new build dependency of aerc.



Bug#1053244: ITP: golang-sourcehut-rockorager-go-jmap -- A JMAP client library

2023-09-29 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-go-jmap
  Version : 0.3.0-1
  Upstream Author : Tim Culverhouse
* URL : https://git.sr.ht/~rockorager/go-jmap
* License : Expat
  Programming Lang: Go
  Description : A JMAP client library

A JMAP client library. Includes support for all core functionality (including
PushSubscription and EventSource event streams), mail, smime-verify, and MDN
specifications.

This is a new build dependency of aerc.



Bug#1053244: ITP: golang-sourcehut-rockorager-go-jmap -- A JMAP client library

2023-09-29 Thread Robin Jarry
Package: wnpp
Severity: wishlist
Owner: Robin Jarry 

* Package name: golang-sourcehut-rockorager-go-jmap
  Version : 0.3.0-1
  Upstream Author : Tim Culverhouse
* URL : https://git.sr.ht/~rockorager/go-jmap
* License : Expat
  Programming Lang: Go
  Description : A JMAP client library

A JMAP client library. Includes support for all core functionality (including
PushSubscription and EventSource event streams), mail, smime-verify, and MDN
specifications.

This is a new build dependency of aerc.



Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-09-29 Thread Robin Jarry via discuss
Felix Huettner, Sep 29, 2023 at 15:23:
> > Distributed mac learning
> > 
[snip]
> >
> > Cons:
> >
> > - How to manage seamless upgrades?
> > - Requires ovn-controller to move/plug ports in the correct bridge.
> > - Multiple openflow connections (one per managed bridge).
> > - Requires ovn-trace to be reimplemented differently (maybe other tools
> >   as well).
>
> - No central information anymore on mac bindings. All nodes need to
>   update their data individually
> - Each bridge generates also a linux network interface. I do not know if
>   there is some kind of limit to the linux interfaces or the ovs bridges
>   somewhere.

That's a good point. However, only the bridges related to one
implemented logical network would need to be created on a single
chassis. Even with the largest OVN deployments, I doubt this would be
a limitation.

> Would you still preprovision static mac addresses on the bridge for all
> port_bindings we know the mac address from, or would you rather leave
> that up for learning as well?

I would leave everything dynamic.

> I do not know if there is some kind of performance/optimization penality
> for moving packets between different bridges.

As far as I know, once the openflow pipeline has been resolved into
a datapath flow, there is no penalty.

> You can also not only use the logical switch that have a local port
> bound. Assume the following topology:
> +---+ +---+ +---+ +---+ +---+ +---+ +---+
> |vm1+-+ls1+-+lr1+-+ls2+-+lr2+-+ls3+-+vm2|
> +---+ +---+ +---+ +---+ +---+ +---+ +---+
> vm1 and vm2 are both running on the same hypervisor. Creating only local
> logical switches would mean only ls1 and ls3 are available on that
> hypervisor. This would break the connection between the two vms which
> would in the current implementation just traverse the two logical
> routers.
> I guess we would need to create bridges for each locally reachable
> logical switch. I am concerned about the potentially significant
> increase in bridges and openflow connections this brings.

That is one of the concerns I raised in the last point. In my opinion
this is a trade off. You remove centralization and require more local
processing. But overall, the processing cost should remain equivalent.

> > Use multicast for overlay networks
> > ==
[snip]
> > - 24bit VNI allows for more than 16 million logical switches. No need
> >   for extended GENEVE tunnel options.
> Note that using vxlan at the moment significantly reduces the ovn
> featureset. This is because the geneve header options are currently used
> for data that would not fit into the vxlan vni.
>
> From ovn-architecture.7.xml:
> ```
> The maximum number of networks is reduced to 4096.
> The maximum number of ports per network is reduced to 2048.
> ACLs matching against logical ingress port identifiers are not supported.
> OVN interconnection feature is not supported.
> ```

In my understanding, the main reason why GENEVE replaced VXLAN is
because Openstack uses full mesh point to point tunnels and that the
sender needs to know behind which chassis any mac address is to send it
into the correct tunnel. GENEVE allowed to reduce the lookup time both
on the sender and receiver thanks to ingress/egress port metadata.

https://blog.russellbryant.net/2017/05/30/ovn-geneve-vs-vxlan-does-it-matter/
https://dani.foroselectronica.es/ovn-geneve-encapsulation-541/

If VXLAN + multicast and address learning was used, the "correct" tunnel
would be established ad-hoc and both sender and receiver lookups would
only be a simple mac forwarding with learning. The ingress pipeline
would probably cost a little more.

Maybe multicast + address learning could be implemented for GENEVE as
well. But it would not be interoperable with other VTEPs.

> > - Limited and scoped "flooding" with IGMP/MLD snooping enabled in
> >   top-of-rack switches. Multicast is only used for BUM traffic.
> > - Only one VXLAN output port per implemented logical switch on a given
> >   chassis.
>
> Would this actually work with one VXLAN output port? Would you not need
> one port per target node to send unicast traffic (as you otherwise flood
> all packets to all participating nodes)?

You would need one VXLAN output port per implemented logical switch on
a given chassis. The port would have a VNI (unique per logical switch)
and an associated multicast IP address. Any chassis that implement this
logical switch would subscribe to that multicast group. The flooding
would be limited to first packets and broadcast/multicast traffic (ARP
requests, mostly). Once the receiver node replies, all communication
will happen with unicast.

https://networkdirection.net/articles/routingandswitching/vxlanoverview/vxlanaddresslearning/#BUM_Traffic

> > Cons:
> >
> > - OVS does not support VXLAN address learning yet.
> > - The number of usable multicast groups in a fabric network may be
> >   limited?
> > - How to manage seamless upgrades and 

Re: [ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - issues

2023-09-29 Thread Robin Jarry via discuss
Hi Felix,

Thanks a lot for your message.

Felix Huettner, Sep 29, 2023 at 14:35:
> I can get that when running 10k ovn-controllers the benefits of
> optimizing cpu and memory load are quite significant. However i am
> unsure about reducing the footprint of ovn-northd.
> When running so many nodes i would have assumed that having an
> additional (or maybe two) dedicated machines for ovn-northd would
> be completely acceptable, as long as it can still actually do what
> it should in a reasonable timeframe.
> Would the goal for ovn-northd be more like "Reduce the full/incremental
> recompute time" then?

The main goal of this thread is to get a consensus on the actual issues
that prevent scaling at the moment. We can discuss solutions in the
other thread.

> > * Allow support for alternative datapath implementations.
>
> Does this mean ovs datapths (e.g. dpdk) or something different?

See the other thread.

> > Southbound Design
> > =
...
> Note that also ovn-controller consumes the "state" of other chassis to
> e.g build the tunnels to other chassis. To visualize my understanding
>
> ++---++
> || configuration |   state|
> ++---++
> |   ovn-northd   |  write-only   | read-only  |
> ++---++
> | ovn-controller |   read-only   | read-write |
> ++---++
> |some cms|  no access?   | read-only  |
> ++---++

I think ovn-controller only consumes the logical flows. The chassis and
port bindings tables are used by northd to updated these logical flows.

> > Centralized decisions
> > =
> >
> > Every chassis needs to be "aware" of all other chassis in the cluster.
>
> I think we need to accept this as fundamental truth. Indepentent if you
> look at centralized designs like ovn or the neutron-l2 implementation
> or if you look at decentralized designs like bgp or spanning tree. In
> all cases if we need some kind of organized communication we need to
> know all relevant peers.
> Designs might diverge if you need to be "aware" of all peers or just
> some of them, but that is just a tradeoff between data size and options
> you have to forward data.
>
> > This requirement mainly comes from overlay networks that are implemented
> > over a full-mesh of point-to-point GENEVE tunnels (or VXLAN with some
> > limitations). It is not a scaling issue by itself, but it implies
> > a centralized decision which in turn puts pressure on the central node
> > at scale.
>
> +1. On the other hand it removes signaling needs between the nodes (like
> you would have with bgp).

Exactly, but was the signaling between the nodes ever an issue?

> > Due to ovsdb monitoring and caching, any change in the southbound DB
> > (either by northd or by any of the chassis controllers) is replicated on
> > every chassis. The monitor_all option is often enabled on large clusters
> > to avoid the conditional monitoring CPU cost on the central node.
>
> This is, i guess, something that should be possible to fix. We have also
> enabled this setting as it gave us stability improvements and we do not
> yet see performance issues with it

So you have enabled monitor_all=true as well? Or did you test at scale
with monitor_all=false.

What I am saying is that without monitor_all=true, the southbound
ovsdb-server needs to do checks to determine what updates to send to
which client. Since the server is single threaded, it becomes an issue
at scale. I know that there were some significant improvements made
recently but it will only push the limit further. I don't have hard data
to prove my point yet unfortunately.

> > This leads to high memory usage on all chassis, control plane traffic
> > and possible disruptions in the ovs-vswitchd datapath flow cache.
> > Unfortunately, I don't have any hard data to back this claim. This is
> > mainly coming from discussions I had with neutron contributors and from
> > brainstorming sessions with colleagues.
>
> Could you maybe elaborate on the datapath flow cache issue, as it sounds
> like it might affect actual live traffic and i am not aware of details
> there.

I may have had a wrong understanding of the mechanisms of OVS here.
I was under the impression that any update of the openflow rules would
invalidate of all datapath flows. It is far more subtle than this [1].
So unless there is an actual change in the packet pipeline, live traffic
should not be affected.

[1] 
https://developers.redhat.com/articles/2022/10/19/open-vswitch-revalidator-process-explained

> The memory usage and the traffic would be fixed by not having to rely on
> monitor_all, right?

The memory usage would be reduced but I don't know to which point. One
of the main consumers is the logical flows table which is required
everywhere. Unless there is a way to only sync a portion of this table

[ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - proposals

2023-09-28 Thread Robin Jarry via discuss
Hello OVN community,

This is a follow up on the message I have sent today [1]. That second
part focuses on some ideas I have to remove the limitations that were
mentioned in the previous email.

[1] 
https://mail.openvswitch.org/pipermail/ovs-discuss/2023-September/052695.html

If you didn't read it, my goal is to start a discussion about how we
could improve OVN on the following topics:

- Reduce the memory and CPU footprint of ovn-controller, ovn-northd.
- Support scaling of L2 connectivity across larger clusters.
- Simplify CMS interoperability.
- Allow support for alternative datapath implementations.

Disclaimer:

This message does not mention anything about L3/L4 features of OVN.
I didn't have time to work on these, yet. I hope we can discuss how
these fit with my ideas.

Distributed mac learning


Use one OVS bridge per logical switch with mac learning enabled. Only
create the bridge if the logical switch has a port bound to the local
chassis.

Pros:

- Minimal openflow rules required in each bridge (ACLs and NAT mostly).
- No central mac binding table required.
- Mac table aging comes for free.
- Zero access to southbound DB for learned addresses nor for aging.

Cons:

- How to manage seamless upgrades?
- Requires ovn-controller to move/plug ports in the correct bridge.
- Multiple openflow connections (one per managed bridge).
- Requires ovn-trace to be reimplemented differently (maybe other tools
  as well).

Use multicast for overlay networks
==

Use a unique 24bit VNI per overlay network. Derive a multicast group
address from that VNI. Use VXLAN address learning [2] to remove the need
for ovn-controller to know the destination chassis for every mac address
in advance.

[2] https://datatracker.ietf.org/doc/html/rfc7348#section-4.2

Pros:

- Nodes do not need to know about others in advance. The control plane
  load is distributed across the cluster.
- 24bit VNI allows for more than 16 million logical switches. No need
  for extended GENEVE tunnel options.
- Limited and scoped "flooding" with IGMP/MLD snooping enabled in
  top-of-rack switches. Multicast is only used for BUM traffic.
- Only one VXLAN output port per implemented logical switch on a given
  chassis.

Cons:

- OVS does not support VXLAN address learning yet.
- The number of usable multicast groups in a fabric network may be
  limited?
- How to manage seamless upgrades and interoperability with older OVN
  versions?

Connect ovn-controller to the northbound DB
===

This idea extends on a previous proposal to migrate the logical flows
creation in ovn-controller [3].

[3] 
https://patchwork.ozlabs.org/project/ovn/patch/20210625233130.3347463-1-num...@ovn.org/

If the first two proposals are implemented, the southbound database can
be removed from the picture. ovn-controller can directly translate the
northbound schema into OVS configuration bridges, ports and flow rules.

For other components that require access to the southbound DB (e.g.
neutron metadata agent), ovn-controller should provide an interface to
expose state and configuration data for local consumption.

All state information present in the NB DB should be moved to a separate
state database [4] for CMS consumption.

[4] https://mail.openvswitch.org/pipermail/ovs-dev/2023-April/403675.html

For those who like visuals, I have started working on basic use cases
and how they would be implemented without a southbound database [5].

[5] https://link.excalidraw.com/p/readonly/jwZgJlPe4zhGF8lE5yY3

Pros:

- The northbound DB is smaller by design: reduced network bandwidth and
  memory usage in all chassis.
- If we keep the northbound read-only for ovn-controller, it removes
  scaling issues when one controller updates one row that needs to be
  replicated everywhere.
- The northbound schema knows nothing about flows. We could introduce
  alternative dataplane backends configured by ovn-controller via
  plugins. I have done a minimal PoC to check if it could work with the
  linux network stack [6].

[6] https://github.com/rjarry/ovn-nb-agent/blob/main/backend/linux/bridge.go

Cons:

- This would be a serious API breakage for systems that depend on the
  southbound DB.
- Can all OVN constructs be implemented without a southbound DB?
- Is the community interested in alternative datapaths?

Closing thoughts


I mainly focused on OpenStack use cases for now, but I think these
propositions could benefit Kubernetes as well.

I hope I didn't bore everyone to death. Let me know what you think.

Cheers!

-- 
Robin Jarry
Red Hat, Telco/NFV

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


[ovs-discuss] OVN: scaling L2 networks beyond 10k chassis - issues

2023-09-28 Thread Robin Jarry via discuss
Hello OVN community,

I'm glad the subject of this message has caught your attention :-)

I would like to start a discussion about how we could improve OVN on the
following topics:

* Reduce the memory and CPU footprint of ovn-controller, ovn-northd.
* Support scaling of L2 connectivity across larger clusters.
* Simplify CMS interoperability.
* Allow support for alternative datapath implementations.

This first email will focus on the current issues that (in my view) are
preventing OVN from scaling L2 networks on larger clusters. I will send
another message with some change proposals to remove or fix these
issues.

Disclaimer:

I am fairly new to this project and my perception and understanding may
be incorrect in some aspects. Please forgive me in advance if I use the
wrong terms and/or make invalid statements. My intent is only to make
things better and not to put the blame on anyone for the current design
choices.

Southbound Design
=

In the current architecture, both databases contain a mix of state and
configuration. While this does not seem to cause any scaling issues for
the northbound DB, it can become a bottleneck for the southbound with
large numbers of chassis and logical network constructs.

The southbound database contains a mix of configuration (logical flows
transformed from the logical network topology) and state (chassis, port
bindings, mac bindings, FDB entries, etc.).

The "configuration" part is consumed by ovn-controller to implement the
network on every chassis and the "state" part is consumed by ovn-northd
to update the northbound "state" entries and to update logical flows.
Some CMS's [1] also depend on the southbound "state" in order to
function properly.

[1] 
https://opendev.org/openstack/neutron/src/tag/22.0.0/neutron/agent/ovn/metadata/ovsdb.py#L39-L40

Centralized decisions
=

Every chassis needs to be "aware" of all other chassis in the cluster.
This requirement mainly comes from overlay networks that are implemented
over a full-mesh of point-to-point GENEVE tunnels (or VXLAN with some
limitations). It is not a scaling issue by itself, but it implies
a centralized decision which in turn puts pressure on the central node
at scale.

Due to ovsdb monitoring and caching, any change in the southbound DB
(either by northd or by any of the chassis controllers) is replicated on
every chassis. The monitor_all option is often enabled on large clusters
to avoid the conditional monitoring CPU cost on the central node.

This leads to high memory usage on all chassis, control plane traffic
and possible disruptions in the ovs-vswitchd datapath flow cache.
Unfortunately, I don't have any hard data to back this claim. This is
mainly coming from discussions I had with neutron contributors and from
brainstorming sessions with colleagues.

I hope that the current work on OVN heater to integrate openstack
support [2] will allow getting more insight.

[2] https://github.com/ovn-org/ovn-heater/pull/179

Dynamic mac learning


Logical switch ports on a given chassis are all connected to the same
OVS bridge, in the same VLAN. This prevents from using local mac address
learning and shifts the responsibility to a centralized ovn-northd to
create all the required logical flows to properly segment the network.

When using mac_address=unknown ports, centralized mac learning is
enabled and when a new address is seen entering a port, OVS sends it to
the local controller which updates the FDB table and recomputes flow
rules accordingly. With logical switches spanning across a large number
of chassis, this centralized mac address learning and aging can have an
impact on control plane and dataplane performance.

Closing thoughts


My understanding of L3 and L4 capabilities of OVN are too limited to
discuss if there are other issues that would prevent scaling to
thousands of nodes. My point was mainly focused on L2 network scaling.

I would love to get other opinions on these statements.

Cheers!

-- 
Robin Jarry
Red Hat, Telco/NFV

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


Re: [ovs-dev] [PATCH] dpdk: expose cpu usage stats on telemetry socket

2023-09-19 Thread Robin Jarry
Ilya Maximets, Sep 19, 2023 at 13:47:
> With flexibility of appctl comes absolutely no guarantee for API
> stability.  But as soon as we have structured output, someone will
> expect it.  If we can agree that users cannot rely on the structure
> of that structured output, then it's fine.  Otherwise, OVSDB with
> its defined schema is a much better choice, IMO.  Constructing a
> single 'select' transaction for OVSDB isn't really much more
> difficult than constructing an appctl JSON-PRC request.

I would argue that the ovsdb schema could also be modified so I guess
this comes up to deciding whether API can be broken or not.

However, going through ovsdb simply as an API proxy to query live stats
to ovs-vswitchd seems complex and not resource efficient. Especially if
the appctl socket is already available and allows to reach vswitchd
directly.

I think that for statistics, it would make more sense to go with the
lightweight option.

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


Re: [ovs-dev] [PATCH] dpdk: expose cpu usage stats on telemetry socket

2023-09-19 Thread Robin Jarry
Adrian Moreno, Sep 19, 2023 at 09:18:
> > Both OVSDB and appctl are literally JSON-RPC protocols.  There is no
> > need to re-invent anything.
>
> Right. Isn't appctl simpler in this case? IIUC, it would still satisfy
> the requirements: client decides update interval, flexible schema, etc
> with the benefits of incurring in less cost (no ovs-vswithcd <-> ovsdb
> communication, no need to store data in both places) and probably
> involving less internal code change.
>
> Just to clarify: I'm referring to allowing JSON output of the (already
> JSON-RPC) appctl protocol.

I agree. Using ovsdb for ephemeral stats seems weird to me. appctl and
a more fluid schema/data structure would be suitable.

What kind of API did you have in mind to structure the JSON output?

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


Re: [ovs-dev] [PATCH] dpdk: expose cpu usage stats on telemetry socket

2023-09-12 Thread Robin Jarry
Eelco Chaudron, Sep 12, 2023 at 09:17:
> I feel like if we do need another way of getting (real time)
> statistics out of OVS, we should use the same communication channel as
> the other ovs-xxx utilities are using. But rather than returning
> text-based responses, we might be able to make it JSON (which is
> already used by the dbase). I know that Adrian is already
> investigating machine-readable output for some existing utilities,
> maybe it can be extended for the (pmd) statistics use case.
>
> Using something like the DPDK telemetry socket, might not work for
> other use cases where DPDK is not in play.

Maybe the telemetry socket code could be reused even when DPDK is not in
play. It already has all the APIs to return structured data and
serialize it to JSON. It would be nice not to have to reinvent the
wheel.

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


Re: [ovs-dev] [PATCH] dpdk: expose cpu usage stats on telemetry socket

2023-09-11 Thread Robin Jarry
Hey Kevin,

Kevin Traynor, Sep 07, 2023 at 15:37:
> This came up in conversation with other maintainers as I mentioned I was 
> reviewing and the question raised was - Why add this ? if you want these 
> values exposed, wouldn't it be better to to add to ovsdb ?

That's a good point. I had considered using ovsdb but it seemed to me
less suitable for a few reasons:

* I had understood that ovsdb is a configuration database, not a state
  reporting database.

* To have reliable and up to date numbers, ovs would need to push them
  at high rate to the database so that clients to get outdated cpu
  usage. The DPDK telemetry socket is real-time, the current numbers are
  returned on every request.

* I would need to define a custom schema / table to store structured
  information in the db. The DPDK telemetry socket already has a schema
  defined for this.

* Accessing ovsdb requires a library making it more complex to use for
  telemetry scrapers. The DPDK telemetry socket can be accessed with
  a standalone python script with no external dependencies[1].

[1]: 
https://github.com/rjarry/dpdk/blob/main/usertools/prometheus-dpdk-exporter.py#L135-L143

Maybe my observations are wrong, please do correct me if they are.

> Are you looking for individual lcore usage with identification of that 
> pmd? or overall aggregate usage ?
>
> I ask because it will report lcore id's which would need to be mapped to 
> pmd core id's for anything regarding individual pmds.
>
> That can be found in ovs-vswitchd.log or checked locally with 
> 'ovs-appctl dpdk/lcore-list' but assuming if they were available, then 
> user would not be using dpdk telemetry anyway.

I would assume that the important data is the aggregate usage for
overall monitoring and resource planing. Individual pmd usage can be
accessed for fine tuning and debugging via appctl.

> These stats are cumulative so in the absence of 'ovs-appctl 
> dpif-netdev/pmd-stats-clear'  that would need to be taken care of with 
> some post-processing by whatever is pulling these stats - otherwise 
> you'll get cumulative stats for an unknown time period and unknown 
> traffic profile (e.g. it would be counting before any traffic started).
>
> These might also be reset with pmd-stats-clear independently, so that 
> would need to be accounted for too.

The only important data point that we need is the ratio between
busy/(busy + idle) over a specified delta which any scraper can do.
I consider these numbers like any other counter that can eventually be
reset.

See this reply from Morten Brørup on dpdk-dev for more context:

https://lore.kernel.org/dpdk-dev/98cbd80474fa8b44bf855df32c47dc35d87...@smartserver.smartshare.dk/

> Another thing I noticed is that without the pmd-sleep info the stats in 
> isolation can be misleading. Example below:
>
> With low rate traffic and clearing stats between 10 sec runs
>
> 2023-09-07T13:14:56Z|00158|dpif_netdev|INFO|PMD max sleep request is 0 
> usecs.
> 2023-09-07T13:14:56Z|00159|dpif_netdev|INFO|PMD load based sleeps are 
> disabled.
>
> Time: 13:15:06.842
> Measurement duration: 10.009 s
>
> pmd thread numa_id 0 core_id 8:
>
>Iterations: 51712564  (0.19 us/it)
>- Used TSC cycles:   26021354654  (100.0 % of total cycles)
>- idle iterations:  51710963  ( 99.9 % of used cycles)
>- busy iterations:  1601  (  0.1 % of used cycles)
>- sleep iterations:0  (  0.0 % of iterations)
> ^^^ can see here that pmd does not sleep and is 0.1% busy
>
>Sleep time (us):   0  (  0 us/iteration avg.)
>Rx packets:37250  (4 Kpps, 866 cycles/pkt)
>Datapath passes:   37250  (1.00 passes/pkt)
>- PHWOL hits:  0  (  0.0 %)
>- MFEX Opt hits:   0  (  0.0 %)
>- Simple Match hits:   37250  (100.0 %)
>- EMC hits:0  (  0.0 %)
>- SMC hits:0  (  0.0 %)
>- Megaflow hits:   0  (  0.0 %, 0.00 subtbl lookups/hit)
>- Upcalls: 0  (  0.0 %, 0.0 us/upcall)
>- Lost upcalls:0  (  0.0 %)
>Tx packets:0
>
> {
>"/eal/lcore/usage": {
>  "lcore_ids": [
>1
>  ],
>  "total_cycles": [
>26127284389
>  ],
>  "busy_cycles": [
>32370313
>  ]
>}
> }
>
> ^^^ This in isolation implies pmd is 32370313/26127284389 0.12% busy 
> which is true
>
> 2023-09-07T13:15:06Z|00160|dpif_netdev|INFO|PMD max sleep request is 500 
> usecs.
> 2023-09-07T13:15:06Z|00161|dpif_netdev|INFO|PMD load based sleeps are 
> enabled.
>
> Time: 13:15:16.908
> Measurement duration: 10.008 s
>
> pmd thread numa_id 0 core_id 8:
>
>Iterations:75197  (133.09 us/it)
>- Used TSC cycles: 237910969  (  0.9 % of total cycles)
>- idle iterations: 73782  ( 74.4 % of used cycles)
>- busy iterations:  1415  ( 25.6 % of used cycles)
>- sleep iterations:

Bug#1049256: python-bonsai: Fails to build source after successful build

2023-09-05 Thread Robin Jarry
Lucas Nussbaum, Aug 14, 2023 at 03:54:
> If you fail to reproduce this, please provide a build log and diff it with 
> mine
> so that we can identify if something relevant changed in the meantime.

Hi Lucas,

I didn't manage to reproduce this. Here are both build logs binary and
source. I generated them with:

dpkg-buildpackage -us -uc; dpkg-buildpackage -S -us -uc

I don't use sbuild so I don't have the same logs than you have. Making
the diff a bit complex.
dpkg-buildpackage: info: source package python-bonsai
dpkg-buildpackage: info: source version 1.5.0+ds-3
dpkg-buildpackage: info: source distribution unstable
dpkg-buildpackage: info: source changed by Jelmer Vernooij 
dpkg-buildpackage: info: host architecture amd64
 dpkg-source --before-build .
dpkg-source: info: using patch list from debian/patches/series
dpkg-source: info: applying docs-disable-furo-theme.patch
dpkg-source: info: applying setup-do-not-use-distutils.patch
 debian/rules clean
dh clean --with python3,sphinxdoc --buildsystem=pybuild
   dh_auto_clean -O--buildsystem=pybuild
I: pybuild base:291: python3.11 setup.py clean 
running clean
removing '/<>/.pybuild/cpython3_3.11/build' (and everything under it)
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.11' does not exist -- can't clean it
I: pybuild pybuild:340: rm -rf /<>/docs/_build
   dh_autoreconf_clean -O--buildsystem=pybuild
   dh_clean -O--buildsystem=pybuild
 dpkg-source -b .
dpkg-source: info: using source format '3.0 (quilt)'
dpkg-source: info: building python-bonsai using existing ./python-bonsai_1.5.0+ds.orig.tar.xz
dpkg-source: info: using patch list from debian/patches/series
dpkg-source: info: building python-bonsai in python-bonsai_1.5.0+ds-3.debian.tar.xz
dpkg-source: info: building python-bonsai in python-bonsai_1.5.0+ds-3.dsc
 debian/rules binary
dh binary --with python3,sphinxdoc --buildsystem=pybuild
   dh_update_autotools_config -O--buildsystem=pybuild
   dh_autoreconf -O--buildsystem=pybuild
   dh_auto_configure -O--buildsystem=pybuild
I: pybuild base:291: python3.11 setup.py config 
running config
   debian/rules override_dh_auto_build
make[1]: Entering directory '/<>'
blhc: ignore-line-regexp: .*test_krb5\.o .*test_krb5
dh_auto_build -O--buildsystem=pybuild
I: pybuild base:291: /usr/bin/python3 setup.py build 
running build
running build_py
creating /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/__init__.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/errors.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapclient.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapconnection.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapdn.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapentry.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapreference.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapurl.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldapvaluelist.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/ldif.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/pool.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
copying src/bonsai/utils.py -> /<>/.pybuild/cpython3_3.11/build/bonsai
creating /<>/.pybuild/cpython3_3.11/build/bonsai/active_directory
copying src/bonsai/active_directory/__init__.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/active_directory
copying src/bonsai/active_directory/acl.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/active_directory
copying src/bonsai/active_directory/sid.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/active_directory
creating /<>/.pybuild/cpython3_3.11/build/bonsai/asyncio
copying src/bonsai/asyncio/__init__.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/asyncio
copying src/bonsai/asyncio/aioconnection.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/asyncio
copying src/bonsai/asyncio/aiopool.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/asyncio
creating /<>/.pybuild/cpython3_3.11/build/bonsai/gevent
copying src/bonsai/gevent/__init__.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/gevent
copying src/bonsai/gevent/geventconnection.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/gevent
creating /<>/.pybuild/cpython3_3.11/build/bonsai/tornado
copying src/bonsai/tornado/__init__.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/tornado
copying src/bonsai/tornado/tornadoconnection.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/tornado
creating /<>/.pybuild/cpython3_3.11/build/bonsai/trio
copying src/bonsai/trio/__init__.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/trio
copying src/bonsai/trio/trioconnection.py -> /<>/.pybuild/cpython3_3.11/build/bonsai/trio
running egg_info
creating bonsai.egg-info
writing bonsai.egg-info/PKG-INFO
writing dependency_links to bonsai.egg-info/dependency_links.txt
writing requirements to bonsai.egg-info/requires.txt
writing top-level names to bonsai.egg-info/top_level.txt
writing manifest file 

Re: [ovs-dev] [PATCH] dpdk: expose cpu usage stats on telemetry socket

2023-08-31 Thread Robin Jarry
Robin Jarry, Aug 31, 2023 at 15:11:
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 70b953ae6dd3..ebf43a0f62e4 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1427,6 +1427,41 @@ dpif_netdev_pmd_rebalance(struct unixctl_conn *conn, 
> int argc,
>  ds_destroy();
>  }
>  
> +static void
> +dpif_netdev_get_pmd_cycles(unsigned int core_id,
> +uint64_t *busy_cycles, uint64_t *total_cycles)
> +{
> +struct dp_netdev_pmd_thread **pmd_list = NULL;
> +uint64_t stats[PMD_N_STATS];
> +struct dp_netdev *dp;
> +size_t num_pmds;
> +
> +ovs_mutex_lock(_netdev_mutex);
> +
> +if (shash_count(_netdevs) != 1) {
> +goto out;
> +}
> +
> +dp = shash_first(_netdevs)->data;
> +sorted_poll_thread_list(dp, _list, _pmds);
> +
> +for (size_t i = 0; i < num_pmds; i++) {
> +struct dp_netdev_pmd_thread *pmd = pmd_list[i];
> +
> +if (pmd->core_id == core_id) {
> +continue;
> +}

This logic is reversed. Too bad for the last minute cleanup/rework.

Before sending a v2, I'll wait to see if there are other things to
change.

By the way, this patch is targeted for the dpdk-latest branch. I forgot
to change the subject prefix. I'll do that for v2 as well.

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


[ovs-dev] [PATCH] dpdk: expose cpu usage stats on telemetry socket

2023-08-31 Thread Robin Jarry
Since DPDK 23.03, it is possible to register a callback to report lcore
TSC cycles usage. Reuse the busy/idle cycles gathering in dpif-netdev
and expose them to the DPDK telemetry socket.

Upon dpdk_attach_thread, record the mapping between the DPDK lcore_id
and the dpif-netdev core_id. Reuse that mapping in the lcore usage
callback to invoke dpif_netdev_get_pmd_cycles.

Here is an example output:

  ~# ovs-appctl dpif-netdev/pmd-stats-show | grep -e ^pmd -e cycles:
  pmd thread numa_id 0 core_id 8:
idle cycles: 2720796781680 (100.00%)
processing cycles: 3566020 (0.00%)
  pmd thread numa_id 0 core_id 9:
idle cycles: 2718974371440 (100.00%)
processing cycles: 3136840 (0.00%)
  pmd thread numa_id 0 core_id 72:
  pmd thread numa_id 0 core_id 73:

  ~# echo /eal/lcore/usage | dpdk-telemetry.py | jq
  {
"/eal/lcore/usage": {
  "lcore_ids": [
3,
5,
11,
15
  ],
  "total_cycles": [
2725722342740,
2725722347480,
2723899464040,
2725722354980
  ],
  "busy_cycles": [
3566020,
3566020,
3136840,
3566020
  ]
}
  }

Link: https://git.dpdk.org/dpdk/commit/?id=9ab1804922ba583b0b16
Cc: David Marchand 
Cc: Kevin Traynor 
Signed-off-by: Robin Jarry 
---
 lib/dpdk-stub.c   |  5 +++
 lib/dpdk.c| 95 ++-
 lib/dpdk.h|  5 +++
 lib/dpif-netdev.c | 38 +++
 4 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
index 58ebf6cb62cd..02fb561bea7b 100644
--- a/lib/dpdk-stub.c
+++ b/lib/dpdk-stub.c
@@ -49,6 +49,11 @@ dpdk_detach_thread(void)
 {
 }
 
+void
+dpdk_register_core_usage_callback(dpdk_core_usage_cb *cb OVS_UNUSED)
+{
+}
+
 bool
 dpdk_available(void)
 {
diff --git a/lib/dpdk.c b/lib/dpdk.c
index d76d53f8f16c..31871300f719 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -23,6 +23,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -310,6 +311,10 @@ malloc_dump_stats_wrapper(FILE *stream)
 rte_malloc_dump_stats(stream, NULL);
 }
 
+#ifdef ALLOW_EXPERIMENTAL_API
+static int dpdk_get_lcore_cycles(unsigned int, struct rte_lcore_usage *);
+#endif
+
 static bool
 dpdk_init__(const struct smap *ovs_other_config)
 {
@@ -440,6 +445,10 @@ dpdk_init__(const struct smap *ovs_other_config)
 /* We are called from the main thread here */
 RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
 
+#ifdef ALLOW_EXPERIMENTAL_API
+rte_lcore_register_usage_cb(dpdk_get_lcore_cycles);
+#endif
+
 /* Finally, register the dpdk classes */
 netdev_dpdk_register(ovs_other_config);
 netdev_register_flow_api_provider(_offload_dpdk);
@@ -490,9 +499,52 @@ dpdk_available(void)
 return initialized;
 }
 
+struct lcore_id_map {
+unsigned int lcore_id;
+unsigned int pmd_core_id;
+};
+
+/* Protects against changes to 'lcore_id_maps'. */
+struct ovs_mutex lcore_id_maps_mutex = OVS_MUTEX_INITIALIZER;
+
+/* Contains all 'struct lcore_id_map's. */
+static struct shash lcore_id_maps OVS_GUARDED_BY(lcore_id_maps_mutex)
+= SHASH_INITIALIZER(_id_maps);
+
+static void
+lcore_id_to_str(char *buf, size_t len, unsigned int lcore_id)
+{
+int n;
+
+n = snprintf(buf, len, "%u", lcore_id);
+if (n < 0) {
+VLOG_WARN("Failed to format lcore_id: %s", ovs_strerror(errno));
+n = 0;
+}
+buf[n] = '\0';
+}
+
+static void
+lcore_id_map_update(unsigned int lcore_id, unsigned int cpu, bool add)
+{
+char buf[128];
+
+lcore_id_to_str(buf, sizeof buf, lcore_id);
+
+ovs_mutex_lock(_id_maps_mutex);
+if (add) {
+shash_replace(_id_maps, buf, (void *) (uintptr_t) cpu);
+} else {
+shash_find_and_delete(_id_maps, buf);
+}
+ovs_mutex_unlock(_id_maps_mutex);
+}
+
 bool
 dpdk_attach_thread(unsigned cpu)
 {
+unsigned int lcore_id;
+
 /* NON_PMD_CORE_ID is reserved for use by non pmd threads. */
 ovs_assert(cpu != NON_PMD_CORE_ID);
 
@@ -506,7 +558,9 @@ dpdk_attach_thread(unsigned cpu)
 return false;
 }
 
-VLOG_INFO("PMD thread uses DPDK lcore %u.", rte_lcore_id());
+lcore_id = rte_lcore_id();
+lcore_id_map_update(lcore_id, cpu, true);
+VLOG_INFO("PMD thread uses DPDK lcore %u.", lcore_id);
 return true;
 }
 
@@ -516,10 +570,49 @@ dpdk_detach_thread(void)
 unsigned int lcore_id;
 
 lcore_id = rte_lcore_id();
+lcore_id_map_update(lcore_id, 0, false);
+
 rte_thread_unregister();
 VLOG_INFO("PMD thread released DPDK lcore %u.", lcore_id);
 }
 
+static dpdk_core_usage_cb_t *core_usage_cb;
+
+void
+dpdk_register_core_usage_callback(dpdk_core_usage_cb_t *cb)
+{
+core_usage_cb = cb;
+}
+
+#ifdef ALLOW_EXPERIMENTAL_API
+static int
+dpdk_get_lcore_cycles(unsigned int lcore_id, struct rte_lcore_usage *usage)
+{
+struct shash_node *node;
+unsigned int core_id;
+

Re: [ovs-dev] [PATCH] tests/mfex: Don't require python cryptography.

2023-08-30 Thread Robin Jarry
Eelco Chaudron, Aug 30, 2023 at 11:21:
> > This is very unlikely that any code will trigger a UserWarning with that
> > text content.
>
> Well you never know ;) I would prefer the original one, as it’s more strict.

Fair enough :)

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


Re: [ovs-dev] [PATCH] tests/mfex: Don't require python cryptography.

2023-08-30 Thread Robin Jarry
David Marchand, Aug 30, 2023 at 11:11:
> > warnings.filterwarnings("ignore", r"(blowfish|cast5)", UserWarning)
>
> It means waiving more warnings than before, but I don't think it
> really matters for this script.
> No strong opinion against your proposal.

This is very unlikely that any code will trigger a UserWarning with that
text content.

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


Re: [ovs-dev] [PATCH] tests/mfex: Don't require python cryptography.

2023-08-30 Thread Robin Jarry
David Marchand, Aug 30, 2023 at 10:16:
> Tests using mfex_fuzzy.py will fail on systems lacking the Python
> cryptography library.
> cryptography is not required, it is only imported in mfex_fuzzy.py to
> silence some warnings when scapy tries to load some libraries.
>
> Fixes: c3ed0bf34b8a ("tests/mfex: Silence Blowfish/CAST5 deprecation 
> warnings.")
> Signed-off-by: David Marchand 
> ---
>  tests/mfex_fuzzy.py | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tests/mfex_fuzzy.py b/tests/mfex_fuzzy.py
> index 30028ba7a0..50b9870641 100755
> --- a/tests/mfex_fuzzy.py
> +++ b/tests/mfex_fuzzy.py
> @@ -3,12 +3,15 @@
>  import sys
>  import warnings
>  
> -from cryptography.utils import CryptographyDeprecationWarning
> -warnings.filterwarnings(
> -"ignore",
> -category=CryptographyDeprecationWarning,
> -message=r"(blowfish|cast5)",
> -)
> +try:
> +from cryptography.utils import CryptographyDeprecationWarning
> +warnings.filterwarnings(
> +"ignore",
> +category=CryptographyDeprecationWarning,
> +message=r"(blowfish|cast5)",
> +)
> +except ModuleNotFoundError:
> +pass

Hi David,

Since CryptographyDeprecationWarning is a subclass of UserWarning, you
can also that simpler form:

warnings.filterwarnings("ignore", r"(blowfish|cast5)", UserWarning)

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


Bug#1042578: aerc: No Reply-To completion

2023-08-24 Thread Robin Jarry
On Sun, 30 Jul 2023 17:48:07 +0200 Pelle  wrote:
> I think it would make sense if address book completion was enabled for
> the reply-to header field like it is for other fields for entering
> email addresses (From, To, CC, BCC).

Hi Pelle,

this is not supported at the moment. The list of headers for which there
is address completion is hard coded.

https://git.sr.ht/~rjarry/aerc/tree/0.15.2/item/completer/completer.go#L66-74

Feel free to submit a patch to add "reply-to" to that list. It should be
trivial ;)



[ovs-dev] [PATCH v2 3/3] python: Use build to generate PEP517 compatible archives.

2023-08-23 Thread Robin Jarry
Quoting Paul Ganssle, setuptools maintainer:

  * The setuptools project has stopped maintaining all direct
invocations of setup.py years ago, and distutils is deprecated.
There are undoubtedly many ways that your setup.py-based system is
broken today, even if it's not failing loudly or obviously.

  * Direct invocations of setup.py cannot bootstrap their own
dependencies, and so some CLI is necessary for dependency
management.

  * The setuptools project no longer wants to provide any public CLI,
and will be actively removing the existing interface (though the
time scale for this is long).

  * PEP 517, 518 and other standards-based packaging are the future of
the Python ecosystem and a lot of progress has been made on making
this upgrade seamless.

As described in the recommendations in the end of the article: `python3
setup.py sdist` should be replaced by `python3 -m build --sdist`.

Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary
Signed-off-by: Robin Jarry 
---
 python/automake.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/automake.mk b/python/automake.mk
index d9f6803691d7..84cf2eab57e8 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -114,7 +114,7 @@ ovs-install-data-local:
 
 .PHONY: python-sdist
 python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-   (cd python/ && $(PYTHON3) setup.py sdist)
+   cd python/ && $(PYTHON3) -m build --sdist
 
 .PHONY: pypi-upload
 pypi-upload: python-sdist
-- 
2.41.0

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


[ovs-dev] [PATCH v2 2/3] python: Use twine to upload sdist package to pypi.org.

2023-08-23 Thread Robin Jarry
setup.py upload is now deprecated. When used, pypi.org returns an error:

  Upload failed (400): Invalid value for blake2_256_digest. Error: Use
  a valid, hex-encoded, BLAKE2 message digest.

Use twine which is the recommended replacement tool to upload on
pypi.org.

Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary
Reported-by: Terry Wilson 
Signed-off-by: Robin Jarry 
---
 python/automake.mk | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/python/automake.mk b/python/automake.mk
index 6c7ac84b9c40..d9f6803691d7 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -112,11 +112,14 @@ ovs-install-data-local:
$(INSTALL_DATA) python/ovs/dirs.py.tmp 
$(DESTDIR)$(pkgdatadir)/python/ovs/dirs.py
rm python/ovs/dirs.py.tmp
 
+.PHONY: python-sdist
 python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
(cd python/ && $(PYTHON3) setup.py sdist)
 
-pypi-upload: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-   (cd python/ && $(PYTHON3) setup.py sdist upload)
+.PHONY: pypi-upload
+pypi-upload: python-sdist
+   twine upload python/dist/ovs-$(VERSION).tar.gz
+
 install-data-local: ovs-install-data-local
 
 UNINSTALL_LOCAL += ovs-uninstall-local
-- 
2.41.0

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


[ovs-dev] [PATCH v2 0/3] Python ovsdb bindings distribution improvements

2023-08-23 Thread Robin Jarry
v2:

* Keep code in the python dir but rename it so that it does not clash
  with `python -m build`.
* Remove duplication in makefiles.

Robin Jarry (3):
  python: Rename build related code to ovs_build_helpers.
  python: Use twine to upload sdist package to pypi.org.
  python: Use build to generate PEP517 compatible archives.

 Makefile.am   |  2 +-
 build-aux/extract-ofp-fields  | 12 -
 build-aux/gen_ofp_field_decoders  |  4 +--
 build-aux/sodepends.py|  3 ++-
 build-aux/soexpand.py |  3 ++-
 build-aux/xml2nroff   | 10 
 ovsdb/ovsdb-doc   |  2 +-
 python/automake.mk| 25 +++
 .../{build => ovs_build_helpers}/__init__.py  |  0
 .../extract_ofp_fields.py |  0
 python/{build => ovs_build_helpers}/nroff.py  |  0
 python/{build => ovs_build_helpers}/soutil.py |  0
 12 files changed, 33 insertions(+), 28 deletions(-)
 rename python/{build => ovs_build_helpers}/__init__.py (100%)
 rename python/{build => ovs_build_helpers}/extract_ofp_fields.py (100%)
 rename python/{build => ovs_build_helpers}/nroff.py (100%)
 rename python/{build => ovs_build_helpers}/soutil.py (100%)

-- 
2.41.0

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


[ovs-dev] [PATCH v2 1/3] python: Rename build related code to ovs_build_helpers.

2023-08-23 Thread Robin Jarry
The python/build folder contents are completely unrelated to the ovs
python bindings. These files are only used during the build for various
subsystems (docs, man pages, code generation, etc.).

Having that folder in that location prevents from running:

cd python && python3 -m build

Which is a way to generate PEP517 compatible source archives and binary
wheel packages.

Rename that folder to ovs_build_helpers which is more explicit. Update
all imports accordingly.

Link: https://peps.python.org/pep-0517/
Link: https://pypi.org/project/build/
Signed-off-by: Robin Jarry 
---
 Makefile.am  |  2 +-
 build-aux/extract-ofp-fields | 12 ++--
 build-aux/gen_ofp_field_decoders |  4 ++--
 build-aux/sodepends.py   |  3 ++-
 build-aux/soexpand.py|  3 ++-
 build-aux/xml2nroff  | 10 +-
 ovsdb/ovsdb-doc  |  2 +-
 python/automake.mk   | 16 
 python/{build => ovs_build_helpers}/__init__.py  |  0
 .../extract_ofp_fields.py|  0
 python/{build => ovs_build_helpers}/nroff.py |  0
 python/{build => ovs_build_helpers}/soutil.py|  0
 12 files changed, 27 insertions(+), 25 deletions(-)
 rename python/{build => ovs_build_helpers}/__init__.py (100%)
 rename python/{build => ovs_build_helpers}/extract_ofp_fields.py (100%)
 rename python/{build => ovs_build_helpers}/nroff.py (100%)
 rename python/{build => ovs_build_helpers}/soutil.py (100%)

diff --git a/Makefile.am b/Makefile.am
index db341504d37f..52c5d6e33a17 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -415,7 +415,7 @@ endif
 CLEANFILES += flake8-check
 
 -include manpages.mk
-manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py python/build/soutil.py
+manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py 
python/ovs_build_helpers/soutil.py
@PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) 
$(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) $(MAN_ROOTS) >$(@F).tmp
@if cmp -s $(@F).tmp $@; then \
  touch $@; \
diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index efec59c25b35..5b58f4454694 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -4,9 +4,9 @@ import getopt
 import sys
 import os.path
 import xml.dom.minidom
-import build.nroff
 
-from build.extract_ofp_fields import (
+from ovs_build_helpers import nroff
+from ovs_build_helpers.extract_ofp_fields import (
 extract_ofp_fields,
 PREREQS,
 OXM_CLASSES,
@@ -297,7 +297,7 @@ l lx.
 body += [".TE\n"]
 
 body += [".PP\n"]
-body += [build.nroff.block_xml_to_nroff(field_node.childNodes)]
+body += [nroff.block_xml_to_nroff(field_node.childNodes)]
 
 
 def group_xml_to_nroff(group_node, fields):
@@ -310,11 +310,11 @@ def group_xml_to_nroff(group_node, fields):
 id_ = node.attributes["id"].nodeValue
 field_to_xml(node, fields[id_], body, summary)
 else:
-body += [build.nroff.block_xml_to_nroff([node])]
+body += [nroff.block_xml_to_nroff([node])]
 
 content = [
 ".bp\n",
-'.SH "%s"\n' % build.nroff.text_to_nroff(title.upper() + " FIELDS"),
+'.SH "%s"\n' % nroff.text_to_nroff(title.upper() + " FIELDS"),
 '.SS "Summary:"\n',
 ".TS\n",
 "tab(;);\n",
@@ -422,7 +422,7 @@ ovs\-fields \- protocol header fields in OpenFlow and Open 
vSwitch
 elif node.nodeType == node.COMMENT_NODE:
 pass
 else:
-s += build.nroff.block_xml_to_nroff([node])
+s += nroff.block_xml_to_nroff([node])
 
 for f in fields:
 if "used" not in f:
diff --git a/build-aux/gen_ofp_field_decoders b/build-aux/gen_ofp_field_decoders
index 0b797ee8c8c1..0cb6108c2223 100755
--- a/build-aux/gen_ofp_field_decoders
+++ b/build-aux/gen_ofp_field_decoders
@@ -2,7 +2,7 @@
 
 import argparse
 
-import build.extract_ofp_fields as extract_fields
+from ovs_build_helpers.extract_ofp_fields import extract_ofp_fields
 
 
 def main():
@@ -19,7 +19,7 @@ def main():
 
 args = parser.parse_args()
 
-fields = extract_fields.extract_ofp_fields(args.metaflow)
+fields = extract_ofp_fields(args.metaflow)
 
 field_decoders = {}
 aliases = {}
diff --git a/build-aux/sodepends.py b/build-aux/sodepends.py
index 45812bcbd700..ac8dd61a4b2f 100755
--- a/build-aux/sodepends.py
+++ b/build-aux/sodepends.py
@@ -14,9 +14,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from build import soutil
 import sys
 
+from ovs_build_helpers import soutil
+
 
 def sodepends(include_dirs, filenames, dst):
 ok = True
diff --git a/build

[ovs-dev] [PATCH 3/3] python: Use build to generate PEP517 compatible archives.

2023-08-02 Thread Robin Jarry
Quoting Paul Ganssle, setuptools maintainer:
> * The setuptools project has stopped maintaining all direct
>   invocations of setup.py years ago, and distutils is deprecated.
>   There are undoubtedly many ways that your setup.py-based system is
>   broken today, even if it's not failing loudly or obviously.
>
> * Direct invocations of setup.py cannot bootstrap their own
>   dependencies, and so some CLI is necessary for dependency
>   management.
>
> * The setuptools project no longer wants to provide any public CLI,
>   and will be actively removing the existing interface (though the
>   time scale for this is long).
>
> * PEP 517, 518 and other standards-based packaging are the future of
>   the Python ecosystem and a lot of progress has been made on making
>   this upgrade seamless.

As described in the recommendations in the end of the article:
`python3 setup.py sdist` should be replaced by `python3 -m build
--sdist`

Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary
Signed-off-by: Robin Jarry 
---
 python/automake.mk | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/automake.mk b/python/automake.mk
index 8854e656a5ae..19d9d91119c2 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -101,10 +101,11 @@ ovs-install-data-local:
rm python/ovs/dirs.py.tmp
 
 python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-   (cd python/ && $(PYTHON3) setup.py sdist)
+   cd python/ && $(PYTHON3) -m build --sdist
 
 pypi-upload: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-   (cd python/ && $(PYTHON3) setup.py sdist && twine upload 
dist/ovs-$(VERSION).tar.gz)
+   cd python/ && $(PYTHON3) -m build --sdist && twine upload 
dist/ovs-$(VERSION).tar.gz
+
 install-data-local: ovs-install-data-local
 
 UNINSTALL_LOCAL += ovs-uninstall-local
-- 
2.41.0

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


[ovs-dev] [PATCH 2/3] python: Use twine to upload sdist package to pypi.org.

2023-08-02 Thread Robin Jarry
setup.py upload is now deprecated. When used, pypi.org returns an error:

> Upload failed (400): Invalid value for blake2_256_digest. Error: Use
> a valid, hex-encoded, BLAKE2 message digest.

Use twine which is the recommended replacement tool to upload on
pypi.org.

Link: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary
Reported-by: Terry Wilson 
Signed-off-by: Robin Jarry 
---
 python/automake.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/automake.mk b/python/automake.mk
index 8b6266de214e..8854e656a5ae 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -104,7 +104,7 @@ python-sdist: $(srcdir)/python/ovs/version.py 
$(ovs_pyfiles) python/ovs/dirs.py
(cd python/ && $(PYTHON3) setup.py sdist)
 
 pypi-upload: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
-   (cd python/ && $(PYTHON3) setup.py sdist upload)
+   (cd python/ && $(PYTHON3) setup.py sdist && twine upload 
dist/ovs-$(VERSION).tar.gz)
 install-data-local: ovs-install-data-local
 
 UNINSTALL_LOCAL += ovs-uninstall-local
-- 
2.41.0

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


[ovs-dev] [PATCH 1/3] python: Move build related code into build-aux.

2023-08-02 Thread Robin Jarry
The python/build folder contents are completely unrelated to the ovs
python bindings. These files are only used during the build for various
subsystems (docs, man pages, code generation, etc.).

Having that folder in that location prevents from running:

cd python && python3 -m build

Which is a way to generate PEP517 compatible source archives and binary
wheel packages.

Move that folder into build-aux. Update PYTHONPATH accordingly.

Link: https://peps.python.org/pep-0517/
Link: https://pypi.org/project/build/
Signed-off-by: Robin Jarry 
---
 Makefile.am   |  6 +++---
 build-aux/automake.mk |  8 
 {python => build-aux}/build/__init__.py   |  0
 {python => build-aux}/build/extract_ofp_fields.py |  0
 {python => build-aux}/build/nroff.py  |  0
 {python => build-aux}/build/soutil.py |  0
 python/automake.mk| 12 
 7 files changed, 11 insertions(+), 15 deletions(-)
 rename {python => build-aux}/build/__init__.py (100%)
 rename {python => build-aux}/build/extract_ofp_fields.py (100%)
 rename {python => build-aux}/build/nroff.py (100%)
 rename {python => build-aux}/build/soutil.py (100%)

diff --git a/Makefile.am b/Makefile.am
index db341504d37f..fca138ea9fd6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -58,7 +58,7 @@ endif
 # foo/__init__.py into an (older) version with plain foo.py, since
 # foo/__init__.pyc will cause Python to ignore foo.py.
 run_python = \
-   PYTHONPATH=$(top_srcdir)/python$(psep)$$PYTHONPATH \
+   
PYTHONPATH=$(top_srcdir)/python$(psep)$(top_srcdir)/build-aux$(psep)$$PYTHONPATH
 \
PYTHONDONTWRITEBYTECODE=yes $(PYTHON3)
 
 ALL_LOCAL =
@@ -151,7 +151,7 @@ ro_shell = printf '\043 Generated automatically -- do not 
modify!-*- buffer-
 
 SUFFIXES += .in
 .in:
-   $(AM_V_GEN)PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) 
$(srcdir)/build-aux/soexpand.py -I$(srcdir) < $< | \
+   $(AM_V_GEN)$(PYTHON3) $(srcdir)/build-aux/soexpand.py -I$(srcdir) < $< 
| \
  $(PYTHON3) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
  sed \
-e 's,[@]PKIDIR[@],$(PKIDIR),g' \
@@ -416,7 +416,7 @@ CLEANFILES += flake8-check
 
 -include manpages.mk
 manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py python/build/soutil.py
-   @PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON3) 
$(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) $(MAN_ROOTS) >$(@F).tmp
+   @$(PYTHON3) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) 
$(MAN_ROOTS) >$(@F).tmp
@if cmp -s $(@F).tmp $@; then \
  touch $@; \
  rm -f $(@F).tmp; \
diff --git a/build-aux/automake.mk b/build-aux/automake.mk
index b9a77a51cfed..b4426d7b23dd 100644
--- a/build-aux/automake.mk
+++ b/build-aux/automake.mk
@@ -1,4 +1,8 @@
 EXTRA_DIST += \
+   build-aux/build/__init__.py \
+   build-aux/build/extract_ofp_fields.py \
+   build-aux/build/nroff.py \
+   build-aux/build/soutil.py \
build-aux/calculate-schema-cksum \
build-aux/cccl \
build-aux/cksum-schema-check \
@@ -13,6 +17,10 @@ EXTRA_DIST += \
build-aux/xml2nroff
 
 FLAKE8_PYFILES += \
+build-aux/build/__init__.py \
+build-aux/build/extract_ofp_fields.py \
+build-aux/build/nroff.py \
+build-aux/build/soutil.py \
 build-aux/dpdkstrip.py \
 build-aux/gen_ofp_field_decoders \
 build-aux/sodepends.py \
diff --git a/python/build/__init__.py b/build-aux/build/__init__.py
similarity index 100%
rename from python/build/__init__.py
rename to build-aux/build/__init__.py
diff --git a/python/build/extract_ofp_fields.py 
b/build-aux/build/extract_ofp_fields.py
similarity index 100%
rename from python/build/extract_ofp_fields.py
rename to build-aux/build/extract_ofp_fields.py
diff --git a/python/build/nroff.py b/build-aux/build/nroff.py
similarity index 100%
rename from python/build/nroff.py
rename to build-aux/build/nroff.py
diff --git a/python/build/soutil.py b/build-aux/build/soutil.py
similarity index 100%
rename from python/build/soutil.py
rename to build-aux/build/soutil.py
diff --git a/python/automake.mk b/python/automake.mk
index 82a50878741a..8b6266de214e 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -63,14 +63,6 @@ ovs_pytests = \
python/ovs/tests/test_odp.py \
python/ovs/tests/test_ofp.py
 
-# These python files are used at build time but not runtime,
-# so they are not installed.
-EXTRA_DIST += \
-   python/build/__init__.py \
-   python/build/extract_ofp_fields.py \
-   python/build/nroff.py \
-   python/build/soutil.py
-
 # PyPI support.
 EXTRA_DIST += \
python/ovs/compat/sortedcontainers/LICENSE \
@@ -88,10 +80,6 @@ PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
 
 FLAKE8_PYFILES += \
$(filter-out python/ovs/compat/% python/ovs/dirs.py,$(PYFILES)) \
-

[ovs-dev] [PATCH 0/3] Python ovsdb bindings distribution improvements

2023-08-02 Thread Robin Jarry
Hi all,

This is a small series to fix the python ovs package publication on
pypi.org.

Robin Jarry (3):
  python: Move build related code into build-aux.
  python: Use twine to upload sdist package to pypi.org.
  python: Use build to generate PEP517 compatible archives.

 Makefile.am |  6 +++---
 build-aux/automake.mk   |  8 
 {python => build-aux}/build/__init__.py |  0
 .../build/extract_ofp_fields.py |  0
 {python => build-aux}/build/nroff.py|  0
 {python => build-aux}/build/soutil.py   |  0
 python/automake.mk  | 17 +++--
 7 files changed, 14 insertions(+), 17 deletions(-)
 rename {python => build-aux}/build/__init__.py (100%)
 rename {python => build-aux}/build/extract_ofp_fields.py (100%)
 rename {python => build-aux}/build/nroff.py (100%)
 rename {python => build-aux}/build/soutil.py (100%)

-- 
2.41.0

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


Re: [ovs-dev] [RFC ovn-heater] OVN scale testing with OpenStack workloads - questionnaire

2023-07-28 Thread Robin Jarry
Hello OVN community,

Following up on my previous email:

Robin Jarry, Jul 26, 2023 at 21:19:
> as discussed yesterday during the community meeting and today on IRC,
> I have created a public form that we can use to aggregate the results.
>
> https://forms.gle/bmZARvrxgfrJvzkn6
>
> No account is required to fill in the form and the results are
> anonymous.

The form is now live.

If you run one (or several) OpenStack cluster(s) using OVN, please do
take the time to respond to this survey.

All results are publicly available and will be added to this spreadsheet
automatically as responses are provided:

https://docs.google.com/spreadsheets/d/1HMAhy0zQDhhD59PNJWeS3PWCrVmNkAkwo-BrqDay4FU

Thanks!

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


Re: [ovs-dev] [RFC ovn-heater] OVN scale testing with OpenStack workloads - questionnaire

2023-07-26 Thread Robin Jarry
Hi all,

as discussed yesterday during the community meeting and today on IRC,
I have created a public form that we can use to aggregate the results.

https://forms.gle/bmZARvrxgfrJvzkn6

No account is required to fill in the form and the results are
anonymous.

Please hold off before providing any responses. It is planned to do
a final review of all questions during the OVN IRC meeting tomorrow
(Thursday, July 26 2023, 17:15 UTC) before making it official.

https://www.ovn.org/en/#irc-meetings

All results are publicly available in various formats:

Web page:
https://docs.google.com/spreadsheets/d/e/2PACX-1vSR9TrcGLg1WO327Uy1ihAeJaACfnJSgBl_JcTez0mjYdFZpgF9NAUPe6fod0nyI5_fatJ7_GSre_I3/pubhtml?gid=1526075983=true
CSV:
https://docs.google.com/spreadsheets/d/e/2PACX-1vSR9TrcGLg1WO327Uy1ihAeJaACfnJSgBl_JcTez0mjYdFZpgF9NAUPe6fod0nyI5_fatJ7_GSre_I3/pub?gid=1526075983=true=csv
TSV:
https://docs.google.com/spreadsheets/d/e/2PACX-1vSR9TrcGLg1WO327Uy1ihAeJaACfnJSgBl_JcTez0mjYdFZpgF9NAUPe6fod0nyI5_fatJ7_GSre_I3/pub?gid=1526075983=true=tsv

Let me know if you have any remarks, corrections or questions regarding
this form. It was based on the document created by Dumitru:

https://docs.google.com/document/d/151saO5a5PmCt7cIZQ7DkgvU755od76BlN2bKjXc1n08/edit?usp=sharing

Cheers,

-- 
Robin

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


Bug#1040907: aerc: Freeze when pasting to body on email compose

2023-07-14 Thread Robin Jarry
Nilesh Patra, Jul 14, 2023 at 20:35:
> On Fri, Jul 14, 2023 at 08:13:16PM +0200, Robin Jarry wrote:
> > Nilesh Patra, Jul 14, 2023 at 19:59:
> > > There you go
> > >
> > > https://pb.envs.net/?816fb8c61575e2ba#4kD2xS4wJMGrsCd8tBhPfqHDJZ7qeM6qERaobgoYj46F
> > 
> > Thanks a lot that helps. It looks like there is a deadlock due to two
> > goroutines trying to interact with the terminal:
>
> I'm going ahead with an upload in debian with this patch backported.
> From a distro pov I consider this a major breakeage. Given that aerc is
> designed to facilitate patch workflow as a goal, it makes it useless if
> I can't paste more than 4-5 lines in one go.
>
> If you disagree, feel free to revert the upload. You have the rights.

Sure thing. It is certainly less risky than tagging a release in a rush.



Bug#1040907: aerc: Freeze when pasting to body on email compose

2023-07-14 Thread Robin Jarry
Nilesh Patra, Jul 14, 2023 at 19:59:
> There you go
>
> https://pb.envs.net/?816fb8c61575e2ba#4kD2xS4wJMGrsCd8tBhPfqHDJZ7qeM6qERaobgoYj46F

Thanks a lot that helps. It looks like there is a deadlock due to two
goroutines trying to interact with the terminal:

goroutine 1 [sync.Mutex.Lock]:

sync.(*Mutex).Lock(...)
sync/mutex.go:90
git.sr.ht/~rockorager/tcell-term.(*VT).HandleEvent(0xc000158c80, {0xc5ce80?, 
0xc0007fff60?})
git.sr.ht/~rockorager/tcell-term@v0.7.1/vt.go:452 +0x6d fp=0xc000a2db40 
sp=0xc000a2dad8 pc=0x9a72cd
git.sr.ht/~rjarry/aerc/widgets.(*Terminal).Event(0xc000b26280?, {0xc5ce80?, 
0xc0007fff60?})
git.sr.ht/~rjarry/aerc/widgets/terminal.go:189 +0x72 fp=0xc000a2db68 
sp=0xc000a2db40 pc=0x9e72d2
git.sr.ht/~rjarry/aerc/widgets.(*Composer).Event(0xc000a2dc48?, {0xc5ce80?, 
0xc0007fff60?})
git.sr.ht/~rjarry/aerc/widgets/compose.go:669 +0xcf fp=0xc000a2dbb8 
sp=0xc000a2db68 pc=0x9c43af
git.sr.ht/~rjarry/aerc/widgets.(*Aerc).Event(0xc062e8?, {0xc5ce80?, 
0xc0007fff60?})
git.sr.ht/~rjarry/aerc/widgets/aerc.go:316 +0xc3 fp=0xc000a2dc30 
sp=0xc000a2dbb8 pc=0x9ba1a3
git.sr.ht/~rjarry/aerc/lib/ui.(*UI).HandleEvent(0xc4a040, {0xc5ce80?, 
0xc0007fff60})
git.sr.ht/~rjarry/aerc/lib/ui/ui.go:150 +0x162 fp=0xc000a2dc80 
sp=0xc000a2dc30 pc=0x6a79c2
main.main()
git.sr.ht/~rjarry/aerc/main.go:266 +0xb45 fp=0xc000a2df80 
sp=0xc000a2dc80 pc=0xa32fe5

goroutine 102 [chan send]:

git.sr.ht/~rjarry/aerc/lib/ui.QueueRedraw(...)
git.sr.ht/~rjarry/aerc/lib/ui/ui.go:30
git.sr.ht/~rjarry/aerc/widgets.(*Terminal).HandleEvent(0x8?, {0xc5c140?, 
0xc00094e850?})
git.sr.ht/~rjarry/aerc/widgets/terminal.go:168 +0x9c fp=0xc000435e38 
sp=0xc000435e10 pc=0x9e71bc
git.sr.ht/~rjarry/aerc/widgets.(*Terminal).HandleEvent-fm({0xc5c140?, 
0xc00094e850?})
:1 +0x39 fp=0xc000435e60 sp=0xc000435e38 pc=0x9e9fb9
git.sr.ht/~rockorager/tcell-term.(*VT).postEvent(...)
git.sr.ht/~rockorager/tcell-term@v0.7.1/vt.go:414
git.sr.ht/~rockorager/tcell-term.(*VT).update(0xc000158c80, {0xb119c0?, 
0xc000b24940?})
git.sr.ht/~rockorager/tcell-term@v0.7.1/vt.go:195 +0x489 
fp=0xc000435f80 sp=0xc000435e60 pc=0x9a50e9
git.sr.ht/~rockorager/tcell-term.(*VT).Start.func1()
git.sr.ht/~rockorager/tcell-term@v0.7.1/vt.go:165 +0x2d fp=0xc000435fe0 
sp=0xc000435f80 pc=0x9a4b2d
runtime.goexit()
runtime/asm_amd64.s:1598 +0x1 fp=0xc000435fe8 sp=0xc000435fe0 
pc=0x469781
created by git.sr.ht/~rockorager/tcell-term.(*VT).Start
git.sr.ht/~rockorager/tcell-term@v0.7.1/vt.go:155 +0x33c

It looks like a known issue that has been fixed by this commit:

https://git.sr.ht/~rjarry/aerc/commit/916bca33ea6cf2530117071fdd9b7b15e00e2f29

Not released yet.



Bug#1040907: aerc: Freeze when pasting to body on email compose

2023-07-14 Thread Robin Jarry
If you manage to reproduce this easily, could you try the following:

1) aerc >trace.log 2>&1
2) Compose a message reproduce the freeze.
3) From another terminal, kill aerc with SIGQUIT.

After aerc has died, share the trace.log file contents. Ideally via
paste service, lists.sr.ht bounce emails that have attachments.

Thanks



Bug#1040907: aerc: Freeze when pasting to body on email compose

2023-07-14 Thread Robin Jarry
Nilesh Patra, Jul 14, 2023 at 19:14:
> Are you using debian though?

No, I am on Fedora 38.

Linux ringo 6.3.11-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Sun Jul  2 13:17:31 
UTC 2023 x86_64 GNU/Linux
go version go1.20.5 linux/amd64
sway version 1.8.1
foot version: 1.14.0 -pgo +ime +graphemes -assertions
aerc 0.15.2-96-gae07f640371e +notmuch (go1.20.5 amd64 linux)
NVIM v0.9.1
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
Compilation: /usr/bin/gcc -O2 -g -Og -g -Wall -Wextra -pedantic 
-Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion 
-Wvla -Wdouble-promotion -Wmissing-noreturn -Wmissing-format-attribute 
-Wmissing-prototypes -fno-common -Wno-unused-result -Wimplicit-fallthrough 
-fdiagnostics-color=auto -fstack-protector-strong -DUNIT_TESTING 
-DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -I/usr/include/luajit-2.1 
-I/usr/include -I/usr/include/luv 
-I/builddir/build/BUILD/neovim-0.9.1/redhat-linux-build/src/nvim/auto 
-I/builddir/build/BUILD/neovim-0.9.1/redhat-linux-build/include 
-I/builddir/build/BUILD/neovim-0.9.1/redhat-linux-build/cmake.config 
-I/builddir/build/BUILD/neovim-0.9.1/src -I/usr/include -I/usr/include 
-I/usr/include -I/usr/include -I/usr/include -I/usr/include -I/usr/include



Bug#1040907: aerc: Freeze when pasting to body on email compose

2023-07-14 Thread Robin Jarry
pelle, Jul 14, 2023 at 18:23:
> > Robin and I are both on wayland...I'm guessing with xclip you are on
> > X? I wonder if that comes into play at all?
>
> I'm having the issue in Sway. I've noticed that the freeze happens in
> vim when pasting with `CTRL+V` or `SHIFT+INSERT` while pasting works
> fine with `p`.

I assume that you enter insert mode before pasting with `CTRL+V` or
`SHIFT+INSERT`. Tim, could it be a race with tcell term?



Bug#1040907: aerc: Freeze when pasting to body on email compose

2023-07-14 Thread Robin Jarry
Nilesh Patra, Jul 13, 2023 at 19:27:
> Hi,
>
> On Wed, 12 Jul 2023 11:09:03 +0200 Pelle  wrote:
> > Package: aerc
> > Version: 0.15.2-1
> > Severity: normal
> > 
> > I find that aerc almost always freezes when pasting into the compose area.
> > 
> > It can easily be reproduced here:
> >* Open aerc.
> >* Compose an email.
> >* Move cursor from headers onto the email body compose area.
> >* In another window, open a text editor, enter "hello world" 100 times 
> > and copy it.
> >* Back in aerc, paste into the compose area by `SHIFT+INSERT`
> >* Some of the text will be pasted, but then aerc will get stuck, 
> > sometimes like "hell|"
> >* Aerc stops responding to any input, including `CTRL+C`
> > 
> > I have tested an confirmed this is an issue whether using vim or nvim text 
> > editor, and whether using kitty, alacritty or foot terminal emulator.
>
> Thanks for reporting this, and I can confirm the behavior. @Robin, is
> this an issue known to you as upstream?
> Could you please take a look at this?

Hi Nilesh,

I cannot reproduce this but maybe Tim can have a look.

What editor is this?



Re: [ovs-dev] [PATCH v14] netdev-dpdk: Add custom rx-steering configuration.

2023-07-10 Thread Robin Jarry
Ilya Maximets, Jul 10, 2023 at 15:48:
> In order to speed up the process a bit, proposing a following incremental
> change:
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 27b2fa5e0..aa87ee546 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2074,23 +2074,23 @@ dpdk_set_rx_steer_config(struct netdev *netdev, 
> struct netdev_dpdk *dev,
>  const char *arg = smap_get_def(args, "rx-steering", "rss");
>  uint64_t flags = 0;
>  
> -if (nullable_string_is_equal(arg, "rss+lacp")) {
> +if (!strcmp(arg, "rss+lacp")) {
>  flags = DPDK_RX_STEER_LACP;
> -} else if (!nullable_string_is_equal(arg, "rss")) {
> -VLOG_WARN_BUF(errp, "%s options:rx-steering "
> +} else if (strcmp(arg, "rss")) {
> +VLOG_WARN_BUF(errp, "%s: options:rx-steering "
>"unsupported parameter value '%s'",
>netdev_get_name(netdev), arg);
>  }
>  
>  if (flags && dev->type != DPDK_DEV_ETH) {
> -VLOG_WARN_BUF(errp, "%s options:rx-steering "
> +VLOG_WARN_BUF(errp, "%s: options:rx-steering "
>"is only supported on ethernet ports",
>netdev_get_name(netdev));
>  flags = 0;
>  }
>  
>  if (flags && netdev_is_flow_api_enabled()) {
> -VLOG_WARN_BUF(errp, "%s options:rx-steering "
> +VLOG_WARN_BUF(errp, "%s: options:rx-steering "
>"is incompatible with hw-offload",
>netdev_get_name(netdev));
>  flags = 0;
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 2f756b1b7..01408e90a 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3539,9 +3539,9 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 
> type=patch options:peer=p1 \
>
>  
>  
> -  If the user has already configured multiple
> -  options:n_rxq on the port, an additional one will be
> -  allocated for the specified protocols. Even if the hardware cannot
> +  If the user has already configured multiple  +  column="options" key="n_rxq" /> on the port, an additional one will
> +  be allocated for the specified protocols. Even if the hardware 
> cannot
>satisfy the requested number of requested Rx queues, the last Rx
>queue will be used. If only one Rx queue is available or if the
>hardware does not support the rte_flow matchers/actions required to
> @@ -3551,10 +3551,14 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 
> type=patch options:peer=p1 \
>  
>This feature is mutually exclusive with
>
> -  as it may conflict with the offloaded RTE flows. If both are 
> enabled,
> +  as it may conflict with the offloaded flows. If both are enabled,
>rx-steering will fall back to default rss
>mode.
>  
> +
> +  This option is only applicable to interfaces with type
> +  dpdk.
> +
>
>  
> ---
>
> If that looks good to you, I could fold it in while applying the patch.
> What do you think?
>
> Best regards, Ilya Maximets.

Yes, that'll be faster. Thanks!

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


[ovs-dev] [PATCH v14] netdev-dpdk: Add custom rx-steering configuration.

2023-07-04 Thread Robin Jarry
ffic explicitly. It only guarantees that some
protocols have a lower chance of being dropped because the PMD cores
cannot keep up with regular traffic.

The choice of protocols is limited on purpose. This is not meant to be
configurable by users. Some limited configurability could be considered
in the future but it would expose to more potential issues if users are
accidentally redirecting all traffic in the isolated queue.

Cc: Anthony Harivel 
Cc: Christophe Fontaine 
Cc: David Marchand 
Cc: Ilya Maximets 
Signed-off-by: Robin Jarry 
Acked-by: Kevin Traynor 
---

Notes:
v13 -> v14:

* rebased on b4c7009c20e7 ("system-offloads-traffic.at: Add vxlan gbp 
offload test.")

* renamed "RTE flow" to "rte_flow" (or "the rte_flow API" depending on
  the context).

* removed ambiguity about rx-steering being "disabled" whereas it always
  falls back to default rss mode.

* fixed code style:
 - nullable_string_is_equal instead of strcmp
 - default "rx-steering" parameter to "rss" to simplify checks
 - no parentheses for sizeof argument

* fixed typos

 Documentation/topics/dpdk/phy.rst |  87 +
 NEWS  |   3 +
 lib/netdev-dpdk.c | 315 +-
 vswitchd/vswitch.xml  |  40 
 4 files changed, 442 insertions(+), 3 deletions(-)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 4b0fe8dded3a..f66b106c46af 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -131,6 +131,93 @@ possible with DPDK acceleration. It is possible to 
configure multiple Rx queues
 for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For
 information on configuring PMD threads, refer to :doc:`pmd`.
 
+Traffic Rx Steering
+---
+
+.. warning:: This feature is experimental.
+
+Some control protocols are used to maintain link status between forwarding
+engines. In SDN environments, these packets share the same physical network
+with the user data traffic.
+
+When the system is not sized properly, the PMD threads may not be able to
+process all incoming traffic from the configured Rx queues. When a signaling
+packet of such protocols is dropped, it can cause link flapping, worsening the
+situation.
+
+Some physical NICs can be programmed to put these protocols in a dedicated
+hardware Rx queue using the rte_flow__ API.
+
+__ https://doc.dpdk.org/guides-22.11/prog_guide/rte_flow.html
+
+.. warning::
+
+   This feature is not compatible with all NICs. Refer to the DPDK
+   `compatibilty matrix`__ and vendor documentation for more details.
+
+   __ https://doc.dpdk.org/guides-22.11/nics/overview.html
+
+Rx steering must be enabled for specific protocols per port. The
+``rx-steering`` option takes one of the following values:
+
+``rss``
+   Do regular RSS on all configured Rx queues. This is the default behaviour.
+
+``rss+lacp``
+   Do regular RSS on all configured Rx queues. An extra Rx queue is configured
+   for LACP__ packets (ether type ``0x8809``).
+
+   __ https://www.ieee802.org/3/ad/public/mar99/seaman_1_0399.pdf
+
+Example::
+
+   $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0 options:n_rxq=2 \
+options:rx-steering=rss+lacp
+
+.. note::
+
+   If multiple Rx queues are already configured, regular hash-based RSS
+   (Receive Side Scaling) queue balancing is done on all but the extra Rx
+   queue.
+
+.. tip::
+
+   You can check if Rx steering is supported on a port with the following
+   command::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rss_queues="0-1", rx_steering_queue="2"}
+
+   This will also show in ``ovs-vswitchd.log``::
+
+  INFO|dpdk-p0: rx-steering: redirecting lacp traffic to queue 2
+  INFO|dpdk-p0: rx-steering: applying rss on queues 0-1
+
+   If the hardware does not support redirecting the specified protocols to
+   a dedicated queue, it will be explicit::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rx_steering=unsupported}
+
+   More details can often be found in ``ovs-vswitchd.log``::
+
+  WARN|dpdk-p0: rx-steering: failed to add lacp flow: Unsupported pattern
+
+To disable Rx steering on a port, use the following command::
+
+   $ ovs-vsctl remove Interface dpdk-p0 options rx-steering
+
+You can see that it has been disabled in ``ovs-vswitchd.log``::
+
+   INFO|dpdk-p0: rx-steering: default rss
+
+.. warning::
+
+   This feature is mutually exclusive with ``other-config:hw-offload`` as it
+   may conflict with the offloaded flows. If both are enabled, ``rx-steering``
+   will fall back to default ``rss`` mode.
+
 .. _dpdk-phy-flow-control:
 
 Flow Control
diff --git a/NEWS b/NEWS
index 6a990c92151a..eedaad07b13b 100644
--- a/NEWS
+++ b/NE

Re: [ovs-dev] Scale testing OVN with ovn-heater for OpenStack use cases

2023-07-04 Thread Robin Jarry
Dumitru Ceara, Jul 04, 2023 at 16:16:
> Given that July 5th is tomorrow I'm assuming there won't be a community
> meeting as it seems quite short notice.
>
> Frode, is Tuesday July 11th 13:30 UTC still an option for you?  If I'm
> not wrong this was the slot that worked for everyone that replied.

July 11th 13:30 UTC works for me if that matters :)

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


[ovs-dev] [PATCH v13] netdev-dpdk: Add custom rx-steering configuration.

2023-07-01 Thread Robin Jarry
er chance of being dropped because the PMD cores
cannot keep up with regular traffic.

The choice of protocols is limited on purpose. This is not meant to be
configurable by users. Some limited configurability could be considered
in the future but it would expose to more potential issues if users are
accidentally redirecting all traffic in the isolated queue.

Cc: Anthony Harivel 
Cc: Christophe Fontaine 
Cc: David Marchand 
Cc: Ilya Maximets 
Signed-off-by: Robin Jarry 
Acked-by: Kevin Traynor 
---

Notes:
v12 -> v13:

* Add "rx_steering" and adjust "requested_n_rxq" to dev->user_n_rxq in
  netdev_dpdk_get_config().

* Add comment about not resetting RETA in dpdk_rx_steer_unconfigure().

* Call netdev_is_flow_api_enabled() in dpdk_set_rx_steer_config() to
  check if hw-offload is enabled instead of tinkering with internal API.

* s/VLOG_DBG/VLOG_WARN/ when a flow cannot be unconfigured.

* Add missing empty lines.

* Rebased on c2433bdfc0d2 ("dpif-netdev: Lockless meters.").

 Documentation/topics/dpdk/phy.rst |  87 +
 NEWS  |   3 +
 lib/netdev-dpdk.c | 315 +-
 vswitchd/vswitch.xml  |  39 
 4 files changed, 441 insertions(+), 3 deletions(-)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 4b0fe8dded3a..8f57bf4efe2d 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -131,6 +131,93 @@ possible with DPDK acceleration. It is possible to 
configure multiple Rx queues
 for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For
 information on configuring PMD threads, refer to :doc:`pmd`.
 
+Traffic Rx Steering
+---
+
+.. warning:: This feature is experimental.
+
+Some control protocols are used to maintain link status between forwarding
+engines. In SDN environments, these packets share the same physical network
+than the user data traffic.
+
+When the system is not sized properly, the PMD threads may not be able to
+process all incoming traffic from the configured Rx queues. When a signaling
+packet of such protocols is dropped, it can cause link flapping, worsening the
+situation.
+
+Some physical NICs can be programmed to put these protocols in a dedicated
+hardware Rx queue using the rte_flow__ API.
+
+__ https://doc.dpdk.org/guides-22.11/prog_guide/rte_flow.html
+
+.. warning::
+
+   This feature is not compatible with all NICs. Refer to the DPDK
+   `compatibilty matrix`__ and vendor documentation for more details.
+
+   __ https://doc.dpdk.org/guides-22.11/nics/overview.html
+
+Rx steering must be enabled for specific protocols per port. The
+``rx-steering`` option takes one of the following values:
+
+``rss``
+   Do regular RSS on all configured Rx queues. This is the default behaviour.
+
+``rss+lacp``
+   Do regular RSS on all configured Rx queues. An extra Rx queue is configured
+   for LACP__ packets (ether type ``0x8809``).
+
+   __ https://www.ieee802.org/3/ad/public/mar99/seaman_1_0399.pdf
+
+Example::
+
+   $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0 options:n_rxq=2 \
+options:rx-steering=rss+lacp
+
+.. note::
+
+   If multiple Rx queues are already configured, regular hash-based RSS
+   (Receive Side Scaling) queue balancing is done on all but the extra Rx
+   queue.
+
+.. tip::
+
+   You can check if Rx steering is supported on a port with the following
+   command::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rss_queues="0-1", rx_steering_queue="2"}
+
+   This will also show in ``ovs-vswitchd.log``::
+
+  INFO|dpdk-p0: rx-steering: redirecting lacp traffic to queue 2
+  INFO|dpdk-p0: rx-steering: applying rss on queues 0-1
+
+   If the hardware does not support redirecting the specified protocols to
+   a dedicated queue, it will be explicit::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rx_steering=unsupported}
+
+   More details can often be found in ``ovs-vswitchd.log``::
+
+  WARN|dpdk-p0: rx-steering: failed to add lacp flow: Unsupported pattern
+
+To disable Rx steering on a port, use the following command::
+
+   $ ovs-vsctl remove Interface dpdk-p0 options rx-steering
+
+You can see that it has been disabled in ``ovs-vswitchd.log``::
+
+   INFO|dpdk-p0: rx-steering: disabled
+
+.. warning::
+
+   This feature is mutually exclusive with ``other_options:hw-offload`` as it
+   may conflict with the offloaded RTE flows. If both are enabled,
+   ``rx-steering`` will be forcibly disabled.
+
 .. _dpdk-phy-flow-control:
 
 Flow Control
diff --git a/NEWS b/NEWS
index 0b5dc3db15c8..ce356f45d1df 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,9 @@ Post-v3.1.0
  * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
with the --hw-ra

Re: [ovs-dev] Scale testing OVN with ovn-heater for OpenStack use cases

2023-06-30 Thread Robin Jarry
Frode Nordahl, Jun 30, 2023 at 09:25:
> Below are some proposals for dates and times, please respond with a
> prioritized list of what works best for you and we'll try to land on a
> single date/time for a first meeting:
> Wednesday July 5th 13:00 UTC
> Tuesday July 11th 13:30 UTC
> Wednesday July 12th 8:30 UTC

Hi Frode,

thanks a lot for organizing this. I will be interested in joining as
well. All three time slots are suitable for me.

Cheers,

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


Re: [ovs-dev] [PATCH v12] netdev-dpdk: Add custom rx-steering configuration.

2023-06-30 Thread Robin Jarry
Hi Ilya,

Ilya Maximets, Jun 29, 2023 at 23:57:
> > +if (flags && ovsrcu_get(void *, >hw_info.offload_data)) {
>
> We probbaly shouldn't use the offload_data outside of netdev-offload
> modules.  Simply checking netdev_is_flow_api_enabled() should be
> enough.  Since both features require rte_flow oflload it's unlikely
> that rx-steering can work if flow api is enabled globally.  And
> netdev-offload-dpdk doesn't really check device capabilities.

OK, I'll change that for v13.

> >  static int
> >  netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
> > char **errp)
>
> Aside from the set_config, we also need a get_config callback update.
> get_config() should return everything that set_config() set.  i.e.
> we should return the "rx-steering": "rss+lacp", if user configured it.
> The output will be visible in the dpif/show.

Hmm, I had missed the get_config callback. I have added something for
get status but I assume it is different.

> > +memset(reta_conf, 0, sizeof(reta_conf));
> > +for (uint16_t i = 0; i < info.reta_size; i++) {
> > +uint16_t idx = i / RTE_ETH_RETA_GROUP_SIZE;
> > +uint16_t shift = i % RTE_ETH_RETA_GROUP_SIZE;
>
> An empty line here.
>
> > +reta_conf[idx].mask |= 1ULL << shift;
> > +reta_conf[idx].reta[shift] = i % rss_n_rxq;
> > +}
>
> And here.

Ack.

> > +for (int i = 0; i < dev->rx_steer_flows_num; i++) {
> > +set_error(, RTE_FLOW_ERROR_TYPE_NONE);
> > +if (rte_flow_destroy(dev->port_id, dev->rx_steer_flows[i], 
> > )) {
> > +VLOG_DBG("%s: rx-steering: failed to destroy flow: %s",
>
> Maybe a WARN ?

Will change that.

> So, this function destroys all the flows, OK.
> But we also need to restore the redirection table, right?  Or is it handled
> somehow in the driver when number of queues changed?  From my experience,
> drivers tend to not restore this kind of configuration even on device detach
> and it gets preserved even across OVS restarts.

>From what I saw during testing, the RETA is reset every time the device
is reset. In the first versions of this patch, I did reset the table but
I seem to remember some discussions with Kevin and/or David where we
concluded that it was redundant. I will check again to make sure before
sending a respin.

Thanks for the review.

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


Re: [ovs-dev] [PATCH v12] netdev-dpdk: Add custom rx-steering configuration.

2023-06-22 Thread Robin Jarry
Hi Kevin,

Kevin Traynor, Jun 22, 2023 at 15:06:
> Hi Robin,
>
> As discussed offline, the issue is fixed in v12. In the case where 
> offload has failed on device A, and device B triggers set_config the 
> set_config for device B will see that device B has the correct number of 
> queues (user_n_rxq), so there will not be a reconfigure of *that* 
> device. I tested this by faking some failed offloads with gdb.
>
> If a reconfigure for device A does occur for some other reason, then it 
> will attempt to apply the config again, (presumably) the offload will 
> fail again and the normal retry occurs without offload. This seems the 
> correct behaviour to me.

Somehow I did assume that netdev_dpdk_reconfigure() would be called for
all ports regardless if their configuration had changed. Good to know
that the problem is now fixed.

> For v12, I also tested some basic increasing decreasing rxqs, enabling 
> hwol and it's working as expected.
>
> thanks,
> Kevin.
>
> Acked-by: Kevin Traynor 

Thanks a lot!

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


[ovs-dev] [PATCH v12] netdev-dpdk: Add custom rx-steering configuration.

2023-06-21 Thread Robin Jarry
er chance of being dropped because the PMD cores
cannot keep up with regular traffic.

The choice of protocols is limited on purpose. This is not meant to be
configurable by users. Some limited configurability could be considered
in the future but it would expose to more potential issues if users are
accidentally redirecting all traffic in the isolated queue.

Cc: Anthony Harivel 
Cc: Christophe Fontaine 
Cc: David Marchand 
Cc: Kevin Traynor 
Cc: Ilya Maximets 
Signed-off-by: Robin Jarry 
---
v11 -> 12:

* Rebased on c91867030234 ("MAINTAINERS: Add Eelco Chaudron.")

* Added dev->user_n_rxq back. It is required to handle reconfiguration
  triggered by non-user config events.

* Fixed alignment for dpdk_set_rx_steer_config() continuation line.

* Removed last trace of "cp protection".

Unfortunately, adding dev->user_n_rxq back does not fix the issue
reported by Kevin on v11:

> If rx-steering is set for the port and the flow has previously not been
> able to be offloaded, the dev->requested_n_rxq will always be different
> than the netdev->n_rxq. That means this device will do a  reconfigure
> anytime there is a config change on any device.
>
> e.g. If rx sterring on device A and device A cannot offload flows (this
> is acceptable). Any config change to device B will result in reconfigure
> of device A, not based on flags but based on num of rxqs.

I don't know how to fix this. Since it will only occur if the hardware
does not support rte flow offloading. This quirk can simply be "fixed"
by removing the unsupported rx-steering configuration.

 Documentation/topics/dpdk/phy.rst |  87 +
 NEWS  |   3 +
 lib/netdev-dpdk.c | 303 +-
 vswitchd/vswitch.xml  |  39 
 4 files changed, 430 insertions(+), 2 deletions(-)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 4b0fe8dded3a..8f57bf4efe2d 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -131,6 +131,93 @@ possible with DPDK acceleration. It is possible to 
configure multiple Rx queues
 for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For
 information on configuring PMD threads, refer to :doc:`pmd`.
 
+Traffic Rx Steering
+---
+
+.. warning:: This feature is experimental.
+
+Some control protocols are used to maintain link status between forwarding
+engines. In SDN environments, these packets share the same physical network
+than the user data traffic.
+
+When the system is not sized properly, the PMD threads may not be able to
+process all incoming traffic from the configured Rx queues. When a signaling
+packet of such protocols is dropped, it can cause link flapping, worsening the
+situation.
+
+Some physical NICs can be programmed to put these protocols in a dedicated
+hardware Rx queue using the rte_flow__ API.
+
+__ https://doc.dpdk.org/guides-22.11/prog_guide/rte_flow.html
+
+.. warning::
+
+   This feature is not compatible with all NICs. Refer to the DPDK
+   `compatibilty matrix`__ and vendor documentation for more details.
+
+   __ https://doc.dpdk.org/guides-22.11/nics/overview.html
+
+Rx steering must be enabled for specific protocols per port. The
+``rx-steering`` option takes one of the following values:
+
+``rss``
+   Do regular RSS on all configured Rx queues. This is the default behaviour.
+
+``rss+lacp``
+   Do regular RSS on all configured Rx queues. An extra Rx queue is configured
+   for LACP__ packets (ether type ``0x8809``).
+
+   __ https://www.ieee802.org/3/ad/public/mar99/seaman_1_0399.pdf
+
+Example::
+
+   $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0 options:n_rxq=2 \
+options:rx-steering=rss+lacp
+
+.. note::
+
+   If multiple Rx queues are already configured, regular hash-based RSS
+   (Receive Side Scaling) queue balancing is done on all but the extra Rx
+   queue.
+
+.. tip::
+
+   You can check if Rx steering is supported on a port with the following
+   command::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rss_queues="0-1", rx_steering_queue="2"}
+
+   This will also show in ``ovs-vswitchd.log``::
+
+  INFO|dpdk-p0: rx-steering: redirecting lacp traffic to queue 2
+  INFO|dpdk-p0: rx-steering: applying rss on queues 0-1
+
+   If the hardware does not support redirecting the specified protocols to
+   a dedicated queue, it will be explicit::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rx_steering=unsupported}
+
+   More details can often be found in ``ovs-vswitchd.log``::
+
+  WARN|dpdk-p0: rx-steering: failed to add lacp flow: Unsupported pattern
+
+To disable Rx steering on a port, use the following command::
+
+   $ ovs-vsctl remove Interface dpdk-p0 options rx-steering
+
+You can see t

[ovs-dev] [PATCH v4] Add editorconfig file.

2023-06-19 Thread Robin Jarry
EditorConfig is a file format and collection of text editor plugins for
maintaining consistent coding styles between different editors and IDEs.

Initialize the file following the coding rules in
Documentation/internals/contributing/coding-style.rst and add exceptions
declared in build-aux/initial-tab-allowed-files. Only enforce rules for
*.c and *.h files. Other files should use the default indenting rules
from text editors.

In order for this file to be taken into account (unless they use an
editor with built-in EditorConfig support), developers will have to
install a plugin.

Notes:

* All matching rules are considered. The last matching rule's properties
  will override the previous ones.
* The max_line_length property is only supported by a limited number of
  EditorConfig plugins. It will be ignored if unsupported.

Link: https://editorconfig.org/
Link: https://github.com/editorconfig/editorconfig-emacs
Link: https://github.com/editorconfig/editorconfig-vim
Link: 
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Signed-off-by: Robin Jarry 
Cc: Mike Pattrick 
Cc: Eelco Chaudron 
Cc: Ilya Maximets 
---

Notes:
v4:

* Listed files that use tabs more restrictively.
* I assumed that every header under include/linux uses tabs since this
  is the coding style of the kernel.
* Also, any header named include/sparse/rte_*.h most likely comes from
  DPDK which also uses tabs for indentation.

 .editorconfig | 48 
 Makefile.am   |  1 +
 2 files changed, 49 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index ..685c7275005f
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,48 @@
+# See https://editorconfig.org/ for syntax reference.
+
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+charset = utf-8
+
+[*.{c,h}]
+indent_style = space
+indent_size = 4
+max_line_length = 79
+
+[include/linux/**.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[include/sparse/rte_*.h]
+indent_style = tab
+tab_width = 8
+
+[include/windows/getopt.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[include/windows/netinet/{icmp6,ip6}.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/getopt_long.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/sflow*.{c,h}]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/strsep.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
diff --git a/Makefile.am b/Makefile.am
index df9c33dfe631..db341504d37f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST = \
.ci/osx-build.sh \
.ci/osx-prepare.sh \
.cirrus.yml \
+   .editorconfig \
.github/workflows/build-and-test.yml \
appveyor.yml \
boot.sh \
-- 
2.41.0

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


Re: [ovs-dev] [PATCH RESEND v11] netdev-dpdk: Add custom rx-steering configuration.

2023-06-19 Thread Robin Jarry
Kevin Traynor, Jun 15, 2023 at 12:53:
> >   new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
> > +if (dev->requested_rx_steer_flags != 0) {
> > +new_n_rxq += 1;
>
> If rx-steering is set for the port and the flow has previously not been 
> able to be offloaded, the dev->requested_n_rxq will always be different 
> than the netdev->n_rxq. That means this device will do a  reconfigure 
> anytime there is a config change on any device.
>
> e.g. If rx sterring on device A and device A cannot offload flows (this 
> is acceptable). Any config change to device B will result in reconfigure 
> of device A, not based on flags but based on num of rxqs.

This is why I had added an intermediate dev->user_n_rxq field in v10.
I tried to get rid of it as you suggested but it causes this issue.

Also, I think there's another side effect that is actually worse, if the
rte flow offload fails, dev->requested_n_rxq is decremented by one. If
netdev_dpdk_reconfigure() is called without reinitializing
dev->requested_n_rxq (mtu change or some other event), the value will
eventually drop down to zero and then to negative values.

I'll reintroduce user_n_rxq as an intermediate value that cannot be
modified other by than the user so that these problems cannot occur.

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


Re: [ovs-dev] [PATCH v3] Add editorconfig file.

2023-06-14 Thread Robin Jarry
Ilya Maximets, Jun 14, 2023 at 13:21:
> While files are present in tabs-allowed it doesn't mean that
> all of them actually use tabs.  I'm not sure about other
> folders, but at least the windows include folder contains files
> with different styles.
>
> Is it possible to just exclude the folder instead of specifying
> a format?

No but I can list only the files which already contain tabs.

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


[ovs-dev] [PATCH v3] Add editorconfig file.

2023-06-13 Thread Robin Jarry
EditorConfig is a file format and collection of text editor plugins for
maintaining consistent coding styles between different editors and IDEs.

Initialize the file following the coding rules in
Documentation/internals/contributing/coding-style.rst and add exceptions
declared in build-aux/initial-tab-allowed-files. Only enforce rules for
*.c and *.h files. Other files should use the default indenting rules
from text editors.

In order for this file to be taken into account (unless they use an
editor with built-in EditorConfig support), developers will have to
install a plugin.

Notes:

* All matching rules are considered. The last matching rule's properties
  will override the previous ones.
* The max_line_length property is only supported by a limited number of
  EditorConfig plugins. It will be ignored if unsupported.

Link: https://editorconfig.org/
Link: https://github.com/editorconfig/editorconfig-emacs
Link: https://github.com/editorconfig/editorconfig-vim
Link: 
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Signed-off-by: Robin Jarry 
Acked-by: Mike Pattrick 
Acked-by: Eelco Chaudron 
Cc: Ilya Maximets 
---

Notes:
v3: added exceptions for some *.{c,h} files that are using tabs

 .editorconfig | 43 +++
 Makefile.am   |  1 +
 2 files changed, 44 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index ..6d1842ca0f35
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,43 @@
+# See https://editorconfig.org/ for syntax reference.
+
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+charset = utf-8
+
+[*.{c,h}]
+indent_style = space
+indent_size = 4
+max_line_length = 79
+
+[include/linux/**.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[include/sparse/rte_*.h]
+indent_style = tab
+tab_width = 8
+
+[include/windows/**.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/getopt_long.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/sflow*.{c,h}]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/strsep.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
diff --git a/Makefile.am b/Makefile.am
index df9c33dfe631..db341504d37f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST = \
.ci/osx-build.sh \
.ci/osx-prepare.sh \
.cirrus.yml \
+   .editorconfig \
.github/workflows/build-and-test.yml \
appveyor.yml \
boot.sh \
-- 
2.40.1

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


Re: [ovs-dev] [PATCH v2] Add editorconfig file

2023-06-12 Thread Robin Jarry
Ilya Maximets, Jun 12, 2023 at 21:53:
> The build-aux/initial-tab-allowed-files seems to list most of the
> non-conventionally styled files.

I'll have a look there. Thanks.

> Some files with different styles are in the same subtree, so describing
> at the root sounds better.

Ack.

> Well, AFAIU, for example, vim plugin will perform trimming of trailing
> whitespaces in the whole file on save, if trim_trailing_whitespace is set.

Good point. I'll remove that setting.

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


Re: [ovs-dev] [PATCH v2] Add editorconfig file

2023-06-12 Thread Robin Jarry
Hi Ilya,

Ilya Maximets, Jun 12, 2023 at 17:47:
> I'm a bit concerned that this patch is applying the same config
> to all the .c|.h files in the repo while not all of them have
> the same coding style.  The most obvious is that linux headers
> are using tabs, not spaces.  Not so obvious - sFlow implementation
> that was imported as-is from the other source.  There are some
> other examples.

I was not sure what these rules were so I went with what was written in
the docs. Is there a place where these special cases are listed? If not,
could you list them here so they can be made explicit?

We can either put one .editorconfig file per subtree that has different
indenting rules or we can list everything at the root.

> I'm afraid that some editors will try to fix existing code while
> saving unrelated changes.

Editorconfig plugins are not automatic formatters. The only purpose of
these is to automatically configure editors to use the project's
preferences.

Thanks!

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


Re: [ovs-dev] [PATCH v2] Add editorconfig file

2023-06-12 Thread Robin Jarry
Hi Mike, Eelco,

Eelco Chaudron, Jun 12, 2023 at 16:54:
> I also like the idea, but the editoconfig does not seem to bring much
> to the table for C. And it looks like there is not much activity on
> the project. It might be better to look at clang-format, however, this
> is c/c++ specific.
>
> Just my 2c, but we might end up with a configuration file for each
> person’s favorite editor/linter.

The editorconfig project itself does not do much. And since it only
affects line length and indent style, I don't expect there should be
many improvements.

It is less strict that full C formatters like clang-format and
uncrustify. But it is simple enough and allows jumping across multiple
projects which have all different indent rules and not loose your
sanity.

In any case, having a .editorconfig file is not mutually exclusive with
other tools.

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


[ovs-dev] [PATCH RESEND v11] netdev-dpdk: Add custom rx-steering configuration.

2023-06-01 Thread Robin Jarry
ed because the PMD cores
cannot keep up with regular traffic.

The choice of protocols is limited on purpose. This is not meant to be
configurable by users. Some limited configurability could be considered
in the future but it would expose to more potential issues if users are
accidentally redirecting all traffic in the isolated queue.

Cc: Anthony Harivel 
Cc: Christophe Fontaine 
Cc: David Marchand 
Cc: Kevin Traynor 
Cc: Ilya Maximets 
Signed-off-by: Robin Jarry 
---
v11 -> v11 (resend):

* Rebased on 359cabbd6eb2 ("netdev-offload: Fix some typos.")

v10 -> v11:

* Rename the feature from control plane protection to rx steering. The
  following sed expressions where applied in that order:

s/cp-protection/rx-steering/g
s/cp_protection/rx_steering/g
s/CP_PROT/RX_STEER/g
s/cp_prot/rx_steer/g

  The remaining references to "control plane protection" were adjusted
  manually.

* options:rx-steering now takes "rss" or "rss+lacp".

* Remove user_n_rxq as it is not needed with a goto retry and the
  proper boolean logic in netdev_dpdk_reconfigure().

 Documentation/topics/dpdk/phy.rst |  87 +
 NEWS  |   3 +
 lib/netdev-dpdk.c | 292 ++
 vswitchd/vswitch.xml  |  39 
 4 files changed, 421 insertions(+)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 4b0fe8dded3a..8f57bf4efe2d 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -131,6 +131,93 @@ possible with DPDK acceleration. It is possible to 
configure multiple Rx queues
 for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For
 information on configuring PMD threads, refer to :doc:`pmd`.
 
+Traffic Rx Steering
+---
+
+.. warning:: This feature is experimental.
+
+Some control protocols are used to maintain link status between forwarding
+engines. In SDN environments, these packets share the same physical network
+than the user data traffic.
+
+When the system is not sized properly, the PMD threads may not be able to
+process all incoming traffic from the configured Rx queues. When a signaling
+packet of such protocols is dropped, it can cause link flapping, worsening the
+situation.
+
+Some physical NICs can be programmed to put these protocols in a dedicated
+hardware Rx queue using the rte_flow__ API.
+
+__ https://doc.dpdk.org/guides-22.11/prog_guide/rte_flow.html
+
+.. warning::
+
+   This feature is not compatible with all NICs. Refer to the DPDK
+   `compatibilty matrix`__ and vendor documentation for more details.
+
+   __ https://doc.dpdk.org/guides-22.11/nics/overview.html
+
+Rx steering must be enabled for specific protocols per port. The
+``rx-steering`` option takes one of the following values:
+
+``rss``
+   Do regular RSS on all configured Rx queues. This is the default behaviour.
+
+``rss+lacp``
+   Do regular RSS on all configured Rx queues. An extra Rx queue is configured
+   for LACP__ packets (ether type ``0x8809``).
+
+   __ https://www.ieee802.org/3/ad/public/mar99/seaman_1_0399.pdf
+
+Example::
+
+   $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0 options:n_rxq=2 \
+options:rx-steering=rss+lacp
+
+.. note::
+
+   If multiple Rx queues are already configured, regular hash-based RSS
+   (Receive Side Scaling) queue balancing is done on all but the extra Rx
+   queue.
+
+.. tip::
+
+   You can check if Rx steering is supported on a port with the following
+   command::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rss_queues="0-1", rx_steering_queue="2"}
+
+   This will also show in ``ovs-vswitchd.log``::
+
+  INFO|dpdk-p0: rx-steering: redirecting lacp traffic to queue 2
+  INFO|dpdk-p0: rx-steering: applying rss on queues 0-1
+
+   If the hardware does not support redirecting the specified protocols to
+   a dedicated queue, it will be explicit::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rx_steering=unsupported}
+
+   More details can often be found in ``ovs-vswitchd.log``::
+
+  WARN|dpdk-p0: rx-steering: failed to add lacp flow: Unsupported pattern
+
+To disable Rx steering on a port, use the following command::
+
+   $ ovs-vsctl remove Interface dpdk-p0 options rx-steering
+
+You can see that it has been disabled in ``ovs-vswitchd.log``::
+
+   INFO|dpdk-p0: rx-steering: disabled
+
+.. warning::
+
+   This feature is mutually exclusive with ``other_options:hw-offload`` as it
+   may conflict with the offloaded RTE flows. If both are enabled,
+   ``rx-steering`` will be forcibly disabled.
+
 .. _dpdk-phy-flow-control:
 
 Flow Control
diff --git a/NEWS b/NEWS
index cfd430da..fe9209133861 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,9 @@ Post-v3.1.0
  * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
 

[ovs-dev] [PATCH v10] netdev-dpdk: Add custom rx-steering configuration.

2023-05-31 Thread Robin Jarry
ed because the PMD cores
cannot keep up with regular traffic.

The choice of protocols is limited on purpose. This is not meant to be
configurable by users. Some limited configurability could be considered
in the future but it would expose to more potential issues if users are
accidentally redirecting all traffic in the isolated queue.

Cc: Anthony Harivel 
Cc: Christophe Fontaine 
Cc: David Marchand 
Cc: Kevin Traynor 
Cc: Ilya Maximets 
Signed-off-by: Robin Jarry 
---
v10 -> v11:

* Rename the feature from control plane protection to rx steering. The
  following sed expressions where applied in that order:

s/cp-protection/rx-steering/g
s/cp_protection/rx_steering/g
s/CP_PROT/RX_STEER/g
s/cp_prot/rx_steer/g

  The remaining references to "control plane protection" were adjusted
  manually.

* options:rx-steering now takes "rss" or "rss+lacp".

* Remove user_n_rxq as it is not needed with a goto retry and the
  proper boolean logic in netdev_dpdk_reconfigure().

 Documentation/topics/dpdk/phy.rst |  87 +
 NEWS  |   3 +
 lib/netdev-dpdk.c | 292 ++
 vswitchd/vswitch.xml  |  39 
 4 files changed, 421 insertions(+)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 4b0fe8dded3a..8f57bf4efe2d 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -131,6 +131,93 @@ possible with DPDK acceleration. It is possible to 
configure multiple Rx queues
 for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For
 information on configuring PMD threads, refer to :doc:`pmd`.
 
+Traffic Rx Steering
+---
+
+.. warning:: This feature is experimental.
+
+Some control protocols are used to maintain link status between forwarding
+engines. In SDN environments, these packets share the same physical network
+than the user data traffic.
+
+When the system is not sized properly, the PMD threads may not be able to
+process all incoming traffic from the configured Rx queues. When a signaling
+packet of such protocols is dropped, it can cause link flapping, worsening the
+situation.
+
+Some physical NICs can be programmed to put these protocols in a dedicated
+hardware Rx queue using the rte_flow__ API.
+
+__ https://doc.dpdk.org/guides-22.11/prog_guide/rte_flow.html
+
+.. warning::
+
+   This feature is not compatible with all NICs. Refer to the DPDK
+   `compatibilty matrix`__ and vendor documentation for more details.
+
+   __ https://doc.dpdk.org/guides-22.11/nics/overview.html
+
+Rx steering must be enabled for specific protocols per port. The
+``rx-steering`` option takes one of the following values:
+
+``rss``
+   Do regular RSS on all configured Rx queues. This is the default behaviour.
+
+``rss+lacp``
+   Do regular RSS on all configured Rx queues. An extra Rx queue is configured
+   for LACP__ packets (ether type ``0x8809``).
+
+   __ https://www.ieee802.org/3/ad/public/mar99/seaman_1_0399.pdf
+
+Example::
+
+   $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
+options:dpdk-devargs=:01:00.0 options:n_rxq=2 \
+options:rx-steering=rss+lacp
+
+.. note::
+
+   If multiple Rx queues are already configured, regular hash-based RSS
+   (Receive Side Scaling) queue balancing is done on all but the extra Rx
+   queue.
+
+.. tip::
+
+   You can check if Rx steering is supported on a port with the following
+   command::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rss_queues="0-1", rx_steering_queue="2"}
+
+   This will also show in ``ovs-vswitchd.log``::
+
+  INFO|dpdk-p0: rx-steering: redirecting lacp traffic to queue 2
+  INFO|dpdk-p0: rx-steering: applying rss on queues 0-1
+
+   If the hardware does not support redirecting the specified protocols to
+   a dedicated queue, it will be explicit::
+
+  $ ovs-vsctl get interface dpdk-p0 status
+  {..., rx_steering=unsupported}
+
+   More details can often be found in ``ovs-vswitchd.log``::
+
+  WARN|dpdk-p0: rx-steering: failed to add lacp flow: Unsupported pattern
+
+To disable Rx steering on a port, use the following command::
+
+   $ ovs-vsctl remove Interface dpdk-p0 options rx-steering
+
+You can see that it has been disabled in ``ovs-vswitchd.log``::
+
+   INFO|dpdk-p0: rx-steering: disabled
+
+.. warning::
+
+   This feature is mutually exclusive with ``other_options:hw-offload`` as it
+   may conflict with the offloaded RTE flows. If both are enabled,
+   ``rx-steering`` will be forcibly disabled.
+
 .. _dpdk-phy-flow-control:
 
 Flow Control
diff --git a/NEWS b/NEWS
index cfd430da..fe9209133861 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,9 @@ Post-v3.1.0
  * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
with the --hw-rawio-access command line option.  This allows the
process extra pri

[ovs-dev] [PATCH v2] netdev-dpdk: Fix warning with gcc 13.

2023-05-31 Thread Robin Jarry
GCC now reports uninitialized warnings from function return values.

../lib/netdev-dpdk.c: In function 'netdev_dpdk_mempool_configure':
../lib/netdev-dpdk.c:964:22: warning: 'dmp' may be used uninitialized 
[-Wmaybe-uninitialized]
  964 | dev->dpdk_mp = dmp;
  | ~^
../lib/netdev-dpdk.c:854:21: note: 'dmp' was declared here
  854 | struct dpdk_mp *dmp, *next;
  | ^~~

NB: this looks like a false positive, gcc 13 probably fails to see the link
between reuse and dmp in dpdk_mp_get().

Signed-off-by: Robin Jarry 
Reviewed-by: David Marchand 
---

Notes:
v2: Fixed commit title and added note about possible false positive.

 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index fb0dd43f75c5..e11508cd5c31 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -840,7 +840,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
 static struct dpdk_mp *
 dpdk_mp_get(struct netdev_dpdk *dev, int mtu)
 {
-struct dpdk_mp *dmp, *next;
+struct dpdk_mp *dmp = NULL, *next;
 bool reuse = false;
 
 ovs_mutex_lock(_mp_mutex);
-- 
2.40.1

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


[ovs-dev] [PATCH v2] Add editorconfig file

2023-05-30 Thread Robin Jarry
EditorConfig is a file format and collection of text editor plugins for
maintaining consistent coding styles between different editors and IDEs.

Initialize the file following the coding rules in
Documentation/internals/contributing/coding-style.rst

In order for this file to be taken into account (unless they use an
editor with built-in EditorConfig support), developers will have to
install a plugin.

Note: The max_line_length property is only supported by a limited number
of EditorConfig plugins. It will be ignored if unsupported.

Link: https://editorconfig.org/
Link: https://github.com/editorconfig/editorconfig-emacs
Link: https://github.com/editorconfig/editorconfig-vim
Link: 
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Signed-off-by: Robin Jarry 
---

Notes:
v2: add .editorconfig to EXTRA_DIST

 .editorconfig | 14 ++
 Makefile.am   |  1 +
 2 files changed, 15 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index ..f7f43ecfeea3
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,14 @@
+# See https://editorconfig.org/ for syntax reference.
+
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+charset = utf-8
+max_line_length = 79
+
+[*.{c,h}]
+indent_style = space
+indent_size = 4
diff --git a/Makefile.am b/Makefile.am
index df9c33dfe631..db341504d37f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST = \
.ci/osx-build.sh \
.ci/osx-prepare.sh \
.cirrus.yml \
+   .editorconfig \
.github/workflows/build-and-test.yml \
appveyor.yml \
boot.sh \
-- 
2.40.1

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


[ovs-dev] [PATCH] netdev-dpdk: fix warning with gcc 13

2023-05-30 Thread Robin Jarry
GCC now reports uninitialized warnings from function return values.

../lib/netdev-dpdk.c: In function 'netdev_dpdk_mempool_configure':
../lib/netdev-dpdk.c:964:22: warning: 'dmp' may be used uninitialized 
[-Wmaybe-uninitialized]
  964 | dev->dpdk_mp = dmp;
  | ~^
../lib/netdev-dpdk.c:854:21: note: 'dmp' was declared here
  854 | struct dpdk_mp *dmp, *next;
  | ^~~

Signed-off-by: Robin Jarry 
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9f380a910e9f..829764c59550 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -851,7 +851,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
 static struct dpdk_mp *
 dpdk_mp_get(struct netdev_dpdk *dev, int mtu)
 {
-struct dpdk_mp *dmp, *next;
+struct dpdk_mp *dmp = NULL, *next;
 bool reuse = false;
 
 ovs_mutex_lock(_mp_mutex);
-- 
2.40.1

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


[ovs-dev] [PATCH] Add editorconfig file

2023-05-30 Thread Robin Jarry
EditorConfig is a file format and collection of text editor plugins for
maintaining consistent coding styles between different editors and IDEs.

Initialize the file following the coding rules in
Documentation/internals/contributing/coding-style.rst

In order for this file to be taken into account (unless they use an
editor with built-in EditorConfig support), developers will have to
install a plugin.

Note: The max_line_length property is only supported by a limited number
of EditorConfig plugins. It will be ignored if unsupported.

Link: https://editorconfig.org/
Link: https://github.com/editorconfig/editorconfig-emacs
Link: https://github.com/editorconfig/editorconfig-vim
Link: 
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Signed-off-by: Robin Jarry 
---
 .editorconfig | 14 ++
 1 file changed, 14 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index ..f7f43ecfeea3
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,14 @@
+# See https://editorconfig.org/ for syntax reference.
+
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+charset = utf-8
+max_line_length = 79
+
+[*.{c,h}]
+indent_style = space
+indent_size = 4
-- 
2.40.1

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


Re: [ovs-dev] [PATCH v10] netdev-dpdk: add control plane protection support

2023-05-25 Thread Robin Jarry
Hey Ilya,

Ilya Maximets, May 24, 2023 at 17:05:
> I had a '+' because rss and lacp are two different entities and I looked
> at it as a mode of operation. i.e. RSS plus special handling for LACP.
> RSS looks strange in a comma-separated list, IMO.

For now, there is only LACP but if other protocols are added (e.g. BFD),
wouldn't it be weird to have them separated as well?

options:rx-steering=rss+lacp+bfd

Since lacp and bfd will most likely be put in the additional rxq, it
would make sense to identify them as a group.

I also have other reserves about specifying rss here after thinking some
more about it:

- rss shouldn't be disabled anyway, this forces users to always specify
  it. This is not great from a usability point of view.

- When there is a single rxq configured by the user, there is no RSS
  happening per-se since all other traffic will be put in a single
  queue. The additional rxq being reserved for lacp and/or other special
  traffic.

What do you think about removing "rss" altogether from the items?

options:rx-steering=lacp,bfd,...

Cheers.

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


Re: [ovs-dev] [PATCH v10] netdev-dpdk: add control plane protection support

2023-05-24 Thread Robin Jarry
Kevin Traynor, May 24, 2023 at 16:54:
> > The "rss" item should be mandatory anyway. There should be no way to
> > disable it. I assume that it is what Ilya meant with these "+" symbols.
> > That would leave us with these examples:
> > 
> > # lacp+bfd on separate queue, rss on other queues
> > 
> >  options:rx-steering=rss,lacp,bfd
> > 
> > # same
> > 
> >  options:rx-steering=lacp,bfd,rss
>
> ^ It looks a little odd to me that some values are for single queues and 
> some are for the rest of the queues, with no way to distinguish.
>
> So maybe Ilya had placed significance in the order with his suggestion ? 
> i.e. [default]+[single queue proto]+[other single queue proto]+...
>
> I don't want to bike shed too much on it, i guess with good docs, it can 
> be clear anyway.
>
> > # only regular rss, same as no config at all
> > 
> >  options:rx-steering=rss
> > 
> > # invalid configurations
> > 
> >  options:rx-steering=lacp
>
> >  options:rx-steering=
> (^ This will be ok and use the default rss)

In that case, maybe we should exclude "rss" from the available items to
avoid confusion and only require users to specify the protocols that
need to be put in a separate queue. The remaining queues being rss by
default.

Would that be ok?

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


Re: [ovs-dev] [PATCH v10] netdev-dpdk: add control plane protection support

2023-05-24 Thread Robin Jarry
Hey Kevin,

Kevin Traynor, May 24, 2023 at 16:13:
> Hi Robin,
>
> I tested combinations of enabling/disabling cp-proto and enabling
> hwol. It is working as expected and hwol feature always has a clear
> priority, regardless of the order they are enabled in.
>
> I didn't test lacp traffic, but I was able to see that the rxq was
> added/removed for it and that rss was working on the other rxqs.
>
> I also tested combinations of different numbers of rxqs,
> adding/deleting rxqs, enabling/disabling cp-proto etc and working
> fine. I also tested enabling cp-proto and changing rxqs in the same
> command, working fine.

Thanks for testing!

> > +/* User input for n_rxq + an optional control plane protection 
> > queue
> > + * (see netdev_dpdk_reconfigure). This field is different from the
> > + * other requested_* fields as it may contain a different value 
> > than
> > + * the user input. */
>
> ^ I don't think this comment is really needed. requested_rxq_size can
> also be adjusted and they don't always come from user input, sometimes
> just the OVS default.
...
> > +/* User input for n_rxq (see dpdk_set_rxq_config). */
> > +int user_n_rxq;
>
> I also put in a similar 3rd stage status in recent patches for
> rx/tx-descriptors but managed to get rid of it. You might be able to
> remove this and just check check cp-proto flags do a +1/-1 where
> needed?

I had already tried this and it was not possible. I will have another
fresh look. Maybe I was thinking about this the wrong way.

> > +/*
> > + * Some drivers set reta_size equal to the total number of rxqs 
> > that
> > + * are configured when it is a power of two. Since we are actually
> > + * reconfiguring the redirection table to exclude the last rxq, we 
> > may
> > + * end up with an imbalanced redirection table. For example, such
> > + * configuration:
> > + *
> > + *   options:n_rxq=3 options:cp-protection=lacp
> > + *
> > + * Will actually configure 4 rxqs on the NIC, and the default reta 
> > to:
> > + *
> > + *   [0, 1, 2, 3]
> > + *
> > + * And dpdk_cp_prot_rss_configure() will reconfigure reta to:
> > + *
> > + *   [0, 1, 2, 0]
> > + *
> > + * Causing queue 0 to receive twice as much traffic as queues 1 
> > and 2.
> > + *
> > + * Work around that corner case by forcing a bigger redirection 
> > table
> > + * size to 128 entries when reta_size is not a multiple of 
> > rss_n_rxq
> > + * and when reta_size is less than 128. This value seems to be
> > + * supported by most of the drivers that also support rte flow.
> > + */
> > +info.reta_size = RTE_ETH_RSS_RETA_SIZE_128;
>
> Thanks for following up on previous comments and adding this
> workaround. I think you didn't get much response about it, so might be
> worth adding a DPDK bugzilla to formally report/track it.

I don't think it is a bug. If the default redirection table is not
modified, there will be proper balancing on all Rx queues. The issue we
have here is that we are reconfiguring the table and removing the last
queue from it which can lead to imbalances in some cases. Do you think
this is worth reporting anyway?

> > +try_cp_prot = dev->requested_cp_prot_flags != 0;
> > +dev->requested_n_rxq = dev->user_n_rxq;
> > +if (try_cp_prot) {
> > +dev->requested_n_rxq += 1;
>
> Minor, but adjusting the requested_n_rxq's seems more suited to
> dpdk_cp_prot_set_config() (and move it after dpdk_set_rxq_config() in
> netdev_dpdk_set_config()) as this is the function that applies the
> configuration. I admit, it's partly aesthetic, so that this function
> still just checks for changes that require a reconfig and returns if
> there are none :-)

This is related to the introduction of user_n_rxq. I had tried to make
it work and this was the only way. I'll have another fresh look.

Cheers,
Robin

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


  1   2   3   >