feat: act on the radio's answer to a modification, not only the UE's by midwell · Pull Request #668 · omec-project/smf

midwell · 2026-09-24T14:17:26Z

Nine commits on main: the feature is e9231ca, the five after it answer Copilot's first review, the next two its second, and the last its third. Part of the four-PR split of #623.

A modification is answered twice: the UE acknowledges the command, and the radio reports what it actually established. Only the UE's answer was read, so a radio that refused the modification, or built only part of it, changed nothing. The session's record, the UE and the user plane then disagreed. A downlink packet classified onto a flow the radio never built went to a QoS flow with no radio bearer and was dropped rather than falling back.

The response transfer is now decoded into the flows the radio established and the flows it refused, and each outcome is handled differently:

Everything established: nothing to do.
Nothing established: a response refusing every flow the modification added or modified is realigned like a partial one, and waits for the UE's answer (c30bd61). TS 38.413 subclause 8.2.3.2 has the radio pass the Command to the UE "only if at least one of the requests included in the PDU Session Resource Modify Request Transfer IE is successful (i.e. the PDU session is included in the PDU Session Resource Modify Response Item IE ...)". A session the radio could not modify at all is reported in the Unsuccessful Transfer instead. The request carries the session AMBR beside the flows, so a radio that applies the AMBR and refuses every flow reports the session in the response. TS 23.502 clause 4.3.3.2 step 7 draws the same line: the SMF assumes the NAS message was not forwarded only when no part of the N2 SM request is successful, and otherwise assumes it was sent and aligns the UE afterwards. So the UE has the Command and answers it. Abandoning instead left the UE on flows the session no longer had, and started a held decision whose Command the UE's late answer to this one would then have answered. The releases, which the answer does not list, are committed with the rest (63b29d7 first did that for a modification that also released flows). Of the radio's two answers, only the failure message, for which the radio does not pass the Command on, abandons the modification, and the session settles back on the parameters it had. After a retransmission, a held decision still waits out the Command's copies first.
Some established: what the radio built is what the session has. The pending update is pruned to those flows before it is committed, since committing all of it would record flows that do not exist. The refused flows are then withdrawn from the UE and the user plane by an ordinary corrective modification. A refused change to an established flow is dropped from the update but not withdrawn: the radio keeps that flow on its previous parameters, and withdrawing it would take away a flow whose bearer still exists, with every rule that refers to it (720d3ee). After a partial rejection, TS 23.502 subclause 4.3.3.2 calls for "a separate NAS PDU Session Modification procedure" rather than a retry of the one that was partly accepted.

The two answers can arrive in either order. When the UE's comes first, the completion commits the whole update, so the radio's answer keeps the committed update and builds the correction from that instead. Both refusal paths are covered, because they arrive at different handlers: the response naming every flow as refused, and the failure message refusing the modification entirely.

A correction does not start on a session released before it (4243027). It is queued, and a release can reach the session first. ApplyModification checks, under the hold that starts the modification, that the session has a tunnel and is Active or PfcpModify, as held work is checked. Otherwise it leaves the session alone and gives back the correction's reservation.

A corrective modification asks the radio to release nothing. The flows it withdraws are the ones the radio refused and never built, so a release would name flows the radio does not have.

A refused flow is matched by the flow's own QosId, parsed at full width with qos.ParseQosFlowId as #660 does for the release list, not by its QoS data key.

