claircore icon indicating copy to clipboard operation
claircore copied to clipboard

cvss: swap parsers for ragel-generated parsers

Open hdonnay opened this issue 1 year ago • 3 comments
trafficstars

Had a wild hair over the weekend a wrote ragel machines for CVSS vectors. This also needed a second CVSSv4 parser for the fragments that are used in the scores, so I moved the static score data into a code generator command to avoid having a second v4 parser linked into a resulting binary.

Benchmark results
commit c027695369c7728bcabddf0320ef599ed1a68a59 (HEAD)
Author: Hank Donnay <[email protected]>
Date:   Mon Jul 22 10:02:29 2024 -0500

    cvss: add benchmarks

    Add some benchmarks to be able to measure changes to this package.

    Signed-off-by: Hank Donnay <[email protected]>
goos: linux
goarch: amd64
pkg: github.com/quay/claircore/toolkit/types/cvss
cpu: 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
BenchmarkUnmarshal/V2/List-16             125175               861.5 ns/op        58.91 MB/s        4544 B/op        224 allocs/op
--- BENCH: BenchmarkUnmarshal/V2/List-16
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
BenchmarkUnmarshal/V2/Heartbleed-16      1847350               689.6 ns/op        37.70 MB/s         176 B/op         10 allocs/op
BenchmarkUnmarshal/V3/List-16              35264              1047 ns/op          42.71 MB/s       13054 B/op        687 allocs/op
--- BENCH: BenchmarkUnmarshal/V3/List-16
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
BenchmarkUnmarshal/V3/Heartbleed-16      1447320               804.9 ns/op        54.66 MB/s         208 B/op         13 allocs/op
BenchmarkUnmarshal/V4/List-16              23634              1214 ns/op          58.32 MB/s       21591 B/op       1097 allocs/op
--- BENCH: BenchmarkUnmarshal/V4/List-16
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
BenchmarkUnmarshal/V4/B-16               1312682               963.7 ns/op        65.38 MB/s         228 B/op         15 allocs/op
BenchmarkUnmarshal/V4/BT-16              1246820              1032 ns/op          64.90 MB/s         376 B/op         17 allocs/op
BenchmarkUnmarshal/V4/BE-16               777355              1577 ns/op          91.30 MB/s         748 B/op         31 allocs/op
BenchmarkUnmarshal/V4/BTES-16             592453              1773 ns/op         100.42 MB/s         820 B/op         37 allocs/op
PASS
ok      github.com/quay/claircore/toolkit/types/cvss    15.510s
commit aa0cc9ba5576284917e2169b45848ac7e4ebaf5a (HEAD, hack/cvss-ragel)
Author: Hank Donnay <[email protected]>
Date:   Sun Jul 21 18:16:11 2024 -0500

    cvss: move v4 scoring data to generated code

    This moves the `scoreData` variable from the init context to a code
    generation tool. This should have a positive impact on binary size due
    to moving the fragment parser and some helper functions to the `v4data`
    package and not using them at runtime.

    Signed-off-by: Hank Donnay <[email protected]>
goos: linux
goarch: amd64
pkg: github.com/quay/claircore/toolkit/types/cvss
cpu: 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
BenchmarkUnmarshal/V2/List-16             296811               353.2 ns/op       143.69 MB/s           0 B/op          0 allocs/op
--- BENCH: BenchmarkUnmarshal/V2/List-16
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
    cvss_bench_test.go:125: allocations reported per list, not per vector (12 elements)
BenchmarkUnmarshal/V2/Heartbleed-16      6160509               211.6 ns/op       122.87 MB/s           0 B/op          0 allocs/op
BenchmarkUnmarshal/V3/List-16             137432               251.4 ns/op       177.89 MB/s           0 B/op          0 allocs/op
--- BENCH: BenchmarkUnmarshal/V3/List-16
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
    cvss_bench_test.go:91: allocations reported per list, not per vector (33 elements)
BenchmarkUnmarshal/V3/Heartbleed-16      4883041               293.4 ns/op       149.94 MB/s           0 B/op          0 allocs/op
BenchmarkUnmarshal/V4/List-16              72885               409.3 ns/op       173.03 MB/s           0 B/op          0 allocs/op
--- BENCH: BenchmarkUnmarshal/V4/List-16
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
    cvss_bench_test.go:53: allocations reported per list, not per vector (43 elements)
BenchmarkUnmarshal/V4/B-16               3137493               340.8 ns/op       184.87 MB/s           0 B/op          0 allocs/op
BenchmarkUnmarshal/V4/BT-16              2728364               445.0 ns/op       150.58 MB/s           0 B/op          0 allocs/op
BenchmarkUnmarshal/V4/BE-16              1257572               958.2 ns/op       150.28 MB/s           0 B/op          0 allocs/op
BenchmarkUnmarshal/V4/BTES-16            1000000              1186 ns/op         150.04 MB/s           0 B/op          0 allocs/op
PASS
ok      github.com/quay/claircore/toolkit/types/cvss    14.826s

hdonnay avatar Jul 22 '24 15:07 hdonnay

As an added benefit, I made the errors indicate where the parser bails out:

% go test -v -run V4/Error/02
=== RUN   TestV4
=== RUN   TestV4/Error
=== RUN   TestV4/Error/#02
    cvss_v4_test.go:25: CVSS:4.0/AV:Z/AC:L/AT:N/PR:H/UI:N/VC:L/SC:N/VI:L/SI:N/VA:N/SA:N
    cvss_v4_test.go:25: cvss v4: malformed vector: unexpected character #13: CVSS:4.0/AV:→Z←/AC:L/AT:N/PR:H/UI:N/VC:L/SC:N/VI:L/SI:N/VA:N/SA:N
--- PASS: TestV4 (0.00s)
    --- PASS: TestV4/Error (0.00s)
        --- PASS: TestV4/Error/#02 (0.00s)
PASS
ok      github.com/quay/claircore/toolkit/types/cvss    0.002s

hdonnay avatar Jul 22 '24 16:07 hdonnay

Codecov Report

Attention: Patch coverage is 33.64486% with 355 lines in your changes missing coverage. Please review.

Project coverage is 55.25%. Comparing base (483f858) to head (42d3b35). Report is 3 commits behind head on main.

Files Patch % Lines
toolkit/types/cvss/internal/cmd/revlookup/main.go 0.00% 58 Missing :warning:
toolkit/types/cvss/internal/cmd/v4data/main.go 0.00% 56 Missing :warning:
toolkit/types/cvss/internal/cmd/v4data/parser.go 0.00% 54 Missing :warning:
toolkit/internal/cmd/mkragel/main.go 0.00% 51 Missing :warning:
toolkit/types/cvss/internal/cmd/v4data/max_frag.go 0.00% 44 Missing :warning:
...ypes/cvss/internal/cmd/v4data/macrovector_score.go 0.00% 26 Missing :warning:
toolkit/types/cvss/internal/cmd/v4data/eq_depth.go 0.00% 22 Missing :warning:
...it/types/cvss/internal/cmd/v4data/metrics_in_eq.go 0.00% 22 Missing :warning:
toolkit/types/cvss/cvss_v2_parse.go 90.36% 6 Missing and 2 partials :warning:
toolkit/types/cvss/cvss_v4_parse.go 85.45% 5 Missing and 3 partials :warning:
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1367      +/-   ##
==========================================
- Coverage   56.24%   55.25%   -0.99%     
==========================================
  Files         267      278      +11     
  Lines       16854    17199     +345     
==========================================
+ Hits         9479     9504      +25     
- Misses       6414     6736     +322     
+ Partials      961      959       -2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 22 '24 17:07 codecov[bot]

@crozzy the red on this is just codecov complaining that the new code generation tools dropped the test coverage

hdonnay avatar Jul 26 '24 22:07 hdonnay

Okay, should be good to go. Moved the helper from a shell script to a small go package and added docs about how it works. Reorganized when things are introduced to avoid some churn.

hdonnay avatar Aug 05 '24 21:08 hdonnay

/fast-forward

hdonnay avatar Aug 05 '24 22:08 hdonnay