Bug#1064818: ITP: golang-sourcehut-rockorager-vaxis -- A modern TUI library for go
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
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.
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.
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.
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
, 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.
, 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.
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.
, 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.
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.
, 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().
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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.
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
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.
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.
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
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.
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.
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.
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.
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
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
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
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
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
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
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
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
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
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.
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.
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
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.
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
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.
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.
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.
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.
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.
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.
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.
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
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
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
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.
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.
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.
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
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
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
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
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
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
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