Held policy decisions (#667) wait for the correction. When the UE's completion finds that the radio refused part of the update, the correction is committed to there, and the session counts as pending before the correction's goroutine has started anything. A notification arriving meanwhile is held, and the corrected modification's tail leaves the session to the correction. The correction runs as a session task, as a held decision does, so the completion's state machine has finished before it moves the session. With nothing to correct, the completion starts the next held decision, and so does the failure message refusing the modification entirely.

Held decisions wait for the radio's answer too, for a bounded time. The radio's answer can still call for a correction after the UE has completed (TS 23.502 clause 4.3.3.2), and a modification started before it would discard what the completion kept for that answer. So a decision is held until both have answered. The wait for the radio is bounded by one T3591 interval from the UE's completion. When it lapses, the SMF stops waiting, drops what it kept for that answer, and starts what is held. A radio answer that arrives after the lapse, once the next modification has started, is taken as the next modification's: a rejection in it can be misattributed to the next modification's flows. That is the case this PR already cannot separate, since the answer carries no identity beyond the session.

A late Command Reject leaves the radio's answer expected. #667 ignores a Command Reject when no modification is pending, and here that also protects the wait for the radio. After the UE's Complete the procedure is no longer pending, but the radio's answer still is. A reject to another copy of the Command went down the abandonment path, which clears that expectation, so a radio refusal arriving afterwards was ignored and the refused flows stayed on the record.

A paging failure no longer stops a session from taking the radio's answer (822cf4f). #667 keeps a session Active through a paging failure while its modification is pending, because in SmStateInit the answers to it are dropped. Once the UE has completed, the modification still waits for the radio, and the check did not count that wait: a refusal then went uncorrected, and a decision held behind it was dropped when the wait lapsed. It now counts both.

A retransmission asks the radio only while its answer is awaited (3a2b3c3). T3591 retransmits the Command to the UE, and each retransmission carried the radio's request with it. The radio's answer is taken once. After a partial answer, a retransmitted request could establish a refused flow with its answer ignored, and the correction would then withdraw that flow from the UE and the record but leave it on the radio. Once the radio has answered, a retransmission is the UE's Command alone, which the AMF delivers in a downlink NAS transport to a connected UE, and pages an idle one for. Before then it still asks the radio, whose first request may not have arrived. This is decided in the hold of the lock that sends, which is the lock the answer is taken under.

A correction after a duplicated Command waits out its copies. A correction after a Command that was retransmitted, or sent twice by AMF re-discovery, waits out the same T3591 interval a held decision does. Its Command counts as not yet sent while it waits, so a late answer to another copy of the corrected Command is ignored rather than taken as the correction's.

Not in this PR:

Putting the user plane back when a modification is abandoned. At this commit an abandoned modification, one the radio refused by the failure message included, leaves the user plane programmed, and merging this PR alone ships that. fix: a policy update programs each rule on its own PDRs, and an abandoned one is put back #673, stacked on this PR, restores it (cfd227c) and holds every later modification for the session until the restore is answered. The restore needs the rule-by-rule builder fix: a policy update programs each rule on its own PDRs, and an abandoned one is put back #673 introduces.
The correction's user-plane programming. The builder at this commit reads every dedicated QoS data entry in the decision, so a correction re-creates a QER for the flow it withdraws, and merging this PR alone ships that too. fix: a policy update programs each rule on its own PDRs, and an abandoned one is put back #673's builder reads the decision only for what the update adds or changes, and a test there pins that a correction creates nothing.
Restoring the previous parameters after a refused change to an established flow. The UE and the user plane keep the changed parameters until a later modification. When the UE completes first, the commit has already replaced the record the previous values would come from. With fix: a policy update programs each rule on its own PDRs, and an abandoned one is put back #673, which commits modified entries, the two orders differ: when the radio answers first the record keeps the previous values while the UE and the user plane hold the changed ones, so a later decision repeating the change is seen as a change and sent again; when the UE completes first it is not. This includes a response refusing every flow of a modification that only changed flows: the radio falls back to the previous parameters, while the UE and the user plane keep the changed ones.
A radio that reports the session modified but holds the Command back. TS 38.413 says the radio passes the Command on "only if" a request succeeded, so a radio could do this. The first T3591 expiry then delivers the Command without the radio's part, since the radio has answered, at the cost of one interval.
T3591 disabled by configuration. A response refusing every flow now waits for the UE's answer without a bound, as any other modification does under that setting.
An established rule changed to refer to a refused new flow. It is withdrawn with the flow, though it existed before. Keeping it on its previous definition needs the record from before the update; keeping the changed one would leave the UE with a rule on a flow that is gone.
An abandonment after a radio answer that released flows. If the UE then never completes, or rejects the Command, the modification is abandoned and the releases are discarded with it, though the radio has let those bearers go. With fix: a policy update programs each rule on its own PDRs, and an abandoned one is put back #673 the user plane's restore re-installs their rules.
A radio that has not answered when T3591 expires. A retransmission sent before the radio has answered still carries the radio's request, so a slow radio is asked twice and only the first answer is read. If the first answer refuses a flow that the second request establishes, the second answer is ignored, and the UE's completion then withdraws that flow from the UE and the record and leaves its bearer on the radio, as before the fix above. Closing it means telling the answers apart, and an answer carries nothing beyond the session.
A modification that added or changed no flow, refused as a whole after the UE completed it. Nothing is withdrawn, and its releases and session-level changes stay in force. It is logged as a refusal (e8f6e07); putting them back needs the same earlier record.

Verification. Each fix in this PR, other than the log and comment changes in e8f6e07 and 4c6f3d2, has a test that fails without it; every fix was reverted in turn to confirm its test catches it. GOWORK=off pre-commit run --all-files on darwin/arm64 with Go 1.26.0 at c30bd61, all nine hooks passing as pinned in .pre-commit-config.yaml: go test -race ./... -count=20, staticcheck, gci, golangci-lint v2.13.2, gitleaks v8.30.1, reuse v6.2.0, yamlfmt v0.21.0 and the whitespace hooks.

🤖 Generated with Claude Code

A policy decision held behind a modification waits for the radio's answer as well as the UE's (omec-project#668). The radio-answer handlers had started held decisions, but the restore commit earlier in this PR took those starts out, because an abandonment's restore then ran beside the decision it started. So once the radio answered a modification the UE had already completed, nothing started the held decision until the wait for the radio lapsed, one T3591 interval later. The starts are back. They no longer race the restore: a held decision runs as a session task, and a session task waits for an owed revert before it runs, so one started by an abandonment's answer begins only once its restore has finished. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

The previous commit has every queued transaction wait for an owed revert before processing. That is a check followed, some frames later, by the handler's own PFCP exchange, so a revert that became owed in between, from outside the queue, would not be seen, and the two exchanges would wait on the session's single, uncorrelated response channel at once. Nothing sets one from outside the queue. T3591's last expiry (queued since the eighth review of omec-project#667) and the corrective modification after a partial rejection (queued since omec-project#668) already run as session tasks, and the NAS and NGAP handlers already run inside a transaction. This commit changes no behaviour: it says so in the comments that described the old arrangement, and adds two tests that pin it. Raised in review of this PR. The two-rule fixture's uplink session AMBR is now 150 Mbps rather than 100. Nothing reads the value, but the T3591 retransmission tests this is stacked on use "100 Mbps" twice, and a third occurrence trips goconst. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

A policy decision held behind a modification waits for the radio's answer as well as the UE's (omec-project#668). The radio-answer handlers had started held decisions, but the restore commit earlier in this PR took those starts out, because an abandonment's restore then ran beside the decision it started. So once the radio answered a modification the UE had already completed, nothing started the held decision until the wait for the radio lapsed, one T3591 interval later. The starts are back. They no longer race the restore: a held decision runs as a session task, and a session task waits for an owed revert before it runs, so one started by an abandonment's answer begins only once its restore has finished. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

gab-arrobo · 2026-10-15T16:53:40Z

@midwell, please resolve conflicts and rebase PR. Thanks!

A modification is answered twice: the UE acknowledges the command, and the radio reports what it actually established. Only the UE's answer was read, so a radio that refused the modification, or built part of it, changed nothing. The session's record, the UE and the user plane then disagreed, and a downlink packet classified onto a flow the radio never built went to a QoS flow with no radio bearer and was dropped rather than falling back. The response transfer is decoded into the flows the radio established and the flows it refused, and the three shapes are answered differently. Everything established: nothing to do. Nothing established: the modification is abandoned, and the session settles back on the parameters it had. Some established: what the radio built is what the session has, so the pending update is pruned to those flows before it is committed -- committing the whole of it would record flows that do not exist -- and the refused ones are withdrawn from the UE and the user plane by an ordinary corrective modification. TS 23.502 subclause 4.3.3.2 calls for a further procedure rather than a retry of the one that was partly accepted, which is what a deletion through ApplyModification is. The two answers are unordered, and the order where the UE is first has to be handled too: the completion then commits the update whole, so the radio's answer keeps the committed update and builds the correction from that instead. Both refusal paths are covered -- the response naming every flow as refused, and the failure message refusing the modification in its entirety -- because they arrive at different handlers. A corrective modification asks for no release: the flows it withdraws are the ones the radio refused, which it never built, so asking it to release them names flows it does not have. Policy decisions held behind a modification (see the previous PR) wait for the correction too. A correction is committed to when the UE's completion finds a partial rejection, so the session is marked pending from there, before its goroutine starts anything: a notification arriving meanwhile is held, and the corrected modification's tail leaves the session to it. With nothing to correct, the completion starts the next held decision, as does the radio refusing the whole modification. The correction runs as a task in the session's queue, as a held decision does since the previous PR: started on a goroutine from the UE's completion, it could move the session to SmStatePfcpModify before that completion's state machine set Active again, after which the user plane's answer to the correction was never delivered. The radio's answer is part of the modification, so a held policy decision waits for it as well as for the UE's: the answer can still call for a correction after the UE has completed (TS 23.502 clause 4.3.3.2), and a modification started before it would discard what the completion kept for it. The wait is bounded by one T3591 interval from the UE's completion; a radio that never answers then stops holding decisions, and its correction, if one was owed, is given up as it was before. A correction after a Command that was retransmitted, or sent twice by AMF re-discovery, waits out the same interval a held decision does, and its Command counts as not yet sent while it waits, so a late answer to another copy of the corrected Command is ignored rather than taken as the correction's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

The previous commit has every queued transaction wait for an owed revert before processing. That is a check followed, some frames later, by the handler's own PFCP exchange, so a revert that became owed in between, from outside the queue, would not be seen, and the two exchanges would wait on the session's single, uncorrelated response channel at once. Nothing sets one from outside the queue. T3591's last expiry (queued since the eighth review of omec-project#667) and the corrective modification after a partial rejection (queued since omec-project#668) already run as session tasks, and the NAS and NGAP handlers already run inside a transaction. This commit changes no behaviour: it says so in the comments that described the old arrangement, and adds two tests that pin it. Raised in review of this PR. The two-rule fixture's uplink session AMBR is now 150 Mbps rather than 100. Nothing reads the value, but the T3591 retransmission tests this is stacked on use "100 Mbps" twice, and a third occurrence trips goconst. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

A policy decision held behind a modification waits for the radio's answer as well as the UE's (omec-project#668). The radio-answer handlers had started held decisions, but the restore commit earlier in this PR took those starts out, because an abandonment's restore then ran beside the decision it started. So once the radio answered a modification the UE had already completed, nothing started the held decision until the wait for the radio lapsed, one T3591 interval later. The starts are back. They no longer race the restore: a held decision runs as a session task, and a session task waits for an owed revert before it runs, so one started by an abandonment's answer begins only once its restore has finished. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

T3591 retransmits the PDU Session Modification Command (TS 24.501 subclause 6.3.2.5), and each retransmission also carried the radio's PDU Session Resource Modify Request. The radio's answer is taken once. After a partial answer -- flow 2 established, flow 3 refused -- a retransmission before the UE completed could have the radio establish flow 3, and that answer was ignored. The UE's completion then withdrew flow 3 from the UE and the record and, being corrective, left its bearer on the radio. The radio's part is now attached only while its answer is expected, decided in the hold of SMLock that sends, which is the lock the answer is taken under. Before the radio answers, a retransmission still asks it, since the first request may never have arrived. An answer still in transit when a retransmission goes out is not seen. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

The previous commit has every queued transaction wait for an owed revert before processing. That is a check followed, some frames later, by the handler's own PFCP exchange, so a revert that became owed in between, from outside the queue, would not be seen, and the two exchanges would wait on the session's single, uncorrelated response channel at once. Nothing sets one from outside the queue. T3591's last expiry (queued since the eighth review of omec-project#667) and the corrective modification after a partial rejection (queued since omec-project#668) already run as session tasks, and the NAS and NGAP handlers already run inside a transaction. This commit changes no behaviour: it says so in the comments that described the old arrangement, and adds two tests that pin it. Raised in review of this PR. The two-rule fixture's uplink session AMBR is now 150 Mbps rather than 100. Nothing reads the value, but the T3591 retransmission tests this is stacked on use "100 Mbps" twice, and a third occurrence trips goconst. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

A policy decision held behind a modification waits for the radio's answer as well as the UE's (omec-project#668). The radio-answer handlers had started held decisions, but the restore commit earlier in this PR took those starts out, because an abandonment's restore then ran beside the decision it started. So once the radio answered a modification the UE had already completed, nothing started the held decision until the wait for the radio lapsed, one T3591 interval later. The starts are back. They no longer race the restore: a held decision runs as a session task, and a session task waits for an owed revert before it runs, so one started by an abandonment's answer begins only once its restore has finished. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

The correction after a partial rejection carries the original policy decision, refused flows included. The builder this PR replaces read every dedicated QoS data entry in that decision, so the correction re-created a QER for the refused flow and hung it on a rule's PDRs. The rule-by-rule builder reads the decision only for what the update adds or changes, and a correction adds and changes nothing. This pins that. Run against omec-project#668's builder, it fails: the correction creates QERs for QFI 1 and the refused QFI 2 and reprograms two PDRs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

Copilot

🔵 Needs a closer look

Unresolved answer-ordering risk and user-plane recovery deferred to a stacked PR require coordinated human review.

1 open finding

Radio refusal lacks quiet interval, allowing late UE answers to affect next com… · New

2 resolved since last review

🧠 Review effort: Balanced

A radio response that refused every added or changed flow, with no release in the modification, abandoned it at once. But TS 38.413 subclause 8.2.3.2 has the radio pass the Command to the UE "only if at least one of the requests included in the PDU Session Resource Modify Request Transfer IE is successful (i.e. the PDU session is included in the PDU Session Resource Modify Response Item IE ...)", and a session the radio could not modify at all is reported in the Unsuccessful Transfer instead. The request carries the session AMBR beside the flows, so a radio that applies the AMBR and refuses every flow reports the session in the response. TS 23.502 clause 4.3.3.2 step 7 draws the same line: the SMF assumes the NAS message was not forwarded only when no part of the N2 SM request is successful. So the UE has the Command and answers it. Abandoning left the UE on flows the session no longer had, and started a held decision at once; the UE's late Complete or Reject to the first Command, carrying the same PDU session ID and PTI, was then taken as the held decision's answer. Every response that refuses flows is now realigned and waits for the UE's answer, as one that also released flows already did. Only the failure message, for which the radio does not pass the Command on, abandons; after a retransmission it still waits out the Command's copies, which a new test pins. ReleasesQosFlows, which only told the two kinds of whole refusal apart, is removed, and the comments that still described a response abandoning are corrected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

The previous commit has every queued transaction wait for an owed revert before processing. That is a check followed, some frames later, by the handler's own PFCP exchange, so a revert that became owed in between, from outside the queue, would not be seen, and the two exchanges would wait on the session's single, uncorrelated response channel at once. Nothing sets one from outside the queue. T3591's last expiry (queued since the eighth review of omec-project#667) and the corrective modification after a partial rejection (queued since omec-project#668) already run as session tasks, and the NAS and NGAP handlers already run inside a transaction. This commit changes no behaviour: it says so in the comments that described the old arrangement, and adds two tests that pin it. Raised in review of this PR. The two-rule fixture's uplink session AMBR is now 150 Mbps rather than 100. Nothing reads the value, but the T3591 retransmission tests this is stacked on use "100 Mbps" twice, and a third occurrence trips goconst. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

A policy decision held behind a modification waits for the radio's answer as well as the UE's (omec-project#668). The radio-answer handlers had started held decisions, but the restore commit earlier in this PR took those starts out, because an abandonment's restore then ran beside the decision it started. So once the radio answered a modification the UE had already completed, nothing started the held decision until the wait for the radio lapsed, one T3591 interval later. The starts are back. They no longer race the restore: a held decision runs as a session task, and a session task waits for an owed revert before it runs, so one started by an abandonment's answer begins only once its restore has finished. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

The correction after a partial rejection carries the original policy decision, refused flows included. The builder this PR replaces read every dedicated QoS data entry in that decision, so the correction re-created a QER for the refused flow and hung it on a rule's PDRs. The rule-by-rule builder reads the decision only for what the update adds or changes, and a correction adds and changes nothing. This pins that. Run against omec-project#668's builder, it fails: the correction creates QERs for QFI 1 and the refused QFI 2 and reprograms two PDRs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Edvin Lindqvist <edvin.lindqvist@forsway.com>

Copilot

🔵 Needs a closer look

Cross-layer procedure coordination and user-plane recovery deferred to #673 require human review of merge sequencing.

0 open findings

1 resolved since last review

Radio refusal lacks quiet interval, allowing late UE answers to affect next com…