Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[draft] Rewrite suricatasc in Rust - v0 #8571

Closed
wants to merge 3 commits into from

Conversation

jasonish
Copy link
Member

@jasonish jasonish commented Mar 4, 2023

This is a reimplementation of suricatasc I've been sitting on for about a year
as I didn't get around to fixing the Windows compilation issue (now fixed).

What I don't like is how the vendor size has grown due to the Rust
reedline-like lib, "rustyline" pulling in all sorts of Windows crates as it's
cross platform. We could just delete those and we don't build the full
suricatasc tool on Windows yet due to unix sockets being required.

This aims to be 100% compatible with the current Python "suricatasc".

Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6287

@codecov
Copy link

codecov bot commented Mar 4, 2023

Codecov Report

Merging #8571 (32f7c2e) into master (185f605) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8571      +/-   ##
==========================================
- Coverage   82.16%   82.15%   -0.01%     
==========================================
  Files         968      969       +1     
  Lines      274204   274213       +9     
==========================================
- Hits       225302   225292      -10     
- Misses      48902    48921      +19     
Flag Coverage Δ
fuzzcorpus 64.08% <ø> (-0.01%) ⬇️
suricata-verify 60.86% <ø> (-0.03%) ⬇️
unittests 62.88% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.flow.end.tcp_state.last_ack 1004 257 25.6%
SURI_TLPR1_stats_chk
.flow.end.tcp_state.fin_wait2 156072 127814 81.89%
.flow.memuse 527608240 663423400 125.74%

Pipeline 12678

@victorjulien
Copy link
Member

Do we have options to selectively vendor things, e.g. indeed excluding windows specific crates?

@jasonish
Copy link
Member Author

jasonish commented Mar 6, 2023

Do we have options to selectively vendor things, e.g. indeed excluding windows specific crates?

No, its been a long standing issue: rust-lang/cargo#7058

Most common option these days is to delete the stuff you know won't be needed.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 13590

@jasonish jasonish self-assigned this May 5, 2023
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.tcp.rst 113274 144996 128.0%

Pipeline 13620

@victorjulien
Copy link
Member

What is the difference in the size of the dist with and without the windows stuff?

@jasonish
Copy link
Member Author

jasonish commented May 6, 2023

What is the difference in the size of the dist with and without the windows stuff?

  • 36 MB with all the vendor stuff
  • 29 MB with the bulk of the Windows stuff removed (all the .a files)

7.0.0-rc1 was 22 MB.

However, pruning the vendor directory still causes failures build failures on Windows. So far only the the verification of dependencies, not actually compile. So maybe that could be patched up at vendor time as well, but probably to find another issue. Which is silly, cause we don't even build the part that enters this dependency chain on Windows. Maybe refactoring the layout a bit and some autoconf glue might be a better path to look at next.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 13637

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.tcp.rst 113274 145173 128.16%

Pipeline 13741

@catenacyber catenacyber added the needs rebase Needs rebase to master label Jun 20, 2023
@catenacyber
Copy link
Contributor

What do you expect here @jasonish ?

@jasonish
Copy link
Member Author

What do you expect here @jasonish ?

This is just more of an experimentation with eliminating Python from end-user distributable, given its probably only get harder to keep such tooling working as Python marches forward. I plan to keep this up to date, its not 100% yet replacing our Pythonctl tool.

@victorjulien victorjulien added this to the 8.0 milestone Jul 11, 2023
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.tcp.rst 113274 145173 128.16%

Pipeline 13741

@catenacyber
Copy link
Contributor

Is there a ticket for this ?

@jasonish
Copy link
Member Author

Is there a ticket for this ?

Is now https://redmine.openinfosecfoundation.org/issues/6287.

@jasonish jasonish force-pushed the rust-suricatactl/v0 branch 4 times, most recently from 78c0c54 to 4cb041b Compare September 6, 2023 23:04
This is a re-implementation of suricatasc program in Rust that
attempts to be a 100% drop-in replacement.

This adds 2 crates, the "client" and "suricatasc". The idea is that
the client is a more permissively licensed crate for communicating
with a Suricata server.
As we have 2 Windows builds, do one using the release-style
distribution file.
@jasonish jasonish removed the needs rebase Needs rebase to master label Sep 7, 2023
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 15851

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 15852

@jasonish
Copy link
Member Author

Replaced by #9817.

@jasonish jasonish closed this Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants