Add vectorscan (same abi as hyperscan) for arm 64 #29881

sambercovici · 2023-09-30T02:59:42Z

Commit Message: Fix #29276
Additional Description: Use Vectorscan for Arm 64 CPUs
Risk Level: Low
Testing: Covered by original commit
Docs Changes:
Release Notes:
Platform Specific Features: aarch64

repokitteh-read-only · 2023-09-30T02:59:47Z

Hi @sambercovici, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #29881 was opened by sambercovici.

see: more, trace.

repokitteh-read-only · 2023-09-30T02:59:51Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @htuch

🐱

Caused by: #29881 was opened by sambercovici.

see: more, trace.

Signed-off-by: Samuel Bercovici <[email protected]>

this is now handled downstream Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: Samuel Bercovici <[email protected]>

…y#29845) Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: Samuel Bercovici <[email protected]>

Signed-off-by: Samuel Bercovici <[email protected]>

…scan)-for-Arm-64

Signed-off-by: Samuel Bercovici <[email protected]>

htuch · 2023-10-02T03:13:11Z

bazel/foreign_cc/vectorscan.patch

+ #define AARCH64_CPUID_INLINE_H_
+
+ #if defined(__linux__)
+#include <asm/hwcap.h>


Can you fix this upstream?

opened VectorCamp/vectorscan#176 waiting for response on ISSUE-1 there

VectorCamp/vectorscan#180

Will this be fixed before we merge?

You may want to wait a few days, it will be included yes, but I'm fixing clang 15/16 issues right now as they are higher priority and ironing out a few issues with the new CI. I'll post an update here when that happens.

htuch · 2023-10-02T03:17:22Z

bazel/repository_locations.bzl

@@ -484,6 +484,24 @@ REPOSITORY_LOCATIONS_SPEC = dict(
 license = "BSD-3-Clause",
 license_url = "https:/intel/hyperscan/blob/v{version}/LICENSE",
 ),
+ io_vectorscan = dict(
+ project_name = "Vectorscan",


Can you run through https:/envoyproxy/envoy/blob/main/DEPENDENCY_POLICY.md?

Given regex processing can in many paces be considered security sensitive (correctness of rule matching, CPU/memory DoS for example), I'd like to weigh up the win we're getting from this dependency, which seems to be a community fork.

Hi @htuch

The vectorscan is a company (https://www.vectorcamp.gr/) and community led fork (under paid contract: https://www.vectorcamp.gr/2020/05/09/we-are-thrilled-to-announce-our-first-customer/) to support hyperscan ABI in Arm CPUs and additional CPUs with the same license, same tests, scans and fixes.
see: #29276 (comment) for additional information on CI builds, tests, etc.

Let me know if you want me to fill the items as described in: https:/envoyproxy/envoy/blob/main/DEPENDENCY_POLICY.md

-Sam.

Thanks - yeah, please fill in the table so we can evaluate this new dep.

Adding VectorCamp/vectorscan#177 as reference.

Cloud Native Computing Foundation (CNCF) approved license - BSD License - https:/VectorCamp/vectorscan/blob/vectorscan/5.4.10.1/LICENSE

Dependencies must not substantially increase the binary size unless they are optional (i.e. confined to specific extensions) - same as hyperscan which is approved.

No duplication of existing dependencies - Hyperscan does not cover Arm CPUs, Vectorscan is used instead.

Hosted on a git repository and the archive fetch must directly reference this repository. We will NOT support intermediate artifacts built by-hand located on GCS, S3, etc. - YES - https:/VectorCamp/vectorscan

CVE history appears reasonable, no pathological CVE arcs - Should be similar to Hyperscan

Code review (ideally PRs) before merge - YES - see: Add Vectorscan (same ABI as Hyperscan) for Arm 64 #29276 (comment)

Security vulnerability process exists, with contact details and reporting/disclosure process - YES - see: Governance Questions VectorCamp/vectorscan#177 (comment)

1 contributor responsible for a non-trivial number of commits - YES - see: https:/VectorCamp/vectorscan/graphs/contributors

Tests run in CI - YES - see: Add Vectorscan (same ABI as Hyperscan) for Arm 64 #29276 (comment) and Governance Questions VectorCamp/vectorscan#177 (comment)

High test coverage (also static/dynamic analysis, fuzzing) - YES - see: Add Vectorscan (same ABI as Hyperscan) for Arm 64 #29276 (comment)

Envoy can obtain advanced notification of vulnerabilities or of security releases - Not sure

Do other significant projects have shared fate by using this dependency? - ClickHouse

Releases (with release notes) - YES

Commits/releases in last 90 days - YES - see: https:/VectorCamp/vectorscan/graphs/commit-activity

OSSF Scorecard result is low. Security policy and code review would help.

scorecard --repo=https:/VectorCamp/vectorscan/ RESULTS ------- Aggregate score: 4.1 / 10 Check scores: |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | SCORE | NAME | REASON | DOCUMENTATION/REMEDIATION | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Binary-Artifacts | no binaries found in the repo | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#binary-artifacts | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 1 / 10 | Branch-Protection | branch protection is not | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#branch-protection | | | | maximal on development and all | | | | | release branches | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | CI-Tests | 0 out of 6 merged PRs | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#ci-tests | | | | checked by a CI test -- score | | | | | normalized to 0 | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | CII-Best-Practices | no effort to earn an OpenSSF | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#cii-best-practices | | | | best practices badge detected | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 1 / 10 | Code-Review | found 6 unreviewed changesets | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#code-review | | | | out of 7 -- score normalized | | | | | to 1 | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Contributors | 6 different organizations | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#contributors | | | | found -- score normalized to | | | | | 10 | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Dangerous-Workflow | no workflows found | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#dangerous-workflow | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Dependency-Update-Tool | no update tool detected | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#dependency-update-tool | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Fuzzing | project is not fuzzed | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#fuzzing | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 9 / 10 | License | license file detected | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#license | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Maintained | 30 commit(s) out of 30 and 0 | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#maintained | | | | issue activity out of 30 found | | | | | in the last 90 days -- score | | | | | normalized to 10 | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Packaging | no published package detected | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#packaging | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Pinned-Dependencies | no dependencies found | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#pinned-dependencies | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | SAST | SAST tool is not run on all | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#sast | | | | commits -- score normalized to | | | | | 0 | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Security-Policy | security policy file not | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#security-policy | | | | detected | | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Signed-Releases | no releases found | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#signed-releases | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Token-Permissions | no github tokens found | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#token-permissions | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Vulnerabilities | no vulnerabilities detected | https:/ossf/scorecard/blob/7a94273ba14c1c702688f9efb57754d9fe112519/docs/checks.md#vulnerabilities | |---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|

fwiw, all PRs are CI tested (about 100 configurations), unfortunately there is a new CI in development now (buildbot-based) which is not yet integrated with Github. This was already in progress for weeks and it should be ready the next days. FTR, here is the new CI: https://buildbot-ci.vectorcamp.gr/

Also, we do the Debian packaging ourselves, it's directly uploaded to Debian and Ubuntu, there is a debian branch for that purpose. We will not produce Binary releases, signed or unsigned within Github. For the same reason, dependency tracking is either done in the CMakeFile or the debian package.

The score is artificially low. see: #29276 (comment)

sambercovici · 2023-10-05T10:59:52Z

Hi @htuch , how can we progress?

htuch · 2023-10-06T03:36:26Z

I think that given we're in the same position as Hyperscan, this is contrib/ only anyway and most tablestakes from our policy are there, we should go ahead. Thanks everyone (including the Vectorscan folks) for your inputs.

jmarantz · 2023-10-12T18:45:01Z

@htuch what's the next step; are more changes required or should we add more review comments? Or is it time to close or merge this?

I have no horse in this race and have not read the code; just trying to figure out how triage the PR as maintainer oncall.

htuch · 2023-10-13T20:47:44Z

I was waiting for https:/envoyproxy/envoy/pull/29881/files#r1348331292 to be resolved, CC @markos who has the AI on that. LGTM when that is fixed, or if the fix is going to be substantively delayed, we can merge now if a followup issue is filed.

alyssawilk · 2023-10-23T18:38:40Z

/wait-any on the unresolved issues being resolved

markos · 2023-10-30T13:58:59Z

@htuch the new release of vectorscan with this fix, will happen the next days. In the meantime, I just learned myself about recent changes in hyperscan license: https://networkbuilders.intel.com/docs/networkbuilders/accelerate-snort-performance-with-hyperscan-and-intel-xeon-processors-on-public-clouds-1680176363.pdf (section 2.2)
In essence, versions 5.5 onwards are closed/proprietary license.

You might want to consider that for the future.

htuch · 2023-10-31T05:08:31Z

Relevant passage is:

The latest open-source version (BSD-3 license) of Hyperscan on Github is 5.4. Intel conducts continuous internal
development and delivers new Hyperscan releases under Intel Proprietary License (IPL) beginning from 5.5 for interested
customers. Please contact authors to learn more about getting new Hyperscan releases.

I think this means we will be unable to promote Hyperscan to a non-contrib extension, unless there is some LTS assurance on security releases. What will Vectorscan do going forward?

CC @envoyproxy/dependency-shepherds

htuch · 2023-11-21T05:33:30Z

@markos LMK when fixed and I can merge.

/wait

markos · 2023-11-21T13:12:08Z

@htuch https:/VectorCamp/vectorscan/releases/tag/vectorscan%2F5.4.11 thanks for being patient.

Signed-off-by: Samuel Bercovici <[email protected]>

sambercovici · 2023-11-21T14:23:57Z

updated sha256 to use the latest 5.4.11 tar.gz.
@htuch , @markos , let me know if anything else is needed.
Otherwise, please approve and merge.

…scan)-for-Arm-64

markos · 2023-11-21T14:25:18Z

@sambercovici @htuch I've already uploaded it to Debian: https://buildd.debian.org/status/package.php?p=vectorscan nothing else is left on my side. Thanks for supporting the project.

…scan)-for-Arm-64

Signed-off-by: Samuel Bercovici <[email protected]>

…scan)-for-Arm-64

Signed-off-by: Samuel Bercovici <[email protected]>

…ttps:/sambercovici/envoy into Add-Vectorscan-(same-ABI-as-Hyperscan)-for-Arm-64 Signed-off-by: Samuel Bercovici <[email protected]>

sambercovici · 2023-11-25T06:05:41Z

Hi @htuch ,

@markos added for vectorscan 5.4.11 mandatory dependencies on pkg-config and libsqlite3-dev.
The original hyperscan does not mandate pgk-config and sqlite3.

Envoy dev/build environment does not include those dependency.
I don't feel comfortable adding those dependencies to address the changes in 5.4.11.
Instead, I created a patch that
(a) reverts VectorCamp/vectorscan#188 and
(b) adds a flag to disable the detection of sqlite3 altogether.

@markos , please consider making pkg-config optional and detect that sqlite3 does not exists, so I can remove that patches.
@htuch, please let me know if you want to handle this differently, or otherwise please approve.

-Sam.

markos · 2023-11-25T10:20:32Z

@markos , please consider making pkg-config optional and detect that sqlite3 does not exists, so I can remove that patches. @htuch, please let me know if you want to handle this differently, or otherwise please approve.

I have no objections making sqlite optional, but pkg-config will stay. I find no reason to duplicate its functionality in the cmakefiles and in the future other possible dependencies will also be able to use it. It's in every distro and it's a really small dependency.

sambercovici · 2023-11-26T18:33:56Z

@htuch , I suggest we merge as is and when the build container(s) / dev instructions adds pkg-config, I will remove the patch part NOT mandating it, ok?

