Feat/tm review fixes - #28
Conversation
…(P1-P2)
First stage of the high-fidelity Traffic Manager that will replace the
output-only NSQueueingLogicPriRL scheduler. This lands the standalone,
event-driven, thread-free core; it is not yet wired into P4CoreV1model.
P1 - Input-side VOQ + accounting:
* VOQ[in][out][priority], 8 priority levels (7 = highest), N*N*8 queues,
stored flat because TmItem is move-only.
* Finite-buffer accounting: global / per-input / per-VOQ byte counters
with configurable limits (0 = unlimited).
* Admission control with explicit drop reasons (global, input, VOQ full);
egress drop reasons defined for later phases.
* Modular packet-format boundary: carries an opaque move-only TmPayload,
never assumes ns3::Packet (bm::Packet wrapper comes with integration).
* TracedCallbacks (enqueue/dequeue/drop/delays) and cumulative TmStats.
P2 - Fabric scheduler:
* Priority-first maximal matching (one input and one output per round),
behind an overridable DoRunFabricScheduler() so iSLIP / round-robin can
drop in later.
Tests (test/p4-traffic-manager-test-suite.cc, 6 cases, all passing on
ns-3.39): enqueue/dequeue + accounting, priority scheduling, per-input VOQ
isolation, one-in/one-out matching, drop-reason correctness, and delay
measurement in simulated time.
Builds on the standalone VOQ + fabric core (P1-P2). Adds the ns-3
event-driven timing model and the egress side, all thread-free.
P3 - Event-driven fabric loop (opt-in via the "EventDriven" attribute;
default off preserves the manual RunFabricScheduler()/DequeueFromVoq()
API used by the low-level unit tests):
* EnqueueToVoq arms a self-clocking fabric round (Simulator::Schedule).
* Each round applies the scheduler's grants (VOQ -> egress) and re-arms
while demand remains; the fabric is busy for the arbitration delay plus
the transfer time of the largest granted packet (FabricRate).
P4 - Egress side:
* Per-output-port strict-priority queues (8 levels, 7 = highest).
* Serialising egress scheduler: non-preemptive, one packet in service per
port, next packet chosen by strict priority, serialised at PortRate.
* Egress finite-buffer accounting (per-port + per-queue) with
EGRESS_PORT_BUFFER_FULL / EGRESS_QUEUE_FULL drop reasons.
* Byte-neutral VOQ->egress hand-off keeps the global counter consistent
(a packet is counted in global for its whole residence).
* TransmitCallback delivery hook (move-only payload) for wiring to an
ns-3 NetDevice at integration time; egress/total delay traces + stats.
DoDispose cancels pending fabric/egress events.
Adds 3 cases exercising the event-driven path under Simulator::Run,
bringing the suite to 9 (all passing on ns-3.39):
* end-to-end drain: a permutation demand is arbitrated, moved to egress
and serialised to the wire; buffers empty and every packet delivered
via TransmitCallback.
* egress strict priority: a packet in service is not preempted, but the
highest-priority waiting packet goes next (order 2, 7, 5).
* egress overflow: a full egress queue drops with EGRESS_QUEUE_FULL while
the granted-from-VOQ count still advances.
Wire P4TrafficManager into the v1model core behind a default-off EnableVoqFabric switch. When enabled, packets leaving the ingress pipeline are steered into the TM (VOQ -> priority-first fabric -> strict-priority egress) via a new BmPacketPayload wrapper; the TM's TransmitCallback runs the egress pipeline + deparse + send. The legacy output-queued path (egress_buffer, event-driven dequeue, PortTxComplete, QueueDisc) is left fully intact and remains the default, so behaviour is unchanged unless the switch opts in. Out-of-range ports (CPU/drop) fall through to the legacy path. - SetEnableVoqFabric()/GetEnableVoqFabric()/GetTrafficManager() accessors - TM constructed in start_and_return_() once ports/link rate are known - TM PortRate seeded from the physical link rate
Add an EnableVoqFabric attribute to P4SwitchNetDevice that propagates to
the V1model core before start, so the VOQ+fabric datapath can be turned
on declaratively (SetDeviceAttribute), matching how the switch is
otherwise configured. Default false preserves the legacy datapath.
Add examples/p4-voq-fabric-integration.cc: a self-validating end-to-end
check that runs a UDP flow over a 2-host/1-switch topology with the real
simple_v1model P4 program and asserts:
- EnableVoqFabric=true instantiates a Traffic Manager and moves all
traffic through it (VOQ enqueue + wire serialisation counters), with
the sink receiving the data;
- EnableVoqFabric=false instantiates no Traffic Manager (additive
contract) and still delivers over the legacy datapath.
Shipped as an example rather than a unit-test suite because a full bmv2
P4 program cannot be booted inside the ns-3 test-runner (bmv2 per-context
PHV pools crash there); this matches every other P4-program scenario in
the module. Verified: both datapaths deliver an identical 296000 B
(functional parity); rc=0 for --run=voq and --run=legacy.
Review comment HapCommSys#2: a full-duplex serial link keeps pumping new bits behind the frames already on the wire, so a slot must be able to start the next frame as soon as the previous one finishes serialising -- it should not be blocked for the whole propagation delay. SwitchedEthernetChannel now tracks serialisation (m_State: IDLE/TRANSMITTING) separately from in-flight propagation (m_propCount per slot): - TransmitStart is refused only while the slot is still serialising (TRANSMITTING), not while merely PROPAGATING. - TransmitEnd frees the slot immediately and bumps the in-flight count; delivery to the peer is still scheduled after the propagation delay. - PropagationCompleteEvent just retires one in-flight frame and never resets the slot state (a newer frame may already be serialising). - IsBusy() means 'mid-serialisation' only; GetState() derives PROPAGATING from a positive in-flight count. Update channel-state-test-suite to the new semantics (TransmitStart now succeeds during PROPAGATING; IsBusy is false while only propagating).
Review comment #1: the Traffic Manager decides which packet goes next, but the PHY/MAC should decide when the frame is actually serialised and signal completion back; and a frame must be counted as transmitted only after that signal, not before it is handed off. Add an opt-in completion-driven egress mode (attribute EgressCompletionDriven, default off): - EgressServiceEvent() now hands the frame to the TransmitCallback FIRST, accounting only the queue-residence effects (buffer release + queueing delays) at dequeue time. - In completion-driven mode it then marks the port in-flight and waits; the datapath calls NotifyEgressTxComplete(outPort, success), which does the 'on the wire' accounting (totalTransmitted, per-priority, per-port bytes) and serves the next frame. - The legacy self-clocked path (PortRate) is unchanged and remains the default, so the existing unit tests are unaffected. Review comment HapCommSys#3: IngressPipelineDelay was declared and had an attribute but was never applied. It now delays the first fabric round after the fabric goes idle (a freshly arrived packet traverses the ingress pipeline before it can be arbitrated). Default 0 keeps existing behaviour.
Review comment #1 (datapath side): drive the Traffic Manager's egress from a real completion signal sourced by the PHY. - Enable EgressCompletionDriven on the TM. - TmTransmit() no longer implies transmission is done: for a real send it schedules TmNotifyTxDone() after the channel's serialisation time (the PHY decides the duration); for drop/recirc/error paths it frees the port immediately. TmNotifyTxDone -> NotifyEgressTxComplete() lets the TM count the frame and serve the next one. Review comment HapCommSys#4: dispose the Traffic Manager before the core is destroyed. ~P4CoreV1model() now calls m_trafficManager->Dispose() first, cancelling its pending fabric/egress events and clearing the transmit callback (which captures the core) so no scheduled TM event can fire on a half-destroyed core.
|
I give a review based on #25 - #28, there are some issues that may need to be checked. With the help of @copilot and @claude, here is the comment with all the details you may need. 1.
|
…ne events Address review feedback on the completion-driven egress path (HapCommSys#25-HapCommSys#28): - NotifyEgressTxComplete() now takes a TmTxOutcome instead of a hardcoded success=true. A frame the egress pipeline drop()s, or one that is recirculate()d, is no longer counted as transmitted: SendNs3Packet() and TransmitOn() report whether the frame actually reached the wire and hand back its serialisation time (so it is not recomputed independently and cannot drift). A new totalRecirculated counter and a new EGRESS_POST_DEQUEUE_DROP drop reason keep recirculated frames distinct from real drops, and post-dequeue drops now show up in the drop totals instead of being silently lost from the conservation counters. - P4CoreV1model records each pending TmNotifyTxDone EventId in m_pendingTxDone and cancels them in its destructor before disposing the Traffic Manager, closing the same mid-simulation teardown race the TM Dispose() change was meant to close. - Wire test/channel-state-test-suite.cc into TEST_SOURCES so its regression tests actually run, and fix the stale CheckPropagating assertion (IsBusy() is false while only PROPAGATING, matching the other cases). Signed-off-by: Vineet1101 <vineetgoel692@gmail.com>
|
@Mingyumaz I have updated the code as per your suggestions |
2 participants
Round-1 review fixes for the VOQ + fabric Traffic Manager.
NotifyEgressTxComplete). A frame is counted transmitted only after that signal.Verified: TM suite 10/10; integration example passes for
--run=voqand--run=legacy(both deliver 296000 B, 0 drops).Note: #2 changes shared channel timing, so the legacy datapath also frees its sender after serialisation.