Stage for pkt io #925
Stage for pkt io #925
Conversation
Fredi-raspall
force-pushed
the
pr/fredi/pkt-io
branch
3 times, most recently
from
3ea6b45 to
5ccaffc
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new pkt-io crate that provides packet injection and punting capabilities for the gateway pipeline. The main purpose is to enable bidirectional packet flow control: injecting packets into the pipeline from external threads and extracting packets from the pipeline for local consumption.
Key changes:
- New
PktIonetwork function stage with configurable injection and punt queues - Added
LOCALmetadata flag to mark packets for local consumption - New
InternalDropdone reason for resource-constrained scenarios - Integration of
pkt-iostage into the main dataplane packet processor pipeline
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkt-io/Cargo.toml | Defines the new pkt-io crate with dependencies on crossbeam, net, pipeline, and tracectl |
| pkt-io/src/lib.rs | Library entry point exposing the PktIo network function |
| pkt-io/src/nf.rs | Implements PktIo with dual-queue architecture for packet injection and punting |
| pkt-io/src/tests.rs | Comprehensive test suite covering injection, punting, and multi-pipeline scenarios |
| net/src/packet/meta.rs | Adds LOCAL flag and InternalDrop reason to packet metadata |
| net/src/packet/display.rs | Refactors metadata display to show all flags including new LOCAL flag |
| dataplane/Cargo.toml | Adds pkt-io dependency to dataplane crate |
| dataplane/src/packet_processor/mod.rs | Integrates PktIo stage into main packet processing pipeline |
| dataplane/src/packet_processor/ipforward.rs | Updates local packet handling to use Unhandled instead of removed Local reason |
| Cargo.toml | Adds pkt-io workspace member and crossbeam dependency |
| // criteria may work. QUESTION: do we need/want this to be configurable?, or should this | ||
| // stage remain simple and the preceding stages keep the knowledge of what needs to be | ||
| // punted and set the local flag? |
There was a problem hiding this comment.
What would it require to make it configurable, passing a closure to filter packets to pull? Would that be easy to do at a later time, if necessary?
There was a problem hiding this comment.
Yes, something like that. I think it would. We do a similar thing already for the dumper function, where we pass a filter to know what packets to dump. That being said, I believe that the best would be to have a fixed criteria based on some packet annotation set by preceding NFs.
| .add_stage(flow_lookup_nf) | ||
| .add_stage(stateless_nat) | ||
| .add_stage(iprouter2) | ||
| .add_stage(pktio1) |
There was a problem hiding this comment.
Is my understanding correct that this new stage has no effect until we set the queues calling set_injectq() and set_puntq()?
There was a problem hiding this comment.
We can have the behavior that you mention if we create the pkt-io without buffers.
In this commit, I added 10K buffer queue for injection and 100 pkts for punting.
Since some packets may be marked as local, they will be punted. But since no one checks the punt queue, they will be cached there forever. So this PR is not to be merged (I set 100 pkts so that we'd not cache megabytes) as is. The expectation was to add more commits from @daniel-noland to do the actual consumption of those packets).
If we'd want to merge this PR, we'd just need to set zero buffering to the pkt-io, in which case it would be transparent.
Replace DoneReason::Local by a local flag in the metadata flags. The reason is that Local is not a terminating condition but a hint of how to further process a packet. Signed-off-by: Fredi Raspall <[email protected]>
This reason may be set to packets that can't be handled due to lack of resources. Signed-off-by: Fredi Raspall <[email protected]>
Signed-off-by: Fredi Raspall <[email protected]>
Fredi-raspall
force-pushed
the
pr/fredi/pkt-io
branch
from
5ccaffc to
afc4032
Compare
Pkt-io implements a network function capable of injecting packets into a pipeline or pulling off packets from it. Pulling packets off the pipeline may be used to locally process them (e.g. by punting them to the kernel). The injection capability may be used to transmit locally-generated packets (e.g. control-plane, ICMP, ARP, etc.) Signed-off-by: Fredi Raspall <[email protected]>
Signed-off-by: Fredi Raspall <[email protected]>
This is just a toy proof of concept. Signed-off-by: Fredi Raspall <[email protected]>
Signed-off-by: Fredi Raspall <[email protected]>
Add a pkt-io instance to pipeline before egress stage. The injection buffer is 10K packets. The punting buffer is only 100 packets large. This is temporary: we currently mark some packets as local, but do not handle them yet. Therefore, the first 100 of local packets will be cached (forever) and the rest removed. Signed-off-by: Fredi Raspall <[email protected]>
Fredi-raspall
force-pushed
the
pr/fredi/pkt-io
branch
from
afc4032 to
87bbea3
Compare
Alternatively, we may: - define a struct - and/or define a trait Queue to abstract the implementation. Signed-off-by: Fredi Raspall <[email protected]>
This is not a full abstraction, but may suffice for an easy switch of the inner queue type. Signed-off-by: Fredi Raspall <[email protected]>
Fredi-raspall
force-pushed
the
pr/fredi/pkt-io
branch
2 times, most recently
from
fa3be59 to
6aae11b
Compare
We need to abstract the creation of buffers. Add trait Allocate to PacketBufferMut. Signed-off-by: Fredi Raspall <[email protected]>
Fredi-raspall
force-pushed
the
pr/fredi/pkt-io
branch
from
6aae11b to
f717b77
Compare
No description provided.