[ovs-dev] [PATCH ovn] ofctrl: Wait at S_WAIT_BEFORE_CLEAR only once.
The ovn-ofctrl-wait-before-clear setting is designed to minimize downtime during the initial start-up of the ovn-controller. For this purpose, the ovn-controller should wait only once upon entering the S_WAIT_BEFORE_CLEAR state for the first time. Subsequent reconnections to the OVS, such as those occurring during an OVS restart/upgrade, should not trigger this wait. However, the current implemention always waits for the configured time in the S_WAIT_BEFORE_CLEAR state, which can inadvertently delay flow installations during OVS restart/upgrade, potentially causing more harm than good. (The extent of the impact varies based on the method used to restart OVS, including whether flow save/restore tools and the flow-restore-wait feature are employed.) This patch avoids the unnecessary wait after the initial one. Fixes: 896adfd2d8b3 ("ofctrl: Support ovn-ofctrl-wait-before-clear to reduce down time during upgrade.") Signed-off-by: Han Zhou --- controller/ofctrl.c | 1 - tests/ovn-controller.at | 9 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index f14cd79a8dbb..0d72ecbaa167 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -634,7 +634,6 @@ run_S_WAIT_BEFORE_CLEAR(void) if (!wait_before_clear_time || (wait_before_clear_expire && time_msec() >= wait_before_clear_expire)) { -wait_before_clear_expire = 0; state = S_CLEAR_FLOWS; return; } diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 37f1ded1bd26..b65e11722cbb 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2284,10 +2284,15 @@ lflow_run_2=$(ovn-appctl -t ovn-controller coverage/read-counter lflow_run) AT_CHECK_UNQUOTED([echo $lflow_run_1], [0], [$lflow_run_2 ]) -# Restart OVS this time, and wait until flows are reinstalled +# Restart OVS this time. Flows should be reinstalled without waiting. OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif -vunixctl -OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 | grep -vF 2.2.2.2]) + +# Sleep for 3s, which is long enough for the flows to be installed, but +# shorter than the wait-before-clear (5s), to make sure the flows are installed +# without waiting. +sleep 3 +AT_CHECK([ovs-ofctl dump-flows br-int | grep -F 10.1.2.4 | grep -vF 2.2.2.2], [0], [ignore]) check ovn-nbctl --wait=hv lb-add lb3 3.3.3.3 10.1.2.5 \ -- ls-lb-add ls1 lb3 -- 2.38.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] utilities: Make database connection optional for ovn-detrace.
The ovn-detrace required both connections (SB and NB) to provide some output. However, it is a valid scenario to have only one database available, usually the SB. With that in mind make the connection optional and print warning into stderr that the output will not contain information from DB that is not available. Reported-at: https://issues.redhat.com/browse/FDP-68 Signed-off-by: Ales Musil --- utilities/ovn_detrace.py.in | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/utilities/ovn_detrace.py.in b/utilities/ovn_detrace.py.in index 364f11a71..12df6e52a 100755 --- a/utilities/ovn_detrace.py.in +++ b/utilities/ovn_detrace.py.in @@ -37,6 +37,9 @@ except Exception: argv0 = sys.argv[0] version = "@VERSION@" +DB_CONNECTION_ERR = ('The connection to {0} DB is not available,' + ' {0} information will be missing from the detrace.') + def usage(): print("""\ @@ -77,6 +80,11 @@ def chassis_str(chassis): ch = chassis[0] return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname) + +class ConnectionException(Exception): +pass + + class OVSDB(object): STREAM_TIMEOUT_MS = 1000 @@ -111,7 +119,7 @@ class OVSDB(object): os.strerror(error))) strm = None if not strm: -raise Exception('Unable to connect to %s' % self.remote) +raise ConnectionException() rpc = jsonrpc.Connection(strm) req = jsonrpc.Message.create_request('get_schema', [schema_name]) @@ -167,6 +175,8 @@ class CookieHandlerByUUUID(CookieHandler): super(CookieHandlerByUUUID, self).__init__(db, table, printer) def get_records(self, cookie): +if not self._db: +return [] # Adjust cookie to include leading zeroes if needed. cookie = cookie.zfill(8) return self._db.find_rows_by_partial_uuid(self._table, cookie) @@ -488,8 +498,19 @@ def main(): if ovs and not ovs_db: ovs_db = 'unix:%s/db.sock' % ovs_rundir -ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound', leader_only=leader_only) -ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound', leader_only=leader_only) +try: +ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound', +leader_only=leader_only) +except ConnectionException: +print(DB_CONNECTION_ERR.format('SB'), file=sys.stderr) +ovsdb_ovnsb = None + +try: +ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound', +leader_only=leader_only) +except ConnectionException: +print(DB_CONNECTION_ERR.format('NB'), file=sys.stderr) +ovsdb_ovnnb = None printer = Printer() cookie_handlers = get_cookie_handlers(ovsdb_ovnnb, ovsdb_ovnsb, printer) -- 2.44.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] Fix broken link for LTS release.
I found the broken link at https://docs.ovn.org/en/latest/internals/release-process.html. I believe the correct link is https://www.ovn.org/en/releases/#long-term-support Signed-off-by: Igor Zhukov --- Documentation/internals/release-process.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/internals/release-process.rst b/Documentation/internals/release-process.rst index 26d3f8d4d..988257975 100644 --- a/Documentation/internals/release-process.rst +++ b/Documentation/internals/release-process.rst @@ -203,5 +203,5 @@ Contact Use d...@openvswitch.org to discuss the OVN development and release process. -__ https://www.ovn.org/en/releases/long_term_support/ +__ https://www.ovn.org/en/releases/#long-term-support __ https://www.ovn.org -- 2.43.0.windows.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 4/4] appveyor: Build with OpenSSL 3.0.
That’s understandable, I think we should be fine for now and if someone asks for a backport we can address it then. Thank you again for all the work you put into this! Alin > > On 4 Mar 2024, at 23:44, Ilya Maximets wrote: > > On 3/1/24 22:29, Alin Serdean wrote: >> Lgtm. Thank you so much Ilya >> >> Acked-by: Alin-Gabriel Serdean > > Thanks, Alin and Simon! > > I applied the set now. > > Also backported patches 2 and 3 to branch-3.3, since they are bug fixes > and 3.3 is planned to be our next LTS. We may backport further, but > I'm a little hesitant to do so since we don't have CI for stable branches > and no-one complained so far. Let me know if you think we should backport > further, I can do that. > > Best regards, Ilya Maximets. > >> >>> On 1 Mar 2024, at 22:10, Ilya Maximets wrote: >>> >>> OpenSSL 1.0.2u is long deprecated and not available for download. >>> So, our CI never actually downloads it and uses whatever is in the >>> OpenSSL-Win64 folder provided by AppVeyor. Luckily, it happens to >>> be OpenSSL 1.0.2u today. >>> >>> The oldest supported version of OpenSSL upstream today is 3.0. >>> And it is an LTS version. 3.1 and 3.2 are not LTS. >>> >>> Use OpenSSL 3.0 for testing instead. >>> >>> This commit does a few things to achieve that: >>> >>> 1. Removes the folder provided by AppVeyor. This way we will fail >>> the build if something goes wrong instead of silently using >>> OpenSSL version provided by AppVeyor. >>> >>> 2. Obtains the JSON description of available releases and downloads >>> the latest minor version of OpenSSL 3.0 64-bit. With this approach >>> we should not need to update the download link that frequently. >>> New minor releases will be picked up automatically. They should >>> not have any breaking changes, so should be fine to use in CI. >>> OpenSSL 3.0 is supported until at least Sep 2026. >>> >>> The JSON file is an official file referenced on the: >>> https://slproweb.com/products/Win32OpenSSL.html >>> So, it should be safe to use. >>> >>> 3. Executes the downloaded installer with 'Start-Process -Wait' to >>> properly wait for installation to finish instead of just sleeping >>> for 30 seconds. >>> >>> 4. Caches the downloaded installer, so we're not downloading 300 MB >>> on each CI run as that is not nice to do. We know the hash of the >>> latest version, so we will re-download only when the binary changes, >>> i.e. on a new minor release. >>> >>> For the cache to work we need to introduce the 'install' phase, >>> because caches are populated after 'init', but before 'install'. >>> Alternatively, we could have just renamed 'init' to 'install', >>> but I think it's a little nicer to have separate phases, and we >>> can also move 'windows-prepare.sh' to the install phase. >>> >>> Cache is also invalidated whenever appveyor.yml changes. >>> >>> Acked-by: Simon Horman >>> Signed-off-by: Ilya Maximets >>> --- >>> appveyor.yml | 52 ++-- >>> 1 file changed, 42 insertions(+), 10 deletions(-) >>> >>> diff --git a/appveyor.yml b/appveyor.yml >>> index 373f01a43..29cc44d6c 100644 >>> --- a/appveyor.yml >>> +++ b/appveyor.yml >>> @@ -8,28 +8,60 @@ configuration: >>> - Release >>> clone_folder: C:\openvswitch_compile >>> shallow_clone: true >>> + >>> init: >>> - ps: $env:PATH ="C:\Python312-x64;"+$env:PATH >>> - ps: New-Item -Type HardLink -Path "C:\Python312-x64\python3.exe" >>> -Value "C:\Python312-x64\python.exe" >>> + >>> +cache: >>> +- C:\ovs-build-downloads -> appveyor.yml >>> + >>> +install: >>> - ps: | >>> -mkdir C:\ovs-build-downloads >>> +Remove-Item -Recurse -Force -Path C:/OpenSSL-Win64 >>> +New-Item -ItemType Directory -Force -Path C:\ovs-build-downloads >>> + >>> +# Find and download the latest stable OpenSSl 3.0. >>> +$URL = >>> "https://raw.githubusercontent.com/slproweb/opensslhashes/master/win32_openssl_hashes.json"; >>> +$webData = (Invoke-WebRequest -Uri $URL).content | ConvertFrom-Json >>> +$source = ($webData.files.PSObject.Properties | Where-Object { >>> +$_.Value.basever -match "3.0.*" -and >>> +$_.Value.bits -eq"64"-and >>> +$_.Value.arch -eq"INTEL" -and >>> +$_.Value.installer -eq"exe" -and >>> +-not $_.Value.light >>> +} | Select-Object Value).PSObject.Properties.Value >>> + >>> +Write-Host "Latest OpenSSL 3.0:" ($source | Format-List | Out-String) >>> + >>> +$destination = "C:\ovs-build-downloads\Win64OpenSSL.exe" >>> +if (Test-Path $destination) { >>> +$fileHash = (Get-FileHash $destination -Algorithm >>> SHA256).Hash.ToLower() >>> +if ($fileHash -ne $source.sha256) { >>> +Write-Host "Cache miss:" $fileHash "!=" $source.sha256 >>> +Remove-Item -Path $destination >>> +} >>> +} >>> >>> -$source = "https://slproweb.com/download/Win64OpenS
Re: [ovs-dev] [PATCH v2 4/4] appveyor: Build with OpenSSL 3.0.
On 3/1/24 22:29, Alin Serdean wrote: > Lgtm. Thank you so much Ilya > > Acked-by: Alin-Gabriel Serdean Thanks, Alin and Simon! I applied the set now. Also backported patches 2 and 3 to branch-3.3, since they are bug fixes and 3.3 is planned to be our next LTS. We may backport further, but I'm a little hesitant to do so since we don't have CI for stable branches and no-one complained so far. Let me know if you think we should backport further, I can do that. Best regards, Ilya Maximets. > >> >> On 1 Mar 2024, at 22:10, Ilya Maximets wrote: >> >> OpenSSL 1.0.2u is long deprecated and not available for download. >> So, our CI never actually downloads it and uses whatever is in the >> OpenSSL-Win64 folder provided by AppVeyor. Luckily, it happens to >> be OpenSSL 1.0.2u today. >> >> The oldest supported version of OpenSSL upstream today is 3.0. >> And it is an LTS version. 3.1 and 3.2 are not LTS. >> >> Use OpenSSL 3.0 for testing instead. >> >> This commit does a few things to achieve that: >> >> 1. Removes the folder provided by AppVeyor. This way we will fail >> the build if something goes wrong instead of silently using >> OpenSSL version provided by AppVeyor. >> >> 2. Obtains the JSON description of available releases and downloads >> the latest minor version of OpenSSL 3.0 64-bit. With this approach >> we should not need to update the download link that frequently. >> New minor releases will be picked up automatically. They should >> not have any breaking changes, so should be fine to use in CI. >> OpenSSL 3.0 is supported until at least Sep 2026. >> >> The JSON file is an official file referenced on the: >>https://slproweb.com/products/Win32OpenSSL.html >> So, it should be safe to use. >> >> 3. Executes the downloaded installer with 'Start-Process -Wait' to >> properly wait for installation to finish instead of just sleeping >> for 30 seconds. >> >> 4. Caches the downloaded installer, so we're not downloading 300 MB >> on each CI run as that is not nice to do. We know the hash of the >> latest version, so we will re-download only when the binary changes, >> i.e. on a new minor release. >> >> For the cache to work we need to introduce the 'install' phase, >> because caches are populated after 'init', but before 'install'. >> Alternatively, we could have just renamed 'init' to 'install', >> but I think it's a little nicer to have separate phases, and we >> can also move 'windows-prepare.sh' to the install phase. >> >> Cache is also invalidated whenever appveyor.yml changes. >> >> Acked-by: Simon Horman >> Signed-off-by: Ilya Maximets >> --- >> appveyor.yml | 52 ++-- >> 1 file changed, 42 insertions(+), 10 deletions(-) >> >> diff --git a/appveyor.yml b/appveyor.yml >> index 373f01a43..29cc44d6c 100644 >> --- a/appveyor.yml >> +++ b/appveyor.yml >> @@ -8,28 +8,60 @@ configuration: >> - Release >> clone_folder: C:\openvswitch_compile >> shallow_clone: true >> + >> init: >> - ps: $env:PATH ="C:\Python312-x64;"+$env:PATH >> - ps: New-Item -Type HardLink -Path "C:\Python312-x64\python3.exe" >> -Value "C:\Python312-x64\python.exe" >> + >> +cache: >> +- C:\ovs-build-downloads -> appveyor.yml >> + >> +install: >> - ps: | >> -mkdir C:\ovs-build-downloads >> +Remove-Item -Recurse -Force -Path C:/OpenSSL-Win64 >> +New-Item -ItemType Directory -Force -Path C:\ovs-build-downloads >> + >> +# Find and download the latest stable OpenSSl 3.0. >> +$URL = >> "https://raw.githubusercontent.com/slproweb/opensslhashes/master/win32_openssl_hashes.json"; >> +$webData = (Invoke-WebRequest -Uri $URL).content | ConvertFrom-Json >> +$source = ($webData.files.PSObject.Properties | Where-Object { >> +$_.Value.basever -match "3.0.*" -and >> +$_.Value.bits -eq"64"-and >> +$_.Value.arch -eq"INTEL" -and >> +$_.Value.installer -eq"exe" -and >> +-not $_.Value.light >> +} | Select-Object Value).PSObject.Properties.Value >> + >> +Write-Host "Latest OpenSSL 3.0:" ($source | Format-List | Out-String) >> + >> +$destination = "C:\ovs-build-downloads\Win64OpenSSL.exe" >> +if (Test-Path $destination) { >> +$fileHash = (Get-FileHash $destination -Algorithm >> SHA256).Hash.ToLower() >> +if ($fileHash -ne $source.sha256) { >> +Write-Host "Cache miss:" $fileHash "!=" $source.sha256 >> +Remove-Item -Path $destination >> +} >> +} >> >> -$source = "https://slproweb.com/download/Win64OpenSSL-1_0_2u.exe"; >> -$destination = "C:\ovs-build-downloads\Win64OpenSSL-1_0_2u.exe" >> -Invoke-WebRequest $source -OutFile $destination >> +if (Test-Path $destination) { >> +Write-Host "Using cached:" $destination >> +} else { >> +Write-Host "Downloading:" $source.url >> +Invoke-WebRequest $source.url -OutFile $destin
Re: [ovs-dev] [PATCH v3] conntrack: Fix flush not flushing all elements.
On Mon, Mar 4, 2024 at 10:22 AM Xavier Simonart wrote: > > On netdev datapath, when a ct element was cleaned, the cmap > could be shrinked, potentially causing some elements to be skipped > in the flush iteration. > > Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.") > Signed-off-by: Xavier Simonart Acked-by: Mike Pattrick ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v9 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
On 4 Mar 2024, at 16:46, Aaron Conole wrote: > Eelco Chaudron writes: > >> On 20 Feb 2024, at 22:47, Aaron Conole wrote: >> >>> From: Kevin Sprague >>> >>> During normal operations, it is useful to understand when a particular flow >>> gets removed from the system. This can be useful when debugging performance >>> issues tied to ofproto flow changes, trying to determine deployed traffic >>> patterns, or while debugging dynamic systems where ports come and go. >>> >>> Prior to this change, there was a lack of visibility around flow expiration. >>> The existing debugging infrastructure could tell us when a flow was added to >>> the datapath, but not when it was removed or why. >>> >>> This change introduces a USDT probe at the point where the revalidator >>> determines that the flow should be removed. Additionally, we track the >>> reason for the flow eviction and provide that information as well. With >>> this change, we can track the complete flow lifecycle for the netlink >>> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and >>> the revalidator USDT, letting us watch as flows are added and removed from >>> the kernel datapath. >>> >>> This change only enables this information via USDT probe, so it won't be >>> possible to access this information any other way (see: >>> Documentation/topics/usdt-probes.rst). >>> >>> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py) >>> which serves as a demonstration of how the new USDT probe might be used >>> going forward. >>> >>> Acked-by: Han Zhou >>> Signed-off-by: Kevin Sprague >>> Co-authored-by: Aaron Conole >>> Signed-off-by: Aaron Conole >> >> Thanks for doing the v9, some small comments remain below. >> >> Cheers, >> >> Eelco >> >>> --- >>> v8 -> v9: Reorganized the flow delete reasons enum >>> Updated flow_reval_monitor to use pahole to extract fields >>> Added the purge reason with a proper USDT point >>> Updated documentation >>> Dropped all the outstanding ACKs >>> >>> Documentation/topics/usdt-probes.rst | 43 + >>> ofproto/ofproto-dpif-upcall.c| 48 +- >>> utilities/automake.mk| 3 + >>> utilities/usdt-scripts/flow_reval_monitor.py | 997 +++ >>> 4 files changed, 1085 insertions(+), 6 deletions(-) >>> create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py >>> >>> diff --git a/Documentation/topics/usdt-probes.rst >>> b/Documentation/topics/usdt-probes.rst >>> index e527f43bab..015614a6b8 100644 >>> --- a/Documentation/topics/usdt-probes.rst >>> +++ b/Documentation/topics/usdt-probes.rst >>> @@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``: >>> - dpif_recv:recv_upcall >>> - main:poll_block >>> - main:run_start >>> +- revalidate:flow_result >>> - revalidate_ukey\_\_:entry >>> - revalidate_ukey\_\_:exit >>> +- revalidator_sweep\_\_:flow_result >>> - udpif_revalidator:start_dump >>> - udpif_revalidator:sweep_done >>> >>> @@ -443,6 +445,47 @@ sweep phase was completed. >>> - ``utilities/usdt-scripts/reval_monitor.py`` >>> >>> >>> +probe revalidate:flow_result >>> + >>> + >>> +**Description**: >>> +This probe is triggered when the revalidator has executed on a particular >>> +flow key to make a determination whether to evict a flow, and the cause >>> +for eviction. The revalidator runs periodically, and this probe will only >>> +be triggered when a flow is flagged for revalidation. >>> + >>> +**Arguments**: >>> + >>> +- *arg0*: ``(enum reval_result) result`` >>> +- *arg1*: ``(enum flow_del_reason) reason`` >> >> nit: variable name changed, so should be del_reason. > > Good catch, I'll update. > >>> +- *arg2*: ``(struct udpif *) udpif`` >>> +- *arg3*: ``(struct udpif_key *) ukey`` >>> + >> >> I think you missed my previous comment on re-ordering the arguments to >> be more inline with existing probes, i.e.: >> >> +OVS_USDT_PROBE(revalidator_sweep__, flow_result, udpif, >> ukey, >> + result, del_reason); > > Guess so. I'll fix it. > >>> +**Script references**: >>> + >>> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >>> + >>> + >>> +probe revalidator_sweep\_\_:flow_result >>> +~~~ >>> + >>> +**Description**: >>> +This probe is placed in the path of the revalidator sweep, and is executed >>> +under the condition that a flow entry is in an unexpected state, or the >>> +flows were asked to be purged due to a user action. >>> + >>> +**Arguments**: >>> + >>> +- *arg0*: ``(enum reval_result) result`` >>> +- *arg1*: ``(enum flow_del_reason) reason`` >> >> nit: variable name changed, so should be del_reason. > > Okay. > >>> +- *arg2*: ``(struct udpif *) udpif`` >>> +- *arg3*: ``(struct udpif_key *) ukey`` >> >> See comments above on argument ordering. >> >>> + >>> +**Script references**: >>> + >>> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >>> + >>> + >>>
Re: [ovs-dev] [PATCH v9 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
Eelco Chaudron writes: > On 20 Feb 2024, at 22:47, Aaron Conole wrote: > >> From: Kevin Sprague >> >> During normal operations, it is useful to understand when a particular flow >> gets removed from the system. This can be useful when debugging performance >> issues tied to ofproto flow changes, trying to determine deployed traffic >> patterns, or while debugging dynamic systems where ports come and go. >> >> Prior to this change, there was a lack of visibility around flow expiration. >> The existing debugging infrastructure could tell us when a flow was added to >> the datapath, but not when it was removed or why. >> >> This change introduces a USDT probe at the point where the revalidator >> determines that the flow should be removed. Additionally, we track the >> reason for the flow eviction and provide that information as well. With >> this change, we can track the complete flow lifecycle for the netlink >> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and >> the revalidator USDT, letting us watch as flows are added and removed from >> the kernel datapath. >> >> This change only enables this information via USDT probe, so it won't be >> possible to access this information any other way (see: >> Documentation/topics/usdt-probes.rst). >> >> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py) >> which serves as a demonstration of how the new USDT probe might be used >> going forward. >> >> Acked-by: Han Zhou >> Signed-off-by: Kevin Sprague >> Co-authored-by: Aaron Conole >> Signed-off-by: Aaron Conole > > Thanks for doing the v9, some small comments remain below. > > Cheers, > > Eelco > >> --- >> v8 -> v9: Reorganized the flow delete reasons enum >> Updated flow_reval_monitor to use pahole to extract fields >> Added the purge reason with a proper USDT point >> Updated documentation >> Dropped all the outstanding ACKs >> >> Documentation/topics/usdt-probes.rst | 43 + >> ofproto/ofproto-dpif-upcall.c| 48 +- >> utilities/automake.mk| 3 + >> utilities/usdt-scripts/flow_reval_monitor.py | 997 +++ >> 4 files changed, 1085 insertions(+), 6 deletions(-) >> create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py >> >> diff --git a/Documentation/topics/usdt-probes.rst >> b/Documentation/topics/usdt-probes.rst >> index e527f43bab..015614a6b8 100644 >> --- a/Documentation/topics/usdt-probes.rst >> +++ b/Documentation/topics/usdt-probes.rst >> @@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``: >> - dpif_recv:recv_upcall >> - main:poll_block >> - main:run_start >> +- revalidate:flow_result >> - revalidate_ukey\_\_:entry >> - revalidate_ukey\_\_:exit >> +- revalidator_sweep\_\_:flow_result >> - udpif_revalidator:start_dump >> - udpif_revalidator:sweep_done >> >> @@ -443,6 +445,47 @@ sweep phase was completed. >> - ``utilities/usdt-scripts/reval_monitor.py`` >> >> >> +probe revalidate:flow_result >> + >> + >> +**Description**: >> +This probe is triggered when the revalidator has executed on a particular >> +flow key to make a determination whether to evict a flow, and the cause >> +for eviction. The revalidator runs periodically, and this probe will only >> +be triggered when a flow is flagged for revalidation. >> + >> +**Arguments**: >> + >> +- *arg0*: ``(enum reval_result) result`` >> +- *arg1*: ``(enum flow_del_reason) reason`` > > nit: variable name changed, so should be del_reason. Good catch, I'll update. >> +- *arg2*: ``(struct udpif *) udpif`` >> +- *arg3*: ``(struct udpif_key *) ukey`` >> + > > I think you missed my previous comment on re-ordering the arguments to > be more inline with existing probes, i.e.: > > +OVS_USDT_PROBE(revalidator_sweep__, flow_result, udpif, ukey, > + result, del_reason); Guess so. I'll fix it. >> +**Script references**: >> + >> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >> + >> + >> +probe revalidator_sweep\_\_:flow_result >> +~~~ >> + >> +**Description**: >> +This probe is placed in the path of the revalidator sweep, and is executed >> +under the condition that a flow entry is in an unexpected state, or the >> +flows were asked to be purged due to a user action. >> + >> +**Arguments**: >> + >> +- *arg0*: ``(enum reval_result) result`` >> +- *arg1*: ``(enum flow_del_reason) reason`` > > nit: variable name changed, so should be del_reason. Okay. >> +- *arg2*: ``(struct udpif *) udpif`` >> +- *arg3*: ``(struct udpif_key *) ukey`` > > See comments above on argument ordering. > >> + >> +**Script references**: >> + >> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >> + >> + >> Adding your own probes >> -- >> >> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c >> index b5cbeed878..fbc7858690 100644 >> --- a/ofproto/ofproto
Re: [ovs-dev] [PATCH v2 3/4] m4: Fix linking with OpenSSL 1.1.0+ and 3+ on Windows.
On Fri, Mar 01, 2024 at 10:10:39PM +0100, Ilya Maximets wrote: > OpenSSL 1.1.0 changed the library names from libeay32 and ssleay32 to > standard libssl and libcrypto. All the versions of OpenSSL that used > old names reached their official EoL, so it should be safe to just > migrate to new names. They can still be supported via premium support > option, but I don't think that is important for us. > > Also, OpenSSL installers for older versions had the following folder > structure: > > C:\OPENSSL-WIN64\ > +---bin > +---include > | +---openssl > +---lib > | libeay32.lib > | ssleay32.lib > +---VC > libeay32MD.lib > libeay32MDd.lib > libeay32MT.lib > libeay32MTd.lib > ssleay32MD.lib > ssleay32MDd.lib > ssleay32MT.lib > ssleay32MTd.lib > > With newer OpenSSL 3+ the structure is different: > > C:\OPENSSL-WIN64 > +---bin > +---include > | +---openssl > +---lib > +---VC > +---x64 > +---MD > | libcrypto.lib > | libssl.lib > +---MDd > | libcrypto.lib > | libssl.lib > +---MT > | libcrypto.lib > | libssl.lib > +---MTd > libcrypto.lib > libssl.lib > > Basically, instead of one generic library in the lib folder and a bunch > of differently named versions of it for different type of linkage, we > now have multiple instances of the library located in different folders > based on the linkage type. So, we have to provide an exact path in > order to find the library. > > 'lib/VC/x64/MT' was chosen in this patch since it is a way used for > building in build-aux/ccl. > MD stands for dynamic linking, MT is static, 'd' stands for debug > versions of the libraries. > > While at it, fixing documentation examples to point to Win64 default > installation folder. > > Signed-off-by: Ilya Maximets Acked-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] conntrack: Fix flush not flushing all elements.
On netdev datapath, when a ct element was cleaned, the cmap could be shrinked, potentially causing some elements to be skipped in the flush iteration. Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.") Signed-off-by: Xavier Simonart --- v2: - Updated commit message. - Use compose-packet instead of hex packet content. - Use dnl for comments. - Remove unnecessary errors in OVS_TRAFFIC_VSWITCHD_STOP. - Rebased on origin/master. v3: - removed cm_pos. - Use zone=1 in flush-conntrack and dump-conntrack. --- lib/conntrack.c | 14 lib/conntrack.h | 2 +- tests/system-traffic.at | 47 + 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 8a7056bac..5786424f6 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2651,25 +2651,19 @@ conntrack_dump_start(struct conntrack *ct, struct conntrack_dump *dump, dump->ct = ct; *ptot_bkts = 1; /* Need to clean up the callers. */ +dump->cursor = cmap_cursor_start(&ct->conns); return 0; } int conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry) { -struct conntrack *ct = dump->ct; long long now = time_msec(); -for (;;) { -struct cmap_node *cm_node = cmap_next_position(&ct->conns, - &dump->cm_pos); -if (!cm_node) { -break; -} -struct conn_key_node *keyn; -struct conn *conn; +struct conn_key_node *keyn; +struct conn *conn; -INIT_CONTAINER(keyn, cm_node, cm_node); +CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, &dump->cursor) { if (keyn->dir != CT_DIR_FWD) { continue; } diff --git a/lib/conntrack.h b/lib/conntrack.h index ee7da099e..8ab8b0017 100644 --- a/lib/conntrack.h +++ b/lib/conntrack.h @@ -107,8 +107,8 @@ struct conntrack_dump { struct conntrack *ct; unsigned bucket; union { -struct cmap_position cm_pos; struct hmap_position hmap_pos; +struct cmap_cursor cursor; }; bool filter_zone; uint16_t zone; diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 98e494abf..891a1de0a 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -8389,6 +8389,53 @@ AT_CHECK([ovs-pcap client.pcap | grep 20102000], [0], [dnl OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - Flush many conntrack entries by port]) +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +AT_DATA([flows.txt], [dnl +priority=100,in_port=1,udp,action=ct(zone=1,commit),2 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl 20 packets from port 1 and 1 packet from port 2. +flow_l3="\ +eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\ +nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no" + +for i in $(seq 1 20); do +frame=$(ovs-ofctl compose-packet --bare "$flow_l3, udp_src=1,udp_dst=$i") +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame actions=resubmit(,0)"]) +done +frame=$(ovs-ofctl compose-packet --bare "$flow_l3, udp_src=2,udp_dst=1") +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame actions=resubmit(,0)"]) + +: > conntrack + +for i in $(seq 1 20); do +echo "udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1" >> conntrack +done +echo "udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1" >> conntrack + +sort conntrack > expout + +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," | sort ], [0], [expout]) + +dnl Check that flushing conntrack by port 1 flush all ct for port 1 but keeps ct for port 2. +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1 'ct_nw_proto=17,ct_tp_src=1']) +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," | sort ], [0], [dnl +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1 +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_BANNER([IGMP]) AT_SETUP([IGMP - flood under normal action]) -- 2.41.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] conntrack: Fix flush not flushing all elements.
Thanks Mike and Ilya. Sending v3 to handle comments from Mike and Ilya. Thanks Xavier On Fri, Mar 1, 2024 at 7:48 PM Ilya Maximets wrote: > On 2/26/24 11:49, Xavier Simonart wrote: > > On netdev datapath, when a ct element was cleaned, the cmap > > could be shrinked, potentially causing some elements to be skipped > > in the flush iteration. > > > > Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.") > > Signed-off-by: Xavier Simonart > > --- > > v2: - Updated commit message. > > - Use compose-packet instead of hex packet content. > > - Use dnl for comments. > > - Remove unnecessary errors in OVS_TRAFFIC_VSWITCHD_STOP. > > - Rebased on origin/master. > > Thanks, Xavier! > > Beside the comment from Mike I have a couple nits for the test. > See below. Otherwise, the change looks good. > > Best regards, Ilya Maximets. > > > --- > > lib/conntrack.c | 14 > > lib/conntrack.h | 1 + > > tests/system-traffic.at | 47 + > > 3 files changed, 52 insertions(+), 10 deletions(-) > > > > diff --git a/lib/conntrack.c b/lib/conntrack.c > > index 8a7056bac..5786424f6 100644 > > --- a/lib/conntrack.c > > +++ b/lib/conntrack.c > > @@ -2651,25 +2651,19 @@ conntrack_dump_start(struct conntrack *ct, > struct conntrack_dump *dump, > > > > dump->ct = ct; > > *ptot_bkts = 1; /* Need to clean up the callers. */ > > +dump->cursor = cmap_cursor_start(&ct->conns); > > return 0; > > } > > > > int > > conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry > *entry) > > { > > -struct conntrack *ct = dump->ct; > > long long now = time_msec(); > > > > -for (;;) { > > -struct cmap_node *cm_node = cmap_next_position(&ct->conns, > > - &dump->cm_pos); > > -if (!cm_node) { > > -break; > > -} > > -struct conn_key_node *keyn; > > -struct conn *conn; > > +struct conn_key_node *keyn; > > +struct conn *conn; > > > > -INIT_CONTAINER(keyn, cm_node, cm_node); > > +CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, &dump->cursor) { > > if (keyn->dir != CT_DIR_FWD) { > > continue; > > } > > diff --git a/lib/conntrack.h b/lib/conntrack.h > > index ee7da099e..aa12a1847 100644 > > --- a/lib/conntrack.h > > +++ b/lib/conntrack.h > > @@ -109,6 +109,7 @@ struct conntrack_dump { > > union { > > struct cmap_position cm_pos; > > struct hmap_position hmap_pos; > > +struct cmap_cursor cursor; > > }; > > bool filter_zone; > > uint16_t zone; > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index 98e494abf..34f93b2e5 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -8389,6 +8389,53 @@ AT_CHECK([ovs-pcap client.pcap | grep > 20102000], [0], [dnl > > OVS_TRAFFIC_VSWITCHD_STOP > > AT_CLEANUP > > > > +AT_SETUP([conntrack - Flush many conntrack entries by port]) > > +CHECK_CONNTRACK() > > +OVS_TRAFFIC_VSWITCHD_START() > > + > > +ADD_NAMESPACES(at_ns0, at_ns1) > > + > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > > + > > +AT_DATA([flows.txt], [dnl > > +priority=100,in_port=1,udp,action=ct(zone=1,commit),2 > > +]) > > + > > +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > > + > > +dnl 20 packets from port 1 and 1 packet from port 2. > > +flow_l3="\ > > +eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\ > > +nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no" > > + > > +for i in $(seq 1 20); do > > +frame=$(ovs-ofctl compose-packet --bare "$flow_l3, > udp_src=1,udp_dst=$i") > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=$frame actions=resubmit(,0)"]) > > +done > > +frame=$(ovs-ofctl compose-packet --bare "$flow_l3, udp_src=2,udp_dst=1") > > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=$frame actions=resubmit(,0)"]) > > + > > +: > conntrack > > + > > +for i in $(seq 1 20); do > > +echo > "udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1" > >> conntrack > > +done > > +echo > "udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1" > >> conntrack > > + > > +sort conntrack > expout > > + > > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep -F "src=10.1.1.1," | > sort ], [0], [expout]) > > + > > +dnl Check that flushing conntrack by port 1 flush all ct for port 1 but > keeps ct for port 2. > > +AT_CHECK([ovs-appctl dpctl/flush-conntrack > 'ct_nw_proto=17,ct_tp_src=1']) > > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep -F "src=10.1.1.1," | > sort ], [0], [dnl > > > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1 > > +]) > > Can we
[ovs-dev] [Patch ovn 2/2] northd.c: Fix direct access to SNAT network on DR.
This change fixes bug that breaks ability of machines from external networks to communicate with machines in SNATed networks (specifically when using a Distributed router). Currently when a machine (S1) on an external network tries to talk over TCP with a machine (A1) in a network that has enabled SNAT, the connection is established successfully. However after the three-way handshake, any packets that come from the A1 machine will have their source address translated by the Distributed router, breaking the communication. Existing rule in `build_lrouter_out_snat_flow` that decides which packets should be SNATed already tries to avoid SNATing packets in reply direction with `(!ct.trk || !ct.rpl)`. However, previous stages in the distributed LR egress pipeline do not initiate the CT state. Additionally we need to commit new connections that originate from external networks into CT, so that the packets in the reply direction can be properly identified. Rationale: In my original RFC [0], there were questions about the motivation for fixing this issue. I'll try to summarize why I think this is a bug that should be fixed. 1. Current implementation for Distributed router already tries to avoid SNATing packets in the reply direction, it's just missing initialized CT states to make proper decisions. 2. This same scenario works with Gateway Router. I tested with following setup: foo -- R1 -- join - R3 -- alice | bar --R2 R1 is a Distributed router with SNAT for foo. R2 is a Gateway router with SNAT for bar. R3 is a Gateway router with no SNAT. Using 'alice1' as a client I was able to talk over TCP with 'bar1' but connection with 'foo1' failed. 3. Regarding security and "leaking" of internal IPs. Reading through RFC 4787 [1], 5382 [2] and their update in 7857 [3], the specifications do not seem to mandate that SNAT implementations must filter incoming traffic destined directly to the internal network. Sections like "5. Filtering Behavior" in 4787 and "4.3 Externally Initiated Connections" in 5382 describe only behavior for traffic destined to external IP/Port associated with NAT on the device that performs NAT. Besides, with the current implementation, it's already possible to scan the internal network with pings and TCP syn scanning. If an additional security is needed, ACLs can be used to filter out incomming traffic from external networks. 4. We do have customers/clouds that depend on this functionality. This is a scenario that used to work in Openstack with ML2/OVS and migrating those clouds to ML2/OVN would break it. [0]https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411670.html [1]https://datatracker.ietf.org/doc/html/rfc4787 [2]https://datatracker.ietf.org/doc/html/rfc5382 [3]https://datatracker.ietf.org/doc/html/rfc7857 Signed-off-by: Martin Kalcok --- northd/northd.c | 82 - northd/ovn-northd.8.xml | 29 +++ tests/ovn-northd.at | 15 +++- tests/system-ovn.at | 69 ++ 4 files changed, 185 insertions(+), 10 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 2c3560ce2..4b79b357c 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -14437,21 +14437,28 @@ build_lrouter_out_is_dnat_local(struct lflow_table *lflows, } static void -build_lrouter_out_snat_match(struct lflow_table *lflows, - const struct ovn_datapath *od, - const struct nbrec_nat *nat, struct ds *match, - bool distributed_nat, int cidr_bits, bool is_v6, - struct ovn_port *l3dgw_port, - struct lflow_ref *lflow_ref) +build_lrouter_out_snat_direction_match__(struct lflow_table *lflows, + const struct ovn_datapath *od, + const struct nbrec_nat *nat, + struct ds *match, + bool distributed_nat, int cidr_bits, + bool is_v6, + struct ovn_port *l3dgw_port, + struct lflow_ref *lflow_ref, + bool is_reverse) { ds_clear(match); -ds_put_format(match, "ip && ip%c.src == %s", is_v6 ? '6' : '4', +ds_put_format(match, "ip && ip%c.%s == %s", + is_v6 ? '6' : '4', + is_reverse ? "dst" : "src", nat->logical_ip); if (!od->is_gw_router) { /* Distributed router. */ -ds_put_format(match, " && outport == %s", l3dgw_port->json_key); +ds_put_format(match, " && %s == %s", + is_reverse ? "inport" : "outport", + l3dgw_port->json_key);
[ovs-dev] [Patch ovn 1/2] actions.c/h: Enable specifying zone for ct_commit.
Action `ct_commit` currently does not allow specifying zone into which connection is committed. For example, in LR datapath, the `ct_commit` will always use the DNAT zone. This change adds option to use `ct_commit(snat)` or `ct_commit(dnat)` to explicitly specify the zone into which the connection will be committed. Original behavior of `ct_commit` without the arguments remains unchanged. Signed-off-by: Martin Kalcok --- include/ovn/actions.h | 9 + lib/actions.c | 20 +++- ovn-sb.xml| 9 + 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 49fb96fc6..ce9597cf5 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -259,11 +259,20 @@ struct ovnact_ct_next { uint8_t ltable;/* Logical table ID of next table. */ }; +/* Conntrack zone to be used for commiting CT entries by the action. + * DEFAULT uses default zone for given datapath. */ +enum ovnact_ct_zone { +OVNACT_CT_ZONE_DEFAULT, +OVNACT_CT_ZONE_SNAT, +OVNACT_CT_ZONE_DNAT, +}; + /* OVNACT_CT_COMMIT_V1. */ struct ovnact_ct_commit_v1 { struct ovnact ovnact; uint32_t ct_mark, ct_mark_mask; ovs_be128 ct_label, ct_label_mask; +enum ovnact_ct_zone zone; }; /* Type of NAT used for the particular action. diff --git a/lib/actions.c b/lib/actions.c index a45874dfb..319e65b6f 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -707,6 +707,7 @@ static void parse_ct_commit_v1_arg(struct action_context *ctx, struct ovnact_ct_commit_v1 *cc) { +cc->zone = OVNACT_CT_ZONE_DEFAULT; if (lexer_match_id(ctx->lexer, "ct_mark")) { if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { return; @@ -737,6 +738,10 @@ parse_ct_commit_v1_arg(struct action_context *ctx, return; } lexer_get(ctx->lexer); +} else if (lexer_match_id(ctx->lexer, "snat")) { +cc->zone = OVNACT_CT_ZONE_SNAT; +} else if (lexer_match_id(ctx->lexer, "dnat")) { +cc->zone = OVNACT_CT_ZONE_DNAT; } else { lexer_syntax_error(ctx->lexer, NULL); } @@ -814,7 +819,20 @@ encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc, struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts); ct->flags = NX_CT_F_COMMIT; ct->recirc_table = NX_CT_RECIRC_NONE; -ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); +switch (cc->zone) { +case OVNACT_CT_ZONE_SNAT: +ct->zone_src.field = mf_from_id(MFF_LOG_SNAT_ZONE); +break; + +case OVNACT_CT_ZONE_DNAT: +ct->zone_src.field = mf_from_id(MFF_LOG_DNAT_ZONE); +break; + +case OVNACT_CT_ZONE_DEFAULT: +default: +ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE); +break; +} ct->zone_src.ofs = 0; ct->zone_src.n_bits = 16; diff --git a/ovn-sb.xml b/ovn-sb.xml index ac4e585f2..66cb9747d 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -1405,6 +1405,8 @@ ct_commit { ct_mark=value[/mask]; }; ct_commit { ct_label=value[/mask]; }; ct_commit { ct_mark=value[/mask]; ct_label=value[/mask]; }; +ct_commit(snat); +ct_commit(dnat); Commit the flow to the connection tracking entry associated with it @@ -1421,6 +1423,13 @@ in order to have specific bits set. + +Parameters ct_commit(snat) or ct_commit(dnat) + can be used to explicitly specify into which zone should be +connection committed. Without this parameter, the connection is +committed to the default zone for the Datapath. + + Note that if you want processing to continue in the next table, you must execute the next action after -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v6 2/2] netlink-conntrack: Optimize flushing ct zone.
Previously the kernel did not provide a netlink interface to flush/list only conntrack entries matching a specific zone. With [1] and [2] it is now possible to flush and list conntrack entries filtered by zone. Older kernels not yet supporting this feature will ignore the filter. For the list request that means just returning all entries (which we can then filter in userspace as before). For the flush request that means deleting all conntrack entries. The implementation is now identical to the windows one, so we combine them. These significantly improves the performance of flushing conntrack zones when the conntrack table is large. Since flushing a conntrack zone is normally triggered via an openflow command it blocks the main ovs thread and thereby also blocks new flows from being applied. Using this new feature we can reduce the flushing time for zones by around 93%. In combination with OVN the creation of a Logical_Router (which causes the flushing of a ct zone) could block other operations, e.g. the failover of Logical_Routers (as they cause new flows to be created). This is visible from a user perspective as a ovn-controller that is idle (as it waits for vswitchd) and vswitchd reporting: "blocked 1000 ms waiting for main to quiesce" (potentially with ever increasing times). The following performance tests where run in a qemu vm with 500.000 conntrack entries distributed evenly over 500 ct zones using `ovstest test-netlink-conntrack flush zone=`. | flush zone with 1000 entries | flush zone with no entry | +-+--+-+--| | with the patch| without | with the patch| without | +--+--+--+--+--+--| | v6.8-rc4 | v6.7.1 | v6.8-rc4 | v6.8-rc4 | v6.7.1 | v6.8-rc4 | +-+--+--+--+--+--+--| | Min | 0.260 | 3.946 | 3.497 | 0.228 | 3.462 | 3.212 | | Median | 0.319 | 4.237 | 4.349 | 0.298 | 4.460 | 4.010 | | 90%ile | 0.335 | 4.367 | 4.522 | 0.325 | 4.662 | 4.572 | | 99%ile | 0.348 | 4.495 | 4.773 | 0.340 | 4.931 | 6.003 | | Max | 0.362 | 4.543 | 5.054 | 0.348 | 5.390 | 6.396 | | Mean| 0.320 | 4.236 | 4.331 | 0.296 | 4.430 | 4.071 | | Total | 80.02 | 1058| 1082| 73.93 | 1107| 1017| [1]: https://github.com/torvalds/linux/commit/eff3c558bb7e61c41b53e4c8130e514a5a4df9ba [2]: https://github.com/torvalds/linux/commit/fa173a1b4e3fd1ab5451cbc57de6fc624c824b0a Co-Authored-By: Luca Czesla Signed-off-by: Luca Czesla Co-Authored-By: Max Lamprecht Signed-off-by: Max Lamprecht Signed-off-by: Felix Huettner --- v5->v6: none v4->v5: none v3->v4: - combine the flush logic with windows implementation v2->v3: - update description to include upstream fix (Thanks to Ilya for finding that issue) v1->v2: - fixed wrong signed-off-by lib/netlink-conntrack.c | 52 - tests/system-traffic.at | 8 +++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c index 492bfcffb..d9015d9f0 100644 --- a/lib/netlink-conntrack.c +++ b/lib/netlink-conntrack.c @@ -141,6 +141,9 @@ nl_ct_dump_start(struct nl_ct_dump_state **statep, const uint16_t *zone, nl_msg_put_nfgenmsg(&state->buf, 0, AF_UNSPEC, NFNL_SUBSYS_CTNETLINK, IPCTNL_MSG_CT_GET, NLM_F_REQUEST); +if (zone) { +nl_msg_put_be16(&state->buf, CTA_ZONE, htons(*zone)); +} nl_dump_start(&state->dump, NETLINK_NETFILTER, &state->buf); ofpbuf_clear(&state->buf); @@ -263,11 +266,9 @@ out: return err; } -#ifdef _WIN32 -int -nl_ct_flush_zone(uint16_t flush_zone) +static int +nl_ct_flush_zone_with_cta_zone(uint16_t flush_zone) { -/* Windows can flush a specific zone */ struct ofpbuf buf; int err; @@ -282,24 +283,63 @@ nl_ct_flush_zone(uint16_t flush_zone) return err; } + +#ifdef _WIN32 +int +nl_ct_flush_zone(uint16_t flush_zone) +{ +return nl_ct_flush_zone_with_cta_zone(flush_zone); +} #else + +static bool +netlink_flush_supports_zone(void) +{ +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; +static bool supported = false; + +if (ovsthread_once_start(&once)) { +if (ovs_kernel_is_version_or_newer(6, 8)) { +supported = true; +} else { +VLOG_INFO("disabling conntrack flush by zone. " + "Not supported in Linux kernel"); +} +ovsthread_once_done(&once); +} +return supported; +} + int nl_ct_flush_zone(uint16_t flush_zone) { -/* Apparently, there's no netlink interface to flush a specific zone. +/* In older kernels, there was no netlink interface to flush a specific + * conntrack zone. * This code d
[ovs-dev] [PATCH v6 1/2] util: Support checking for kernel versions.
Extract checking for a given kernel version to a separate function. It will be used also in the next patch. Signed-off-by: Felix Huettner --- v5->v6: - fix ovs_kernel_is_version_or_newer returning false if major and minor are equal (thanks Mike) v4->v5: - fix wrong ifdef that broke on macos - fix ovs_kernel_is_version_or_newer working in reverse than desired - ovs_kernel_is_version_or_newer now always returns false if uname errors (Thanks Eelco) v4: - extract function to check kernel version lib/netdev-linux.c | 15 +++ lib/util.c | 27 +++ lib/util.h | 4 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index bf91ef462..a8424ed82 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -40,7 +40,6 @@ #include #include #include -#include #include #include #include @@ -6427,18 +6426,10 @@ getqdisc_is_safe(void) static bool safe = false; if (ovsthread_once_start(&once)) { -struct utsname utsname; -int major, minor; - -if (uname(&utsname) == -1) { -VLOG_WARN("uname failed (%s)", ovs_strerror(errno)); -} else if (!ovs_scan(utsname.release, "%d.%d", &major, &minor)) { -VLOG_WARN("uname reported bad OS release (%s)", utsname.release); -} else if (major < 2 || (major == 2 && minor < 35)) { -VLOG_INFO("disabling unsafe RTM_GETQDISC in Linux kernel %s", - utsname.release); -} else { +if (ovs_kernel_is_version_or_newer(2, 35)) { safe = true; +} else { +VLOG_INFO("disabling unsafe RTM_GETQDISC in Linux kernel"); } ovsthread_once_done(&once); } diff --git a/lib/util.c b/lib/util.c index 3fb3a4b40..5c31d983a 100644 --- a/lib/util.c +++ b/lib/util.c @@ -27,6 +27,7 @@ #include #ifdef __linux__ #include +#include #endif #include #include @@ -2500,3 +2501,29 @@ OVS_CONSTRUCTOR(winsock_start) { } } #endif + +#ifdef __linux__ +bool +ovs_kernel_is_version_or_newer(int target_major, int target_minor) +{ +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; +static int current_major, current_minor = -1; + +if (ovsthread_once_start(&once)) { +struct utsname utsname; + +if (uname(&utsname) == -1) { +VLOG_WARN("uname failed (%s)", ovs_strerror(errno)); +} else if (!ovs_scan(utsname.release, "%d.%d", +¤t_major, ¤t_minor)) { +VLOG_WARN("uname reported bad OS release (%s)", utsname.release); +} +ovsthread_once_done(&once); +} +if (current_major == -1 || current_minor == -1) { +return false; +} +return current_major > target_major || ( +current_major == target_major && current_minor >= target_minor); +} +#endif diff --git a/lib/util.h b/lib/util.h index f2d45bcac..55718fd87 100644 --- a/lib/util.h +++ b/lib/util.h @@ -611,4 +611,8 @@ int ftruncate(int fd, off_t length); } #endif +#ifdef __linux__ +bool ovs_kernel_is_version_or_newer(int target_major, int target_minor); +#endif + #endif /* util.h */ base-commit: 436aba68d52891fb5775ec7651282ccf9d04176b -- 2.43.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev