-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
devguide/transactions: refinements and add WIP section #6265
Conversation
- extending/applayer/transactions: third draft - typo fix - add definition of `flow` as found in inliniac's blog - strike off parenthesis text about per-flow state that didn't make much sense - doc/devguide: add transactions.rst - doc/devguide/extending/app-layer/index.rst: add transactions.rst
Add explanation about how transaction state is used for rule matching
Grammar refinement. Add examples of simple protocols. Refine some statements. Nit: fix markup.
add sequence diagrams as pngs generated by mscgen via a script Add image description to Sequence Diagrams Change table of contents be built with markup tell git to ignore *.png files
mscgen is used for generating Sequence Diagrams images. Let's make sure it's present before generating devguide docs.
- detail Template Protocol Sequence Diagram further - add note about HTTP2 transactions
Not addressed in this PR:
|
Codecov Report
@@ Coverage Diff @@
## master #6265 +/- ##
==========================================
- Coverage 76.94% 76.93% -0.02%
==========================================
Files 611 611
Lines 186146 186149 +3
==========================================
- Hits 143236 143212 -24
- Misses 42910 42937 +27
Flags with carried forward coverage won't be shown. Click here to find out more. |
What it looks like right now: https://devguide-transactions.surge.sh/ |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall good to me.
It should be read by the next person who knows nothing yet about transactions to see if it is clear ;-)
================ | ||
|
||
For Suricata, transactions are abstractions that help with detecting and logging. An example of complete transaction is | ||
a pair of messages Request (from frontend to backend) and Response (from backend to frontend) in HTTP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not mention frontend and backend, but rather client and server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall good to me.
It should be read by the next person who knows nothing yet about transactions to see if it is clear ;-)
Ok, I can run it by some folks, but not sure if there's a better way. Would it make sense to post it in our forum and ask for feedback, or is that too much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not mention frontend and backend, but rather client and server
Ok. May I ask why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I can run it by some folks, but not sure if there's a better way.
No, just something to think about next time a newcomer arrives
I would not mention frontend and backend, but rather client and server
For me client/server, as in TCP, is more self-explaining
frontend and backend are rather all parts of the server, the frontend is just the interface, and the backend is where the database, the business logic is. That may come from me being French and using words such as front-office and back-office...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks for taking the time to answer. I think I got used to frontend/backend because of the Postgresql documentation. I'll make another PR incorporating your feedback regarding this and the other comments.
In addition, for file transfer protocols, or similar ones where there may be several messages before the file exchange | ||
is completed (NFS, SMB), it is possible to create a level of abstraction to handle such complexity. This could be achieved by adding phases to the protocol implemented model (e.g., protocol negotiation phase (SMB), request parsed (HTTP), and so on). | ||
|
||
This is controlled by implementing states. In Suricata, those will be enums that are incremented as the parsing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would word it progress state
|
||
Currently we are working to have files be part of the transaction instead of the per-flow state, as seen in https://redmine.openinfosecfoundation.org/issues/4444. | ||
|
||
Another work in progress is to limit the number of transactions per flow, to prevent Denial of Service by quadratic complexity (see https://redmine.openinfosecfoundation.org/issues/4530). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can precise that this DOS is only for protocols which can have multiple transactions at the same time such as HTTP2 so-called streams...
WARNING:
|
Followed by: #6268 |
Multi-tenancy uses loader threads that initialize detection engines. During this, esp the AC family of MPM implementations, there is significant stack usage. In most OS' threads have a lower stack size by default. In Linux, when using the Musl C library, a thread by default gets 128KiB. This patch does 2 things: 1. it centralizes the handling of the `threading.stack-size`. It it is not longer handled by the runmodes, but called from the global initialization logic. 2. it sets a minimum per thread stack size of 512k, unless `threading.stack-size` is set. Ticket: OISF#6265.
Multi-tenancy uses loader threads that initialize detection engines. During this, esp the AC family of MPM implementations, there is significant stack usage. In most OS' threads have a lower stack size by default. In Linux, when using the Musl C library, a thread by default gets 128KiB. This patch does 2 things: 1. it centralizes the handling of the `threading.stack-size`. It it is not longer handled by the runmodes, but called from the global initialization logic. 2. it sets a minimum per thread stack size of 512k, unless `threading.stack-size` is set. Ticket: OISF#6265.
Multi-tenancy uses loader threads that initialize detection engines. During this, esp the AC family of MPM implementations, there is significant stack usage. In most OS' threads have a lower stack size by default. In Linux, when using the Musl C library, a thread by default gets 128KiB. This patch does 2 things: 1. it centralizes the handling of the `threading.stack-size`. It it is not longer handled by the runmodes, but called from the global initialization logic. 2. it sets a minimum per thread stack size of 512k, unless `threading.stack-size` is set. Ticket: OISF#6265.
Multi-tenancy uses loader threads that initialize detection engines. During this, esp the AC family of MPM implementations, there is significant stack usage. In most OS' threads have a lower stack size by default. In Linux, when using the Musl C library, a thread by default gets 128KiB. This patch does 2 things: 1. it centralizes the handling of the `threading.stack-size`. It it is not longer handled by the runmodes, but called from the global initialization logic. 2. it sets a minimum per thread stack size of 512k, unless `threading.stack-size` is set. Ticket: OISF#6265.
Multi-tenancy uses loader threads that initialize detection engines. During this, esp the AC family of MPM implementations, there is significant stack usage. In most OS' threads have a lower stack size by default. In Linux, when using the Musl C library, a thread by default gets 128KiB. This patch does 2 things: 1. it centralizes the handling of the `threading.stack-size`. It it is not longer handled by the runmodes, but called from the global initialization logic. 2. it sets a minimum per thread stack size of 512k, unless `threading.stack-size` is set. Ticket: OISF#6265.
Multi-tenancy uses loader threads that initialize detection engines. During this, esp the AC family of MPM implementations, there is significant stack usage. In most OS' threads have a lower stack size by default. In Linux, when using the Musl C library, a thread by default gets 128KiB. This patch does 2 things: 1. it centralizes the handling of the `threading.stack-size`. It it is not longer handled by the runmodes, but called from the global initialization logic. 2. it sets a minimum per thread stack size of 512k, unless `threading.stack-size` is set. Ticket: OISF#6265.
Transactions documentation - v10
Describe changes:
Link to Redmine issue: https://redmine.openinfosecfoundation.org/issues/4396
Previous PR: #6245