feat: act on the radio's answer to a modification, not only the UE's

context/ngap_build.go

@@ -895,7 +895,7 @@ func releasedQosFlowItems(ctx *SMContext) []ngapType.QosFlowWithCauseItem {
 		return nil
 	}
 	update := ctx.SmPolicyUpdates[0]
-	if update == nil {
+	if update == nil || update.Corrective {
 		return nil
 	}
 

context/ngap_modify_handler.go

@@ -0,0 +1,87 @@
+// SPDX-FileCopyrightText: 2026 Forsway Scandinavia AB
+// SPDX-License-Identifier: Apache-2.0
+
+package context
+
+import (
+	"github.com/omec-project/ngap/v2/aper"
+	"github.com/omec-project/ngap/v2/ngapType"
+)
+
+// ModifyResponse is what the radio access network made of a modification: which QoS flows it
+// established and which it refused.
+//
+// A response means the radio modified the session and passed the Command to the UE (TS 38.413
+// subclause 8.2.3.2), so a refusal of some flows or of all of them means the UE was told about flows
+// that were not established, and the network owes it a further modification saying which are
+// actually in force. A radio that refuses the whole modification answers with the failure message
+// instead, and that modification is abandoned.
+type ModifyResponse struct {
+	AcceptedQFIs []int64
+	RejectedQFIs []int64
+}
+
+// WhollyRejected reports that the radio access network established none of the flows.
+func (r ModifyResponse) WhollyRejected() bool {
+	return len(r.AcceptedQFIs) == 0 && len(r.RejectedQFIs) > 0
+}
+
+// PartiallyRejected reports that some flows were established and some were not.
+func (r ModifyResponse) PartiallyRejected() bool {
+	return len(r.AcceptedQFIs) > 0 && len(r.RejectedQFIs) > 0
+}
+
+// HandlePDUSessionResourceModifyResponseTransfer decodes the radio access network's answer to a
+// PDU session modification.
+func HandlePDUSessionResourceModifyResponseTransfer(b []byte, ctx *SMContext) (ModifyResponse, error) {
+	transfer := ngapType.PDUSessionResourceModifyResponseTransfer{}
+	if err := aper.UnmarshalWithParams(b, &transfer, "valueExt"); err != nil {
+		return ModifyResponse{}, err
+	}
+
+	var result ModifyResponse
+
+	if transfer.QosFlowAddOrModifyResponseList != nil {
+		for _, item := range transfer.QosFlowAddOrModifyResponseList.List {
+			result.AcceptedQFIs = append(result.AcceptedQFIs, item.QosFlowIdentifier.Value)
+		}
+	}
+
+	if transfer.QosFlowFailedToAddOrModifyList != nil {
+		for _, item := range transfer.QosFlowFailedToAddOrModifyList.List {
+			result.RejectedQFIs = append(result.RejectedQFIs, item.QosFlowIdentifier.Value)
+		}
+	}
+
+	ctx.SubPduSessLog.Infof("PDU session resource modify response: %d flows established, %d refused",
+		len(result.AcceptedQFIs), len(result.RejectedQFIs))
+
+	return result, nil
+}
+
+// HandlePDUSessionResourceModifyUnsuccessfulTransfer decodes the radio access network's refusal
+// of a modification in its entirety.
+func HandlePDUSessionResourceModifyUnsuccessfulTransfer(b []byte, ctx *SMContext) (ngapType.Cause, error) {
+	transfer := ngapType.PDUSessionResourceModifyUnsuccessfulTransfer{}
+	if err := aper.UnmarshalWithParams(b, &transfer, "valueExt"); err != nil {
+		return ngapType.Cause{}, err
+	}
+
+	ctx.SubPduSessLog.Warnf("PDU session resource modify refused in its entirety, cause present %d",
+		transfer.Cause.Present)
+
+	return transfer.Cause, nil
+}
+
+// PendingRealignment records that the radio access network established only some of the flows a
+// modification authorized, so the UE is holding a view of the session that is wider than what
+// exists.
+//
+// TS 23.502 clause 4.3.3.2 puts the corrective procedure after step 11 — after the UE has
+// acknowledged the first modification — so the need is recorded when the response arrives and
+// acted on when that acknowledgement comes. Sending a second command while the first is still
+// unacknowledged would collide with it.
+type PendingRealignment struct {
+	EstablishedQFIs []int64
+	RefusedQFIs     []int64
+}

context/ngap_modify_handler_test.go

@@ -0,0 +1,194 @@
+// SPDX-FileCopyrightText: 2026 Forsway Scandinavia AB
+// SPDX-License-Identifier: Apache-2.0
+
+package context
+
+import (
+	"testing"
+
+	"github.com/omec-project/ngap/v2/aper"
+	"github.com/omec-project/ngap/v2/ngapType"
+	"go.uber.org/zap"
+)
+
+func modifyResponseBytes(t *testing.T, established, refused []int64) []byte {
+	t.Helper()
+
+	// DLNGUUPTNLInformation and ULNGUUPTNLInformation are OPTIONAL in TS 38.413 but the generated
+	// type marks them mandatory, so they must be populated for the encoder to accept the message.
+	// See TestModifyResponseWithoutTunnelInformation for why that matters.
+	tnl := &ngapType.UPTransportLayerInformation{
+		Present: ngapType.UPTransportLayerInformationPresentGTPTunnel,
+		GTPTunnel: &ngapType.GTPTunnel{
+			TransportLayerAddress: ngapType.TransportLayerAddress{Value: aper.BitString{Bytes: []byte{10, 0, 0, 1}, BitLength: 32}},
+			GTPTEID:               ngapType.GTPTEID{Value: []byte{0, 0, 0, 1}},
+		},
+	}
+	transfer := ngapType.PDUSessionResourceModifyResponseTransfer{
+		DLNGUUPTNLInformation: tnl,
+		ULNGUUPTNLInformation: tnl,
+	}
+
+	if len(established) > 0 {
+		list := &ngapType.QosFlowAddOrModifyResponseList{}
+		for _, qfi := range established {
+			list.List = append(list.List, ngapType.QosFlowAddOrModifyResponseItem{
+				QosFlowIdentifier: ngapType.QosFlowIdentifier{Value: qfi},
+			})
+		}
+		transfer.QosFlowAddOrModifyResponseList = list
+	}
+
+	if len(refused) > 0 {
+		list := &ngapType.QosFlowListWithCause{}
+		for _, qfi := range refused {
+			list.List = append(list.List, ngapType.QosFlowWithCauseItem{
+				QosFlowIdentifier: ngapType.QosFlowIdentifier{Value: qfi},
+				Cause: ngapType.Cause{
+					Present: ngapType.CausePresentRadioNetwork,
+					RadioNetwork: &ngapType.CauseRadioNetwork{
+						Value: ngapType.CauseRadioNetworkPresentNotSupported5QIValue,
+					},
+				},
+			})
+		}
+		transfer.QosFlowFailedToAddOrModifyList = list
+	}
+
+	b, err := aper.MarshalWithParams(transfer, "valueExt")
+	if err != nil {
+		t.Fatalf("encoding the modify response failed: %v", err)
+	}
+	return b
+}
+
+// A partial rejection cannot be produced with one flow, and it is the case the realignment
+// exists for. This drives a genuinely encoded response through the decoder.
+func TestModifyResponseClassification(t *testing.T) {
+	ctx := &SMContext{SubPduSessLog: zap.NewNop().Sugar()}
+
+	tests := []struct {
+		name              string
+		established       []int64
+		refused           []int64
+		wantWholeRejected bool
+		wantPartial       bool
+	}{
+		{"all flows established", []int64{1, 2, 3}, nil, false, false},
+		{"some established, some refused", []int64{1, 2}, []int64{3}, false, true},
+		{"more refused than established", []int64{1}, []int64{2, 3, 4}, false, true},
+		{"none established", nil, []int64{1, 2}, true, false},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result, err := HandlePDUSessionResourceModifyResponseTransfer(
+				modifyResponseBytes(t, tc.established, tc.refused), ctx)
+			if err != nil {
+				t.Fatalf("decoding failed: %v", err)
+			}
+
+			if len(result.AcceptedQFIs) != len(tc.established) {
+				t.Errorf("established = %v, want %v", result.AcceptedQFIs, tc.established)
+			}
+			if len(result.RejectedQFIs) != len(tc.refused) {
+				t.Errorf("refused = %v, want %v", result.RejectedQFIs, tc.refused)
+			}
+			if got := result.WhollyRejected(); got != tc.wantWholeRejected {
+				t.Errorf("WhollyRejected = %v, want %v", got, tc.wantWholeRejected)
+			}
+			if got := result.PartiallyRejected(); got != tc.wantPartial {
+				t.Errorf("PartiallyRejected = %v, want %v", got, tc.wantPartial)
+			}
+		})
+	}
+}
+
+// A response establishing nothing and one establishing part of a modification are told apart in
+// what the SMF reports, so they must not be confusable. This pins the boundary between them.
+func TestWholeAndPartialRejectionAreDistinct(t *testing.T) {
+	ctx := &SMContext{SubPduSessLog: zap.NewNop().Sugar()}
+
+	whole, err := HandlePDUSessionResourceModifyResponseTransfer(
+		modifyResponseBytes(t, nil, []int64{1, 2}), ctx)
+	if err != nil {
+		t.Fatalf("decoding failed: %v", err)
+	}
+	partial, err := HandlePDUSessionResourceModifyResponseTransfer(
+		modifyResponseBytes(t, []int64{1}, []int64{2}), ctx)
+	if err != nil {
+		t.Fatalf("decoding failed: %v", err)
+	}
+
+	if whole.PartiallyRejected() {
+		t.Error("a response establishing nothing must not be classed as partial")
+	}
+	if partial.WhollyRejected() {
+		t.Error("a response establishing something must not be classed as whole; part of it has to be kept")
+	}
+}
+
+// conformantModifyResponseTransfer is PDUSessionResourceModifyResponseTransfer as TS 38.413
+// defines it, with DL/UL NG-U UP TNL Information OPTIONAL. It stands in for what a real gNB
+// encodes against, so the test below decodes a peer's encoding rather than its own: encoder and
+// decoder agreeing with each other is precisely what hid the defect this guards against.
+type conformantModifyResponseTransfer struct {
+	DLNGUUPTNLInformation                *ngapType.UPTransportLayerInformation                                              `aper:"valueLB:0,valueUB:1,optional"`
+	ULNGUUPTNLInformation                *ngapType.UPTransportLayerInformation                                              `aper:"valueLB:0,valueUB:1,optional"`
+	QosFlowAddOrModifyResponseList       *ngapType.QosFlowAddOrModifyResponseList                                           `aper:"optional"`
+	AdditionalDLQosFlowPerTNLInformation *ngapType.QosFlowPerTNLInformationList                                             `aper:"optional"`
+	QosFlowFailedToAddOrModifyList       *ngapType.QosFlowListWithCause                                                     `aper:"optional"`
+	IEExtensions                         *ngapType.ProtocolExtensionContainerPDUSessionResourceModifyResponseTransferExtIEs `aper:"optional"`
+}
+
+// The SMF has to read a conformant gNB's modification response in both the form that omits the
+// tunnel information and the form that carries it.
+//
+// omec-project/ngap generated DL/UL NG-U UP TNL Information without the aper "optional" tag up to
+// and including v2.1.3, so the codec treated them as always present. A modification that changes
+// QoS without moving the tunnel — the ordinary case, and what this change produces — has a gNB omit
+// both fields, and its encoding then carries six optionality bits where that decoder expected four.
+// The decode failed on the misalignment. Fixed upstream in ngap#118 and released in v2.1.4, which
+// this module now requires; the two tests that documented the broken state are gone with it.
+func TestModifyResponseReadsAConformantGnbMessage(t *testing.T) {
+	base := conformantModifyResponseTransfer{
+		QosFlowAddOrModifyResponseList: &ngapType.QosFlowAddOrModifyResponseList{
+			List: []ngapType.QosFlowAddOrModifyResponseItem{
+				{QosFlowIdentifier: ngapType.QosFlowIdentifier{Value: 5}},
+			},
+		},
+	}
+	withTunnel := base
+	withTunnel.DLNGUUPTNLInformation = &ngapType.UPTransportLayerInformation{
+		Present: ngapType.UPTransportLayerInformationPresentGTPTunnel,
+		GTPTunnel: &ngapType.GTPTunnel{
+			TransportLayerAddress: ngapType.TransportLayerAddress{
+				Value: aper.BitString{Bytes: []byte{10, 0, 0, 1}, BitLength: 32},
+			},
+			GTPTEID: ngapType.GTPTEID{Value: []byte{0, 0, 0, 7}},
+		},
+	}
+
+	for name, msg := range map[string]conformantModifyResponseTransfer{
+		"tunnel omitted": base,
+		"tunnel present": withTunnel,
+	} {
+		t.Run(name, func(t *testing.T) {
+			wire, err := aper.MarshalWithParams(msg, "valueExt")
+			if err != nil {
+				t.Fatalf("could not build a conformant peer's message: %v", err)
+			}
+
+			var got ngapType.PDUSessionResourceModifyResponseTransfer
+			if err := aper.UnmarshalWithParams(wire, &got, "valueExt"); err != nil {
+				t.Fatalf("the shipped type could not decode a conformant gNB message (% x): %v", wire, err)
+			}
+			if got.QosFlowAddOrModifyResponseList == nil || len(got.QosFlowAddOrModifyResponseList.List) != 1 {
+				t.Fatal("the QoS flow list was lost")
+			}
+			if (msg.DLNGUUPTNLInformation != nil) != (got.DLNGUUPTNLInformation != nil) {
+				t.Error("tunnel information presence did not survive the decode")
+			}
+		})
+	}
+}

context/ngap_modify_request_test.go

@@ -130,6 +130,47 @@ func TestModifyRequestFallsBackToTheDefaultFlowWhenNoFlowsAreNamed(t *testing.T)
 	}
 }
 
+// A corrective modification carries only deletions, and the radio must not be asked to modify a
+// flow that was never in question.
+//
+// After a partial rejection the SMF withdraws the flows the radio refused. Those flows were never
+// built at the radio, so the correct N2 content is nothing — the UE needs the NAS withdrawal and
+// the user plane needs its rules removed, but the radio has no state to change. The no-flows
+// fallback above was written for a modification that names none at all, such as a session-AMBR
+// change, and a delete-only update reaches it too. Falling back there spends a radio
+// reconfiguration per partial rejection to re-assert a flow nobody asked about, which on a
+// constrained air interface is exactly the cost the rest of this work is careful about.
+func TestCorrectiveModificationDoesNotAskTheRadioAboutTheDefaultFlow(t *testing.T) {
+	ctx := modifyingContext(t, map[string]*models.QosData{
+		"2": {QosId: "2", Var5qi: openapi.PtrInt32(1)},
+		"3": {QosId: "3", Var5qi: openapi.PtrInt32(2)},
+	})
+
+	// The radio refused flow 3; RemoveFlows prunes the pending update to what was established and
+	// returns the delete-only corrective, which is what the realignment sends.
+	corrective := ctx.SmPolicyUpdates[0].RemoveFlows(qos.RefusedFlowSet([]int64{3}))
+	if corrective == nil {
+		t.Fatal("no corrective was produced, so the rest of this test would pass vacuously")
+	}
+	ctx.SmPolicyUpdates = []*qos.PolicyUpdate{corrective}
+
+	encoded, err := BuildPDUSessionResourceModifyRequestTransfer(ctx)
+	if err != nil {
+		t.Fatalf("building the corrective transfer failed: %v", err)
+	}
+
+	list := decodeModifyRequest(t, encoded)
+	if list == nil {
+		return // nothing asked of the radio, which is the correct answer
+	}
+	var qfis []int64
+	for _, item := range list.List {
+		qfis = append(qfis, item.QosFlowIdentifier.Value)
+	}
+	t.Errorf("the corrective asked the radio to add or modify QoS flow(s) %v; it carries only deletions, so the radio should be asked for nothing",
+		qfis)
+}
+
 // A QoS identifier that does not parse must not reach the radio as QFI 0.
 //
 // GetQosFlowIdFromQosId returns 0 for an identifier it cannot read, and TS 23.501 table 5.7.1.1
@@ -259,6 +300,32 @@ func TestModifyRequestNamesTheFlowsAnOrdinaryDeletionWithdraws(t *testing.T) {
 	}
 }
 
+// The corrective modification is the exception. Its deletions are the flows the radio refused, so
+// they were never established there: asking for their release would name a QFI the radio has no
+// record of, for a realignment whose whole purpose is to stop claiming they exist.
+func TestCorrectiveModificationAsksForNoRelease(t *testing.T) {
+	ctx := modifyingContext(t, map[string]*models.QosData{
+		"2": {QosId: "2", Var5qi: openapi.PtrInt32(1)},
+		"3": {QosId: "3", Var5qi: openapi.PtrInt32(2)},
+	})
+
+	corrective := ctx.SmPolicyUpdates[0].RemoveFlows(qos.RefusedFlowSet([]int64{3}))
+	if corrective == nil {
+		t.Fatal("no corrective was produced, so the rest of this test would pass vacuously")
+	}
+	ctx.SmPolicyUpdates = []*qos.PolicyUpdate{corrective}
+
+	encoded, err := BuildPDUSessionResourceModifyRequestTransfer(ctx)
+	if err != nil {
+		t.Fatalf("building the corrective transfer failed: %v", err)
+	}
+
+	if released := decodeReleaseList(t, encoded); released != nil && len(released.List) > 0 {
+		t.Errorf("the corrective asked the radio to release %d flow(s) it never established",
+			len(released.List))
+	}
+}
+
 // The release list is built from the names of deleted QoS data, and those go on the wire as flow
 // identifiers. GetQosFlowIdFromQosId narrows to uint8 before anything can range-check it, so a
 // QoS id of 257 arrives as 1 — and the radio is asked to release whatever flow 1 is on this