Skip to content

feat: update QER when PCF notifies AMBR change#191

Open
timyl wants to merge 1 commit into
free5gc:mainfrom
timyl:fix/pcf-notify-ambr-update
Open

feat: update QER when PCF notifies AMBR change#191
timyl wants to merge 1 commit into
free5gc:mainfrom
timyl:fix/pcf-notify-ambr-update

Conversation

@timyl

@timyl timyl commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Background

Per 3GPP TS 29.512, PCF may send SmPolicyUpdateNotification to update
session-AMBR at any time during an active PDU session. SMF should enforce
this by updating UPF rate limiting via PFCP.

Problem

SMF processed the PCF notification and updated internal context, but did
not send PFCP SessionModificationRequest to update the QER (MBR_UL/MBR_DL).
As a result, UPF continued enforcing the original AMBR value.

Solution

  • updateAMBRQer(): finds the AMBR QER in the SMF context and marks it
    for update when PCF notifies a new session-AMBR
  • qerToUpdateQER(): converts a QER to PFCP UpdateQER IE
  • BuildPfcpSessionModificationRequest(): handles RULE_UPDATE to include
    UpdateQER IE in the modification request

Testing

Tested with free5gc v4.2.0 + UERANSIM + vendor PCF:

  • PCF notifies 4Mbps → 2Mbps: UPF correctly enforces 2Mbps
  • PCF notifies 2Mbps → 4Mbps: UPF correctly enforces 4Mbps
  • Multiple consecutive AMBR updates work correctly

Related Fixes

This PR requires the following additional fixes to work end-to-end:

  1. free5gc/utilhttpwrapper/httpwrapper.go: IdleTimeout is set
    to 1ms, causing the h2c connection to be closed before PCF can deliver
    the notify callback. Should be changed to a reasonable value (e.g. 30 or 0).

  2. free5gc/gtp5gsrc/genl/genl_qer.c: missing rcu_read_unlock()
    and rtnl_unlock() in the QER UPDATE path causes kernel deadlock on the
    second UpdateQER call, making UPF completely unresponsive. A separate
    bug fix PR will be submitted to free5gc/gtp5g.

When SMF receives SmPolicyUpdateNotification from PCF with updated
session-AMBR, propagate the change to UPF via PFCP SessionModification
by updating the corresponding QER (MBR_UL/MBR_DL).

Previously, PCF AMBR notification was processed by SMF but the QER
was never updated, so UPF rate limiting remained at the original value.

Changes:
- sm_context_policy.go: add updateAMBRQer() to find and update AMBR QER
- build.go: add qerToUpdateQER() and handle RULE_UPDATE in
  BuildPfcpSessionModificationRequest()

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds end-to-end support for enforcing PCF-triggered session-AMBR changes by updating UPF rate limiting via PFCP Session Modification (UpdateQER), aligning SMF behavior with 3GPP TS 29.512 expectations.

Changes:

  • Add QER→UpdateQER conversion and include UpdateQER IEs in PFCP SessionModificationRequest when QER state is RULE_UPDATE.
  • Update SM policy application flow to detect session-AMBR changes and mark the AMBR QER(s) for PFCP update.
  • Introduce AMBR QER update helper that walks activated datapaths and updates QER MBR values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
internal/pfcp/message/build.go Adds qerToUpdateQER() and emits UpdateQER during PFCP session modification when QER state is RULE_UPDATE.
internal/context/sm_context_policy.go Parses updated session-AMBR from the selected session rule and updates/marks AMBR QERs as RULE_UPDATE for the next PFCP modification.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +80 to +93
visited := make(map[uint32]bool) // avoid updating the same QER twice
for _, dataPath := range c.Tunnel.DataPathPool {
if !dataPath.Activated {
continue
}
for node := dataPath.FirstDPNode; node != nil; node = node.Next() {
qerID, ok := c.AMBRQerMap[node.UPF.uuid]
if !ok {
continue
}
if visited[qerID] {
continue
}
visited[qerID] = true

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

visited is keyed only by qerID, but QER IDs are allocated per-UPF and can collide across different UPFs (e.g., both UPF1 and UPF2 may have QERID=1). This can cause AMBR QER updates to be skipped on subsequent UPFs. Key the visited map by UPF UUID (or a composite key of UPF UUID + QERID), or remove visited and instead ensure each UPF is updated once per call.

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +106
qer.MBR = &pfcpType.MBR{
ULMBR: ulMbrkbps,
DLMBR: dlMbrkbps,
}
qer.State = RULE_UPDATE
c.Log.Infof("updateAMBRQer: QER[%d] updated to UL=%d DL=%d kbps, state=RULE_UPDATE",
qerID, ulMbrkbps, dlMbrkbps)
}

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateAMBRQer always overwrites qer.MBR and forces qer.State = RULE_UPDATE even when the requested UL/DL MBR values are unchanged. This can trigger unnecessary PFCP SessionModificationRequests (and extra UpdateQER calls) on every policy update. Consider comparing the existing qer.MBR values first and only marking RULE_UPDATE (and logging) when UL/DL actually change.

Copilot uses AI. Check for mistakes.
Comment on lines 577 to 585
for _, qer := range qerList {
if qer.State == context.RULE_INITIAL {
switch qer.State {
case context.RULE_INITIAL:
msg.CreateQER = append(msg.CreateQER, qerToCreateQER(qer))
case context.RULE_UPDATE:
msg.UpdateQER = append(msg.UpdateQER, qerToUpdateQER(qer))
}
qer.State = context.RULE_CREATE
}

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new RULE_UPDATE handling for QERs is not covered by the existing PFCP message builder unit tests. Add a test case that sets a QER to RULE_UPDATE with a non-nil MBR, calls BuildPfcpSessionModificationRequest, and asserts that UpdateQER contains exactly one entry with the expected QERID/MBR fields (and that the QER state transitions to RULE_CREATE).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants