Add additional RoCE extended transport headers (such as DETH and ImmDt)
Allow parsing unreliable datagram headers. The code adds guess_payload_class functions to find the correct sub-header according to BTH.opcode, and also automatic selection of the opcode when possible.
Codecov Report
Merging #3408 (e47caf2) into master (1de61c7) will increase coverage by
0.00%. The diff coverage is97.29%.
:exclamation: Current head e47caf2 differs from pull request most recent head 215e36a. Consider uploading reports for the commit 215e36a to get more accurate results
@@ Coverage Diff @@
## master #3408 +/- ##
=======================================
Coverage 85.95% 85.96%
=======================================
Files 275 275
Lines 56834 56870 +36
=======================================
+ Hits 48854 48886 +32
- Misses 7980 7984 +4
| Impacted Files | Coverage Δ | |
|---|---|---|
| scapy/contrib/roce.py | 95.09% <97.29%> (+1.15%) |
:arrow_up: |
| scapy/arch/windows/__init__.py | 67.73% <0.00%> (-0.57%) |
:arrow_down: |
| scapy/layers/inet.py | 65.92% <0.00%> (-0.08%) |
:arrow_down: |
| scapy/contrib/automotive/gm/gmlan_scanner.py | 86.42% <0.00%> (+0.27%) |
:arrow_up: |
Very sorry about the delay regarding this PR !
Very sorry about the delay regarding this PR !
Don't worry about it 😄
is this PR going to be merged?
Not as-is, no. A lot feedback has not been addressed.
Hi @gpotter2, I would be happy to fix this pull request (and continue with other parts of the RoCE spec), but I wanted to consult with you on how to best represent the RoCE protocol stack within scapy. I believe the main issue with the pull request was how the code examines upper layers in order to determine the next layer. With RoCE, a single opcode field within the BTH header can decide several subsequent layers. For example, an opcode of UD_SEND_ONLY_WITH_IMMEDIATE should tell the parser that the next header after BTH is the DETH header, and the one after that is the ImmDt. I'm not sure what's the best way to represent this in scapy. I can see three alternatives:
- Using logic in guess_payload and post_build to describe the layers that are expected (as this patch tried to do).
- Using conditional fields and storing the full set of alternative headers as one big layer. E.g., we could have all the DETH fields and ImmDt fields moved into BTH under conditions that test the opcode for UD_SEND_ONLY or UD_SEND_ONLY_WITH_IMMEDIATE.
- Creating a separate layer for every combination under BTH and use bind_layers to link each opcode with the appropriate layer. For example, we could have a DETHImmDt layer for the combination of DETH and ImmDt and bind it to BTH when the opcode is UD_SEND_ONLY_WITH_IMMEDIATE.
I can see there are pros and cons to each method, so I wonder if the scapy project has a preferred approach, or if you have any other alternative I haven't considered.
Thanks, Haggai