mana: TX SGE0 and WQE for LSO should meet GDMA requirements #2279
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements GDMA hardware requirements for LSO (Large Send Offload) TX operations by ensuring SGE0 contains only the packet header, does not exceed 256 bytes, and the TX WQE has multiple SGL entries. The changes modify coalescing logic to avoid combining the header segment when LSO is enabled, and add validation to drop invalid LSO packets.
Key changes:
- Modified segment coalescing logic to preserve SGE0 for LSO packets
- Added validation to ensure LSO packets have more than one SGL entry
- Extended test coverage with comprehensive LSO scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| vm/devices/net/net_mana/src/test.rs | New file containing comprehensive test suite for LSO functionality, segment coalescing, and error handling scenarios |
| vm/devices/net/net_mana/src/lib.rs | Updated LSO handling to prevent SGE0 coalescing, added validation for LSO packets, and moved tests to separate module |
sunilmut
force-pushed
the
sunilmut/mana_lso
branch
from
5411432 to
37a6745
Compare
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
sunilmut
force-pushed
the
sunilmut/mana_lso
branch
from
ec4d8f9 to
75499ad
Compare
sunilmut
force-pushed
the
sunilmut/mana_lso
branch
from
9cf32a6 to
1b51122
Compare
| let mut rx_packets_n = 0; | ||
| let mut tx_done = [TxId(0); 2]; | ||
| let mut tx_done_n = 0; | ||
| while rx_packets_n == 0 { |
There was a problem hiding this comment.
fyi: I believe you added a loop here in another version of this function, but I am fine with or without (just pointing out a diff).
vm/devices/net/net_mana/src/lib.rs
Outdated
| // the header and should not exceed 256 bytes. Otherwise, it treats | ||
| // the WQE as "corrupt", disables the queue and return GDMA error. | ||
| // To meet the hardware requirements, do not coalesce SGE0 when LSO is enabled. | ||
| let coalesce_sge_possible = !meta.flags.offload_tcp_segmentation() |
There was a problem hiding this comment.
I think a cleaner way to do this might just be to set sgl_idx to 1 for LSO. Right now it gets set to 0, so we try to coalesce into SGE0.
There was a problem hiding this comment.
I think thats a bit trickier because adjusting sgl_idx by itself is not sufficient. The code will try to coalesce tail-idx -1 and so the tail_idx will also needs to be adjusted to that part of the code needs to be skipped for LSO.
There was a problem hiding this comment.
Well, I changed this path a bit anyway. Once you rebase, you'll see in the new model you push SQEs one at a time (to avoid having to put all of them on the stack). Once the SQE has been pushed, you can't modify it again. So in this case, you'd just make sure that you push SQE0 for LSO packets before doing the coalescing work.
sunilmut
force-pushed
the
sunilmut/mana_lso
branch
from
ea956da to
a6e6093
Compare
GDMA requires that SGE0 for the LSO TX WQE should: - Only contain the header - Should not exceed 256 bytes - There should be > 1 SGL in the TX LSO WQE Also added: - Test cases to cover the above requirements - Test configuration hook
sunilmut
force-pushed
the
sunilmut/mana_lso
branch
from
a6e6093 to
ea3cbc5
Compare
3 participants
GDMA requires that SGE0 for the LSO TX WQE should:
Also added test cases to cover the above requirements