…scan)-for-Arm-64

htuch

LGTM but please followup to remove this patch!

htuch · 2023-11-28T23:09:37Z

OK, this has lingered for super long so let's jut merge but I think we shouldn't need an sqlite dependency and I'm hoping there is some clean way to resolve the pkgconfig thing.

sambercovici requested a review from soulxu as a code owner September 30, 2023 02:59

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Sep 30, 2023

repokitteh-read-only bot assigned htuch Sep 30, 2023

sambercovici and others added 6 commits September 30, 2023 03:01

add vercotrscan for arm64

c066771

Signed-off-by: Samuel Bercovici <[email protected]>

ci/publishing: Remove postsubmit docs build (envoyproxy#29853)

4586d31

this is now handled downstream Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: Samuel Bercovici <[email protected]>

docs/build: Assorted fixes for building as external target (envoyprox…

94de4b4

…y#29845) Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: Samuel Bercovici <[email protected]>

update tools/spelling/spelling_dictionary.txt

00a16bc

Signed-off-by: Samuel Bercovici <[email protected]>

adding note for support for Arm 64

cd61a64

Signed-off-by: Samuel Bercovici <[email protected]>

after code format fix

8b34041

Signed-off-by: Samuel Bercovici <[email protected]>

sambercovici force-pushed the Add-Vectorscan-(same-ABI-as-Hyperscan)-for-Arm-64 branch from c909e2a to 8b34041 Compare September 30, 2023 03:03

sambercovici added 4 commits September 30, 2023 03:27

rewording

7e5b14a

Signed-off-by: Samuel Bercovici <[email protected]>

Merge branch 'envoyproxy:main' into Add-Vectorscan-(same-ABI-as-Hyper…

a47886b

…scan)-for-Arm-64

Vectorscan to vectorscan

9c1f042

Signed-off-by: Samuel Bercovici <[email protected]>

unwind documentation

a7419b9

Signed-off-by: Samuel Bercovici <[email protected]>

htuch reviewed Oct 2, 2023

View reviewed changes

This was referenced Oct 2, 2023

Governance Questions VectorCamp/vectorscan#177

Closed

OSSF Scorecard result is low VectorCamp/vectorscan#182

Closed

markos mentioned this pull request Oct 10, 2023

Ubuntu 20.04 gcc does not define HWCAP2_SVE2 #180 VectorCamp/vectorscan#186

Merged

repokitteh-read-only bot added the waiting:any label Oct 23, 2023

repokitteh-read-only bot removed the waiting:any label Oct 30, 2023

repokitteh-read-only bot added the waiting label Nov 21, 2023

update sha256 for correct tar.gz

7345159

Signed-off-by: Samuel Bercovici <[email protected]>

sambercovici dismissed htuch’s stale review via 7345159 November 21, 2023 14:22

repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Nov 21, 2023

Merge branch 'envoyproxy:main' into Add-Vectorscan-(same-ABI-as-Hyper…

e659bbe

…scan)-for-Arm-64

htuch enabled auto-merge (squash) November 21, 2023 16:06

sambercovici added 2 commits November 23, 2023 07:51

Merge branch 'envoyproxy:main' into Add-Vectorscan-(same-ABI-as-Hyper…

103aa77

…scan)-for-Arm-64

disable shared libs

dc924ec

Signed-off-by: Samuel Bercovici <[email protected]>

auto-merge was automatically disabled November 25, 2023 01:18
Head branch was pushed to by a user without write access

sambercovici added 6 commits November 25, 2023 05:35

make colm all static

c6d765c

Signed-off-by: Samuel Bercovici <[email protected]>

make colm all static

961fb00

Signed-off-by: Samuel Bercovici <[email protected]>

make ragel all static

8e240a0

Signed-off-by: Samuel Bercovici <[email protected]>

Merge branch 'envoyproxy:main' into Add-Vectorscan-(same-ABI-as-Hyper…

6252a00

…scan)-for-Arm-64

remove dependency on pkg_config and sqlite3 from vectorscan

3ffb0cd

Signed-off-by: Samuel Bercovici <[email protected]>

Merge branch 'Add-Vectorscan-(same-ABI-as-Hyperscan)-for-Arm-64' of h…

9d5148f

…ttps:/sambercovici/envoy into Add-Vectorscan-(same-ABI-as-Hyperscan)-for-Arm-64 Signed-off-by: Samuel Bercovici <[email protected]>

Merge branch 'envoyproxy:main' into Add-Vectorscan-(same-ABI-as-Hyper…

d070ed7

…scan)-for-Arm-64

htuch approved these changes Nov 28, 2023

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 28, 2023

htuch enabled auto-merge (squash) November 28, 2023 23:10

sambercovici mentioned this pull request Nov 28, 2023

add pkg-config to build containers envoyproxy/envoy-build-tools#303

Closed

htuch merged commit dcf8850 into envoyproxy:main Nov 28, 2023
109 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vectorscan (same abi as hyperscan) for arm 64 #29881

Add vectorscan (same abi as hyperscan) for arm 64 #29881

sambercovici commented Sep 30, 2023 •

edited

Loading

repokitteh-read-only bot commented Sep 30, 2023

repokitteh-read-only bot commented Sep 30, 2023

htuch Oct 2, 2023

sambercovici Oct 3, 2023

sambercovici Oct 5, 2023

htuch Oct 6, 2023

markos Oct 6, 2023

htuch Oct 2, 2023

sambercovici Oct 2, 2023

htuch Oct 3, 2023

sambercovici Oct 3, 2023

moderation Oct 4, 2023

markos Oct 4, 2023 •

edited

Loading

markos Oct 4, 2023 •

edited

Loading

sambercovici Oct 4, 2023

sambercovici commented Oct 5, 2023

htuch commented Oct 6, 2023 •

edited

Loading

jmarantz commented Oct 12, 2023 •

edited

Loading

htuch commented Oct 13, 2023 •

edited

Loading

alyssawilk commented Oct 23, 2023

markos commented Oct 30, 2023

htuch commented Oct 31, 2023

htuch commented Nov 21, 2023

markos commented Nov 21, 2023

sambercovici commented Nov 21, 2023

markos commented Nov 21, 2023

sambercovici commented Nov 25, 2023

markos commented Nov 25, 2023

sambercovici commented Nov 26, 2023

htuch left a comment

htuch commented Nov 28, 2023

Add vectorscan (same abi as hyperscan) for arm 64 #29881

Add vectorscan (same abi as hyperscan) for arm 64 #29881

Conversation

sambercovici commented Sep 30, 2023 • edited Loading

repokitteh-read-only bot commented Sep 30, 2023

repokitteh-read-only bot commented Sep 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markos Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

markos Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sambercovici commented Oct 5, 2023

htuch commented Oct 6, 2023 • edited Loading

jmarantz commented Oct 12, 2023 • edited Loading

htuch commented Oct 13, 2023 • edited Loading

alyssawilk commented Oct 23, 2023

markos commented Oct 30, 2023

htuch commented Oct 31, 2023

htuch commented Nov 21, 2023

markos commented Nov 21, 2023

sambercovici commented Nov 21, 2023

markos commented Nov 21, 2023

sambercovici commented Nov 25, 2023

markos commented Nov 25, 2023

sambercovici commented Nov 26, 2023

htuch left a comment

Choose a reason for hiding this comment

htuch commented Nov 28, 2023

sambercovici commented Sep 30, 2023 •

edited

Loading

markos Oct 4, 2023 •

edited

Loading

markos Oct 4, 2023 •

edited

Loading

htuch commented Oct 6, 2023 •

edited

Loading

jmarantz commented Oct 12, 2023 •

edited

Loading

htuch commented Oct 13, 2023 •

edited

Loading