Add comprehensive SCTP protocol support #2031
Conversation
jordanvrtanoski
force-pushed
the
sctp
branch
from
07854bb to
0844b5c
Compare
jordanvrtanoski
force-pushed
the
sctp
branch
from
0844b5c to
fb63b3b
Compare
Dimi1010
left a comment
Dimi1010
left a comment
There was a problem hiding this comment.
Thank you for the submission. :)
| /// @class SctpChunk | ||
| /// A wrapper class for SCTP chunks. This class does not create or modify chunk records, | ||
| /// but rather serves as a wrapper and provides useful methods for retrieving data from them | ||
| class SctpChunk |
There was a problem hiding this comment.
This is essentially a view class right?
Somewhat thinking out loud but can we split this class into several view subclasses for each chunk type?
Currently the class seems a bit unwieldy with all the different chunk type methods?
Can we instead have:
- SctpChunk - Common Methods (
getChunkType(), etc)- SctpDataChunk - A data chunk.
- SctpInitChunk - Init/InitAck chunk.
- ScptSackChunk - Sack chunk.
- ...
Since the chunk objects only hold a pointer, they can be treated as value objects, similar to std::string_view.
class SctpChunk
{
/* Generic chunk methods */
SctpChunkType getChunkType() const;
}
class SctpDataChunk : public SctpChunk {
SctpDataChunk(SctpChunk chunk) { /* validate correct chunk type */ }
/* Data specific chunk accessors */
}
// Other chunk types.The inheritance in the above example is mostly for code reuse of the common methods, not for polymorphism, therefor the lack of any virtual methods.
Do you think that would work or is there something I am missing?
There was a problem hiding this comment.
That's good observation, let me see what it takes to refactor it to be more aligned to OO principles.
There was a problem hiding this comment.
The reason the SctpChunk is designed as a monolith class in the first pace is to avoid the heap overhead that inheritance would create, as well as it's impact on the cache as the objects in the hierarchy have bad locality.
Having the chunks as views classes on the SctpChunk will address my original performance concerns, so I will refactor the code based on the suggestion.
|
Converted the PR to draft to address the proposed refactoring. |
There was a problem hiding this comment.
Can you please add the original pcap file used for these .dat files?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #2031 +/- ##
==========================================
- Coverage 83.47% 83.45% -0.02%
==========================================
Files 311 314 +3
Lines 54574 58524 +3950
Branches 11516 12677 +1161
==========================================
+ Hits 45555 48842 +3287
- Misses 7777 8481 +704
+ Partials 1242 1201 -41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Implement full SCTP (Stream Control Transmission Protocol) layer with support for: Core Protocol (RFC 9260): - All standard chunk types (DATA, INIT, INIT-ACK, SACK, HEARTBEAT, ABORT, SHUTDOWN, ERROR, COOKIE-ECHO, COOKIE-ACK, ECNE, CWR, SHUTDOWN-COMPLETE) - CRC32c checksum calculation and validation - Chunk and parameter action bits helpers - Bundling validation per RFC 9260 - Host Name Address deprecation detection Extensions: - AUTH chunk with HMAC-SHA1/SHA256 computation and verification (RFC 4895) - ASCONF/ASCONF-ACK for dynamic address reconfiguration (RFC 5061) - RE-CONFIG for stream reconfiguration (RFC 6525) - FORWARD-TSN for partial reliability (RFC 3758) - I-DATA and I-FORWARD-TSN for message interleaving (RFC 8260) - PAD chunk (RFC 4820) - NR-SACK experimental support (IANA registered, draft-based) - Zero Checksum Acceptable parameter (RFC 9653) Additional features: - Parameter iterators for INIT, RE-CONFIG, and ASCONF chunks - Error cause iterator for ABORT/ERROR chunks - Extended PPID enum with 70+ protocol identifiers from IANA registry - Chunk creation APIs for building SCTP packets Includes 72 test cases covering parsing, creation, and validation.
3 participants
Implement full SCTP (Stream Control Transmission Protocol) layer with support for:
Core Protocol (RFC 9260):
Extensions:
Additional features:
Includes 72 test cases covering parsing, creation, and validation.